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@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.
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.