On Sat Dec 14, 2024 at 3:22 PM CET, Jonathan Cameron wrote:
On Thu, 12 Dec 2024 18:56:32 +0100 Javier Carrasco javier.carrasco.cruz@gmail.com 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.
Add available_scan_masks for all channels and only-color channels to let the IIO core demux and repack the enabled channels.
Cc: stable@vger.kernel.org Fixes: 403e5586b52e ("iio: light: as73211: New driver") Tested-by: Christian Eggers ceggers@arri.de Signed-off-by: Javier Carrasco javier.carrasco.cruz@gmail.com
This issue was found after attempting to make the same mistake for a driver I maintain, which was fortunately spotted by Jonathan [1].
Keeping old sensor values if the channel configuration changes is known and not considered an issue, which is also mentioned in [1], so it has not been addressed by this series. That keeps most of the drivers out of the way because they store the scan element in iio private data, which is kzalloc() allocated.
This series only addresses cases where uninitialized i.e. unknown data is pushed to the userspace, either due to holes in structs or uninitialized struct members/array elements.
While analyzing involved functions, I found and fixed some triviality (wrong function name) in the documentation of iio_dev_opaque.
Link: https://lore.kernel.org/linux-iio/20241123151634.303aa860@jic23-huawei/ [1]
Changes in v3:
- as73211.c: add available_scan_masks for all channels and only-color channels to let the IIO core demux and repack the enabled channels.
- Link to v2: https://lore.kernel.org/r/20241204-iio_memset_scan_holes-v2-0-3f941592a76d@g...
Changes in v2:
- as73211.c: shift channels if no temperature is available and initialize chan[3] to zero.
- Link to v1: https://lore.kernel.org/r/20241125-iio_memset_scan_holes-v1-0-0cb6e98d895c@g...
drivers/iio/light/as73211.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
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,
I probably mislead you on this :( Needs to be the other way around as the core code starts at first entry whilst trying to find a mask that is a superset of what is turned on. here that means it will always use the first one. See iio_scan_mask_match() - strict isn't set int this case.
Thanks for your feedback. I copied your suggestion, but it was my fault that I did not make sure it was right for the intended behavior.
I will send a new version with that fixed soon.
Best regards, Javier Carrasco