[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bccecef7-76d2-6d53-b3e3-20cea9957ae6@linaro.org>
Date: Tue, 9 May 2023 09:01:52 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: dinh.nguyen@...ux.intel.com, linux-hwmon@...r.kernel.org
Cc: dinguyen@...nel.org, devicetree@...r.kernel.org,
robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
linux-kernel@...r.kernel.org, jdelvare@...e.com, linux@...ck-us.net
Subject: Re: [PATCHv2 5/6] dt-bindings: hwmon: intel: add hardware monitor
bindings for SoCFPGA
On 08/05/2023 23:28, dinh.nguyen@...ux.intel.com wrote:
> From: Dinh Nguyen <dinh.nguyen@...ux.intel.com>
>
> Document the hardware monitoring bindings for SoCFPGA 64-bit platforms.
>
> Signed-off-by: Dinh Nguyen <dinh.nguyen@...ux.intel.com>
> ---
> v2: Add intel,socfpga-agilex-hwmon, intel,socfpga-n5x-hwmon and
> intel,socfpga-stratix10-hwmon
> Add patternProperties
> ---
> .../bindings/hwmon/intel,socfpga-hwmon.yaml | 258 ++++++++++++++++++
Bindings are before they are used in driver.
> 1 file changed, 258 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..e634311a0e81
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/intel,socfpga-hwmon.yaml
> @@ -0,0 +1,258 @@
> +# 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@...ux.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-agilex-hwmon
> + - intel,socfpga-n5x-hwmon
> + - intel,socfpga-stratix10-hwmon
> + - intel,socfpga-hwmon
The last one is too gerneric. Anyway your driver suggests all of them
are compatible.
> +
> +patternProperties:
> + "^(voltage)|input@([2-7])$":
That's not what your DTS is saying. Are you sure you tested it?
> + type: object
> + description: Specifies the possible mappings of the voltage sensors on the
> + SoCFPGA analog to digital converter of the Secure Device Manager(SDM).
additionalProperties: false
> +
> + properties:
> + reg:
> + description:
> + 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.
> + items:
> + minimum: 0
> + maximum: 1
voltage does not have reg. also, the minimum/maximum does not fit your
2-7 at all. Entire part should be probably just maxItems, but anyway I
don't really understand what you want to express here. Especially that
it does not match your DTS.
> +
> + label:
> + description: A descriptive name for this channel, i.e "0.8V VCC" or "1.8V VCCIO_SDM".
> +
> + "^(temperature)|input@([10001-40002])$":
> + type: object
> + description: Specifies the possible mappings of the temperature sensors on
> + the SoCFPGA analog to digital converter of the Secure Device Manager(SDM).
additionalProperties: false
> +
> + properties:
> + reg:
> + 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.
> + items:
> + minimum: 0
> + maximum: 1
Same problems, does not make any sense. 40002 is higher than 1.
> +
> + label:
> + description: A descriptive name for this channel, i.e "Main Die SDM" or
> + "Main Die corner bottom left 1".
> +
> +required:
> + - compatible
> +
> +additionalProperties: false
Best regards,
Krzysztof
Powered by blists - more mailing lists