[PATCH v4 07/10] iio: adc: ti-ads7924: Respect device tree config

[Hugo Villeneuve]

On Sun, 2 Mar 2025 15:10:12 +0200
Matti Vaittinen <mazziesaccount at gmail.com> wrote:

> On 02/03/2025 05:27, Jonathan Cameron wrote:
> > On Wed, 26 Feb 2025 08:39:11 +0200
> > Matti Vaittinen <mazziesaccount at gmail.com> wrote:
> > 
> >> On 26/02/2025 02:09, David Lechner wrote:
> >>> On 2/24/25 12:34 PM, Matti Vaittinen wrote:
> >>>> 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 at 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 :)

Hi Matti,
I haven't really been able to follow all discussions, but as an
historic note I developped this driver for a prototype that never
materialized. So any changes would not have an impact, as far as I am
concerned.

Hugo.