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

Emil Gedenryd
2024-09-13T07:18:54Z

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