[PATCH v4 17/20] spl: nor: add lzma decompression support for legacy image

[Simon Goldschmidt]

Sorry if it seems I ignored this mail yesterday, I just found it now :)

On Wed, Feb 12, 2020 at 10:05 AM Stefan Roese <mail at roese.nl> wrote:
>
> On 12.02.20 09:57, Weijie Gao wrote:
>
> <snip>
>
> >> And more general: why do we need to code this in every loader? I think it would
> >> be preferrable to have the loader load the binary data and do the decompression
> >> (and entry_point assignment) in a central place so that all loaders can benefit
> >> from compression. As it is now, we are duplicating code when implementing LZMA
> >> in the next loader.
>
> I agree with Simon, that it would make sense to move this code into a
> even more generic location, so that other "loaders" can also use this
> feature. I know, that I suggested to add it here and I think we can
> make this move into the common SPL interface at a later point, after
> this patch set has been integrated.

My concern is that we add poor code now and that cleanup at a "later point"
just gets forgotten.

To me, this patch looks like another "get the stuff I need in fast" thing,
without thinking about overall code quality. Yes it might be more work to
do it properly, but in my opinion, the result will be code that is easier to
maintain in the long run.

Regards,
Simon

>
> > This feature is originally designed for the case that u-boot is stored
> > in a small capacity storage device, mostly NOR flashes, and the space
> > reserved for u-boot is very small. Most loaders (MMC, NAND, SATA, ...)
> > do not need this at all.
>
> Yes and no. As you pointed out, it might be faster to load and
> decompress a smaller U-Boot proper image than just loading a bigger
> image. So other platforms might very well take advantage of this
> feature. And size increase is always a big issue in modern U-Boot. So a
> smaller image is always welcome. ;)
>
> Thanks,
> Stefan