Re: [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 5 Sep 2025 15:44:20 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 9/5/25 9:21 AM, Erim, Salih wrote:
> 
> ...
> 
> >>>  
> >>>> +
> >>>> +      xlnx,bipolar:
> >>>> +        $ref: /schemas/types.yaml#/definitions/flag
> >>>> +        description:
> >>>> +          If the supply has a bipolar type and the output will be signed.  
> >>>
> >>> This is very generic.  We have it described for ADC channels already
> >>> in bindings/iio/adc/adc.yaml.  Why can't we use that here?  
> >>
> >> no issue with it.
> >> And likely
> >> Documentation/devicetree/bindings/iio/adc/xlnx,zynqmp-ams.yaml
> >> should deprecated it and start to use new one.
> >>
> >>
> >>  
> >>> That binding does rely on matching against 'channel' for node names though.
> >>> Where a 'type of channel' has been relevant IIRC we've always added a
> >>> separate property rather than using the child node name.  
> >>
> >> Is this related to supply/temp channel name?
> >>
> >> I think one issue with the binding is that current schema allows to define
> >> supply@1  and also temp@1
> >> but both of them have reg = <1> which is not allowed (duplicate unit-address).
> >>
> >> Salih: What does this reg value means? Is it physical address where that sensor is
> >> placed?  
> > 
> > Reg is offset index to offset base of the memory addresses for each. Supplies and temp values
> > are located in different offsets.
> >   
> 
> Sounds like the .dts should looks like:
> 
> 	adc@f1270000 {
> 		compatible = "xlnx,versal-sysmon";
> 		reg = <0xf1270000 0x4000>;
> 		...
> 
> 		supply-channels {
> 			#size-cells = <0>;
> 			#address-cells = <1>;
> 
> 			channel@0 {
> 				reg = <0>;
> 				label = "vccint";
> 			};
> 
> 			...
> 		};
> 
> 		temperature-channels {
> 			#size-cells = <0>;
> 			#address-cells = <1>;
> 
> 			channel@0 {
> 				reg = <0>;
> 				label = "aie-temp-ch0";
> 			};
> 
> 			...
> 		};
> 	};

This works for me. Alternative would be something similar to what we did for
dt-bindings: iio: adc: Add AD4170-4 
Where there is an adi,sensor-type property in channels.
There they two types of channels were the same underlying hardware, it just provided
a way to constrain the other properties in the channel nodes.  That differs
from here where, as I understand it (Salih?) they are different hardware blocks
so 'reg' is an offset into a different set of registers. 

DT maintainers, I think this discussion would benefit from your guidance!

Thanks,

Jonathan