Re: [PATCH v4 02/14] mfd: lm3533: Remove driver specific regmap wrappers

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Sat, Jun 06, 2026 at 07:57:26AM +0300, Svyatoslav Ryhel wrote:
> 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.

...

>  {
>  	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.

...

>  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

	struct regmap *map = als->lm3533->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.

...

>  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);

>  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.

-- 
With Best Regards,
Andy Shevchenko