Re: [flashrom] Giving to -i option some real use

[Salvador Eduardo Tropea]

Hi David!

El 21/04/16 a las 18:19, David Hendricks escribió:
> On Thu, Apr 21, 2016 at 1:41 PM, Salvador Eduardo Tropea 
> <[e-mail] > wrote:
> Hi David:
>
> Thanks for your comments.
>
>
> El 20/04/16 a las 20:42, David Hendricks escribió:
>
>     Hello Salvador,
>     Yes, this is a very useful feature - we've had it in the
>     chromiumos branch for a while now :-)
>
>     I need to read your implementation. Ours is called
>     "fast-verify" which will read and only verify portions of the
>     flash specified with -i arguments.
>
>
> What about writes? My problem is that I have a 4 MiB flash and
> that usually need to use 32kB from it.
>
>
> Yes, it works for writes as well. Using your layout file as an example:
> 00000000:00007e2b fpga
> 00007e2c:003fffff free
>
> If the eraseblock size is 4KB and you run /"/flashrom -p <programmer> 
> -l layout -i fpga -w foo.bin --fast-verify/"/, chromium's flashrom will:
> Detect the flash chip.
> Parse the -i argument
> Do a partial read. This is broken into a multiple steps.
>
> 3a. Determine the "required_erase_size", for example, 4KB. Right now 
> the mechanism is crude and chooses the smallest block size that is 
> eraseable.
> 3b. Align regions that are read based on required_erase_size. 0000 is 
> already aligned, but 7e2b will be aligned up to 7fff.
> 3c. Read the aligned region content, which is 0000:7fff instead of 
> 0000:7e2b, into the new content buffer.
> 4. Copy the new content from the "fpga" region into the new content 
> buffer.
> 5. Erase and write 0000:7fff
> 6. Verify from 0000:7fff
>
> Overall it looks pretty similar to what you posted.

Yes.
> Here's the original implementation: 
> https://chromium-review.googlesource.com/#/c/240176/ 
> https://chromium-review.googlesource.com/#/c/240176/4. There are a 
> couple of follow-up patches as well, in case you're interested.

I taked a look at this. The patch have a lot in common.
In my patch I use the first "usable" eraser, just like 
erase_and_write_flash. If this eraser fails the code will try to recover 
reading the whole memory. So I think this is ok.
So assuming that the first "usable" eraser is a good choice I compute 
the alignments using *all* the eraseblocks information. My intention is 
to support cases like it:

.block_erasers    =
         {
             {
                 .eraseblocks = {
                     {16 * 1024, 1},
                     {8 * 1024, 2},
                     {32 * 1024, 1},
                     {64 * 1024, 3},
                 },
                 .block_erase = erase_sector_jedec,
             }, {
                 .eraseblocks = { {256 * 1024, 1} },
                 .block_erase = erase_chip_block_jedec,
             },
         },

Here the first eraser is "erase_sector_jedec", but it have really 
complex "block size", is not one size, but a crazy list of sizes: 16 
kiB, 8 kiB, 8 kiB, 32 kiB, 64 kiB, 64 kiB, 64 kiB
So I walk the list computing the beggining of each block and checking if 
the address is inside it. From what I see in the link above your patch 
uses a single size, the smallest.
> One more thing to note - Be careful about interactions with the 
> proposed patch to read the current contents of flash from a file: 
> https://www.flashrom.org/pipermail/flashrom/2015-December/014034.html. 
> Specifically, make sure that the aligned offsets are used for reading 
> old content no matter if the old content is in a flash chip or a file.

Thanks. It looks compatible.

Regards, SET