Hi Heiko,
Hello Kim,
Kim Phillips wrote:
On Thu, 20 Aug 2009 13:03:09 +0200 Detlev Zundel dzu@denx.de wrote:
or maybe even always use the mask, define it in the board config and do a
#if !defined(LCCR_MASK) #define LCCR_MASK 0xFFFFFFFF #endif
ack.
nack, it should be
#if !defined(LCCR_MASK) #define LCCR_MASK 0x00000000 #endif
because I did:
+#if defined(CONFIG_MPC832x)
- /* LCRR - Clock Ratio Register (10.3.1.14) */
- im->lbus.lcrr = (im->lbus.lcrr & LCRR_MASK) | \
(CONFIG_SYS_LCRR & ~LCRR_MASK);+#else
but I can do of course a:
- /* LCRR - Clock Ratio Register (10.3.1.14) */
- im->lbus.lcrr = (im->lbus.lcrr & ~LCRR_MASK) | \
(CONFIG_SYS_LCRR & LCRR_MASK);Which way is prefered?
Personally I'd prefer the latter, as I expect a bitmask to specifiy which bits I'm allowed to _write_.
But I am not sure if this is necessary, I prefer here the conditional in code, because, if I port a new board to u-boot, I immidiately see, there is something special, if I have an 832x ... and this construct is not so ugly I think ...
Well, I really do not see any need for a conditional _in code_. Remember, such conditionals multiply the number of possible "source input to the compiler" by two, i.e. they double correct testing whereas different constants usually preserve the c level input to the compiler.
This really depends if and how this applies to the other members of the 83xx family.
they're all the same, really.
OK, if it is so ;-)
Then why not put this into mpc83xx.h?
And, by the way, we should _really_ be using accessor macros all around ;)
actually LCRR itself has specific instructions that say if written, it then should be read, and then an isync be issued.
Hmm.. actual code did not this! Where is this documented? And shouldn;t we update this in code?
Of course "we" should update this ;)
Cheers Detlev