On Tue, 5 Dec 2023 02:24:51 +0100
Marek Vasut <marex@xxxxxxx> wrote:
> On 12/4/23 15:35, Jonathan Cameron wrote:
> > On Mon, 4 Dec 2023 12:23:06 +0100
> > Marek Vasut <marex@xxxxxxx> wrote:
> >
> >> On 12/4/23 12:20, Jonathan Cameron wrote:
> >>> On Mon, 27 Nov 2023 22:26:53 +0100
> >>> Marek Vasut <marex@xxxxxxx> wrote:
> >>>
> >>>> The ISL76682 is very basic ALS which only supports ALS or IR mode
> >>>> in four ranges, 1k/4k/16k/64k LUX. There is no IRQ support or any
> >>>> other fancy functionality.
> >>>>
> >>>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> >>>> Reviewed-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
> >>>> Signed-off-by: Marek Vasut <marex@xxxxxxx>
> >>> Hi Marek,
> >>>
> >>> Discussion around available on v5 made me look closer at that aspect.
> >>> You are providing all the available entries in the callback but they
> >>> shouldn't be exposed to actually read unless the *_available bitmap
> >>> bits corresponding to them are set.
> >>>
> >>> If you like I can just rip the unused code out whilst applying?
> >>> Or if you'd prefer to send a v7 that's great too.
> >>>
> >>> Otherwise everything looks good to me.
> >>
> >> Maybe just do that while applying and I'll test it right after to see
> >> whether something broke, that's probably fastest. Just let me know where
> >> this got applied. I have the device on my desk .
> >
> > Diff is below. Applied to the togreg
>
> I only found the commit in 'testing' branch , so I used that one .
I messed up it seems and didn't actually push the updated version.
Done so now along with squashing in the bracket tidy up.
>
> > branch of iio.git and initially pushed out
> > as testing for normal reasons + for you to test.
> >
> > Thanks,
> >
> > Jonathan
> >
> >
> > diff --git a/drivers/iio/light/isl76682.c b/drivers/iio/light/isl76682.c
> > index 15a68609985b..8605187bfb62 100644
> > --- a/drivers/iio/light/isl76682.c
> > +++ b/drivers/iio/light/isl76682.c
> > @@ -184,8 +184,6 @@ static int intensity_scale_available[] = {
> > 0, 673000,
> > };
> >
> > -static int integration_time_available[] = { 0, ISL76682_INT_TIME_US };
> > -
> > static int isl76682_read_avail(struct iio_dev *indio_dev,
> > struct iio_chan_spec const *chan,
> > const int **vals, int *type,
> > @@ -207,11 +205,6 @@ static int isl76682_read_avail(struct iio_dev *indio_dev,
> > default:
> > return -EINVAL;
> > }
> > - case IIO_CHAN_INFO_INT_TIME:
> > - *vals = integration_time_available;
> > - *length = ARRAY_SIZE(integration_time_available);
> > - *type = IIO_VAL_INT_PLUS_MICRO;
> > - return IIO_AVAIL_LIST;
> > default:
> > return -EINVAL;
> > }
>
> Ah, looking at the attrs before and after, now I get it:
>
> $ grep -H . /sys/bus/iio/devices/iio\:device0/*
> /sys/bus/iio/devices/iio:device0/in_illuminance_raw:21
> /sys/bus/iio/devices/iio:device0/in_illuminance_scale:0.015000
> /sys/bus/iio/devices/iio:device0/in_illuminance_scale_available:0.015
> 0.06 0.24 0.96
> /sys/bus/iio/devices/iio:device0/in_intensity_raw:33
> /sys/bus/iio/devices/iio:device0/in_intensity_scale:0.010500
> /sys/bus/iio/devices/iio:device0/in_intensity_scale_available:0.0105
> 0.042 0.168 0.673
> /sys/bus/iio/devices/iio:device0/integration_time_available:0.090
> /sys/bus/iio/devices/iio:device0/name:isl76682
>
> /sys/bus/iio/devices/iio:device0/in_illuminance_raw:22
> /sys/bus/iio/devices/iio:device0/in_illuminance_scale:0.015000
> /sys/bus/iio/devices/iio:device0/in_illuminance_scale_available:0.015000
> 0.060000 0.240000 0.960000
> /sys/bus/iio/devices/iio:device0/in_intensity_raw:24
> /sys/bus/iio/devices/iio:device0/in_intensity_scale:0.010500
> /sys/bus/iio/devices/iio:device0/in_intensity_scale_available:0.010500
> 0.042000 0.168000 0.673000
> /sys/bus/iio/devices/iio:device0/integration_time:0.090000
> /sys/bus/iio/devices/iio:device0/name:isl76682
>
> Thanks !
>
