Re: [PATCH V7 3/4] i3c: master: Add endianness support for i3c_readl_fifo() and i3c_writel_fifo()
From: "Arnd Bergmann" <arnd@arndb.de>
Date: 2025-09-24 14:06:00
Also in:
linux-arch, linux-hardening, linux-i3c, lkml
On Wed, Sep 24, 2025, at 14:22, Guntupalli, Manikanta wrote:
quoted
@@ -55,7 +55,7 @@ static inline void i3c_readl_fifo(const void __iomem *addr, void*buf, if (nbytes & 3) { u32 tmp; - tmp = readl(addr); + readsl(addr, &tmp, 1); memcpy(buf + (nbytes & ~3), &tmp, nbytes & 3); } }We have not observed any issue on little-endian systems in our testing so far (as I mentioned earlier in asm-generic/io.h: Add big-endian MMIO accessors).
Did you test the little-endian system with the 'endian' flag set to I3C_FIFO_BIG_ENDIAN though? Your v7 code will still work on little-endian kernels if that flag is set to I3C_FIFO_LITTLE_ENDIAN, and it will also work on big-endian kernels if the flag is set to I3C_FIFO_BIG_ENDIAN. But is broken for the other two: - on little-endian kernels with I3C_FIFO_BIG_ENDIAN, the entire data buffer is byteswapped in 32-bit chunks - on big-endian kernels with I3C_FIFO_LITTLE_ENDIAN, you run into the existing bug of the swapped tail word.
That said, I understand your point about FIFO semantics being different
from fixed-endian registers. To cover both cases, we considered using
writesl() for little-endian and introducing a writesl_be() helper for
big-endian, as shown below:
static inline void i3c_writel_fifo(void __iomem *addr, const void *buf,
int nbytes, enum i3c_fifo_endian endian)
{
if (endian)
writesl_be(addr, buf, nbytes / 4);
else
writesl(addr, buf, nbytes / 4);
if (nbytes & 3) {
u32 tmp = 0;
memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
if (endian)
writesl_be(addr, &tmp, 1);
else
writesl(addr, &tmp, 1);
}
}
With this approach, both little-endian and big-endian cases works as expected.
This version should fix the cases where you have a big-endian
kernel with either I3C_FIFO_BIG_ENDIAN or I3C_FIFO_LITTLE_ENDIAN,
as neither combination does any byte swaps.
However I'm fairly sure it's still broken for little-endian
kernels when a driver asks for a I3C_FIFO_BIG_ENDIAN conversion,
same as v7.
Arnd