Re: [U-Boot] [PATCH 2/3] Add generic bit operations

On Thu, Jun 04, 2009 at 01:59:22PM +0200, Wolfgang Denk wrote:
And the functions I removed from asm-arm/bitops.h did that?
No. Let's be happy that we have eliminated some poor code, and if we add a replacement, let's make sure not to repeat the mistakes of the past again.
Ok. I just saw ubifs implementing its own set_bit() functions and considered that the wrong place for such functions to live in. ext2 seems to do the same things, also minix. Platforms which want to enable support for these filesystems have to do an evil #define to map ext2_set_bit() to the platform specific version. Which is all bogus, you might agree. And because of that situation, ubifs can't currently build for anything else than ppc (according to 'git grep -w fls include/asm*').
Hence I thought it might be a good idea (or at least a good start thereof) to put the functions where I believe they belong to - a 'generic' place.
Anyway - applying the first patch of this series would at least prevent others from being mislead by dead and wrong code.
Daniel

On Thursday 04 June 2009 20:00:42 Daniel Mack wrote:
On Thu, Jun 04, 2009 at 01:59:22PM +0200, Wolfgang Denk wrote:
And the functions I removed from asm-arm/bitops.h did that?
No. Let's be happy that we have eliminated some poor code, and if we add a replacement, let's make sure not to repeat the mistakes of the past again.
Ok. I just saw ubifs implementing its own set_bit() functions and considered that the wrong place for such functions to live in. ext2 seems to do the same things, also minix. Platforms which want to enable support for these filesystems have to do an evil #define to map ext2_set_bit() to the platform specific version. Which is all bogus, you might agree.
Ack. It would a good thing to have those functions available for all architectures so that common code like UBIFS etc can use it. Please note that UBIFS uses these bit functions for memory operations only (not IO access). So we don't have to use accessor functions etc here. The "standard" Linux implementation should do.
And because of that situation, ubifs can't currently build for anything else than ppc (according to 'git grep -w fls include/asm*').
Hence I thought it might be a good idea (or at least a good start thereof) to put the functions where I believe they belong to - a 'generic' place.
Yes, I do like this approach. And from my point of view, we should continue this way.
Best regards, Stefan
===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================

Dear Daniel Mack,
In message 20090604180042.GN26688@buzzloop.caiaq.de you wrote:
Ok. I just saw ubifs implementing its own set_bit() functions and considered that the wrong place for such functions to live in. ext2 seems to do the same things, also minix. Platforms which want to enable support for these filesystems have to do an evil #define to map ext2_set_bit() to the platform specific version. Which is all bogus, you might agree. And because of that situation, ubifs can't currently build for anything else than ppc (according to 'git grep -w fls include/asm*').
Yes, we have a mess here, and we need toclean this up ASAP.
I'm trying to find out if there is any agreement what a portable set of I/O and bit macro definitions should look like.
Anyway - applying the first patch of this series would at least prevent others from being mislead by dead and wrong code.
ACK on that.
Best regards,
Wolfgang Denk
participants (3)
-
Daniel Mack
-
Stefan Roese
-
Wolfgang Denk