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: Thu, 9 Jul 2009 11:17:05 -0700 [thread overview]
Message-ID: <20090709181705.GD29383@ovro.caltech.edu> (raw)
In-Reply-To: <FC7C9C56-A1E7-4A02-BDEA-F55E10567313@kernel.crashing.org>
On Thu, Jul 09, 2009 at 12:58:53PM -0500, Kumar Gala wrote:
>> Hello Kumar,
>>
>> I must not understand something going on here. Your proposed code
>> doesn't work at all on my board. The
>> /sys/devices/system/edac/mc/mc0/size_mb doesn't come out correctly.
>
> What does it come out as? How much memory do you have in the system?
>
The size_mb shows as 0 with your code. See the explanation below. With
my code it shows as 256MB, as expected.
I have 256MB memory in the system.
>> The attached patch DOES work on my board, but I'm confident that it
>> does
>> NOT work on a system with PAGE_SIZE != 4096. Any idea what I did
>> wrong?
>>
>> If I'm reading things correctly:
>> csrow->first_page full address of the first page (NOT pfn)
>> csrow->last_page full address of the last page (NOT pfn)
>> csrow->nr_pages number of pages
>>
>> The EDAC subsystem does csrow->nr_pages * PAGE_SIZE to get the size_mb
>> sysfs value.
>>
>> If csrow->first_page and csrow->last_page ARE supposed to be the pfn,
>> then I think the original code got it wrong, and the calculation for
>> csrow->nr_pages needs to be changed.
>>
>> Thanks,
>> Ira
>
> [snip]
>
>> /************************ MC SYSFS parts
>> ***********************************/
>>
>> @@ -790,18 +792,19 @@ static void __devinit mpc85xx_init_csrows(struct
>> mem_ctl_info *mci)
>> 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 (start)
>> - start |= 0xfffff;
>> - if (end)
>> - end |= 0xfffff;
>
> can you printk what cs_bnds values are in your setup.
>
I am only using a single chip select. CS0_BNDS (register 0xe0002000) is
0x0000000F.
>> +
>> + start = (cs_bnds & 0xffff0000) >> 16;
>> + end = (cs_bnds & 0x0000ffff);
>>
This is the same in both our versions.
start == 0x0
end == 0xF
>> if (start == end)
>> continue; /* not populated */
>>
>> - csrow->first_page = start >> PAGE_SHIFT;
>> - csrow->last_page = end >> PAGE_SHIFT;
>> + start <<= PAGE_SHIFT;
>> + end <<= PAGE_SHIFT;
>> + end |= (1 << PAGE_SHIFT) - 1;
>> +
MY VERSION
start == 0x0
end == 0xffff
first_page == 0x0
last_page == 0xffff
YOUR VERSION (<<= (20 - PAGE_SHIFT), etc.)
start == 0x0
end == 0xfff
first_page == 0x0
last_page == 0x0
>> + csrow->first_page = start;
>> + csrow->last_page = end;
>
> This seems odd to me... I can't believe this is working out properly.
>
>>
>> csrow->nr_pages = csrow->last_page + 1 - csrow->first_page;
The calculation is unchanged here from the original code. Due to the
">> PAGE_SHIFT", nr_pages ends up as 1 in your version.
MY VERSION
nr_pages == 0xffff + 1 - 0 == 0x10000
0x10000 * 4096 / 1024 / 1024 == 256 MB
YOUR VERSION
nr_pages == 0x0 + 1 - 0x0 == 1
0x1 * 4096 / 1024 / 1024 == 0MB
>> csrow->grain = 8;
>> csrow->mtype = mtype;
>>
>
> Lets get some real values on the table for your system so I can get a
> sense of what's really going on.
>
Thanks for the help.
Ira
next prev parent reply other threads:[~2009-07-09 18:17 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
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 [this message]
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=20090709181705.GD29383@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