linux-kernel - Re: [PATCH 2/2] iio: light: opt3001: Add Support for opt3004 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:
 <PN0P287MB2843562E6C965196D05E2BCFFF0C2@PN0P287MB2843.INDP287.PROD.OUTLOOK.COM>
Date: Wed, 25 Dec 2024 09:56:36 +0000
From: Hardevsinh Palaniya <hardevsinh.palaniya@...iconsignals.io>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC: "jic23@...nel.org" <jic23@...nel.org>, Lars-Peter Clausen
	<lars@...afoo.de>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
	<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Emil Gedenryd
	<emil.gedenryd@...s.com>, Javier Carrasco <javier.carrasco.cruz@...il.com>,
	Arthur Becker <arthur.becker@...tec.com>, Mudit Sharma
	<muditsharma.info@...il.com>, Subhajit Ghosh <subhajit.ghosh@...aklogic.com>,
	Julien Stephan <jstephan@...libre.com>, Uwe Kleine-König
	<u.kleine-koenig@...gutronix.de>, Andreas Dannenberg <dannenberg@...com>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] iio: light: opt3001: Add Support for opt3004 light
 sensor

Hi Andy,

Thanks for the reviews 

> On Tue, Dec 24, 2024 at 11:43:16AM +0530, Hardevsinh Palaniya wrote:
> > Add Support for OPT3004 Digital ambient light sensor (ALS) with
> > increased angular IR rejection
>
> Missing period here.
> 
> > The OPT3004 sensor shares the same functionality and scale range as
> > the OPT3001. This Adds the compatible string for OPT3004, enabling
> > the driver to support it without any functional changes.
> >
> > Datasheet: https://www.ti.com/lit/gpn/opt3004
>
> >
> 
> This blank line is not needed.
>
> > Tested-by: Hardevsinh Palaniya <hardevsinh.palaniya@...iconsignals.io>
>
> This tag is superfluous, it's assumed that author testing their code.

okay , i will remove 

> > Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@...iconsignals.io>
>
> ...
>
> >       help
> >         If you say Y or M here, you get support for Texas Instruments
> > -       OPT3001 Ambient Light Sensor, OPT3002 Light-to-Digital Sensor.
> > +       OPT3001 Ambient Light Sensor, OPT3002 Light-to-Digital Sensor,
> > +       OPT3004 Digital ambient light sensor.
> 
> Can you rather convert this to a list (one item per line)?
> 
>          - OPT3001 Ambient Light Sensor
>          - OPT3002 Light-to-Digital Sensor
>          - OPT3004 Digital ambient light sensor

Sure , i will 

> >  static const struct of_device_id opt3001_of_match[] = {
> >       { .compatible = "ti,opt3001", .data = &opt3001_chip_information },
> >       { .compatible = "ti,opt3002", .data = &opt3002_chip_information },
> > +     { .compatible = "ti,opt3004", .data = &opt3001_chip_information },
> >       { }
> >  };
> 
> I'm always puzzled why do we need a new compatible for the existing driver
> data? Is this hardware has an additional feature that driver does not (yet)
> implement?

OPT3001 and OPT3004 sensors are functionally identical, and there are no
additional features in the OPT3004 that require separate handling in the driver.

The new compatible string for the OPT3004 is being added, which will allow the
driver to recognize and support this sensor in the same way it handles the OPT3001.

Best Regards,
Hardev