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. I'll submit a new patch now. Thanks for the help, Ira _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
