[flashrom] [PATCH] Refine granularity handling in preparation of AT45DB series support.
Mark Marshall
markmarshall14 at gmail.com
Thu Mar 28 11:41:21 CET 2013
Hi.
I'm currently struggling to understand what the write granularity is for
and how it
interacts with the page size?
I'm also a little confused by flashrom's use of page size?
I am mainly interested in SPI flash chips. For most SPI chips the "page
size" is 256 bytes.
I have a chip where the page size is 512 bytes.
The "page size" is the amount that writes using the PP command have to be
chunked by.
So "spi_nbyte_program" must be used to write no more than "page size" bytes
and must
not cross a "page size" boundary.
So far this all makes sense, apart from the "spi_chip_write_256" function,
which should
be named "spi_chip_write_page"? (as all chips currently have a page size
of 256 this is
correct at the moment, but I'm about to add one that doesn't).
I think that the "chunksize" parameter in spi_write_chunked is to do with
possible restrictions
of some programmer hardware? This also makes sense, apart from all of the
comments which
are very out of date?
According to the manuals of the SPI Flash chips I've looked at, when using
the PP command
you can program any part of a page, and the correct thing will happen (1's
can only be written
to 0's). So I don't see how this matches up with "write granularity",
which claims that the other
bytes in the granularity region will be "undefined"?
Is write granularity for something else?
Is there a post somewhere that I missed that explains all of this?
Thanks for any help,
Mark M.
On 14 March 2013 12:13, Stefan Tauner <stefan.tauner at student.tuwien.ac.at>wrote:
> This adds a number of new granularities in a rather inelegant way.
> Until we figure out how to better handle various granularities this has
> to be accepted.
>
> It also refactors the handling of n-byte granularities by extracting the
> checking code into a helper function which reduces the pain of the above
> significantly.
>
> Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
> ---
> This is one possibility to solve the granularity issue and i think the
> most likely to get the patch series in fastest...
>
> flash.h | 24 +++++++++++++----------
> flashrom.c | 64
> ++++++++++++++++++++++++++++++++++++++++++++++--------------
> 2 files changed, 63 insertions(+), 25 deletions(-)
>
> diff --git a/flash.h b/flash.h
> index 9c4b0ac..845d898 100644
> --- a/flash.h
> +++ b/flash.h
> @@ -59,18 +59,22 @@ enum chipbustype {
> };
>
> /*
> - * The following write granularities are known:
> - * - 1 bit: Each bit can be cleared individually.
> - * - 1 byte: A byte can be written once. Further writes to an already
> written byte cause its contents to be
> - * either undefined or to stay unchanged.
> - * - 128 bytes: If less than 128 bytes are written, the rest will be
> erased. Each write to a 128-byte region
> - * will trigger an automatic erase before anything is written. Very
> uncommon behaviour.
> - * - 256 bytes: If less than 256 bytes are written, the contents of the
> unwritten bytes are undefined.
> + * The following enum defines possible write granularities of flash
> chips. These tend to reflect the properties
> + * of the actual hardware not necesserily the write function(s) defined
> by the respective struct flashchip.
> + * The latter might (and should) be more precisely specified, e.g. they
> might bail out early if their execution
> + * would result in undefined chip contents.
> */
> enum write_granularity {
> - write_gran_256bytes = 0, /* We assume 256 byte granularity by
> default. */
> - write_gran_1bit,
> - write_gran_1byte,
> + /* We assume 256 byte granularity by default. */
> + write_gran_256bytes = 0,/* If less than 256 bytes are written, the
> unwritten bytes are undefined. */
> + write_gran_1bit, /* Each bit can be cleared individually. */
> + write_gran_1byte, /* A byte can be written once. Further
> writes to an already written byte cause
> + * its contents to be either undefined or
> to stay unchanged. */
> + write_gran_264bytes, /* If less than 264 bytes are written, the
> unwritten bytes are undefined. */
> + write_gran_512bytes, /* If less than 512 bytes are written, the
> unwritten bytes are undefined. */
> + write_gran_528bytes, /* If less than 528 bytes are written, the
> unwritten bytes are undefined. */
> + write_gran_1024bytes, /* If less than 1024 bytes are written,
> the unwritten bytes are undefined. */
> + write_gran_1056bytes, /* If less than 1056 bytes are written,
> the unwritten bytes are undefined. */
> };
>
> /*
> diff --git a/flashrom.c b/flashrom.c
> index 225b6f0..4e5f484 100644
> --- a/flashrom.c
> +++ b/flashrom.c
> @@ -676,6 +676,23 @@ out_free:
> return ret;
> }
>
> +/* Helper function for need_erase() that focuses on granularities of n
> bytes. */
> +static int need_erase_n_bytes(uint8_t *have, uint8_t *want, unsigned int
> len, unsigned int n)
> +{
> + unsigned int i, j, limit;
> + for (j = 0; j < len / n; j++) {
> + limit = min (n, len - j * n);
> + /* Are 'have' and 'want' identical? */
> + if (!memcmp(have + j * n, want + j * n, limit))
> + continue;
> + /* have needs to be in erased state. */
> + for (i = 0; i < limit; i++)
> + if (have[j * n + i] != 0xff)
> + return 1;
> + }
> + return 0;
> +}
> +
> /*
> * Check if the buffer @have can be programmed to the content of @want
> without
> * erasing. This is only possible if all chunks of size @gran are either
> kept
> @@ -693,7 +710,7 @@ out_free:
> int need_erase(uint8_t *have, uint8_t *want, unsigned int len, enum
> write_granularity gran)
> {
> int result = 0;
> - unsigned int i, j, limit;
> + unsigned int i;
>
> switch (gran) {
> case write_gran_1bit:
> @@ -711,20 +728,22 @@ int need_erase(uint8_t *have, uint8_t *want,
> unsigned int len, enum write_granul
> }
> break;
> case write_gran_256bytes:
> - for (j = 0; j < len / 256; j++) {
> - limit = min (256, len - j * 256);
> - /* Are 'have' and 'want' identical? */
> - if (!memcmp(have + j * 256, want + j * 256, limit))
> - continue;
> - /* have needs to be in erased state. */
> - for (i = 0; i < limit; i++)
> - if (have[j * 256 + i] != 0xff) {
> - result = 1;
> - break;
> - }
> - if (result)
> - break;
> - }
> + result = need_erase_n_bytes(have, want, len, 256);
> + break;
> + case write_gran_264bytes:
> + result = need_erase_n_bytes(have, want, len, 264);
> + break;
> + case write_gran_512bytes:
> + result = need_erase_n_bytes(have, want, len, 512);
> + break;
> + case write_gran_528bytes:
> + result = need_erase_n_bytes(have, want, len, 528);
> + break;
> + case write_gran_1024bytes:
> + result = need_erase_n_bytes(have, want, len, 1024);
> + break;
> + case write_gran_1056bytes:
> + result = need_erase_n_bytes(have, want, len, 1056);
> break;
> default:
> msg_cerr("%s: Unsupported granularity! Please report a bug
> at "
> @@ -772,6 +791,21 @@ static unsigned int get_next_write(uint8_t *have,
> uint8_t *want, unsigned int le
> case write_gran_256bytes:
> stride = 256;
> break;
> + case write_gran_264bytes:
> + stride = 264;
> + break;
> + case write_gran_512bytes:
> + stride = 512;
> + break;
> + case write_gran_528bytes:
> + stride = 528;
> + break;
> + case write_gran_1024bytes:
> + stride = 1024;
> + break;
> + case write_gran_1056bytes:
> + stride = 1056;
> + break;
> default:
> msg_cerr("%s: Unsupported granularity! Please report a bug
> at "
> "flashrom at flashrom.org\n", __func__);
> --
> Kind regards, Stefan Tauner
>
>
> _______________________________________________
> flashrom mailing list
> flashrom at flashrom.org
> http://www.flashrom.org/mailman/listinfo/flashrom
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20130328/96565430/attachment.html>
More information about the flashrom
mailing list