[U-Boot-Users] [PATCH] add: fpga loadi <dev> <src> sub command.

[Grant Likely <grant.likely@secretlab.ca>]

On 7/4/07, eran liberty <eran.liberty@gmail.com> wrote:
> On 7/3/07, Grant Likely <grant.likely@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