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 [off-list ref] wrote:

quoted

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 <redacted>
Signed-off-by: Anand Ashok Dumbre <redacted>
Co-developed-by: Anish Kadamathikuttiyil Karthikeyan Pillai <redacted>
Signed-off-by: Anish Kadamathikuttiyil Karthikeyan Pillai <redacted>
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.yaml

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

quoted

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

quoted

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

quoted

+
+      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?

quoted

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

quoted

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

quoted

+
+    required:
+      - reg
+      - xlnx,name
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - xlnx,numchannels
+
+additionalProperties: false

Thanks,
Michal