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 10:26:14 +0100	[thread overview]
Message-ID: <m3d1rxqa4p.fsf@t19.piap.pl> (raw)
In-Reply-To: <4239208.KBB6rfivoa@wuerfel> (Arnd Bergmann's message of "Mon, 15 Feb 2016 17:12:54 +0100")

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

> 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.
-- 
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 10:26:14 +0100	[thread overview]
Message-ID: <m3d1rxqa4p.fsf@t19.piap.pl> (raw)
In-Reply-To: <4239208.KBB6rfivoa@wuerfel> (Arnd Bergmann's message of "Mon, 15 Feb 2016 17:12:54 +0100")

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

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

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

  reply	other threads:[~2016-02-16  9:26 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 [this message]
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
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=m3d1rxqa4p.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.