lkml.org 
[lkml]   [2022]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2] iio: light: ltrf216a: Add raw attribute
From

On 14/08/22 21:52, Jonathan Cameron wrote:
> On Fri, 12 Aug 2022 15:34:24 +0530
> Shreeya Patel <shreeya.patel@collabora.com> wrote:
>
>> Add IIO_CHAN_INFO_RAW to the mask to be able to read raw values
>> from the light sensor.
>>
>> The userspace code for brightness control in steam deck uses the
>> in_illuminance_input value through sysfs and multiplies it
>> with a constant stored in BIOS at factory calibration time.
>>
>> The downstream driver for LTRF216A that we have been using
>> has incorrect formula for LUX calculation which we corrected
>> in the upstreamed driver.
>>
>> Now to be able to use the upstreamed driver, we need to add some
>> magic in userspace so that the brightness control works like before
>> even with the updated LUX formula.
>>
>> Hence, we need the raw data to calculate a constant that can be
>> added in userspace code.
>>
>> Downstream driver LUX formula :-
>> (greendata*8*LTRF216A_WIN_FAC) / (data->als_gain_fac*data->int_time_fac*10)
>>
>> Upstreamed driver LUX formula :-
>> (greendata*45*LTRF216A_WIN_FAC) / (data->als_gain_fac*data->int_time_fac)
>>
>> greendata is the ALS_DATA which we would like to get through sysfs using
>> the raw attribute.
>>
>> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
> Hi Shreeya.
>
> Your description above makes me wonder though if we should support
> this as an intensity channel as we did for many of the early Ambient light
> sensors. Not sure why it's called 'greendata' btw!
> For those early tsl2583 IIRC and similar, we had two sensors with infrared vs
> visible+infrared (which is basically what clear is here).
> The readings given were of those two sensors then we did a bunch of maths
> to convert those to LUX (in simplest drivers we simply subtracted
> the infrared part from visible and applied a scale factor)
>
> That lead to IIO_TYPE_BOTH though we later added IIO_TYPE_CLEAR which is
> subtly different as that was for color sensors with RGB and clearish
> filters. The value you want here doesn't really correspond to any of
> those modifiers
>
> I guess that brings us back around to LIGHT(illuminance) + raw as you have it.
> or adding a 'visible' modifier which is also rather ugly and hard
> to define.
>
> Let's leave this on list a while longer to see if others comment.
> For now I'm inclined to just accept this as a dirty hack needed for this
> corner case.
Hi Jonathan,

I was wondering if it's fine to merge this now since we haven't got
any other comments on it.


Thanks
Shreeya Patel
> Jonathan
>
>> ---
>>
>> Changes in v2
>> - Add a better commit message explaining why we want this change.
>> - Call ltrf216a_set_power_state(data, false) before return.
>>
>>
>> drivers/iio/light/ltrf216a.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/iio/light/ltrf216a.c b/drivers/iio/light/ltrf216a.c
>> index e6e24e70d2b9..4b8ef36b6912 100644
>> --- a/drivers/iio/light/ltrf216a.c
>> +++ b/drivers/iio/light/ltrf216a.c
>> @@ -93,6 +93,7 @@ static const struct iio_chan_spec ltrf216a_channels[] = {
>> {
>> .type = IIO_LIGHT,
>> .info_mask_separate =
>> + BIT(IIO_CHAN_INFO_RAW) |
>> BIT(IIO_CHAN_INFO_PROCESSED) |
>> BIT(IIO_CHAN_INFO_INT_TIME),
>> .info_mask_separate_available =
>> @@ -259,6 +260,18 @@ static int ltrf216a_read_raw(struct iio_dev *indio_dev,
>> int ret;
>>
>> switch (mask) {
>> + case IIO_CHAN_INFO_RAW:
>> + ret = ltrf216a_set_power_state(data, true);
>> + if (ret)
>> + return ret;
>> + mutex_lock(&data->lock);
>> + ret = ltrf216a_read_data(data, LTRF216A_ALS_DATA_0);
>> + mutex_unlock(&data->lock);
>> + ltrf216a_set_power_state(data, false);
>> + if (ret < 0)
>> + return ret;
>> + *val = ret;
>> + return IIO_VAL_INT;
>> case IIO_CHAN_INFO_PROCESSED:
>> mutex_lock(&data->lock);
>> ret = ltrf216a_get_lux(data);
>

\
 
 \ /
  Last update: 2022-08-23 09:39    [W:0.064 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site