Hm... This stuff is more broken than I realized when I first looked at it. And the truth is that I don't know the hardware so I can't say exactly what the fix is. Probably best to just leave it alone for now... On Wed, Jan 10, 2024 at 08:25:33AM +0200, Ariel Silver wrote: > >> Hello Dan and thank you for the quick response. Much appreciated! > >> 1) I am probably wrong, but as I see it, the current code assumes that > >> 'CARDqGetTSFOffset' returns a little endian value. > >> So it calls 'qwTSFOffset = le64_to_cpu(qwTSFOffset)'. However digging in > >> the code of 'CARDqGetTSFOffset' I don't see a reason to assume that. > >> Which in my opinion can cuase a bad endianness cast on big endianness > >> systems. You're actually correct. But the thing is that le64_to_cpu() and cpu_to_le64() do the exactly the same thing. If the hardware is little endian they do nothing, but if the hardware is big endian they call bswap_64(). The only difference is how a human reader understands it and for git annotations. What I suspect is that we should be passing little endian data to iowrite32(). In other words we should change the le64_to_cpu() to cpu_to_le64(). This wouldn't change how the code works, it would just change how it is annotated. However, on the other hand, I see plenty of examples where it's passing CPU endian data to iowrite32() so maybe we should get rid of the conversion instead. Who knows? I suspect is that this driver has never worked on big endian CPUs... I would say let's not change this until we're more sure what's going on. And probably if we make changes that affect runtime as opposed to just modifying endian annotations, then that needs to be tested. regards, dan carpenter
