On Wednesday, 4 December 2024, 00:55:32 CET, Javier Carrasco wrote:
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.
Cc: stable@vger.kernel.org Fixes: 403e5586b52e ("iio: light: as73211: New driver") Signed-off-by: Javier Carrasco javier.carrasco.cruz@gmail.com
drivers/iio/light/as73211.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/light/as73211.c b/drivers/iio/light/as73211.c index be0068081ebb..2d45dfeda406 100644 --- a/drivers/iio/light/as73211.c +++ b/drivers/iio/light/as73211.c @@ -672,9 +672,12 @@ static irqreturn_t as73211_trigger_handler(int irq __always_unused, void *p) /* AS73211 starts reading at address 2 */ ret = i2c_master_recv(data->client,
(char *)&scan.chan[1], 3 * sizeof(scan.chan[1]));
(char *)&scan.chan[0], 3 * sizeof(scan.chan[0]));/* Avoid pushing uninitialized data */scan.chan[3] = 0;if (data_result) { @@ -682,9 +685,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) {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);}iio_push_to_buffers_with_timestamp(indio_dev, &scan, iio_get_time_ns(indio_dev));
With this change, having only X, Y and Z in the scan_mask (without the temperature channel) works fine.
But it looks that there is still another problem if a single color channel (e.g. X) is omitted from the scan mask (which probably wouldn't make much sense in practice). If I am right, the layout of scan.chan[] is also wrong for that case, so e.g. if omitting X, the application will get the X values where it expects the temperature value (which isn't read from the hardware at all).
Does it make sense to write a follow-up patch for this? I fear that taking all possible combinations into account could make the driver more complicated than necessary. Or is there a good example how to handle such combinations?
Tested-by: Christian Eggers ceggers@arri.de