Re: [PATCH v4 02/14] mfd: lm3533: Remove driver specific regmap wrappers
Svyatoslav Ryhel <[email protected]>
| Newsgroups | org.kernel.vger.linux-fbdev,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-iio,org.kernel.vger.linux-kernel,org.kernel.vger.linux-leds |
|---|---|
| Message-ID | <CAPVz0n1fHWCpHN93v-dKfzGYhvB-dC1E3JgQmXpvfr6bGNLqcA@mail.gmail.com> |
вт, 9 черв. 2026 р. о 22:02 Andy Shevchenko <[email protected]> пише: > > 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. > Introducing and assigning a local variable for a single use regmap operation seems excessive especially since next patch removes operator chaining leaving only als->regmap. > > > 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 > >