Re: [PATCH v4 2/2] iio: light: ROHM BH1745 colour sensor

[Matti Vaittinen]

la 8. kesäk. 2024 klo 19.22 Jonathan Cameron (jic23@xxxxxxxxxx) kirjoitti:
>
> On Thu,  6 Jun 2024 17:29:42 +0100
> Mudit Sharma <muditsharma.info@xxxxxxxxx> wrote:
>
> > Add support for BH1745, which is an I2C colour sensor with red, green,
> > blue and clear channels. It has a programmable active low interrupt
> > pin. Interrupt occurs when the signal from the selected interrupt
> > source channel crosses set interrupt threshold high or low level.
> >
> > This driver includes device attributes to configure the following:
> > - Interrupt pin latch: The interrupt pin can be configured to
> >   be latched (until interrupt register (0x60) is read or initialized)
> >   or update after each measurement.
> > - Interrupt source: The colour channel that will cause the interrupt
> >   when channel will cross the set threshold high or low level.
> >
> > This driver also includes device attributes to present valid
> > configuration options/values for:
> > - Integration time
> > - Interrupt colour source
> > - Hardware gain
> >

> > +
> > +#define BH1745_CHANNEL(_colour, _si, _addr)                                   \
> > +     {                                                                     \
> > +             .type = IIO_INTENSITY, .modified = 1,                         \
> > +             .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),                 \
> > +             .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_HARDWAREGAIN) | \
>
> Provide _SCALE instead of HARDWAREGAIN
> As it's an intensity channel (and units are tricky for color sensors given
> frequency dependence etc) all you need to do is ensure that if you halve
> the _scale and measure the same light source, the computed
> _RAW * _SCALE value remains constant.

...Which is likely to cause also the integration time setting to
impact the SCALE.

You may or may not want to see the GTS-helpers
(drivers/iio/industrialio-gts-helper.c) - which have their own tricky
corners. I think Jonathan once suggested to me to keep the
HARDWAREGAIN as a read-only attribute to ease seeing what is going on.
For the last couple of days I've been reworking the BU27034 driver to
work with the new sensor variant - and I can definitely see the value
of the read-only HARDWAREGAIN when we have per channel gain settings +
integration time setting which all contribute to the scale...


Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Discuss - Estimate - Plan - Report and finally accomplish this:
void do_work(int time) __attribute__ ((const));