devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] xilinx: Add support for Xilinx Sysmon IP
@ 2025-09-05  8:41 Michal Simek
  2025-09-05  8:41 ` [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon Michal Simek
  2025-09-05  8:41 ` [PATCH 4/6] dt-bindings: thermal: versal: Add description for Versal Thermal Michal Simek
  0 siblings, 2 replies; 11+ messages in thread
From: Michal Simek @ 2025-09-05  8:41 UTC (permalink / raw)
  To: linux-kernel, monstr, michal.simek, git
  Cc: Andy Shevchenko, Conor Dooley, Daniel Lezcano, David Lechner,
	Jonathan Cameron, Krzysztof Kozlowski, Lukasz Luba, Nuno Sá,
	Rafael J. Wysocki, Rob Herring, Salih Erim, Zhang Rui,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:IIO SUBSYSTEM AND DRIVERS, open list:THERMAL

Hi,

the series is adding support for Xilinx Sysmon IP available on Versal SOCs.
The series also contains thermal driver which is using Sysmon for providing
temperature information.
As the part of it is also monitoring AI engine.

Thanks,
Michal


Anish Kadamathikuttiyil Karthikeyan Pillai (2):
  iio: adc: versal-sysmon: Support AI Engine thermal monitoring
  thermal: versal-thermal: Support thermal management in AI Engine

Salih Erim (4):
  dt-bindings: iio: xilinx: Add Documentation for Sysmon
  iio: versal-sysmon: add driver for Versal Sysmon
  dt-bindings: thermal: versal: Add description for Versal Thermal
  thermal: versal-thermal: Add Versal thermal driver

 .../bindings/iio/adc/xlnx,versal-sysmon.yaml  |  235 +++
 .../bindings/thermal/xlnx,versal-thermal.yaml |   45 +
 MAINTAINERS                                   |   13 +
 drivers/iio/adc/Kconfig                       |   15 +
 drivers/iio/adc/Makefile                      |    2 +
 drivers/iio/adc/versal-sysmon-core.c          | 1379 +++++++++++++++++
 drivers/iio/adc/versal-sysmon.c               |  297 ++++
 drivers/iio/adc/versal-sysmon.h               |  278 ++++
 drivers/thermal/Kconfig                       |   12 +
 drivers/thermal/Makefile                      |    1 +
 drivers/thermal/versal_thermal.c              |  221 +++
 include/linux/iio/adc/versal-sysmon-events.h  |   56 +
 12 files changed, 2554 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml
 create mode 100644 Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml
 create mode 100644 drivers/iio/adc/versal-sysmon-core.c
 create mode 100644 drivers/iio/adc/versal-sysmon.c
 create mode 100644 drivers/iio/adc/versal-sysmon.h
 create mode 100644 drivers/thermal/versal_thermal.c
 create mode 100644 include/linux/iio/adc/versal-sysmon-events.h

-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon
  2025-09-05  8:41 [PATCH 0/6] xilinx: Add support for Xilinx Sysmon IP Michal Simek
@ 2025-09-05  8:41 ` Michal Simek
  2025-09-05 11:30   ` Jonathan Cameron
  2025-09-05  8:41 ` [PATCH 4/6] dt-bindings: thermal: versal: Add description for Versal Thermal Michal Simek
  1 sibling, 1 reply; 11+ messages in thread
From: Michal Simek @ 2025-09-05  8:41 UTC (permalink / raw)
  To: linux-kernel, monstr, michal.simek, git
  Cc: Salih Erim, Anand Ashok Dumbre,
	Anish Kadamathikuttiyil Karthikeyan Pillai, Andy Shevchenko,
	Conor Dooley, David Lechner, Jonathan Cameron,
	Krzysztof Kozlowski, Nuno Sá, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:IIO SUBSYSTEM AND DRIVERS

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.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.
+
+  xlnx,numchannels:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 160
+    description:
+      Number of supply channels enabled in the design.
+
+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
+
+      xlnx,bipolar:
+        $ref: /schemas/types.yaml#/definitions/flag
+        description:
+          If the supply has a bipolar type and the output will be signed.
+
+    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
+
+      xlnx,name:
+        $ref: /schemas/types.yaml#/definitions/string
+        description:
+          Name of temperature satellite exposed
+
+    required:
+      - reg
+      - xlnx,name
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - xlnx,numchannels
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    sysmon@f1270000 {
+        compatible = "xlnx,versal-sysmon";
+        reg = <0xf1270000 0x4000>;
+        interrupts = <0 0x90 IRQ_TYPE_LEVEL_HIGH>;
+        xlnx,numchannels = <8>;
+        xlnx,numaiechannels = <8>;
+        xlnx,nodeid = <0x18224055>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+        supply@0 {
+            reg = <0>;
+            xlnx,name = "vccint";
+        };
+
+        supply@31 {
+            reg = <31>;
+            xlnx,name = "vccsoc";
+        };
+
+        supply@32 {
+            reg = <32>;
+            xlnx,bipolar;
+            xlnx,name = "vccram";
+        };
+
+        supply@63 {
+            reg = <63>;
+            xlnx,bipolar;
+            xlnx,name = "vccaux";
+        };
+
+        supply@64 {
+            reg = <64>;
+            xlnx,name = "vccbram";
+        };
+
+        supply@95 {
+            reg = <95>;
+            xlnx,name = "gt_avaux";
+        };
+
+        supply@96 {
+            reg = <96>;
+            xlnx,name = "gt_vccaux";
+        };
+
+        supply@159 {
+            reg = <159>;
+            xlnx,name = "vccint_ir";
+        };
+
+        temp@7 {
+            reg = <7>;
+            xlnx,aie-temp;
+            xlnx,name = "aie-temp-ch0";
+        };
+
+        temp@8 {
+            reg = <8>;
+            xlnx,aie-temp;
+            xlnx,name = "aie-temp-ch1";
+        };
+
+        temp@14 {
+            reg = <14>;
+            xlnx,aie-temp;
+            xlnx,name = "aie-temp-ch2";
+        };
+
+        temp@15 {
+            reg = <15>;
+            xlnx,aie-temp;
+            xlnx,name = "aie-temp-ch3";
+        };
+
+        temp@16 {
+            reg = <16>;
+            xlnx,aie-temp;
+            xlnx,name = "aie-temp-ch4";
+        };
+
+        temp@30 {
+            reg = <30>;
+            xlnx,aie-temp;
+            xlnx,name = "aie-temp-ch5";
+        };
+
+        temp@33 {
+            reg = <33>;
+            xlnx,aie-temp;
+            xlnx,name = "aie-temp-ch6";
+        };
+
+        temp@34 {
+            reg = <34>;
+            xlnx,aie-temp;
+            xlnx,name = "aie-temp-ch7";
+        };
+    };
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/6] dt-bindings: thermal: versal: Add description for Versal Thermal
  2025-09-05  8:41 [PATCH 0/6] xilinx: Add support for Xilinx Sysmon IP Michal Simek
  2025-09-05  8:41 ` [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon Michal Simek
@ 2025-09-05  8:41 ` Michal Simek
  2025-09-05 18:30   ` Conor Dooley
  1 sibling, 1 reply; 11+ messages in thread
From: Michal Simek @ 2025-09-05  8:41 UTC (permalink / raw)
  To: linux-kernel, monstr, michal.simek, git
  Cc: Salih Erim, Anish Kadamathikuttiyil Karthikeyan Pillai,
	Conor Dooley, Daniel Lezcano, Krzysztof Kozlowski, Lukasz Luba,
	Rafael J. Wysocki, Rob Herring, Zhang Rui,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:THERMAL

From: Salih Erim <salih.erim@amd.com>

Add description of Versal Thermal which describes IO channels to
be binded to thermal. Constant channel of "sysmon-temp-channel" used as
mapped iio channel.

One temperature zone is used for the AI Engine temperature monitoring.

Signed-off-by: Salih Erim <salih.erim@amd.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: Michal Simek <michal.simek@amd.com>
---

 .../bindings/thermal/xlnx,versal-thermal.yaml | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml

diff --git a/Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml b/Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml
new file mode 100644
index 000000000000..c374d7ae2d2a
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/xlnx,versal-thermal.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx Versal Thermal
+
+maintainers:
+  - Salih Erim <salih.erim@amd.com>
+
+description:
+  Versal Thermal uses Versal Sysmon hardware for temperature reading.
+  It works as sensor interface to be defined in thermal zones.
+
+properties:
+  compatible:
+    const: xlnx,versal-thermal
+
+  '#thermal-sensor-cells':
+    const: 1
+
+  io-channels:
+    maxItems: 1
+
+  io-channel-names:
+    const: sysmon-temp-channel
+
+required:
+  - compatible
+  - '#thermal-sensor-cells'
+  - io-channels
+  - io-channel-names
+
+additionalProperties: false
+
+examples:
+  - |
+    versal-thermal-sensor {
+        compatible = "xlnx,versal-thermal";
+        #thermal-sensor-cells = <1>;
+        io-channels = <&sysmon0>;
+        io-channel-names = "sysmon-temp-channel";
+    };
+...
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon
  2025-09-05  8:41 ` [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon Michal Simek
@ 2025-09-05 11:30   ` Jonathan Cameron
  2025-09-05 12:29     ` Michal Simek
  0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2025-09-05 11:30 UTC (permalink / raw)
  To: Michal Simek
  Cc: linux-kernel, monstr, michal.simek, git, Salih Erim,
	Anand Ashok Dumbre, Anish Kadamathikuttiyil Karthikeyan Pillai,
	Andy Shevchenko, Conor Dooley, David Lechner, Jonathan Cameron,
	Krzysztof Kozlowski, Nuno Sá, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:IIO SUBSYSTEM AND DRIVERS

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


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

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

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

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

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

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon
  2025-09-05 11:30   ` Jonathan Cameron
@ 2025-09-05 12:29     ` Michal Simek
  2025-09-05 14:21       ` Erim, Salih
  0 siblings, 1 reply; 11+ messages in thread
From: Michal Simek @ 2025-09-05 12:29 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-kernel, monstr, michal.simek, git, Salih Erim,
	Anand Ashok Dumbre, Anish Kadamathikuttiyil Karthikeyan Pillai,
	Andy Shevchenko, Conor Dooley, David Lechner, Jonathan Cameron,
	Krzysztof Kozlowski, Nuno Sá, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:IIO SUBSYSTEM AND DRIVERS



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

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

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon
  2025-09-05 12:29     ` Michal Simek
@ 2025-09-05 14:21       ` Erim, Salih
  2025-09-05 20:44         ` David Lechner
  0 siblings, 1 reply; 11+ messages in thread
From: Erim, Salih @ 2025-09-05 14:21 UTC (permalink / raw)
  To: Simek, Michal, Jonathan Cameron
  Cc: linux-kernel@vger.kernel.org, monstr@monstr.eu,
	michal.simek@xilinx.com, git@xilinx.com, Anand Ashok Dumbre,
	Kadamathikuttiyil Karthikeyan Pillai, Anish, Andy Shevchenko,
	Conor Dooley, David Lechner, Jonathan Cameron,
	Krzysztof Kozlowski, Nuno Sá, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:IIO SUBSYSTEM AND DRIVERS

[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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/6] dt-bindings: thermal: versal: Add description for Versal Thermal
  2025-09-05  8:41 ` [PATCH 4/6] dt-bindings: thermal: versal: Add description for Versal Thermal Michal Simek
@ 2025-09-05 18:30   ` Conor Dooley
  2025-09-08  6:39     ` Michal Simek
  0 siblings, 1 reply; 11+ messages in thread
From: Conor Dooley @ 2025-09-05 18:30 UTC (permalink / raw)
  To: Michal Simek
  Cc: linux-kernel, monstr, michal.simek, git, Salih Erim,
	Anish Kadamathikuttiyil Karthikeyan Pillai, Conor Dooley,
	Daniel Lezcano, Krzysztof Kozlowski, Lukasz Luba,
	Rafael J. Wysocki, Rob Herring, Zhang Rui,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:THERMAL

[-- Attachment #1: Type: text/plain, Size: 2674 bytes --]

On Fri, Sep 05, 2025 at 10:41:47AM +0200, Michal Simek wrote:
> From: Salih Erim <salih.erim@amd.com>
> 
> Add description of Versal Thermal which describes IO channels to
> be binded to thermal. Constant channel of "sysmon-temp-channel" used as
> mapped iio channel.
> 
> One temperature zone is used for the AI Engine temperature monitoring.
> 
> Signed-off-by: Salih Erim <salih.erim@amd.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: Michal Simek <michal.simek@amd.com>
> ---
> 
>  .../bindings/thermal/xlnx,versal-thermal.yaml | 45 +++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml
> 
> diff --git a/Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml b/Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml
> new file mode 100644
> index 000000000000..c374d7ae2d2a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/xlnx,versal-thermal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx Versal Thermal
> +
> +maintainers:
> +  - Salih Erim <salih.erim@amd.com>
> +
> +description:
> +  Versal Thermal uses Versal Sysmon hardware for temperature reading.
> +  It works as sensor interface to be defined in thermal zones.
> +
> +properties:
> +  compatible:
> +    const: xlnx,versal-thermal

BTW Michal, what's the story with using amd v xlnx for bindings?
Planning to use amd for new devices and xlnx for stuff sold before the
purchase or something like that?

> +
> +  '#thermal-sensor-cells':
> +    const: 1
> +
> +  io-channels:
> +    maxItems: 1
> +
> +  io-channel-names:
> +    const: sysmon-temp-channel
> +
> +required:
> +  - compatible
> +  - '#thermal-sensor-cells'
> +  - io-channels
> +  - io-channel-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    versal-thermal-sensor {

Drop the versal from the node name here please.
With that,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

> +        compatible = "xlnx,versal-thermal";
> +        #thermal-sensor-cells = <1>;
> +        io-channels = <&sysmon0>;
> +        io-channel-names = "sysmon-temp-channel";
> +    };
> +...
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon
  2025-09-05 14:21       ` Erim, Salih
@ 2025-09-05 20:44         ` David Lechner
  2025-09-07 10:51           ` Jonathan Cameron
  0 siblings, 1 reply; 11+ messages in thread
From: David Lechner @ 2025-09-05 20:44 UTC (permalink / raw)
  To: Erim, Salih, Simek, Michal, Jonathan Cameron
  Cc: linux-kernel@vger.kernel.org, monstr@monstr.eu,
	michal.simek@xilinx.com, git@xilinx.com, Anand Ashok Dumbre,
	Kadamathikuttiyil Karthikeyan Pillai, Anish, Andy Shevchenko,
	Conor Dooley, Jonathan Cameron, Krzysztof Kozlowski, Nuno Sá,
	Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:IIO SUBSYSTEM AND DRIVERS

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";
			};

			...
		};
	};

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon
  2025-09-05 20:44         ` David Lechner
@ 2025-09-07 10:51           ` Jonathan Cameron
  2025-09-08 11:13             ` Erim, Salih
  0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2025-09-07 10:51 UTC (permalink / raw)
  To: David Lechner
  Cc: Erim, Salih, Simek, Michal, Jonathan Cameron,
	linux-kernel@vger.kernel.org, monstr@monstr.eu,
	michal.simek@xilinx.com, git@xilinx.com, Anand Ashok Dumbre,
	Kadamathikuttiyil Karthikeyan Pillai, Anish, Andy Shevchenko,
	Conor Dooley, Krzysztof Kozlowski, Nuno Sá, Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:IIO SUBSYSTEM AND DRIVERS

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




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/6] dt-bindings: thermal: versal: Add description for Versal Thermal
  2025-09-05 18:30   ` Conor Dooley
@ 2025-09-08  6:39     ` Michal Simek
  0 siblings, 0 replies; 11+ messages in thread
From: Michal Simek @ 2025-09-08  6:39 UTC (permalink / raw)
  To: Conor Dooley
  Cc: linux-kernel, monstr, michal.simek, git, Salih Erim,
	Anish Kadamathikuttiyil Karthikeyan Pillai, Conor Dooley,
	Daniel Lezcano, Krzysztof Kozlowski, Lukasz Luba,
	Rafael J. Wysocki, Rob Herring, Zhang Rui,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:THERMAL



On 9/5/25 20:30, Conor Dooley wrote:
> On Fri, Sep 05, 2025 at 10:41:47AM +0200, Michal Simek wrote:
>> From: Salih Erim <salih.erim@amd.com>
>>
>> Add description of Versal Thermal which describes IO channels to
>> be binded to thermal. Constant channel of "sysmon-temp-channel" used as
>> mapped iio channel.
>>
>> One temperature zone is used for the AI Engine temperature monitoring.
>>
>> Signed-off-by: Salih Erim <salih.erim@amd.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: Michal Simek <michal.simek@amd.com>
>> ---
>>
>>   .../bindings/thermal/xlnx,versal-thermal.yaml | 45 +++++++++++++++++++
>>   1 file changed, 45 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml b/Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml
>> new file mode 100644
>> index 000000000000..c374d7ae2d2a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/thermal/xlnx,versal-thermal.yaml
>> @@ -0,0 +1,45 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/thermal/xlnx,versal-thermal.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Xilinx Versal Thermal
>> +
>> +maintainers:
>> +  - Salih Erim <salih.erim@amd.com>
>> +
>> +description:
>> +  Versal Thermal uses Versal Sysmon hardware for temperature reading.
>> +  It works as sensor interface to be defined in thermal zones.
>> +
>> +properties:
>> +  compatible:
>> +    const: xlnx,versal-thermal
> 
> BTW Michal, what's the story with using amd v xlnx for bindings?
> Planning to use amd for new devices and xlnx for stuff sold before the
> purchase or something like that?

yes exactly that is the plan. Newly developed drivers or SOC will start to use 
amd prefixes instead of xlnx.
This is the IP which is around for a while that's why xlnx is still used.

> 
>> +
>> +  '#thermal-sensor-cells':
>> +    const: 1
>> +
>> +  io-channels:
>> +    maxItems: 1
>> +
>> +  io-channel-names:
>> +    const: sysmon-temp-channel
>> +
>> +required:
>> +  - compatible
>> +  - '#thermal-sensor-cells'
>> +  - io-channels
>> +  - io-channel-names
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    versal-thermal-sensor {
> 
> Drop the versal from the node name here please.
> With that,

will do.

> Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Michal


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon
  2025-09-07 10:51           ` Jonathan Cameron
@ 2025-09-08 11:13             ` Erim, Salih
  0 siblings, 0 replies; 11+ messages in thread
From: Erim, Salih @ 2025-09-08 11:13 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner
  Cc: Simek, Michal, Jonathan Cameron, linux-kernel@vger.kernel.org,
	monstr@monstr.eu, michal.simek@xilinx.com, git@xilinx.com,
	Anand Ashok Dumbre, Kadamathikuttiyil Karthikeyan Pillai, Anish,
	Andy Shevchenko, Conor Dooley, Krzysztof Kozlowski, Nuno Sá,
	Rob Herring,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:IIO SUBSYSTEM AND DRIVERS

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-09-08 11:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-05  8:41 [PATCH 0/6] xilinx: Add support for Xilinx Sysmon IP Michal Simek
2025-09-05  8:41 ` [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for Sysmon Michal Simek
2025-09-05 11:30   ` Jonathan Cameron
2025-09-05 12:29     ` Michal Simek
2025-09-05 14:21       ` Erim, Salih
2025-09-05 20:44         ` David Lechner
2025-09-07 10:51           ` Jonathan Cameron
2025-09-08 11:13             ` Erim, Salih
2025-09-05  8:41 ` [PATCH 4/6] dt-bindings: thermal: versal: Add description for Versal Thermal Michal Simek
2025-09-05 18:30   ` Conor Dooley
2025-09-08  6:39     ` Michal Simek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).