From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752667AbcBOJdz (ORCPT ); Mon, 15 Feb 2016 04:33:55 -0500 Received: from mout.kundenserver.de ([217.72.192.74]:61424 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752241AbcBOJdu convert rfc822-to-8bit (ORCPT ); Mon, 15 Feb 2016 04:33:50 -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 10:33:08 +0100 Message-ID: <2926427.2IOxhm2GZo@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> <2202960.HHsdV23CYi@wuerfel> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="utf-8" X-Provags-ID: V03:K0:4jVgux7DaajbtGEpWmKZb2JiQVtGSt42qMC6gsUq6mlenVdh8FV gZwYO5H8pRwXHWWJTGtkV+SzjQrPuPyMd+OXem7ujsDc99B5WtgYY8LgeVd8FhmOQQRBiUx f6/uax/E7d+tpdbek+0wqsTyFCQrwh4VGD3Eaa2fHKTEM4EQg+YtqUL5OimK8TkTnO4nFqn RT197HJUSSq0+IMF5paIA== X-UI-Out-Filterresults: notjunk:1;V01:K0:dSSi/0b6yA0=:hM4Rbf+caWmJ9+zPYLJKcs TycQ1l6L6kKmEKyZplhHGMj3L8waflTe1fEEtH9DNW8ixb1JPaypHKa3FO+fxx5sWKubdcal+ wQ764ykUsqMFtWFrXw1+SnjwlSwfFO2cUXW//f1IiBV7tp/psoZd3MouY/u61FcdxI2OmV5j2 L+yTUg+m8jq7Mc5L8qI/psPH0g98xJrumH9vj3TcL5cqbbVlSM2ecjHSdEMSPKjUWjmI51XAq ScjPtMXpg+pb+mXfdFkTyY8Gby1d+fYQGSPmiQGL2Bt4eNTO8fSokQmf0uuLWrPy1hqYjAvX4 jyNuvP7RmDfS3asO0BKU5RwnyjL1TG6Shobhu3KtDNLxBFIjlP9PtcrSMN9wq38ddc49u4N3r WZPE/U0tRWeG3OnykXCBNZzWQoPNePnufvHnp+d4dYiXcW8kmAX8RJi5xfXwSzAsSI2Q7sgBV EpvEfs+eVHwFnqivf4+gSlFV2MaMtOapvak/Nci9RD3JjxCJUG6K6kuRf7W8CYaw3CbcPAlt/ TJzOfK6FbA+c9RZx3SVpNHshh0gZzDuhDUxq17MQnRGG1x4yZrxTT+ai7R2Z8pDHcyJHL/uOx tvDupsM5xztjVNUiazpp5+kxUCxSgv7IjP1QzKZscM5ieJ0F1Oo8CGLU9PHTVX+dc5j8piiCs 20ghvqYHndqAVywCsWAxQ2ZJs7EUPjtIDX1E7/aXZc13lwbRcJ7khiwUJh7ndzo0caNN66UYX 8akdoMycsFi18GlP Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday 15 February 2016 08:33:37 Krzysztof Hałasa wrote: > Arnd Bergmann writes: > > >> 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. > > Well, it is possible. I recall I probably used __raw_* instead of > readl()/writel() in qmgr/npe/Ethernet drivers for this very reason. > > I think Direct pointer references are only used for locations in RAM > (DMA buffers), they have their own share of problems because the > peripherals are always big endian. > > I think I still have an early IXP425 board with UDC connector so I will > try to dig it up and test this code. > > I wonder if we should switch the driver to use __raw_*, too. The problem > is almost nobody uses UDC with this CPU. > 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). 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. Arnd