Re: [LinuxBIOS] do_ram_command

[Corey Osgood]

Joseph Smith wrote:
> Quoting Corey Osgood :
> > Joseph Smith wrote:
> > > Hello,
> > > Corey or Uwe maybe you can help me with this. Does this look 
> > > workable to you?
> > > I'm just not sure with all this bit shifting it is going to return the
> > > correct value. The DRC register has seperate bits for the commands and
> > > the refreshes.
> > >
> > > /* DRC[10:8] - Refresh Mode Select (RMS). */
> > > #define RAM_COMMAND_REFRESH_1     0x1 /* Refresh interval 15.6 us
> > > for 133MHz */
> > > #define RAM_COMMAND_REFRESH_2     0x2 /* Refresh interval 7.8 us for
> > > 133MHz */
> > > #define RAM_COMMAND_REFRESH_3  0x7 /* Refresh interval 128 Clocks.
> > > (Fast Refresh Mode) */
> > >
> > > /* DRC[6:4] - SDRAM Mode Select (SMS). */
> > > #define RAM_COMMAND_NOP         0x1
> > > #define RAM_COMMAND_PRECHARGE     0x2
> > > #define RAM_COMMAND_MRS         0x3
> > > #define RAM_COMMAND_CBR         0x6
> > > #define RAM_COMMAND_NORMAL     0x7
> > >
> > > static void do_ram_command(const struct mem_controller *ctrl, uint32_t
> > > command,
> > >                uint32_t addr_offset, uint32_t row_offset)
> > > {
> > > uint32_t reg;
> > >
> > > /* TODO: Support for multiple DIMMs. */
> > >
> > > /* Configure the RAM command. */
> > > reg = pci_read_config32(ctrl->d0, DRC);
> > > reg &= 0x0;        /* Clear all bits to default: 00000000h. */
> >
> > you want to make this 0xdffff00f or something similar. That way, you're
> > only clearing the bits that you're not affecting, especially if they're
> > marked reserved. Other than that, looks good, but I've forgotten which
> > northbridge this is for so I haven't checked the datasheet.
> >
> > -Corey
>
> Little bit confused by the bit clearing?? So I want to turn only the
> bits I want to clear to 0 zero correct?? But doen't this turn the
> remainder of the bits to ones also?? If so that would be bad.

No. The binary and is kinda weird, if you have, say, 11100011 and
01010101, the binary and will give an answer of which bits are the same
between the two, so 01000001. I wish I could explain it better, check
out binary operands on wikipedia.
> > > reg |= command << 4;
> > >    /* If RAM_COMMAND_NORMAL set the refresh mode and IC bit 29. */
> > > if (command == RAM_COMMAND_NORMAL) {
> > > reg |= ((RAM_COMMAND_REFRESH_1 << 8) | (1 << 29));
> > >    }
> > > pci_write_config32(ctrl->d0, DRC, reg);
> > >
> > > /* RAM_COMMAND_NORMAL affects only the memory controller and
> > >    doesn't need to be "sent" to the DIMMs. */
> > > /* if (command == RAM_COMMAND_NORMAL) return; */
> > >
> > > PRINT_DEBUG("    Sending RAM command 0x");
> > > PRINT_DEBUG_HEX32(reg);
> > > PRINT_DEBUG(" to 0x");
> > > PRINT_DEBUG_HEX32(0 + addr_offset); // FIXME
> > > PRINT_DEBUG("\r\n");
> > >
> > > /* Read from (DIMM start address + addr_offset). */
> > > read32(0 + addr_offset);    //first offset is always 0
> > > read32(row_offset + addr_offset);
> > >
> > > }
> > >
> > > Thanks - Joe
>
> This is for the i82830 chipset.
> Let me be more specific. I think I make bit shifting more confusing
> than it is, especially if the same value gets shifted more than once.
> If I use the RAM_COMMAND_NORMAL for command.
>
> #define RAM_COMMAND_NORMAL         0x7 ---(binary that?s 0111)
> #define RAM_COMMAND_REFRESH_1         0x1 ---(binary that?s 0001)
>
> And then
>
> reg |= command << 4;   ---(binary that would make 01110000)
>
> And Then
>
> reg |= ((RAM_COMMAND_REFRESH_1 << 8) | (1 << 29));
> ---(in binary that would turn reg into 000101110000)
> ---(in binary that would turn reg into 00100000000000000000000101110000)
>
> Is 00100000000000000000000101110000 the correct value that would get
> returned??
>
> Thanks - Joe

Yeah, looks all good to me.

-Corey