[PATCH v4 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs
andriy.shevchenko at linux.intel.com
andriy.shevchenko at linux.intel.com
Mon Feb 24 15:36:30 UTC 2025
On Mon, Feb 24, 2025 at 03:20:13PM +0000, Aditya Garg wrote:
> > On 24 Feb 2025, at 8:41 PM, andriy.shevchenko at 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.shevchenko at 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.shevchenko at 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.
Right, the problem is the macro itself which does really weird things altogether.
Using integer + comment much clearer in my opinion.
> >>> (assuming we stick with __leXX for now). This will be much less confusing.
--
With Best Regards,
Andy Shevchenko
More information about the dri-devel
mailing list