[PATCH] edac: mpc85xx: add support for mpc83xx memory controller

Ira W. Snyder iws at ovro.caltech.edu
Fri Jul 10 07:38:33 EST 2009


On Thu, Jul 09, 2009 at 03:15:29PM -0500, Kumar Gala wrote:
>
> On Jul 9, 2009, at 2:35 PM, Ira W. Snyder wrote:
>
>> On Thu, Jul 09, 2009 at 01:25:43PM -0500, Kumar Gala wrote:
>>>
>>> On Jul 9, 2009, at 1:17 PM, Ira W. Snyder wrote:
>>>
>>>> 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.)
>>>
>>> My math was wrong it should be ( <<= (24 - PAGE_SHIFT) )
>>>
>>> With that I think things work out.
>>>
>>
>> Yep, that works out great. This solution is much better than my  
>> original
>> code. The 83xx doesn't need to be special-cased anymore.
>>
>> I checked the math for a 85xx with 64GB of memory. Assuming it uses  
>> 64K
>> pages (PAGE_SHIFT == 16), then everything works out.
>
> Does the math work for a 64GB system w/4K pages?
>

Yes, the math works there too. I just double-checked by hand. The v2
patch should work fine on both the 83xx and 85xx (both 4K and 64K
pages).

Thanks again for the help on this one.
Ira


More information about the Linuxppc-dev mailing list