[U-Boot] [PATCH] ARM: Convert {in,out}s[bwl] to inline functions

The size of uboot binary grows by a few bytes, but the gain (better type checking) is worth it.
Signed-off-by: Marek Vasut marek.vasut@gmail.com Cc: Wolfgang Denk wd@denx.de Cc: Nick Thompson nick.thompson@ge.com Cc: Simon Glass sjg@chromium.org --- arch/arm/include/asm/io.h | 34 ++++++++++++++++++++++++++++------ 1 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h index 61f4987..d22325d 100644 --- a/arch/arm/include/asm/io.h +++ b/arch/arm/include/asm/io.h @@ -255,13 +255,35 @@ extern inline void __raw_readsl(unsigned int addr, void *data, int longlen) #define inw(p) ({ unsigned int __v = le16_to_cpu(__raw_readw(__io(p))); __v; }) #define inl(p) ({ unsigned int __v = le32_to_cpu(__raw_readl(__io(p))); __v; })
-#define outsb(p,d,l) __raw_writesb(__io(p),d,l) -#define outsw(p,d,l) __raw_writesw(__io(p),d,l) -#define outsl(p,d,l) __raw_writesl(__io(p),d,l) +extern inline void outsb(unsigned int addr, const void *data, int bytelen) +{ + __raw_writesb(addr, data, bytelen); +} + +extern inline void outsw(unsigned int addr, const void *data, int wordlen) +{ + __raw_writesw(addr, data, wordlen); +} + +extern inline void outsl(unsigned int addr, const void *data, int longlen) +{ + __raw_writesl(addr, data, longlen); +}
-#define insb(p,d,l) __raw_readsb(__io(p),d,l) -#define insw(p,d,l) __raw_readsw(__io(p),d,l) -#define insl(p,d,l) __raw_readsl(__io(p),d,l) +extern inline void insb(unsigned int addr, void *data, int bytelen) +{ + __raw_readsb(addr, data, bytelen); +} + +extern inline void insw(unsigned int addr, void *data, int wordlen) +{ + __raw_readsw(addr, data, wordlen); +} + +extern inline void insl(unsigned int addr, void *data, int longlen) +{ + __raw_readsl(addr, data, longlen); +} #endif
#define outb_p(val,port) outb((val),(port))

On 26/09/11 19:48, Marek Vasut wrote:
The size of uboot binary grows by a few bytes, but the gain (better type checking) is worth it.
Signed-off-by: Marek Vasut marek.vasut@gmail.com Cc: Wolfgang Denk wd@denx.de Cc: Nick Thompson nick.thompson@ge.com Cc: Simon Glass sjg@chromium.org
Do you want to cc: Albert ARIBAUD albert.u.boot@aribaud.net as well?
arch/arm/include/asm/io.h | 34 ++++++++++++++++++++++++++++------ 1 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h index 61f4987..d22325d 100644 --- a/arch/arm/include/asm/io.h +++ b/arch/arm/include/asm/io.h @@ -255,13 +255,35 @@ extern inline void __raw_readsl(unsigned int addr, void *data, int longlen) #define inw(p) ({ unsigned int __v = le16_to_cpu(__raw_readw(__io(p))); __v; }) #define inl(p) ({ unsigned int __v = le32_to_cpu(__raw_readl(__io(p))); __v; })
-#define outsb(p,d,l) __raw_writesb(__io(p),d,l) -#define outsw(p,d,l) __raw_writesw(__io(p),d,l) -#define outsl(p,d,l) __raw_writesl(__io(p),d,l)
These changes are clearly related, but we started out talking about '__arch_putb', which is in the same file of course. Did I miss something?
This specific patch looks reasonable to me though.
Reviewed-by: Nick Thompson nick.thompson@ge.com
+extern inline void outsb(unsigned int addr, const void *data, int bytelen) +{
- __raw_writesb(addr, data, bytelen);
+}
+extern inline void outsw(unsigned int addr, const void *data, int wordlen) +{
- __raw_writesw(addr, data, wordlen);
+}
+extern inline void outsl(unsigned int addr, const void *data, int longlen) +{
- __raw_writesl(addr, data, longlen);
+}
-#define insb(p,d,l) __raw_readsb(__io(p),d,l) -#define insw(p,d,l) __raw_readsw(__io(p),d,l) -#define insl(p,d,l) __raw_readsl(__io(p),d,l) +extern inline void insb(unsigned int addr, void *data, int bytelen) +{
- __raw_readsb(addr, data, bytelen);
+}
+extern inline void insw(unsigned int addr, void *data, int wordlen) +{
- __raw_readsw(addr, data, wordlen);
+}
+extern inline void insl(unsigned int addr, void *data, int longlen) +{
- __raw_readsl(addr, data, longlen);
+} #endif
#define outb_p(val,port) outb((val),(port))

Dear Marek Vasut,
In message 1317062895-3847-1-git-send-email-marek.vasut@gmail.com you wrote:
The size of uboot binary grows by a few bytes, but the gain (better type checking) is worth it.
And what _exactly_ are "a few bytes" ?
Best regards,
Wolfgang Denk

On Tuesday, September 27, 2011 11:31:15 AM Wolfgang Denk wrote:
Dear Marek Vasut,
In message 1317062895-3847-1-git-send-email-marek.vasut@gmail.com you wrote:
The size of uboot binary grows by a few bytes, but the gain (better type checking) is worth it.
And what _exactly_ are "a few bytes" ?
Nevermind, it must have been some kind of a fluctuation yesterday. Right now, I made a new measurement and the size didn't change with/without the patch (this is more what I'd expect to happen).
Cheers
Best regards,
Wolfgang Denk

On 27/09/11 11:21, Marek Vasut wrote:
On Tuesday, September 27, 2011 11:31:15 AM Wolfgang Denk wrote:
Dear Marek Vasut,
In message 1317062895-3847-1-git-send-email-marek.vasut@gmail.com you wrote:
The size of uboot binary grows by a few bytes, but the gain (better type checking) is worth it.
And what _exactly_ are "a few bytes" ?
Nevermind, it must have been some kind of a fluctuation yesterday. Right now, I made a new measurement and the size didn't change with/without the patch (this is more what I'd expect to happen).
Cheers
Pure speculation on my part, but /could/ this be because ARM drivers don't tend to use these macros/functions. write[bwl] and the like are much more common. I don't know this to be a fact though.
Nick.

On Tuesday, September 27, 2011 01:57:52 PM Nick Thompson wrote:
On 27/09/11 11:21, Marek Vasut wrote:
On Tuesday, September 27, 2011 11:31:15 AM Wolfgang Denk wrote:
Dear Marek Vasut,
In message 1317062895-3847-1-git-send-email-marek.vasut@gmail.com you
wrote:
The size of uboot binary grows by a few bytes, but the gain (better type checking) is worth it.
And what _exactly_ are "a few bytes" ?
Nevermind, it must have been some kind of a fluctuation yesterday. Right now, I made a new measurement and the size didn't change with/without the patch (this is more what I'd expect to happen).
Cheers
Pure speculation on my part, but /could/ this be because ARM drivers don't tend to use these macros/functions. write[bwl] and the like are much more common. I don't know this to be a fact though.
No, I'm dead sure I use this macro in the test.
Nick.

On Tue, Sep 27, 2011 at 5:02 AM, Marek Vasut marek.vasut@gmail.com wrote:
On Tuesday, September 27, 2011 01:57:52 PM Nick Thompson wrote:
On 27/09/11 11:21, Marek Vasut wrote:
On Tuesday, September 27, 2011 11:31:15 AM Wolfgang Denk wrote:
Dear Marek Vasut,
In message 1317062895-3847-1-git-send-email-marek.vasut@gmail.com you
wrote:
The size of uboot binary grows by a few bytes, but the gain (better type checking) is worth it.
And what _exactly_ are "a few bytes" ?
Nevermind, it must have been some kind of a fluctuation yesterday. Right now, I made a new measurement and the size didn't change with/without the patch (this is more what I'd expect to happen).
Cheers
Pure speculation on my part, but /could/ this be because ARM drivers don't tend to use these macros/functions. write[bwl] and the like are much more common. I don't know this to be a fact though.
No, I'm dead sure I use this macro in the test.
Nick.
Hi,
Can't comment on the patch format, etc.
I tested this on my Seaboard, with no code size increase, and all worked as expected. I can't see why it would increase code size either.
But I have a few questions: what devices actually uses this macro? Otherwise I'm not sure if I am testing anything. Also, why not convert all the macros in this file? Seems like a good idea to me. Or is this patch just to test the waters? :-)
Regards, Simon

On Wednesday, September 28, 2011 12:40:01 AM Simon Glass wrote:
On Tue, Sep 27, 2011 at 5:02 AM, Marek Vasut marek.vasut@gmail.com wrote:
On Tuesday, September 27, 2011 01:57:52 PM Nick Thompson wrote:
On 27/09/11 11:21, Marek Vasut wrote:
On Tuesday, September 27, 2011 11:31:15 AM Wolfgang Denk wrote:
Dear Marek Vasut,
In message 1317062895-3847-1-git-send-email-marek.vasut@gmail.com you
wrote:
The size of uboot binary grows by a few bytes, but the gain (better type checking) is worth it.
And what _exactly_ are "a few bytes" ?
Nevermind, it must have been some kind of a fluctuation yesterday. Right now, I made a new measurement and the size didn't change with/without the patch (this is more what I'd expect to happen).
Cheers
Pure speculation on my part, but /could/ this be because ARM drivers don't tend to use these macros/functions. write[bwl] and the like are much more common. I don't know this to be a fact though.
No, I'm dead sure I use this macro in the test.
Nick.
Hi,
Can't comment on the patch format, etc.
I tested this on my Seaboard, with no code size increase, and all worked as expected. I can't see why it would increase code size either.
But I have a few questions: what devices actually uses this macro?
common/cmd_ide.c for example.
Otherwise I'm not sure if I am testing anything. Also, why not convert all the macros in this file? Seems like a good idea to me. Or is this patch just to test the waters? :-)
We should eventually get rid of all that crap altogether and unify the hardware access. But that seems like a long-term plan :-(
Cheers
Regards, Simon

Dear Marek Vasut,
In message 1317062895-3847-1-git-send-email-marek.vasut@gmail.com you wrote:
The size of uboot binary grows by a few bytes, but the gain (better type checking) is worth it.
Signed-off-by: Marek Vasut marek.vasut@gmail.com Cc: Wolfgang Denk wd@denx.de Cc: Nick Thompson nick.thompson@ge.com Cc: Simon Glass sjg@chromium.org
arch/arm/include/asm/io.h | 34 ++++++++++++++++++++++++++++------ 1 files changed, 28 insertions(+), 6 deletions(-)
And will you PLEASE get used to sticking with the rules?
There is no patch version in the Subject line.
There is no change log in the comment section either.
NAK.
Wolfgang Denk

On Tuesday, September 27, 2011 11:32:38 AM Wolfgang Denk wrote:
Dear Marek Vasut,
In message 1317062895-3847-1-git-send-email-marek.vasut@gmail.com you wrote:
The size of uboot binary grows by a few bytes, but the gain (better type checking) is worth it.
Signed-off-by: Marek Vasut marek.vasut@gmail.com Cc: Wolfgang Denk wd@denx.de Cc: Nick Thompson nick.thompson@ge.com Cc: Simon Glass sjg@chromium.org
arch/arm/include/asm/io.h | 34 ++++++++++++++++++++++++++++------ 1 files changed, 28 insertions(+), 6 deletions(-)
And will you PLEASE get used to sticking with the rules?
There is no patch version in the Subject line.
There is no change log in the comment section either.
This is a new patch ... that's why there's no changelog and no V1.
Cheers
NAK.
Wolfgang Denk

Dear Marek Vasut,
In message 201109271208.09363.marek.vasut@gmail.com you wrote:
And will you PLEASE get used to sticking with the rules?
There is no patch version in the Subject line.
There is no change log in the comment section either.
This is a new patch ... that's why there's no changelog and no V1.
Oh, is it?
And what is this, then:
09/26 Marek Vasut [PATCH] ARM: Convert {in,out}s[bwl] to inline functions
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/109509
Looks to be exactly the same, judging by the Subject: line...
Best regards,
Wolfgang Denk

On Wednesday, September 28, 2011 10:46:57 PM Wolfgang Denk wrote:
Dear Marek Vasut,
In message 201109271208.09363.marek.vasut@gmail.com you wrote:
And will you PLEASE get used to sticking with the rules?
There is no patch version in the Subject line.
There is no change log in the comment section either.
This is a new patch ... that's why there's no changelog and no V1.
Oh, is it?
And what is this, then:
09/26 Marek Vasut [PATCH] ARM: Convert {in,out}s[bwl] to inline functions
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/109509
Looks to be exactly the same, judging by the Subject: line...
Well ... that's this patch, right ?
Cheers

Dear Marek Vasut,
In message 201109282258.10007.marek.vasut@gmail.com you wrote:
This is a new patch ... that's why there's no changelog and no V1.
Oh, is it?
And what is this, then:
09/26 Marek Vasut [PATCH] ARM: Convert {in,out}s[bwl] to inline functions
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/109509
Looks to be exactly the same, judging by the Subject: line...
Well ... that's this patch, right ?
Oops. You are right. Sorry.
Best regards,
Wolfgang Denk
participants (4)
-
Marek Vasut
-
Nick Thompson
-
Simon Glass
-
Wolfgang Denk