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

Use func() when referring to a function in the commit message.

...

>  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_assign_bits(als->regmap, LM3533_REG_ALS_CONF,
> +                              LM3533_ALS_INPUT_MODE_MASK, pdata->pwm_mode);
>       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)
> +                     return dev_err_probe(dev, -EINVAL,
> +                                          "invalid resistor value\n");
> +
> +             ret = regmap_write(als->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;

Wondering if it would be better to

        /* Bail out when in PWM-mode */
        if (pdata->pwm_mode)
                return 0;

        /* ALS input is always high impedance in PWM-mode. */
        ...

as the above changes almost every line in that conditional.

-- 
With Best Regards,
Andy Shevchenko


Reply via email to