On Wed, Nov 26, 2008 at 11:51 AM, FENG Yu Ning fengyuning1984@gmail.com wrote:
On Wed, Nov 26, 2008 at 9:17 AM, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
I think I saw a data sheet which stated that you may not read arbitrarily large chunks of the chip. Although the current ICH SPI code is not matching the design of the parallel flash chip drivers, your change will effectively change the chunk size for reads.
We need to document it then. I wondered about it when reading the code, but I just could not figure out the reason. The original design is, after all, a loop inside another. Atomic operations (to the programmers) are reads of not more than 64 bytes, which, I think, have seperated them from each other. If the difference is some time delay between two chunk reads, it is better to delay it explicitly with a delay function.
I find some info which supports both your memory and my suggestion.
I read the data sheet of SST29{E,L,V}E020 (which we support) again, carefully. Here is some excerpt:
"The Write operation has three functional cycles: the Software Data Protection load sequence, the page-load cycle, and the internal Write cycle. ... "See Figures 5 and 6 for the Page-Write cycle timing diagrams. If after the completion of the three-byte SDP load sequence or the initial byte-load cycle, the host loads a second byte into the page buffer within a byte-load cycle time (TBLC) of 100 µs, the SST29EE/LE/VE020 will stay in the page-load cycle. Additional bytes are then loaded consecutively. The page-load cycle will be terminated if no additional byte is loaded into the page buffer within 200 µs (TBLCO) from the last byte-load cycle, i.e., no subsequent WE# or CE# high-to-low transition after the last rising edge of WE# or CE#. Data in the page buffer can be changed by a subsequent byte-load cycle. The page-load period can continue indefinitely, as long as the host continues to load the device within the byte-load cycle time of 100 µs. The page to be loaded is determined by the page address of the last byte loaded."
Here are the things I can think of now: 1. We need a "random-write-or-page-write" field in the flashchip struct, to indicate different types of chips. Again, a good name is needed. The erase-write function would have an if-branch to 2. 2. For page-write chips, we need page size and delay-between-page-write info in the flashchip struct.
There are some other architectural changes which look good at first, but they are unneeded. Look at ich_spi_read_block and tell me why you want block_size as a parameter. The block size should be stored in struct flashchip and not be passed around separately.
You are right. I have considered that, too. But, That could be a temporary solution. Let me explain a little further. At present, the flashchip structure change and this simplification is related to each other. I choose to divide them. Changing the flashchip structure affects more, therefore I deal with simplfication first. And this non-optimal design change is necessary to keep the code work while the flashchip structure stays unchanged. Later, we can also change the structure, without worrying about its effect to the ich spi operations. At some time we feel good(or not) about the structure, we can improve ich spi operations, making the design optimal.
I am not sure but changing the flashchip structure could be quite some work, is it?
What do you think?
yu ning