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

["Mallesh, Koujalagi" <mallesh.koujalagi@intel.com>]

On 28-10-2026 05:06 pm, Poosa, Karthik wrote:
>
> 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?

Agree, no alignment or endianness issue since we are using cpu_to_le32() 
which provides the correct byte representation. This is purely a 
code-hygiene cleanup to silence the static analysis warning via 
put_unaligned_le32 so I'll drop the Fixes: tag and add no functional 
change. Please let me know.


Thanks,

-/Mallesh

>>
>>
>> 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++) {