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

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

[Marek Vasut]

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))
-- 
1.7.5.4

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

[Nick Thompson]

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))

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

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"This is a test of the Emergency Broadcast System. If this had been an
actual emergency, do you really think we'd stick around to tell you?"

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

[Marek Vasut]

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

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

[Nick Thompson]

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.

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

[Marek Vasut]

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.

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

[Simon Glass]

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

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

[Marek Vasut]

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

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

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Women are more easily and more deeply terrified ...  generating  more
sheer horror than the male of the species.
	-- Spock, "Wolf in the Fold", stardate 3615.4

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

[Marek Vasut]

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

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

[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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The amount of time between slipping on the peel and  landing  on  the
pavement is precisely 1 bananosecond.

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

[Marek Vasut]

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

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

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I often quote myself; it adds spice to my conversation.  - G. B. Shaw