Hi,
SPI flash on some chipsets (ICH9 and others) poses a real challenge: There are multiple write and erase commands, but each flash chip may support only a subset of them and each chipset may also only support a subset due to in-chipset command filtering. With our current approach (hardcoding erase commands etc.) a command may fail because it is not in the common subset although there would be an equivalent command in the common subset.
I'd like to introduce another field into struct flashchip: u32 flags. flags would hold a bit for every supported (and important) opcode and one bit for every variant of a supported opcode. Example follows:
#define SPIFLAG_CHIP_ERASE_60 (1<<0) #define SPIFLAG_CHIP_ERASE_C7 (1<<1) #define SPIFLAG_BLOCK_ERASE_52 (1<<2) #define SPIFLAG_BLOCK_ERASE_D8 (1<<3) #define SPIFLAG_SECTOR_ERASE_20 (1<<4) #define SPIFLAG_BYTE_PROGRAM_BYTE (1<<16) //only one byte per command #define SPIFLAG_BYTE_PROGRAM_PAGE (1<<17) //256 bytes per command
We could fill in this bitfield for chipset drivers as well and use only those commands/variants which are set in the bitwise AND of supported chip+chipset commands.
Regards, Carl-Daniel
On Sun, May 11, 2008 at 05:28:30PM +0200, Carl-Daniel Hailfinger wrote:
I'd like to introduce another field into struct flashchip: u32 flags.
The concept is completely neccessary, we have to keep track of more and more information as memory chips become more and more complex.
I'm afraid I'm not sure about this implementation thought. Specifically I'm hesitant about adding a new field that can not apply to all entries - that usually means the model isn't so nice.
I think I prefer a separate database for the more complex information that we need for the SPI chips.
//Peter
On 12.05.2008 04:56, Peter Stuge wrote:
On Sun, May 11, 2008 at 05:28:30PM +0200, Carl-Daniel Hailfinger wrote:
I'd like to introduce another field into struct flashchip: u32 flags.
The concept is completely neccessary, we have to keep track of more and more information as memory chips become more and more complex.
I'm afraid I'm not sure about this implementation thought. Specifically I'm hesitant about adding a new field that can not apply to all entries - that usually means the model isn't so nice.
Well, we could easily use the flags for parallel flash as well. A lot of our parallel flash chips have very similar write routines and we could use the flags two decide which variant to use.
I think I prefer a separate database for the more complex information that we need for the SPI chips.
Since this would be a per-chip database like the per-chip database we already have in flashchips.c, avoiding the problem of keeping both databases in sync would be really appreciated.
Regards, Carl-Daniel
On Sunday 11 May 2008 20:58, Carl-Daniel Hailfinger wrote:
I'd like to introduce another field into struct flashchip: u32 flags. flags would hold a bit for every supported (and important) opcode and one bit for every variant of a supported opcode. Example follows:
#define SPIFLAG_CHIP_ERASE_60 (1<<0) #define SPIFLAG_CHIP_ERASE_C7 (1<<1) #define SPIFLAG_BLOCK_ERASE_52 (1<<2) #define SPIFLAG_BLOCK_ERASE_D8 (1<<3) #define SPIFLAG_SECTOR_ERASE_20 (1<<4) #define SPIFLAG_BYTE_PROGRAM_BYTE (1<<16) //only one byte per command #define SPIFLAG_BYTE_PROGRAM_PAGE (1<<17) //256 bytes per command
AT45DB321D 32Mb (4MB) device has user configurable page size of 512 or 528 bytes.
On 12.05.2008 07:15, jtd wrote:
On Sunday 11 May 2008 20:58, Carl-Daniel Hailfinger wrote:
I'd like to introduce another field into struct flashchip: u32 flags. flags would hold a bit for every supported (and important) opcode and one bit for every variant of a supported opcode. Example follows:
#define SPIFLAG_CHIP_ERASE_60 (1<<0) #define SPIFLAG_CHIP_ERASE_C7 (1<<1) #define SPIFLAG_BLOCK_ERASE_52 (1<<2) #define SPIFLAG_BLOCK_ERASE_D8 (1<<3) #define SPIFLAG_SECTOR_ERASE_20 (1<<4) #define SPIFLAG_BYTE_PROGRAM_BYTE (1<<16) //only one byte per command #define SPIFLAG_BYTE_PROGRAM_PAGE (1<<17) //256 bytes per command
AT45DB321D 32Mb (4MB) device has user configurable page size of 512 or 528 bytes.
Hey, that's evil! I have read the complete data sheet and implementing a driver for this is going to be very painful (different command set, lots of conditional bahviour). It can be done, but I'd like to wait until we encounter such a chip on a mainboard. Thanks for pointing this out, though.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
jtd -
Peter Stuge