From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752415AbcBOQNj (ORCPT ); Mon, 15 Feb 2016 11:13:39 -0500 Received: from mout.kundenserver.de ([217.72.192.75]:51691 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751310AbcBOQNh convert rfc822-to-8bit (ORCPT ); Mon, 15 Feb 2016 11:13:37 -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: Mon, 15 Feb 2016 17:12:54 +0100 Message-ID: <4239208.KBB6rfivoa@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> <2926427.2IOxhm2GZo@wuerfel> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="utf-8" X-Provags-ID: V03:K0:RCJp4yerrtR1jqr72jmV6/x8O0Bf7nYQDrbNhMWmyItnrN69OUi pXXJiwjxGi1GNnhDQ6AjNh/iS4SDrmWuMKsI3cnHWFSxNBvpJP4cv8M8DyHUB3sEtp2w/a5 XmjI8T8wbhdQE/0rhQk3vRpaw7Ip+aOG8S9bdMAe0+Z3RY33e797/92UfZTgFWULqZJeNRX ++FN42N/ydFGTFiArUzkg== X-UI-Out-Filterresults: notjunk:1;V01:K0:ilfluRB8wAY=:mapmIHdsVgBxakX7PZVCOP s57NkPqPtzMtvmoBhEQEQNJPsYDAmHmhOhJcx1wjPGl+EFRb27c4WVoOQ4dF0JpRsCW8t7skc PX2taKo8FV/Ow1iJIbYtuKC7bnWoRwOYcBJA5yNZZbNi3DaMtfLg/nYrcH5q9+HUjV0UvJ6oO myyl76jUfHz4UyJE/VNUA7Cfr+Q0JPrsKFdMo2ro6j6SJcWFI479yUufV9qrXf8LiodR1lTtp FjwyzYHN43ti1otzEZDGVyfvNWmipzX3sByhWaOghHHjNyll9SXSzqg8LxqtzW0RqtyOpvfYh Wxm7DaCjeXHu+Qmt+0npmH+U7/qKS/VReseegTjdM5NSK/ekGMA+oLA42D0iTq95DXY5B0P0s PH94a+nwxbRAwmhMG4nV+EhnQR9ZwJfxbI381QtpH4A3gUFQF983eyY5ogYPxNqdiTIxyzt/o QKhAJQ5kWdbu+ofnNgtdy2ZM8PpgLMCJWmVV0b1RLv/SYKH3M31GOoN0OmzL2RUhVvSMugv8b Z94BHEeumrPQY2WjP0CVFCIX7l7dPdKgdUgI+QIRz8cIdRaqxcRUXwix0JyhliWzaHdfv2VHp kUegLE5PgAd7NESQIB9VETjwhzZZ261zd2ENXjgDNabh2mMI0bBa45mgEPLFDn1AaqrWSZS8p gpU1IJSlaVt9f+xm+wkJxoDmCDZALMKqZc1aD1ermpYoraNmy5Qz4UayDe14BQA7E7LNRMZmp R0ePCgx+eELVSnqw Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday 15 February 2016 14:51:13 Krzysztof Hałasa wrote: > Arnd Bergmann writes: > > > I consider the use of __raw_* accessors a bug, I don't think we should > > ever do that because it hides how the hardware actually works, it doesn't > > work with spinlocks, and it can lead to the compiler splitting up accesses > > into byte sized ones (not on ARM with the current definition, but > > possible in general). > > Well, then maybe we should fix them, or add another set. > Why don't they work with spinlocks? The barriers on a spinlock synchronize between CPUs but not an external bus, so (on some architectures) a spinlock protecting an MMIO register does not guarantee that two CPUs doing spin_lock(); __raw_writel(address); __raw_writel(data); spin_unlock(); would cause pairs of address/data to be seen on the bus. Of course this is meaningless on ixp4xx, as there is only one CPU. > To be honest, I remember this was already discussed a bit years ago. > I think I proposed back then a set of read_le32 (which would be > equivalent of current readl(), and could be named pci_readl() as well), > read_be32, read_host (without swapping). > The names could be better, though. It keeps coming back, but so far the status quo has been stronger, any it generally works for portable code either hardcoding whichever endianess the device has, or using runtime detection when the same device can be used in various ways (e.g. USB ehci). On powerpc, we have in_le32/in_be32 for SoC-internal register access, while only PCI devices are allowed to be accessed using readl(). > > Almost all hardware is fixed-endian, so you have to use swapping accessors > > when the CPU is the other way, except for device RAM and FIFO registers > > that are always used to transfer a byte stream (see the definition of > > readsl() and memcpy_fromio()). When you have hardware that adds byteswaps > > on the bus interface, you typically end up with MMIO registers requiring > > no swap (or double swap) and readsl()/memcpy_fromio()) suddenly requiring > > a swap that is counterintuitive. > > Sure, but the __raw_* are used just to be sure there is absolutely no > swapping. > E.g. for IXP4xx, the registers never require swapping, thus readl() etc. > are not suitable for this. What is needed here is simple "atomic" 32-bit > straight to/from register (MM)IO (assuming 4-byte address alignment). > > If not __raw_* then what? I would suggest using an ixp4xx specific set of accessors that comes down to either readl() or ioread32_be(), depending on whether CONFIG_CPU_BIG_ENDIAN is set. That makes it clear that there is a magic bus involved and that it works on this platform but not in portable code. Arnd