Messages in this thread | | | Date | Sat, 7 May 2022 17:59:12 +0100 | From | Jonathan Cameron <> | Subject | Re: [PATCH v3 3/3] iio: light: Add support for ltrf216a sensor |
| |
On Tue, 3 May 2022 20:13:54 +0530 Shreeya Patel <shreeya.patel@collabora.com> wrote:
> From: Zhigang Shi <Zhigang.Shi@liteon.com> > > Add initial support for ltrf216a ambient light sensor. > > Datasheet: gitlab.steamos.cloud/shreeya/iio/-/blob/main/LTRF216A.pdf > Co-developed-by: Shreeya Patel <shreeya.patel@collabora.com> > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com> > Signed-off-by: Zhigang Shi <Zhigang.Shi@liteon.com>
One locking bug, otherwise just the lock documentation to resolve.
Thanks,
Jonathan
> + > +struct ltrf216a_data { > + struct i2c_client *client; > + u32 int_time; > + u16 int_time_fac; > + u8 als_gain_fac; > + struct mutex mutex; /* Protect read and write operations */
See other branch of thread for feedback on that comment
Thanks for highlighting that btw. I'd forgotten about it so might have missed it on a fresh read through.
> +}; > + > +/* open air. need to update based on TP transmission rate. */ > +#define WIN_FAC 1 > +
> + > +static int ltrf216a_read_data(struct ltrf216a_data *data, u8 addr) > +{ > + int i, ret = -1, tries = 25; > + u8 buf[3]; > + > + while (tries--) { > + ret = i2c_smbus_read_byte_data(data->client, LTRF216A_MAIN_STATUS); > + if (ret < 0) > + return ret; > + if (ret & LTRF216A_ALS_DATA_STATUS) > + break; > + msleep(20); > + } > + > + for (i = 0; i < 3; i++) { > + ret = i2c_smbus_read_byte_data(data->client, addr);
Might be worth seeing if the device copes with i2c_smbus_read_i2c_block_data() The datasheet doesn't mention it though so it might well not work.
> + if (ret < 0) > + return ret; > + buf[i] = ret; > + addr++; > + } > + > + return get_unaligned_le24(&buf[0]); > +}
> + > +static int ltrf216a_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int *val, > + int *val2, long mask) > +{ > + int ret; > + struct ltrf216a_data *data = iio_priv(indio_dev); > + > + mutex_lock(&data->mutex); > + > + switch (mask) { > + case IIO_CHAN_INFO_PROCESSED: > + ret = ltrf216a_get_lux(data); > + if (ret < 0) > + return ret;
Check your locking. This path doesn't unlock the mutex.
> + *val = ret; > + ret = IIO_VAL_INT; > + break; > + case IIO_CHAN_INFO_INT_TIME: > + ret = ltrf216a_get_int_time(data, val, val2); > + break; > + default: > + ret = -EINVAL; > + } > + > + mutex_unlock(&data->mutex); > + > + return ret; > +} > +
> +
| |