On Wed, Feb 27, 2008 at 7:16 AM, <joe@smittys.pointclark.net> wrote:
Quoting Corey Osgood <corey.osgood@gmail.com>:

> On Tue, 2008-02-26 at 21:33 -0500, joe@smittys.pointclark.net wrote:
>> Quoting joe@smittys.pointclark.net:
>>
>> > Quoting Corey Osgood <corey.osgood@gmail.com>:
>> >
>> >>
>> >> On Tue, 2008-02-26 at 16:19 -0500, joe@smittys.pointclark.net wrote:
>> >>> >> +static void do_ram_command(const struct mem_controller *ctrl,
>> >>> >> uint32_t command, uint32_t addr_offset)
>> >>> >> +{
>> >>> >> <snip>
>> >>> >> +
>> >>> >> +     /* Read from (DIMM start address + addr_offset). */
>> >>> >> +     read32(0 + addr_offset);        //first offset is always 0
>> >>> >> +}
>> >>> >
>> >>> > This isn't ready for multiple dimms yet. See the cn700 patch I
>> >>> > recently sent (but haven't committed yet, I think it was acked).
>> >>> >
>> >>> Oh you mean this? What do I need to do to adapt it for the i82830?
>> >>>
>> >>> +        /* NOTE: Dual-sided ready */
>> >>>          read32(0 + addr_offset);
>> >>> +        for(i = 0; i < (ARRAY_SIZE(ctrl->channel0) * 2); i++) {
>> >>> +                reg8 = pci_read_config8(ctrl->d0f3, 0x40 + i);
>> >>> +        if(reg8) read32((reg8 << 26) + addr_offset);
>> >>> + }
>> >>
>> >> This reads from register 0x40 + i, where 0x40 is the top of the first
>> >> dimm, and i counts to the max number of dimms.
>> >
>> > Why read this from a register?? Why not just use (i = 0; i <
>> > DIMM_SOCKETS; i++)
>
> ARRAY_SIZE(ctrl->channel0) is the number of spd addresses, which can be
> less than the max number of dimms.
>
>> >
>> >> Reg 0x40 contains bits
>> >> 33:26 of the top address, hence the << 26. So you'd need to adjust both
>> >> of these to fit the i830's DRB mechanism. If the i830 uses an ugly
>> >> format like the i810 does, then it may be easier to store the size in
>> >> scratch registers somewhere after it's calculated, and then use those
>> >> values instead. Sorry I can't get more specific, I've got to head out to
>> >> my next service call.
>> >>
>> > The DRB i830 is not ugly it is very simple, simple, simple. So simple
>> > that it just calculates the memory size in ticks of 32 per side
>> (Sorry typo I meant) +
>> > the next side up to 4 sides. Like this:
>> >
>> > Example: 128MB SINGLE sided so-dimm in slot 1
>> >           128MB DOUBLE sided so-dimm in slot 2
>> >
>> > There are 4 DRB registers(actually 6 but but only 4 that are usable -
>> > Intel design glitch), 1 for each side. So in this example they would
>> > look like this:
>> >
>> > DRB1 = 0x04
>> > DRB2 = 0x04
>> > DRB3 = 0x06
>> > DRB4 = 0x08
>> >
>> > So if we needed to convert this we would do something like this (DRB *
>> > 32) * 1024 correct? But if we just need to calculate the end of
>> > so-dimm #1 and the start of so-dimm #2 the only important register
>> > would be DRB2 (side 2 of so-dimm #1) and DRB3(side 1 of so-dimm #2),
>> > correct?
>
> The modified version would be something like
> read32(0 + addr_offset);
> for(i = 0; i < ARRAY_SIZE(ctrl->channel0); i++) {
>       reg8 = pci_read_config8(ctrl->d0/*?*/, DRB1 + i);
>       read32(reg8 * 32);
> }
>
Ok starting to make sense but what is all this about? /*?*/ wouldn't
it just be reg8 = pci_read_config8(ctrl->d0, DRB1 = i);

DRB1 + i, but yes. The /*?*/ is an inline comment, because I wasn't sure if it was actually device 0 function 0 that had the memory registers.
 
This is going to show our results in MB, is that what we want? For
accuracy shouldn't it be in bytes? Sorry for so many questions, I just
want to get this right.

Right again, it would need to be read32(reg8 * 32 * 1024 * 1024) (If I'm thinking correctly).

-Corey
 



Thanks - Joe