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

[Simon Goldschmidt]

On Thu, Feb 13, 2020 at 8:53 AM Stefan <sr at denx.de> wrote:
>
> 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. ;)

Ok, I'm not the one in charge to decide if it's ok to merge this code.

Regards,
Simon

>
> Thanks,
> Stefan