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

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 31/03/2025 12:48, Jonathan Cameron wrote:
> On Mon, 31 Mar 2025 08:39:35 +0300
> Matti Vaittinen <mazziesaccount@gmail.com> 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@gmail.com>
>>>
>>> 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@baylibre.com/#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