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

Erim, Salih
2025-09-08T11:14:01Z

Hi David and Jonathan,

> -----Original Message-----
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, September 7, 2025 11:51 AM
> To: David Lechner <dlechner@baylibre.com>
> Cc: Erim, Salih <Salih.Erim@amd.com>; Simek, Michal
> <michal.simek@amd.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; linux-kernel@vger.kernel.org;
> monstr@monstr.eu; michal.simek@xilinx.com; git@xilinx.com; Anand Ashok
> Dumbre <anand.ashok.dumbre@xilinx.com>; Kadamathikuttiyil Karthikeyan Pillai,
> Anish <anish.kadamathikuttiyil-karthikeyan-pillai@amd.com>; Andy Shevchenko
> <andy@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Nuno Sá <nuno.sa@analog.com>; Rob Herring
> <robh@kernel.org>; open list:OPEN FIRMWARE AND FLATTENED DEVICE
> TREE BINDINGS <devicetree@vger.kernel.org>; open list:IIO SUBSYSTEM AND
> DRIVERS <linux-iio@vger.kernel.org>
> Subject: Re: [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> 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 seems to logical to me as well, only I am not sure if there are any restrictions.

> 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.

Yes, there two types of channels, temperatures, and supplies(voltages) distributed across SoC
Connected to same hardware block but sets of sensor value registers are separately located (base offsets).
Reg is just offset to sensors memory addresses their base offsets.

I will check the AD4170 example how it manages.
>
> DT maintainers, I think this discussion would benefit from your guidance!
>
> Thanks,
>
> Jonathan
>
>

Regards,
Salih.