From mboxrd@z Thu Jan 1 00:00:00 1970 From: khalasa@piap.pl (Krzysztof =?utf-8?Q?Ha=C5=82asa?=) Date: Mon, 15 Feb 2016 14:51:13 +0100 Subject: [PATCH 3/7] usb: gadget: pxa25x_udc: use readl/writel for mmio In-Reply-To: <2926427.2IOxhm2GZo@wuerfel> (Arnd Bergmann's message of "Mon, 15 Feb 2016 10:33:08 +0100") References: <1453997722-3489596-1-git-send-email-arnd@arndb.de> <2202960.HHsdV23CYi@wuerfel> <2926427.2IOxhm2GZo@wuerfel> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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? 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. > 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? -- Krzysztof Halasa Industrial Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753416AbcBONvW (ORCPT ); Mon, 15 Feb 2016 08:51:22 -0500 Received: from ni.piap.pl ([195.187.100.4]:53364 "EHLO ni.piap.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753358AbcBONvR (ORCPT ); Mon, 15 Feb 2016 08:51:17 -0500 From: khalasa@piap.pl (Krzysztof =?utf-8?Q?Ha=C5=82asa?=) To: Arnd Bergmann 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 References: <1453997722-3489596-1-git-send-email-arnd@arndb.de> <2202960.HHsdV23CYi@wuerfel> <2926427.2IOxhm2GZo@wuerfel> Date: Mon, 15 Feb 2016 14:51:13 +0100 In-Reply-To: <2926427.2IOxhm2GZo@wuerfel> (Arnd Bergmann's message of "Mon, 15 Feb 2016 10:33:08 +0100") Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-KLMS-Rule-ID: 1 X-KLMS-Message-Action: clean X-KLMS-AntiSpam-Lua-Profiles: 91350 [Feb 15 2016] X-KLMS-AntiSpam-Version: 5.5.9.33 X-KLMS-AntiSpam-Envelope-From: khalasa@piap.pl X-KLMS-AntiSpam-Rate: 0 X-KLMS-AntiSpam-Status: not_detected X-KLMS-AntiSpam-Method: none X-KLMS-AntiSpam-Moebius-Timestamps: 3963379, 3963407, 3963338 X-KLMS-AntiSpam-Info: LuaCore: 412 412 4b6b6fc5d3af82f77698bd9ab71b5a4bf748b220, Auth:dkim=none X-KLMS-AntiSpam-Interceptor-Info: scan successful X-KLMS-AntiPhishing: Clean, 2016/02/15 10:28:41 X-KLMS-AntiVirus: Kaspersky Security 8.0 for Linux Mail Server, version 8.0.1.721, bases: 2016/02/14 23:24:00 #7167992 X-KLMS-AntiVirus-Status: Clean, skipped Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? 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. > 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? -- Krzysztof Halasa Industrial Research Institute for Automation and Measurements PIAP Al. Jerozolimskie 202, 02-486 Warsaw, Poland