public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Ben Warren <biggerbadderben@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/1] net: Add Xilinx LL Temac driver version2
Date: Sun, 22 Feb 2009 22:45:24 -0800	[thread overview]
Message-ID: <49A24604.5050207@gmail.com> (raw)
In-Reply-To: <1233486999-6223-1-git-send-email-monstr@monstr.eu>

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@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@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

  parent reply	other threads:[~2009-02-23  6:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-01 11:16 [U-Boot] [PATCH 1/1] net: Add Xilinx LL Temac driver version2 monstr at monstr.eu
2009-02-17  9:35 ` Michal Simek
2009-02-23  6:45 ` Ben Warren [this message]
2009-02-23  8:03   ` Michal Simek
2009-02-23 13:51   ` Michal Simek
2009-02-23 18:29     ` Ben Warren
2009-02-24 15:11       ` Michal Simek
2009-02-24 17:51         ` Ben Warren
2009-02-24 18:27           ` Michal Simek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49A24604.5050207@gmail.com \
    --to=biggerbadderben@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox