patch: add zero fill command option to lar

[ron minnich]

You can now say:
lar z build/bios.bin

and get a zero-filled lar entry. The "zeros" are actually 0xff, which
is flash-friendly and makes flashrom fast.

Parts of stream.c could use the new functions, in particular hlen, but
that patch is for later.

This change dramatically speeds up booting on the alix 1c.

ron

[Carl-Daniel Hailfinger]

On 09.02.2008 18:56, ron minnich wrote:
> You can now say:
> lar z build/bios.bin
>
> and get a zero-filled lar entry.

If I read the code correctly, the command will try to fill the first
occurrence of free space in the lar, not the biggest one. That means we
might have to call it multiple times.
And a general problem with the code: If it finds the offset of the first
empty area, it checks whether the area is big enough. If not, it does
not search for another empty area, but returns instead with an error.
This will cause zerofilling for some archives to fail.
I give you an ack below, bu I'd like to see the points above addressed
in some later patch (or now, if you feel like it).
> The "zeros" are actually 0xff,

Should we really call this "zerofill"?
> which is flash-friendly and makes flashrom fast.

Well, flashing 0xff is not generally faster for SPI due to various
chip/chipset/protocol limitations. It is possible to speed this up for a
few special cases (one of them would be naturally-aligned 256-byte
blocks of 0xff).
Thanks for giving me this idea!
> Parts of stream.c could use the new functions, in particular hlen, but
> that patch is for later.
>
> This change dramatically speeds up booting on the alix 1c.
>
> ron
>
> Add a zero-fill command to lar.
>
> Signed-off-by: Ronald G. Minnich

Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel

[ron minnich]

On Feb 9, 2008 1:06 PM, Carl-Daniel Hailfinger
 wrote:
> If I read the code correctly, the command will try to fill the first
> occurrence of free space in the lar, not the biggest one. That means we
> might have to call it multiple times.

hmm. I obviously did not think of this.
> And a general problem with the code: If it finds the offset of the first
> empty area, it checks whether the area is big enough. If not, it does
> not search for another empty area, but returns instead with an error.

Actually, I think as written it is broken for the general case of lots
of empty spots. It should just look for the room left from start of
empty space to next LAR header. It's just that I have never had a LAR
that has more than one block of empty space, and that empty space is
always at the gap before the bootblock.

How about this. I'd like to commit now, and then fix this later, as I
really need this code in there for speed and it works fine with all
"fresh" images built with v3.
> Should we really call this "zerofill"?

"emptyfill"? I'll let it go for now but you make  good point.
> Acked-by: Carl-Daniel Hailfinger

OK, this is clearly a work in progress but even in this form it is so
useful I'm going to commit it with a note, "This needs more work", and
with luck someone else will improve it once we need it improved -- the
basic structure is there.

ron

[ron minnich]

Committed revision 583.

With flaws still in place ...

ron

[Carl-Daniel Hailfinger]

On 09.02.2008 22:17, ron minnich wrote:
> Committed revision 583.
>
> With flaws still in place ...

Thanks, with your commit message it will be obvious what to improve.

Regards,
Carl-Daniel

[Carl-Daniel Hailfinger]

Peter?

This should answer your "zerofill" question.

Regards,
Carl-Daniel

On 09.02.2008 22:13, ron minnich wrote:
> On Feb 9, 2008 1:06 PM, Carl-Daniel Hailfinger
>  wrote:
> > If I read the code correctly, the command will try to fill the first
> > occurrence of free space in the lar, not the biggest one. That means we
> > might have to call it multiple times.
>
> hmm. I obviously did not think of this.
> > And a general problem with the code: If it finds the offset of the first
> > empty area, it checks whether the area is big enough. If not, it does
> > not search for another empty area, but returns instead with an error.
>
> Actually, I think as written it is broken for the general case of lots
> of empty spots. It should just look for the room left from start of
> empty space to next LAR header. It's just that I have never had a LAR
> that has more than one block of empty space, and that empty space is
> always at the gap before the bootblock.
>
> How about this. I'd like to commit now, and then fix this later, as I
> really need this code in there for speed and it works fine with all
> "fresh" images built with v3.
> > Should we really call this "zerofill"?
>
> "emptyfill"? I'll let it go for now but you make  good point.
> > Acked-by: Carl-Daniel Hailfinger
>
> OK, this is clearly a work in progress but even in this form it is so
> useful I'm going to commit it with a note, "This needs more work", and
> with luck someone else will improve it once we need it improved -- the
> basic structure is there.