All of lore.kernel.org
 help / color / mirror / Atom feed
* ROHM ALS, integration time
@ 2023-01-30 12:04 Matti Vaittinen
  2023-01-30 13:02 ` Jonathan Cameron
  0 siblings, 1 reply; 20+ messages in thread
From: Matti Vaittinen @ 2023-01-30 12:04 UTC (permalink / raw)
  To: Vaittinen, Matti, Matti Vaittinen; +Cc: Jonathan Cameron, linux-iio

Hi deee Ho peeps,

I am currently writing drivers for couple of ROHM light sensors. At 
least two RGBC+IR and one ALS. I have some difficulties deciding how 
some of the IIO API values should be mapped to the sensor configs. (So, 
not missing much, right? Basically just THE THING any IIO driver is 
expected to do XD).

Okay, that's for the intro. I'll ask about the ALS first.

=== The Hardware:

The sensor gets the data from 3 photo-diodes - values from each are 
readable via own channel (say, data0, data1, data2).

data0 and data1 have the sensitivity peaks around 500 and 600 nm - which 
is visible light but channels are not really R/G/B. The data2 is 
probably IR - but I am not really 100% sure so I plan to skip it at first.

The channel gain can be set individually,. The sampling time can be set 
globally for all channels.


=== The driver draft I'm working with

I have some kind of formula for converting the channel0 and channel1 
data to lux. So, I thought I'll provide 3 channels from driver - one 
IIO_CHAN_INFO_PROCESSED IIO_LIGHT channel spilling out luxes (with 
appropriate scale) - and two IIO_INTENSITY channels spilling out the raw 
register values so that if greater accuracy is needed one can do better 
algorithms to user-space.

Q1 - does this sound like reasonable option?

Then, I thought I'll support setting the GAIN for channels using the 
IIO_CHAN_INFO_SCALE. Straightforward division / multiplication (I hope).

Eg, for the IIO_INTENSITY channels I though I'll just implement 
raw_write/raw_read for scale so, that raw_read returns 
IIO_VAL_INT_PLUS_NANO - and then for example gain 4x would be
val = 0, val2 = 250 MEGA, 8x would be 0, 1 * GIGA / 8 ...
Eg, val2 for gain values > 1 would be GIGA/gain using IIO_VAL_INT_PLUS_NANO.

I hope this makes sense :)

Well, currently my "not-yet-implemented using C w/o floats" - algorithm 
takes the scale into account and always returns luxes (for 
IIO_CHAN_INFO_PROCESSED IIO_LIGHT - channel). However, the IIO_INTENSITY 
channels return raw data from registers - so setting GAIN (scale) has 
direct impact to the INTENSITY values. I guess this is Ok as the 
IIO_CHAN_INFO_SCALE is only set in .info_mask_separate for the 
IIO_INTENSITY type channels - not for the IIO_CHAN_INFO_PROCESSED 
IIO_LIGHT channel. [Even though I decided to go this route I am still 
somewhat unsure if this is the right thing to do(tm). My problem is that 
in HW level, setting the GAIN does also impact the data based on which 
the IIO_LIGHT values are computed. The scale just is not visible in 
computed values as it is taken into account by the equation. 
Furthermore, setting GAIN(or scale) for IIO_LIGHT would not be 
unambiguous as the IIO_LIGHT is composed from two channels, both having 
own gain settings.]

I hope this is all Ok from interface POV.

Now, finally, my dear persistent readers - the question:
As mentioned, sensor allows setting the sampling time. I thought I'll 
map this to the IIO_CHAN_INFO_INT_TIME. This config is not per/channel 
in the hardware. Again, my lux-computing algorithm takes the integration 
time into account - and changing the time should not be reflected to the 
IIO_LIGHT channel values (other than accuracy). However, the values 
spilled from raw IIO_INTENSITY channels will change when integration 
time is changed. So, should I use the info_mask_shared_by_type = 
BIT(IIO_CHAN_INFO_INT_TIME) for IIO_INTENSITY channels?

Sorry for the long post. I do appreciate all help/pointers on my journey 
to writing my first light sensor drivers ;) And yes, my plan is to send 
out the patches - when I first get the sensor hardware at my hands ;)

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-01-30 12:04 ROHM ALS, integration time Matti Vaittinen
@ 2023-01-30 13:02 ` Jonathan Cameron
  2023-01-30 13:42   ` Vaittinen, Matti
  2023-01-31  9:31   ` Vaittinen, Matti
  0 siblings, 2 replies; 20+ messages in thread
From: Jonathan Cameron @ 2023-01-30 13:02 UTC (permalink / raw)
  To: Matti Vaittinen; +Cc: Vaittinen, Matti, Jonathan Cameron, linux-iio

On Mon, 30 Jan 2023 14:04:53 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Hi deee Ho peeps,
> 
> I am currently writing drivers for couple of ROHM light sensors. At 
> least two RGBC+IR and one ALS. I have some difficulties deciding how 
> some of the IIO API values should be mapped to the sensor configs. (So, 
> not missing much, right? Basically just THE THING any IIO driver is 
> expected to do XD).
> 
> Okay, that's for the intro. I'll ask about the ALS first.
> 
> === The Hardware:
> 
> The sensor gets the data from 3 photo-diodes - values from each are 
> readable via own channel (say, data0, data1, data2).
> 
> data0 and data1 have the sensitivity peaks around 500 and 600 nm - which 
> is visible light but channels are not really R/G/B. The data2 is 
> probably IR - but I am not really 100% sure so I plan to skip it at first.

Ah.  The color type interface has always been a big 'clunky'.
We have in the past discussed adding some ABI to describe actual sensitive
ranges but it's tricky as these curves are often complex.  I'm open
to suggestions on how to do this though (as long as you also add it
to a few existing drivers as examples!)

> 
> The channel gain can be set individually,. The sampling time can be set 
> globally for all channels.
> 
> 
> === The driver draft I'm working with
> 
> I have some kind of formula for converting the channel0 and channel1 
> data to lux. So, I thought I'll provide 3 channels from driver - one 
> IIO_CHAN_INFO_PROCESSED IIO_LIGHT channel spilling out luxes (with 
> appropriate scale) - and two IIO_INTENSITY channels spilling out the raw 
> register values so that if greater accuracy is needed one can do better 
> algorithms to user-space.
> 
> Q1 - does this sound like reasonable option?
> 
> Then, I thought I'll support setting the GAIN for channels using the 
> IIO_CHAN_INFO_SCALE. Straightforward division / multiplication (I hope).
> 
> Eg, for the IIO_INTENSITY channels I though I'll just implement 
> raw_write/raw_read for scale so, that raw_read returns 
> IIO_VAL_INT_PLUS_NANO - and then for example gain 4x would be
> val = 0, val2 = 250 MEGA, 8x would be 0, 1 * GIGA / 8 ...
> Eg, val2 for gain values > 1 would be GIGA/gain using IIO_VAL_INT_PLUS_NANO.
> 
> I hope this makes sense :)
> 
> Well, currently my "not-yet-implemented using C w/o floats" - algorithm 
> takes the scale into account and always returns luxes (for 
> IIO_CHAN_INFO_PROCESSED IIO_LIGHT - channel). However, the IIO_INTENSITY 
> channels return raw data from registers - so setting GAIN (scale) has 
> direct impact to the INTENSITY values. I guess this is Ok as the 
> IIO_CHAN_INFO_SCALE is only set in .info_mask_separate for the 
> IIO_INTENSITY type channels - not for the IIO_CHAN_INFO_PROCESSED 
> IIO_LIGHT channel. [Even though I decided to go this route I am still 
> somewhat unsure if this is the right thing to do(tm). My problem is that 
> in HW level, setting the GAIN does also impact the data based on which 
> the IIO_LIGHT values are computed. The scale just is not visible in 
> computed values as it is taken into account by the equation. 
> Furthermore, setting GAIN(or scale) for IIO_LIGHT would not be 
> unambiguous as the IIO_LIGHT is composed from two channels, both having 
> own gain settings.]
> 
> I hope this is all Ok from interface POV.

This matches what I'd expect to see.  

> 
> Now, finally, my dear persistent readers - the question:
> As mentioned, sensor allows setting the sampling time. I thought I'll 
> map this to the IIO_CHAN_INFO_INT_TIME. This config is not per/channel 
> in the hardware. Again, my lux-computing algorithm takes the integration 
> time into account - and changing the time should not be reflected to the 
> IIO_LIGHT channel values (other than accuracy). However, the values 
> spilled from raw IIO_INTENSITY channels will change when integration 
> time is changed. So, should I use the info_mask_shared_by_type = 
> BIT(IIO_CHAN_INFO_INT_TIME) for IIO_INTENSITY channels?

Ah. This problem. The mixture of two things that effectively map to scaling
of raw channels. As _scale must be applied by userspace to the _raw channel
that has to reflect both the result of integration time and a front end amplifier
and is the control typical userspace expects to use to vary the sensitivity.

That makes it messy because it's not always totally obvious whether, when
trying to increase sensitivity, it is better to increase sample time or gain.
Usually you do sample time first as that tends to reduce noise and for light
sensors we rarely need particular quick answers.

So in the interests of keeping things easy to understand for userspace code
you would need to provide writeable _scale that then attempts to find the
best combination of amplifier gain and sampling time.   You can allow read
only access to allow a curious user to find out what was chosen:
INT_TIME for the integration time.
Not really a good option for the amplifier gain though.  I don't like using
HARDWARE_GAIN for this though maybe we could stretch the ABI to cover this
as long as it was read only.

> 
> Sorry for the long post. I do appreciate all help/pointers on my journey 
> to writing my first light sensor drivers ;) And yes, my plan is to send 
> out the patches - when I first get the sensor hardware at my hands ;)
No problem. Light sensors tend to cause us more ABI headaches than almost
anything else (don't get me started on the ones used for blood oxygen level
measurement in which it's a light sensor and a controllable light source).

Jonathan

> 
> Yours,
> 	-- Matti
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-01-30 13:02 ` Jonathan Cameron
@ 2023-01-30 13:42   ` Vaittinen, Matti
  2023-01-30 17:12     ` Jonathan Cameron
  2023-01-31  9:31   ` Vaittinen, Matti
  1 sibling, 1 reply; 20+ messages in thread
From: Vaittinen, Matti @ 2023-01-30 13:42 UTC (permalink / raw)
  To: Jonathan Cameron, Matti Vaittinen; +Cc: Jonathan Cameron, linux-iio

On 1/30/23 15:02, Jonathan Cameron wrote:
> On Mon, 30 Jan 2023 14:04:53 +0200
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:

Hi Jonathan! Thanks for a _very fast_ response! It's nice "chatting" 
with you again!

>>
>> I hope this is all Ok from interface POV.
> 
> This matches what I'd expect to see.

Glad to hear :)

>> Now, finally, my dear persistent readers - the question:
>> As mentioned, sensor allows setting the sampling time. I thought I'll
>> map this to the IIO_CHAN_INFO_INT_TIME. This config is not per/channel
>> in the hardware. Again, my lux-computing algorithm takes the integration
>> time into account - and changing the time should not be reflected to the
>> IIO_LIGHT channel values (other than accuracy). However, the values
>> spilled from raw IIO_INTENSITY channels will change when integration
>> time is changed. So, should I use the info_mask_shared_by_type =
>> BIT(IIO_CHAN_INFO_INT_TIME) for IIO_INTENSITY channels?
> 
> Ah. This problem. The mixture of two things that effectively map to scaling
> of raw channels. As _scale must be applied by userspace to the _raw channel
> that has to reflect both the result of integration time and a front end amplifier

Ouch. I know that this makes perfectly sense. It indeed may not be clear 
how the integration time impacts the scale. Thus, it is very reasonable 
that the driver code should know this and not leave the burden to the 
applications :/ Ouch because now I need to try inventing this logic in 
driver myself ;)

> and is the control typical userspace expects to use to vary the sensitivity.

Yes. Now that you wrote this it seems obvious.

> That makes it messy because it's not always totally obvious whether, when
> trying to increase sensitivity, it is better to increase sample time or gain.
> Usually you do sample time first as that tends to reduce noise and for light
> sensors we rarely need particular quick answers.
> 
> So in the interests of keeping things easy to understand for userspace code
> you would need to provide writeable _scale that then attempts to find the
> best combination of amplifier gain and sampling time.   You can allow read
> only access to allow a curious user to find out what was chosen:
> INT_TIME for the integration time.

Right. This still makes sense.

> Not really a good option for the amplifier gain though.  I don't like using
> HARDWARE_GAIN for this though maybe we could stretch the ABI to cover this
> as long as it was read only.

Well, I don't (yet) have the need for this but it is good to keep on 
mind :) It shouldn't be a big thing to add reading of (hardware)-gain.

>> Sorry for the long post. I do appreciate all help/pointers on my journey
>> to writing my first light sensor drivers ;) And yes, my plan is to send
>> out the patches - when I first get the sensor hardware at my hands ;)
> No problem. Light sensors tend to cause us more ABI headaches than almost
> anything else

Hm. This is the reason why I wanted to ask this right away. I realized 
that we have a kernel<->user ABI once the sysfs starts spilling the 
values from the driver. It wouldn't be such much fun 'fixing' this later on.

  (don't get me started on the ones used for blood oxygen level
> measurement in which it's a light sensor and a controllable light source).

Hmm... I think a colleague of mine did actually one such driver (AFAIR a 
BH<add some numbers here>) not so many years ago ;) Not sure if it went 
upstream though.

For an occasional contributor like me it could be helpful if the defines 
like IIO_INTENSITY, IIO_LIGHT had documentation in headers explaining 
for example the units. Maybe also some words about the 
IIO_CHAN_INFO_INT_TIME and IIO_CHAN_INFO_SCALE as well ;) I guess I can 
cook some doc - but only for couple of defines which I have discussed 
with you this far. Do you think such comment docs would be welcome - 
even if they covered only couple of defines? Maybe others would continue 
from that.

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-01-30 13:42   ` Vaittinen, Matti
@ 2023-01-30 17:12     ` Jonathan Cameron
  2023-01-30 18:19       ` Matti Vaittinen
  0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2023-01-30 17:12 UTC (permalink / raw)
  To: Vaittinen, Matti; +Cc: Matti Vaittinen, Jonathan Cameron, linux-iio

On Mon, 30 Jan 2023 13:42:27 +0000
"Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com> wrote:

> On 1/30/23 15:02, Jonathan Cameron wrote:
> > On Mon, 30 Jan 2023 14:04:53 +0200
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:  
> 
> Hi Jonathan! Thanks for a _very fast_ response! It's nice "chatting" 
> with you again!
> 
> >>
> >> I hope this is all Ok from interface POV.  
> > 
> > This matches what I'd expect to see.  
> 
> Glad to hear :)
> 
> >> Now, finally, my dear persistent readers - the question:
> >> As mentioned, sensor allows setting the sampling time. I thought I'll
> >> map this to the IIO_CHAN_INFO_INT_TIME. This config is not per/channel
> >> in the hardware. Again, my lux-computing algorithm takes the integration
> >> time into account - and changing the time should not be reflected to the
> >> IIO_LIGHT channel values (other than accuracy). However, the values
> >> spilled from raw IIO_INTENSITY channels will change when integration
> >> time is changed. So, should I use the info_mask_shared_by_type =
> >> BIT(IIO_CHAN_INFO_INT_TIME) for IIO_INTENSITY channels?  
> > 
> > Ah. This problem. The mixture of two things that effectively map to scaling
> > of raw channels. As _scale must be applied by userspace to the _raw channel
> > that has to reflect both the result of integration time and a front end amplifier  
> 
> Ouch. I know that this makes perfectly sense. It indeed may not be clear 
> how the integration time impacts the scale. Thus, it is very reasonable 
> that the driver code should know this and not leave the burden to the 
> applications :/ Ouch because now I need to try inventing this logic in 
> driver myself ;)
> 
> > and is the control typical userspace expects to use to vary the sensitivity.  
> 
> Yes. Now that you wrote this it seems obvious.
> 
> > That makes it messy because it's not always totally obvious whether, when
> > trying to increase sensitivity, it is better to increase sample time or gain.
> > Usually you do sample time first as that tends to reduce noise and for light
> > sensors we rarely need particular quick answers.
> > 
> > So in the interests of keeping things easy to understand for userspace code
> > you would need to provide writeable _scale that then attempts to find the
> > best combination of amplifier gain and sampling time.   You can allow read
> > only access to allow a curious user to find out what was chosen:
> > INT_TIME for the integration time.  
> 
> Right. This still makes sense.
> 
> > Not really a good option for the amplifier gain though.  I don't like using
> > HARDWARE_GAIN for this though maybe we could stretch the ABI to cover this
> > as long as it was read only.  
> 
> Well, I don't (yet) have the need for this but it is good to keep on 
> mind :) It shouldn't be a big thing to add reading of (hardware)-gain.
> 
> >> Sorry for the long post. I do appreciate all help/pointers on my journey
> >> to writing my first light sensor drivers ;) And yes, my plan is to send
> >> out the patches - when I first get the sensor hardware at my hands ;)  
> > No problem. Light sensors tend to cause us more ABI headaches than almost
> > anything else  
> 
> Hm. This is the reason why I wanted to ask this right away. I realized 
> that we have a kernel<->user ABI once the sysfs starts spilling the 
> values from the driver. It wouldn't be such much fun 'fixing' this later on.
> 
>   (don't get me started on the ones used for blood oxygen level
> > measurement in which it's a light sensor and a controllable light source).  
> 
> Hmm... I think a colleague of mine did actually one such driver (AFAIR a 
> BH<add some numbers here>) not so many years ago ;) Not sure if it went 
> upstream though.
> 
> For an occasional contributor like me it could be helpful if the defines 
> like IIO_INTENSITY, IIO_LIGHT had documentation in headers explaining 
> for example the units. Maybe also some words about the 
> IIO_CHAN_INFO_INT_TIME and IIO_CHAN_INFO_SCALE as well ;) I guess I can 
> cook some doc - but only for couple of defines which I have discussed 
> with you this far. Do you think such comment docs would be welcome - 
> even if they covered only couple of defines? Maybe others would continue 
> from that.

I'd worry about the Docs disagreeing with the ABI docs
in Documentation/ABI/testing/sysfs-bus-iio
which needs to be the 'one true source' of this stuff.

So might be fine but would need careful consideration of that risk.

Jonathan
> 
> Yours,
> 	-- Matti
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-01-30 17:12     ` Jonathan Cameron
@ 2023-01-30 18:19       ` Matti Vaittinen
  2023-01-30 20:19         ` Jonathan Cameron
  0 siblings, 1 reply; 20+ messages in thread
From: Matti Vaittinen @ 2023-01-30 18:19 UTC (permalink / raw)
  To: Jonathan Cameron, Vaittinen, Matti; +Cc: Jonathan Cameron, linux-iio

On 1/30/23 19:12, Jonathan Cameron wrote:
> On Mon, 30 Jan 2023 13:42:27 +0000
> "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> 
>> On 1/30/23 15:02, Jonathan Cameron wrote:
>>> On Mon, 30 Jan 2023 14:04:53 +0200
>>> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
>> For an occasional contributor like me it could be helpful if the defines
>> like IIO_INTENSITY, IIO_LIGHT had documentation in headers explaining
>> for example the units. Maybe also some words about the
>> IIO_CHAN_INFO_INT_TIME and IIO_CHAN_INFO_SCALE as well ;) I guess I can
>> cook some doc - but only for couple of defines which I have discussed
>> with you this far. Do you think such comment docs would be welcome -
>> even if they covered only couple of defines? Maybe others would continue
>> from that.
> 
> I'd worry about the Docs disagreeing with the ABI docs
> in Documentation/ABI/testing/sysfs-bus-iio
> which needs to be the 'one true source' of this stuff.

Oh, right. It might've been just me - but I did overlook this golden 
documentation. I did actually land on this document but didn't really 
pay the required attention. I guess I allowed the kernel version in the 
page to distract me thinking it is some sort of 'history' stuff.

What would have been enough (even for me) would've been a short 
description of a define - and then the link to a entry which corresponds 
the define in this document. Something along the lines:

/**
  * iio_chan_type - Types of channel
  *
  * Please find the detailed documentation for reported values from the
  * Documentation/ABI/testing/sysfs-bus-iio. Pointer to correct keyword
  * in documentation is mentioned at the channel define description
  * below.
  *
  * IIO_INTENSITY:	Channel for unitless intensity.
  *			Doc keyword: in_intensityY_raw			
  *
  * IIO_LIGHT:		Channel for visible light intensity in lux
  * 			Doc keyword: in_illuminance_raw
  */

I have a feeling that this would already have helped me. Nevertheless, I 
would not mind seeing docs also for the iio_chan_info_enum values - but 
linking to the correct spot in sysfs-bus-iio might not be as easy.

Anyays, Thanks for the help once again :)

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-01-30 18:19       ` Matti Vaittinen
@ 2023-01-30 20:19         ` Jonathan Cameron
  2023-01-31 19:58           ` Jonathan Corbet
  0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2023-01-30 20:19 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Jonathan Cameron, Vaittinen, Matti, linux-iio,
	Mauro Carvalho Chehab, linux-doc, Jonathan Corbet

On Mon, 30 Jan 2023 20:19:07 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 1/30/23 19:12, Jonathan Cameron wrote:
> > On Mon, 30 Jan 2023 13:42:27 +0000
> > "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> >   
> >> On 1/30/23 15:02, Jonathan Cameron wrote:  
> >>> On Mon, 30 Jan 2023 14:04:53 +0200
> >>> Matti Vaittinen <mazziesaccount@gmail.com> wrote:  
> >> For an occasional contributor like me it could be helpful if the defines
> >> like IIO_INTENSITY, IIO_LIGHT had documentation in headers explaining
> >> for example the units. Maybe also some words about the
> >> IIO_CHAN_INFO_INT_TIME and IIO_CHAN_INFO_SCALE as well ;) I guess I can
> >> cook some doc - but only for couple of defines which I have discussed
> >> with you this far. Do you think such comment docs would be welcome -
> >> even if they covered only couple of defines? Maybe others would continue
> >> from that.  
> > 
> > I'd worry about the Docs disagreeing with the ABI docs
> > in Documentation/ABI/testing/sysfs-bus-iio
> > which needs to be the 'one true source' of this stuff.  
> 
> Oh, right. It might've been just me - but I did overlook this golden 
> documentation. I did actually land on this document but didn't really 
> pay the required attention. I guess I allowed the kernel version in the 
> page to distract me thinking it is some sort of 'history' stuff.
> 
> What would have been enough (even for me) would've been a short 
> description of a define - and then the link to a entry which corresponds 
> the define in this document. Something along the lines:
> 
> /**
>   * iio_chan_type - Types of channel
>   *
>   * Please find the detailed documentation for reported values from the
>   * Documentation/ABI/testing/sysfs-bus-iio. Pointer to correct keyword
>   * in documentation is mentioned at the channel define description
>   * below.
>   *
>   * IIO_INTENSITY:	Channel for unitless intensity.
>   *			Doc keyword: in_intensityY_raw			
>   *
>   * IIO_LIGHT:		Channel for visible light intensity in lux
>   * 			Doc keyword: in_illuminance_raw
>   */
> 
> I have a feeling that this would already have helped me. Nevertheless, I 
> would not mind seeing docs also for the iio_chan_info_enum values - but 
> linking to the correct spot in sysfs-bus-iio might not be as easy.
> 
> Anyays, Thanks for the help once again :)

Both the kernel-doc for this header and the ABI docs end up in
the kernel html docs.  I wonder if a link is possible...
https://docs.kernel.org/driver-api/iio/core.html#industrial-i-o-devices
would have the iio_chan_type docs I think if there were any.
https://docs.kernel.org/admin-guide/abi-testing.html?highlight=abi#abi-sys-iio-devicex-in-intensityy-raw 
is the matching ABI doc. 

Mauro, Jon, other docs system experts...

I couldn't immediately find a way to link to a specific ABI docs entry,
is there a means to do it from kernel-doc in a header?

Thanks,

Jonathan


> 
> Yours,
> 	-- Matti
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-01-30 13:02 ` Jonathan Cameron
  2023-01-30 13:42   ` Vaittinen, Matti
@ 2023-01-31  9:31   ` Vaittinen, Matti
  2023-02-02 16:57     ` Jonathan Cameron
  1 sibling, 1 reply; 20+ messages in thread
From: Vaittinen, Matti @ 2023-01-31  9:31 UTC (permalink / raw)
  To: Jonathan Cameron, Matti Vaittinen; +Cc: Jonathan Cameron, linux-iio

On 1/30/23 15:02, Jonathan Cameron wrote:
> On Mon, 30 Jan 2023 14:04:53 +0200
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:

>> However, the values
>> spilled from raw IIO_INTENSITY channels will change when integration
>> time is changed. So, should I use the info_mask_shared_by_type =
>> BIT(IIO_CHAN_INFO_INT_TIME) for IIO_INTENSITY channels?
> 
> Ah. This problem. The mixture of two things that effectively map to scaling
> of raw channels. As _scale must be applied by userspace to the _raw channel
> that has to reflect both the result of integration time and a front end amplifier
> and is the control typical userspace expects to use to vary the sensitivity.
> 
> That makes it messy because it's not always totally obvious whether, when
> trying to increase sensitivity, it is better to increase sample time or gain.
> Usually you do sample time first as that tends to reduce noise and for light
> sensors we rarely need particular quick answers.
> 
> So in the interests of keeping things easy to understand for userspace code
> you would need to provide writeable _scale that then attempts to find the
> best combination of amplifier gain and sampling time.

There is (at least) one more thing which I just noticed when I continued 
writing the code.

Changing the integration time impacts all intensity channels. So, scale 
will be adjusted for all channels when a request to set scale for one 
channel causes integration time to change. (Gain on the other hand is 
adjustable separately for each channel.) Do you think a typical 
user-space application can cope with this?

I am unsure if I should just use the biggest integration time (400mS) by 
default and only decrease this when very small amplification is 
requested by setting scale > 1. TBH, I don't like this. It prevents 
having shorter measurement times with gains greater than 1x - and I 
believe that users may want to have higher gains also in cases where 
they wish quicker measurements from the sensor.

Some other options I am considering:
1. Skip time config for now - easiest but does not give full usability
2. Allow setting the time via devicetree at probe time - slightly better 
but not very dynamic.
3. Custom device-specific sysfs file for setting the time so 
specifically tailored userland apps have access to it - adding new ABI 
for this is probably not something we prefer ;)
4. Allow setting the integration time in situations where the driver can 
internally hide the scale change by changing the gain as well - this 
operation is not atomic in the HW and adds some extra complexity to the 
driver. Also, this fails for configurations where the gain setting is 
such it can't compensate the scale change.

I would be grateful for any suggestions :)

Finally, the BU27034 allows pretty big gains - from 1x to 4096x. After a 
quick look in existing light sensor drivers - I think the available 
scales for IIO_INTENSITY channels are usually from 1.0 downwards. ("1.0 
0.xxx 0.yyy 0.zzz"). 4096x (or 32768x if we take the max measurement 
time into account) will cause some loss of accuracy even with NANO scale 
if we use scale range starting from 1.0. Is there anything that prevents 
starting the available scales for example from 16.0 ending 976562.5 
NANOs (to decrease loss of precision assuming both the full gain range 
and all timing values [except the 5 mS] are supported)? At least I 
didn't see any limitations in the sysfs-bus-iio ;) My guess is the 
userspace usually handles the integer portion of scale just fine(?)

Oh, by the way - I found a publicly available data-sheet for the ALS 
sensor I am working with. :)

https://fscdn.rohm.com/en/products/databook/datasheet/ic/sensor/light/bu27034nuc-e.pdf

Seems like no public datasheet for the other sensors I mentioned though :(

As a side note - I had always thought measuring the light is just simple 
value reading from a sensor. I never regarded the fact that human eye 
sees only certain wavelengths or that the sensors sensitivity changes 
depending on the wavelength. It's funny how I always end up knowing less 
when I know more ;)

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-01-30 20:19         ` Jonathan Cameron
@ 2023-01-31 19:58           ` Jonathan Corbet
  2023-02-01  5:55             ` Matti Vaittinen
  0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Corbet @ 2023-01-31 19:58 UTC (permalink / raw)
  To: Jonathan Cameron, Matti Vaittinen
  Cc: Jonathan Cameron, Vaittinen, Matti, linux-iio,
	Mauro Carvalho Chehab, linux-doc

Jonathan Cameron <jic23@kernel.org> writes:

> Both the kernel-doc for this header and the ABI docs end up in
> the kernel html docs.  I wonder if a link is possible...
> https://docs.kernel.org/driver-api/iio/core.html#industrial-i-o-devices
> would have the iio_chan_type docs I think if there were any.
> https://docs.kernel.org/admin-guide/abi-testing.html?highlight=abi#abi-sys-iio-devicex-in-intensityy-raw 
> is the matching ABI doc. 
>
> Mauro, Jon, other docs system experts...
>
> I couldn't immediately find a way to link to a specific ABI docs entry,
> is there a means to do it from kernel-doc in a header?

It should just be possible to write out the type or function name and
have the links generated automatically.  Have you tried just putting
"enum iio_chan_type" into the text?  If the automarkup code doesn't pick
it up, please let me know.

Thanks,

jon

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-01-31 19:58           ` Jonathan Corbet
@ 2023-02-01  5:55             ` Matti Vaittinen
  0 siblings, 0 replies; 20+ messages in thread
From: Matti Vaittinen @ 2023-02-01  5:55 UTC (permalink / raw)
  To: Jonathan Corbet, Jonathan Cameron
  Cc: Jonathan Cameron, Vaittinen, Matti, linux-iio,
	Mauro Carvalho Chehab, linux-doc

On 1/31/23 21:58, Jonathan Corbet wrote:
> Jonathan Cameron <jic23@kernel.org> writes:
> 
>> Both the kernel-doc for this header and the ABI docs end up in
>> the kernel html docs.  I wonder if a link is possible...
>> https://docs.kernel.org/driver-api/iio/core.html#industrial-i-o-devices
>> would have the iio_chan_type docs I think if there were any.
>> https://docs.kernel.org/admin-guide/abi-testing.html?highlight=abi#abi-sys-iio-devicex-in-intensityy-raw
>> is the matching ABI doc.
>>
>> Mauro, Jon, other docs system experts...
>>
>> I couldn't immediately find a way to link to a specific ABI docs entry,
>> is there a means to do it from kernel-doc in a header?
> 
> It should just be possible to write out the type or function name and
> have the links generated automatically.  Have you tried just putting
> "enum iio_chan_type" into the text?  If the automarkup code doesn't pick
> it up, please let me know.

As far as I understand - the problem here is that we would like to have 
a link from the enum/definition doc to a specific place in the ABI 
document. The ABI document does not have the enum / define / function 
"link _target_". (I think adding the enum/function name in the ABI doc 
would generate link from ABI doc to enum/function doc - which would be 
helpful - but in this case we would like to have a link to opposite 
direction. From function/enum doc to specific position in a (sysfs) ABI 
documentation. Please, bear with me if this was what your suggestion 
should achieve. I'd just like to ensure we are ton a same page with this.)

Yours,
	--Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-01-31  9:31   ` Vaittinen, Matti
@ 2023-02-02 16:57     ` Jonathan Cameron
  2023-02-06 14:34       ` Vaittinen, Matti
  0 siblings, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2023-02-02 16:57 UTC (permalink / raw)
  To: Vaittinen, Matti; +Cc: Jonathan Cameron, Matti Vaittinen, linux-iio

On Tue, 31 Jan 2023 09:31:53 +0000
"Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com> wrote:

> On 1/30/23 15:02, Jonathan Cameron wrote:
> > On Mon, 30 Jan 2023 14:04:53 +0200
> > Matti Vaittinen <mazziesaccount@gmail.com> wrote:  
> 
> >> However, the values
> >> spilled from raw IIO_INTENSITY channels will change when integration
> >> time is changed. So, should I use the info_mask_shared_by_type =
> >> BIT(IIO_CHAN_INFO_INT_TIME) for IIO_INTENSITY channels?  
> > 
> > Ah. This problem. The mixture of two things that effectively map to scaling
> > of raw channels. As _scale must be applied by userspace to the _raw channel
> > that has to reflect both the result of integration time and a front end amplifier
> > and is the control typical userspace expects to use to vary the sensitivity.
> > 
> > That makes it messy because it's not always totally obvious whether, when
> > trying to increase sensitivity, it is better to increase sample time or gain.
> > Usually you do sample time first as that tends to reduce noise and for light
> > sensors we rarely need particular quick answers.
> > 
> > So in the interests of keeping things easy to understand for userspace code
> > you would need to provide writeable _scale that then attempts to find the
> > best combination of amplifier gain and sampling time.  
> 
> There is (at least) one more thing which I just noticed when I continued 
> writing the code.
> 
> Changing the integration time impacts all intensity channels. 

Oh yuck. 

> So, scale 
> will be adjusted for all channels when a request to set scale for one 
> channel causes integration time to change. (Gain on the other hand is 
> adjustable separately for each channel.) Do you think a typical 
> user-space application can cope with this?

Sensor is slow anyway, so how about updating it before a read
if the value is different from requested for the current channel?

> 
> I am unsure if I should just use the biggest integration time (400mS) by 
> default and only decrease this when very small amplification is 
> requested by setting scale > 1. TBH, I don't like this. It prevents 
> having shorter measurement times with gains greater than 1x - and I 
> believe that users may want to have higher gains also in cases where 
> they wish quicker measurements from the sensor.

Possibly shorter times might be desired, though uncommon for people
to care a lot about read speed on a light sensor.  Light levels
usually change slowly (assuming no one is doing anything exciting
with the sensor)

> 
> Some other options I am considering:
> 1. Skip time config for now - easiest but does not give full usability

Not great - for same reason as below (saturation)

> 2. Allow setting the time via devicetree at probe time - slightly better 
> but not very dynamic.

Don't do that one.  Usual reason to want to set this stuff is to avoid
saturation of the sensor.

> 3. Custom device-specific sysfs file for setting the time so 
> specifically tailored userland apps have access to it - adding new ABI 
> for this is probably not something we prefer ;)

Indeed - new ABI is normally unused ABI.

> 4. Allow setting the integration time in situations where the driver can 
> internally hide the scale change by changing the gain as well - this 
> operation is not atomic in the HW and adds some extra complexity to the 
> driver. Also, this fails for configurations where the gain setting is 
> such it can't compensate the scale change.

So we have had a few examples that might apply here (can't remember if
any of them actually got implemented).

My aim would be to have the normal ABI do the right thing, but...
If we were to provide extra ABI that allows switching out of a default
mode then we can assume the code using it knows what it is doing.

So maybe something like

auto_integration_time_from_scale that defaults to 1/true.
When 0/false,
integration_time becomes writeable.  If you write integration time
it would then affect scale appropriately and any change to scale
would be clamped within whatever range is possible for the integration
time provided.

If you want to write an auto tuning routine you could do that and mostly
avoid the problem.
We have a few drivers doing that IIRC.  The only signal userspace
normally cares about is illuminance and hopefully the calculations
take the different settings into account.  I never much liked
the cases we have of this but there are a few e.g. gp2ap020a00f
In my view that should be a userspace problem as the algorithms to
tune this could be quite complex.

> 
> I would be grateful for any suggestions :)
> 
> Finally, the BU27034 allows pretty big gains - from 1x to 4096x. After a 
> quick look in existing light sensor drivers - I think the available 
> scales for IIO_INTENSITY channels are usually from 1.0 downwards. ("1.0 
> 0.xxx 0.yyy 0.zzz"). 4096x (or 32768x if we take the max measurement 
> time into account) will cause some loss of accuracy even with NANO scale 
> if we use scale range starting from 1.0. 

*sigh* someone implemented an insane gain because they happened to be able
to. I'd imagine that adds so much noise to be effectively pointless.
That dynamic range seems unlikely to be of much practical use.


> Is there anything that prevents 
> starting the available scales for example from 16.0 ending 976562.5 
> NANOs (to decrease loss of precision assuming both the full gain range 
> and all timing values [except the 5 mS] are supported)?

That's fine.  Intensity is defined as unit free (because there are no
sensible units) so no code should be making any assumptions.

> At least I 
> didn't see any limitations in the sysfs-bus-iio ;) My guess is the 
> userspace usually handles the integer portion of scale just fine(?)

It definitely should! There are other types of device where gains
tend to be much greater than 1

> 
> Oh, by the way - I found a publicly available data-sheet for the ALS 
> sensor I am working with. :)
> 
> https://fscdn.rohm.com/en/products/databook/datasheet/ic/sensor/light/bu27034nuc-e.pdf
> 
> Seems like no public datasheet for the other sensors I mentioned though :(
> 
> As a side note - I had always thought measuring the light is just simple 
> value reading from a sensor. I never regarded the fact that human eye 
> sees only certain wavelengths or that the sensors sensitivity changes 
> depending on the wavelength. It's funny how I always end up knowing less 
> when I know more ;)

Light sensors are a pain in many ways!  We've had a few datasheets over the
years that have insisted that the color channels have well specified units
despite there being no such definition that I'm aware of (and no means
to define one for cheap sensors. What is standard Red? :))  All the
illuminance values are just approximations to the official curve.

Sometime in the past we debated trying to describe the curves, but it's
hard to do as they aren't nice shapes (so 3DB points don't make much
sense).

> 
> Yours,
> 	-- Matti
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-02-02 16:57     ` Jonathan Cameron
@ 2023-02-06 14:34       ` Vaittinen, Matti
  2023-02-18 17:20         ` Jonathan Cameron
  2023-02-25  9:35         ` [low prio, just pondering] About the light sensor "sensitivity area" Matti Vaittinen
  0 siblings, 2 replies; 20+ messages in thread
From: Vaittinen, Matti @ 2023-02-06 14:34 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Jonathan Cameron, Matti Vaittinen, linux-iio, Mutanen, Mikko

On 2/2/23 18:57, Jonathan Cameron wrote:
> On Tue, 31 Jan 2023 09:31:53 +0000
> "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> 
>> On 1/30/23 15:02, Jonathan Cameron wrote:
>>> On Mon, 30 Jan 2023 14:04:53 +0200
>>> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
>>
>>>> However, the values
>>>> spilled from raw IIO_INTENSITY channels will change when integration
>>>> time is changed. So, should I use the info_mask_shared_by_type =
>>>> BIT(IIO_CHAN_INFO_INT_TIME) for IIO_INTENSITY channels?
>>>
>>> Ah. This problem. The mixture of two things that effectively map to scaling
>>> of raw channels. As _scale must be applied by userspace to the _raw channel
>>> that has to reflect both the result of integration time and a front end amplifier
>>> and is the control typical userspace expects to use to vary the sensitivity.
>>>
>>> That makes it messy because it's not always totally obvious whether, when
>>> trying to increase sensitivity, it is better to increase sample time or gain.
>>> Usually you do sample time first as that tends to reduce noise and for light
>>> sensors we rarely need particular quick answers.
>>>
>>> So in the interests of keeping things easy to understand for userspace code
>>> you would need to provide writeable _scale that then attempts to find the
>>> best combination of amplifier gain and sampling time.
>>
>> There is (at least) one more thing which I just noticed when I continued
>> writing the code.
>>
>> Changing the integration time impacts all intensity channels.
> 
> Oh yuck.
> 
>> So, scale
>> will be adjusted for all channels when a request to set scale for one
>> channel causes integration time to change. (Gain on the other hand is
>> adjustable separately for each channel.) Do you think a typical
>> user-space application can cope with this?
> 
> Sensor is slow anyway, so how about updating it before a read
> if the value is different from requested for the current channel?

I think this could be doable. The bu27034 has no IRQ and no buffer so I 
plan to only support the raw_read() and raw_write(). The driver could 
cache requested integration-time values for each channel and set them to 
HW before reading data from each channel. I am not really a fan of 
caching the values - but this could simplify the integration time 
setting as the new time would not necessarily need to suit all channels.

I will think of this - but current version I drafted does not support it.

>> I am unsure if I should just use the biggest integration time (400mS) by
>> default and only decrease this when very small amplification is
>> requested by setting scale > 1. TBH, I don't like this. It prevents
>> having shorter measurement times with gains greater than 1x - and I
>> believe that users may want to have higher gains also in cases where
>> they wish quicker measurements from the sensor.
> 
> Possibly shorter times might be desired, though uncommon for people
> to care a lot about read speed on a light sensor.  Light levels
> usually change slowly (assuming no one is doing anything exciting
> with the sensor)
> 
>>
>> Some other options I am considering:
>> 1. Skip time config for now - easiest but does not give full usability
> 
> Not great - for same reason as below (saturation)
> 
>> 2. Allow setting the time via devicetree at probe time - slightly better
>> but not very dynamic.
> 
> Don't do that one.  Usual reason to want to set this stuff is to avoid
> saturation of the sensor.
> 
>> 3. Custom device-specific sysfs file for setting the time so
>> specifically tailored userland apps have access to it - adding new ABI
>> for this is probably not something we prefer ;)
> 
> Indeed - new ABI is normally unused ABI >
>> 4. Allow setting the integration time in situations where the driver can
>> internally hide the scale change by changing the gain as well - this
>> operation is not atomic in the HW and adds some extra complexity to the
>> driver. Also, this fails for configurations where the gain setting is
>> such it can't compensate the scale change.

I started with the option 4) and wrote first 100% untested code draft. I 
don't yet have the hardware at my hands so testing and fine-tuning needs 
to wait for a while.

I wonder if I should send out an RFC to explain what I was up-to, or if 
I should wait for the hardware, do testing and only send out the patches 
when I believe things work as intended :)

Sending RFC early-on would perhaps be beneficial for me as it can help 
me avoid working/finalizing a solution that will not be appreciated ;)

Yet, sending out and RFC with untested code may result reviewers (who 
skip the "small print" explaining the code is untested and to be 
reviewed for a concept only) to do unnecessary work. And in my 
experience, no letters are large enough that all reviewers didn't skip 
the resulting "small print" :)

> 
> So we have had a few examples that might apply here (can't remember if
> any of them actually got implemented).
> 
> My aim would be to have the normal ABI do the right thing, but...
> If we were to provide extra ABI that allows switching out of a default
> mode then we can assume the code using it knows what it is doing.

Hm. Maybe it's because all the virtual waffles and beer (last weekend 
was FOSDEM weekend - but I was participating only remotely) - but I am 
not 100% sure I know what you mean by the default mode.

> So maybe something like
> 
> auto_integration_time_from_scale that defaults to 1/true.
> When 0/false,
> integration_time becomes writeable.  If you write integration time
> it would then affect scale appropriately and any change to scale
> would be clamped within whatever range is possible for the integration
> time provided.

Hm. Does this mean that writing the integration time would
1) Change integration time in HW
2) Not change gain in HW
3) Change integration time and scale values in sysfs?

My current attempt if to not provide the 'mode' change flag - but 
provide R/W integration time and R/W scale.

Regarding the scale:

When scale is changed by user, the driver attempts to maintain 
integration time and perform only the gain change. If requested scale 
can not be supported by any available gain using the current integration 
time, then the driver attempts to change both the gain and integration 
time to achieve requested scale. (If this also fails, then an error is 
returned).

I guess this is what is expected to happen in "normal mode".

As I mentioned earlier, this does not allow a great control over the 
integration time for users who (may?) wish to have shorter time with 
gain bigger than 1x.

Hence the writeable integration time.
Now, an user may request different integration time by writing the 
integration time. I assumed this is also normal operation assuming this 
does not cause a scale change? Hence, my current attempt is to 
compensate the caused scale change by adjusting also gain. It appears to 
me (based on the equation from my HW colleagues) that the doubled 
integration time also doubles the reported intensity value. I think this 
is also expected. For example: Let's assume a case where we originally have
gain 8x, int-time 400 mS

user wants to set new int-time 200 mS.

Halving the measurement time would mean halving the reported intensity 
values, causeing the scale to change, right? Now, in order to maintain 
the scale, the driver would be not only dropping the integration time 
but also internally doubling the gain from 8x to 16x.

> 
> If you want to write an auto tuning routine you could do that and mostly
> avoid the problem.
> We have a few drivers doing that IIRC.

Hm. Yes, it must be the virtual waffles :) I am unsure what you mean by 
autotuning. Do you mean increasing the gain/integration time in driver 
based on the raw-data to ensure values are as accurate as possible w/o 
being saturated?

   The only signal userspace
> normally cares about is illuminance and hopefully the calculations
> take the different settings into account.  I never much liked
> the cases we have of this but there are a few e.g. gp2ap020a00f
> In my view that should be a userspace problem as the algorithms to
> tune this could be quite complex.
> 

This definitely sounds like a computation better done in user-space to 
me. I had hard enough time writing a simple lux-calculation in my driver 
resulting bunch of overflow-checks and do_div()s... Luckily the 
performance was not a priority.

>>
>> I would be grateful for any suggestions :)
>>
>> Finally, the BU27034 allows pretty big gains - from 1x to 4096x. After a
>> quick look in existing light sensor drivers - I think the available
>> scales for IIO_INTENSITY channels are usually from 1.0 downwards. ("1.0
>> 0.xxx 0.yyy 0.zzz"). 4096x (or 32768x if we take the max measurement
>> time into account) will cause some loss of accuracy even with NANO scale
>> if we use scale range starting from 1.0.
> 
> *sigh* someone implemented an insane gain because they happened to be able
> to. I'd imagine that adds so much noise to be effectively pointless.
> That dynamic range seems unlikely to be of much practical use.

I can't say anything to this as I don't have the sensor yet :)

>>
>> As a side note - I had always thought measuring the light is just simple
>> value reading from a sensor. I never regarded the fact that human eye
>> sees only certain wavelengths or that the sensors sensitivity changes
>> depending on the wavelength. It's funny how I always end up knowing less
>> when I know more ;)
> 
> Light sensors are a pain in many ways!  We've had a few datasheets over the
> years that have insisted that the color channels have well specified units
> despite there being no such definition that I'm aware of (and no means
> to define one for cheap sensors. What is standard Red? :))  All the
> illuminance values are just approximations to the official curve.
> 
> Sometime in the past we debated trying to describe the curves, but it's
> hard to do as they aren't nice shapes (so 3DB points don't make much
> sense).

This is easy to agree after only a very brief look on the topic :)

Yours,
-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-02-06 14:34       ` Vaittinen, Matti
@ 2023-02-18 17:20         ` Jonathan Cameron
  2023-02-18 18:08           ` Matti Vaittinen
  2023-02-25  9:35         ` [low prio, just pondering] About the light sensor "sensitivity area" Matti Vaittinen
  1 sibling, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2023-02-18 17:20 UTC (permalink / raw)
  To: Vaittinen, Matti
  Cc: Jonathan Cameron, Matti Vaittinen, linux-iio, Mutanen, Mikko

> >> such it can't compensate the scale change.  
> 
> I started with the option 4) and wrote first 100% untested code draft. I 
> don't yet have the hardware at my hands so testing and fine-tuning needs 
> to wait for a while.
> 
> I wonder if I should send out an RFC to explain what I was up-to, or if 
> I should wait for the hardware, do testing and only send out the patches 
> when I believe things work as intended :)
> 
> Sending RFC early-on would perhaps be beneficial for me as it can help 
> me avoid working/finalizing a solution that will not be appreciated ;)
> 
> Yet, sending out and RFC with untested code may result reviewers (who 
> skip the "small print" explaining the code is untested and to be 
> reviewed for a concept only) to do unnecessary work. And in my 
> experience, no letters are large enough that all reviewers didn't skip 
> the resulting "small print" :)

Who would ever do that? *evil laugh*

RFC is fine.  If people spend time reviewing without checking 'why it is
an RFC' it is their own fault.  This is one reason I moan about any RFC
I see without the reasons being clearly called otu.

> 
> > 
> > So we have had a few examples that might apply here (can't remember if
> > any of them actually got implemented).
> > 
> > My aim would be to have the normal ABI do the right thing, but...
> > If we were to provide extra ABI that allows switching out of a default
> > mode then we can assume the code using it knows what it is doing.  
> 
> Hm. Maybe it's because all the virtual waffles and beer (last weekend 
> was FOSDEM weekend - but I was participating only remotely) - but I am 
> not 100% sure I know what you mean by the default mode.

Have the driver do 'best effort' on scaling etc with default being that
it doesn't care about time to take a reading. But have ABI that allows 
an override. Normal user would never use the override but would get something
reasonable.  Advanced user can mess with the settings if they want to.

> 
> > So maybe something like
> > 
> > auto_integration_time_from_scale that defaults to 1/true.
> > When 0/false,
> > integration_time becomes writeable.  If you write integration time
> > it would then affect scale appropriately and any change to scale
> > would be clamped within whatever range is possible for the integration
> > time provided.  
> 
> Hm. Does this mean that writing the integration time would
> 1) Change integration time in HW
> 2) Not change gain in HW
> 3) Change integration time and scale values in sysfs?

Yes. If this magic mode bit was set. 

> 
> My current attempt if to not provide the 'mode' change flag - but 
> provide R/W integration time and R/W scale.

That's best place to start I think.

> 
> Regarding the scale:
> 
> When scale is changed by user, the driver attempts to maintain 
> integration time and perform only the gain change. If requested scale 
> can not be supported by any available gain using the current integration 
> time, then the driver attempts to change both the gain and integration 
> time to achieve requested scale. (If this also fails, then an error is 
> returned).
> 
> I guess this is what is expected to happen in "normal mode".

Interesting. I was actually thinking prefer to change integration time
but your way may make more sense.

> 
> As I mentioned earlier, this does not allow a great control over the 
> integration time for users who (may?) wish to have shorter time with 
> gain bigger than 1x.
> 
> Hence the writeable integration time.
> Now, an user may request different integration time by writing the 
> integration time. I assumed this is also normal operation assuming this 
> does not cause a scale change?

If magic mode write hasn't happened, then integration time should reject
writes. Interface is too complex otherwise because if a user writes the
integration time then the scale, they'll expect that their integration
time has not changed.

If you ever used a film camera, kind of equivalent of switching between
an auto mode where the camera could adjust aperture (gain) and shutter
timing (integration time) to a shutter priority mode in which the 
shutter timing (integration time) remains what you set it to and the
scale can only be adjusted in a fashion that doesn't change it.

> Hence, my current attempt is to 
> compensate the caused scale change by adjusting also gain. It appears to 
> me (based on the equation from my HW colleagues) that the doubled 
> integration time also doubles the reported intensity value. I think this 
> is also expected. For example: Let's assume a case where we originally have
> gain 8x, int-time 400 mS

Yes. That's what I'd expect for a light sensor that was fairly linear.

> 
> user wants to set new int-time 200 mS.
> 
> Halving the measurement time would mean halving the reported intensity 
> values, causeing the scale to change, right? Now, in order to maintain 
> the scale, the driver would be not only dropping the integration time 
> but also internally doubling the gain from 8x to 16x.

yes.

> 
> > 
> > If you want to write an auto tuning routine you could do that and mostly
> > avoid the problem.
> > We have a few drivers doing that IIRC.  
> 
> Hm. Yes, it must be the virtual waffles :) I am unsure what you mean by 
> autotuning. Do you mean increasing the gain/integration time in driver 
> based on the raw-data to ensure values are as accurate as possible w/o 
> being saturated?

yup.  Then return a _processed value that incorporates the autotuned
scale.  If _scale and _integration_time are then provided they are
for info only and not guaranteed to be the values used to grab the reading.

> 
>    The only signal userspace
> > normally cares about is illuminance and hopefully the calculations
> > take the different settings into account.  I never much liked
> > the cases we have of this but there are a few e.g. gp2ap020a00f
> > In my view that should be a userspace problem as the algorithms to
> > tune this could be quite complex.
> >   
> 
> This definitely sounds like a computation better done in user-space to 
> me. I had hard enough time writing a simple lux-calculation in my driver 
> resulting bunch of overflow-checks and do_div()s... Luckily the 
> performance was not a priority.

It's 'fun' but we have had people do it. I never bothered for the light
sensor I wrote a long time back (which got dropped eventually)

> 
> >>
> >> I would be grateful for any suggestions :)
> >>
> >> Finally, the BU27034 allows pretty big gains - from 1x to 4096x. After a
> >> quick look in existing light sensor drivers - I think the available
> >> scales for IIO_INTENSITY channels are usually from 1.0 downwards. ("1.0
> >> 0.xxx 0.yyy 0.zzz"). 4096x (or 32768x if we take the max measurement
> >> time into account) will cause some loss of accuracy even with NANO scale
> >> if we use scale range starting from 1.0.  
> > 
> > *sigh* someone implemented an insane gain because they happened to be able
> > to. I'd imagine that adds so much noise to be effectively pointless.
> > That dynamic range seems unlikely to be of much practical use.  
> 
> I can't say anything to this as I don't have the sensor yet :)
> 
> >>
> >> As a side note - I had always thought measuring the light is just simple
> >> value reading from a sensor. I never regarded the fact that human eye
> >> sees only certain wavelengths or that the sensors sensitivity changes
> >> depending on the wavelength. It's funny how I always end up knowing less
> >> when I know more ;)  
> > 
> > Light sensors are a pain in many ways!  We've had a few datasheets over the
> > years that have insisted that the color channels have well specified units
> > despite there being no such definition that I'm aware of (and no means
> > to define one for cheap sensors. What is standard Red? :))  All the
> > illuminance values are just approximations to the official curve.
> > 
> > Sometime in the past we debated trying to describe the curves, but it's
> > hard to do as they aren't nice shapes (so 3DB points don't make much
> > sense).  
> 
> This is easy to agree after only a very brief look on the topic :)

One of those areas of technology where things aren't easily controlled or
well behaved!

J
> 
> Yours,
> -- Matti
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-02-18 17:20         ` Jonathan Cameron
@ 2023-02-18 18:08           ` Matti Vaittinen
  2023-02-26 17:26             ` Jonathan Cameron
  2023-02-26 17:30             ` Jonathan Cameron
  0 siblings, 2 replies; 20+ messages in thread
From: Matti Vaittinen @ 2023-02-18 18:08 UTC (permalink / raw)
  To: Jonathan Cameron, Vaittinen, Matti
  Cc: Jonathan Cameron, linux-iio, Mutanen, Mikko

Thanks a lot Jonathan,

You have been super helpful :) Thanks!

On 2/18/23 19:20, Jonathan Cameron wrote:
>>>> such it can't compensate the scale change.
>>
>> Regarding the scale:
>>
>> When scale is changed by user, the driver attempts to maintain
>> integration time and perform only the gain change. If requested scale
>> can not be supported by any available gain using the current integration
>> time, then the driver attempts to change both the gain and integration
>> time to achieve requested scale. (If this also fails, then an error is
>> returned).
>>
>> I guess this is what is expected to happen in "normal mode".
> 
> Interesting. I was actually thinking prefer to change integration time
> but your way may make more sense.
> 
>>
>> As I mentioned earlier, this does not allow a great control over the
>> integration time for users who (may?) wish to have shorter time with
>> gain bigger than 1x.
>>
>> Hence the writeable integration time.
>> Now, an user may request different integration time by writing the
>> integration time. I assumed this is also normal operation assuming this
>> does not cause a scale change?
> 
> If magic mode write hasn't happened, then integration time should reject
> writes. Interface is too complex otherwise because if a user writes the
> integration time then the scale, they'll expect that their integration
> time has not changed.

I agree that changing the integration time set by the user could be 
unexpected. OTOH, if we take the approach I explained above (Eg. try 
keeping integration time intact when scale is changed and only change 
int-time if gain alone can't provide requested scale), then the 
integration time would not change is user asks for scale we can support 
with the set integration time.

There is few bumps on the road though... I am not exactly sure how to 
decide available scales to advertise. Nor am I sure how to deal with the 
scale settings when requested scale can't be met with gain change alone 
but where scale is also not at the one of the extreme ends (which could 
clearly warrant the integration time change) - but is somewhere in the 
mid-range. This can happen because the supported gains have "jumps". 1x 
to 4x (skipping 2x), to 16x (skipping 8x) to 32, 64, and then to 256 
(skip 128), 512, 1024, 2048 and 4096.

Integration times are 50, 100, 200, 400 - which means that some of the 
'mid range' scales can be only supported by some integration times.

I will try to cook an RFC next week to show what I have drafted if there 
is no big surprizes on the road..

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [low prio, just pondering] About the light sensor "sensitivity area"
  2023-02-06 14:34       ` Vaittinen, Matti
  2023-02-18 17:20         ` Jonathan Cameron
@ 2023-02-25  9:35         ` Matti Vaittinen
  2023-03-04 20:26           ` Jonathan Cameron
  1 sibling, 1 reply; 20+ messages in thread
From: Matti Vaittinen @ 2023-02-25  9:35 UTC (permalink / raw)
  To: Vaittinen, Matti, Jonathan Cameron; +Cc: Jonathan Cameron, linux-iio

On 2/6/23 16:34, Vaittinen, Matti wrote:
> On 2/2/23 18:57, Jonathan Cameron wrote:
>> On Tue, 31 Jan 2023 09:31:53 +0000
>> "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com> wrote:
>>
>>> On 1/30/23 15:02, Jonathan Cameron wrote:
>>>> On Mon, 30 Jan 2023 14:04:53 +0200
>>>> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
>>>
>>> As a side note - I had always thought measuring the light is just simple
>>> value reading from a sensor. I never regarded the fact that human eye
>>> sees only certain wavelengths or that the sensors sensitivity changes
>>> depending on the wavelength. It's funny how I always end up knowing less
>>> when I know more ;)
>>
>> Light sensors are a pain in many ways!  We've had a few datasheets over the
>> years that have insisted that the color channels have well specified units
>> despite there being no such definition that I'm aware of (and no means
>> to define one for cheap sensors. What is standard Red? :))  All the
>> illuminance values are just approximations to the official curve.
>>
>> Sometime in the past we debated trying to describe the curves, but it's
>> hard to do as they aren't nice shapes (so 3DB points don't make much
>> sense).

This is a low-priority mail with just some very initial pondering. Feel 
free to skip this if you're in a hurry.

I guess the problem of telling what the sensor values represent for 
sensors where the sensitivity is a poor match to a colour has been 
dwelling in the background :)

I don't have any long experience on these devices so I have seen only 
couple of the light sensor data-sheets, and mostly just for ROHM 
sensors. Maybe this is the reason why the common thing I have seen in 
these data-sheets representing the sensitivity to wave lengths has been 
a "spectral response" curve. All of the data-sheets have represented a 
curve where "sensor responsivity" is in the Y-Axis and wave length at 
the X-axis. And yes, in many cases this curve (especially for a CLEAR 
light) is of arbitrary shape for example like this:




S                                           ***
e                                      *       *
n                                  *            *
s                        **       *             *
i                   *       **   *               *
t                *             *                  *
i              *                                  *
v             *                                    *
i         **                                       *
t      *                                           *
y    *                                             *
    *                                                ***
   *                                                     ******
  *                                                            *
  *                                                            *
400		500		600		700		800nm
                W a v e l e n g h t


Having this in mind it seems to be impossible to have just one or a few 
categories of sensitivity, or to describe it accurately by just some 
"peak-sensitivity" wave-lenght and a value representing "width of the 
curve".

So, maybe we should abandon the idea of having a great categorization / 
abstraction in-driver or IIO framework (other than the R,G,B,C,IR,UV - 
which works fine for some sensors). What I could think of is providing a 
set of 'data points' representing the sensitivity curves. Say, we had 
in_sensitivity_wavelength_calibpoints and 
in_sensitivity_wavelength_num_calibpoints (or what ever could fit for 
the IIO naming scheme) - where user could get sensor provided datapoints 
that represent the sensitivity as a function of wavelength. Userland 
could then decide the best curve fitting for the data-points and compute 
the sensitivity according to the best available algorithms. I think this 
kind of curve-fitting-to-datapoints is quite standard stuff in the 
user-space these days - but it feels like an overwhelming task in the 
kernel land/drivers...

This all is just some pondering. I do not have a proper use-case for 
this kind of a sensitivity curve data as I work for a component vendor 
instead of doing actual systems utilizing these components :/ It's 
actually a little sad as I seem to keep thinking what kind of a device I 
could build using these components - just to end up noticing that I am 
not in a position where I was building these devices :p (You wouldn't 
believe how cool imaginary clocking device for driving a camera clock 
with light sensor detecting flickering I just designed in my head the 
other night XD).

Well, I still hope I can help creating device driver/framework stuff 
people can use to build devices - in the end of the day it will also 
benefit the component vendor as the components are typically used in 
these devices ;)

Oh. Got carried away. Anyways, have you considered just offering an 
entry with sensitivity data-points instead of offering wavelength and 
3DB-limits? Do you think that could be useful?

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-02-18 18:08           ` Matti Vaittinen
@ 2023-02-26 17:26             ` Jonathan Cameron
  2023-02-26 17:30             ` Jonathan Cameron
  1 sibling, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2023-02-26 17:26 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Vaittinen, Matti, Jonathan Cameron, linux-iio, Mutanen, Mikko

On Sat, 18 Feb 2023 20:08:10 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Thanks a lot Jonathan,
> 
> You have been super helpful :) Thanks!
> 
> On 2/18/23 19:20, Jonathan Cameron wrote:
> >>>> such it can't compensate the scale change.  
> >>
> >> Regarding the scale:
> >>
> >> When scale is changed by user, the driver attempts to maintain
> >> integration time and perform only the gain change. If requested scale
> >> can not be supported by any available gain using the current integration
> >> time, then the driver attempts to change both the gain and integration
> >> time to achieve requested scale. (If this also fails, then an error is
> >> returned).
> >>
> >> I guess this is what is expected to happen in "normal mode".  
> > 
> > Interesting. I was actually thinking prefer to change integration time
> > but your way may make more sense.
> >   
> >>
> >> As I mentioned earlier, this does not allow a great control over the
> >> integration time for users who (may?) wish to have shorter time with
> >> gain bigger than 1x.
> >>
> >> Hence the writeable integration time.
> >> Now, an user may request different integration time by writing the
> >> integration time. I assumed this is also normal operation assuming this
> >> does not cause a scale change?  
> > 
> > If magic mode write hasn't happened, then integration time should reject
> > writes. Interface is too complex otherwise because if a user writes the
> > integration time then the scale, they'll expect that their integration
> > time has not changed.  
> 
> I agree that changing the integration time set by the user could be 
> unexpected. OTOH, if we take the approach I explained above (Eg. try 
> keeping integration time intact when scale is changed and only change 
> int-time if gain alone can't provide requested scale), then the 
> integration time would not change is user asks for scale we can support 
> with the set integration time.
> 
> There is few bumps on the road though... I am not exactly sure how to 
> decide available scales to advertise. Nor am I sure how to deal with the 
> scale settings when requested scale can't be met with gain change alone 
> but where scale is also not at the one of the extreme ends (which could 
> clearly warrant the integration time change) - but is somewhere in the 
> mid-range. This can happen because the supported gains have "jumps". 1x 
> to 4x (skipping 2x), to 16x (skipping 8x) to 32, 64, and then to 256 
> (skip 128), 512, 1024, 2048 and 4096.
> 
> Integration times are 50, 100, 200, 400 - which means that some of the 
> 'mid range' scales can be only supported by some integration times.

Tricky corner indeed. I'd be tempted to say don't always give people what
they request.  Generally speaking (ignore the complexity of your case)
the aim is to increase scale to the maximum that can be done without saturating
for the range people care about.  It is rarely a problem if we undershoot a little
in order to map to something that works on the hardware.  That freedom might help
you a little in this case.

> 
> I will try to cook an RFC next week to show what I have drafted if there 
> is no big surprizes on the road..
> 
> Yours,
> 	-- Matti
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-02-18 18:08           ` Matti Vaittinen
  2023-02-26 17:26             ` Jonathan Cameron
@ 2023-02-26 17:30             ` Jonathan Cameron
  2023-02-27  7:22               ` Matti Vaittinen
  1 sibling, 1 reply; 20+ messages in thread
From: Jonathan Cameron @ 2023-02-26 17:30 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Vaittinen, Matti, Jonathan Cameron, linux-iio, Mutanen, Mikko

On Sat, 18 Feb 2023 20:08:10 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Thanks a lot Jonathan,
> 
> You have been super helpful :) Thanks!
> 
> On 2/18/23 19:20, Jonathan Cameron wrote:
> >>>> such it can't compensate the scale change.  
> >>
> >> Regarding the scale:
> >>
> >> When scale is changed by user, the driver attempts to maintain
> >> integration time and perform only the gain change. If requested scale
> >> can not be supported by any available gain using the current integration
> >> time, then the driver attempts to change both the gain and integration
> >> time to achieve requested scale. (If this also fails, then an error is
> >> returned).
> >>
> >> I guess this is what is expected to happen in "normal mode".  
> > 
> > Interesting. I was actually thinking prefer to change integration time
> > but your way may make more sense.
> >   
> >>
> >> As I mentioned earlier, this does not allow a great control over the
> >> integration time for users who (may?) wish to have shorter time with
> >> gain bigger than 1x.
> >>
> >> Hence the writeable integration time.
> >> Now, an user may request different integration time by writing the
> >> integration time. I assumed this is also normal operation assuming this
> >> does not cause a scale change?  
> > 
> > If magic mode write hasn't happened, then integration time should reject
> > writes. Interface is too complex otherwise because if a user writes the
> > integration time then the scale, they'll expect that their integration
> > time has not changed.  
> 
> I agree that changing the integration time set by the user could be 
> unexpected. OTOH, if we take the approach I explained above (Eg. try 
> keeping integration time intact when scale is changed and only change 
> int-time if gain alone can't provide requested scale), then the 
> integration time would not change is user asks for scale we can support 
> with the set integration time.
> 
> There is few bumps on the road though... I am not exactly sure how to 
> decide available scales to advertise. Nor am I sure how to deal with the 
> scale settings when requested scale can't be met with gain change alone 
> but where scale is also not at the one of the extreme ends (which could 
> clearly warrant the integration time change) - but is somewhere in the 
> mid-range. This can happen because the supported gains have "jumps". 1x 
> to 4x (skipping 2x), to 16x (skipping 8x) to 32, 64, and then to 256 
> (skip 128), 512, 1024, 2048 and 4096.
> 
> Integration times are 50, 100, 200, 400 - which means that some of the 
> 'mid range' scales can be only supported by some integration times.
> 
> I will try to cook an RFC next week to show what I have drafted if there 
> is no big surprizes on the road..

Hmm. There is another approach that I'd not thought of in this case because
in my head integration time is more continuous than it is for this part and
that is to fiddle the _raw values (we do this for oversampling or SAR ADCs
where things tend to be powers of 2).  The trick is to shift the raw value
always so that the 'scale' due to (in this case) integration time remains
constant.  That separates the two controls completely.

However, I'm not sure that makes sense here where the thing we typically
want to change when scaling due to saturation is integration time.

Jonathan
> 
> Yours,
> 	-- Matti
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-02-26 17:30             ` Jonathan Cameron
@ 2023-02-27  7:22               ` Matti Vaittinen
  2023-02-27  9:54                 ` Matti Vaittinen
  0 siblings, 1 reply; 20+ messages in thread
From: Matti Vaittinen @ 2023-02-27  7:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Vaittinen, Matti, Jonathan Cameron, linux-iio, Mutanen, Mikko

On 2/26/23 19:30, Jonathan Cameron wrote:
> On Sat, 18 Feb 2023 20:08:10 +0200
> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> 
>> Thanks a lot Jonathan,
>>
>> You have been super helpful :) Thanks!
>>
>> On 2/18/23 19:20, Jonathan Cameron wrote:
> Hmm. There is another approach that I'd not thought of in this case because
> in my head integration time is more continuous than it is for this part and
> that is to fiddle the _raw values (we do this for oversampling or SAR ADCs
> where things tend to be powers of 2).  The trick is to shift the raw value
> always so that the 'scale' due to (in this case) integration time remains
> constant.  That separates the two controls completely.

Holy cow! That's a neat trick which I didn't think of!

Basically, we could do >> 1 for the data when time is 100 mS, >> 2 when 
200 mS and >> 3 when 400 mS. We would want to use 19-bit channel values 
then.

> However, I'm not sure that makes sense here where the thing we typically
> want to change when scaling due to saturation is integration time.

That's a bit problematic, yes. We could "fool" the user by doing the 
saturation check in driver, and then just returning the max value of all 
19-bits set if the saturation is detected. This, however, would yield 
raw values that are slightly off. OTOH, with max sift of 3 bits that's 
only 7 'raw ticks' - which I hope is acceptable. I hope the user will 
then be switching to shorter integration time and start getting correct 
readings.

It's slightly sad to say "good bye" to the gain-time-scale helpers but I 
guess you just helped me to solve this with a _really_ simple way. We 
can keep those helpers in "back pocket" for the day when we need them ;)

I will see what comes out of this idea - thanks for the help again!

> 
> Jonathan
>>
>> Yours,
>> 	-- Matti
>>
> 

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-02-27  7:22               ` Matti Vaittinen
@ 2023-02-27  9:54                 ` Matti Vaittinen
  2023-03-04 18:37                   ` Jonathan Cameron
  0 siblings, 1 reply; 20+ messages in thread
From: Matti Vaittinen @ 2023-02-27  9:54 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Vaittinen, Matti, Jonathan Cameron, linux-iio, Mutanen, Mikko

On 2/27/23 09:22, Matti Vaittinen wrote:
> On 2/26/23 19:30, Jonathan Cameron wrote:
>> On Sat, 18 Feb 2023 20:08:10 +0200
>> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
>>
>>> Thanks a lot Jonathan,
>>>
>>> You have been super helpful :) Thanks!
>>>
>>> On 2/18/23 19:20, Jonathan Cameron wrote:
>> Hmm. There is another approach that I'd not thought of in this case 
>> because
>> in my head integration time is more continuous than it is for this 
>> part and
>> that is to fiddle the _raw values (we do this for oversampling or SAR 
>> ADCs
>> where things tend to be powers of 2).  The trick is to shift the raw 
>> value
>> always so that the 'scale' due to (in this case) integration time remains
>> constant.  That separates the two controls completely.
> 
> Holy cow! That's a neat trick which I didn't think of!
> 
> Basically, we could do >> 1 for the data when time is 100 mS, >> 2 when 
> 200 mS and >> 3 when 400 mS. We would want to use 19-bit channel values 
> then.

Please ignore my previous mail. It seems I am once again not knowing 
what I am talking about. If we take this approach, we shift << 3 when 
int time is 55, << 2 for 100 and << 1 for 200. With 400 mS we would not 
shift.

>> However, I'm not sure that makes sense here where the thing we typically
>> want to change when scaling due to saturation is integration time.
> 
> That's a bit problematic, yes. We could "fool" the user by doing the 
> saturation check in driver, and then just returning the max value of all 
> 19-bits set if the saturation is detected. This, however, would yield 
> raw values that are slightly off. OTOH, with max sift of 3 bits that's 
> only 7 'raw ticks' - which I hope is acceptable. I hope the user will 
> then be switching to shorter integration time and start getting correct 
> readings.
> 
> It's slightly sad to say "good bye" to the gain-time-scale helpers but I 
> guess you just helped me to solve this with a _really_ simple way. We 
> can keep those helpers in "back pocket" for the day when we need them ;)
> 
> I will see what comes out of this idea - thanks for the help again!
> 

But as you surely knew from the start, the saturation problems kick in 
with the 'non maximum sifts' when the _highest_ bits never get set. 
There the 'saturation detection' would cause a huge jump by suddenly 
setting the high bits. So, yes - this does not seem like a feasible 
option here :/

/me feels stupid...

Sorry for the noise!

--Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: ROHM ALS, integration time
  2023-02-27  9:54                 ` Matti Vaittinen
@ 2023-03-04 18:37                   ` Jonathan Cameron
  0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2023-03-04 18:37 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Vaittinen, Matti, Jonathan Cameron, linux-iio, Mutanen, Mikko

On Mon, 27 Feb 2023 11:54:59 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 2/27/23 09:22, Matti Vaittinen wrote:
> > On 2/26/23 19:30, Jonathan Cameron wrote:  
> >> On Sat, 18 Feb 2023 20:08:10 +0200
> >> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> >>  
> >>> Thanks a lot Jonathan,
> >>>
> >>> You have been super helpful :) Thanks!
> >>>
> >>> On 2/18/23 19:20, Jonathan Cameron wrote:  
> >> Hmm. There is another approach that I'd not thought of in this case 
> >> because
> >> in my head integration time is more continuous than it is for this 
> >> part and
> >> that is to fiddle the _raw values (we do this for oversampling or SAR 
> >> ADCs
> >> where things tend to be powers of 2).  The trick is to shift the raw 
> >> value
> >> always so that the 'scale' due to (in this case) integration time remains
> >> constant.  That separates the two controls completely.  
> > 
> > Holy cow! That's a neat trick which I didn't think of!
> > 
> > Basically, we could do >> 1 for the data when time is 100 mS, >> 2 when 
> > 200 mS and >> 3 when 400 mS. We would want to use 19-bit channel values 
> > then.  
> 
> Please ignore my previous mail. It seems I am once again not knowing 
> what I am talking about. If we take this approach, we shift << 3 when 
> int time is 55, << 2 for 100 and << 1 for 200. With 400 mS we would not 
> shift.

Spot on.

> 
> >> However, I'm not sure that makes sense here where the thing we typically
> >> want to change when scaling due to saturation is integration time.  
> > 
> > That's a bit problematic, yes. We could "fool" the user by doing the 
> > saturation check in driver, and then just returning the max value of all 
> > 19-bits set if the saturation is detected. This, however, would yield 
> > raw values that are slightly off. OTOH, with max sift of 3 bits that's 
> > only 7 'raw ticks' - which I hope is acceptable. I hope the user will 
> > then be switching to shorter integration time and start getting correct 
> > readings.
> > 
> > It's slightly sad to say "good bye" to the gain-time-scale helpers but I 
> > guess you just helped me to solve this with a _really_ simple way. We 
> > can keep those helpers in "back pocket" for the day when we need them ;)
> > 
> > I will see what comes out of this idea - thanks for the help again!
> >   
> 
> But as you surely knew from the start, the saturation problems kick in 
> with the 'non maximum sifts' when the _highest_ bits never get set.

Yes, thats what we'd expect to see as we can only measure high light levels
if the integration time is short.

> There the 'saturation detection' would cause a huge jump by suddenly 
> setting the high bits. So, yes - this does not seem like a feasible 
> option here :/

Yes, there is no consistent value for saturation if you are changing
the integration time as the real light levels that cause saturation are
dependent on the integration time.

> 
> /me feels stupid...
> 
> Sorry for the noise!
No problem.  It's interesting to understand where the limitations on
some of these techniques lie and I hadn't thought about the issue
of saturation as previous times we've done this have typically been
on ADCs doing oversampling or similar where we don't get the same problem.

Jonathan

> 
> --Matti
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [low prio, just pondering] About the light sensor "sensitivity area"
  2023-02-25  9:35         ` [low prio, just pondering] About the light sensor "sensitivity area" Matti Vaittinen
@ 2023-03-04 20:26           ` Jonathan Cameron
  0 siblings, 0 replies; 20+ messages in thread
From: Jonathan Cameron @ 2023-03-04 20:26 UTC (permalink / raw)
  To: Matti Vaittinen; +Cc: Vaittinen, Matti, Jonathan Cameron, linux-iio

On Sat, 25 Feb 2023 11:35:14 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 2/6/23 16:34, Vaittinen, Matti wrote:
> > On 2/2/23 18:57, Jonathan Cameron wrote:  
> >> On Tue, 31 Jan 2023 09:31:53 +0000
> >> "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> >>  
> >>> On 1/30/23 15:02, Jonathan Cameron wrote:  
> >>>> On Mon, 30 Jan 2023 14:04:53 +0200
> >>>> Matti Vaittinen <mazziesaccount@gmail.com> wrote:  
> >>>
> >>> As a side note - I had always thought measuring the light is just simple
> >>> value reading from a sensor. I never regarded the fact that human eye
> >>> sees only certain wavelengths or that the sensors sensitivity changes
> >>> depending on the wavelength. It's funny how I always end up knowing less
> >>> when I know more ;)  
> >>
> >> Light sensors are a pain in many ways!  We've had a few datasheets over the
> >> years that have insisted that the color channels have well specified units
> >> despite there being no such definition that I'm aware of (and no means
> >> to define one for cheap sensors. What is standard Red? :))  All the
> >> illuminance values are just approximations to the official curve.
> >>
> >> Sometime in the past we debated trying to describe the curves, but it's
> >> hard to do as they aren't nice shapes (so 3DB points don't make much
> >> sense).  
> 
> This is a low-priority mail with just some very initial pondering. Feel 
> free to skip this if you're in a hurry.
> 
> I guess the problem of telling what the sensor values represent for 
> sensors where the sensitivity is a poor match to a colour has been 
> dwelling in the background :)
> 
> I don't have any long experience on these devices so I have seen only 
> couple of the light sensor data-sheets, and mostly just for ROHM 
> sensors. Maybe this is the reason why the common thing I have seen in 
> these data-sheets representing the sensitivity to wave lengths has been 
> a "spectral response" curve. All of the data-sheets have represented a 
> curve where "sensor responsivity" is in the Y-Axis and wave length at 
> the X-axis. And yes, in many cases this curve (especially for a CLEAR 
> light) is of arbitrary shape for example like this:

That is indeed typical info to find on a datasheet though IIRC the
y axis meaning varies a bit (log values sometimes for example)

> 
> 
> 
> 
> S                                           ***
> e                                      *       *
> n                                  *            *
> s                        **       *             *
> i                   *       **   *               *
> t                *             *                  *
> i              *                                  *
> v             *                                    *
> i         **                                       *
> t      *                                           *
> y    *                                             *
>     *                                                ***
>    *                                                     ******
>   *                                                            *
>   *                                                            *
> 400		500		600		700		800nm
>                 W a v e l e n g h t
> 
> 
> Having this in mind it seems to be impossible to have just one or a few 
> categories of sensitivity, or to describe it accurately by just some 
> "peak-sensitivity" wave-lenght and a value representing "width of the 
> curve".
> 
> So, maybe we should abandon the idea of having a great categorization / 
> abstraction in-driver or IIO framework (other than the R,G,B,C,IR,UV - 

Can't abandon them in general as ABI we need to carry on supporting and
in many cases that's enough info for the application.  Can expand beyond
them though.

> which works fine for some sensors). What I could think of is providing a 
> set of 'data points' representing the sensitivity curves. Say, we had 
> in_sensitivity_wavelength_calibpoints and 
> in_sensitivity_wavelength_num_calibpoints (or what ever could fit for 
> the IIO naming scheme) - where user could get sensor provided datapoints 
> that represent the sensitivity as a function of wavelength. Userland 
> could then decide the best curve fitting for the data-points and compute 
> the sensitivity according to the best available algorithms. I think this 
> kind of curve-fitting-to-datapoints is quite standard stuff in the 
> user-space these days - but it feels like an overwhelming task in the 
> kernel land/drivers...

A more general approach would be to mandate the curve fitting then require
drivers to provide sufficient values that the approximation is within
X percent of the value from the datasheet.

Otherwise it becomes a question of what wavelengths to use.
> 
> This all is just some pondering. I do not have a proper use-case for 
> this kind of a sensitivity curve data as I work for a component vendor 
> instead of doing actual systems utilizing these components :/ It's 
> actually a little sad as I seem to keep thinking what kind of a device I 
> could build using these components - just to end up noticing that I am 
> not in a position where I was building these devices :p (You wouldn't 
> believe how cool imaginary clocking device for driving a camera clock 
> with light sensor detecting flickering I just designed in my head the 
> other night XD).

They are fun. I'll admit my main experience of these has come with devices
that 'happen to have them' rather than ever having designed them into
anything (and last actual board design I got involved in was many years
ago).

> 
> Well, I still hope I can help creating device driver/framework stuff 
> people can use to build devices - in the end of the day it will also 
> benefit the component vendor as the components are typically used in 
> these devices ;)
> 
> Oh. Got carried away. Anyways, have you considered just offering an 
> entry with sensitivity data-points instead of offering wavelength and 
> 3DB-limits? Do you think that could be useful?

As you've noted, 3DB doesn't work for this, but I think remains
useful for time domain filters. 

I think it would potentially be useful to have better data, but what
we really need is a user to tell use what they need.

Until that happens we may well be either over or under designing
the solution.

Jonathan
 
> 
> Yours,
> 	-- Matti
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-03-04 20:26 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-30 12:04 ROHM ALS, integration time Matti Vaittinen
2023-01-30 13:02 ` Jonathan Cameron
2023-01-30 13:42   ` Vaittinen, Matti
2023-01-30 17:12     ` Jonathan Cameron
2023-01-30 18:19       ` Matti Vaittinen
2023-01-30 20:19         ` Jonathan Cameron
2023-01-31 19:58           ` Jonathan Corbet
2023-02-01  5:55             ` Matti Vaittinen
2023-01-31  9:31   ` Vaittinen, Matti
2023-02-02 16:57     ` Jonathan Cameron
2023-02-06 14:34       ` Vaittinen, Matti
2023-02-18 17:20         ` Jonathan Cameron
2023-02-18 18:08           ` Matti Vaittinen
2023-02-26 17:26             ` Jonathan Cameron
2023-02-26 17:30             ` Jonathan Cameron
2023-02-27  7:22               ` Matti Vaittinen
2023-02-27  9:54                 ` Matti Vaittinen
2023-03-04 18:37                   ` Jonathan Cameron
2023-02-25  9:35         ` [low prio, just pondering] About the light sensor "sensitivity area" Matti Vaittinen
2023-03-04 20:26           ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.