Re: [PATCH v6 2/2] iio: light: isl76682: Add ISL76682 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 .

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 !




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux