Re: [flashrom] [PATCH] Partial writes for SPI

[Carl-Daniel Hailfinger]

Hi Michael,

On 22.05.2010 01:38, Carl-Daniel Hailfinger wrote:
> On 25.03.2010 10:46, Michael Karcher wrote:
> > Am Donnerstag, den 25.03.2010, 03:10 +0100 schrieb Carl-Daniel
> > Hailfinger:
> > > /* 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) {
> > > 	towrite = min(chunksize, lenhere - j);
> > > 	rc = spi_nbyte_program(starthere + j, buf + starthere - start + j, towrite);
> > > 	if (rc)
> > > 		break;
> > > 	while (spi_read_status_register() & JEDEC_RDSR_BIT_WIP)
> > > 		programmer_delay(10);
> > > }
> > > if (rc)
> > > 	break;
> > > }
> >
> > i would write this as (carful, untested):
> > startofs = start % pagesize;  /* offset relative to page begin */
> > /* iterate over all pages to write */
> > for(pageaddr = start - startofs; pageaddr < start + len; 
> >     pageaddr += page_size) {
> >     endofs = min(pagesize, start + len - pageaddr);
> >     /* iterate over chunks in this page */
> >     for(j = startofs; j < endofs; j += chunksize) {
> >         towrite = min(chunksize, endofs - j);
> >         rc = spi_nbyte_program(pageaddr + j, buf, towrite);
> >         buf += towrite;
> >         if (rc)
> >             goto leave_page_loop;
> >         /* Copy/paste wait here */
> >     }
> >     startofs = 0;  /* all pages but the first need to be programmed
> >                       from the beginning */
> > }
> >
> > leave_page_loop:
>
> If you could send your code as patch for spi_read_chunked and
> spi_write_chunked, I'd be really happy.
> > Your logic seem right, too, so this is a matter of taste. I think my
> > version needs less explanation, that's why I would prefer it that way.
>
> Fully agreed. We want your logic.

Could you send your logic as a patch which changes spi_read_chunked and
spi_write_chunked?

Thanks!

Regards,
Carl-Daniel