[U-Boot] [PATCH] post: fix up I/O helper usage
The I/O API from Linux defaults to little endian accesses. In order to do big endian accesses, there are a "be" variants. The "le32" variants are arch-specific and not terribly common, so change it to the normal Linux API funcs.
Signed-off-by: Mike Frysinger vapier@gentoo.org --- include/post.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/post.h b/include/post.h index 519cef1..c9ec2f4 100644 --- a/include/post.h +++ b/include/post.h @@ -78,12 +78,12 @@
static inline ulong post_word_load (void) { - return in_le32((volatile void *)(_POST_WORD_ADDR)); + return inl((volatile void *)(_POST_WORD_ADDR)); }
static inline void post_word_store (ulong value) { - out_le32((volatile void *)(_POST_WORD_ADDR), value); + outl(value, (volatile void *)(_POST_WORD_ADDR)); } #endif /* defined (CONFIG_POST) || defined(CONFIG_LOGBUFFER) */ #endif /* __ASSEMBLY__ */
Dear Mike Frysinger,
In message 1304996478-1536-1-git-send-email-vapier@gentoo.org you wrote:
The I/O API from Linux defaults to little endian accesses. In order to do big endian accesses, there are a "be" variants. The "le32" variants are arch-specific and not terribly common, so change it to the normal Linux API funcs.
Signed-off-by: Mike Frysinger vapier@gentoo.org
include/post.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/post.h b/include/post.h index 519cef1..c9ec2f4 100644 --- a/include/post.h +++ b/include/post.h @@ -78,12 +78,12 @@
static inline ulong post_word_load (void) {
- return in_le32((volatile void *)(_POST_WORD_ADDR));
- return inl((volatile void *)(_POST_WORD_ADDR));
}
Is this supposed to fix any real problem, or just a change according to your personal preferences?
My initial impression is that we lose a bit of documentation (explicitely telling the reader that these are LE accesses has a certain benefit over hoping the reader has this information present).
Best regards,
Wolfgang Denk
On Tue, Jul 26, 2011 at 07:43, Wolfgang Denk wrote:
Mike Frysinger you wrote:
The I/O API from Linux defaults to little endian accesses. In order to do big endian accesses, there are a "be" variants. The "le32" variants are arch-specific and not terribly common, so change it to the normal Linux API funcs.
--- a/include/post.h +++ b/include/post.h @@ -78,12 +78,12 @@
static inline ulong post_word_load (void) {
- return in_le32((volatile void *)(_POST_WORD_ADDR));
- return inl((volatile void *)(_POST_WORD_ADDR));
}
Is this supposed to fix any real problem, or just a change according to your personal preferences?
the "in_le32" funcs (and all the other related "le32" helpers) never made it into the common Linux API and many ports (such as the Blackfin arch) never defined them. so it fixes building for all the ports which lack "in_le32". i dont have a source tree by me atm, but i'd imagine that this is most arches. -mike
Dear Mike Frysinger,
In message CAJaTeTpdbcSx5oWsQaP=YQfKYnZcVgc-w4G18ZYmka5GgBAcxg@mail.gmail.com you wrote:
return in_le32((volatile void *)(_POST_WORD_ADDR));
return inl((volatile void *)(_POST_WORD_ADDR));}
Is this supposed to fix any real problem, or just a change according to your personal preferences?
the "in_le32" funcs (and all the other related "le32" helpers) never made it into the common Linux API and many ports (such as the Blackfin arch) never defined them. so it fixes
True. But the same is also true for any other of the so-called "standard accessors". The fact that some architectures are slow to adapt to the standards does not convince me that we have to always accept the lowest common denominator. We can also push forward, at least sometimes.
building for all the ports which lack "in_le32". i dont have a source tree by me atm, but i'd imagine that this is most arches.
outl() feels wrong to me - the signature (second argument is an "I/O port") and the definition in Linux (include/asm-generic/io.h: return readl(addr + PCI_IOBASE);) this is for accessing I/O ports. The documentation ("memory-barriers.txt") says:
(*) inX(), outX():
These are intended to talk to I/O space rather than memory space ...
What we should be using (and standardizing for) is probably this (at least some of the PTBs said so in the past):
(*) ioreadX(), iowriteX()
These will perform appropriately for the type of access they're actually doing, be it inX()/outX() or readX()/writeX().
However, even less architectres have these. [But funny enough, in U-Boot it's BF which leads by example.]
Sorry, but I will not accept inl() here.
Best regards,
Wolfgang Denk
On Thu, Jul 28, 2011 at 07:23, Wolfgang Denk wrote:
Mike Frysinger wrote:
- return in_le32((volatile void *)(_POST_WORD_ADDR));
- return inl((volatile void *)(_POST_WORD_ADDR));
}
Is this supposed to fix any real problem, or just a change according to your personal preferences?
the "in_le32" funcs (and all the other related "le32" helpers) never made it into the common Linux API and many ports (such as the Blackfin arch) never defined them. so it fixes
True. But the same is also true for any other of the so-called "standard accessors". The fact that some architectures are slow to adapt to the standards does not convince me that we have to always accept the lowest common denominator. We can also push forward, at least sometimes.
sorry, but i dont think this qualifies. ppc choosing to add some arbitrary new macros by themselves which one or two other arches happen to copy does not make them part of the influx standard. looking at the linux/u-boot trees, ppc seems to be the only one that implements it in both (and perhaps m68k).
What we should be using (and standardizing for) is probably this (at least some of the PTBs said so in the past):
(*) ioreadX(), iowriteX()
that's fine by me
Sorry, but I will not accept inl() here.
so not even to fix build failures for most non-ppc arches ? i wont be able to post an updated patch for a while. -mike
On Friday, July 29, 2011 14:40:23 Mike Frysinger wrote:
On Thu, Jul 28, 2011 at 07:23, Wolfgang Denk wrote:
What we should be using (and standardizing for) is probably this (at least some of the PTBs said so in the past):
(*) ioreadX(), iowriteX()that's fine by me
Sorry, but I will not accept inl() here.
so not even to fix build failures for most non-ppc arches ? i wont be able to post an updated patch for a while.
so i can post a patch to use io{read,write}32, but then it'll only build for Blackfin targets whereas the patch i posted originally works for all arches. what exactly do you want done ? -mike
Dear Mike Frysinger,
In message 201108141645.40439.vapier@gentoo.org you wrote:
Sorry, but I will not accept inl() here.
so not even to fix build failures for most non-ppc arches ? i wont be able to post an updated patch for a while.
so i can post a patch to use io{read,write}32, but then it'll only build for Blackfin targets whereas the patch i posted originally works for all arches. what exactly do you want done ?
Best would be to add io{read,write}*() support for those architectures that don't have it yet.
Thanks.
Wolfgang Denk
participants (2)
-
Mike Frysinger -
Wolfgang Denk