Re: [PATCH v5 3/3] iio: light: Add support for APDS9306 Light Sensor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 21/1/24 19:52, Christophe JAILLET wrote:
Le 21/01/2024 à 06:17, Subhajit Ghosh a écrit :
Driver support for Avago (Broadcom) APDS9306 Ambient Light Sensor.
It has two channels - ALS and CLEAR. The ALS (Ambient Light Sensor)
channel approximates the response of the human-eye providing direct
read out where the output count is proportional to ambient light levels.
It is internally temperature compensated and rejects 50Hz and 60Hz flicker
caused by artificial light sources. Hardware interrupt configuration is
optional. It is a low power device with 20 bit resolution and has
configurable adaptive interrupt mode and interrupt persistence mode.
The device also features inbuilt hardware gain, multiple integration time
selection options and sampling frequency selection options.

This driver also uses the IIO GTS (Gain Time Scale) Helpers Namespace for
Scales, Gains and Integration time implementation.

Signed-off-by: Subhajit Ghosh <subhajit.ghosh-ojZBjWEdjYKukZHgTAicrQ@xxxxxxxxxxxxxxxx>
---

Hi,

a few nits and a few real comment/question below.

Just my 2c.

CJ
...

+
+static int apds9306_read_event(struct iio_dev *indio_dev,
+                   const struct iio_chan_spec *chan,
+                   enum iio_event_type type,
+                   enum iio_event_direction dir,
+                   enum iio_event_info info,
+                   int *val, int *val2)
+{
+    struct apds9306_data *data = iio_priv(indio_dev);
+    int ret;

Other functions below that look really similar have a:
    guard(mutex)(&data->mutex);

Is it needed here?
You are right, don't think so. Regmap lock is being used.
I will review the locking mechanism. Acknowledging all other comments.

Thanks for the review.

Regards,
Subhajit Ghosh





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux