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

[Jonathan Cameron]

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?  

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  
>