On Fri, 13 Sep 2024 11:57:04 +0200 Emil Gedenryd <emil.gedenryd@xxxxxxxx> wrote: > TI's opt3002 light sensor shares most properties with the opt3001 > model, with the exception of supporting a wider spectrum range. > > Add support for TI's opt3002 by extending the TI opt3001 driver. > > Datasheet: https://www.ti.com/product/OPT3002 > No blank line here. Datasheet: should be part of this tags block. > Signed-off-by: Emil Gedenryd <emil.gedenryd@xxxxxxxx> > --- > drivers/iio/light/Kconfig | 2 +- > drivers/iio/light/opt3001.c | 169 +++++++++++++++++++++++++++++++++++--------- > 2 files changed, 138 insertions(+), 33 deletions(-) > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > index b68dcc1fbaca..c35bf962dae6 100644 > --- a/drivers/iio/light/Kconfig > +++ b/drivers/iio/light/Kconfig > @@ -461,7 +461,7 @@ config OPT3001 > depends on I2C > help > If you say Y or M here, you get support for Texas Instruments > - OPT3001 Ambient Light Sensor. > + OPT3001 Ambient Light Sensor, OPT3002 Light-to-Digital Sensor. > > If built as a dynamically linked module, it will be called > opt3001. > diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c > index 176e54bb48c3..83c49f4517b7 100644 > --- a/drivers/iio/light/opt3001.c > +++ b/drivers/iio/light/opt3001.c > @@ -70,6 +70,21 @@ > #define OPT3001_RESULT_READY_SHORT 150 > #define OPT3001_RESULT_READY_LONG 1000 > > +struct opt3001_scale { > + int val; > + int val2; > +}; > + > +struct opt300x_chip_info { Don't use wild cards. Just call it opt3001_chip_info. Wild cards tend to bite us as manufacturers have an 'amusing' habit of filling in gaps with unrelated devices. > + const struct iio_chan_spec (*channels)[2]; > + enum iio_chan_type chan_type; > + const struct opt3001_scale (*scales)[12]; > + int factor_whole; > + int factor_integer; > + int factor_decimal; These three aren't obvious when just looking to fill in this structure. Add some docs to hint at what they are. > + bool has_id; > +}; > @@ -610,7 +690,7 @@ static int opt3001_read_id(struct opt3001 *opt) > ret = i2c_smbus_read_word_swapped(opt->client, OPT3001_DEVICE_ID); > if (ret < 0) { > dev_err(opt->dev, "failed to read register %02x\n", > - OPT3001_DEVICE_ID); > + OPT3001_DEVICE_ID); In general whitespace only changes belong in their own patch, but I guess this one is pretty minor so we can skip that requirement this time. > return ret; > } > @@ -746,7 +827,7 @@ static int opt3001_probe(struct i2c_client *client) > struct iio_dev *iio; > struct opt3001 *opt; > int irq = client->irq; > - int ret; > + int ret = 0; > > iio = devm_iio_device_alloc(dev, sizeof(*opt)); > if (!iio) > @@ -755,12 +836,14 @@ static int opt3001_probe(struct i2c_client *client) > opt = iio_priv(iio); > opt->client = client; > opt->dev = dev; > + opt->chip_info = device_get_match_data(&client->dev); > > mutex_init(&opt->lock); > init_waitqueue_head(&opt->result_ready_queue); > i2c_set_clientdata(client, iio); > > - ret = opt3001_read_id(opt); > + if (opt->chip_info->has_id) > + ret = opt3001_read_id(opt); > if (ret) > return ret; > Only check the ret if the function ran. That way no need for the dance with ret = 0 above. > +static const struct iio_chan_spec opt3002_channels[] = { > + { > + .type = IIO_INTENSITY, > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | > + BIT(IIO_CHAN_INFO_INT_TIME), Generally intensity channels can't be processed because there are no defined units as what you measure depends entirely on the frequency sensitivity. There are some defined measurements such as illuminance that use a specific sensivitiy curve, but if it's just intensity we normally stick to _RAW.. > + .event_spec = opt3001_event_spec, > + .num_event_specs = ARRAY_SIZE(opt3001_event_spec), > + }, > + IIO_CHAN_SOFT_TIMESTAMP(1), > +}; > +
