Dear Detlev Zundel,
In message m2tyzsw7hr.fsf@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