Re: [PATCH 2/5] iio: adc: Add ti-ads1262 driver

[David Lechner]

On 6/22/26 4:47 AM, Jonathan Cameron wrote:
> On Sun, 21 Jun 2026 19:18:33 -0500
> "Kurt Borja" <kuurtb@xxxxxxxxx> wrote:
> 
>> On Sun Jun 21, 2026 at 9:33 AM -05, Jonathan Cameron wrote:
>>> On Mon, 15 Jun 2026 06:30:28 +0200
>>> Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote:
>>>  
>>>> On 14/06/2026 22:56, Kurt Borja wrote:  
>>>>> On Sat Jun 13, 2026 at 1:59 PM -05, Krzysztof Kozlowski wrote:
>>>>>
>>>>> [...]
>>>>>     
>>>>>> Functions used by probe() should be before probe(), not somewhere in the
>>>>>> middle of the code. IOW, entire probe is together.    
>>>>>
>>>>> I they all are, it's just that regmap stuff takes a huge chunk. I'll
>>>>> check how to reorganize.
>>>>>
>>>>> [...]
>>>>>     
>>>>>>> +static const struct of_device_id ads1262_of_match[] = {
>>>>>>> +	{ .compatible = "ti,ads1262" },
>>>>>>> +	{ .compatible = "ti,ads1263" },    
>>>>>>
>>>>>> So devices are fully compatible? Then it should be expressed in the
>>>>>> binding and drop one entry here.    
>>>>>
>>>>> Not fully compatible as Jonathan said. One is a subset of the other.    
>>>>
>>>> This is THE meaning of compatible!  
>>>
>>> This one I'm in agreement with. It is a strict subset, so should be
>>> using a fallback.  If the fallback is used, you just get support of the
>>> stuff in the simpler chip (or if you can override it with a chip ID
>>> you might still 'upgrade' to the more complex driver support).
>>> If you do end up with properties that only apply to 'new' parts of
>>> the more complex chip then they should be verified as part of the
>>> binding (assuming you can do that without the verifier complaining
>>> - I haven't checked!)  
>>
>> In v1 I had the "adc" subnode which was specific to ADS1263. Then I
>> agreed to drop the subnode but I'm having second thoughts...
>>
>> If we dropped it, then we would still have some specific stuff.
>> #io-channel-cells would be "const: 2" in ADS1263 chips. Also ADS1263's
>> channels would have an extra ti,vref-adc2 prop, for ADC2 voltage
>> reference selection. I should maybe also add a vref-adc2-supply.
>>
>> Maybe it's better to keep the subnode or, again, go for something like:
>>
>>     spi {
>>         multi-adc@0 {
>>             adc@0 {
>>                 ...
>>                 vref-suppy = <&adc1-vref>;
>>
>>                 channel@0 {
>>                     ...
>>                     reference-source = <ADS1262_VREF_AIN0_AIN1>;
>>                 };
>>             };
>>             adc@1 {
>>                 ...
>>                 vref-suppy = <&adc2-vref>;
>>
>>                 channel@0 {
>>                     ...
>>                     reference-source = <ADS1262_VREF_AIN2_AIN3>;
>>                 };
>>             };
>>         };
>>     };
>>
>> In this case we would have to kinda duplicate channel description, but I
>> don't think it's that bad.
>>
>> Jonathan, Krzysztof, David, thoughts?
>>
>> IMO the ADC2 specific voltage reference stuff is a strong argument for a
>> subnode or the above solution.
> 
> Given you end up with channel specific stuff that differs I think it probably
> makes sense - though I do wonder a bit if that is real.  What's the use case
> for using a different reference for the monitoring / debug than the main one?
> I could imagine some dynamic use where you want to sanity check against
> a wider reference range, but maybe that needs userspace control rather than
> in here?  


I think is is going to mostly be the same, so could be simpler to just
add extra channel properties on an as-needed basis if things do actually
differ between ADC1 and ADC2 rather than having to define all channels
twice.

This seems pretty similar to the discussion of how to handle e.g. measuring
the same inputs with and without the burn-out current enabled in the
ti,ads112c14 series and I think you have convinced me that we should not
be having a separate channel in the devicetree for that either.

> 
> Jonathan
> 
> 
>>
>>>
>>> The SLF3F discussion is about (to me) less obvious case of not a strict
>>> subset, but rather being detectable parts with different channel related
>>> properties.  In that case the ID match is necessary for anything to work.
>>> Anyhow, that discussion is in a different thread and not really relevant
>>> here.
>>>
>>> Jonathan
>>>  
>>>>
>>>>
>>>> Best regards,
>>>> Krzysztof  
>>
>