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

Stefan sr at denx.de
Thu Feb 13 08:53:01 CET 2020


Hi Simon,

On 13.02.20 08:40, Simon Goldschmidt wrote:
> 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.

I understand.

> 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.

I fully agree. But I already pushed Weijie to make many enhancements to
the code and I fear that this work gets just too complex (time
consuming) right now. As this type of generalization is not restricted
on this new lzma implementation, but will very likely touch other
features as well.

So again, I'm still okay with adding this feature for spi_nor only right
now. But if Weijie volunteers to move it to a even more generic
location, that would be very welcome of course. ;)

Thanks,
Stefan


More information about the U-Boot mailing list