On Sun, 2 Mar 2025 15:10:12 +0200 Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote: > On 02/03/2025 05:27, Jonathan Cameron wrote: > > On Wed, 26 Feb 2025 08:39:11 +0200 > > Matti Vaittinen <mazziesaccount@xxxxxxxxx> 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@xxxxxxxxx> > >>>> > >>>> --- > >>>> 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.
