[coreboot] flashrom: simplify ich spi read/write operations

[FENG Yu Ning]

On Wed, Nov 26, 2008 at 11:51 AM, FENG Yu Ning <fengyuning1984 at gmail.com> wrote:
> On Wed, Nov 26, 2008 at 9:17 AM, Carl-Daniel Hailfinger
> <c-d.hailfinger.devel.2006 at 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
>