Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <55a82f21-2cfe-cc75-58b3-c1bb96835582@linaro.org>
Date:   Tue, 11 Apr 2023 08:53:52 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.1
Subject: Re: [PATCH 2/5] dt-bindings: hwmon: intel: add hardware monitor
 bindings for SoCFPGA
Content-Language: en-US
To:     dinh.nguyen@linux.intel.com, linux-hwmon@vger.kernel.org
Cc:     dinguyen@kernel.org, devicetree@vger.kernel.org,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        linux-kernel@vger.kernel.org, jdelvare@suse.com, linux@roeck-us.net
References: <20230410153314.27127-1-dinh.nguyen@linux.intel.com>
 <20230410153314.27127-2-dinh.nguyen@linux.intel.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20230410153314.27127-2-dinh.nguyen@linux.intel.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/04/2023 17:33, dinh.nguyen@linux.intel.com wrote:
> From: Dinh Nguyen <dinh.nguyen@linux.intel.com>
> 
> Document the hardware monitoring bindings for SoCFPGA 64-bit platforms.
> 
> Signed-off-by: Dinh Nguyen <dinh.nguyen@linux.intel.com>
> ---
>  .../bindings/hwmon/intel,socfpga-hwmon.yaml   | 241 ++++++++++++++++++
>  1 file changed, 241 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/intel,socfpga-hwmon.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/intel,socfpga-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/intel,socfpga-hwmon.yaml
> new file mode 100644
> index 000000000000..ec9d9eabdc37
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/intel,socfpga-hwmon.yaml
> @@ -0,0 +1,241 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/intel,socfpga-hwmon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel SoCFPGA Hardware monitor
> +
> +maintainers:
> +  - Dinh Nguyen <dinh.nguyen@linux.intel.com>
> +
> +description: |
> +  The Intel SoCFPGA hardware monitor unit provides on-chip voltage and
> +  temperature sensors. You can use these sensors to monitor external
> +  voltages and on-chip operating conditions such as internal power rails
> +  and on-chip junction temperatures.
> +
> +  The specific sensor configurations vary for each device family and
> +  each device within a family does not offer all potential sensor
> +  options. The information below attempts to illustrate the super set of
> +  possible sensor options that are potentially available within each
> +  device family, but the user should check the documentation for the
> +  specific device they are using to verify which sensor options it
> +  actually provides.
> +
> +  Stratix 10 Device Family
> +
> +    Stratix 10 Voltage Sensors
> +
> +      page 0, channel 2 = 0.8V VCC
> +      page 0, channel 3 = 1.0V VCCIO
> +      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
> +
> +  Agilex Device Family
> +
> +    Agilex Voltage Sensors
> +
> +      page 0, channel 2 = 0.8V VCC
> +      page 0, channel 3 = 1.8V VCCIO_SDM
> +      page 0, channel 4 = 1.8V VCCPT
> +      page 0, channel 5 = 1.2V VCCRCORE
> +      page 0, channel 6 = 0.9V VCCH
> +      page 0, channel 7 = 0.8V VCCL
> +
> +    Agilex Temperature Sensors
> +
> +      page 0, channel 0 = main die sdm max
> +      page 0, channel 1 = main die sdm 1
> +
> +      page 1, channel 0 = main die corner bottom left max
> +      page 1, channel 1 = main die corner bottom left 1
> +      page 1, channel 2 = main die corner bottom left 2
> +
> +      page 2, channel 0 = main die corner top left max
> +      page 2, channel 1 = main die corner top left 1
> +      page 2, channel 2 = main die corner top left 2
> +
> +      page 3, channel 0 = main die corner bottom right max
> +      page 3, channel 1 = main die corner bottom right 1
> +      page 3, channel 2 = main die corner bottom right 2
> +
> +      page 4, channel 0 = main die corner top right max
> +      page 4, channel 1 = main die corner top right 1
> +      page 4, channel 2 = main die corner top right 2
> +
> +      page 5, channel 0 = tile die bottom left max
> +      page 5, channel 1 = tile die bottom left 1
> +      page 5, channel 6..2 = tile die bottom left 6..2 R-tile only
> +      page 5, channel 5..2 = tile die bottom left 5..2 F-tile only
> +      page 5, channel 4..2 = tile die bottom left 4..2 E-tile only
> +
> +      page 7, channel 0 = tile die top left max
> +      page 7, channel 1 = tile die top left 1
> +      page 7, channel 6..2 = tile die top left 6..2 R-tile only
> +      page 7, channel 5..2 = tile die top left 5..2 F-tile only
> +      page 7, channel 4..2 = tile die top left 4..2 E-tile only
> +
> +      page 8, channel 0 = tile die bottom right max
> +      page 8, channel 1 = tile die bottom right 1
> +      page 8, channel 6..2 = tile die bottom right 6..2 R-tile only
> +      page 8, channel 5..2 = tile die bottom right 5..2 F-tile only
> +      page 8, channel 4..2 = tile die bottom right 4..2 E-tile only
> +
> +      page 10, channel 0 = tile die top right max
> +      page 10, channel 1 = tile die top right 1
> +      page 10, channel 6..2 = tile die top right 6..2 R-tile only
> +      page 10, channel 5..2 = tile die top right 5..2 F-tile only
> +      page 10, channel 4..2 = tile die top right 4..2 E-tile only
> +
> +  N5X Device Family
> +
> +    N5X Voltage Sensors
> +
> +      page 0, channel 2 = 0.8V VDD
> +      page 0, channel 3 = 0.8V VDD_SDM
> +      page 0, channel 4 = 1.8V VCCADC
> +      page 0, channel 5 = 1.8V VCCPD
> +      page 0, channel 6 = 1.8V VCCIO_SDM
> +      page 0, channel 7 = 0.8V VDD_HPS
> +
> +    N5X Temperature Sensors
> +
> +      page 0, channel 0 = main die
> +
> +properties:
> +  compatible:
> +    enum:
> +      - intel,socfpga-hwmon

You should have SoC specific compatibles, followed by one specific or by
generic.

> +
> +  reg:
> +    maxItems: 1
> +    description:
> +      The sensor mapping address is denoted by the lower 16-bits being
> +      the channel mask location that defines the channel number.
> +      The upper 16-bits denotes the page number.
> +      The bit mask of 0x1 represents channel 1. The supported
> +      page and channel is dependent on the SoCFPGA variant.
> +      Page number greater than 0 is only supported on the
> +      temperature sensors.
> +
> +  temperature:
> +    description:
> +      Specifies the possible mappings of temperature sensors
> +      diodes on the SoCFPGA main die and tile die.

What's this? No ref/type, not constraints?

> +
> +  voltage:
> +    description:
> +      Specifies the possible mappings of the voltage sensors on the
> +      SoCFPGA analog to digital converter of the Secure Device Manager
> +      (SDM).

Same here.

> +
> +  input:
> +    description:
> +      Specifies each sensor.

And here.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    temp_volt {

No underscores in node names.

> +      compatible = "intel,socfpga-hwmon";
> +      voltage {
> +        #address-cells = <1>;
> +        #size-cells = <0>;

So this is object?


Best regards,
Krzysztof