Re: [PATCH v4 07/10] iio: adc: ti-ads7924: Respect device tree config
From: Matti Vaittinen <mazziesaccount@gmail.com>
Date: 2025-03-02 13:10:16
Also in:
linux-acpi, linux-devicetree, linux-iio, linux-renesas-soc, linux-sunxi, lkml
On 02/03/2025 05:27, Jonathan Cameron wrote:
On Wed, 26 Feb 2025 08:39:11 +0200 Matti Vaittinen [off-list ref] wrote:quoted
On 26/02/2025 02:09, David Lechner wrote:quoted
On 2/24/25 12:34 PM, Matti Vaittinen wrote:quoted
The ti-ads7924 driver ignores the device-tree ADC channel specification and always exposes all 4 channels to users whether they are present in the device-tree or not. Additionally, the "reg" values in the channel nodes are ignored, although an error is printed if they are out of range. Register only the channels described in the device-tree, and use the reg property as a channel ID. Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> --- Revision history: v3 => v4: - Adapt to 'drop diff-channel support' changes to ADC-helpers - select ADC helpers in the Kconfig v2 => v3: New patch Please note that this is potentially breaking existing users if they have wrong values in the device-tree. I believe the device-tree should ideally be respected, and if it says device X has only one channel, then we should believe it and not register 4. Well, we don't live in the ideal world, so even though I believe this is TheRightThingToDo - it may cause havoc because correct device-tree has not been required from the day 1. So, please review and test and apply at your own risk :)The DT bindings on this one are a little weird. Usually, if we don't use any extra properties from adc.yaml, we leave out the channels. In this case it does seem kind of like the original intention was to work like you are suggesting, but hard to say since the driver wasn't actually implemented that way. I would be more inclined to actually not make the breaking change here and instead relax the bindings to make channel nodes optional and just have the driver ignore the channel nodes by dropping the ads7924_get_channels_config() function completely. This would make the driver simpler instead of more complex like this patch does.I have no strong opinion on this. I see this driver says 'Supported' in MAINTAINERS. Maybe Hugo is able to provide some insight?This seems to be ABI breakage. Never something we can take if there is a significant chance of anyone noticing. Here it looks like risk is too high.
Ok. I'll just drop this patch then. Thanks David & Jonathan :) Yours, -- Matti