Re: [PATCH] hwmon: (socfpga) add Agilex 5 channel mapping

[Guenter Roeck <linux@roeck-us.net>]

On 8/10/26 23:11, tze.yee.ng@altera.com wrote:
> From: Tze Yee Ng <tze.yee.ng@altera.com>
> 
> Add temperature channel mapping for Agilex 5 and bind it to the
> "intel,agilex5-svc" compatible string. Reuse the Agilex voltage channels.
> Agilex 5 omits temperature channel 2 (top-left corner) because that
> sensor is not present; remaining sensors keep Agilex channel numbers.
> 
> Match intel,agilex5-svc before intel,agilex-svc so dual-compatible DT
> nodes select the Agilex 5 board data.
> 
> Update the socfpga-hwmon documentation to list Agilex 5, the matching
> compatible string, and the SDM temperature/voltage channel tables.
> 
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
>   Documentation/hwmon/socfpga-hwmon.rst | 52 +++++++++++++++++++++++++++
>   drivers/hwmon/socfpga-hwmon.c         | 27 ++++++++++++++
>   2 files changed, 79 insertions(+)
> 
> diff --git a/Documentation/hwmon/socfpga-hwmon.rst b/Documentation/hwmon/socfpga-hwmon.rst
> index e5da42556a62..664103a38195 100644
> --- a/Documentation/hwmon/socfpga-hwmon.rst
> +++ b/Documentation/hwmon/socfpga-hwmon.rst
> @@ -7,6 +7,7 @@ Supported chips:
>   
>    * Altera Stratix 10 SoC FPGA
>    * Altera Agilex SoC FPGA
> + * Altera Agilex 5 SoC FPGA
>   
>   Authors:
>         - Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
> @@ -32,3 +33,54 @@ driver based on the service layer compatible string:
>   
>     * intel,stratix10-svc
>     * intel,agilex-svc
> +  * intel,agilex5-svc
> +
> +Channel mappings are fixed in the driver (not described in DT). The tables
> +below list the SDM page/channel encodings used for each family.
> +
> +Temperature channels
> +~~~~~~~~~~~~~~~~~~~~
> +
> +==========  ====  =======  =================================
> +Family      Page  Channel  Label
> +==========  ====  =======  =================================
> +Stratix 10  0     0        Main Die SDM
> +Agilex      0     0        Main Die SDM
> +Agilex      1     0        Main Die corner bottom left max
> +Agilex      2     0        Main Die corner top left max
> +Agilex      3     0        Main Die corner bottom right max
> +Agilex      4     0        Main Die corner top right max
> +Agilex 5    0     0        Main Die SDM
> +Agilex 5    1     0        Main Die corner bottom left max
> +Agilex 5    3     0        Main Die corner bottom right max
> +Agilex 5    4     0        Main Die corner top right max
> +==========  ====  =======  =================================
> +
> +Agilex 5 omits SDM temperature channel 2 (top-left corner on Agilex)
> +because that sensor is not present in hardware. The remaining sensors keep
> +the same channel numbers as Agilex.
> +
> +Voltage channels
> +~~~~~~~~~~~~~~~~
> +
> +==========  ====  =======  =================
> +Family      Page  Channel  Label
> +==========  ====  =======  =================
> +Stratix 10  0     2        0.8V VCC
> +Stratix 10  0     3        1.8V VCCIO_SDM
> +Stratix 10  0     6        0.9V VCCERAM
> +Agilex      0     2        0.8V VCC
> +Agilex      0     3        1.8V VCCIO_SDM
> +Agilex      0     4        1.8V VCCPT
> +Agilex      0     5        1.2V VCCCRCORE
> +Agilex      0     6        0.9V VCCH
> +Agilex      0     7        0.8V VCCL
> +Agilex 5    0     2        0.8V VCC
> +Agilex 5    0     3        1.8V VCCIO_SDM
> +Agilex 5    0     4        1.8V VCCPT
> +Agilex 5    0     5        1.2V VCCCRCORE
> +Agilex 5    0     6        0.9V VCCH
> +Agilex 5    0     7        0.8V VCCL
> +==========  ====  =======  =================
> +
> +Agilex 5 reuses the Agilex voltage SDM page/channel layout and labels.
> diff --git a/drivers/hwmon/socfpga-hwmon.c b/drivers/hwmon/socfpga-hwmon.c
> index 5b43274d0aa2..92bffc02e309 100644
> --- a/drivers/hwmon/socfpga-hwmon.c
> +++ b/drivers/hwmon/socfpga-hwmon.c
> @@ -433,6 +433,30 @@ static const struct socfpga_hwmon_board_data agilex_hwmon_board = {
>   	.num_volt = ARRAY_SIZE(agilex_hwmon_volt_channels),
>   };
>   
> +/*
> + * Agilex 5 exposes the SDM and three corner temperature sensors. Channel 2
> + * (top-left corner on Agilex) is not present in hardware, so the SDM channel
> + * numbering keeps the gap (0, 1, 3, 4) rather than renumbering.
> + */
> +static const struct socfpga_hwmon_channel agilex5_hwmon_temp_channels[] = {
> +	{ SOCFPGA_HWMON_CHAN(0, 0), "Main Die SDM" },
> +	{ SOCFPGA_HWMON_CHAN(1, 0), "Main Die corner bottom left max" },
> +	{ SOCFPGA_HWMON_CHAN(3, 0), "Main Die corner bottom right max" },
> +	{ SOCFPGA_HWMON_CHAN(4, 0), "Main Die corner top right max" },
> +};
> +
> +/*
> + * Agilex 5 reuses the Agilex voltage SDM page/channel encoding and labels.
> + * Check more specific "intel,agilex5-svc" before "intel,agilex-svc" below so a
> + * node that lists both compatibles selects this board data.
> + */
> +static const struct socfpga_hwmon_board_data agilex5_hwmon_board = {
> +	.temp = agilex5_hwmon_temp_channels,
> +	.num_temp = ARRAY_SIZE(agilex5_hwmon_temp_channels),
> +	.volt = agilex_hwmon_volt_channels,
> +	.num_volt = ARRAY_SIZE(agilex_hwmon_volt_channels),
> +};
> +
>   static const struct socfpga_hwmon_board_data *
>   socfpga_hwmon_get_board(struct device *dev)
>   {
> @@ -443,6 +467,9 @@ socfpga_hwmon_get_board(struct device *dev)
>   
>   	if (of_device_is_compatible(np, "intel,stratix10-svc"))
>   		return &s10_hwmon_board;
> +	/* Prefer the more specific Agilex 5 compatible over generic Agilex. */

I don't think that is going to work (see Sashiko feedback). On top of that, it is
conceptually wrong.

Guenter

> +	if (of_device_is_compatible(np, "intel,agilex5-svc"))
> +		return &agilex5_hwmon_board;
>   	if (of_device_is_compatible(np, "intel,agilex-svc"))
>   		return &agilex_hwmon_board;
>