On Tue, 20 May 2008 10:32:24 +0300 David Saada David.Saada@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