[Public] Hi,
> -----Original Message----- > From: Arnd Bergmann <[email protected]> > Sent: Wednesday, September 24, 2025 12:08 AM > To: Guntupalli, Manikanta <[email protected]>; git (AMD-Xilinx) > <[email protected]>; Simek, Michal <[email protected]>; Alexandre Belloni > <[email protected]>; Frank Li <[email protected]>; Rob Herring > <[email protected]>; [email protected]; Conor Dooley <[email protected]>; > Przemysław Gaj <[email protected]>; Wolfram Sang <wsa+renesas@sang- > engineering.com>; [email protected]; > [email protected]; S-k, Shyam-sundar <[email protected]>; > Sakari Ailus <[email protected]>; '[email protected]' > <[email protected]>; Kees Cook <[email protected]>; Gustavo A. R. Silva > <[email protected]>; Jarkko Nikula <[email protected]>; Jorge > Marques <[email protected]>; [email protected]; > [email protected]; [email protected]; Linux-Arch <linux- > [email protected]>; [email protected] > Cc: Pandey, Radhey Shyam <[email protected]>; Goud, Srinivas > <[email protected]>; Datta, Shubhrajyoti <[email protected]>; > [email protected] > Subject: Re: [PATCH V7 2/4] asm-generic/io.h: Add big-endian MMIO accessors > > On Tue, Sep 23, 2025, at 17:45, Manikanta Guntupalli wrote: > > Add MMIO accessors to support big-endian memory operations. These > > helpers include {read, write}{w, l, q}_be() and {read, write}s{w, l, > > q}_be(), which allows to access big-endian memory regions while > > returning the results in the CPU’s native endianness. > > > > This provides a consistent interface to interact with hardware using > > big-endian register layouts. > > > > Signed-off-by: Manikanta Guntupalli <[email protected]> > > I feel like we already have too many accessor functions like these, what's > wrong with > just using io{read,write}{8,16,32,64}be() in your driver? > > On most architectures (including arm, riscv, powerpc and microblaze, but not > x86), > the ioread/write helpers are identical to the readl/writel style helpers, the > only > difference being that on x86 they add an extra indirection for the port I/O > check. > > At the moment, there are only six drivers that use the > io{read,write}{8,16,32,64}be() style helpers. They are all powerpc specific > and can > probably be changed to io{read,write}be. The existing helpers io{read,write}{8,16,32,64}be() internally rely on {read,write}{b,w,l,q}(). From both description and implementation, {read,write}{b,w,l,q}() access little-endian memory and return the result in native endianness: /* * {read,write}{b,w,l,q}() access little endian memory * and return result in native endianness. */ static inline u32 readl(const volatile void __iomem *addr) { u32 val; log_read_mmio(32, addr, _THIS_IP_, _RET_IP_); __io_br(); val = __le32_to_cpu((__le32 __force)__raw_readl(addr)); __io_ar(val); log_post_read_mmio(val, 32, addr, _THIS_IP_, _RET_IP_); return val; } The io{read,write}*be() helpers then perform a byte swap, e.g.: static inline u32 ioread32be(const volatile void __iomem *addr) { return swab32(readl(addr)); } This works on little-endian CPUs, but on big-endian CPUs the {read,write}{b,w,l,q}() helpers already return data in big-endian format. The extra byte swap in io{read,write}*be() therefore corrupts the data, producing little-endian values when big-endian is expected. The newly introduced {read,write}{w,l,q}_be() helpers directly access big-endian IO memory and return results in native endianness, avoiding this mismatch. Thanks, Manikanta.
