On Wed, Jul 08, 2009 at 01:09:39PM -0500, Kumar Gala wrote:
>
> On Jul 8, 2009, at 11:19 AM, Ira W. Snyder wrote:
>
>> Add support for the Freescale MPC83xx memory controller to the
>> existing
>> driver for the Freescale MPC85xx memory controller. The only
>> difference
>> between the two processors are in the CS_BNDS register parsing code.
>>
>> The L2 cache controller does not exist on the MPC83xx, but the OF
>> subsystem
>> will not use the driver if the device is not present in the OF device
>> tree.
>>
>> Signed-off-by: Ira W. Snyder <i...@ovro.caltech.edu>
>> ---
>>
>> This was tested on an MPC8349EMDS-based board.
>>
>> I don't really like how the MCE disable works on mpc85xx (clearing the
>> HID1 register), but this can be revisited when mpc86xx support gets
>> added. It sucks to have this happen before the probe() routine is
>> called
>> and we know what kind of hardware we're actually running on.
>>
>> drivers/edac/Kconfig | 6 +++---
>> drivers/edac/mpc85xx_edac.c | 38 +++++++++++++++++++++++++++
>> +----------
>> drivers/edac/mpc85xx_edac.h | 3 +++
>> 3 files changed, 34 insertions(+), 13 deletions(-)
>
> [snip]
>
>> /************************ MC SYSFS parts
>> ***********************************/
>>
>> @@ -738,7 +740,8 @@ static irqreturn_t mpc85xx_mc_isr(int irq, void
>> *dev_id)
>> return IRQ_HANDLED;
>> }
>>
>> -static void __devinit mpc85xx_init_csrows(struct mem_ctl_info *mci)
>> +static void __devinit mpc85xx_init_csrows(struct mem_ctl_info *mci,
>> + const struct of_device_id *match)
>> {
>> struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
>> struct csrow_info *csrow;
>> @@ -784,18 +787,26 @@ static void __devinit mpc85xx_init_csrows(struct
>> mem_ctl_info *mci)
>> }
>>
>> for (index = 0; index < mci->nr_csrows; index++) {
>> - u32 start;
>> - u32 end;
>> + u32 start, end, extra;
>>
>> csrow = &mci->csrows[index];
>> cs_bnds = in_be32(pdata->mc_vbase + MPC85XX_MC_CS_BNDS_0 +
>> (index * MPC85XX_MC_CS_BNDS_OFS));
>> - start = (cs_bnds & 0xfff0000) << 4;
>> - end = ((cs_bnds & 0xfff) << 20);
>> +
>> + if (match->data && match->data == MPC83xx) {
>> + start = (cs_bnds & 0x00ff0000) << 8;
>> + end = (cs_bnds & 0x000000ff) << 24;
>> + extra = 0x00ffffff;
>> + } else {
>> + start = (cs_bnds & 0x0fff0000) << 4;
>> + end = (cs_bnds & 0x00000fff) << 20;
>> + extra = 0x000fffff;
>> + }
>
> I don't understand this at all.. The only difference between 83xx & 85xx
> is that we should have an extra 4-bits for 36-bit physical addresses.
>
> We should be able to write this code in such a way that it works for
> both 83xx & 85xx.
>
> start = (cs_bnds & 0xffff0000) >> 16;
> end = (cs_bnds & 0xffff);
>
> if (start == end)
> continue;
> start <<= (20 - PAGE_SHIFT);
> end <<= (20 - PAGE_SHIFT);
> end |= (1 << (20 - PAGE_SHIFT)) - 1;
>
Sorry, I don't know a thing about PAGE_SHIFT. Looking in
arch/powerpc/platforms/Kconfig.cputype, PPC_85xx doesn't seem to imply a
change in PAGE_SIZE, which changes the PAGE_SHIFT in
arch/powerpc/include/asm/page.h.
Also, are you sure you cannot use 4K pages on an 85xx? If you can use 4K
pages on 85xx, then PAGE_SHIFT == 12, and your solution breaks.
I admit I don't know much about all of the different powerpc platforms.
Ira
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
