From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754805AbcBPL1d (ORCPT ); Tue, 16 Feb 2016 06:27:33 -0500 Received: from mout.kundenserver.de ([212.227.126.133]:56118 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754612AbcBPL1G convert rfc822-to-8bit (ORCPT ); Tue, 16 Feb 2016 06:27:06 -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 12:26:23 +0100 Message-ID: <2702068.KRp0Bplb5Q@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> <4239208.KBB6rfivoa@wuerfel> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="utf-8" X-Provags-ID: V03:K0:ccETUoplQFBrnx5qX+M279jZU0KtsNHQ666Uq6O0duR53++Vw+M WOPX50eWYXARzBiIOntf0Yc4Nq+vQtEKOyOTw60K2NW9Qcz61biM6boLfAaEFapjGr+4tC8 Tcg/RiSrVNyc0N83IND/2JVIqeOKzVU92XAPz7JoUIAPPpwpW0JtSPjRDl8sU2DkN1GRbIl U7fnAx+9hRylegJYkd1VA== X-UI-Out-Filterresults: notjunk:1;V01:K0:rSZPafdzN3M=:h4+1taASe8Oav9f7YUrDi6 7Lxv8lV8alRejrY37yQAGR+A52kAtc5eC1BHqBmvHMhs96cKTtvG+g/7hjAghwaNL2ZPxdqNK 7tWaP8AXxf/d5sGMTpKaIZ795ZL8p0BmJCpaU0/ZAIg83WlCrgo6ZAaCNt6O9bwQh4FPOfZ2X EHOhFHEFmKglKH2OvO2iCYZvdTeI6OlxczBCR5CIpU9p/Z0EQbXT8LPLUYzA9UWGorVZSqQcq EOHQME3YU4r4qdakIXysOf27otT6g0h/xST5D22zdwqiS/bHOynjbvBM2TrCu5/DWKp+GQrwa MttJ5Ggy6dZ34+90gMl0LTKKDuBm+dQYhkrOLvFhEpEqApTrh+pjXFPwTjL4f0HfWR7fhKC7u C6i0bgHKFQBSYFNEAHbYzbdev1NRd6Hjo+r2Z3sYkIOUyTHpjCWMEs1/41Mt4eil6Rtn/fa1l 3A//xTR+RZc6th/cN9DcCJ7np2E8AwTSE3UPY1wn4cfjxEkuGSzOsbbQNv0FTZxzwrrj76loO QZLu6MNW8VfUpU/6iLc9VlWM8T2IO2XoLp40OfbvX4EBGceP8yT25vW3MbqtgtCnvoRguS7vs 3gOUUhz3ky8KSLwFJ6MT/I10idzrxFWDWEnOwU7SbF52MdLGDPj/AZ/Cdwwror/hxGNe6rZyq l7hKkwGr+z7jGgsm+Cplw4SdiG5d+wiatlyKd5PZ6IDC7+NgaQXzFLSzYArw/ChDl3x3XCXx9 8jRAebY/4Wkz2qHy Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 16 February 2016 10:26:14 Krzysztof Hałasa wrote: > Arnd Bergmann writes: > > > 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. > > I still don't get it. If the spinlocks synchronize between CPUs, there > can only be one CPU (or core) doing the pair of raw_writel(), so how > would it be possible to not get the address/data pair written out? > IOW, how is it different from a system with a single CPU? 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. > > On powerpc, we have in_le32/in_be32 for SoC-internal register access, > > while only PCI devices are allowed to be accessed using readl(). > > Yeah, this seems like a sane solution. > > > 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. > > Hmm. This is actually the opposite - while there may be some magic > (swapping) in readl() and friends, there is absolutely no magic in the > __raw_readl() etc. They are essentially equivalent to > *(volatile u32 *)ptr. This is constant and doesn't depend on endianess, > PCI, anything. 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(). 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. Arnd