Re: [PATCH v3] iio: light: as73211: fix channel handling in only-color triggered buffer

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

 



Le 12/12/2024 à 18:56, Javier Carrasco a écrit :
The channel index is off by one unit if AS73211_SCAN_MASK_ALL is not
set (optimized path for color channel readings), and it must be shifted
instead of leaving an empty channel for the temperature when it is off.

Once the channel index is fixed, the uninitialized channel must be set
to zero to avoid pushing uninitialized data.

Add available_scan_masks for all channels and only-color channels to let
the IIO core demux and repack the enabled channels.

Cc: stable@xxxxxxxxxxxxxxx
Fixes: 403e5586b52e ("iio: light: as73211: New driver")
Tested-by: Christian Eggers <ceggers@xxxxxxx>
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx>

...

diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c
index be0068081ebb..4be2e349a068 100644
--- a/drivers/iio/light/as73211.c
+++ b/drivers/iio/light/as73211.c
@@ -177,6 +177,12 @@ struct as73211_data {
  	BIT(AS73211_SCAN_INDEX_TEMP) | \
  	AS73211_SCAN_MASK_COLOR)
+static const unsigned long as73211_scan_masks[] = {
+	AS73211_SCAN_MASK_ALL,
+	AS73211_SCAN_MASK_COLOR,
+	0,
+};
+
  static const struct iio_chan_spec as73211_channels[] = {
  	{
  		.type = IIO_TEMP,
@@ -672,9 +678,12 @@ static irqreturn_t as73211_trigger_handler(int irq __always_unused, void *p)
/* AS73211 starts reading at address 2 */

Should this comment be updated?

Or maybe moved close to "if (*indio_dev->active_scan_mask == AS73211_SCAN_MASK_ALL)" below?

  		ret = i2c_master_recv(data->client,
-				(char *)&scan.chan[1], 3 * sizeof(scan.chan[1]));
+				(char *)&scan.chan[0], 3 * sizeof(scan.chan[0]));
  		if (ret < 0)
  			goto done;
+
+		/* Avoid pushing uninitialized data */
+		scan.chan[3] = 0;
  	}
if (data_result) {
@@ -682,9 +691,15 @@ static irqreturn_t as73211_trigger_handler(int irq __always_unused, void *p)
  		 * Saturate all channels (in case of overflows). Temperature channel
  		 * is not affected by overflows.
  		 */
-		scan.chan[1] = cpu_to_le16(U16_MAX);
-		scan.chan[2] = cpu_to_le16(U16_MAX);
-		scan.chan[3] = cpu_to_le16(U16_MAX);
+		if (*indio_dev->active_scan_mask == AS73211_SCAN_MASK_ALL) {

Should [0]...

+			scan.chan[1] = cpu_to_le16(U16_MAX);
+			scan.chan[2] = cpu_to_le16(U16_MAX);
+			scan.chan[3] = cpu_to_le16(U16_MAX);
+		} else {
+			scan.chan[0] = cpu_to_le16(U16_MAX);
+			scan.chan[1] = cpu_to_le16(U16_MAX);
+			scan.chan[2] = cpu_to_le16(U16_MAX);

... and [3] be forced as-well?
(just a blind guess)

CJ

+		}
  	}
iio_push_to_buffers_with_timestamp(indio_dev, &scan, iio_get_time_ns(indio_dev));
@@ -758,6 +773,7 @@ static int as73211_probe(struct i2c_client *client)
  	indio_dev->channels = data->spec_dev->channels;
  	indio_dev->num_channels = data->spec_dev->num_channels;
  	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->available_scan_masks = as73211_scan_masks;
ret = i2c_smbus_read_byte_data(data->client, AS73211_REG_OSR);
  	if (ret < 0)

---
base-commit: 91e71d606356e50f238d7a87aacdee4abc427f07
change-id: 20241123-iio_memset_scan_holes-a673833ef932

Best regards,





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux