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

[Wolfgang Grandegger]

Sebastian Siewior wrote:
> * 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?

The port for Socrates and TQM8548 have been developped concarently and I 
  was therefore not aware of an upmconfig for MPC85xx. TQM85xx modules 
use UPMC for CAN and the TQM8548 uses UPMB for NAND.

>>> 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?

For the time being, I prefer to keep upmconfig() compatible with the 
others similar implementation:

   $ find . -type f | xargs grep upmconfig
   ./mpc83xx/cpu.c:void upmconfig (uint upm, uint *table, uint size)
   ./mpc8260/cpu.c:void upmconfig (uint upm, uint * table, uint size)
   ./mpc8xx/cpu.c:void upmconfig (uint upm, uint * table, uint size)
   ./mpc85xx/cpu.c:void upmconfig (uint upm, uint * table, uint size)

mxmr should be defined in the platform specific code, as currently all 
boards using upmconfig() do, if necessary. If we unify them, we have 
more choices.

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

See http://lxr.linux.no/linux+v2.6.26/drivers/mtd/nand/fsl_upm.c. A 
similar implementation exists for U-Boot in drivers/mtd/nand/fsl_upm.c,
which is used by the TQM8548, for example.

Wolfgang.