Re: [PATCH 1/5] dt-bindings: hwmon: add Altera Stratix 10 hardware monitor binding

[Krzysztof Kozlowski <krzk@kernel.org>]

On Fri, Jun 19, 2026 at 02:38:52AM -0700, tze.yee.ng@altera.com wrote:
> From: Tze Yee Ng <tze.yee.ng@altera.com>
> 
> Document the device tree binding for the Altera Stratix 10 SoC FPGA
> hardware monitor, including temperature and voltage sensor nodes.
> 
> Signed-off-by: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
>  .../bindings/hwmon/altr,stratix10-hwmon.yaml  | 164 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 171 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml
> new file mode 100644
> index 000000000000..5bd98660ee7b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml
> @@ -0,0 +1,164 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/altr,stratix10-hwmon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Altera Hardware Monitor for Stratix 10 SoC FPGA
> +
> +maintainers:
> +  - Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> +  - Tze Yee Ng <tze.yee.ng@altera.com>
> +
> +description: |
> +  The Altera Stratix 10 SoC FPGA hardware monitor unit provides on-chip
> +  voltage and temperature sensors. These sensors can be used to monitor
> +  external voltages and on-chip operating conditions such as internal
> +  power rails and on-chip junction temperatures.
> +
> +  The specific sensor configuration varies by device. Check the device
> +  documentation to verify which sensors are available.
> +
> +  Stratix 10 voltage sensors:
> +
> +    page 0, channel 2 = 0.8V VCC
> +    page 0, channel 3 = 1.8V VCCIO_SDM
> +    page 0, channel 6 = 0.9V VCCERAM
> +
> +  Stratix 10 temperature sensors:
> +
> +    page 0, channel 0 = main die
> +    page 0, channel 1 = tile bottom left
> +    page 0, channel 2 = tile middle left
> +    page 0, channel 3 = tile top left
> +    page 0, channel 4 = tile bottom right
> +    page 0, channel 5 = tile middle right
> +    page 0, channel 6 = tile top right
> +    page 0, channel 7 = hbm2 bottom
> +    page 0, channel 8 = hbm2 top
> +
> +properties:
> +  compatible:
> +    const: altr,stratix10-hwmon
> +

Where are top-level properties? clocks? interrupts? power-domain,
resets? address space? Anything? Did you read writing bindings doc?

> +  temperature:
> +    description:
> +      The temperature node specifies mappings of temperature sensor diodes on
> +      the Stratix 10 SoC FPGA main die and tile die.
> +    type: object

Blank line

> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0

Blank line

> +    patternProperties:
> +      "^input(@[0-9a-f]+)?$":

Unit address should not be optional.

Also, use consistent style of quotes.

> +        description:
> +          The input node specifies each individual temperature sensor.
> +        type: object
> +        properties:
> +          reg:
> +            description:
> +              Sensor channel index in the lower 16-bits (0-15). For temperature
> +              sensors, the page number is encoded in the upper 16-bits.
> +              The driver encodes the SMC request argument as a channel
> +              bitmask (1 << channel) in bits 0..15, with the page number
> +              placed in bits 16..31. Channel values >= 16 are rejected to
> +              avoid overlap with the page field. For example, reg = <2>
> +              selects channel 2 and the driver passes 0x4 to the service layer.
> +          label:
> +            description:
> +              A descriptive name for this channel (e.g. "Main Die" or
> +              "Tile Bottom Left").
> +        required:
> +          - reg
> +        additionalProperties: false

All this is difficult to read. Please open other bindings to understand
the style used in Linux kernel.

> +    required:
> +      - '#address-cells'
> +      - '#size-cells'
> +    additionalProperties: false
> +
> +  voltage:
> +    description:
> +      The voltage node specifies mappings of voltage sensors on the Stratix 10
> +      SoC FPGA analog to digital converter of the Secure Device Manager (SDM).
> +    type: object
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +    patternProperties:
> +      "^input(@[0-9a-f]+)?$":

Isn't this usually called channel?

> +        description:
> +          The input node specifies each individual voltage sensor.
> +        type: object
> +        properties:
> +          reg:
> +            description:
> +              Sensor channel index in the lower 16-bits (0-15). The driver
> +              encodes the SMC request argument as a channel bitmask
> +              (1 << channel). For example, reg = <2> selects channel 2 and
> +              the driver passes 0x4 to the service layer.
> +          label:
> +            description:
> +              A descriptive name for this channel (e.g. "0.8V VCC" or
> +              "1.8V VCCIO_SDM").
> +        required:
> +          - reg
> +        additionalProperties: false
> +    required:
> +      - '#address-cells'
> +      - '#size-cells'
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    hwmon {
> +      compatible = "altr,stratix10-hwmon";
> +
> +      voltage {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        input@2 {
> +          label = "0.8V VCC";
> +          reg = <2>;
> +        };
> +
> +        input@3 {
> +          label = "1.8V VCCIO_SDM";
> +          reg = <3>;
> +        };
> +
> +        input@6 {
> +          label = "0.9V VCCERAM";
> +          reg = <6>;
> +        };
> +      };
> +
> +      temperature {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        input@0 {
> +          label = "Main Die";
> +          reg = <0>;
> +        };
> +
> +        input@1 {
> +          label = "Tile Bottom Left";
> +          reg = <1>;
> +        };
> +
> +        input@2 {
> +          label = "Tile Middle Left";
> +          reg = <2>;
> +        };
> +      };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6aa3fe2ee1bb..678f6c429627 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -937,6 +937,13 @@ ALPS PS/2 TOUCHPAD DRIVER
>  R:	Pali Rohár <pali@kernel.org>
>  F:	drivers/input/mouse/alps.*
>  
> +ALTERA STRATIX 10 SoC FPGA HWMON DRIVER
> +M:	Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> +M:	Tze Yee Ng <tze.yee.ng@altera.com>
> +L:	linux-hwmon@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/hwmon/altr,stratix10-hwmon.yaml
> +
>  ALTERA MAILBOX DRIVER

Does not look sorted. Please read this file before changing it.

Best regards,
Krzysztof