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

From: Jonathan Cameron
Date: Mon Sep 09 2024 - 15:14:05 EST


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