lkml.org 
[lkml]   [2022]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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;
> +}
> +

> +

\
 
 \ /
  Last update: 2022-05-07 18:51    [W:0.065 / U:1.512 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site