Re: [PATCH V7 3/5] dt-bindings: iio: adc: Add support for QCOM PMIC5 Gen3 ADC

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 20-Oct-25 17:55, Krzysztof Kozlowski wrote:
> On 20/10/2025 14:51, Konrad Dybcio wrote:
>> On 10/17/25 3:40 PM, Krzysztof Kozlowski wrote:
>>> On 17/10/2025 13:18, Jishnu Prakash wrote:
>>>> Hi Krzysztof,
>>>>
>>>> On 10/9/2025 5:22 AM, Krzysztof Kozlowski wrote:
>>>>> On 08/10/2025 23:20, Jishnu Prakash wrote:
>>>>>> Hi Krzysztof,
>>>>>>
>>>>>> On 10/4/2025 12:22 PM, Krzysztof Kozlowski wrote:
>>>>>>> On Sat, 4 Oct 2025 at 11:42, Jishnu Prakash
>>>>>>> <jishnu.prakash@oss.qualcomm.com> wrote:
>>>>>>>>
>>>>>>>> Hi Jonathan,
>>>>>>>>
>>>>>>>> On 9/27/2025 7:17 PM, Jonathan Cameron wrote:
>>>>>>>>> On Fri, 19 Sep 2025 20:17:43 +0530
>>>>>>>>> Jishnu Prakash <jishnu.prakash@oss.qualcomm.com> wrote:
>>
>> [...]
>>
>>>> Can you please provide your suggestions on changes we can make
>>>> in the above points ?
>>>
>>> You just pasted DT. I asked about SW, software. Please read carefully
>>> previous comments.
>>
>> Is the problem that Jishnu included some indices in dt-bindings without
>> also adding them in the driver's adc5_gen3_chans_pmic[] array?
>>
>> As in, would the resolution to this thread be simply handling all of
>> them in the driver correctly?
> 
> The solution is to remove them from the bindings, just like we do with
> many other hardware constants. Of course if these are not hardware
> constants, but part of ABI, then solution would be different but no one
> provided proof or argument that this is any binding. All proofs were
> "but I want to use it in my DTS", which proofs nothing. Not a binding.
> 
> While this issue is not that important, we keep discussing it because
> author does not try to understand the problem or even keep up the
> discussion. Instead repeats the same without really reading my
> messages... and then disappears for month or more.

In Bulgaria, people shake their heads left to right to say "yes", and
up&down to say "no" (or so I've heard).. I feel like we're having a
similar situation here..

I'll try to make a case for keeping these defines in some form.
Here's hopefully all the related aspects, condensed down:

1. In a multi-PMIC setup, only the main PMIC's ADC is accessible by the OS.
   It then mediates accesses to secondary PMICs' ADCs through internal
   mechanisms, which requires the SID of the target to be retrieved and written
   to a register, along with the physical index of the desired channel to be
   measured (see patch 3/5 commit msg).

2. The PMIC SIDs are fixed per board and are the values of PMIC top-level
   nodes' reg property (since forever)

3. The channel indices are fixed in HW, but this patchset proposed to reuse
   them for logical mappings consumed through io-channels = <> as well (because
   of 1.), with the drivers taking the lower 8 bits that of reg/io-channels[1]
   value as the ADC channel id and the higher 8 bits as the SID (this is the
   define macros with an argument)

4. Fixing 3. in a "simply define all possible options and bind them to
   consecutive integers" fashion would require a huge table matching 0..n to
   [0-max_sid][0-max_chan] which is unreasonable

The alternative to the SID packing would be to reference the target PMIC
somehow, be it by referencing the PMIC itself:

io-channels = <&pm8550_adc &pmr735a CHANNEL_XYZ>

or by creating a faux node for the actual inaccessible ADC onboard each of
the PMICs:

io-channels = <&pm8550_adc &pmr735a_adc CHANNEL_XYZ>

and have the OS retrieve the SID from the DT node & encode that value instead
of hardcoding it in the DT, leaving just the actual channel IDs in dt-bindings.


The define macros without an argument do specify physical channel indices, but
we do need some sort of an identifier to put into io-channels (which is why this
lives in dt-bindings in the first place), and a 1:1 mapping to the physical id
sounds like a good option.


I don't think anyone objects to any of these resolutions, so long as they
are acceptable from your side

Konrad