linux-kernel - Re: [PATCH 2/3] iio: light: opt3001: add support for TI's opt3002 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] [day] [month] [year] [list]

Message-ID: <7c08bcb758d032d3fe37afb18fa31d052026006d.camel@axis.com>
Date: Fri, 13 Sep 2024 07:18:49 +0000
From: Emil Gedenryd <Emil.Gedenryd@...s.com>
To: "jic23@...nel.org" <jic23@...nel.org>
CC: "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, "robh@...nel.org"
	<robh@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "dannenberg@...com" <dannenberg@...com>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "lars@...afoo.de"
	<lars@...afoo.de>, "conor+dt@...nel.org" <conor+dt@...nel.org>, Kernel
	<Kernel@...s.com>
Subject: Re: [PATCH 2/3] iio: light: opt3001: add support for TI's opt3002
 light sensor

On Mon, 2024-09-09 at 20:13 +0100, Jonathan Cameron wrote:
> Hi Emil,
> 
> > > > diff --git a/drivers/iio/light/opt3001.c b/drivers/iio/light/opt3001.c
> > > > index 176e54bb48c3..e6098f88dd04 100644
> > > > --- a/drivers/iio/light/opt3001.c
> > > > +++ b/drivers/iio/light/opt3001.c
> > > > @@ -70,6 +70,19 @@
> > > >  #define OPT3001_RESULT_READY_SHORT	150
> > > >  #define OPT3001_RESULT_READY_LONG	1000
> > > >  
> > > > +/* The opt3002 doesn't have a device id register, predefine value instead */
> > > > +#define OPT3002_DEVICE_ID_VALUE		3002  
> > > 
> > > Why?  Just make the code not care about the value for this
> > > device.  Add a flag to the chip info structure to say it doesn't have
> > > one and check that before using it.  
> > 
> > The device id is used to log the model. Should I not log the
> > model for the opt3002 then or should I have the callback just return
> > 3002? I thought it would be cleaner to have the id value as a defined
> > constant instead of a "magic" number in the code. Is there a preferred
> > way of doing it?
> 
> Given the lack of register means you can't check the model, don't
> report one at all. So don't print that message for this
> device.
> 
> For future replies crop out anything that doesn't need a reply.
> Saves a reader having to scroll and potentially miss something
> important!
> 
> Thanks,
> 
> Jonathan

Hi Jonathan,
Sorry for the delay and thank you for the suggestion. I'll do
the finishing touches on version 2 and submit it as soon as possible.

Best regards,
Emil