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

Erim, Salih
2025-09-05T14:21:51Z

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Michal and Jonathan,

> -----Original Message-----
> From: Simek, Michal <michal.simek@amd.com>
> Sent: Friday, September 5, 2025 1:30 PM
> To: Jonathan Cameron <jonathan.cameron@huawei.com>
> Cc: linux-kernel@vger.kernel.org; monstr@monstr.eu; michal.simek@xilinx.com;
> git@xilinx.com; Erim, Salih <Salih.Erim@amd.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>; David Lechner
> <dlechner@baylibre.com>; Jonathan Cameron <jic23@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
>
>
>
> On 9/5/25 13:30, Jonathan Cameron wrote:
> > On Fri, 5 Sep 2025 10:41:44 +0200
> > Michal Simek <michal.simek@amd.com> wrote:
> >
> >> From: Salih Erim <salih.erim@amd.com>
> >>
> >> Add devicetree documentation for Xilinx Sysmon IP which is used for
> >> internal chip monitoring on Xilinx Versal SOCs.
> >>
> >> Co-developed-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
> >> Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@xilinx.com>
> >> Co-developed-by: Anish Kadamathikuttiyil Karthikeyan Pillai
> >> <anish.kadamathikuttiyil-karthikeyan-pillai@amd.com>
> >> Signed-off-by: Anish Kadamathikuttiyil Karthikeyan Pillai
> >> <anish.kadamathikuttiyil-karthikeyan-pillai@amd.com>
> >> Signed-off-by: Salih Erim <salih.erim@amd.com>
> >> Signed-off-by: Michal Simek <michal.simek@amd.com>
> >> ---
> >>
> >> .../bindings/iio/adc/xlnx,versal-sysmon.yaml | 235 ++++++++++++++++++
> >> 1 file changed, 235 insertions(+)
> >> create mode 100644
> >> Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml
> >> b/Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml
> >> new file mode 100644
> >> index 000000000000..a768395cade7
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.ya
> >> +++ ml
> >> @@ -0,0 +1,235 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/iio/adc/xlnx,versal-sysmon.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Xilinx Versal Sysmon
> >> +
> >> +maintainers:
> >> + - Salih Erim <salih.erim@amd.com>
> >> +
> >> +description:
> >> + The Xilinx Sysmon provides on-chip monitoring and control for the
> >> +supply
> >> + voltages and temperatures across the chip. Since there are only
> >> +160 supply
> >> + voltage registers and 184 measurement points, there is no constant
> >> +mapping
> >> + of supply voltage registers and the measurement points. User has
> >> +to select
> >> + the voltages to monitor in design tool. Depending on the
> >> +selection, a voltage
> >> + supply gets mapped to one of the supply registers. So, this
> >> +mapping information
> >> + is provided via description which contain the information of name of
> >> + the supply enabled and the supply register it maps to.
> >> +
> >> +properties:
> >> + compatible:
> >> + items:
> >> + - const: xlnx,versal-sysmon
> >> +
> >> + reg:
> >> + maxItems: 1
> >> + description: Sysmon Registers.
> >> +
> >> + interrupts:
> >> + maxItems: 1
> >> + description: Interrupt line for Sysmon.
> >> +
> >> + '#address-cells':
> >> + const: 1
> >> +
> >> + '#size-cells':
> >> + const: 0
> >> +
> >> + '#io-channel-cells':
> >> + const: 0
> >> +
> >> + xlnx,hbm:
> >> + type: boolean
> >> + description:
> >> + Exists if node refers to a HBM (High Bandwidth Memory) SLR (Super Logic
> Region).
> >> +
> >> + xlnx,nodeid:
> >> + $ref: /schemas/types.yaml#/definitions/uint32
> >> + description:
> >> + PLM specified sysmon node id.
> >> +
> >> + xlnx,numaiechannels:
> >> + $ref: /schemas/types.yaml#/definitions/uint32
> >> + minimum: 1
> >> + maximum: 64
> >> + description:
> >> + Total number of sysmon satellites close to AI Engine exposed as channels.
> >
> > Feels like some use - would make this easier to parse. xlnx,num-aie-channels.
> > Similar to the next one. How is this related to the number of child nodes?
>
> it is number of childs below. They can be calculated to get this number.
>
> >
> >
> >> +
> >> + xlnx,numchannels:
> >> + $ref: /schemas/types.yaml#/definitions/uint32
> >> + minimum: 1
> >> + maximum: 160
> >> + description:
> >> + Number of supply channels enabled in the design.
> >
> > Given you have subnodes called supplyxxx why is a count of those
> > needed or is this not counting those?
>
> possible.
>
>
> >
> >> +
> >> +patternProperties:
> >> + "^supply@([0-9]{1,2}|1[0-5][0-9])$":
> >> + type: object
> >> + description:
> >> + Represents the supplies configured in the design.
> >> +
> >> + properties:
> >> + reg:
> >> + $ref: /schemas/types.yaml#/definitions/uint32
> >> + minimum: 0
> >> + maximum: 159
> >> + description:
> >> + The supply number associated with the voltage.
> >> +
> >> + xlnx,name:
> >> + $ref: /schemas/types.yaml#/definitions/string
> >> + description:
> >> + Name of the supply enabled
> >
> > Would the generic property "label" be useable here?
>
> label should be fine.
>
>
> >
> >> +
> >> + 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.

>
> >
> >> +
> >> + required:
> >> + - reg
> >> + - xlnx,name
> >> +
> >> + additionalProperties: false
> >> +
> >> + "^temp@([1-9]|[1-5][0-9]|6[0-4])$":
> >> + type: object
> >> + description:
> >> + Represents the sysmon temperature satellites.
> >> +
> >> + properties:
> >> + reg:
> >> + minimum: 1
> >> + maximum: 64
> >> + description:
> >> + The sysmon temperature satellite number.
> >> +
> >> + xlnx,aie-temp:
> >> + $ref: /schemas/types.yaml#/definitions/flag
> >> + description:
> >> + If present it indicates the temperature satellite is in
> >> + close proximity with AI Engine
> >
> > This one seems unusual. I guess it makes a configuration difference
> > of some type. I'll look at the code to see if that answers the question.
>
> it is supposed to be identify location of this sensor.
>
> >
> >> +
> >> + xlnx,name:
> >> + $ref: /schemas/types.yaml#/definitions/string
> >> + description:
> >> + Name of temperature satellite exposed
> >
> > As above. label tends to get used for things like this.
>
> no issue with this.
> >> +
> >> + required:
> >> + - reg
> >> + - xlnx,name
> >> +
> >> + additionalProperties: false
> >> +
> >> +required:
> >> + - compatible
> >> + - reg
> >> + - xlnx,numchannels
> >> +
> >> +additionalProperties: false
> >
> Thanks,
> Michal