| Message ID | 1309275583-11763-4-git-send-email-robherring2@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Dear Rob Herring, In message <1309275583-11763-4-git-send-email-robherring2@gmail.com> you wrote: > From: Rob Herring <rob.herring@calxeda.com> > > Add __ilog2 function for ARM. Needed for ahci.c > > Signed-off-by: Rob Herring <rob.herring@calxeda.com> > Cc: Albert ARIBAUD <albert.aribaud@free.fr> > --- > arch/arm/include/asm/bitops.h | 9 +++++++++ > 1 files changed, 9 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h > index 270f163..0420182 100644 > --- a/arch/arm/include/asm/bitops.h > +++ b/arch/arm/include/asm/bitops.h > @@ -106,6 +106,15 @@ static inline int test_bit(int nr, const void * addr) > return ((unsigned char *) addr)[nr >> 3] & (1U << (nr & 7)); > } > > +extern __inline__ int __ilog2(unsigned int x) checkpatch says: WARNING: plain inline is preferred over __inline__ Best regards, Wolfgang Denk
Wolfgang, On 07/04/2011 05:13 AM, Wolfgang Denk wrote: > Dear Rob Herring, > > In message <1309275583-11763-4-git-send-email-robherring2@gmail.com> you wrote: >> From: Rob Herring <rob.herring@calxeda.com> >> >> Add __ilog2 function for ARM. Needed for ahci.c >> >> Signed-off-by: Rob Herring <rob.herring@calxeda.com> >> Cc: Albert ARIBAUD <albert.aribaud@free.fr> >> --- >> arch/arm/include/asm/bitops.h | 9 +++++++++ >> 1 files changed, 9 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h >> index 270f163..0420182 100644 >> --- a/arch/arm/include/asm/bitops.h >> +++ b/arch/arm/include/asm/bitops.h >> @@ -106,6 +106,15 @@ static inline int test_bit(int nr, const void * addr) >> return ((unsigned char *) addr)[nr >> 3] & (1U << (nr & 7)); >> } >> >> +extern __inline__ int __ilog2(unsigned int x) > > checkpatch says: WARNING: plain inline is preferred over __inline__ > Sorry about this. I thought I was running checkpatch, but there was a problem in my script I used and it was failing silently. Rob
Hi Rob, On Tuesday 28 June 2011 09:09 PM, Rob Herring wrote: > From: Rob Herring<rob.herring@calxeda.com> > > Add __ilog2 function for ARM. Needed for ahci.c > > Signed-off-by: Rob Herring<rob.herring@calxeda.com> > Cc: Albert ARIBAUD<albert.aribaud@free.fr> > --- > arch/arm/include/asm/bitops.h | 9 +++++++++ > 1 files changed, 9 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h > index 270f163..0420182 100644 > --- a/arch/arm/include/asm/bitops.h > +++ b/arch/arm/include/asm/bitops.h > @@ -106,6 +106,15 @@ static inline int test_bit(int nr, const void * addr) > return ((unsigned char *) addr)[nr>> 3]& (1U<< (nr& 7)); > } > > +extern __inline__ int __ilog2(unsigned int x) > +{ > + int ret; > + > + asm("clz\t%0, %1" : "=r" (ret) : "r" (x)); I tried doing the same sometime back for the same need. I had to abandon it because CLZ seems to be added only in ARMv5. And at least one ARMv7 SoC uses -march=armv4 while building. best regards, Aneesh
Dear Aneesh V, In message <4E129653.7050807@ti.com> you wrote: > > I tried doing the same sometime back for the same need. I had to > abandon it because CLZ seems to be added only in ARMv5. And at least > one ARMv7 SoC uses -march=armv4 while building. Does this setting make any sense, or should we fix that? Best regards, Wolfgang Denk
Dear Wolfgang, On Tuesday 05 July 2011 03:28 PM, Wolfgang Denk wrote: > Dear Aneesh V, > > In message<4E129653.7050807@ti.com> you wrote: >> >> I tried doing the same sometime back for the same need. I had to >> abandon it because CLZ seems to be added only in ARMv5. And at least >> one ARMv7 SoC uses -march=armv4 while building. > > Does this setting make any sense, or should we fix that? Looks like it makes sense. Here is what the comment says in arch/arm/cpu/armv7/tegra2/config.mk # Use ARMv4 for Tegra2 - initial code runs on the AVP, which is an ARM7TDI. PLATFORM_CPPFLAGS += -march=armv4 Even if we could fix this, Rob is adding the API for all 'arm', which I feel is not correct. best regards, Aneesh
Dear Aneesh V, In message <4E12E2C6.7020401@ti.com> you wrote: > > Looks like it makes sense. Here is what the comment says in > arch/arm/cpu/armv7/tegra2/config.mk > > # Use ARMv4 for Tegra2 - initial code runs on the AVP, which is an ARM7TDI. > PLATFORM_CPPFLAGS += -march=armv4 I'm not sure what "initial code" here means - all of U-Boot? > Even if we could fix this, Rob is adding the API for all 'arm', which I > feel is not correct. Agreed. But then, it's the overwhelming majority that could use this code. Best regards, Wolfgang Denk
Le 05/07/2011 13:17, Wolfgang Denk a écrit : > Dear Aneesh V, > > In message<4E12E2C6.7020401@ti.com> you wrote: >> >> Looks like it makes sense. Here is what the comment says in >> arch/arm/cpu/armv7/tegra2/config.mk >> >> # Use ARMv4 for Tegra2 - initial code runs on the AVP, which is an ARM7TDI. >> PLATFORM_CPPFLAGS += -march=armv4 > > I'm not sure what "initial code" here means - all of U-Boot? > >> Even if we could fix this, Rob is adding the API for all 'arm', which I >> feel is not correct. > > Agreed. But then, it's the overwhelming majority that could use this > code. Maybe we could condition this __ilog2 definition on whether we build for armv4 (and below) or armv5t (or above)? > Best regards, > > Wolfgang Denk Amicalement,
diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h index 270f163..0420182 100644 --- a/arch/arm/include/asm/bitops.h +++ b/arch/arm/include/asm/bitops.h @@ -106,6 +106,15 @@ static inline int test_bit(int nr, const void * addr) return ((unsigned char *) addr)[nr >> 3] & (1U << (nr & 7)); } +extern __inline__ int __ilog2(unsigned int x) +{ + int ret; + + asm("clz\t%0, %1" : "=r" (ret) : "r" (x)); + ret = 31 - ret; + return ret; +} + /* * ffz = Find First Zero in word. Undefined if no zero exists, * so code should check against ~0UL first..