From: Baruch Siach <baruch@tkos•co.il>
To: Shawn Guo <shawn.guo@freescale•com>
Cc: gerg@snapgear•com, B32542@freescale•com, netdev@vger•kernel.org,
s.hauer@pengutronix•de, u.kleine-koenig@pengutronix•de,
w.sang@pengutronix•de, r64343@freescale•com, eric@eukrea•com,
bryan.wu@canonical•com, davem@davemloft•net,
linux-arm-kernel@lists•infradead.org, lw@karo-electronics•de
Subject: Re: [PATCH v2 05/10] net/fec: add dual fec support for mx28
Date: Tue, 4 Jan 2011 11:59:16 +0200 [thread overview]
Message-ID: <20110104095916.GE2987@jasper.tkos.co.il> (raw)
In-Reply-To: <1294133056-21195-6-git-send-email-shawn.guo@freescale.com>
Hi Shawn,
On Tue, Jan 04, 2011 at 05:24:11PM +0800, Shawn Guo wrote:
> This patch is to add mx28 dual fec support. Here are some key notes
> for mx28 fec controller.
>
> - mx28 fec design made an assumption that it runs on a
> big-endian system, which is incorrect. As the result, the
> driver has to swap every frame going to and coming from
> the controller.
> - external phys can only be configured by fec0, which means
> fec1 can not work independently and both phys need to be
> configured by mii_bus attached on fec0.
> - mx28 fec reset will get mac address registers reset too.
> - MII/RMII mode and 10M/100M speed are configured differently
> from i.mx/mxs fec controller.
> - ETHER_EN bit must be set to get interrupt work.
>
> Signed-off-by: Shawn Guo <shawn.guo@freescale•com>
> ---
> Changes for v2:
> - Use module parameter fec.macaddr over new kernel command line
> fec_mac to pass mac address
Since you introduce this new kernel command line parameter in patch #3 of this
series, why not just make it right in the first place? This should make both
patches smaller and easier for review.
> - Update comment in fec_get_mac() to stop using confusing word
> "default"
> - Fix copyright breakage in fec.h
Ditto.
> drivers/net/Kconfig | 7 ++-
> drivers/net/fec.c | 139 ++++++++++++++++++++++++++++++++++++++++----------
> drivers/net/fec.h | 5 +-
> include/linux/fec.h | 3 +-
> 4 files changed, 120 insertions(+), 34 deletions(-)
[snip]
> diff --git a/drivers/net/fec.c b/drivers/net/fec.c
> index f147508..b2b3e37 100644
> --- a/drivers/net/fec.c
> +++ b/drivers/net/fec.c
> @@ -17,6 +17,8 @@
> *
> * Bug fixes and cleanup by Philippe De Muyter (phdm@macqel•be)
> * Copyright (c) 2004-2006 Macq Electronique SA.
> + *
> + * Copyright (C) 2010 Freescale Semiconductor, Inc.
> */
>
> #include <linux/module.h>
> @@ -45,21 +47,34 @@
>
> #include <asm/cacheflush.h>
>
> -#ifndef CONFIG_ARCH_MXC
> +#if !defined(CONFIG_ARCH_MXC) && !defined(CONFIG_SOC_IMX28)
> #include <asm/coldfire.h>
> #include <asm/mcfsim.h>
> #endif
>
> #include "fec.h"
>
> -#ifdef CONFIG_ARCH_MXC
> -#include <mach/hardware.h>
Since you now remove mach/hardware.h for ARCH_MXC, does this build for all
i.MX variants?
> +#ifdef CONFIG_SOC_IMX28
> +/*
> + * mx28 does not have MIIGSK registers
> + */
> +#undef FEC_MIIGSK_ENR
> +#include <mach/mxs.h>
> +#else
> +#define cpu_is_mx28() (0)
> +#endif
This breaks kernels for multiple archs (e.g. i.MX28 and i.MX25). Please use
run-time detection of CPU type, and do the MII/RMII etc. configuration
accordingly.
> +
> +#if defined(CONFIG_ARCH_MXC) || defined(CONFIG_SOC_IMX28)
> #define FEC_ALIGNMENT 0xf
> #else
> #define FEC_ALIGNMENT 0x3
> #endif
[snip]
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos•co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
next prev parent reply other threads:[~2011-01-04 9:59 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-04 9:24 [PATCH v2 00/10] net/fec: add dual fec support for i.MX28 Shawn Guo
2011-01-04 9:24 ` [PATCH v2 01/10] net/fec: fix MMFR_OP type in fec_enet_mdio_write Shawn Guo
2011-01-04 9:24 ` [PATCH v2 02/10] net/fec: remove the use of "index" which is legacy Shawn Guo
2011-01-04 9:24 ` [PATCH v2 03/10] net/fec: add mac field into platform data and consolidate fec_get_mac Shawn Guo
2011-01-04 9:24 ` [PATCH v2 04/10] net/fec: improve pm for better suspend/resume Shawn Guo
2011-01-04 9:24 ` [PATCH v2 05/10] net/fec: add dual fec support for mx28 Shawn Guo
2011-01-04 9:59 ` Baruch Siach [this message]
2011-01-04 14:13 ` Shawn Guo
2011-01-04 15:07 ` Baruch Siach
2011-01-05 8:45 ` Sascha Hauer
2011-01-05 9:03 ` Uwe Kleine-König
2011-01-05 9:40 ` Shawn Guo
2011-01-04 9:24 ` [PATCH v2 06/10] ARM: mx28: update clocks for dual fec support Shawn Guo
2011-01-04 13:58 ` Uwe Kleine-König
2011-01-04 14:26 ` Shawn Guo
2011-01-04 9:24 ` [PATCH v2 07/10] ARM: mx28: add the second fec device registration Shawn Guo
2011-01-04 9:24 ` [PATCH v2 08/10] ARM: mxs: add ocotp read function Shawn Guo
2011-01-04 9:24 ` [PATCH v2 09/10] ARM: mx28: read fec mac address from ocotp Shawn Guo
2011-01-04 9:24 ` [PATCH v2 10/10] ARM: mxs: add initial pm support Shawn Guo
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=20110104095916.GE2987@jasper.tkos.co.il \
--to=baruch@tkos$(echo .)co.il \
--cc=B32542@freescale$(echo .)com \
--cc=bryan.wu@canonical$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=eric@eukrea$(echo .)com \
--cc=gerg@snapgear$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=lw@karo-electronics$(echo .)de \
--cc=netdev@vger$(echo .)kernel.org \
--cc=r64343@freescale$(echo .)com \
--cc=s.hauer@pengutronix$(echo .)de \
--cc=shawn.guo@freescale$(echo .)com \
--cc=u.kleine-koenig@pengutronix$(echo .)de \
--cc=w.sang@pengutronix$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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