[U-Boot-Users] [PATCH] mpc85xx: make the MxMR register in upmconfig as a parameter

[Sebastian Siewior]

* Wolfgang Grandegger | 2008-07-15 13:28:40 [+0200]:

>Sebastian Siewior wrote:
>>* Andy Fleming | 2008-07-14 19:27:08 [-0500]:
>>
>>>On Mon, Jul 14, 2008 at 5:54 AM, Sebastian Siewior
>>><bigeasy at linutronix.de> wrote:
>>>>The default value for the MxMR register is not always the right one.
>>>>This patch adds the value of MxMR register as an additional
>>>>parameter (plus a few defines instead of hex coded values).
>>>>
>>>>Signed-off-by: Sebastian Siewior <bigeasy at linutronix.de>
>>>
>>>I'm not convinced this is the right solution.  Anytime we put a
>>>cpu-specific #ifdef for a function definition, we should think long
>>>and hard about why.  Maybe instead of an argument, we should make
>>>mxmr_mode a config value.  Also, unless I'm misreading this patch,
>>>you've broken *every* board with this patch, since there's no change
>>>to any of the invokers of upmconfig to supply the fourth argument.
>>Other sollutions are fine with me :) I did not change any board specific
>>code, because I did not find any users (with 85xx cpus). Still possible
>>that I missed some....
>
>upmconfig was introduced with the socrates board but our internal 
>version now also uses a configurable MXMR value. The TQM85xx boards and 
>likely more should be converted as well.

Socrates uses upmconfig, TQM85xx uses its own implementation. Why
TQM85xx boards? There is only one, isn't it?

>>If you prefer a define for this register the way we deal with BRx/ORx
>>than I could send a patch that does this. What should be done in case
>>the user is going to program UPMA but did not specify MAMR? The default
>>value or build error? Since this value as well as the UPM tables depend
>>very much on the hardware, the user should been told by his hardware
>>guys what to do :)
>
>I think upmconfig() should only touch the relevant bits of the MxMR 
>register. For the TQM8548 NAND support (see board/tqc/tqm85xx/nand.c) I 
>implemented it as Linux does:
>
>        clrsetbits_be32(&lbc->mbmr, MxMR_MAD_MSK,
>                        MxMR_OP_WARR | (addr & MxMR_MAD_MSK));
>
>        out_be32 (&lbc->mdr, val);
>
>        /* dummy access to perform write */
>        out_8 ((void __iomem *)CFG_NAND0_BASE, 0);
>
>        clrbits_be32(&lbc->mbmr, MxMR_OP_WARR);
>
Okey, so you prefer to clear only MAD bits and MODE bits and leave
everythign else untouched. And where should de define them? In
cpu/mpc85xx/cpu_init.c right after the OR/BR settings?

Why does Linux program UPMs? Isn't this one-time-setup?

>Wolfgang.

Sebastian