Message-ID:
 <IA1PR12MB7736D056E505103AC246DC2E9F03A@IA1PR12MB7736.namprd12.prod.outlook.com>
Date: Fri, 5 Sep 2025 14:21:46 +0000
From: "Erim, Salih" <Salih.Erim@....com>
To: "Simek, Michal" <michal.simek@....com>, Jonathan Cameron
	<jonathan.cameron@...wei.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"monstr@...str.eu" <monstr@...str.eu>, "michal.simek@...inx.com"
	<michal.simek@...inx.com>, "git@...inx.com" <git@...inx.com>, Anand Ashok
 Dumbre <anand.ashok.dumbre@...inx.com>, "Kadamathikuttiyil Karthikeyan
 Pillai, Anish" <anish.kadamathikuttiyil-karthikeyan-pillai@....com>, Andy
 Shevchenko <andy@...nel.org>, Conor Dooley <conor+dt@...nel.org>, David
 Lechner <dlechner@...libre.com>, Jonathan Cameron <jic23@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Nuno Sá
	<nuno.sa@...log.com>, Rob Herring <robh@...nel.org>, "open list:OPEN FIRMWARE
 AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>, "open
 list:IIO SUBSYSTEM AND DRIVERS" <linux-iio@...r.kernel.org>
Subject: RE: [PATCH 1/6] dt-bindings: iio: xilinx: Add Documentation for
 Sysmon

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Michal and Jonathan,



> -----Original Message-----
> From: Simek, Michal <michal.simek@....com>
> Sent: Friday, September 5, 2025 1:30 PM
> To: Jonathan Cameron <jonathan.cameron@...wei.com>
> Cc: linux-kernel@...r.kernel.org; monstr@...str.eu; michal.simek@...inx.com;
> git@...inx.com; Erim, Salih <Salih.Erim@....com>; Anand Ashok Dumbre
> <anand.ashok.dumbre@...inx.com>; Kadamathikuttiyil Karthikeyan Pillai, Anish
> <anish.kadamathikuttiyil-karthikeyan-pillai@....com>; Andy Shevchenko
> <andy@...nel.org>; Conor Dooley <conor+dt@...nel.org>; David Lechner
> <dlechner@...libre.com>; Jonathan Cameron <jic23@...nel.org>; Krzysztof
> Kozlowski <krzk+dt@...nel.org>; Nuno Sá <nuno.sa@...log.com>; Rob Herring
> <robh@...nel.org>; open list:OPEN FIRMWARE AND FLATTENED DEVICE
> TREE BINDINGS <devicetree@...r.kernel.org>; open list:IIO SUBSYSTEM AND
> DRIVERS <linux-iio@...r.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@....com> wrote:
> >
> >> From: Salih Erim <salih.erim@....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@...inx.com>
> >> Signed-off-by: Anand Ashok Dumbre <anand.ashok.dumbre@...inx.com>
> >> Co-developed-by: Anish Kadamathikuttiyil Karthikeyan Pillai
> >> <anish.kadamathikuttiyil-karthikeyan-pillai@....com>
> >> Signed-off-by: Anish Kadamathikuttiyil Karthikeyan Pillai
> >> <anish.kadamathikuttiyil-karthikeyan-pillai@....com>
> >> Signed-off-by: Salih Erim <salih.erim@....com>
> >> Signed-off-by: Michal Simek <michal.simek@....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@....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

Powered by blists - more mailing lists