[U-Boot-Users] [PATCH][resubmit] MPC85xx, MPC83xx: Add/Fix UPM configuration support

[Kim Phillips]

On Tue, 20 May 2008 10:32:24 +0300
David Saada <David.Saada at ecitele.com> wrote:

> 
> > Can we merge the register definition files and have a single upm_config
> > for mpc8[356]xx instead of duplicating it as being done here?
> 
> Not sure that I'm the right person for this job, as I don't have an 86xx based board.

86xx can be tested by others; point here is to avoid code duplication.

> Also, I'm not sure this would be a good idea: There are some
> differences between the registers in these 3 families, and the header
> file would look pretty ugly (many ifdefs). Even the UPM code won't be

the registers have a common base, and all the registers this patch uses
are the same on all of them.  I don't see why they can't be unified
into a common include/asm-ppc/immap_8xxx.h file.

> the same: for instance, the mpc85xx code scans the BR registers (which
> are adjacent) to determine the UPM machine, while in the 83xx we need
> to scan the lbus banks (and extract the BRs from there). This means
> that we will also have many ifdefs there as well.

afaict, the code this patch introduces for 85xx is functionally
identical to that of 83xx:

  * Program a UPM with the code supplied in the table.
  *
  * The 'dummy' variable is used to increment the MAD. 'dummy' is
@@ -133,7 +13,7 @@
  *
  * The value can be extracted from the base address bits of the
  * Base Register (BR) associated with the specific UPM.  To find
- * that BR, we need to scan all 8 BRs until we find the one that
+ * that BR, we need to scan all BRs until we find the one that
  * has its MSEL bits matching the UPM we want.  Once we know the
  * right BR, we can extract the base address bits from it.
  *
@@ -147,20 +27,20 @@
  */
 void upmconfig (uint upm, uint *table, uint size)
 {
-	volatile immap_t *immap = (immap_t *) CFG_IMMR;
-	volatile lbus83xx_t *lbus = &immap->lbus;
+    volatile ccsr_lbc_t *lbc = (void *)(CFG_MPC85xx_LBC_ADDR);
 	volatile uchar *dummy = NULL;
-	const u32 msel = (upm + 4) << BR_MSEL_SHIFT;	/* What
the MSEL field in BRn should be */
-	volatile u32 *mxmr = &lbus->mamr + upm;	/* Pointer to
mamr, mbmr, or mcmr */
+       const u32 msel = (upm + 4) << BRx_MS_SHIFT; /* What the MSEL
field in BRn should be */
+       volatile uint *mxmr = &lbc->mamr + upm; /* Pointer to mamr,
mbmr, or mcmr */
+       volatile uint *brx = &lbc->br0; /* Pointer to BRx */
 	uint i;
 
 	/* Scan all the banks to determine the base address of the
device */ for (i = 0; i < 8; i++) {
-		if ((lbus->bank[i].br & BR_MSEL) == msel) {
-			dummy = (uchar *) (lbus->bank[i].br & BR_BA);
+               if ((*brx & BRx_MS_MSK) == msel) {
+                       dummy = (uchar *) (*brx & BRx_BA_MSK);
 			break;
 		}
+               brx += 2; /* Skip to next BRx */
 	}
 
 	if (!dummy) {
@@ -169,10 +49,10 @@
 	}
 
 	/* Set the OP field in the MxMR to "write" and the MAD field
to 000000 */
-	*mxmr = (*mxmr & 0xCFFFFFC0) | 0x10000000;
+       *mxmr = (*mxmr & 0xCFFFFFC0) | MxMR_OP_WARR;
 
 	for (i = 0; i < size; i++) {
-		lbus->mdr = table[i];
+               lbc->mdr = table[i];
 		__asm__ __volatile__ ("sync");
 		*dummy;	/* Write the value to memory and
increment MAD */ __asm__ __volatile__ ("sync");

btw, I don't see arch/powerpc/sysdev/fsl_lbc.c functionally differing
among the families either.

am I missing something?

> David.

Kim