| Message ID | 1317062895-3847-1-git-send-email-marek.vasut@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
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
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
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 <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
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))
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(-)