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

[Wolfgang Grandegger <wg@grandegger.com>]

Sebastian Siewior wrote:
> * Andy Fleming | 2008-07-14 19:27:08 [-0500]:
> 
>> On Mon, Jul 14, 2008 at 5:54 AM, Sebastian Siewior
>> <bigeasy@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@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.

> Another sollution might be to add upconfig_mxmr() and let upmconfig call
> that function with the default value however this sounds nasty.
> 
>> Could you also explain in greater detail what you are trying to do,
>> here?  My familiarity with the LBC code is fairly low.
> The MxMR register specifies the mode in which the UPM is used. I have to
> program two UPMs with different content of this register: in one case I
> have to set UWPL & GPL4 bits in the other case the default value would
> do).
> 
> 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);


I could test the new patch on the TQM8548 and Socrates board.

BTW: upmconfig() should also work as is on the MPC83xx and MPC86xx, IIRC 
but it's not obvious where to put the code if we go for a more generic 
FSL UPM solution.

Wolfgang.