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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250303095717.56ef5ac016b99911fc34e198@hugovil.com>
Date: Mon, 3 Mar 2025 09:57:17 -0500
From: Hugo Villeneuve <hugo@...ovil.com>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Jonathan Cameron <jic23@...nel.org>, David Lechner
 <dlechner@...libre.com>, Matti Vaittinen
 <matti.vaittinen@...rohmeurope.com>, Hugo Villeneuve
 <hvilleneuve@...onoff.com>, Lars-Peter Clausen <lars@...afoo.de>, Rob
 Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
 Dooley <conor+dt@...nel.org>, Andy Shevchenko
 <andriy.shevchenko@...ux.intel.com>, Daniel Scally <djrscally@...il.com>,
 Heikki Krogerus <heikki.krogerus@...ux.intel.com>, Sakari Ailus
 <sakari.ailus@...ux.intel.com>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
 Danilo Krummrich <dakr@...nel.org>, Lad Prabhakar
 <prabhakar.mahadev-lad.rj@...renesas.com>, Chen-Yu Tsai <wens@...e.org>,
 Jernej Skrabec <jernej.skrabec@...il.com>, Samuel Holland
 <samuel@...lland.org>, Nuno Sa <nuno.sa@...log.com>, Javier Carrasco
 <javier.carrasco.cruz@...il.com>, Guillaume Stols <gstols@...libre.com>,
 Olivier Moysan <olivier.moysan@...s.st.com>, Dumitru Ceclan
 <mitrutzceclan@...il.com>, Trevor Gamblin <tgamblin@...libre.com>, Matteo
 Martelli <matteomartelli3@...il.com>, Alisa-Dariana Roman
 <alisadariana@...il.com>, Ramona Alexandra Nechita
 <ramona.nechita@...log.com>, AngeloGioacchino Del Regno
 <angelogioacchino.delregno@...labora.com>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-acpi@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev
Subject: Re: [PATCH v4 07/10] iio: adc: ti-ads7924: Respect device tree
 config

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

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