Hi,
On Fri, Aug 10, 2007 at 12:09:36AM +0200, popkonserve wrote:
the routine does the following:
- disable all caches
- set ram size to 0
- take one row and set its ram size to the maximum value and its type to
edo 4. write some data in the row 5. enable edo detection 6. read the data back 7. if the data read is the data written it is edo 8. if the data read is NOT the data written, write some more data 9. read the data back (without edo detection) 10. if the data read is the data written it is fpm 11. if the data read is NOT the data written, the row is empty 12. detect the real size of the row and the dram technology (i'll write some more about that tomorrow, sorry, it's late already :)) 13. clear all settings for the current row 14. restart at 3. as long as not all rows have been processed 15. write the size and type (edo/fpm) of all rows 16. turn the caches back on ;)
Please add this documentation to the source code as a (Doygen) comment.
/*----------------------------------------------------------------------------- EDO/FP/SDRAM detection routine. ----------------------------------------------------------------------------*/
/**
- Initializes a row before it can be checked for DRAM
*/ static void initialize_row(const struct mem_controller* ctrl, const uint8_t u8_row) {
Please don't encode type information in variable names. "row" is fine.
uint8_t u8_DRAMSize = 0; uint8_t u8_DRAMType = 1;
Use all-lowercase variable and function names, please. There are many other coding style issues in the code, please read
http://linuxbios.org/Development_Guidelines#Coding_Guidelines http://lxr.linux.no/source/Documentation/CodingStyle
and fix the code to use the Linux kernel coding style.
/* Initialize the current row to EDO and 32MB (maximum size) */ u8_DRAMType <<= u8_row; u8_DRAMSize == SIMM_ROW_SIZE_MAX / SIMM_GRANULARITY;
pci_write_config8(ctrl->d0, ((uint8_t)e_DRAM_ROW_BOUNDARY_BASE) + u8_row,
Is the (uint8_t) cast really needed? Otherwise please drop it.
u8_DRAMSize);pci_write_config8(ctrl->d0, ((uint8_t)e_DRAM_ROW_TYPE), u8_DRAMType); }
/**
- Clears a row after it has been checked for DRAM
*/ static void clear_row(const struct mem_controller* ctrl, const uint8_t u8_row) {
I think the 'const' for u8_row is not needed (not needed for integer types in general, I think).
/**
- Detects DRAM row size
*/ static void detect_size(const struct mem_controller* ctrl, uint8_t* au8_DRAMSize, const uint8_t u8_row) { uint32_t u32_AA0 = 0x0; uint32_t u32_AA8 = 1; uint32_t u32_DD1 = 0x55555555UL; uint32_t u32_DD2 = 0xAAAAAAAAUL; uint32_t u32_DD3 = 0xFEDCBA98UL; uint32_t u32_DD4 = 0; uint8_t u8_size_detected = 0; uint8_t u8_type = 0;
for(;;) { /* Write some data */ dimm_write32(u32_AA0, u32_DD1);
switch (au8_DRAMSize[u8_row]) { case e_4MB : {
[...]
case e_8MB : {
[...]
case e_16MB : {
The code here looks pretty similar, can it be generalized in a common function?
static struct S_DRAMTechnology s_DRAMT32MB[] = {
You can make this static const struct S_DRAMTechnology s_DRAMT32MB[] for structs with purely constant content.
{24, 22}, {23, 24} };
Uwe.