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

Jonathan Cameron
2025-09-07T10:51:18Z

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