> On 24 Feb 2025, at 9:26 PM, Aditya Garg <gargadity...@live.com> wrote:
> 
> 
> 
>> On 24 Feb 2025, at 9:23 PM, andriy.shevche...@linux.intel.com wrote:
>> 
>> On Mon, Feb 24, 2025 at 03:40:29PM +0000, Aditya Garg wrote:
>>>>> On 24 Feb 2025, at 9:08 PM, andriy.shevche...@linux.intel.com wrote:
>>>> On Mon, Feb 24, 2025 at 03:32:56PM +0000, Aditya Garg wrote:
>>>>>> On 24 Feb 2025, at 8:50 PM, Aditya Garg <gargadity...@live.com> wrote:
>>>>>>> On 24 Feb 2025, at 8:41 PM, andriy.shevche...@linux.intel.com wrote:
>>>>>>> On Mon, Feb 24, 2025 at 03:03:40PM +0000, Aditya Garg wrote:
>>>>>>>>>> On 24 Feb 2025, at 8:27 PM, andriy.shevche...@linux.intel.com wrote:
>>>>>>>>> On Mon, Feb 24, 2025 at 02:32:37PM +0000, Aditya Garg wrote:
>>>>>>>>>>> On 24 Feb 2025, at 7:30 PM, andriy.shevche...@linux.intel.com wrote:
>>>>>>>>>>>>>> On Mon, Feb 24, 2025 at 01:40:20PM +0000, Aditya Garg wrote:
>>> 
>>> ...
>>> 
>>>>>>>>>>>>>>> +#define __APPLETBDRM_MSG_STR4(str4) ((__le32 __force)((str4[0] 
>>>>>>>>>>>>>>> << 24) | (str4[1] << 16) | (str4[2] << 8) | str4[3]))
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> As commented previously this is quite strange what's going on 
>>>>>>>>>>>>>> with endianess in
>>>>>>>>>>>>>> this driver. Especially the above weirdness when 
>>>>>>>>>>>>>> get_unaligned_be32() is being
>>>>>>>>>>>>>> open coded and force-cast to __le32.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I would assume it was also mimicked from the Windows driver, 
>>>>>>>>>>>>> though I haven't
>>>>>>>>>>>>> really tried exploring this there.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I’d rather be happy if you give me code change suggestions and 
>>>>>>>>>>>>> let me review
>>>>>>>>>>>>> and test them
>>>>>>>>>>>> 
>>>>>>>>>>>> For the starter I would do the following for all related constants 
>>>>>>>>>>>> and
>>>>>>>>>>>> drop that weird and ugly macros at the top (it also has an issue 
>>>>>>>>>>>> with
>>>>>>>>>>>> the str4 length as it is 5 bytes long, not 4, btw):
>>>>>>>>>>>> 
>>>>>>>>>>>> #define APPLETBDRM_MSG_CLEAR_DISPLAY cpu_to_le32(0x434c5244) /* 
>>>>>>>>>>>> CLRD */
>>>>>>>>>> 
>>>>>>>>>> Lemme test this.
>>>>>>>>> 
>>>>>>>>> Just in case it won't work, reverse bytes in the integer. Because I 
>>>>>>>>> was lost in
>>>>>>>>> this conversion.
>>>>>>> 
>>>>>>> It works. What I understand is that you used the macro to get the final 
>>>>>>> hex and converted it into little endian, which on the x86 macs would 
>>>>>>> technically remain the same.
>>>>>> 
>>>>>> And now that I oberved again, %p4cc is actually printing these CLRD, 
>>>>>> REDY etc
>>>>>> in reverse order, probably the reason %p4ch was chosen. And I am unable 
>>>>>> to
>>>>>> find what macro upstream can be used.
>>>>> 
>>>>> %.4s should work as it technically not DRM 4cc, but specifics of the 
>>>>> protocol
>>>>> (that reminds me about ACPI that uses 4cc a lot).
>>> 
>>> I still get reverse order in that.
>> 
>> Ah, right, it will give you the first letter as LSB, indeed. At the end of 
>> the
>> day if it's so important, there are ways how to solve that without using 
>> %p4cc.
>> But if others (and esp. PRINTK maintainers) want to have / don't object 
>> having
>> that extension, why not?
> 
> Right, but what to do about the case of little endian and host endian? I 
> remember the statement "for the sake of completeness" for them. Do you think 
> just host endian and reverse endian should be just fine? Or you got any "no 
> sparse warning" way to get it done? The macros to convert to le32/be32 expect 
> a u32 value, but in those cases we actually are passing a le32/be32 value.

We can convert the __le32 to be printed by %p4cc to __be32.

__be32 expected_response_be = cpu_to_be32(le32_to_cpu(expected_response));

That's seems to be the best possible option to fix this revert.

Reply via email to