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 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
