Re: [PATCH 2/3] iio: light: add support for veml6046x00 RGBIR color sensor

From: Jonathan Cameron
Date: Sun Mar 23 2025 - 07:51:47 EST


> > > + data = iio_priv(iio);
> > > + i2c_set_clientdata(i2c, iio);
> > > + data->dev = dev;
> > > + data->regmap = regmap;
> > > +
> > > + ret = veml6046x00_regfield_init(data);
> > > + if (ret)
> > > + return dev_err_probe(dev, ret, "Failed to init regfield\n");
> > > +
> > > + ret = devm_regulator_get_enable(dev, "vdd");
> > > + if (ret)
> > > + return dev_err_probe(dev, ret, "Failed to enable regulator\n");
> > > +
> > > + ret = devm_add_action_or_reset(dev, veml6046x00_shutdown_action, data);
> >
> > Mostly we want a devm action to match against a specific setup operation. Here is
> > it that the device comes up in non shut down state? Perhaps a comment to
> > make it clear. Also, how do we know it's in a good state rather than part
> > configured by someone else? I'm not seeing a reset sequence though perhaps
> > that effectively happens in setup_device()
>
> In veml6046x00_setup_device() all registers are set up to bring the device in a
> known state. This function also switches the device on. I could move the call to
> setup_device() up to here and add a comment to make it clear.

Perfect.