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

Reply via email to