On Mon, 28 Oct 2024 18:14:02 +0100 Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> wrote: > The integration time of the veml6070 depends on an external resistor > (called Rset in the datasheet) and the value configured in the IT > field of the command register, whose supported values are 1/2x, 1x, > 2x and 4x. > > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> Minor thing inline about not necessarily papering over wrong settings in DT. If someone has a genuine out of range value then we don't want to carry on with a default. If it is a dt bug then we have no idea what the correct value is. Either way I think printing a shouty message and failing the probe is the way to go. Jonathan > --- > drivers/iio/light/veml6070.c | 134 ++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 126 insertions(+), 8 deletions(-) > > diff --git a/drivers/iio/light/veml6070.c b/drivers/iio/light/veml6070.c > index d11ae00f61f8..89b3ccb51515 100644 > --- a/drivers/iio/light/veml6070.c > +++ b/drivers/iio/light/veml6070.c > @@ -6,7 +6,7 @@ > * > * IIO driver for VEML6070 (7-bit I2C slave addresses 0x38 and 0x39) > * > - * TODO: integration time, ACK signal > + * TODO: ACK signal > */ > > #include <linux/bitfield.h> > @@ -15,6 +15,7 @@ > #include <linux/mutex.h> > #include <linux/err.h> > #include <linux/delay.h> > +#include <linux/units.h> > > #include <linux/iio/iio.h> > #include <linux/iio/sysfs.h> > @@ -29,18 +30,84 @@ > #define VEML6070_COMMAND_RSRVD BIT(1) /* reserved, set to 1 */ > #define VEML6070_COMMAND_SD BIT(0) /* shutdown mode when set */ > > -#define VEML6070_IT_10 0x01 /* integration time 1x */ > +#define VEML6070_IT_05 0x00 > +#define VEML6070_IT_10 0x01 > +#define VEML6070_IT_20 0x02 > +#define VEML6070_IT_40 0x03 > + > +#define VEML6070_MIN_RSET_KOHM 75 > +#define VEML6070_MIN_IT_US 15625 /* Rset = 75 kohm, IT = 1/2 */ > > struct veml6070_data { > struct i2c_client *client1; > struct i2c_client *client2; > u8 config; > struct mutex lock; > + u32 rset; > + int it[4][2]; > }; > > +static void veml6070_calc_it(struct device *dev, struct veml6070_data *data) > +{ > + int i, tmp_it; > + > + data->rset = 270000; > + device_property_read_u32(dev, "rset-ohms", &data->rset); > + > + if (data->rset < 75000) { > + dev_warn(dev, "Rset too low, using minimum = 75 kohms\n"); > + data->rset = 75000; Why are we papering over a DT issue? Are there known DT out there that hit this we want to keep working? If not I'd just error out on out of range values - that way they get fixed faster! > + } > + > + if (data->rset > 1200000) { > + dev_warn(dev, "Rset too high, using maximum = 1200 kohms\n"); > + data->rset = 1200000; > + } > + > + /* > + * convert to kohm to avoid overflows and work with the same units as > + * in the datasheet and simplify UVI operations. > + */ > + data->rset /= KILO; > + > + tmp_it = VEML6070_MIN_IT_US * data->rset / VEML6070_MIN_RSET_KOHM; > + for (i = 0; i < ARRAY_SIZE(data->it); i++) { > + data->it[i][0] = (tmp_it << i) / MICRO; > + data->it[i][1] = (tmp_it << i) % MICRO; > + } > +}