Re: [PATCH v10 03/16] drm/xe/hwmon: Fix thermal sensor data handling

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Oct 12, 2026 at 06:33:49PM +0530, Karthik Poosa wrote:
> Read all required thermal groups in get_mc_temp() and store them in a
> dword-aligned buffer, avoiding unaligned pcode accesses and ensuring
> correct memory controller temperature reporting.
> 
> Store the pcode thermal data and thermal limit words as little-endian,
> so their per-byte views match the hardware layout on big-endian hosts.
> 
> Signed-off-by: Karthik Poosa <karthik.poosa@intel.com>
> Fixes: 3a0cb885e111 ("drm/xe/hwmon: Expose memory controller temperature")
> Fixes: c332fba805d6 ("drm/xe/hwmon: Expose temperature limits")

try to keep your sob after the fixes lines

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
> 
> v2:
> - Store thermal data and limit words as little-endian to fix the
>   byte-view decoding on big-endian hosts. (Sashiko)
> 
>  drivers/gpu/drm/xe/xe_hwmon.c | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
> index c63aa708ab6f..2d1fa9159c58 100644
> --- a/drivers/gpu/drm/xe/xe_hwmon.c
> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> @@ -138,13 +138,17 @@ struct xe_hwmon_thermal_info {
>  	union {
>  		/** @limit: temperatures limits */
>  		u8 limit[TEMP_LIMIT_MAX];
> -		/** @data: temperature limits in dwords */
> -		u32 data[DIV_ROUND_UP(TEMP_LIMIT_MAX, sizeof(u32))];
> +		/** @data: temperature limits as little-endian dwords */
> +		__le32 data[DIV_ROUND_UP(TEMP_LIMIT_MAX, sizeof(u32))];
>  	};
>  	/** @count: no of temperature sensors available for the platform */
>  	u8 count;
> -	/** @value: per-sensor raw mailbox temperature; bit7=sign, bits6:0=magnitude */
> -	u8 value[U8_MAX];
> +	union {
> +		/** @value: per-sensor raw mailbox temperature; bit7=sign, bits6:0=magnitude */
> +		u8 value[U8_MAX + 1];
> +		/** @dword: sensor values as little-endian dwords, u32-aligned for pcode reads */
> +		__le32 dword[DIV_ROUND_UP(U8_MAX + 1, sizeof(u32))];
> +	};
>  	/** @vram_label: vram label names */
>  	char vram_label[MAX_VRAM_CHANNELS][MAX_LABEL_SIZE];
>  };
> @@ -798,16 +802,17 @@ static const struct hwmon_channel_info * const hwmon_info[] = {
>  static int xe_hwmon_pcode_read_thermal_info(struct xe_hwmon *hwmon)
>  {
>  	struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
> -	u32 config = 0;
> +	u32 config = 0, data0 = 0, data1 = 0;
>  	int ret;
>  
>  	ret = xe_pcode_read(root_tile, PCODE_MBOX(PCODE_THERMAL_INFO, READ_THERMAL_LIMITS, 0),
> -			    &hwmon->temp.data[0], &hwmon->temp.data[1]);
> +			    &data0, &data1);
>  	if (ret)
>  		return ret;
>  
> -	drm_dbg(&hwmon->xe->drm, "thermal info read val 0x%x val1 0x%x\n",
> -		hwmon->temp.data[0], hwmon->temp.data[1]);
> +	drm_dbg(&hwmon->xe->drm, "thermal info read val 0x%x val1 0x%x\n", data0, data1);
> +	hwmon->temp.data[0] = cpu_to_le32(data0);
> +	hwmon->temp.data[1] = cpu_to_le32(data1);
>  
>  	ret = xe_pcode_read(root_tile, PCODE_MBOX(PCODE_THERMAL_INFO, READ_THERMAL_CONFIG, 0),
>  			    &config, NULL);
> @@ -831,16 +836,18 @@ static int mbx_temp_decode(u8 value)
>  static int get_mc_temp(struct xe_hwmon *hwmon, long *val)
>  {
>  	struct xe_tile *root_tile = xe_device_get_root_tile(hwmon->xe);
> -	u32 *dword = (u32 *)hwmon->temp.value;
>  	s32 average = 0;
>  	int ret, i;
>  
> -	for (i = 0; i < DIV_ROUND_UP(TEMP_LIMIT_MAX, sizeof(u32)); i++) {
> +	for (i = 0; i < DIV_ROUND_UP(hwmon->temp.count, sizeof(u32)); i++) {
> +		u32 data = 0;
> +
>  		ret = xe_pcode_read(root_tile, PCODE_MBOX(PCODE_THERMAL_INFO, READ_THERMAL_DATA, i),
> -				    (dword + i), NULL);
> +				    &data, NULL);
>  		if (ret)
>  			return ret;
> -		drm_dbg(&hwmon->xe->drm, "thermal data for group %d val 0x%x\n", i, dword[i]);
> +		drm_dbg(&hwmon->xe->drm, "thermal data for group %d val 0x%x\n", i, data);
> +		hwmon->temp.dword[i] = cpu_to_le32(data);
>  	}
>  
>  	for (i = TEMP_INDEX_MCTRL; i < hwmon->temp.count - 1; i++)
> -- 
> 2.25.1
>