On Sat, Jun 06, 2026 at 10:22:43AM +0300, Svyatoslav Ryhel wrote:
> сб, 6 черв. 2026 р. о 09:53 Andy Shevchenko <[email protected]> 
> пише:
> > On Sat, Jun 06, 2026 at 07:57:26AM +0300, Svyatoslav Ryhel wrote:

...

> > > +     ret = regmap_assign_bits(als->lm3533->regmap, 
> > > LM3533_REG_ALS_ZONE_INFO,
> > > +                              LM3533_ALS_INT_ENABLE_MASK, enable);
> >
> > In cases like this perhaps leaving mask would be fine and together with
> 
> I prefer to remove intermediate variables it the helper allows to
> directly pass needed value.
> 
> >         struct regmap *map = als->lm3533->regmap;
> 
> next patch drops lm3533 so there will be als->regmap which IMHO is
> more logical instead of passing entire lm3533 to child devices.

Still it's longer than map. A local variable may help with making lines
shorter.

> > this be nice one-liner:
> >
> >         ret = regmap_assign_bits(map, LM3533_REG_ALS_ZONE_INFO, mask, 
> > enable);
> >
> > >       if (ret) {
> > >               dev_err(&indio_dev->dev, "failed to set int mode %d\n",
> > >                                                               enable);
> >
> > In many cases it won't increase LoC count.

-- 
With Best Regards,
Andy Shevchenko


Reply via email to