On Fri, 28 Aug 2009 14:02:15 +0200
Wolfgang Denk <w...@denx.de> wrote:
> Dear Detlev Zundel,
>
> In message <m2tyzsw7hr....@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.
agreed, I was already thinking of ways of rewriting the ugly SCCR code,
but you beat me to it.
I'll try and get something out this weekend.
Kim
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
