All of lore.kernel.org
 help / color / mirror / Atom feed
From: khalasa@piap.pl (Krzysztof Hałasa)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/7] usb: gadget: pxa25x_udc: use readl/writel for mmio
Date: Tue, 16 Feb 2016 14:24:10 +0100	[thread overview]
Message-ID: <m34md8rdol.fsf@t19.piap.pl> (raw)
In-Reply-To: <2702068.KRp0Bplb5Q@wuerfel> (Arnd Bergmann's message of "Tue, 16 Feb 2016 12:26:23 +0100")

Arnd Bergmann <arnd@arndb.de> 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.

> 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.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

WARNING: multiple messages have this Message-ID (diff)
From: khalasa@piap.pl (Krzysztof Hałasa)
To: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arm-kernel@lists.infradead.org,
	Felipe Balbi <balbi@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	Felipe Balbi <balbi@ti.com>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	Daniel Mack <daniel@zonque.org>, Imre Kaloz <kaloz@openwrt.org>,
	Robert Jarzmik <robert.jarzmik@free.fr>
Subject: Re: [PATCH 3/7] usb: gadget: pxa25x_udc: use readl/writel for mmio
Date: Tue, 16 Feb 2016 14:24:10 +0100	[thread overview]
Message-ID: <m34md8rdol.fsf@t19.piap.pl> (raw)
In-Reply-To: <2702068.KRp0Bplb5Q@wuerfel> (Arnd Bergmann's message of "Tue, 16 Feb 2016 12:26:23 +0100")

Arnd Bergmann <arnd@arndb.de> 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.

> 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.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

  reply	other threads:[~2016-02-16 13:24 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-28 16:15 [PATCH 0/7] USB changes for rare warnings Arnd Bergmann
2016-01-28 16:15 ` Arnd Bergmann
2016-01-28 16:17 ` [PATCH 1/7] usb: gadget: pxa25x_udc: move register definitions from arch Arnd Bergmann
2016-01-28 16:17   ` Arnd Bergmann
2016-01-28 16:17   ` [PATCH 2/7] usb: gadget: pxa25x_udc cleanup Arnd Bergmann
2016-01-28 16:17     ` Arnd Bergmann
2016-01-29 10:13     ` Robert Jarzmik
2016-01-29 10:13       ` Robert Jarzmik
2016-01-28 16:17   ` [PATCH 3/7] usb: gadget: pxa25x_udc: use readl/writel for mmio Arnd Bergmann
2016-01-28 16:17     ` Arnd Bergmann
2016-01-29 10:17     ` Robert Jarzmik
2016-01-29 10:17       ` Robert Jarzmik
2016-01-29 16:18     ` Krzysztof Hałasa
2016-01-29 16:18       ` Krzysztof Hałasa
2016-01-29 17:06       ` Arnd Bergmann
2016-01-29 17:06         ` Arnd Bergmann
2016-02-15  7:33         ` Krzysztof Hałasa
2016-02-15  7:33           ` Krzysztof Hałasa
2016-02-15  9:33           ` Arnd Bergmann
2016-02-15  9:33             ` Arnd Bergmann
2016-02-15 13:51             ` Krzysztof Hałasa
2016-02-15 13:51               ` Krzysztof Hałasa
2016-02-15 16:12               ` Arnd Bergmann
2016-02-15 16:12                 ` Arnd Bergmann
2016-02-16  9:26                 ` Krzysztof Hałasa
2016-02-16  9:26                   ` Krzysztof Hałasa
2016-02-16 11:26                   ` Arnd Bergmann
2016-02-16 11:26                     ` Arnd Bergmann
2016-02-16 13:24                     ` Krzysztof Hałasa [this message]
2016-02-16 13:24                       ` Krzysztof Hałasa
2016-02-16 13:55                       ` Arnd Bergmann
2016-02-16 13:55                         ` Arnd Bergmann
2016-02-17  8:36                         ` Krzysztof Hałasa
2016-02-17  8:36                           ` Krzysztof Hałasa
2016-02-17 10:36                           ` Arnd Bergmann
2016-02-17 10:36                             ` Arnd Bergmann
2016-02-17 16:14                             ` Krzysztof Hałasa
2016-02-17 16:14                               ` Krzysztof Hałasa
2016-02-20 20:54                         ` Robert Jarzmik
2016-02-20 20:54                           ` Robert Jarzmik
2016-01-29 18:03       ` Sergei Shtylyov
2016-01-29 18:03         ` Sergei Shtylyov
2016-01-29 21:02         ` Arnd Bergmann
2016-01-29 21:02           ` Arnd Bergmann
2016-01-29  9:32   ` [PATCH 1/7] usb: gadget: pxa25x_udc: move register definitions from arch Robert Jarzmik
2016-01-29  9:32     ` Robert Jarzmik
2016-01-29 10:07     ` Arnd Bergmann
2016-01-29 10:07       ` Arnd Bergmann
2016-01-29 15:26       ` Robert Jarzmik
2016-01-29 15:26         ` Robert Jarzmik
2016-01-29 15:55   ` Krzysztof Hałasa
2016-01-29 15:55     ` Krzysztof Hałasa
2016-02-17 15:08   ` Felipe Balbi
2016-02-17 15:08     ` Felipe Balbi
2016-02-17 16:00     ` Arnd Bergmann
2016-02-17 16:00       ` Arnd Bergmann
2016-01-28 16:20 ` [PATCH 4/7] usb: fsl: drop USB_FSL_MPH_DR_OF Kconfig symbol Arnd Bergmann
2016-01-28 16:20   ` Arnd Bergmann
2016-01-28 16:23 ` [PATCH 5/7] usb: isp1301-omap: mark power_up as __maybe_unused Arnd Bergmann
2016-01-28 16:23   ` Arnd Bergmann
2016-01-28 16:23   ` [PATCH 6/7] usb: musb: use %pad format string from dma_addr_t Arnd Bergmann
2016-01-28 16:23     ` Arnd Bergmann
2016-01-28 17:50     ` Tony Lindgren
2016-01-28 17:50       ` Tony Lindgren
2016-01-28 16:23   ` [PATCH 7/7] usb: musb/ux500: remove duplicate check for dma_is_compatible Arnd Bergmann
2016-01-28 16:23     ` Arnd Bergmann
2016-02-03 18:12 ` [PATCH 0/7] USB changes for rare warnings Felipe Balbi
2016-02-03 18:12   ` Felipe Balbi
2016-02-03 19:15   ` Robert Jarzmik
2016-02-03 19:15     ` Robert Jarzmik
2016-02-03 20:49     ` Arnd Bergmann
2016-02-03 20:49       ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m34md8rdol.fsf@t19.piap.pl \
    --to=khalasa@piap.pl \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.