On Thu, Feb 13, 2020 at 8:53 AM Stefan <s...@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 <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. ;)
Ok, I'm not the one in charge to decide if it's ok to merge this code. Regards, Simon > > Thanks, > Stefan