On Tue, Sep 23, 2025, at 17:45, Manikanta Guntupalli wrote:
> /**
> * i3c_writel_fifo - Write data buffer to 32bit FIFO
> * @addr: FIFO Address to write to
> * @buf: Pointer to the data bytes to write
> * @nbytes: Number of bytes to write
> + * @endian: Endianness of FIFO write
> */
> static inline void i3c_writel_fifo(void __iomem *addr, const void *buf,
> - int nbytes)
> + int nbytes, enum i3c_fifo_endian endian)
> {
> - writesl(addr, buf, nbytes / 4);
> + if (endian)
> + writesl_be(addr, buf, nbytes / 4);
> + else
> + writesl(addr, buf, nbytes / 4);
> +
This seems counter-intuitive: a FIFO doesn't really have
an endianness, it is instead used to transfer a stream of
bytes, so if the device has a fixed endianess, the
FIFO still needs to be read using a plain writesl().
I see that your writesl_be() has an incorrect definition, which
would lead to the i3c_writel_fifo() function accidentally still
working if both the device and CPU use big-endian registers:
static inline void writesl_be(volatile void __iomem *addr,
const void *buffer,
unsigned int count)
{
if (count) {
const u32 *buf = buffer;
do {
__raw_writel((u32 __force)__cpu_to_be32(*buf), addr);
buf++;
} while (--count);
}
}
The __cpu_to_be32() call that you add here means that the
FIFO data is swapped on little-endian CPUs but not swapped
on big-endian ones. Compare this to the normal writesl()
function that never swaps because it writes a byte stream.
> if (nbytes & 3) {
> u32 tmp = 0;
>
> memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
> - writel(tmp, addr);
> +
> + if (endian)
> + writel_be(tmp, addr);
> + else
> + writel(tmp, addr);
This bit however seems to fix a bug, but does so in a
confusing way. The way the FIFO registers usually deal
with excess bytes is to put them into the first bytes
of the FIFO register, so this should just be a
writesl(addr, &tmp, 1);
to write one set of four bytes into the FIFO without
endian-swapping.
Could it be that you are just trying to use a normal
i3c adapter with little-endian registers on a normal
big-endian machine but ran into this bug?
Arnd
