сб, 6 черв. 2026 р. о 09:53 Andy Shevchenko [off-list ref] пише:

On Sat, Jun 06, 2026 at 07:57:26AM +0300, Svyatoslav Ryhel wrote:

quoted

Remove driver-specific regmap wrappers in favor of using regmap helpers
directly.

I like the idea of this patch. Nevertheless I have some suggestions below.

...

quoted

 {
      struct lm3533_als *als = iio_priv(indio_dev);
      u8 reg;
-     u8 val;
+     u32 val;

Strictly speaking this should be unsigned int. The regmap API use unsigned int.

Yes, though regmap defines only 8 bit to be used so it should not matter

...

quoted

 static int lm3533_als_set_int_mode(struct iio_dev *indio_dev, int enable)
 {
      struct lm3533_als *als = iio_priv(indio_dev);
-     u8 mask = LM3533_ALS_INT_ENABLE_MASK;
-     u8 val;
      int ret;

-     if (enable)
-             val = mask;
-     else
-             val = 0;
-
-     ret = lm3533_update(als->lm3533, LM3533_REG_ALS_ZONE_INFO, val, mask);
+     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.

this be nice one-liner:

        ret = regmap_assign_bits(map, LM3533_REG_ALS_ZONE_INFO, mask, enable);

quoted

      if (ret) {
              dev_err(&indio_dev->dev, "failed to set int mode %d\n",
                                                              enable);

In many cases it won't increase LoC count.

...

quoted

 extern int lm3533_ctrlbank_set_brightness(struct lm3533_ctrlbank *cb, u8 val);
-extern int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u8 *val);
+extern int lm3533_ctrlbank_get_brightness(struct lm3533_ctrlbank *cb, u32 *val);

quoted

 extern int lm3533_ctrlbank_set_pwm(struct lm3533_ctrlbank *cb, u8 val);
-extern int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u8 *val);
+extern int lm3533_ctrlbank_get_pwm(struct lm3533_ctrlbank *cb, u32 *val);

Now they become asymmetrical. Perhaps to replace setters, but be careful about
upper bits.

Yes, I have same thoughts. Upper beats should be irrelevant since
regmap should use only fist 8 bits but I will be careful, thanks.

--
With Best Regards,
Andy Shevchenko