| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20110105094011.GA5814@freescale.com>
Date: Wed, 5 Jan 2011 17:40:18 +0800
From: Shawn Guo <shawn.guo@...escale.com>
To: Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>
CC: Sascha Hauer <s.hauer@...gutronix.de>,
Baruch Siach <baruch@...s.co.il>, <B32542@...escale.com>,
<netdev@...r.kernel.org>, <bryan.wu@...onical.com>,
<gerg@...pgear.com>, <w.sang@...gutronix.de>,
<r64343@...escale.com>, <eric@...rea.com>, <davem@...emloft.net>,
<linux-arm-kernel@...ts.infradead.org>, <lw@...o-electronics.de>
Subject: Re: [PATCH v2 05/10] net/fec: add dual fec support for mx28
On Wed, Jan 05, 2011 at 10:03:12AM +0100, Uwe Kleine-König wrote:
> Hello Shawn,
>
> On Wed, Jan 05, 2011 at 09:45:13AM +0100, Sascha Hauer wrote:
> > On Tue, Jan 04, 2011 at 10:13:09PM +0800, Shawn Guo wrote:
> > > Hi Baruch,
> > >
> > > On Tue, Jan 04, 2011 at 11:59:16AM +0200, Baruch Siach wrote:
> > > > 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@...escale.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.
> > > >
> > > Sorry for rushing to send the patch set out. All these updates
> > > should happen on patch #3 than #5. This is a serious problem,
> > > and I will fix it soon and resend as v3.
> > >
> > > > > 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@...qel.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?
> > > >
> > > Did the test build for mx25, mx27, mx3 and mx51.
> > >
> > > > > +#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.
> > > >
> > > I do not find a good way to detect cpu type. Neither adding a new
> > > platform data field nor using __machine_arch_type to enumerate all
> > > mx28 based machine (though there is only one currently) seems to be
> > > good for me.
> > >
> > > I will try to manipulate some mx28 unique register to identify mx28
> > > from other i.mx variants. Hopefully, it will work.
> >
> > There won't be a register which you can safely read on all i.MX
> > variants.FEC_TRUNC_FL
Register FEC_TRUNC_FL (offset 0x1b0) is one I found I can use to
distinguish ENET-MAC from FEC. This address is being reserved on
FEC, and I knew from designer that access the address on FEC
will not generate exception as it's valid in FEC address range.
I proved it on mx51.
/*
* Detect ENET-MAC by writing and reading back on TRUNC_FL register,
* which is accessible on ENET-MAC while always return 0 on FEC.
*/
writel(0x7ff, fep->hwp + 0x1b0);
if (readl(fep->hwp + 0x1b0) == 0x7ff)
fec_is_enetmac = 1;
But it does not matter. I more like the solution that offered by Uwe
below.
Thanks, Sascha.
> > Why don't you implement it the same way the other i.MX do? They do not
> > need SoC detection and the macro expands to 0 at compile time when the
> > cpu is not enabled.
> Alternatively you can use the approach I used for spi-imx and identify
> the device by name.
>
Thanks, Uwe.
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
>
--
Regards,
Shawn
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists