From: Jonathan Cameron <jic23@kernel.org>
To: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
Cc: Jonathan Cameron <Jonathan.Cameron@Huawei.com>,
Matti Vaittinen <mazziesaccount@gmail.com>,
linux-iio <linux-iio@vger.kernel.org>,
"Mutanen, Mikko" <Mikko.Mutanen@fi.rohmeurope.com>
Subject: Re: ROHM ALS, integration time
Date: Sat, 18 Feb 2023 17:20:52 +0000 [thread overview]
Message-ID: <20230218172052.12c44aa5@jic23-huawei> (raw)
In-Reply-To: <11722ea9-7149-0305-5593-7a66dc1d73f0@fi.rohmeurope.com>
> >> 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
>
next prev parent reply other threads:[~2023-02-18 17:06 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230218172052.12c44aa5@jic23-huawei \
--to=jic23@kernel.org \
--cc=Jonathan.Cameron@Huawei.com \
--cc=Matti.Vaittinen@fi.rohmeurope.com \
--cc=Mikko.Mutanen@fi.rohmeurope.com \
--cc=linux-iio@vger.kernel.org \
--cc=mazziesaccount@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox