From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932718AbcBPN4e (ORCPT ); Tue, 16 Feb 2016 08:56:34 -0500 Received: from mout.kundenserver.de ([212.227.126.130]:57187 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932539AbcBPN4b convert rfc822-to-8bit (ORCPT ); Tue, 16 Feb 2016 08:56:31 -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: Tue, 16 Feb 2016 14:55:43 +0100 Message-ID: <3062977.Ya2ztQYFaM@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> <2702068.KRp0Bplb5Q@wuerfel> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="utf-8" X-Provags-ID: V03:K0:EwVv5x2DeuipyX6Qo6DvyXiycDK0bqSjmGLtegESauTybr66oiU eYCYl6GF57UbaZeRjL94dinKip9S1EbYCXygjTyjFUu8n0qA4LNxOGUiiBujJEJvDiVZjx2 obugEvDbi5ZiK/PgkTOXFgmYFtzPks5eMVHRmLyf6RD+EiFe0tZBu3CTYaIiY72TsN2Csvu 3fH9N/wuywIv/McTmbg0g== X-UI-Out-Filterresults: notjunk:1;V01:K0:s1S97EwBTlo=:avR6tkuKy/EcX3tn3UaStN fi9t9jGtsIccxS9HFQ1OAmy6/2ltT7gqOP0s4RF/qkWwVl/unb4Y9+dAcRul9Ea/1+4O0qoTV AZpi6sMF64WYcnm/8aT6nC82aYOrMbpcFq7P99bmD5rCO04/vN2DVVkotTXTmPJfcpt8Km2G2 gybh7vswCiNVuNNeIaWVv09IO7tQhbPYz48EiX60C/nDBWhHy7pWO0Y3P+LaRhLerRn4xDJwA h3ZPptEQWA34eFvl4AarcpvYWajnwMzpyPe7F3cIGir92Nik1ErlaMVMvnqeIu8xyR+WgFSNx 9Tdo4P0DHMsimRWW4G8g0C221kPVwao01s38vOXy75Ds3eIaMNAUm+p3jVRjhbpwp4yb7fm69 tt+gI/G4cAQa6R2OVqt0Lt8KGSUEL/VUYUufqNutoZmo5drnzGdL7s894GsIr1ULVRVdSrP+l s+YM5qXvgJJ0znqkBLo3j78liYsQIpqLQt3dwd107uhV+nKgTgmGFlfgp7MDnHF2JR0a57/yS NmnV3CJUsGcdII/PB4ToUZtzfJybvcXaVjjqL1fqEGNU/FCF5TLUFOsQ44AMTs+OzPm4gB4Mp I+YoyYLSuURzbg5HZotYixEUAz+aNDcW+SLz62kc+JRdS8JfH5QN5RurxrbkUi5NODCw5mj+R MWTb3Dr7DmrCH48NaiBxtZIUE0jcqaexO8fRl38lwm3t2gEexQw8B6yaoMulUWx0LLHL7DNSA PQwaZoW9b+9w/gVV Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 16 February 2016 14:24:10 Krzysztof Hałasa wrote: > Arnd Bergmann writes: > > > Both writes leave the CPU core within the spinlock but are not serialized > > with anything else, so there is no ordering between the CPUs when they > > enter the shared bus, other than having address before data. You'd > > expect to see address0, data0, address1, data1, but it could also > > be e.g. address0, address1, data1, data0. > > Ah, so it's a matter of flushing the write buffers before exiting the > spinlock-protected code. Yes, whatever the specific architecture requires. The normal readl() functions are documented to having all the required barriers and flushes, the __raw_* variants may (e.g. on x86) or may not have that. > > The point is more what the common case is. Almost all machines we support > > have fixed-endian devices, and the drivers are correct when using readl() > > or in_le32() but are not endian-safe when using __raw_readl(). > > Sure, e.g. PCI does it this way (eventually swapping the data lanes if > needed). > > > Using __raw_readl() has the big danger of someone accidentally "fixing" > > the driver to work like all the others in order to solve a theoretical > > endian problem, while it should really be made obvious that the hardware > > actively swaps its data on the bus. > > Sure - if this is the case. On-chip IXP4xx peripherals don't swap data > at all (i.e., they match CPU endianess) - accessing their registers is > like accessing a normal CPU register. That's why they don't use > PCI-style readl() etc. - however a better name than __raw_* would > probably help here. > > Using __raw_* in a PCI driver would be generally wrong. I was really talking about built-in devices here. There are other platforms that have an internal byteswap logic on the bus interface and they also use that for PCI, see e.g. arch/sh/include/mach-common/mach/mangle-port.h. ixp4xx is really special in that it performs hardware swapping for internal devices based on CPU endianess but not on PCI devices. I think some Broadcom MIPS systems are in the same category as IXP4xx, but only because they have an extra byteswap on the PCI bus that they can turn on, but very few other systems are. 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. Arnd