[PATCH] [v2] i3c: fix big-endian FIFO transfers

[PATCH] [v2] i3c: fix big-endian FIFO transfers

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

Short MMIO transfers that are not a multiple of four bytes in size need
a special case for the final bytes, however the existing implementation
is not endian-safe and introduces an incorrect byteswap on big-endian
kernels.

This usually does not cause problems because most systems are
little-endian and most transfers are multiple of four bytes long, but
still needs to be fixed to avoid the extra byteswap.

Change the special case for both i3c_writel_fifo() and i3c_readl_fifo()
to use non-byteswapping writesl() and readsl() with a single element
instead of the byteswapping writel()/readl() that are meant for individual
MMIO registers. As data is copied between a FIFO and a memory buffer,
the writesl()/readsl() loops are typically based on __raw_readl()/
__raw_writel(), resulting in the order of bytes in the FIFO to match
the order in the buffer, regardless of the CPU endianess.

The earlier versions in the dw-i3c and i3c-master-cdns had a correct
implementation, but the generic version that was recently added broke it.

Fixes: 733b439375b4 ("i3c: master: Add inline i3c_readl_fifo() and i3c_writel_fifo()")
Cc: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This was a recent regression, the version in 6.16 still works,
but 6.17-rc is broken.

v2 changes:
 - add code comments
 - write correct data buffer
---
 drivers/i3c/internals.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
index 0d857cc68cc5..79ceaa5f5afd 100644
--- a/drivers/i3c/internals.h
+++ b/drivers/i3c/internals.h
@@ -38,7 +38,11 @@ static inline void i3c_writel_fifo(void __iomem *addr, const void *buf,
 		u32 tmp = 0;
 
 		memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
-		writel(tmp, addr);
+		/*
+		 * writesl() instead of writel() to keep FIFO
+		 * byteorder on big-endian targets
+		 */
+		writesl(addr, &tmp, 1);
 	}
 }
 
@@ -55,7 +59,11 @@ static inline void i3c_readl_fifo(const void __iomem *addr, void *buf,
 	if (nbytes & 3) {
 		u32 tmp;
 
-		tmp = readl(addr);
+		/*
+		 * readsl() instead of readl() to keep FIFO
+		 * byteorder on big-endian targets
+		 */
+		readsl(addr, &tmp, 1);
 		memcpy(buf + (nbytes & ~3), &tmp, nbytes & 3);
 	}
 }
-- 
2.39.5


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers

[Jorge Marques]

On Wed, Sep 24, 2025 at 10:18:33PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Short MMIO transfers that are not a multiple of four bytes in size need
> a special case for the final bytes, however the existing implementation
> is not endian-safe and introduces an incorrect byteswap on big-endian
> kernels.
> 
> This usually does not cause problems because most systems are
> little-endian and most transfers are multiple of four bytes long, but
> still needs to be fixed to avoid the extra byteswap.
> 
> Change the special case for both i3c_writel_fifo() and i3c_readl_fifo()
> to use non-byteswapping writesl() and readsl() with a single element
> instead of the byteswapping writel()/readl() that are meant for individual
> MMIO registers. As data is copied between a FIFO and a memory buffer,
> the writesl()/readsl() loops are typically based on __raw_readl()/
> __raw_writel(), resulting in the order of bytes in the FIFO to match
> the order in the buffer, regardless of the CPU endianess.
> 
> The earlier versions in the dw-i3c and i3c-master-cdns had a correct
> implementation, but the generic version that was recently added broke it.
> 
> Fixes: 733b439375b4 ("i3c: master: Add inline i3c_readl_fifo() and i3c_writel_fifo()")
> Cc: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This was a recent regression, the version in 6.16 still works,
> but 6.17-rc is broken.
> 
> v2 changes:
>  - add code comments
>  - write correct data buffer
> ---
>  drivers/i3c/internals.h | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
> index 0d857cc68cc5..79ceaa5f5afd 100644
> --- a/drivers/i3c/internals.h
> +++ b/drivers/i3c/internals.h
> @@ -38,7 +38,11 @@ static inline void i3c_writel_fifo(void __iomem *addr, const void *buf,
>  		u32 tmp = 0;
>  
>  		memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
> -		writel(tmp, addr);
> +		/*
> +		 * writesl() instead of writel() to keep FIFO
> +		 * byteorder on big-endian targets
> +		 */
> +		writesl(addr, &tmp, 1);
>  	}
>  }
>  
> @@ -55,7 +59,11 @@ static inline void i3c_readl_fifo(const void __iomem *addr, void *buf,
>  	if (nbytes & 3) {
>  		u32 tmp;
>  
> -		tmp = readl(addr);
> +		/*
> +		 * readsl() instead of readl() to keep FIFO
> +		 * byteorder on big-endian targets
> +		 */
> +		readsl(addr, &tmp, 1);
>  		memcpy(buf + (nbytes & ~3), &tmp, nbytes & 3);
>  	}
>  }
Reviewed-by: Jorge Marques <jorge.marques@analog.com>
> -- 
> 2.39.5
> 

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH] [v2] i3c: fix big-endian FIFO transfers

[Guntupalli, Manikanta]

[Public]

Hi,

> -----Original Message-----
> From: Jorge Marques <gastmaier@gmail.com>
> Sent: Thursday, September 25, 2025 12:47 PM
> To: Arnd Bergmann <arnd@kernel.org>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>; Jorge Marques
> <jorge.marques@analog.com>; Wolfram Sang <wsa+renesas@sang-
> engineering.com>; Frank Li <Frank.Li@nxp.com>; Arnd Bergmann
> <arnd@arndb.de>; Guntupalli, Manikanta <manikanta.guntupalli@amd.com>; linux-
> i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers
>
> On Wed, Sep 24, 2025 at 10:18:33PM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > Short MMIO transfers that are not a multiple of four bytes in size
> > need a special case for the final bytes, however the existing
> > implementation is not endian-safe and introduces an incorrect byteswap
> > on big-endian kernels.
> >
> > This usually does not cause problems because most systems are
> > little-endian and most transfers are multiple of four bytes long, but
> > still needs to be fixed to avoid the extra byteswap.
> >
> > Change the special case for both i3c_writel_fifo() and
> > i3c_readl_fifo() to use non-byteswapping writesl() and readsl() with a
> > single element instead of the byteswapping writel()/readl() that are
> > meant for individual MMIO registers. As data is copied between a FIFO
> > and a memory buffer, the writesl()/readsl() loops are typically based
> > on __raw_readl()/ __raw_writel(), resulting in the order of bytes in
> > the FIFO to match the order in the buffer, regardless of the CPU endianess.
> >
> > The earlier versions in the dw-i3c and i3c-master-cdns had a correct
> > implementation, but the generic version that was recently added broke it.
> >
> > Fixes: 733b439375b4 ("i3c: master: Add inline i3c_readl_fifo() and
> > i3c_writel_fifo()")
> > Cc: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > This was a recent regression, the version in 6.16 still works, but
> > 6.17-rc is broken.
> >
> > v2 changes:
> >  - add code comments
> >  - write correct data buffer
> > ---
> >  drivers/i3c/internals.h | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h index
> > 0d857cc68cc5..79ceaa5f5afd 100644
> > --- a/drivers/i3c/internals.h
> > +++ b/drivers/i3c/internals.h
> > @@ -38,7 +38,11 @@ static inline void i3c_writel_fifo(void __iomem *addr, const
> void *buf,
> >             u32 tmp = 0;
> >
> >             memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
> > -           writel(tmp, addr);
> > +           /*
> > +            * writesl() instead of writel() to keep FIFO
> > +            * byteorder on big-endian targets
> > +            */
> > +           writesl(addr, &tmp, 1);
> >     }
> >  }
> >
> > @@ -55,7 +59,11 @@ static inline void i3c_readl_fifo(const void __iomem *addr,
> void *buf,
> >     if (nbytes & 3) {
> >             u32 tmp;
> >
> > -           tmp = readl(addr);
> > +           /*
> > +            * readsl() instead of readl() to keep FIFO
> > +            * byteorder on big-endian targets
> > +            */
> > +           readsl(addr, &tmp, 1);
> >             memcpy(buf + (nbytes & ~3), &tmp, nbytes & 3);
> >     }
> >  }
> Reviewed-by: Jorge Marques <jorge.marques@analog.com>
> > --
> > 2.39.5
> >

This patch fixes the sub-word transfer case on big-endian kernels, but it still does not address the scenario of little-endian kernels accessing big-endian FIFOs.

With the current version, i3c_writel_fifo() and i3c_readl_fifo() only work when the FIFO has the same endianness as the CPU. On platforms such as the ZCU102 (Zynq UltraScale+ MPSoC, Cortex-A53, little-endian), the I3C FIFOs are big-endian, and this patch alone is not sufficient - transfers fail in that configuration.

We have validated this on ZCU102, and the mismatch between LE kernel and BE FIFO is still an issue.

On top of this fix, explicit FIFO endianness support is required, as proposed in [PATCH v7 3/4] "i3c: master: Add endianness support for i3c_readl_fifo() and i3c_writel_fifo()". That approach adds an endian argument and uses writesl_be()/readsl_be() where necessary, e.g.:

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);
        }
}


Thanks,
Manikanta.

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers

[Nuno Sá]

On Thu, 2025-09-25 at 07:37 +0000, Guntupalli, Manikanta wrote:
> [Public]
> 
> Hi,
> 
> > -----Original Message-----
> > From: Jorge Marques <gastmaier@gmail.com>
> > Sent: Thursday, September 25, 2025 12:47 PM
> > To: Arnd Bergmann <arnd@kernel.org>
> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>; Jorge Marques
> > <jorge.marques@analog.com>; Wolfram Sang <wsa+renesas@sang-
> > engineering.com>; Frank Li <Frank.Li@nxp.com>; Arnd Bergmann
> > <arnd@arndb.de>; Guntupalli, Manikanta <manikanta.guntupalli@amd.com>;
> > linux-
> > i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers
> > 
> > On Wed, Sep 24, 2025 at 10:18:33PM +0200, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > > 
> > > Short MMIO transfers that are not a multiple of four bytes in size
> > > need a special case for the final bytes, however the existing
> > > implementation is not endian-safe and introduces an incorrect byteswap
> > > on big-endian kernels.
> > > 
> > > This usually does not cause problems because most systems are
> > > little-endian and most transfers are multiple of four bytes long, but
> > > still needs to be fixed to avoid the extra byteswap.
> > > 
> > > Change the special case for both i3c_writel_fifo() and
> > > i3c_readl_fifo() to use non-byteswapping writesl() and readsl() with a
> > > single element instead of the byteswapping writel()/readl() that are
> > > meant for individual MMIO registers. As data is copied between a FIFO
> > > and a memory buffer, the writesl()/readsl() loops are typically based
> > > on __raw_readl()/ __raw_writel(), resulting in the order of bytes in
> > > the FIFO to match the order in the buffer, regardless of the CPU
> > > endianess.
> > > 
> > > The earlier versions in the dw-i3c and i3c-master-cdns had a correct
> > > implementation, but the generic version that was recently added broke it.
> > > 
> > > Fixes: 733b439375b4 ("i3c: master: Add inline i3c_readl_fifo() and
> > > i3c_writel_fifo()")
> > > Cc: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > > This was a recent regression, the version in 6.16 still works, but
> > > 6.17-rc is broken.
> > > 
> > > v2 changes:
> > >  - add code comments
> > >  - write correct data buffer
> > > ---
> > >  drivers/i3c/internals.h | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h index
> > > 0d857cc68cc5..79ceaa5f5afd 100644
> > > --- a/drivers/i3c/internals.h
> > > +++ b/drivers/i3c/internals.h
> > > @@ -38,7 +38,11 @@ static inline void i3c_writel_fifo(void __iomem *addr,
> > > const
> > void *buf,
> > >             u32 tmp = 0;
> > > 
> > >             memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
> > > -           writel(tmp, addr);
> > > +           /*
> > > +            * writesl() instead of writel() to keep FIFO
> > > +            * byteorder on big-endian targets
> > > +            */
> > > +           writesl(addr, &tmp, 1);
> > >     }
> > >  }
> > > 
> > > @@ -55,7 +59,11 @@ static inline void i3c_readl_fifo(const void __iomem
> > > *addr,
> > void *buf,
> > >     if (nbytes & 3) {
> > >             u32 tmp;
> > > 
> > > -           tmp = readl(addr);
> > > +           /*
> > > +            * readsl() instead of readl() to keep FIFO
> > > +            * byteorder on big-endian targets
> > > +            */
> > > +           readsl(addr, &tmp, 1);
> > >             memcpy(buf + (nbytes & ~3), &tmp, nbytes & 3);
> > >     }
> > >  }
> > Reviewed-by: Jorge Marques <jorge.marques@analog.com>
> > > --
> > > 2.39.5
> > > 
> 
> This patch fixes the sub-word transfer case on big-endian kernels, but it
> still does not address the scenario of little-endian kernels accessing big-
> endian FIFOs.
> 

I would argue that's something for callers of these functions to care about.

- Nuno Sá

> With the current version, i3c_writel_fifo() and i3c_readl_fifo() only work
> when the FIFO has the same endianness as the CPU. On platforms such as the
> ZCU102 (Zynq UltraScale+ MPSoC, Cortex-A53, little-endian), the I3C FIFOs are
> big-endian, and this patch alone is not sufficient - transfers fail in that
> configuration.
> 
> We have validated this on ZCU102, and the mismatch between LE kernel and BE
> FIFO is still an issue.
> 
> On top of this fix, explicit FIFO endianness support is required, as proposed
> in [PATCH v7 3/4] "i3c: master: Add endianness support for i3c_readl_fifo()
> and i3c_writel_fifo()". That approach adds an endian argument and uses
> writesl_be()/readsl_be() where necessary, e.g.:
> 
> 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);
>         }
> }
> 
> 
> Thanks,
> Manikanta.

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH] [v2] i3c: fix big-endian FIFO transfers

[Guntupalli, Manikanta]

[Public]

Hi,

> -----Original Message-----
> From: Nuno Sá <noname.nuno@gmail.com>
> Sent: Thursday, September 25, 2025 1:22 PM
> To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>; Jorge Marques
> <gastmaier@gmail.com>; Arnd Bergmann <arnd@kernel.org>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>; Jorge Marques
> <jorge.marques@analog.com>; Wolfram Sang <wsa+renesas@sang-
> engineering.com>; Frank Li <Frank.Li@nxp.com>; Arnd Bergmann
> <arnd@arndb.de>; linux-i3c@lists.infradead.org; linux-kernel@vger.kernel.org; git
> (AMD-Xilinx) <git@amd.com>; Simek, Michal <michal.simek@amd.com>
> Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers
>
> On Thu, 2025-09-25 at 07:37 +0000, Guntupalli, Manikanta wrote:
> > [Public]
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Jorge Marques <gastmaier@gmail.com>
> > > Sent: Thursday, September 25, 2025 12:47 PM
> > > To: Arnd Bergmann <arnd@kernel.org>
> > > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>; Jorge Marques
> > > <jorge.marques@analog.com>; Wolfram Sang <wsa+renesas@sang-
> > > engineering.com>; Frank Li <Frank.Li@nxp.com>; Arnd Bergmann
> > > <arnd@arndb.de>; Guntupalli, Manikanta
> > > <manikanta.guntupalli@amd.com>;
> > > linux-
> > > i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers
> > >
> > > On Wed, Sep 24, 2025 at 10:18:33PM +0200, Arnd Bergmann wrote:
> > > > From: Arnd Bergmann <arnd@arndb.de>
> > > >
> > > > Short MMIO transfers that are not a multiple of four bytes in size
> > > > need a special case for the final bytes, however the existing
> > > > implementation is not endian-safe and introduces an incorrect
> > > > byteswap on big-endian kernels.
> > > >
> > > > This usually does not cause problems because most systems are
> > > > little-endian and most transfers are multiple of four bytes long,
> > > > but still needs to be fixed to avoid the extra byteswap.
> > > >
> > > > Change the special case for both i3c_writel_fifo() and
> > > > i3c_readl_fifo() to use non-byteswapping writesl() and readsl()
> > > > with a single element instead of the byteswapping writel()/readl()
> > > > that are meant for individual MMIO registers. As data is copied
> > > > between a FIFO and a memory buffer, the writesl()/readsl() loops
> > > > are typically based on __raw_readl()/ __raw_writel(), resulting in
> > > > the order of bytes in the FIFO to match the order in the buffer,
> > > > regardless of the CPU endianess.
> > > >
> > > > The earlier versions in the dw-i3c and i3c-master-cdns had a
> > > > correct implementation, but the generic version that was recently added broke
> it.
> > > >
> > > > Fixes: 733b439375b4 ("i3c: master: Add inline i3c_readl_fifo() and
> > > > i3c_writel_fifo()")
> > > > Cc: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > ---
> > > > This was a recent regression, the version in 6.16 still works, but
> > > > 6.17-rc is broken.
> > > >
> > > > v2 changes:
> > > >  - add code comments
> > > >  - write correct data buffer
> > > > ---
> > > >  drivers/i3c/internals.h | 12 ++++++++++--
> > > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
> > > > index 0d857cc68cc5..79ceaa5f5afd 100644
> > > > --- a/drivers/i3c/internals.h
> > > > +++ b/drivers/i3c/internals.h
> > > > @@ -38,7 +38,11 @@ static inline void i3c_writel_fifo(void __iomem
> > > > *addr, const
> > > void *buf,
> > > >             u32 tmp = 0;
> > > >
> > > >             memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
> > > > -           writel(tmp, addr);
> > > > +           /*
> > > > +            * writesl() instead of writel() to keep FIFO
> > > > +            * byteorder on big-endian targets
> > > > +            */
> > > > +           writesl(addr, &tmp, 1);
> > > >     }
> > > >  }
> > > >
> > > > @@ -55,7 +59,11 @@ static inline void i3c_readl_fifo(const void
> > > > __iomem *addr,
> > > void *buf,
> > > >     if (nbytes & 3) {
> > > >             u32 tmp;
> > > >
> > > > -           tmp = readl(addr);
> > > > +           /*
> > > > +            * readsl() instead of readl() to keep FIFO
> > > > +            * byteorder on big-endian targets
> > > > +            */
> > > > +           readsl(addr, &tmp, 1);
> > > >             memcpy(buf + (nbytes & ~3), &tmp, nbytes & 3);
> > > >     }
> > > >  }
> > > Reviewed-by: Jorge Marques <jorge.marques@analog.com>
> > > > --
> > > > 2.39.5
> > > >
> >
> > This patch fixes the sub-word transfer case on big-endian kernels, but
> > it still does not address the scenario of little-endian kernels
> > accessing big- endian FIFOs.
> >
>
> I would argue that's something for callers of these functions to care about.
If each I3C driver has to handle FIFO endianness individually, it introduces unnecessary duplication and overhead across drivers. Centralizing this in the FIFO access helpers keeps the logic consistent, avoids repeated boilerplate, and reduces the chance of subtle bugs.

>
> > With the current version, i3c_writel_fifo() and i3c_readl_fifo() only
> > work when the FIFO has the same endianness as the CPU. On platforms
> > such as the
> > ZCU102 (Zynq UltraScale+ MPSoC, Cortex-A53, little-endian), the I3C
> > FIFOs are big-endian, and this patch alone is not sufficient -
> > transfers fail in that configuration.
> >
> > We have validated this on ZCU102, and the mismatch between LE kernel
> > and BE FIFO is still an issue.
> >
> > On top of this fix, explicit FIFO endianness support is required, as
> > proposed in [PATCH v7 3/4] "i3c: master: Add endianness support for
> > i3c_readl_fifo() and i3c_writel_fifo()". That approach adds an endian
> > argument and uses
> > writesl_be()/readsl_be() where necessary, e.g.:
> >
> > 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);
> >         }
> > }
> >
> >
Thanks,
Manikanta.
-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers

[Nuno Sá]

On Thu, 2025-09-25 at 08:47 +0000, Guntupalli, Manikanta wrote:
> [Public]
> 
> Hi,
> 
> > -----Original Message-----
> > From: Nuno Sá <noname.nuno@gmail.com>
> > Sent: Thursday, September 25, 2025 1:22 PM
> > To: Guntupalli, Manikanta <manikanta.guntupalli@amd.com>; Jorge Marques
> > <gastmaier@gmail.com>; Arnd Bergmann <arnd@kernel.org>
> > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>; Jorge Marques
> > <jorge.marques@analog.com>; Wolfram Sang <wsa+renesas@sang-
> > engineering.com>; Frank Li <Frank.Li@nxp.com>; Arnd Bergmann
> > <arnd@arndb.de>; linux-i3c@lists.infradead.org;
> > linux-kernel@vger.kernel.org; git
> > (AMD-Xilinx) <git@amd.com>; Simek, Michal <michal.simek@amd.com>
> > Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers
> > 
> > On Thu, 2025-09-25 at 07:37 +0000, Guntupalli, Manikanta wrote:
> > > [Public]
> > > 
> > > Hi,
> > > 
> > > > -----Original Message-----
> > > > From: Jorge Marques <gastmaier@gmail.com>
> > > > Sent: Thursday, September 25, 2025 12:47 PM
> > > > To: Arnd Bergmann <arnd@kernel.org>
> > > > Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>; Jorge Marques
> > > > <jorge.marques@analog.com>; Wolfram Sang <wsa+renesas@sang-
> > > > engineering.com>; Frank Li <Frank.Li@nxp.com>; Arnd Bergmann
> > > > <arnd@arndb.de>; Guntupalli, Manikanta
> > > > <manikanta.guntupalli@amd.com>;
> > > > linux-
> > > > i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers
> > > > 
> > > > On Wed, Sep 24, 2025 at 10:18:33PM +0200, Arnd Bergmann wrote:
> > > > > From: Arnd Bergmann <arnd@arndb.de>
> > > > > 
> > > > > Short MMIO transfers that are not a multiple of four bytes in size
> > > > > need a special case for the final bytes, however the existing
> > > > > implementation is not endian-safe and introduces an incorrect
> > > > > byteswap on big-endian kernels.
> > > > > 
> > > > > This usually does not cause problems because most systems are
> > > > > little-endian and most transfers are multiple of four bytes long,
> > > > > but still needs to be fixed to avoid the extra byteswap.
> > > > > 
> > > > > Change the special case for both i3c_writel_fifo() and
> > > > > i3c_readl_fifo() to use non-byteswapping writesl() and readsl()
> > > > > with a single element instead of the byteswapping writel()/readl()
> > > > > that are meant for individual MMIO registers. As data is copied
> > > > > between a FIFO and a memory buffer, the writesl()/readsl() loops
> > > > > are typically based on __raw_readl()/ __raw_writel(), resulting in
> > > > > the order of bytes in the FIFO to match the order in the buffer,
> > > > > regardless of the CPU endianess.
> > > > > 
> > > > > The earlier versions in the dw-i3c and i3c-master-cdns had a
> > > > > correct implementation, but the generic version that was recently
> > > > > added broke
> > it.
> > > > > 
> > > > > Fixes: 733b439375b4 ("i3c: master: Add inline i3c_readl_fifo() and
> > > > > i3c_writel_fifo()")
> > > > > Cc: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
> > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > > ---
> > > > > This was a recent regression, the version in 6.16 still works, but
> > > > > 6.17-rc is broken.
> > > > > 
> > > > > v2 changes:
> > > > >  - add code comments
> > > > >  - write correct data buffer
> > > > > ---
> > > > >  drivers/i3c/internals.h | 12 ++++++++++--
> > > > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
> > > > > index 0d857cc68cc5..79ceaa5f5afd 100644
> > > > > --- a/drivers/i3c/internals.h
> > > > > +++ b/drivers/i3c/internals.h
> > > > > @@ -38,7 +38,11 @@ static inline void i3c_writel_fifo(void __iomem
> > > > > *addr, const
> > > > void *buf,
> > > > >             u32 tmp = 0;
> > > > > 
> > > > >             memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
> > > > > -           writel(tmp, addr);
> > > > > +           /*
> > > > > +            * writesl() instead of writel() to keep FIFO
> > > > > +            * byteorder on big-endian targets
> > > > > +            */
> > > > > +           writesl(addr, &tmp, 1);
> > > > >     }
> > > > >  }
> > > > > 
> > > > > @@ -55,7 +59,11 @@ static inline void i3c_readl_fifo(const void
> > > > > __iomem *addr,
> > > > void *buf,
> > > > >     if (nbytes & 3) {
> > > > >             u32 tmp;
> > > > > 
> > > > > -           tmp = readl(addr);
> > > > > +           /*
> > > > > +            * readsl() instead of readl() to keep FIFO
> > > > > +            * byteorder on big-endian targets
> > > > > +            */
> > > > > +           readsl(addr, &tmp, 1);
> > > > >             memcpy(buf + (nbytes & ~3), &tmp, nbytes & 3);
> > > > >     }
> > > > >  }
> > > > Reviewed-by: Jorge Marques <jorge.marques@analog.com>
> > > > > --
> > > > > 2.39.5
> > > > > 
> > > 
> > > This patch fixes the sub-word transfer case on big-endian kernels, but
> > > it still does not address the scenario of little-endian kernels
> > > accessing big- endian FIFOs.
> > > 
> > 
> > I would argue that's something for callers of these functions to care about.
> If each I3C driver has to handle FIFO endianness individually, it introduces
> unnecessary duplication and overhead across drivers. Centralizing this in the
> FIFO access helpers keeps the logic consistent, avoids repeated boilerplate,
> and reduces the chance of subtle bugs.

I mean, that's what spi and i2c drivers do already. With enum i3c_fifo_endian
you're already forcing users to care (or know) about endianism so they might as
well just pass the data in the proper order already (not sure if it's such a big
'burden').

That said, I'm not really in the loop for i3c so not sure what the expectations
are. IOW, have no strong feeling about this at all :)

- Nuno Sá

> > 
> > > With the current version, i3c_writel_fifo() and i3c_readl_fifo() only
> > > work when the FIFO has the same endianness as the CPU. On platforms
> > > such as the
> > > ZCU102 (Zynq UltraScale+ MPSoC, Cortex-A53, little-endian), the I3C
> > > FIFOs are big-endian, and this patch alone is not sufficient -
> > > transfers fail in that configuration.
> > > 
> > > We have validated this on ZCU102, and the mismatch between LE kernel
> > > and BE FIFO is still an issue.
> > > 
> > > On top of this fix, explicit FIFO endianness support is required, as
> > > proposed in [PATCH v7 3/4] "i3c: master: Add endianness support for
> > > i3c_readl_fifo() and i3c_writel_fifo()". That approach adds an endian
> > > argument and uses
> > > writesl_be()/readsl_be() where necessary, e.g.:
> > > 
> > > 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);
> > >         }
> > > }
> > > 
> > > 
> Thanks,
> Manikanta.

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers

[Arnd Bergmann]

On Thu, Sep 25, 2025, at 10:58, Nuno Sá wrote:
> On Thu, 2025-09-25 at 08:47 +0000, Guntupalli, Manikanta wrote:
>> > (AMD-Xilinx) <git@amd.com>; Simek, Michal <michal.simek@amd.com>
>> > Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers
>> > On Thu, 2025-09-25 at 07:37 +0000, Guntupalli, Manikanta wrote:
>> > > > i3c@lists.infradead.org; linux-kernel@vger.kernel.org
>> > > > Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers
>> > > > On Wed, Sep 24, 2025 at 10:18:33PM +0200, Arnd Bergmann wrote:
>> > 
>> > I would argue that's something for callers of these functions to care about.
>> If each I3C driver has to handle FIFO endianness individually, it introduces
>> unnecessary duplication and overhead across drivers. Centralizing this in the
>> FIFO access helpers keeps the logic consistent, avoids repeated boilerplate,
>> and reduces the chance of subtle bugs.
>
> I mean, that's what spi and i2c drivers do already. With enum i3c_fifo_endian
> you're already forcing users to care (or know) about endianism so they might as
> well just pass the data in the proper order already (not sure if it's such a big
> 'burden').

Can you give an example of an spi or i2c driver handles a similar
situation to the new i3c driver? As far as I can tell, swapping
the bytes in a FIFO register is very unusual for a hardware design
and probably a mistake rather than an intentional decision.

On the other hand, I can find drivers that are obviously wrong
on big-endian kernels, such as Tegra's i2c_writesl_vi() function
being unintentionally swapped from i2c_writesl() on big-endian.

For the i3c helper, I think Jorge's current version with my
fix should work for every normal driver, and I would not
want to make it more complicated for an obscure case. The
version for the AMD driver can just be in that driver, or
it could be a separate function name in the common header
if there is a chance we'll need it again.

      Arnd

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers

[Nuno Sá]

On Thu, 2025-09-25 at 11:35 +0200, Arnd Bergmann wrote:
> On Thu, Sep 25, 2025, at 10:58, Nuno Sá wrote:
> > On Thu, 2025-09-25 at 08:47 +0000, Guntupalli, Manikanta wrote:
> > > > (AMD-Xilinx) <git@amd.com>; Simek, Michal <michal.simek@amd.com>
> > > > Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers
> > > > On Thu, 2025-09-25 at 07:37 +0000, Guntupalli, Manikanta wrote:
> > > > > > i3c@lists.infradead.org; linux-kernel@vger.kernel.org
> > > > > > Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers
> > > > > > On Wed, Sep 24, 2025 at 10:18:33PM +0200, Arnd Bergmann wrote:
> > > > 
> > > > I would argue that's something for callers of these functions to care
> > > > about.
> > > If each I3C driver has to handle FIFO endianness individually, it
> > > introduces
> > > unnecessary duplication and overhead across drivers. Centralizing this in
> > > the
> > > FIFO access helpers keeps the logic consistent, avoids repeated
> > > boilerplate,
> > > and reduces the chance of subtle bugs.
> > 
> > I mean, that's what spi and i2c drivers do already. With enum
> > i3c_fifo_endian
> > you're already forcing users to care (or know) about endianism so they might
> > as
> > well just pass the data in the proper order already (not sure if it's such a
> > big
> > 'burden').
> 
> Can you give an example of an spi or i2c driver handles a similar
> situation to the new i3c driver? As far as I can tell, swapping
> the bytes in a FIFO register is very unusual for a hardware design
> and probably a mistake rather than an intentional decision.
> 

I meant that i2c and spi drivers (and I meant on the device side) already are
the ones having to care about putting the data in the proper endianism so that
controllers don't have to care (AFAIK).

But I so see now that the above is kind of unrelated.

> On the other hand, I can find drivers that are obviously wrong
> on big-endian kernels, such as Tegra's i2c_writesl_vi() function
> being unintentionally swapped from i2c_writesl() on big-endian.
> 
> For the i3c helper, I think Jorge's current version with my
> fix should work for every normal driver, and I would not
> want to make it more complicated for an obscure case. The
> version for the AMD driver can just be in that driver, or
> it could be a separate function name in the common header
> if there is a chance we'll need it again.
> 

I do agree with the above.

- Nuno Sá

>       Arnd

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers

[Arnd Bergmann]

On Thu, Sep 25, 2025, at 09:37, Guntupalli, Manikanta wrote:
>
> This patch fixes the sub-word transfer case on big-endian kernels, but 
> it still does not address the scenario of little-endian kernels 
> accessing big-endian FIFOs.
>
> With the current version, i3c_writel_fifo() and i3c_readl_fifo() only 
> work when the FIFO has the same endianness as the CPU. On platforms 
> such as the ZCU102 (Zynq UltraScale+ MPSoC, Cortex-A53, little-endian), 
> the I3C FIFOs are big-endian, and this patch alone is not sufficient - 
> transfers fail in that configuration.
>
> We have validated this on ZCU102, and the mismatch between LE kernel 
> and BE FIFO is still an issue.

Hi Manikanta,

Thanks a lot for testing my patch and the description of
how you tested it. I think there is still a communication
problem because the term "big-endian FIFO" makes no sense
to me.

If you need an extra byteswap on a little-endian arm64 kernel,
what you have is what I would describe as a "byte-reversed FIFO",
presumably as the result of a peripheral that was initially
designed for big-endian MMIO access, but then adapted for use with
little-endian readl() helpers by adding a data swizzle in front
of each MMIO access including both the MMIO registers and the FIFO.

The result of that is that FIFO data comes out reversed in readsl(),
but does so on both little-endian and big-endian arm64 kernels,
because the hardware byte-reverse remains in place regardless of
the CPUs internal state.

If I'm interpreting this correctly, the function you'd
actually need to make the driver work on both big-endian
and little-endian (arm64) kernels would look roughly
like

static inline void i3c_writel_fifo_bytereversed(void __iomem *addr,
                         const void *buf, int nbytes)
{
        /*
         * byteswap each 32-bit word to work around FIFO quirk.
         * Note: this is different from iowrite32be(), which
         * would only swap on little-endian kernels.
         */
        while (nbytes >= 4) {
                __raw_writel(swab32p(buf), addr);
                buf += 4;
                nbytes -= 4;
        }

        if (nbytes > 0) {
                u32 tmp = 0;

                memcpy(&tmp, buf, nbytes);
                swab32s(&tmp);
                __raw_writel(addr, &tmp, 1);
        }
}

The idea here is to have a function that works the same
way as i3c_writel_fifo() but instead of never swapping the
FIFO data, it would always swap it regardless of the CPU
state, making it portable to (most of) the architectures we
support in Linux. In your version of writesl_be(), the
swap would happen on little-endian kernels but not happen
on big-endian kernels, which is inconsistent with the
documented writesl() behavior, and still makes no sense to
me conceptually. 

I think the i3c_writel_fifo_bytereversed() function would
be obscure enough that we don't need it (or the
underlying writesl_bytereversed() helper) and could
be part of your own driver as you had in the original
versions, but that is something for the i3c maintainers
to decide.

Are you able to test big-endian arm64 kernels to verify this?
Linux-next now has a patch to disallow that configuration,
but you should be able to still use v6.17 and earlier if you
have access to big-endian userspace.

     Arnd

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers

[Frank Li]

On Wed, Sep 24, 2025 at 10:18:33PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Short MMIO transfers that are not a multiple of four bytes in size need
> a special case for the final bytes, however the existing implementation
> is not endian-safe and introduces an incorrect byteswap on big-endian
> kernels.
>
> This usually does not cause problems because most systems are
> little-endian and most transfers are multiple of four bytes long, but
> still needs to be fixed to avoid the extra byteswap.
>
> Change the special case for both i3c_writel_fifo() and i3c_readl_fifo()
> to use non-byteswapping writesl() and readsl() with a single element
> instead of the byteswapping writel()/readl() that are meant for individual
> MMIO registers. As data is copied between a FIFO and a memory buffer,
> the writesl()/readsl() loops are typically based on __raw_readl()/
> __raw_writel(), resulting in the order of bytes in the FIFO to match
> the order in the buffer, regardless of the CPU endianess.
>
> The earlier versions in the dw-i3c and i3c-master-cdns had a correct
> implementation, but the generic version that was recently added broke it.
>
> Fixes: 733b439375b4 ("i3c: master: Add inline i3c_readl_fifo() and i3c_writel_fifo()")
> Cc: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This was a recent regression, the version in 6.16 still works,
> but 6.17-rc is broken.
>
> v2 changes:
>  - add code comments
>  - write correct data buffer
> ---
>  drivers/i3c/internals.h | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
> index 0d857cc68cc5..79ceaa5f5afd 100644
> --- a/drivers/i3c/internals.h
> +++ b/drivers/i3c/internals.h
> @@ -38,7 +38,11 @@ static inline void i3c_writel_fifo(void __iomem *addr, const void *buf,
>  		u32 tmp = 0;
>
>  		memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
> -		writel(tmp, addr);
> +		/*
> +		 * writesl() instead of writel() to keep FIFO
> +		 * byteorder on big-endian targets
> +		 */

endian look like how CPU decode byte order to MSB to LSB.
targets FIFO define look like

BIT 31..24   23..16     15..8     7..0
    B3        B2         B1       B0

regardless CPU is big endian or little endian system, data in memory should
be

0x000 B0
0x004 B1
0x008 B2
0x00c B3

I think your sentence in commit message is better

/* writesl() instead of writel() to keep FIFO byte orderer to match the order
in the buffer regardless of the CPU endianess.
*/

Frank
> +		writesl(addr, &tmp, 1);
>  	}
>  }
>
> @@ -55,7 +59,11 @@ static inline void i3c_readl_fifo(const void __iomem *addr, void *buf,
>  	if (nbytes & 3) {
>  		u32 tmp;
>
> -		tmp = readl(addr);
> +		/*
> +		 * readsl() instead of readl() to keep FIFO
> +		 * byteorder on big-endian targets
> +		 */
> +		readsl(addr, &tmp, 1);
>  		memcpy(buf + (nbytes & ~3), &tmp, nbytes & 3);
>  	}
>  }
> --
> 2.39.5
>
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

RE: [PATCH] [v2] i3c: fix big-endian FIFO transfers

[Guntupalli, Manikanta]

[Public]

Hi,

> -----Original Message-----
> From: Frank Li <Frank.li@nxp.com>
> Sent: Thursday, September 25, 2025 8:39 PM
> To: Arnd Bergmann <arnd@kernel.org>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>; Jorge Marques
> <jorge.marques@analog.com>; Wolfram Sang <wsa+renesas@sang-
> engineering.com>; Arnd Bergmann <arnd@arndb.de>; Guntupalli, Manikanta
> <manikanta.guntupalli@amd.com>; linux-i3c@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers
>
> On Wed, Sep 24, 2025 at 10:18:33PM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > Short MMIO transfers that are not a multiple of four bytes in size
> > need a special case for the final bytes, however the existing
> > implementation is not endian-safe and introduces an incorrect byteswap
> > on big-endian kernels.
> >
> > This usually does not cause problems because most systems are
> > little-endian and most transfers are multiple of four bytes long, but
> > still needs to be fixed to avoid the extra byteswap.
> >
> > Change the special case for both i3c_writel_fifo() and
> > i3c_readl_fifo() to use non-byteswapping writesl() and readsl() with a
> > single element instead of the byteswapping writel()/readl() that are
> > meant for individual MMIO registers. As data is copied between a FIFO
> > and a memory buffer, the writesl()/readsl() loops are typically based
> > on __raw_readl()/ __raw_writel(), resulting in the order of bytes in
> > the FIFO to match the order in the buffer, regardless of the CPU endianess.
> >
> > The earlier versions in the dw-i3c and i3c-master-cdns had a correct
> > implementation, but the generic version that was recently added broke it.
> >
> > Fixes: 733b439375b4 ("i3c: master: Add inline i3c_readl_fifo() and
> > i3c_writel_fifo()")
> > Cc: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > This was a recent regression, the version in 6.16 still works, but
> > 6.17-rc is broken.
> >
> > v2 changes:
> >  - add code comments
> >  - write correct data buffer
> > ---
> >  drivers/i3c/internals.h | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h index
> > 0d857cc68cc5..79ceaa5f5afd 100644
> > --- a/drivers/i3c/internals.h
> > +++ b/drivers/i3c/internals.h
> > @@ -38,7 +38,11 @@ static inline void i3c_writel_fifo(void __iomem *addr, const
> void *buf,
> >             u32 tmp = 0;
> >
> >             memcpy(&tmp, buf + (nbytes & ~3), nbytes & 3);
> > -           writel(tmp, addr);
> > +           /*
> > +            * writesl() instead of writel() to keep FIFO
> > +            * byteorder on big-endian targets
> > +            */
>
> endian look like how CPU decode byte order to MSB to LSB.
> targets FIFO define look like
>
> BIT 31..24   23..16     15..8     7..0
>     B3        B2         B1       B0
>
> regardless CPU is big endian or little endian system, data in memory should be
>
> 0x000 B0
> 0x004 B1
> 0x008 B2
> 0x00c B3
>
> I think your sentence in commit message is better
>
> /* writesl() instead of writel() to keep FIFO byte orderer to match the order in the
> buffer regardless of the CPU endianess.
> */
>
> Frank
> > +           writesl(addr, &tmp, 1);
> >     }
> >  }
> >
> > @@ -55,7 +59,11 @@ static inline void i3c_readl_fifo(const void __iomem *addr,
> void *buf,
> >     if (nbytes & 3) {
> >             u32 tmp;
> >
> > -           tmp = readl(addr);
> > +           /*
> > +            * readsl() instead of readl() to keep FIFO
> > +            * byteorder on big-endian targets
> > +            */
> > +           readsl(addr, &tmp, 1);
> >             memcpy(buf + (nbytes & ~3), &tmp, nbytes & 3);
> >     }
> >  }
> > --
> > 2.39.5
> >
> >
> > --
> > linux-i3c mailing list
> > linux-i3c@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-i3c

We will include this patch as part of "[PATCH V8 0/5] Add AMD I3C master controller driver and bindings", since the patch series depends on it.

Thanks,
Manikanta.

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH] [v2] i3c: fix big-endian FIFO transfers

[Alexandre Belloni]

On Wed, 24 Sep 2025 22:18:33 +0200, Arnd Bergmann wrote:
> Short MMIO transfers that are not a multiple of four bytes in size need
> a special case for the final bytes, however the existing implementation
> is not endian-safe and introduces an incorrect byteswap on big-endian
> kernels.
> 
> This usually does not cause problems because most systems are
> little-endian and most transfers are multiple of four bytes long, but
> still needs to be fixed to avoid the extra byteswap.
> 
> [...]

Applied, thanks!

[1/1] i3c: fix big-endian FIFO transfers
      https://git.kernel.org/i3c/c/d6ddd9beb1a5

Best regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c