iio: light: vcnl4035: fix scan buffer on big-endian

[PATCH] iio: light: vcnl4035: fix scan buffer on big-endian

Posted by David Lechner 3 weeks, 2 days ago

Rework vcnl4035_trigger_consumer_handler() so that we are not passing
what should be a u16 value as an int * to regmap_read(). This won't
work on bit endian systems.

Instead, add a new unsigned int variable to pass to regmap_read(). Then
copy that value into the buffer struct.

The buffer array is replaced with a struct since there is only one value
being read. This allows us to use the correct u16 data type and has a
side-effect of simplifying the alignment specification.

Also fix the endianness of the scan format from little-endian to CPU
endianness. Since we are using regmap to read the value, it will be
CPU-endian.

Fixes: 55707294c4eb ("iio: light: Add support for vishay vcnl4035")
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/light/vcnl4035.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/light/vcnl4035.c b/drivers/iio/light/vcnl4035.c
index 963747927425..16aeb17067bc 100644
--- a/drivers/iio/light/vcnl4035.c
+++ b/drivers/iio/light/vcnl4035.c
@@ -103,17 +103,23 @@ static irqreturn_t vcnl4035_trigger_consumer_handler(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct vcnl4035_data *data = iio_priv(indio_dev);
 	/* Ensure naturally aligned timestamp */
-	u8 buffer[ALIGN(sizeof(u16), sizeof(s64)) + sizeof(s64)]  __aligned(8) = { };
+	struct {
+		u16 als_data;
+		aligned_s64 timestamp;
+	} buffer = { };
+	unsigned int val;
 	int ret;
 
-	ret = regmap_read(data->regmap, VCNL4035_ALS_DATA, (int *)buffer);
+	ret = regmap_read(data->regmap, VCNL4035_ALS_DATA, &val);
 	if (ret < 0) {
 		dev_err(&data->client->dev,
 			"Trigger consumer can't read from sensor.\n");
 		goto fail_read;
 	}
-	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
-					iio_get_time_ns(indio_dev));
+
+	buffer.als_data = val;
+	iio_push_to_buffers_with_timestamp(indio_dev, &buffer,
+					   iio_get_time_ns(indio_dev));
 
 fail_read:
 	iio_trigger_notify_done(indio_dev->trig);
@@ -381,7 +387,7 @@ static const struct iio_chan_spec vcnl4035_channels[] = {
 			.sign = 'u',
 			.realbits = 16,
 			.storagebits = 16,
-			.endianness = IIO_LE,
+			.endianness = IIO_CPU,
 		},
 	},
 	{
@@ -395,7 +401,7 @@ static const struct iio_chan_spec vcnl4035_channels[] = {
 			.sign = 'u',
 			.realbits = 16,
 			.storagebits = 16,
-			.endianness = IIO_LE,
+			.endianness = IIO_CPU,
 		},
 	},
 };

---
base-commit: ff0843ceb1fb11a6b73e0e77b932ef7967aecd4b
change-id: 20260314-iio-light-vcnl4035c-clean-up-scan-buf-7aa00d36e470

Best regards,
--  
David Lechner <dlechner@baylibre.com>

Re: [PATCH] iio: light: vcnl4035: fix scan buffer on big-endian

Posted by Markus Elfring 3 weeks ago

> Rework vcnl4035_trigger_consumer_handler() so that we are not passing
> what should be a u16 value as an int * to regmap_read(). This won't
> work on bit endian systems.
…
          big?


See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.0-rc4#n34

Regards,
Markus

Re: [PATCH] iio: light: vcnl4035: fix scan buffer on big-endian

Posted by David Lechner 3 weeks ago

On 3/16/26 5:42 AM, Markus Elfring wrote:
>> Rework vcnl4035_trigger_consumer_handler() so that we are not passing
>> what should be a u16 value as an int * to regmap_read(). This won't
>> work on bit endian systems.
> …
>           big?
> 
> 
> See also once more:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.0-rc4#n34

Jonathan always adds the Cc: stable@ when appropriate, so I've gotten
in the habit of leaving it out.

> 
> Regards,
> Markus

Re: [PATCH] iio: light: vcnl4035: fix scan buffer on big-endian

Posted by Andy Shevchenko 3 weeks ago

On Sat, Mar 14, 2026 at 05:18:10PM -0500, David Lechner wrote:
> Rework vcnl4035_trigger_consumer_handler() so that we are not passing
> what should be a u16 value as an int * to regmap_read(). This won't
> work on bit endian systems.
> 
> Instead, add a new unsigned int variable to pass to regmap_read(). Then
> copy that value into the buffer struct.
> 
> The buffer array is replaced with a struct since there is only one value
> being read. This allows us to use the correct u16 data type and has a
> side-effect of simplifying the alignment specification.
> 
> Also fix the endianness of the scan format from little-endian to CPU
> endianness. Since we are using regmap to read the value, it will be
> CPU-endian.

...

> -	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
> -					iio_get_time_ns(indio_dev));
> +
> +	buffer.als_data = val;
> +	iio_push_to_buffers_with_timestamp(indio_dev, &buffer,
> +					   iio_get_time_ns(indio_dev));

Do you have plans to drop this old API as well?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] iio: light: vcnl4035: fix scan buffer on big-endian

Posted by David Lechner 3 weeks ago

On 3/16/26 5:15 AM, Andy Shevchenko wrote:
> On Sat, Mar 14, 2026 at 05:18:10PM -0500, David Lechner wrote:
>> Rework vcnl4035_trigger_consumer_handler() so that we are not passing
>> what should be a u16 value as an int * to regmap_read(). This won't
>> work on bit endian systems.
>>
>> Instead, add a new unsigned int variable to pass to regmap_read(). Then
>> copy that value into the buffer struct.
>>
>> The buffer array is replaced with a struct since there is only one value
>> being read. This allows us to use the correct u16 data type and has a
>> side-effect of simplifying the alignment specification.
>>
>> Also fix the endianness of the scan format from little-endian to CPU
>> endianness. Since we are using regmap to read the value, it will be
>> CPU-endian.
> 
> ...
> 
>> -	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
>> -					iio_get_time_ns(indio_dev));
>> +
>> +	buffer.als_data = val;
>> +	iio_push_to_buffers_with_timestamp(indio_dev, &buffer,
>> +					   iio_get_time_ns(indio_dev));
> 
> Do you have plans to drop this old API as well?
> 

Eventually, yes. My plan is to wait for fixes to go through first.

Re: [PATCH] iio: light: vcnl4035: fix scan buffer on big-endian

Posted by Jonathan Cameron 2 weeks, 2 days ago

On Mon, 16 Mar 2026 09:37:53 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 3/16/26 5:15 AM, Andy Shevchenko wrote:
> > On Sat, Mar 14, 2026 at 05:18:10PM -0500, David Lechner wrote:  
> >> Rework vcnl4035_trigger_consumer_handler() so that we are not passing
> >> what should be a u16 value as an int * to regmap_read(). This won't
> >> work on bit endian systems.
> >>
> >> Instead, add a new unsigned int variable to pass to regmap_read(). Then
> >> copy that value into the buffer struct.
> >>
> >> The buffer array is replaced with a struct since there is only one value
> >> being read. This allows us to use the correct u16 data type and has a
> >> side-effect of simplifying the alignment specification.
> >>
> >> Also fix the endianness of the scan format from little-endian to CPU
> >> endianness. Since we are using regmap to read the value, it will be
> >> CPU-endian.  
> > 
> > ...
> >   
> >> -	iio_push_to_buffers_with_timestamp(indio_dev, buffer,
> >> -					iio_get_time_ns(indio_dev));
> >> +
> >> +	buffer.als_data = val;
> >> +	iio_push_to_buffers_with_timestamp(indio_dev, &buffer,
> >> +					   iio_get_time_ns(indio_dev));  
> > 
> > Do you have plans to drop this old API as well?
> >   
> 
> Eventually, yes. My plan is to wait for fixes to go through first.

Agreed. Given the need for backports, better to do it in two steps.
Sooner or later we'll end up backporting the new _ts() variant
(if it hasn't already happened!) but lets not rush it.

Applied and marked for stable
Thanks,

Jonathan