Re: [PATCH v3 03/11] iio: light: lm3533-als: Remove redundant pdata helpers

[Jonathan Cameron <jic23@kernel.org>]

On Mon,  1 Jun 2026 18:18:23 +0300
Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> The lm3533_als_set_input_mode and lm3533_als_set_resistor functions are
> used only in lm3533_als_setup. Incorporate their code into
> lm3533_als_setup directly to simplify driver readability.
Minor stuff inline.


> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/iio/light/lm3533-als.c | 61 +++++++++-------------------------
>  1 file changed, 16 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> index fb61904f110f..52136ca1abc9 100644
> --- a/drivers/iio/light/lm3533-als.c
> +++ b/drivers/iio/light/lm3533-als.c

>  static int lm3533_als_setup(struct lm3533_als *als,
>  			    const struct lm3533_als_platform_data *pdata)
>  {
> +	struct device *dev = &als->pdev->dev;
>  	int ret;
>  
> -	ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> +	ret = regmap_update_bits(als->lm3533->regmap, LM3533_REG_ALS_CONF,

Maybe a local struct regmap pointer given dereferenced in a couple of places.

> +				 LM3533_ALS_INPUT_MODE_MASK,
> +				 pdata->pwm_mode ? LM3533_ALS_INPUT_MODE_MASK : 0);

Andy raised this in previous patch but in the interests of being specific
regmap_assign_bits() is going to be cleaner here.

>  	if (ret)
> -		return ret;
> +		return dev_err_probe(dev, ret, "failed to set input mode %d\n",
> +				     pdata->pwm_mode);
> +
>  
>  	/* ALS input is always high impedance in PWM-mode. */
>  	if (!pdata->pwm_mode) {
> -		ret = lm3533_als_set_resistor(als, pdata->r_select);
> +		if (pdata->r_select < LM3533_ALS_RESISTOR_MIN ||
> +		    pdata->r_select > LM3533_ALS_RESISTOR_MAX) {
> +			dev_err(&als->pdev->dev, "invalid resistor value\n");
> +			return -EINVAL;
> +		}
> +
> +		ret = regmap_write(als->lm3533->regmap, LM3533_REG_ALS_RESISTOR_SELECT,
> +				   pdata->r_select);
>  		if (ret)
> -			return ret;
> +			return dev_err_probe(dev, ret, "failed to set resistor\n");
>  	}
>  
>  	return 0;