From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arm-kernel@lists.infradead.org,
Sascha Hauer <s.hauer@pengutronix.de>,
Zhang Lily-R58066 <r58066@freescale.com>,
linux-fbdev@vger.kernel.org,
Arnaud Patard <arnaud.patard@rtp-net.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/9] ARM i.MX51: Add ipu clock support
Date: Wed, 15 Dec 2010 17:12:46 +0000 [thread overview]
Message-ID: <20101215171246.GG9937@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <201012151749.59488.arnd@arndb.de>
On Wed, Dec 15, 2010 at 05:49:59PM +0100, Arnd Bergmann wrote:
> On Wednesday 15 December 2010, Russell King - ARM Linux wrote:
> > Lastly, I don't see where atomicity comes into it - __raw_writel vs writel
> > have the same atomicity. Both are single access atomic provided they're
> > naturally aligned. Misaligned device accesses are not predictable.
>
> This is just what gcc turns it into today. In theory, a future gcc or
> a future cpu might change that. If you mark a pointer as
> '__attribute__((packed))', it probably already does, even for aligned
> pointers, while it does not when using writel_{,relaxed}. The point is
> that __raw_* means just that -- we don't give any guarantees on what
> happens on the bus, so people should not use it.
No. It does give guarantees on what happens on the bus. The kind of
pointer you pass in has no bearing on what happens on the bus because it's
casted away immediately.
__raw_writel(v, ptr) doesn't just do *(ptr) = v - that doesn't work when
ptr is void. Instead, it has to do a cast to ensure that we have a valid
C dereference (void pointers are illegal to dereference):
#define __raw_writel(v,a) (__chk_io_ptr(a), \
*(volatile unsigned int __force *)(a) = (v))
Doesn't matter if 'a' was marked as packed or not - that's all casted away.
That's not something that should ever change - otherwise we'll all be
screwed as you could never cast away pointer attributes.
It _may_ possible that the compiler decides that accessing an 'unsigned int'
will not be done as a single word load, in which case we have exactly the
same problems with writel() too.
And in any case, if __raw_writel() was as you're suggesting, then it would
serve absolutely no purpose at all.
WARNING: multiple messages have this Message-ID (diff)
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/9] ARM i.MX51: Add ipu clock support
Date: Wed, 15 Dec 2010 17:12:46 +0000 [thread overview]
Message-ID: <20101215171246.GG9937@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <201012151749.59488.arnd@arndb.de>
On Wed, Dec 15, 2010 at 05:49:59PM +0100, Arnd Bergmann wrote:
> On Wednesday 15 December 2010, Russell King - ARM Linux wrote:
> > Lastly, I don't see where atomicity comes into it - __raw_writel vs writel
> > have the same atomicity. Both are single access atomic provided they're
> > naturally aligned. Misaligned device accesses are not predictable.
>
> This is just what gcc turns it into today. In theory, a future gcc or
> a future cpu might change that. If you mark a pointer as
> '__attribute__((packed))', it probably already does, even for aligned
> pointers, while it does not when using writel_{,relaxed}. The point is
> that __raw_* means just that -- we don't give any guarantees on what
> happens on the bus, so people should not use it.
No. It does give guarantees on what happens on the bus. The kind of
pointer you pass in has no bearing on what happens on the bus because it's
casted away immediately.
__raw_writel(v, ptr) doesn't just do *(ptr) = v - that doesn't work when
ptr is void. Instead, it has to do a cast to ensure that we have a valid
C dereference (void pointers are illegal to dereference):
#define __raw_writel(v,a) (__chk_io_ptr(a), \
*(volatile unsigned int __force *)(a) = (v))
Doesn't matter if 'a' was marked as packed or not - that's all casted away.
That's not something that should ever change - otherwise we'll all be
screwed as you could never cast away pointer attributes.
It _may_ possible that the compiler decides that accessing an 'unsigned int'
will not be done as a single word load, in which case we have exactly the
same problems with writel() too.
And in any case, if __raw_writel() was as you're suggesting, then it would
serve absolutely no purpose at all.
WARNING: multiple messages have this Message-ID (diff)
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/9] ARM i.MX51: Add ipu clock support
Date: Wed, 15 Dec 2010 17:12:46 +0000 [thread overview]
Message-ID: <20101215171246.GG9937@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <201012151749.59488.arnd@arndb.de>
On Wed, Dec 15, 2010 at 05:49:59PM +0100, Arnd Bergmann wrote:
> On Wednesday 15 December 2010, Russell King - ARM Linux wrote:
> > Lastly, I don't see where atomicity comes into it - __raw_writel vs writel
> > have the same atomicity. Both are single access atomic provided they're
> > naturally aligned. Misaligned device accesses are not predictable.
>
> This is just what gcc turns it into today. In theory, a future gcc or
> a future cpu might change that. If you mark a pointer as
> '__attribute__((packed))', it probably already does, even for aligned
> pointers, while it does not when using writel_{,relaxed}. The point is
> that __raw_* means just that -- we don't give any guarantees on what
> happens on the bus, so people should not use it.
No. It does give guarantees on what happens on the bus. The kind of
pointer you pass in has no bearing on what happens on the bus because it's
casted away immediately.
__raw_writel(v, ptr) doesn't just do *(ptr) = v - that doesn't work when
ptr is void. Instead, it has to do a cast to ensure that we have a valid
C dereference (void pointers are illegal to dereference):
#define __raw_writel(v,a) (__chk_io_ptr(a), \
*(volatile unsigned int __force *)(a) = (v))
Doesn't matter if 'a' was marked as packed or not - that's all casted away.
That's not something that should ever change - otherwise we'll all be
screwed as you could never cast away pointer attributes.
It _may_ possible that the compiler decides that accessing an 'unsigned int'
will not be done as a single word load, in which case we have exactly the
same problems with writel() too.
And in any case, if __raw_writel() was as you're suggesting, then it would
serve absolutely no purpose at all.
next prev parent reply other threads:[~2010-12-15 17:13 UTC|newest]
Thread overview: 110+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-09 13:47 [PATCH RFC] i.MX51 Framebuffer support Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` [PATCH 1/9] ARM i.MX51: Add ipu clock support Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-15 15:40 ` Arnd Bergmann
2010-12-15 15:40 ` Arnd Bergmann
2010-12-15 15:40 ` Arnd Bergmann
2010-12-15 16:34 ` Russell King - ARM Linux
2010-12-15 16:34 ` Russell King - ARM Linux
2010-12-15 16:34 ` Russell King - ARM Linux
2010-12-15 16:49 ` Arnd Bergmann
2010-12-15 16:49 ` Arnd Bergmann
2010-12-15 16:49 ` Arnd Bergmann
2010-12-15 17:12 ` Russell King - ARM Linux [this message]
2010-12-15 17:12 ` Russell King - ARM Linux
2010-12-15 17:12 ` Russell King - ARM Linux
2010-12-09 13:47 ` [PATCH 2/9] ARM i.MX51: rename IPU irqs Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 14:34 ` Uwe Kleine-König
2010-12-09 14:34 ` Uwe Kleine-König
2010-12-09 14:34 `
2010-12-09 13:47 ` [PATCH 3/9] Add a mfd IPUv3 driver Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-12 5:21 ` Liu Ying
2010-12-12 5:21 ` Liu Ying
2010-12-13 11:23 ` Sascha Hauer
2010-12-13 11:23 ` Sascha Hauer
2010-12-13 11:23 ` Sascha Hauer
2010-12-14 4:05 ` Liu Ying
2010-12-14 4:05 ` Liu Ying
2010-12-14 4:05 ` Liu Ying
2010-12-14 8:40 ` Sascha Hauer
2010-12-14 8:40 ` Sascha Hauer
2010-12-14 8:40 ` Sascha Hauer
2010-12-14 13:13 ` Liu Ying
2010-12-14 13:13 ` Liu Ying
2010-12-14 13:13 ` Liu Ying
2010-12-09 13:47 ` [PATCH 4/9] fb: export fb mode db table Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2011-01-06 7:26 ` Paul Mundt
2011-01-06 7:26 ` Paul Mundt
2011-01-06 7:26 ` Paul Mundt
2011-01-06 10:04 ` Sascha Hauer
2011-01-06 10:04 ` Sascha Hauer
2011-01-06 10:04 ` Sascha Hauer
2010-12-09 13:47 ` [PATCH 5/9] Add i.MX5 framebuffer driver Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-12 6:13 ` Liu Ying
2010-12-12 6:13 ` Liu Ying
2010-12-12 6:13 ` Liu Ying
2010-12-13 7:23 ` Lothar Waßmann
2010-12-13 7:23 ` Lothar Waßmann
2010-12-13 7:23 ` Lothar Waßmann
2010-12-13 11:35 ` Liu Ying
2010-12-13 11:35 ` Liu Ying
2010-12-13 11:35 ` Liu Ying
2010-12-13 11:38 ` Sascha Hauer
2010-12-13 11:38 ` Sascha Hauer
2010-12-13 11:38 ` Sascha Hauer
2010-12-14 6:40 ` Liu Ying
2010-12-14 6:40 ` Liu Ying
2010-12-14 6:40 ` Liu Ying
2010-12-14 8:45 ` Sascha Hauer
2010-12-14 8:45 ` Sascha Hauer
2010-12-14 8:45 ` Sascha Hauer
2010-12-14 13:23 ` Liu Ying
2010-12-14 13:23 ` Liu Ying
2010-12-14 13:23 ` Liu Ying
2010-12-15 11:17 ` Sascha Hauer
2010-12-15 11:17 ` Sascha Hauer
2010-12-15 11:17 ` Sascha Hauer
2010-12-09 13:47 ` [PATCH 6/9] ARM i.MX51: Add IPU device support Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-15 15:49 ` Arnd Bergmann
2010-12-15 15:49 ` Arnd Bergmann
2010-12-15 15:49 ` Arnd Bergmann
2010-12-15 16:26 ` Arnaud Patard
2010-12-15 16:26 ` Arnaud Patard (Rtp)
2010-12-15 16:26 ` Arnaud Patard
2010-12-15 16:29 ` Arnd Bergmann
2010-12-15 16:29 ` Arnd Bergmann
2010-12-15 16:29 ` Arnd Bergmann
2010-12-09 13:47 ` [PATCH 7/9] ARM i.MX5: Allow to increase max zone order Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` [PATCH 8/9] ARM i.MX5: increase dma consistent size for IPU support Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` [PATCH 9/9] ARM i.MX51 babbage: Add framebuffer support Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-09 13:47 ` Sascha Hauer
2010-12-12 1:37 ` Liu Ying
2010-12-12 1:37 ` Liu Ying
2010-12-12 1:37 ` Liu Ying
2010-12-13 11:43 ` Sascha Hauer
2010-12-13 11:43 ` Sascha Hauer
2010-12-13 11:43 ` Sascha Hauer
2010-12-14 6:47 ` Liu Ying
2010-12-14 6:47 ` Liu Ying
2010-12-14 6:47 ` Liu Ying
2010-12-15 9:35 ` [PATCH RFC] i.MX51 Framebuffer support Peter Horton
2010-12-15 11:24 ` Sascha Hauer
2010-12-20 10:48 [PATCH v2] " Sascha Hauer
2010-12-20 10:48 ` [PATCH 1/9] ARM i.MX51: Add ipu clock support Sascha Hauer
2010-12-20 10:48 ` Sascha Hauer
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=20101215171246.GG9937@n2100.arm.linux.org.uk \
--to=linux@arm.linux.org.uk \
--cc=arnaud.patard@rtp-net.org \
--cc=arnd@arndb.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=r58066@freescale.com \
--cc=s.hauer@pengutronix.de \
/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.