[U-Boot] [PATCH 1/1] net: Add Xilinx LL Temac driver version2

Ben Warren biggerbadderben at gmail.com
Mon Feb 23 07:45:24 CET 2009


Hi Michal,

You submitted this so long ago that this driver should be in the tree 
right now.  Unfortunately, I didn't give a thorough enough review the 
first time and I think it would be wrong to pull it in right now without 
addressing some more things.  If you take care of this, I promise to put 
it in the front of the queue for the next release:

monstr at monstr.eu wrote:
> From: Michal Simek <monstr at monstr.eu>
>
> ---
>   
This '---' line ensures that nothing below it (including your SOB) goes 
in the commit.  Please remove it, add a meaningful commit message and 
move the next few lines below the next '---' line.  Also, the "version 
2" in the title should be in square brackets so it get stripped from the 
commit title.
> v2: Coding style cleanup, use debug() instead of printf
> in debug messages
>
> v1: Initial version
>
> Signed-off-by: Michal Simek <monstr at monstr.eu>
> ---
>  drivers/net/Makefile          |    1 +
>  drivers/net/xilinx_ll_temac.c |  540 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 541 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/net/xilinx_ll_temac.c
>
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 631336a..1e80698 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -68,6 +68,7 @@ COBJS-$(CONFIG_ULI526X) += uli526x.o
>  COBJS-$(CONFIG_VSC7385_ENET) += vsc7385.o
>  COBJS-$(CONFIG_XILINX_EMAC) += xilinx_emac.o
>  COBJS-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o
> +COBJS-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o
>  COBJS-$(CONFIG_SH_ETHER) += sh_eth.o
>  
>   
This doesn't apply any more.  Not your fault :)


I get the following whitespace errors:

Applying net: Add Xilinx LL Temac driver
.dotest/patch:415: trailing whitespace.
   
.dotest/patch:419: trailing whitespace.
   
.dotest/patch:424: trailing whitespace.


All of the above mentioned issues are ones that I could easily deal 
with, but one thing that really does need to change is that you need to 
use the CONFIG_NET_MULTI API.  In other words, your driver should have a 
single initialize() function (prototyped in include/netdev.h), and an 
eth_device struct that gets registered.  All your access functions 
(eth_init, eth_send, eth_recv etc.) will be static and pointed to by the 
eth_device struct.  Most drivers are already this way.

Other than that, it's fine.  Sorry again for taking so very long.

regards,
Ben


More information about the U-Boot mailing list