[LinuxBIOS] RAM detection for Intel 430FX

[Peter Stuge]

On Fri, Aug 10, 2007 at 12:09:36AM +0200, popkonserve wrote:
> /*
>  * This file is part of the LinuxBIOS project.
>  *
>  * Copyright (C)

Please add 2007 Holger Yourlastname


> static void initialize_row(const struct mem_controller* ctrl,
>                            const uint8_t u8_row) {
> 
>   uint8_t u8_DRAMSize = 0;
>   uint8_t u8_DRAMType = 1;
> 
>   /* Initialize the current row to EDO and 32MB (maximum size) */
>   u8_DRAMType <<= u8_row;
>   u8_DRAMSize == SIMM_ROW_SIZE_MAX / SIMM_GRANULARITY;

Typo == instead of = ?

>   pci_write_config8(ctrl->d0,
>                     ((uint8_t)e_DRAM_ROW_BOUNDARY_BASE) + u8_row,
>                     u8_DRAMSize);

..will always write 0 otherwise.


>   pci_write_config8(ctrl->d0,
>                     ((uint8_t)e_DRAM_ROW_TYPE),
>                     u8_DRAMType);

Does this want to be a set_row_type() function?


>   /* Enable EDO detection mode */
>   u8_DRAMC = pci_read_config8(ctrl->d0,
>                               ((uint8_t)e_DRAM_CONTROL));
> 
>   u8_DRAMC |= ((uint8_t)e_DRAMC_EDO_DETECT);
>   pci_write_config8(ctrl->d0,
>                     ((uint8_t)e_DRAM_CONTROL),
>                     u8_DRAMC);

Maybe also candidate for a separate function?


> static void detect_size(const struct mem_controller* ctrl,

[..]

>   for(;;) {

[..]

>     /* Quit if the size of the current row was detected or an error occured */
>     if((u8_size_detected != 0) ||
>         au8_DRAMSize[u8_row] == ((uint8_t)e_EMPTY)){
>       break;
>     }
>   }

Please change the for() into do {} while(); to improve readability.


>   /* Turn off cache (L1 & L2) */

Isn't there a function for this already? If not, should there be?


>   /* Set size of DRAM to 0 for all rows */
>   for(u8_row = 0; u8_row < SIMM_SOCKETS; u8_row++) {
>     au8_DRAMSize[u8_row] = 0;
>     pci_write_config8(ctrl->d0,
>                       ((uint8_t)e_DRAM_ROW_BOUNDARY_BASE) + u8_row,
>                       au8_DRAMSize);
>   }
> 
>   /* Detect RAM presence and type in all rows */
>   for(u8_row = 0; u8_row < SIMM_SOCKETS; u8_row++) {
> 

All rows need to be zeroed first to make this reliable?


>     /* Detect type of ram in the current row */
>     detect_type(ctrl,
>                 au8_DRAMSize,
>                 &u8_DRAMType,
>                 u8_row);
> 
>     /* Check if any DRAM was found in this row */
>     if(au8_DRAMSize[u8_row] == 0) {
>       /* No DRAM found. Skip the rest. */
>       continue;
>     }

A bit unexpected to have detect_type() return meaningful data in a
Size parameter.

Maybe a return value from detect_type() would be nicer?


Looks good! :)


//Peter