From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161123AbcBQKha (ORCPT ); Wed, 17 Feb 2016 05:37:30 -0500 Received: from mout.kundenserver.de ([217.72.192.75]:55656 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030251AbcBQKh1 convert rfc822-to-8bit (ORCPT ); Wed, 17 Feb 2016 05:37:27 -0500 From: Arnd Bergmann To: Krzysztof =?utf-8?B?SGHFgmFzYQ==?= Cc: linux-arm-kernel@lists.infradead.org, Felipe Balbi , Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Felipe Balbi , Haojian Zhuang , Daniel Mack , Imre Kaloz , Robert Jarzmik Subject: Re: [PATCH 3/7] usb: gadget: pxa25x_udc: use readl/writel for mmio Date: Wed, 17 Feb 2016 11:36:49 +0100 Message-ID: <2926671.iEYNiy920m@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: References: <1453997722-3489596-1-git-send-email-arnd@arndb.de> <3062977.Ya2ztQYFaM@wuerfel> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="utf-8" X-Provags-ID: V03:K0:Np69mC1qnpEiaFru234YOBMRpU/gbbwdUNxRAIJn7AIGuYOYFBf d+RR0MJ7DbFMmo+kERYz1peKVOdoHD+EpWPDcH3Wzx6U0glnXEw0hrXh+JD77mfk0v/wxdJ 8PBbIHqdiPT4jcPApPjDNycnWSb3QGk3TAiv08YuX5bHH71YTGz9IX9exH7gRz0LdI/nhRs R+Hob/QvdBmK1uYZygAug== X-UI-Out-Filterresults: notjunk:1;V01:K0:Ww/9SIn8meg=:P0YViYlOV61ur5DRLsO8cR VbGucyagZI4cnFvQfCyc9fClJkCf55BIFv/wswaLJmT/+8/gCj4U43g1qHBDpUd8ve3aOzEfX 4IIE46JNMNvzgAHVaOM1Y5rz8UxXaB7TGm1QYt2NEXk3ojG8MHFZKouX8sFBzlSMD0quw2k/y uF2f1JoEjiGCu2f0HqoOqcIUtB7bkaSZ2GScApb1A9erCAFu/dXuiL9nfo/YVidStcabz+gnJ n8tJvcApJUHdwU1eNlRmLu+CsomE6qLl7To5SWzSgncdOPVVbt6yr2lmBCah2EQtQlP+JBY6C dLbidjEXI+TfPDMaCgvv9NbEqx8dr/hYZZJqF1ngh3py8Zay8ZevD+mlsgfvSJhot4WRn4sd3 eMzM3wJVrdbuinTmfAKcWSBKz8dSdMBjrjm6RAs7Ic6Y/1p0y745brSOzJk3bcyfVHZ6fUawe 0z4xJ0ErZx3mX29oZhSFrPxsgsjybiy4Kt55ntMkva3ufRr1ExbpmjbihKZLuulMpkQ0eRh98 NrU+/uHpMYNbUgijMOykj6uzWz6cJ5SS/0P7N2XMl7Mr0efrIw7qHSRpa2L2I+1fHvAEKereW dEdgOduZ4Ypr3SC2fT6RD3+qjYEDSFWsW3PqT75YZo4uoW7H4YSPhSbnpmgfII5ezUaKwKI/b kw7kOYQrHRbGzXI/aeJzEPfZDQyYzOLojCs/iidwT+c0F0Tv1+xaH4nQUdHvXeMHvAVuWgO9G w9uQT+dqnJbWfu7X Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday 17 February 2016 09:36:37 Krzysztof Hałasa wrote: > Arnd Bergmann writes: > > > ixp4xx is really special in that it performs hardware swapping for > > internal devices based on CPU endianess but not on PCI devices. > > Again, IXP4xx does not perform hardware (nor any other) swapping for > registers of on-chip devices. The registers are connected 1:1, > bit 0 to bit 0 etc. > > (Yes, IXP4xx can be optionally programmed for such swapping, depending > on silicon revision, but it is not used in mainline kernel). > > The only hardware swapping happens on PCI bus (in BE mode), to be > compatible with other platforms and non-IXP4xx-specific PCI drivers. Ok, so I guess what this means is that ixp4xx (or xscale in general) implements its big-endian mode by adding a byteswap on its DRAM and PCI interfaces in be32 mode, rather than by changing the behavior of the load/store operations (as be8 mode does) or by having the byteswap in its load/store pipeline or the top-level AHB bridge? It's a bit surprising but it sounds plausible and explains most of the code we see. I'm still unsure about __indirect_readsl()/ioread32_rep()/insl()/readsl(). insl() does a double-swap on big-endian, which seems right, as we end up with four swaps total, preserving correct byte order. __raw_readsl() performs no swap, which would be correct for PCI (same swap on PCI and RAM, so byteorder is preserved), but wrong for on-chip FIFO registers (one swap on RAM, no swap on MMIO). This is probably fine as well, as I don't see any use of __raw_readsl() on non-PCI devices. However, when CONFIG_IXP4XX_INDIRECT_PCI is set, both ioread32_rep() and readsl() call __indirect_readsl(), which in turn swaps the data once, so I think we actually need this patch: diff --git a/arch/arm/mach-ixp4xx/include/mach/io.h b/arch/arm/mach-ixp4xx/include/mach/io.h index e770858b490a..871f92f3504a 100644 --- a/arch/arm/mach-ixp4xx/include/mach/io.h +++ b/arch/arm/mach-ixp4xx/include/mach/io.h @@ -85,6 +85,8 @@ u8 __indirect_readb(const volatile void __iomem *p); u16 __indirect_readw(const volatile void __iomem *p); u32 __indirect_readl(const volatile void __iomem *p); +/* string functions may need to swap data back to revert the byte swap on + big-endian __indirect_{read,write}{w,l}() accesses */ static inline void __indirect_writesb(volatile void __iomem *bus_addr, const void *p, int count) { @@ -100,7 +102,7 @@ static inline void __indirect_writesw(volatile void __iomem *bus_addr, const u16 *vaddr = p; while (count--) - writew(*vaddr++, bus_addr); + writew((u16 __force)cpu_to_le32(*vaddr++), bus_addr); } static inline void __indirect_writesl(volatile void __iomem *bus_addr, @@ -108,7 +110,7 @@ static inline void __indirect_writesl(volatile void __iomem *bus_addr, { const u32 *vaddr = p; while (count--) - writel(*vaddr++, bus_addr); + writel((u32 __force)cpu_to_le32(*vaddr++), bus_addr); } static inline void __indirect_readsb(const volatile void __iomem *bus_addr, @@ -126,7 +128,7 @@ static inline void __indirect_readsw(const volatile void __iomem *bus_addr, u16 *vaddr = p; while (count--) - *vaddr++ = readw(bus_addr); + *vaddr++ = (u16 __force)cpu_to_le16(readw(bus_addr)); } static inline void __indirect_readsl(const volatile void __iomem *bus_addr, @@ -135,7 +137,7 @@ static inline void __indirect_readsl(const volatile void __iomem *bus_addr, u32 *vaddr = p; while (count--) - *vaddr++ = readl(bus_addr); + *vaddr++ = (u32 __force)cpu_to_le32(readl(bus_addr)); } Does that make sense to you? This is essentially the same thing we already do for inw/inl/outw/outl. > > Coming back to the specific pxa25x_udc case: using __raw_* accessors > > in the driver would possibly end up breaking the PXA25x machines in > > the (very unlikely) case that someone wants to make it work with > > big-endian kernels, assuming it does not have the same hardware > > byteswap logic as ixp4xx. > > I'd expect both CPUs to behave in exactly the same manner, i.e., to > not swap anything on the internal bus. If true, it would mean it should > "just work" in both BE and LE modes (including BE mode on PXA, should > it be actually possible). Ok, I'll change the patch accordingly and resubmit. Arnd