[U-Boot-Users] [PATCH] add: fpga loadi <dev> <src> sub command.
Grant Likely
grant.likely at secretlab.ca
Wed Jul 4 18:23:53 CEST 2007
On 7/4/07, eran liberty <eran.liberty at gmail.com> wrote:
> On 7/3/07, Grant Likely <grant.likely at secretlab.ca> wrote:
> > I might be missing something here; wouldn't it be better to extend the
> > loadmk command? It looks to me that loadmk currently only handles
> > uncompressed images, but that it could be fixed to do compressed ones
> > too. Correct me if I'm wrong.
> >
>
> You are not wrong.
> Still I would like to argue 4 points
>
> 1. To start with, when i wrote this functionality, loadmk was not
> patched in yet... (not a very strong point :) )
>
> 2. If we take a closer look at loadmk, its 10 lines long and this same
> functionality is within my loadi under the "IH_COMP_NONE" case. (some
> what stornger)
>
> 3. with the addition of the decompression functionality, loadmk will
> be too long to be inlined in the the switch - case as it is right now.
> It would be exported to a function loadmk_func() and it will be called
> from the switch case. That is what loadi does. So you can either
> trash the original loadmk or rename my function to loadmk function and
> call it from loadmk... the out come is the same, only syntactical
> change. (Even stronger)
>
> 4. I prefer the name loadi over loadmk. (The best argue in the world :) )
Heh; I also hate it when things change underneath me. :-)
Regardless, you should consolidate. The name doesn't matter much, and
loadmk was in first; so you should stick with loadmk. It's probably
fine if you swap in your implementation, but make sure you cc the
author of loadmk when you submit the patch.
> > > +extern int gunzip (void *dst, int dstlen, unsigned char *src,
> > > + unsigned long *lenp);
> > > +
> >
> > Very bad; please don't do this (even if it is done elsewhere). Please
> > fix/create the appropriate gzip header file instead.
> >
>
> Agree!
> the gunzip function is not exported (defined in the header file)
> I wanted not to change any files I had no direct need to.
> should I add gunzip declaration to a header file?
Add the gunzip decl to the header please. Submit as a separate patch.
For extra credit; include a patch to remove all the spurious extern
gunzip() decls scattered about the various .c files. :-)
> > > + break;
> > > + case IH_COMP_GZIP:
> > > + if (gunzip
> > > + ((char *)CFG_LOAD_ADDR, unc_len, (uchar *) data, &len)
> > > + != 0) {
> >
> > nit: a little ugly; can you split the gunzip call and if() into
> > separate lines for easy readability?
> >
>
> not ugly in my book, but can do. :)
Heh; only ugly because the if() is longer than 80chars and spills over
3 lines. :-)
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely at secretlab.ca
(403) 399-0195
More information about the U-Boot
mailing list