Re: [PATCH 1/5] dt-bindings: iio: adc: Add TI ADS126x ADC family

[Krzysztof Kozlowski <krzk@kernel.org>]

On 14/06/2026 22:53, Kurt Borja wrote:
> Hi Krzysztof,
> 
> On Sat Jun 13, 2026 at 1:54 PM -05, Krzysztof Kozlowski wrote:
>> On Fri, Jun 12, 2026 at 05:46:19PM -0500, Kurt Borja wrote:
>>> +  ti,neg-refmux:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: |
>>> +      Selects the negative voltage reference input:
>>> +      0: Internal 2.5 V reference
>>> +      1: AIN1 pin
>>> +      2: AIN3 pin
>>> +      3: AIN5 pin
>>> +      4: AVSS pin
>>> +    minimum: 0
>>> +    maximum: 4
>>> +    default: 0
>>> +
>>> +  ti,vbias:
>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>> +    description: Enables the level-shift voltage on the AINCOM pin.
>>> +    default: false
>>
>> There is no such syntax, drop.
> 
> The "default: false" syntax? Sure I'll drop.
> 
>>
>>> +
>>> +  ti,idac1-pin:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: |
>>> +      Selects the analog input pin to connect IDAC1:
>>> +      0: AIN0
>>> +      1: AIN1
>>> +      2: AIN2
>>> +      3: AIN3
>>> +      4: AIN4
>>> +      5: AIN5
>>> +      6: AIN6
>>> +      7: AIN7
>>> +      8: AIN8
>>> +      9: AIN9
>>> +      10: AINCOM
>>> +      11: No Connection
>>> +    minimum: 0
>>> +    maximum: 11
>>> +    default: 11
>>> +
>>> +  ti,idac1-microamp:
>>> +    description: Selects the current values of IDAC1.
>>> +    enum: [0, 50, 100, 250, 500, 750, 1000, 1500, 2000, 2500, 3000]
>>> +    default: 0
>>> +
>>> +  ti,idac2-pin:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: |
>>> +      Selects the analog input pin to connect IDAC2:
>>> +      0: AIN0
>>> +      1: AIN1
>>> +      2: AIN2
>>> +      3: AIN3
>>> +      4: AIN4
>>> +      5: AIN5
>>> +      6: AIN6
>>> +      7: AIN7
>>> +      8: AIN8
>>> +      9: AIN9
>>> +      10: AINCOM
>>> +      11: No Connection
>>> +    minimum: 0
>>> +    maximum: 11
>>> +    default: 11
>>> +
>>> +  ti,idac2-microamp:
>>> +    description: Selects the current values of IDAC2.
>>> +    enum: [0, 50, 100, 250, 500, 750, 1000, 1500, 2000, 2500, 3000]
>>> +    default: 0
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  '#io-channel-cells':
>>> +    const: 1
>>> +
>>> +  '#gpio-cells':
>>> +    const: 2
>>> +
>>> +  gpio-controller: true
>>> +
>>> +  adc:
>>> +    $ref: /schemas/iio/adc/ti,ads1263-adc2.yaml#
>>
>> Not a separate device node. Fold into the parent... or explain in
>> commit msg. You have entire commit msg to explain odd things.
>>
>> In that binding description you call it "independent", so it should have
>> its own SPI chip select? Why "independent" and part of this binding?
>> Maybe not independent, so basically part of this device?
> 
> It's independent in the sense that it is a proper subdevice on the same

You cannot use DT syntax as argument why you use DT syntax like that.


> chip. It shares the serial interface but operates completely in
> parallel.

How completely in parallel? If the interface is the same, then it does
not operate in parallel. It's impossible.

> 
> I decided to add a subnode because other devices might request their
> io-channels and most importantly a different voltage reference might be
> connected to it.
> 
> I'll clarify this in the commmit message on the next version. Although
> after seeing this submitted bindings [1], I wonder if it's a better
> approach to do something like
> 
> 	spi@0 {
> 		mydevice@0 {
> 			...
> 			adc@0 { ... };
> 			adc@1 { ... };
> 		};
> 	};
> 
> Any thoughts?

Does not look like separate subnode. You still did not provide arguments
why this is independent.

Best regards,
Krzysztof