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

FENG Yu Ning fengyuning1984 at gmail.com
Wed Nov 26 04:51:16 CET 2008


On Wed, Nov 26, 2008 at 9:17 AM, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006 at gmx.net> wrote:
> Can you change the naming to "chunk"? "page" and "block" have meanings
> which do not match the code very well.

Yes.

> 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.

A (not so confident) suggestion: iff. we do not exactly know what is
the very thing to seperate chunk reads, what about still patch it(with
some additional comment) and wait for the bug to come out? At that
time, we know which chips do not like this and can solve the problem
effectively.

> 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




More information about the coreboot mailing list