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