Re: [PATCH] drm/xe/hwmon: Store mailbox temperature data as bytes

["Poosa, Karthik" <karthik.poosa@intel.com>]

On 26-10-2026 17:24, Mallesh, Koujalagi wrote:
>
> On 26-10-2026 04:47 pm, Poosa, Karthik wrote:
>>
>> On 23-10-2026 16:06, Mallesh Koujalagi wrote:
>>> Store pcode mailbox temperature data directly in the byte array
>>> using put_unaligned_le32() instead of accessing it through
>>> different views of the same storage.
>>>
>>> This keeps the layout unchanged and makes the data access
>>> consistent and endian-safe.
>>
>> 1. Is this patch addressing any issue reported by a static analysis 
>> tool?
>>
> Yes, it's reported by static analysis tool.
>> 2. Given that |cpu_to_le3()| already handles endianness, do we still 
>> need this change?
>
> Yes, cpu_to_le32() already handles endianness correctly. The main 
> reason for this change is not
>
> to fix an endianness issue, however to address the static analysis 
> warning about inconsistent union access.
>
> The code writes data through temp.dword[] and later reads it through 
> temp.value[], which was flagged as
>
> inconsistent access.
>
> To avoid that used put_unaligned_le32 writes directly into the byte 
> array that is later
>
> read, making the access pattern consistent.
I understand the static-analysis warning about writing through |dword[]| 
and reading through |value[]|. However, since |value[]| is accessed as 
|u8|, there should be no alignment issue, and |cpu_to_le32()| already 
provides the expected byte representation. Is there any known functional 
issue with the current code, or is this change solely to address the 
static-analysis warning?
>
>
> Thanks,
>
> -/Mallesh
>
>>
>>>
>>> Fixes: ca6e33701de8 ("drm/xe/hwmon: Fix thermal sensor data handling")
>>> Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_hwmon.c | 15 ++++++++-------
>>>   1 file changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c 
>>> b/drivers/gpu/drm/xe/xe_hwmon.c
>>> index 42acfeb8818c..7bb1223dc30b 100644
>>> --- a/drivers/gpu/drm/xe/xe_hwmon.c
>>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
>>> @@ -7,6 +7,7 @@
>>>   #include <linux/hwmon.h>
>>>   #include <linux/jiffies.h>
>>>   #include <linux/types.h>
>>> +#include <linux/unaligned.h>
>>>   #include <linux/units.h>
>>>     #include <drm/drm_managed.h>
>>> @@ -165,12 +166,12 @@ struct xe_hwmon_thermal_info {
>>>       bool available[CHANNEL_MAX];
>>>       /** @msu_mask: VRAM subsystem enable mask read from MMIO */
>>>       u32 msu_mask;
>>> -    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))];
>>> -    };
>>> +    /**
>>> +     * @value: per-sensor raw mailbox temperature; bit7=sign, 
>>> bits6:0=magnitude.
>>> +     * Written one dword at a time via put_unaligned_le32() to
>> Written -> Write
>>> +     * avoid inconsistent union access between dword and byte views.
>>> +     */
>>> +    u8 value[U8_MAX + 1];
>>>       /** @vram_label: vram label names, dynamically allocated based 
>>> on vram_count */
>>>       char (*vram_label)[MAX_LABEL_SIZE];
>>>   };
>>> @@ -995,7 +996,7 @@ static int get_mc_temp(struct xe_hwmon *hwmon, 
>>> long *val)
>>>           if (ret)
>>>               return ret;
>>>           drm_dbg(&hwmon->xe->drm, "thermal data for group %d val 
>>> 0x%x\n", i, data);
>>> -        hwmon->temp.dword[i] = cpu_to_le32(data);
>>> +        put_unaligned_le32(data, &hwmon->temp.value[i * sizeof(u32)]);
>>>       }
>>>         for (i = start; i < end; i++) {