linux-kernel - Re: [PATCH v4 2/2] iio: light: ROHM BH1745 colour 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANhJrGM9czj0RL3OLCgRHEKc2QOjG9P0AZTrZxvYUk65TCpHRg@mail.gmail.com>
Date: Mon, 10 Jun 2024 08:58:44 +0300
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Mudit Sharma <muditsharma.info@...il.com>, lars@...afoo.de, krzk+dt@...nel.org, 
	conor+dt@...nel.org, robh@...nel.org, ivan.orlov0322@...il.com, 
	javier.carrasco.cruz@...il.com, linux-kernel@...r.kernel.org, 
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v4 2/2] iio: light: ROHM BH1745 colour sensor

la 8. kesäk. 2024 klo 19.22 Jonathan Cameron (jic23@...nel.org) kirjoitti:
>
> On Thu,  6 Jun 2024 17:29:42 +0100
> Mudit Sharma <muditsharma.info@...il.com> 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));