On Wed, 26 Aug 2009 08:28:37 +0200 Heiko Schocher <h...@denx.de> wrote:
> Hello Kim, > > Kim Phillips wrote: > > On Tue, 25 Aug 2009 13:31:34 +0200 > > Heiko Schocher <h...@denx.de> wrote: > >> + /* MPC8379E RM 10-34 says after writting this register > >> + * the register should be reread and an isync should be > >> + * executed. > >> + */ > >> + in_be32(&im->lbus.lcrr); > >> + isync(); > > > > in_be32 and friends does the isync for you. In fact, you can probably > > do it in one fell swoop by using setbits/clrsetbits? > > Argh, of course. But I need the in_be32() for rereading the register > again, as the RM says. right on > >> +++ b/include/mpc83xx.h > >> @@ -198,6 +198,7 @@ > >> #define SICRL_URT_CTPR 0x06000000 > >> #define SICRL_IRQ_CTPR 0x00C00000 > >> > >> +#define LCRR_MASK 0x0003000f > > > > I thought we discussed this - shouldn't this be 0x7fffffff? > > Hmm.. as I did this mpc832x specific, in my version it is not possible > to set reserved 0 bits to 1 ... Ah, I reread your mail again, you wrote: > > I guess I have a shoot-yourself-in-the-foot philosophy - you're free to > > find out what happens when setting reserved bits to 1 if you so wish. > > u-boot protects you up to the point where you veer off into using > > hardcoded values instead of using the predefined CONFIG_SYS_SCCR_* > > macros. > > I think you mean the LCRR_* defines ... yes, thank you > Hmm... so we can say: "feel free to find out what happens, if setting > reserved 1 bits to 0! and can drop this patch" ... precisely - I'd rather have this kept in the board config file if at all possible - that's how other 83xx boards do it atm. > Hmmm, you can use for the mpc832x for example the LCRR_BUFCMDC_1, > what is a valid define for mpc83xx, but it is not valid for the > mpc832x ... so It is not the problem, that a u-boot user use hard- > coded values, instead this processor don;t support all bits valid > for other mpc83xx processors. > > So I tend to protect an u-boot user from doing wrong things, > (setting reserved 0/1 bits to 1/0) if it is easy possible ... right but: 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. 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? Kim _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
