From: "Ira W. Snyder" <iws@ovro•caltech.edu>
To: Kumar Gala <galak@kernel•crashing.org>
Cc: linuxppc-dev@ozlabs•org, bluesmoke-devel@lists•sourceforge.net,
Dave Jiang <djiang@mvista•com>
Subject: Re: [PATCH] edac: mpc85xx: add support for mpc83xx memory controller
Date: Wed, 8 Jul 2009 12:33:55 -0700 [thread overview]
Message-ID: <20090708193355.GE2827@ovro.caltech.edu> (raw)
In-Reply-To: <3887C2A7-3566-43A9-A863-4670B039E5DA@kernel.crashing.org>
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 <iws@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
next prev parent reply other threads:[~2009-07-08 19:33 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-08 16:19 [PATCH] edac: mpc85xx: add support for mpc83xx memory controller Ira W. Snyder
2009-07-08 18:09 ` Kumar Gala
2009-07-08 19:33 ` Ira W. Snyder [this message]
2009-07-08 20:58 ` Kumar Gala
2009-07-09 16:58 ` Ira W. Snyder
2009-07-09 17:58 ` Kumar Gala
2009-07-09 18:17 ` Ira W. Snyder
2009-07-09 18:25 ` Kumar Gala
2009-07-09 19:35 ` Ira W. Snyder
2009-07-09 20:15 ` Kumar Gala
2009-07-09 21:38 ` Ira W. Snyder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090708193355.GE2827@ovro.caltech.edu \
--to=iws@ovro$(echo .)caltech.edu \
--cc=bluesmoke-devel@lists$(echo .)sourceforge.net \
--cc=djiang@mvista$(echo .)com \
--cc=galak@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox