Re: [PATCH 2/3] iio: light: opt3001: add support for TI's opt3002 light sensor

[Emil Gedenryd]

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