Re: [PATCH v2 1/5] dt-bindings: iio: adc: add xlnx,versal-sysmon binding

[Salih Erim]

Hi Krzysztof,

Thanks for the review, Replies inline.

On 5/3/2026 3:20 PM, Krzysztof Kozlowski wrote:

> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - xlnx,versal-sysmon
> > +      - xlnx,versal-sysmon-i2c
>
> This is explicitly mentioned in writing bindings. One device, one
> compatible (not two, not three, not four compatibles).

Accepted. Will use single "xlnx,versal-sysmon" compatible. The I2C 
driver will match via the I2C bus, so the bus type provides the 
differentiation.

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  '#io-channel-cells':
> > +    const: 1
> > +
> > +  supply-channels:
> > +    type: object
> > +    description:
> > +      Container for supply voltage measurement channels.
>
> voltage, not supply. Supply is the source of energy coming to this
> device. But are you measuring that source?

Accepted. Will rename to "voltage-channels".

> > +
> > +    properties:
> > +      '#address-cells':
> > +        const: 1
> > +
> > +      '#size-cells':
> > +        const: 0
> > +
> > +    patternProperties:
> > +      "^channel@([0-9a-f]|[1-9][0-9a-f])$":
>
> Keep consistent quotes, either ' or "

Will use single quotes throughout.

> > +        $ref: adc.yaml
> > +
> > +        description:
> > +          Measures a supply rail voltage. The register index and rail
> > +          name are assigned by the hardware design tool (Vivado).
> > +
> > +        properties:
> > +          reg:
> > +            minimum: 0
> > +            maximum: 159
> > +            description:
> > +              Supply measurement register index assigned by the hardware
> > +              design tool.
> > +
> > +          label:
> > +            description:
> > +              Name of the supply rail being monitored.
>
> Drop property, it's already in adc.yaml.

Accepted, will drop from both voltage and temperature channel blocks.

> > +
> > +          bipolar: true
>
> Drop

Accepted, already defined in adc.yaml.

> > +
> > +        required:
> > +          - reg
> > +          - label
> > +
> > +        unevaluatedProperties: false
> > +
> > +    required:
> > +      - '#address-cells'
> > +      - '#size-cells'
> > +
> > +    additionalProperties: false
> > +
> > +  temperature-channels:
> > +    type: object
> > +    description:
> > +      Container for temperature satellite measurement channels.
> > +
> > +    properties:
> > +      '#address-cells':
> > +        const: 1
> > +
> > +      '#size-cells':
> > +        const: 0
> > +
> > +    patternProperties:
> > +      "^channel@([1-9a-f]|[1-3][0-9a-f]|40)$":
> > +        $ref: adc.yaml
> > +
> > +        description:
> > +          Reads a temperature satellite sensor. Each satellite monitors
> > +          a specific region of the SoC die.
> > +
> > +        properties:
> > +          reg:
> > +            minimum: 1
> > +            maximum: 64
> > +            description:
> > +              Temperature satellite number (1-based hardware index).
> > +
> > +          label:
> > +            description:
> > +              Name identifying this temperature satellite.
>
> Drop property
>
> > +
> > +          xlnx,aie-temp:
> > +            type: boolean
> > +            description:
> > +              Indicates this satellite monitors an AI Engine tile.
>
> What for? What is on the other side of the channel is not really
> relevant to this device, but rather to that other side (consumer).

Agreed. The driver does not use this property. Will remove.

> > +
> > +        required:
> > +          - reg
> > +          - label
> > +
> > +        unevaluatedProperties: false
> > +
> > +    required:
> > +      - '#address-cells'
> > +      - '#size-cells'
> > +
> > +    additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
>
> Best regards,
> Krzysztof

I will address all items in v3.

Salih.