[U-Boot-Users] New and updated patches sent to U-Boot maintainer

[Keith J Outwater]

> Dear Keith,
> 
> in message 
<OF0EE85153.67A4AEC8-ON072571E6.005FB093-072571E6.006006D2 at mck.us.ray.com> 
you wrote:
> > I have submitted four patches against U-Boot to Wolfgang Denk directly 
due 
> > to list attachment size limits.
> 
> Can you please update your patches using a current code base? I get a
> lot of rejects when applying your patches.

OK. I was hoping I could get away with not having to do that.  Sorry!

> 
> Also, please perform the following cleanup / changes:
> 
> * Make sure to adapt the Makefiles to the new build environment
>   extensions (BUILD_DIR and depend).

OK

> 
> * Please create just a single 'drivers/xilinx' directory instead of 5;
>   use subdirectories where really necessary

OK.  I should have done this the first time.

> 
> * Clean up the Coding style (trailing white space in  65  files,  C++
>   comments, indentation not by tabs, trailing empty lines, etc.)

I'll run all the sources through GNU indent.

> 
> * Please add correct author information; "Author: Xilinx, Inc." is not
>   acceptable.
> 
> * Please clean up your license headers. The GPL is pretty clear that
>   there must not be any additional restrictions on the code. Combining
>   this with statements like "YOU ARE RESPONSIBLE FOR OBTAINING ANY
>   THIRD PARTY RIGHTS YOU MAY REQUIRE" is IMHO not acceptable. Either
>   this is GPL, and I have all rights, or it isn't. Also, "Use in such
>   applications is expressly prohibited." is a restriction that is IMO
>   incompatible with the GPL.

Perhaps the current git head does not have these statements in the
Xilinx code in ./board/xilinx/xilinx_enet, for example.  I'll check
tonight.  My source base does have them, though, but my code is a little 
old.

> 
> * Please clean up the function headers and comments. Something like 
this:
> 
>     126 
/*****************************************************************************/
>     127 /**
>     128 *
>     129 * Sets up a callback function to be invoked when an assert 
occurs. If there
>     130 * was already a callback installed, then it is replaced.
>     131 *
>     132 * @param    Routine is the callback to be invoked when an assert 
is taken
>     133 *
>     134 * @return
>     135 *
>     136 * None.
>     137 *
>     138 * @note
>     139 *
>     140 * This function has no effect if NDEBUG is set
>     141 *
>     142 
******************************************************************************/
>     143 void
>     144 XAssertSetCallback(XAssertCallback Routine)
>     145 {
>     146         XAssertCallbackRoutine = Routine;
>     147 }
> 
>   (i. e. 17 lines of mostly empty comment versus 1 line of code) is
>   not acceptable. Please be descriptive, but terse.
> 
>   This code is *terrybly* bloated - so much, that it's actually
>   unreadable.
> 

I tend to agree, but this is the Xilinx provided driver code.  I can take 
not credit for
it's quality :)

BTW, Some of this code is already in U-Boot
(./board/xilinx/common ./board/xilinx_emac ./board/xilinx_iic).
My plan was to put it into a common location after which anyone else using 
the code
in ./boards/xilinx could switch over to the code in ./drivers

>   Is there any chance to clean this up such that only the *necessary*
>   stuff gets included? You don;t want to tell me  that  all  this  is
>   needed or even used?
> 

The idea was to use the driver source code as-is to avoid having to
re-invent the wheel.  I would expect that as more Xilinx FPGA based
boards are added, the percentage use of the Xilinx driver code would 
increase.
I actually do use a lot of the functionality provided by the drivers 
because one
of my primary uses of U-Boot is to assist in hardware design verification 
and debug.
So I end up having to write lots of diagnostic code to examine hardware 
when,
for example, I am trying to get the Xilinx SystemAce to work properly.
Also, the systems I work on do not tend to be resource constrained.  A 
luxury,
to be sure, but it means I can tolerate larger executables.

> 
> Please resubmit after cleanup. Thanks.

OK.

> 
> Best regards,
> 
> Wolfgang Denk
> 
> -- 
> Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> "The bad reputation UNIX has gotten is totally undeserved, laid on by
> people who don't understand, who have not gotten in there  and  tried
> anything."          -- Jim Joyce, owner of Jim Joyce's UNIX Bookstore