On Thu, 27 Aug 2009 13:11:25 +0200 Detlev Zundel dzu@denx.de wrote:
Hi Kim,
o LCRR_PDYP, granted dangerous in your case, is obviously a writeable bit (not read-only), and documented as such in later documentation. In fact, there are no non-writeable bits in LCRR.
Well, "reserved" != "non-writable" (usually there is a comment that writing reserved bits produces undefined behaviour) so I agree with Heiko that as long the documentation that we have access to, designates bits as reserved, it makes sense to have such a mask.
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).
o the user loses visibility into what is going on if they decide to drop/add sensitive bits such as LCRR_DBYP in their board's CONFIG_SYS_LCRR settings, and there's a mask lurking in the background.
o let's be practical here - in a board port, LCRR settings have to be paid attention to, no matter what hidden behaviours or new bits there are lying underneath - perhaps the form of 'protection' you seek is in the form of a comment in the code?
So what is it that you propose? That Heiko uses a LCRR in his board config (over-)writing reserved bits?
that's what other boards do (like the 8323erdb), but I do see the problem for the new board porter - they don't see the bit in their documentation, so they omit it.
but I presume the above fix will allow Heiko and other new board porters happy?
I'll send a patch out this weekend unless someone beats me to it.
Kim