Messages in this thread | | | Date | Sun, 19 Mar 2023 17:59:20 +0200 | Subject | Re: [PATCH v2 5/6] iio: light: ROHM BU27034 Ambient Light Sensor | From | Matti Vaittinen <> |
| |
On 3/18/23 18:54, Jonathan Cameron wrote: > On Mon, 13 Mar 2023 11:39:06 +0200 > Matti Vaittinen <mazziesaccount@gmail.com> wrote: > >> On 3/12/23 17:36, Jonathan Cameron wrote: >>> On Sun, 5 Mar 2023 14:22:51 +0200 >>> Matti Vaittinen <mazziesaccount@gmail.com> wrote: >>> >>>> On 3/4/23 22:17, Jonathan Cameron wrote: >>>>> On Thu, 2 Mar 2023 12:58:59 +0200 >>>>> Matti Vaittinen <mazziesaccount@gmail.com> wrote: >>>>> >> >> // snip >> >>>>>> +static const struct iio_chan_spec bu27034_channels[] = { >>>>>> + { >>>>>> + .type = IIO_LIGHT, >>>>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) | >>>>>> + BIT(IIO_CHAN_INFO_SCALE), >>>>> >>>>> What is this scale for? >>>> >>>> The scale is to inform users that we return data using milli lux. >>>> >>>>> Given the channel is computed from various different inputs, is there a >>>>> clear definition of how it is scaled? What does a write to it mean? >>>> >>>> Nothing. writing anything else but milli lux scale fails with -EINVAL. >>>> >>>> I guess I am doing something in an unusual way here :) Do you have a >>>> suggestion for me? >>> >>> Return data in lux? >> >> That's what I did originally have. But then I noticed we can get >> slightly better accuracy than that. Hence I switched to mLux and added >> the scale. >> >>> Or return it as INFO_RAW - thus making it clear >>> that the reading is not in expected units and a conversion must be >>> applied by userspace. SCALE is not applied to PROCESSED by userspace. >> >> Ah. This makes sense then. Maybe it would be worth adding a warning to >> IIO-core if drivers set both the SCALE and PROCESSED info bits? > > Hmm. I'm not sure that we don't have valid users of it even if they > are unusual. We also have some historical messes that do RAW + SCALE + > PROCESSED so we can't really have a warning on it. > > Warning generally is that the test tools that come with the kernel > will give you the wrong reading. :) > >> >> So, I need to select between the simplicity or better accuracy here? :/ >> I really hate ending up making choices like this without knowing all the >> real use-cases :( And it happens a lot for me. Well, I guess I'll drop >> the scale, use luxes and go with the PROCESSED data. My understanding is >> that the "thing" with the sensor is a wide-range for wavelengths, not >> the accuracy. So, maybe luxes are just good enough - and again, users >> needing something more accurate can utilize the raw intensity channels. > > Hmm. For the sysfs case you could use VAL_INT_PLUS_MICRO but that doesn't > then work well with the buffered path. > > It is perfectly valid to just have this as _RAW and keep your _SCALE so > that's probably the best option > _RAW doesn't have to mean totally raw, it just means userspace is expected > to applying a linear conversion to get a reading in the 'base' units for the channel.
Thanks for the insight!
I'll return the scale and switch to RAW for v5 :)
Yours, -- Matti
-- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
| |