All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.