Re: [PATCH v5 05/14] iio: light: lm3533-als: Remove redundant pdata helpers

[Johan Hovold]

On Tue, Jul 14, 2026 at 04:31:08PM +0300, Svyatoslav Ryhel wrote:
> пт, 3 лип. 2026 р. о 12:55 Johan Hovold <johan@xxxxxxxxxx> пише:
> >
> > On Wed, Jun 17, 2026 at 11:00:22AM +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.
> >
> > That's a debatable claim.
> >
> 
> Adding helpers to wrap custom regmap wrappers seems redundant twice.
> 
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
> > > ---
> > >  drivers/iio/light/lm3533-als.c | 56 ++++++++++------------------------
> > >  1 file changed, 16 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c
> > > index a9af8e2b965f..69bac1b202f1 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)
> > > -{
> > > -     int ret;
> > > -
> > > -     ret = lm3533_als_set_input_mode(als, pdata->pwm_mode);
> > > +                        pdata->r_select);
> > >       if (ret)
> > > -             return ret;
> > > -
> > > -     /* ALS input is always high impedance in PWM-mode. */
> > > -     if (!pdata->pwm_mode) {
> > > -             ret = lm3533_als_set_resistor(als, pdata->r_select);
> > > -             if (ret)
> > > -                     return ret;
> > > -     }
> > > +             return dev_err_probe(dev, ret, "failed to set resistor\n");
> > >       return 0;
> > >  }
> >
> > There's nothing hard to read about the above. To the contrary the logic
> > is more obvious this way.
> >
> > You also remove the high-impedance comment for no good reason.
> >
> 
> Removing one level of indentations without obscuring readability is
> always a good idea.

Again, no. You're obscuring the current logic which is perfectly fine as
is.

Johan