[coreboot] [patch] SST25VF016B (2MB) flash on m57sli (IT8716F).
Ronald Hoogenboom
ronald at zonnet.nl
Wed Mar 12 21:56:55 CET 2008
On Tue, 2008-03-11 at 01:12 +0100, Carl-Daniel Hailfinger wrote:
> On 10.03.2008 09:05, Ronald Hoogenboom wrote:
> > This patch allows direct out-of-SPI-flash boot of a Linux kernel. It
> > circumvents the 512KB limitation in the IT8716f superio of memory
> > mapping an SPI flash device by using a PIO method for reading the data.
> > Limitation: no nrv2b support.
> >
> > Signed-off-by: Ronald Hoogenboom <hoogenboom30 at zonnet.nl>
> >
>
> src/stream/piorom_stream.c:
> A missing license header.
I just imitated rom_stream.c, which hasn't got it either...
>
> src/mainboard/gigabyte/m57sli/Options.lb:
> Changed to be SPI-only (rev 1.x boards will have problems).
Rev. 1.x boards would choose 512K rom, which changes the piorom_stream
back to rom_stream (see the very first #if in piorom_stream). I agree
this is ugly, but I couldn't make it work in the Options.lb with an 'if
ROM_SIZE > 512*1024' - like construct.
> CONFIG_VGA_ROM_RUN settings are added (does not belong into this patch).
True.
>
> src/lib/lzma_cb.c:
>
> This copies the incorrect copyright header from src/lib/lzma.c. I sent a
> patch to correct the copyright header (GPL v2 and later) more than half
> a year ago, IIRC Uwe vetoed the change because it did not introduce the
> long copyright+disclaimer header.
Where is a correct example of the header you would like to see?
> It would be great if you used sizeof() instead of sizeof because it
> helps readability.
That's a matter of taste. I never use brackets on sizeof (except for
types), because it makes it look like a function, which it isn't. But
conforming is easy...
>
> static unsigned char scratchpad[15980];
> static CLzmaDecoderState state;
> static UInt32 outSize;
>
> All three allocations were moved from the stack to rwdata. Is this
> intentional?
This was done, because I wanted to use the out-read mode of the lzma
decoder too, but that became too complex... I think they can be moved
back to the stack now, if that is prefered.
>
> src/lib/lzmadecode.c:
>
> You change the definition of RC_TEST for the normal case (different
> error code). Any reason for that?
No, this was just done while debugging and it remained there. I will
remove that.
>
> I think the big buffer for streaming LZMA decompression is not allocated
> anywhere, but it's already late at night, so I may be mistaken.
rom_stream also just dumps the decompressed result at address 16*2^20,
so I figured that it would be OK if I did it too... (I must say, it made
me frown when I saw it, but hey, it's only a short-lived intermediate
and nobody else is using the memory, but coreboot...)
> Regards,
> Carl-Daniel
>
Can you comment back, then I'll change things and recreate a patch.
Thanks,
Ronald.
More information about the coreboot
mailing list