Re: [PATCH v10 3/8] iio: adc: add helpers for parsing ADC nodes

[Matti Vaittinen]

On 31/03/2025 12:48, Jonathan Cameron wrote:
> On Mon, 31 Mar 2025 08:39:35 +0300
> Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
>
> > Hi Marcelo,
> >
> > Thanks for the review!
> >
> > On 30/03/2025 23:19, Marcelo Schmitt wrote:
> > > Hi Matti,
> > >
> > > The new helpers for ADC drivers look good to me.
> > > I am now very late to complain about anything but am leaving some minor comments
> > > below that can be completely ignored.
> > >
> > > Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@xxxxxxxxx>
> > >
> > > Thanks,
> > > Marcelo
> > >
> > > On 03/24, Matti Vaittinen wrote:
> > > > There are ADC ICs which may have some of the AIN pins usable for other
> > > > functions. These ICs may have some of the AIN pins wired so that they
> > > > should not be used for ADC.
> > > >
> > > > (Preferred?) way for marking pins which can be used as ADC inputs is to
> > > > add corresponding channels@N nodes in the device tree as described in
> > > > the ADC binding yaml.
> > > Not sure it's preferred to have ADC channels always declared in dt. That
> > > question was somewhat also raised during ADC doc review [1].
> >
> > I had missed that doc and the review. Interesting read, thanks for
> > pointing it :)
> >
> > We did also do a bit discussion about this during the review of the
> > earlier versions. I am not sure if we found an ultimate common consensus
> > though :)
> >
> > A recap as seen through my eyes:
> >
> > - It is preferred to have either _all_ or _none_ of the channels
> > described in the device tree.
> > https://lore.kernel.org/all/20250201162631.2eab9a9a@jic23-huawei/
> >
> > - This, however, is not _always_ required to be followed, and it may be
> > impractical in some cases:
> > https://lore.kernel.org/linux-iio/6f6e6550-5246-476f-9168-5e24151ab165@xxxxxxxxxxxx/#t
> >
> > - We do have bunch of existing drivers which we need to support. With
> > some very different approaches to bindings.
> > https://lore.kernel.org/linux-iio/20250302032054.1fb8a011@jic23-huawei/
> >
> >
> > My _personal_ thinking is that:
> >
> > This means that we can't hide the binding parsing in the IIO-core. We
> > can't go and change the channels in existing drivers.
> >
> > But, we can provide helpers (like this one) for drivers to use. I also
> > believe we should still try to have common (and preferred!) approach for
> > the _new_ drivers. Eventually, the new ones will be majority. Some of
> > the old ones die, and if we keep same practices for new ones, the old
> > ones will become rare exceptions while majority follows same principles ;)
> >
> > > In short, ADC
> > > channel may and may not be declared under ADC dt node. ADC bindings often don't
> > > enforce channels to be declared. On IIO side of things, many ADC drivers just
> > > populate channels even if they are not declared in dt.
> > > The ADCs you are supporting in the other patches of this series seem to require
> > > dt declared channels though.
> > >
> > > [1]: https://lore.kernel.org/linux-iio/20250118155153.2574dbe5@jic23-huawei/
> > >
> > > Would something like
> > >
> > > A common way of marking pins that can be used as ADC inputs is to add
> > > corresponding channel@N nodes in the device tree as described in the ADC
> > > binding yaml.
> > >
> > > be a good rephrasing of the above paragraph?
> >
> > Yes, if we don't want to guide new drivers to either have all usable
> > channels, or no channels in the device tree.
> >
> > I think Jonathan said he'll be rebasing this to rc1. I am a newcomer and
> > I should not enforce my view over more experienced ones ;) So, feel free
> > to reword the description as Marcelo suggests if you don't think we
> > should prefer one direction or the other.
>
> I've gone with Marcelo's suggestion because I don't want to be too specific
> here given the complex history.   We can absolutely encourage the all or
> nothing description going forwards though as it is logical in the vast
> majority of cases.

Thanks for taking care of it :)

Yours,
	-- Matti