[U-Boot] [PATCH v2] mpc83xx: update LCRR register handling

[Wolfgang Denk]

Dear Detlev Zundel,

In message <m2tyzsw7hr.fsf at ohwell.denx.de> you wrote:
> 
> > I think we should allow board-configurable writes to the DBYP bit, which
> > is documented as "reserved" on some 83xx, on the 83xx parts that /do/
> > implement it.  So instead of having a mask, perhaps setting absolute
> > values for CONFIG_SYS_LCRR should be replaced with a better scheme that
> > allows board configs to just set LCRR bits by field, such as what the
> > SCCR setting code does.  I.e, deprecate CONFIG_SYS_LCRR and replace with
> > individually-specified CONFIG_SYS_LCRR_{CLKDIV,EADC,ECL,BUDCMDC,DBYP}
> > values.
> >
> > This will allow the reserved bits, whether 1 on reset or
> > not, to be preserved across all 83xx (and 85xx for that matter).
> 
> Hm, you mean like the SCCR stuff in mpc83xx/cpu_init.c?  Please don't.
> 
> This code is plain ugly - and even more, as I have pointed out multiple
> times, in not more than 50 lines there are "only" 1024 non-trivially
> differing c input possibilities coded.  This is what I call bad.

Agreed.

Kim, the code in mpc83xx/cpu_init.c is *really* ugly.

I hereby suggest that this should be cleaned up ASAP.

> Thinking about it, why not do a compromise like e.g. the following:

Hm... I don't like the need for duplicated "#ifdef" lists, but don;t
know of a better way to write it either.

> out_be32(&im->clk.sccr, i(n_be32(&im->clk.sccr) & ~sccr_mask) | sccr_value);

Or rather:

	clrsetbits_be32(&im->clk.sccr, sccr_mask, sccr_value);


> This not only looks a bit nicer, but also (I hope) compiles the *exact*
> same code for all possibilites, only with changing data values.

Agreed.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You got to learn three things. What's  real,  what's  not  real,  and
what's the difference."           - Terry Pratchett, _Witches Abroad_