From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932667AbcA2RHn (ORCPT ); Fri, 29 Jan 2016 12:07:43 -0500 Received: from mout.kundenserver.de ([212.227.126.131]:59861 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756426AbcA2RHl convert rfc822-to-8bit (ORCPT ); Fri, 29 Jan 2016 12:07:41 -0500 From: Arnd Bergmann To: linux-arm-kernel@lists.infradead.org Cc: Krzysztof =?utf-8?B?SGHFgmFzYQ==?= , 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: Fri, 29 Jan 2016 18:06:58 +0100 Message-ID: <2202960.HHsdV23CYi@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> <1453997843-3489728-3-git-send-email-arnd@arndb.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="utf-8" X-Provags-ID: V03:K0:c04luTPHXegtJ69W/DKYEEh+oDePhpie2lThPizrOq2KubPTbg7 UeJB3c9U8l6SGExylFT+oDHmiHMembQkmKcyTJetPNG6HMWPFIFpAmiXnNHf2FyC6VVBE6E 0pgpNKmD06Kc/m0y7258ocPPseQAyGV2OTQjHm6wxdk5ARaAt0qRepKBRyNtxPLMmzcECw3 yhdyqbeAxMbbI9FcILTmg== X-UI-Out-Filterresults: notjunk:1;V01:K0:XX0a4LinM5U=:FIDZSEoTBHF3KBrb1t3lxo yCxvQSuMBVpNkhp/V+F7lnWMOKVyQcWLugr2nkuwklvhEQNc+52QEm9ItXc3o55xs/2VMq/pp Zs5MHyjdNlk1NntGU6blr7S8NpxxmvP4+X9FByZdLyYga+c8EZpjfxxAlvTH/wiagfdMUMuCG CVmJI2pLP0aX2MSdFG0hoNKg2jqXMFJuUVeTMsym2D0Moc7w4De1TI1LMYfZd/DOhzsPvzT/9 UPFQNaYO2rO9DZ4yJUxdKec1mj5bN1tJEntDJFLx0m3tm2KPpDLzD4SEZwjcMsQLGpEQc6nU7 Egi0LVw1O0iG/en8h9WUiHDivDWwfNIiPETvWgUliHEUHQhCMrtCy3R9KNFTFQSkvlPgTMxfU nn/AQClvHGVmV6AwNEvCWgAHqbk49DLyagMKeOhaSOhBPHOL/OlyqWSdhEQmJZvpzjkoeQSNV 63mN+KzUZV30UPMQnSPWArR5182CYl3o6hnjBnQ9RHPwfqgu/fICf7FvXqO2OidbczHt9/lF3 PdfWaj1prf8NhOvcBWDayEDzQq9VFxhGyhDCR7JFP4lrCioXwRFwXpAQ2Ggf9ETfPpRiR0jfI u3UqIDNHkSXc/cbpQOWL5qD6+hIQsFkBjKg8oVw3RCzKjV1hRtBZ/58gQvjpt3AsUvFzg6r0+ iGbwJ0wFo1TAlEaHlj7LpQx5dr/jxJFKvu+EP1KzdVnpmAn/B6Z+Z1HMTPWoeSzW4KHhP3fwA GjTjiyiGw2iBgonR Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday 29 January 2016 17:18:09 Krzysztof Hałasa wrote: > Arnd Bergmann writes: > > > The unclear part here is for IXP4xx, which supports both big-endian > > and little-endian configurations. So far, the driver has done > > no byteswap in either case. I suspect that is wrong and it would > > actually need to swap in one or the other case, but I don't know > > which. > > If at all, I guess it should swap in LE mode. But it's far from certain. > > > It's also possible that there is some magic setting in > > the chip that makes the endianess of the MMIO register match the > > CPU, and in that case, the code actually does the right thing > > for all configurations, both before and after this patch. > > This is IMHO most probable. > > Actually, the IXP4xx is "natural" in BE mode (except for PCI) and > normally in LE mode it's order-coherent, meaning 32-bit "integer" > accesses need no swapping, but 8-bit and (mostly unused) 16-bit > transfers need swapping. I see. > Anyway, I think readl()/writel() do the right thing: in BE mode they > swap PCI accesses and don't swap normal registers, in LE mode nothing is > swapped. This seems to be true when CONFIG_IXP4XX_INDIRECT_PCI is set, but not otherwise. For the indirect variant, writel() is a __raw_writel() without barrier or byteswap on non-PCI memory, while with that option disabled, we use the standard implementation that has both a byteswap and a barrier. According to your description, that would mean the version without indirect PCI access is broken and it appears to have been that way since before the start of git history in 2.6.12. It's possible that nobody cared because all drivers for non-PCI devices on ixp4xx (the on chip ones) just use __raw_readl or direct pointer references. > LE data-coherent mode (which has never landed in the > official kernel) is a bit different, but still, readl()/writel() do the > right thing. > I remember the "string" (block) functions preserve 8-bit ordering, and > thus 32-bit values transfered using them may need swapping. This is the normal behavior, yes: readsl/writesl should not swap data, and you should not use them to access registers other than FIFOs that contain byte-sequential data. If the bus swaps data, then the implementation of the readsl/writesl function has to swap it back, as the indirect versions of these do. Arnd