| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f2ba65ef-b1da-4f06-ac86-fef173dc2b95@altera.com>
Date: Thu, 18 Dec 2025 09:57:11 +0000
From: "Mohamad Jamian, Muhammad Amirul Asyraf"
<muhammad.amirul.asyraf.mohamad.jamian@...era.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, "Mohamad Jamian, Muhammad Amirul
Asyraf" <muhammad.amirul.asyraf.mohamad.jamian@...era.com>
CC: Guenter Roeck <linux@...ck-us.net>, "linux-hwmon@...r.kernel.org"
<linux-hwmon@...r.kernel.org>, Dinh Nguyen <dinguyen@...nel.org>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "Ang, Tien Sung" <tien.sung.ang@...era.com>,
"Romli, Khairul Anuar" <khairul.anuar.romli@...era.com>
Subject: Re: [PATCH v1 1/5] dt-bindings: hwmon: add altr,socfpga-hwmon.yaml
binding
On 18/12/2025 4:25 pm, Krzysztof Kozlowski wrote:
> On Mon, Dec 15, 2025 at 10:49:22PM -0800, muhammadamirulasyraf.mohamadjamian@...era.com wrote:
>> From: Muhammad Amirul Asyraf Mohamad Jamian <muhammad.amirul.asyraf.mohamad.jamian@...era.com>
>>
>> The Altera SoCFPGA platform includes a hardware monitoring (hwmon) device
>> that reports voltage and temperature sensors critical for system stability
>> and safety. Without a proper device tree binding, the kernel and
>> userspace tools cannot correctly interpret or configure these sensors.
>>
>> This binding provides a formal description of the device’s properties,
>> including the 'compatible' string for driver matching, voltage and
>> temperature parameters with scaling and threshold definitions. This enables
>> the kernel to correctly identify the device, interpret sensor data
>> accurately, and manage threshold-based events.
>>
>> Signed-off-by: Khairul Anuar Romli <khairul.anuar.romli@...era.com>
>> Signed-off-by: Muhammad Amirul Asyraf Mohamad Jamian <muhammad.amirul.asyraf.mohamad.jamian@...era.com>
>
> Please use consistent emails - this does not match sender.
>
>> ---
>> .../bindings/hwmon/altr,socfpga-hwmon.yaml | 286 ++++++++++++++++++
>> MAINTAINERS | 7 +
>> 2 files changed, 293 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/hwmon/altr,socfpga-hwmon.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/altr,socfpga-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/altr,socfpga-hwmon.yaml
>> new file mode 100644
>> index 000000000000..b69611c8bc7d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/altr,socfpga-hwmon.yaml
>> @@ -0,0 +1,286 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwmon/altr,socfpga-hwmon.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Altera Hardware monitor SOC FPGA
>
> "Monitor SoC"
>
>> +
>> +maintainers:
>> + - Ang Tien Sung <tiensung.ang@...era.com>
>> + - Muhammad Amirul Asyraf Mohamad Jamian <muhammad.amirul.asyraf.mohamad.jamian@...era.com>
>> +
>> +description: |
>> + The Altera SoC FPGA 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.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
>> +
>> + 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:
>> +
>
> No blank line
>
>> + compatible:
>> + const: altr,socfpga-hwmon
>
> No, look at other bindings. You need soc specific compatibles.
>
>> +
>> + temperature:
>> + description:
>> + The temperature node specifies mappings of temperature sensor diodes on
>> + the SoC FPGA main die and tile die.
>> + type: object
>> + properties:
>> + '#address-cells':
>> + const: 1
>> + '#size-cells':
>> + const: 0
>> + patternProperties:
>> + "^input(@[0-9a-f]+)?$":
>> + description:
>> + The input node specifies each individual temperature sensor.
>> + type: object
>> + properties:
>> + reg:
>> + description:
>> + The temperature sensor address format contains a page number and
>> + a channel number to identify a specific temperature sensor. The
>> + page number selects the region of the device that the sensor
>> + resides. The channel number selects the temperature sensor diode
>> + in the page. The page number is defined in the upper 16-bits of
>> + the reg value while the channel number is defined in the lower
>> + 16-bits of the reg value. Channel 0 is represented by the value 0
>> + and channel 1 is represented by the value 1, and so on.
>> + label:
>> + description:
>> + A label to describe the sensor.
>> + required:
>> + - reg
>> + additionalProperties: false
>> + required:
>> + - '#address-cells'
>> + - '#size-cells'
>> + additionalProperties: false
>
> None of above is readable. Why are you sending completely different code
> than what we have in the kernel?
>
>
>> +
>> + voltage:
>> + description:
>> + The voltage node specifies mappings of voltage sensorson the 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]+)?$":
>> + description:
>> + The input node specifies each individual voltage sensor.
>> + type: object
>> + properties:
>> + reg:
>> + description:
>> + The voltage sensor address format contains a channel number to
>> + identify a specific voltage sensor. The channel number is defined
>> + in the lower 16-bits of the reg value. Channel 0 is represented by
>> + the value 0 and channel 1 is represented by the value 1, and so
>> + on.
>> + label:
>> + description:
>> + A label to describe the sensor.
>> + required:
>> + - reg
>> + additionalProperties: false
>> + required:
>> + - '#address-cells'
>> + - '#size-cells'
>> + additionalProperties: false
>> +
>> +required:
>> + - compatible
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + temp_volt {
>
> No, don't send us downstream code. Look at upstream how this is called.
>
> Read carefully writing bindings and writing schema doc. You sent
> something really needing internal review before posting.
>
> It's another example of recently poor submissions from Altera. I am
> getting tired of pointing out the same problem - you do not perform
> internal review prior posting.
>
> Is anyone from Altera going to respond on this? If not, I will just be
> grumpy NAKing your patches, because such way you just waste community's
> time.
>
> Best regards,
> Krzysztof
>
Thanks for the response,
Will rework the patches based on the feedback and send for internal
review before sending out the next version.
Regards,
Amirul
Powered by blists - more mailing lists