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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <05727e6f-3355-4572-96e4-0b2ac4d8dca7@tweaklogic.com>
Date: Mon, 22 Jan 2024 22:09:19 +1030
From: Subhajit Ghosh <subhajit.ghosh@...aklogic.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: andriy.shevchenko@...ux.intel.com, anshulusr@...il.com,
 conor+dt@...nel.org, devicetree@...r.kernel.org,
 javier.carrasco.cruz@...il.com, jic23@...nel.org,
 krzysztof.kozlowski+dt@...aro.org, lars@...afoo.de,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, marex@...x.de,
 matt@...ostay.sg, mazziesaccount@...il.com, robh+dt@...nel.org,
 stefan.windfeldt-prytz@...s.com
Subject: Re: [PATCH v5 3/3] iio: light: Add support for APDS9306 Light Sensor

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@...lic.gmane.org>
>> ---
> 
> 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