From mboxrd@z Thu Jan  1 00:00:00 1970
From: Igor Grinberg <grinberg@compulab.co.il>
Date: Wed, 30 Nov 2011 09:42:57 +0200
Subject: [U-Boot] [PATCH 01/13] davinci_emac: move arch-independent
 defines to separate header
In-Reply-To: <1322498261-20645-2-git-send-email-yanok@emcraft.com>
References: <1322498261-20645-1-git-send-email-yanok@emcraft.com>
	<1322498261-20645-2-git-send-email-yanok@emcraft.com>
Message-ID: <4ED5DE81.2040106@compulab.co.il>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Ilya,

On 11/28/11 18:37, Ilya Yanok wrote:
> DaVinci EMAC is found not only on DaVinci SoCs but on some OMAP3 SoCs
> also. This patch moves common defines from arch-davinci/emac_defs.h to
> drivers/net/davinci_emac.h
> 
> DaVinci specific PHY drivers hacked to include the new header. We might
> want to switch to phylib in future.
> 
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> ---
>  arch/arm/cpu/arm926ejs/davinci/dp83848.c      |    1 +
>  arch/arm/cpu/arm926ejs/davinci/et1011c.c      |    1 +
>  arch/arm/cpu/arm926ejs/davinci/ksz8873.c      |    1 +
>  arch/arm/cpu/arm926ejs/davinci/lxt972.c       |    1 +
>  arch/arm/include/asm/arch-davinci/emac_defs.h |  289 -----------------------
>  drivers/net/davinci_emac.c                    |    1 +
>  drivers/net/davinci_emac.h                    |  314 +++++++++++++++++++++++++
>  7 files changed, 319 insertions(+), 289 deletions(-)
>  create mode 100644 drivers/net/davinci_emac.h
> 
> diff --git a/arch/arm/cpu/arm926ejs/davinci/dp83848.c b/arch/arm/cpu/arm926ejs/davinci/dp83848.c
> index c71c685..d435e4b 100644
> --- a/arch/arm/cpu/arm926ejs/davinci/dp83848.c
> +++ b/arch/arm/cpu/arm926ejs/davinci/dp83848.c
> @@ -29,6 +29,7 @@
>  #include <net.h>
>  #include <dp83848.h>
>  #include <asm/arch/emac_defs.h>
> +#include "../../../../../drivers/net/davinci_emac.h"

Doesn't this look terrible?
I'd suggest, to split the davinci_emac.h to driver private part
(if there is) and to public part.
Then put the public part into the include/ directory - just as,
for example, dm9000 does.

Other then that, the patch indeed good and very useful.
Thanks.

[...]

> diff --git a/arch/arm/cpu/arm926ejs/davinci/ksz8873.c b/arch/arm/cpu/arm926ejs/davinci/ksz8873.c
> index 634eda0..3546e7f 100644
> --- a/arch/arm/cpu/arm926ejs/davinci/ksz8873.c
> +++ b/arch/arm/cpu/arm926ejs/davinci/ksz8873.c
> @@ -36,6 +36,7 @@
>  #include <net.h>
>  #include <asm/arch/emac_defs.h>
>  #include <asm/io.h>
> +#include "../../../../../drivers/net/davinci_emac.h"

grrr..

>  
>  int ksz8873_is_phy_connected(int phy_addr)
>  {
> diff --git a/arch/arm/cpu/arm926ejs/davinci/lxt972.c b/arch/arm/cpu/arm926ejs/davinci/lxt972.c
> index 733d413..cce1fe4 100644
> --- a/arch/arm/cpu/arm926ejs/davinci/lxt972.c
> +++ b/arch/arm/cpu/arm926ejs/davinci/lxt972.c
> @@ -30,6 +30,7 @@
>  #include <miiphy.h>
>  #include <lxt971a.h>
>  #include <asm/arch/emac_defs.h>
> +#include "../../../../../drivers/net/davinci_emac.h"

grrr..

[...]

> diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
> index 36c33af..43c4373 100644
> --- a/drivers/net/davinci_emac.c
> +++ b/drivers/net/davinci_emac.c
> @@ -43,6 +43,7 @@
>  #include <malloc.h>
>  #include <asm/arch/emac_defs.h>
>  #include <asm/io.h>
> +#include "davinci_emac.h"

then here, it will be both:
#include <davinci_emac.h>
#include "davinci_emac.h"

[...]


-- 
Regards,
Igor.