Messages in this thread |  | | From | Andy Shevchenko <> | Date | Tue, 31 Mar 2020 14:00:55 +0300 | Subject | Re: [PATCH v5 2/2] iio: proximity: Add driver support for vcnl3020 proximity sensor |
| |
On Mon, Mar 30, 2020 at 11:41 PM Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote: > On Mon, 2020-03-30 at 22:07 +0300, Andy Shevchenko wrote: > > On Mon, Mar 30, 2020 at 6:27 PM Ivan Mikhaylov <i.mikhaylov@yadro.com> wrote:
...
> > > +#define VCNL_DRV_NAME "vcnl3020" > > > +#define VCNL_REGMAP_NAME "vcnl3020_regmap" > > > > I'm wondering why you need the second one. > > For regmap initialize as name in > static const struct regmap_config vcnl3020_regmap_config = { > .name = VCNL_REGMAP_NAME, > > I can get rid of it from struct with name definition.
I don't think we need a specific suffix. When somebody will look at it they will already know that they are looking into regmap realm.
...
> > > + rc = device_property_read_u32(data->dev, "vishay,led-current- > > > milliamp", > > > + &led_current); > > > + if (rc == 0) { > > > + rc = regmap_write(data->regmap, VCNL_LED_CURRENT, > > > led_current); > > > + if (rc) > > > + dev_err(data->dev, > > > + "Error (%d) setting LED current", rc); > > > + } > > > + > > > + return rc; > > > > Why not to use standard pattern, i.e. > > > > if (rc) > > return rc; > > ... > > rc = regmap_write(...); > > > > ? > > Optional parameter. There exists a lot of ways to do it:
I'm simple reading the code. And I believe the above I suggested is cleaner equivalent. Is it?
> rc = device_property_read_u32(dev, "milliamp", &led_current); > rc = regmap_write(regmap, VCNL_LED_CURRENT, (!rc) : led_current ? 0);
This seems not equal to above.
> Which one would be more preferable?
One which has better readability and smallest indentation level.
-- With Best Regards, Andy Shevchenko
|  |