[U-Boot] 83xx and LCRR setting

[Detlev Zundel <dzu@denx.de>]

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

-- 
Greenspun's Tenth Rule of Programming: "Any sufficiently complicated C
or Fortran program contains an ad-hoc, informally-specified bug-ridden
slow implementation of half of Common Lisp."
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de