[flashrom] [PATCH] Simplify chunking logic
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Tue Jul 13 16:04:19 CEST 2010
On 11.07.2010 18:42, Michael Karcher wrote:
> Read function tested for chunk size 62 and with start offset 3, works
> as expected.
>
> Signed-off-by: Michael Karcher <flashrom at mkarcher.dialup.fu-berlin.de>
> ---
> spi25.c | 80 +++++++++++++++++++++++++-------------------------------------
> 1 files changed, 32 insertions(+), 48 deletions(-)
>
> diff --git a/spi25.c b/spi25.c
> index 51be397..260dd1c 100644
> --- a/spi25.c
> +++ b/spi25.c
> @@ -879,35 +879,27 @@ int spi_nbyte_read(int address, uint8_t *bytes, int len)
> int spi_read_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize)
> {
> int rc = 0;
> - int i, j, starthere, lenhere;
> + int ofs, startofs, endofs; /* offsets relative to page begin */
> int page_size = flash->page_size;
> - int toread;
> -
> - /* Warning: This loop has a very unusual condition and body.
> - * The loop needs to go through each page with at least one affected
> - * byte. The lowest page number is (start / page_size) since that
> - * division rounds down. The highest page number we want is the page
> - * where the last byte of the range lives. That last byte has the
> - * address (start + len - 1), thus the highest page number is
> - * (start + len - 1) / page_size. Since we want to include that last
> - * page as well, the loop condition uses <=.
> - */
> - for (i = start / page_size; i <= (start + len - 1) / page_size; i++) {
> - /* Byte position of the first byte in the range in this page. */
> - /* starthere is an offset to the base address of the chip. */
> - starthere = max(start, i * page_size);
> - /* Length of bytes in the range in this page. */
> - lenhere = min(start + len, (i + 1) * page_size) - starthere;
> - for (j = 0; j < lenhere; j += chunksize) {
> - toread = min(chunksize, lenhere - j);
> - rc = spi_nbyte_read(starthere + j, buf + starthere - start + j, toread);
> + int pageaddr, toread;
> +
>
/* startofs is an offset to current page address. */
> + startofs = start % page_size;
> + /* iterate over all affected pages */
> + for (pageaddr = start - startofs; pageaddr < start + len;
> + pageaddr += page_size) {
>
Please use tabs here, at least for the first 8-space blocks.
> + endofs = min(page_size, start + len - pageaddr);
>
endofs is a misnomer. The correct name would be one_after_endofs, but
that name is ugly. What about
/* Number of bytes to write in this page. */
lenhere = min(page_size, start + len - pageaddr) - startofs;
and using (startofs + lenhere) in place of endofs everywhere?
> + /* iterate over all chunks in the current page */
> + for (ofs = startofs; ofs < endofs; ofs += chunksize) {
> + toread = min(chunksize, endofs - ofs);
> + rc = spi_nbyte_read(pageaddr + ofs, buf, toread);
> + buf += toread;
> if (rc)
> - break;
> + goto leave_page_loop;
>
return rc;
> }
> - if (rc)
> - break;
> + startofs = 0; /* all further pages processed from start */
> }
>
> +leave_page_loop:
>
Not needed if you return rc immediately in the error case.
> return rc;
> }
>
Here is my variant of your code. Feel free to ignore it completely, I
just wanted to send it for reference.
int spi_read_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize)
{
int rc = 0;
int j, lenhere;
int page_size = flash->page_size;
int pageaddr, toread, startofs;
/* This loop needs to go through each page with at least one affected
* byte.
*/
startofs = start % page_size;
for (pageaddr = start - startofs; pageaddr < start + len; pageaddr += page_size) {
/* startofs is an offset to current page address. */
/* Number of bytes to write in this page. */
lenhere = min(page_size, start + len - pageaddr) - startofs;
for (j = startofs; j < startofs + lenhere; j += chunksize) {
toread = min(chunksize, startofs + lenhere - j);
rc = spi_nbyte_read(pageaddr + j, buf, toread);
if (rc)
return rc;
buf += toread;
}
startofs = 0;
}
return rc;
}
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the flashrom
mailing list