* [PATCH v2 0/3] add fec support for imx6q
@ 2011-09-21 11:10 Shawn Guo
2011-09-21 11:10 ` [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message Shawn Guo
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Shawn Guo @ 2011-09-21 11:10 UTC (permalink / raw)
To: linux-arm-kernel
This series adds imx6q enet support. The imx6q enet is a derivative of
imx28 enet controller. It fixed the frame endian issue found on imx28,
and added 1 Gbps support.
Changes since v1:
* Fix typo pointed out by Francois Romieu
* Drop patch #3 in the v1
* Rebase on net-next tree
Thanks.
Shawn Guo (3):
net/fec: change phy-reset-gpio request warning to debug message
net/fec: fix fec1 check in fec_enet_mii_init()
net/fec: add imx6q enet support
drivers/net/ethernet/freescale/Kconfig | 9 ++--
drivers/net/ethernet/freescale/fec.c | 65 +++++++++++++++++++++++++-------
2 files changed, 55 insertions(+), 19 deletions(-)
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message 2011-09-21 11:10 [PATCH v2 0/3] add fec support for imx6q Shawn Guo @ 2011-09-21 11:10 ` Shawn Guo 2011-09-21 11:25 ` Wolfram Sang 2011-09-21 11:10 ` [PATCH v2 2/3] net/fec: fix fec1 check in fec_enet_mii_init() Shawn Guo 2011-09-21 11:10 ` [PATCH v2 3/3] net/fec: add imx6q enet support Shawn Guo 2 siblings, 1 reply; 15+ messages in thread From: Shawn Guo @ 2011-09-21 11:10 UTC (permalink / raw) To: linux-arm-kernel FEC can work without a phy reset on some platforms, which means not very platform necessarily have a phy-reset gpio encoded in device tree. So it makes more sense to have the phy-reset-gpio request failure as a debug message rather than a warning. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- drivers/net/ethernet/freescale/fec.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c index 158b82e..a057abf 100644 --- a/drivers/net/ethernet/freescale/fec.c +++ b/drivers/net/ethernet/freescale/fec.c @@ -1422,7 +1422,7 @@ static int __devinit fec_reset_phy(struct platform_device *pdev) phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); err = gpio_request_one(phy_reset, GPIOF_OUT_INIT_LOW, "phy-reset"); if (err) { - pr_warn("FEC: failed to get gpio phy-reset: %d\n", err); + pr_debug("FEC: failed to get gpio phy-reset: %d\n", err); return err; } msleep(1); -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message 2011-09-21 11:10 ` [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message Shawn Guo @ 2011-09-21 11:25 ` Wolfram Sang 2011-09-21 12:03 ` Shawn Guo 0 siblings, 1 reply; 15+ messages in thread From: Wolfram Sang @ 2011-09-21 11:25 UTC (permalink / raw) To: linux-arm-kernel Hi Shawn, On Wed, Sep 21, 2011 at 07:10:30PM +0800, Shawn Guo wrote: > FEC can work without a phy reset on some platforms, which means not > very platform necessarily have a phy-reset gpio encoded in device tree. > So it makes more sense to have the phy-reset-gpio request failure as > a debug message rather than a warning. Or remove it entirely? > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > --- > drivers/net/ethernet/freescale/fec.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c > index 158b82e..a057abf 100644 > --- a/drivers/net/ethernet/freescale/fec.c > +++ b/drivers/net/ethernet/freescale/fec.c > @@ -1422,7 +1422,7 @@ static int __devinit fec_reset_phy(struct platform_device *pdev) > phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); > err = gpio_request_one(phy_reset, GPIOF_OUT_INIT_LOW, "phy-reset"); > if (err) { > - pr_warn("FEC: failed to get gpio phy-reset: %d\n", err); > + pr_debug("FEC: failed to get gpio phy-reset: %d\n", err); > return err; I also wanted to suggested to drop returning the error code, since it is not an error anymore, strictly speaking. Then I noticed that the caller does not check the error code. So, this could be added or turn the function to void? > } > msleep(1); > -- > 1.7.4.1 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110921/ac4f8693/attachment.sig> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message 2011-09-21 11:25 ` Wolfram Sang @ 2011-09-21 12:03 ` Shawn Guo 2011-09-21 12:11 ` Wolfram Sang 0 siblings, 1 reply; 15+ messages in thread From: Shawn Guo @ 2011-09-21 12:03 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 21, 2011 at 01:25:55PM +0200, Wolfram Sang wrote: > Hi Shawn, > > On Wed, Sep 21, 2011 at 07:10:30PM +0800, Shawn Guo wrote: > > FEC can work without a phy reset on some platforms, which means not > > very platform necessarily have a phy-reset gpio encoded in device tree. > > So it makes more sense to have the phy-reset-gpio request failure as > > a debug message rather than a warning. > > Or remove it entirely? > I would like to keep it. When people want to debug at this point, they do not need to type the debug message. > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > --- > > drivers/net/ethernet/freescale/fec.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c > > index 158b82e..a057abf 100644 > > --- a/drivers/net/ethernet/freescale/fec.c > > +++ b/drivers/net/ethernet/freescale/fec.c > > @@ -1422,7 +1422,7 @@ static int __devinit fec_reset_phy(struct platform_device *pdev) > > phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0); > > err = gpio_request_one(phy_reset, GPIOF_OUT_INIT_LOW, "phy-reset"); > > if (err) { > > - pr_warn("FEC: failed to get gpio phy-reset: %d\n", err); > > + pr_debug("FEC: failed to get gpio phy-reset: %d\n", err); > > return err; > > I also wanted to suggested to drop returning the error code, since it is > not an error anymore, strictly speaking. Then I noticed that the caller > does not check the error code. So, this could be added or turn the > function to void? > To me, keep the return value as integer is more scalable. Someday, someone need to add more stuff in the function, or want to improve the caller to check return value, it plays. Regards, Shawn > > } > > msleep(1); > > -- ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message 2011-09-21 12:03 ` Shawn Guo @ 2011-09-21 12:11 ` Wolfram Sang 2011-09-21 12:44 ` Shawn Guo 0 siblings, 1 reply; 15+ messages in thread From: Wolfram Sang @ 2011-09-21 12:11 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 21, 2011 at 08:03:42PM +0800, Shawn Guo wrote: > On Wed, Sep 21, 2011 at 01:25:55PM +0200, Wolfram Sang wrote: > > Hi Shawn, > > > > On Wed, Sep 21, 2011 at 07:10:30PM +0800, Shawn Guo wrote: > > > FEC can work without a phy reset on some platforms, which means not > > > very platform necessarily have a phy-reset gpio encoded in device tree. > > > So it makes more sense to have the phy-reset-gpio request failure as > > > a debug message rather than a warning. > > > > Or remove it entirely? > > > I would like to keep it. When people want to debug at this point, they > do not need to type the debug message. I just think the message might be confusing in case you don't need the gpio, because then failing is expected behaviour. For those platforms, it is not even an error then, so you must drop returning the error. To be very precise, you should check of_get_named_gpio() and return if no gpio is specified. Then, you can distinguish that case from problems when getting the GPIO. > > I also wanted to suggested to drop returning the error code, since it is > > not an error anymore, strictly speaking. Then I noticed that the caller > > does not check the error code. So, this could be added or turn the > > function to void? > > > To me, keep the return value as integer is more scalable. Someday, > someone need to add more stuff in the function, or want to improve > the caller to check return value, it plays. I agree that keeping it int is way better. But why not add it now to keep things proper and tested? If this patch gets accepted as it is and later someone else will add error checking to the caller, your platform will lose FEC support as a regression. -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110921/8880ce14/attachment.sig> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message 2011-09-21 12:11 ` Wolfram Sang @ 2011-09-21 12:44 ` Shawn Guo 2011-09-21 12:59 ` Wolfram Sang 0 siblings, 1 reply; 15+ messages in thread From: Shawn Guo @ 2011-09-21 12:44 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 21, 2011 at 02:11:59PM +0200, Wolfram Sang wrote: > On Wed, Sep 21, 2011 at 08:03:42PM +0800, Shawn Guo wrote: > > On Wed, Sep 21, 2011 at 01:25:55PM +0200, Wolfram Sang wrote: > > > Hi Shawn, > > > > > > On Wed, Sep 21, 2011 at 07:10:30PM +0800, Shawn Guo wrote: > > > > FEC can work without a phy reset on some platforms, which means not > > > > very platform necessarily have a phy-reset gpio encoded in device tree. > > > > So it makes more sense to have the phy-reset-gpio request failure as > > > > a debug message rather than a warning. > > > > > > Or remove it entirely? > > > > > I would like to keep it. When people want to debug at this point, they > > do not need to type the debug message. > > I just think the message might be confusing in case you don't need the > gpio, because then failing is expected behaviour. For those platforms, > it is not even an error then, so you must drop returning the error. To > be very precise, you should check of_get_named_gpio() and return if no > gpio is specified. Then, you can distinguish that case from problems > when getting the GPIO. > The whole fec_reset_phy() should not be a show-stopper failure. Even on platforms that have the gpio, FEC can work without resetting the phy in FEC driver for some cases, for example, boot loader has done it. It might be good enough to give a warning message rather than getting the probe fail. > > > I also wanted to suggested to drop returning the error code, since it is > > > not an error anymore, strictly speaking. Then I noticed that the caller > > > does not check the error code. So, this could be added or turn the > > > function to void? > > > > > To me, keep the return value as integer is more scalable. Someday, > > someone need to add more stuff in the function, or want to improve > > the caller to check return value, it plays. > > I agree that keeping it int is way better. But why not add it now to > keep things proper and tested? If this patch gets accepted as it is and > later someone else will add error checking to the caller, your platform > will lose FEC support as a regression. > Again, fec_reset_phy() failure is not a show-stopper. We might not want to make the probe fail because of that. -- Regards, Shawn ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message 2011-09-21 12:44 ` Shawn Guo @ 2011-09-21 12:59 ` Wolfram Sang 2011-09-21 13:18 ` Shawn Guo 0 siblings, 1 reply; 15+ messages in thread From: Wolfram Sang @ 2011-09-21 12:59 UTC (permalink / raw) To: linux-arm-kernel > > I agree that keeping it int is way better. But why not add it now to > > keep things proper and tested? If this patch gets accepted as it is and > > later someone else will add error checking to the caller, your platform > > will lose FEC support as a regression. > > > Again, fec_reset_phy() failure is not a show-stopper. We might not > want to make the probe fail because of that. That would be an argument to make the function returning void? Well, I still think something like /* No phy reset configured */ if (phy_reset == -ENODEV) return 0; might be cleaner, yet I don't have the setup to test such an approach. So, I'll be quiet now and hope for no problems. -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110921/1320571c/attachment.sig> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message 2011-09-21 12:59 ` Wolfram Sang @ 2011-09-21 13:18 ` Shawn Guo 0 siblings, 0 replies; 15+ messages in thread From: Shawn Guo @ 2011-09-21 13:18 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 21, 2011 at 02:59:05PM +0200, Wolfram Sang wrote: > > > I agree that keeping it int is way better. But why not add it now to > > > keep things proper and tested? If this patch gets accepted as it is and > > > later someone else will add error checking to the caller, your platform > > > will lose FEC support as a regression. > > > > > Again, fec_reset_phy() failure is not a show-stopper. We might not > > want to make the probe fail because of that. > > That would be an argument to make the function returning void? > Hmm, it seems to be. Ok, will send v3 to return void. Regards, Shawn > Well, I still think something like > > /* No phy reset configured */ > if (phy_reset == -ENODEV) > return 0; > > might be cleaner, yet I don't have the setup to test such an approach. > > So, I'll be quiet now and hope for no problems. > > -- > Pengutronix e.K. | Wolfram Sang | > Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/3] net/fec: fix fec1 check in fec_enet_mii_init() 2011-09-21 11:10 [PATCH v2 0/3] add fec support for imx6q Shawn Guo 2011-09-21 11:10 ` [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message Shawn Guo @ 2011-09-21 11:10 ` Shawn Guo 2011-09-21 11:10 ` [PATCH v2 3/3] net/fec: add imx6q enet support Shawn Guo 2 siblings, 0 replies; 15+ messages in thread From: Shawn Guo @ 2011-09-21 11:10 UTC (permalink / raw) To: linux-arm-kernel In function fec_enet_mii_init(), it uses non-zero pdev->id as part of the condition to check the second fec instance (fec1). This works before the driver supports device tree probe. But in case of device tree probe, pdev->id is -1 which is also non-zero, so the logic becomes broken when device tree probe gets supported. The patch change the logic to check "pdev->id > 0" as the part of the condition for identifying fec1. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- drivers/net/ethernet/freescale/fec.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c index a057abf..ca6f551 100644 --- a/drivers/net/ethernet/freescale/fec.c +++ b/drivers/net/ethernet/freescale/fec.c @@ -996,7 +996,7 @@ static int fec_enet_mii_init(struct platform_device *pdev) * mdio interface in board design, and need to be configured by * fec0 mii_bus. */ - if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id) { + if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id > 0) { /* fec1 uses fec0 mii_bus */ fep->mii_bus = fec0_mii_bus; return 0; -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] net/fec: add imx6q enet support 2011-09-21 11:10 [PATCH v2 0/3] add fec support for imx6q Shawn Guo 2011-09-21 11:10 ` [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message Shawn Guo 2011-09-21 11:10 ` [PATCH v2 2/3] net/fec: fix fec1 check in fec_enet_mii_init() Shawn Guo @ 2011-09-21 11:10 ` Shawn Guo 2011-09-21 11:07 ` Fabio Estevam 2011-09-21 11:32 ` Wolfram Sang 2 siblings, 2 replies; 15+ messages in thread From: Shawn Guo @ 2011-09-21 11:10 UTC (permalink / raw) To: linux-arm-kernel The imx6q enet is a derivative of imx28 enet controller. It fixed the frame endian issue found on imx28, and added 1 Gbps support. It also fixes a typo on vendor name in Kconfig. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- drivers/net/ethernet/freescale/Kconfig | 9 ++--- drivers/net/ethernet/freescale/fec.c | 61 +++++++++++++++++++++++++------ 2 files changed, 53 insertions(+), 17 deletions(-) diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig index 4dbe41f..1cf6716 100644 --- a/drivers/net/ethernet/freescale/Kconfig +++ b/drivers/net/ethernet/freescale/Kconfig @@ -7,7 +7,7 @@ config NET_VENDOR_FREESCALE default y depends on FSL_SOC || QUICC_ENGINE || CPM1 || CPM2 || PPC_MPC512x || \ M523x || M527x || M5272 || M528x || M520x || M532x || \ - IMX_HAVE_PLATFORM_FEC || MXS_HAVE_PLATFORM_FEC || \ + ARCH_MXC || ARCH_MXS || \ (PPC_MPC52xx && PPC_BESTCOMM) ---help--- If you have a network (Ethernet) card belonging to this class, say Y @@ -16,16 +16,15 @@ config NET_VENDOR_FREESCALE Note that the answer to this question doesn't directly affect the kernel: saying N will just cause the configurator to skip all - the questions about IBM devices. If you say Y, you will be asked for - your specific card in the following questions. + the questions about Freescale devices. If you say Y, you will be + asked for your specific card in the following questions. if NET_VENDOR_FREESCALE config FEC bool "FEC ethernet controller (of ColdFire and some i.MX CPUs)" depends on (M523x || M527x || M5272 || M528x || M520x || M532x || \ - IMX_HAVE_PLATFORM_FEC || MXS_HAVE_PLATFORM_FEC) - default IMX_HAVE_PLATFORM_FEC || MXS_HAVE_PLATFORM_FEC if ARM + ARCH_MXC || ARCH_MXS) select PHYLIB ---help--- Say Y here if you want to use the built-in 10/100 Fast ethernet diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c index ca6f551..9a64ce8 100644 --- a/drivers/net/ethernet/freescale/fec.c +++ b/drivers/net/ethernet/freescale/fec.c @@ -18,7 +18,7 @@ * Bug fixes and cleanup by Philippe De Muyter (phdm at macqel.be) * Copyright (c) 2004-2006 Macq Electronique SA. * - * Copyright (C) 2010 Freescale Semiconductor, Inc. + * Copyright (C) 2010-2011 Freescale Semiconductor, Inc. */ #include <linux/module.h> @@ -72,6 +72,10 @@ #define FEC_QUIRK_SWAP_FRAME (1 << 1) /* Controller uses gasket */ #define FEC_QUIRK_USE_GASKET (1 << 2) +/* Controller has GBIT support */ +#define FEC_QUIRK_HAS_GBIT (1 << 3) +/* Controller's phy_speed bit field need to minus one */ +#define FEC_QUIRK_PHY_SPEED_MINUS_ONE (1 << 4) static struct platform_device_id fec_devtype[] = { { @@ -88,6 +92,10 @@ static struct platform_device_id fec_devtype[] = { .name = "imx28-fec", .driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME, }, { + .name = "imx6q-fec", + .driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT | + FEC_QUIRK_PHY_SPEED_MINUS_ONE, + }, { /* sentinel */ } }; @@ -97,12 +105,14 @@ enum imx_fec_type { IMX25_FEC = 1, /* runs on i.mx25/50/53 */ IMX27_FEC, /* runs on i.mx27/35/51 */ IMX28_FEC, + IMX6Q_FEC, }; static const struct of_device_id fec_dt_ids[] = { { .compatible = "fsl,imx25-fec", .data = &fec_devtype[IMX25_FEC], }, { .compatible = "fsl,imx27-fec", .data = &fec_devtype[IMX27_FEC], }, { .compatible = "fsl,imx28-fec", .data = &fec_devtype[IMX28_FEC], }, + { .compatible = "fsl,imx6q-fec", .data = &fec_devtype[IMX6Q_FEC], }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, fec_dt_ids); @@ -373,6 +383,7 @@ fec_restart(struct net_device *ndev, int duplex) int i; u32 temp_mac[2]; u32 rcntl = OPT_FRAME_SIZE | 0x04; + u32 ecntl = 0x2; /* ETHEREN */ /* Whack a reset. We should wait for this. */ writel(1, fep->hwp + FEC_ECNTRL); @@ -442,18 +453,23 @@ fec_restart(struct net_device *ndev, int duplex) /* Enable flow control and length check */ rcntl |= 0x40000000 | 0x00000020; - /* MII or RMII */ - if (fep->phy_interface == PHY_INTERFACE_MODE_RMII) + /* RGMII, RMII or MII */ + if (fep->phy_interface == PHY_INTERFACE_MODE_RGMII) + rcntl |= (1 << 6); + else if (fep->phy_interface == PHY_INTERFACE_MODE_RMII) rcntl |= (1 << 8); else rcntl &= ~(1 << 8); - /* 10M or 100M */ - if (fep->phy_dev && fep->phy_dev->speed == SPEED_100) - rcntl &= ~(1 << 9); - else - rcntl |= (1 << 9); - + /* 1G, 100M or 10M */ + if (fep->phy_dev) { + if (fep->phy_dev->speed == SPEED_1000) + ecntl |= (1 << 5); + else if (fep->phy_dev->speed == SPEED_100) + rcntl &= ~(1 << 9); + else + rcntl |= (1 << 9); + } } else { #ifdef FEC_MIIGSK_ENR if (id_entry->driver_data & FEC_QUIRK_USE_GASKET) { @@ -478,8 +494,15 @@ fec_restart(struct net_device *ndev, int duplex) } writel(rcntl, fep->hwp + FEC_R_CNTRL); + if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) { + /* enable ENET endian swap */ + ecntl |= (1 << 8); + /* enable ENET store and forward mode */ + writel(1 << 8, fep->hwp + FEC_X_WMRK); + } + /* And last, enable the transmit and receive processing */ - writel(2, fep->hwp + FEC_ECNTRL); + writel(ecntl, fep->hwp + FEC_ECNTRL); writel(0, fep->hwp + FEC_R_DES_ACTIVE); /* Enable interrupts we wish to service */ @@ -490,6 +513,8 @@ static void fec_stop(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); + const struct platform_device_id *id_entry = + platform_get_device_id(fep->pdev); /* We cannot expect a graceful transmit stop without link !!! */ if (fep->link) { @@ -504,6 +529,10 @@ fec_stop(struct net_device *ndev) udelay(10); writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED); writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK); + + /* We have to keep ENET enabled to have MII interrupt stay working */ + if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) + writel(2, fep->hwp + FEC_ECNTRL); } @@ -918,6 +947,8 @@ static int fec_enet_mdio_reset(struct mii_bus *bus) static int fec_enet_mii_probe(struct net_device *ndev) { struct fec_enet_private *fep = netdev_priv(ndev); + const struct platform_device_id *id_entry = + platform_get_device_id(fep->pdev); struct phy_device *phy_dev = NULL; char mdio_bus_id[MII_BUS_ID_SIZE]; char phy_name[MII_BUS_ID_SIZE + 3]; @@ -949,14 +980,18 @@ static int fec_enet_mii_probe(struct net_device *ndev) snprintf(phy_name, MII_BUS_ID_SIZE, PHY_ID_FMT, mdio_bus_id, phy_id); phy_dev = phy_connect(ndev, phy_name, &fec_enet_adjust_link, 0, - PHY_INTERFACE_MODE_MII); + fep->phy_interface); if (IS_ERR(phy_dev)) { printk(KERN_ERR "%s: could not attach to PHY\n", ndev->name); return PTR_ERR(phy_dev); } /* mask with MAC supported features */ - phy_dev->supported &= PHY_BASIC_FEATURES; + if (id_entry->driver_data & FEC_QUIRK_HAS_GBIT) + phy_dev->supported &= PHY_GBIT_FEATURES; + else + phy_dev->supported &= PHY_BASIC_FEATURES; + phy_dev->advertising = phy_dev->supported; fep->phy_dev = phy_dev; @@ -1008,6 +1043,8 @@ static int fec_enet_mii_init(struct platform_device *pdev) * Set MII speed to 2.5 MHz (= clk_get_rate() / 2 * phy_speed) */ fep->phy_speed = DIV_ROUND_UP(clk_get_rate(fep->clk), 5000000) << 1; + if (id_entry->driver_data & FEC_QUIRK_PHY_SPEED_MINUS_ONE) + fep->phy_speed--; writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED); fep->mii_bus = mdiobus_alloc(); -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] net/fec: add imx6q enet support 2011-09-21 11:10 ` [PATCH v2 3/3] net/fec: add imx6q enet support Shawn Guo @ 2011-09-21 11:07 ` Fabio Estevam 2011-09-21 11:28 ` Shawn Guo 2011-09-21 11:32 ` Wolfram Sang 1 sibling, 1 reply; 15+ messages in thread From: Fabio Estevam @ 2011-09-21 11:07 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 21, 2011 at 8:10 AM, Shawn Guo <shawn.guo@linaro.org> wrote: ... > diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig > index 4dbe41f..1cf6716 100644 > --- a/drivers/net/ethernet/freescale/Kconfig > +++ b/drivers/net/ethernet/freescale/Kconfig > @@ -7,7 +7,7 @@ config NET_VENDOR_FREESCALE > ? ? ? ?default y > ? ? ? ?depends on FSL_SOC || QUICC_ENGINE || CPM1 || CPM2 || PPC_MPC512x || \ > ? ? ? ? ? ? ? ? ? M523x || M527x || M5272 || M528x || M520x || M532x || \ > - ? ? ? ? ? ? ? ? ?IMX_HAVE_PLATFORM_FEC || MXS_HAVE_PLATFORM_FEC || \ > + ? ? ? ? ? ? ? ? ?ARCH_MXC || ARCH_MXS || \ Do you need to get rid of IMX_HAVE_PLATFORM_FEC and MXS_HAVE_PLATFORM_FEC? By doing that you are selecting FEC for SoCs that don?t have this module such as MX1/MX21/MX31. Regards, Fabio Estevan ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] net/fec: add imx6q enet support 2011-09-21 11:07 ` Fabio Estevam @ 2011-09-21 11:28 ` Shawn Guo 0 siblings, 0 replies; 15+ messages in thread From: Shawn Guo @ 2011-09-21 11:28 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 21, 2011 at 08:07:42AM -0300, Fabio Estevam wrote: > On Wed, Sep 21, 2011 at 8:10 AM, Shawn Guo <shawn.guo@linaro.org> wrote: > ... > > diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig > > index 4dbe41f..1cf6716 100644 > > --- a/drivers/net/ethernet/freescale/Kconfig > > +++ b/drivers/net/ethernet/freescale/Kconfig > > @@ -7,7 +7,7 @@ config NET_VENDOR_FREESCALE > > ? ? ? ?default y > > ? ? ? ?depends on FSL_SOC || QUICC_ENGINE || CPM1 || CPM2 || PPC_MPC512x || \ > > ? ? ? ? ? ? ? ? ? M523x || M527x || M5272 || M528x || M520x || M532x || \ > > - ? ? ? ? ? ? ? ? ?IMX_HAVE_PLATFORM_FEC || MXS_HAVE_PLATFORM_FEC || \ > > + ? ? ? ? ? ? ? ? ?ARCH_MXC || ARCH_MXS || \ > > Do you need to get rid of IMX_HAVE_PLATFORM_FEC and MXS_HAVE_PLATFORM_FEC? > Yes, I do. We are moving to device tree, so the Kconfig symbol IMX_HAVE_PLATFORM_FEC makes no sense any more, as the platform device registration will be done by DT core according whether there is a FEC node in device tree or not. > By doing that you are selecting FEC for SoCs that don?t have this > module such as MX1/MX21/MX31. > No. By doing that, FEC will not be selected for any SoCs by default. You need to select it explicitly if you are building a platform with FEC support. -- Regards, Shawn ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] net/fec: add imx6q enet support 2011-09-21 11:10 ` [PATCH v2 3/3] net/fec: add imx6q enet support Shawn Guo 2011-09-21 11:07 ` Fabio Estevam @ 2011-09-21 11:32 ` Wolfram Sang 2011-09-21 11:58 ` Shawn Guo 1 sibling, 1 reply; 15+ messages in thread From: Wolfram Sang @ 2011-09-21 11:32 UTC (permalink / raw) To: linux-arm-kernel > +/* Controller has GBIT support */ > +#define FEC_QUIRK_HAS_GBIT (1 << 3) Heh, this is not really a quirk, but a nice feature :) I think we can drop QUIRK if we see driver_data more as "flags" instead of "quirks"? Minor, though. > MODULE_DEVICE_TABLE(of, fec_dt_ids); > @@ -373,6 +383,7 @@ fec_restart(struct net_device *ndev, int duplex) > int i; > u32 temp_mac[2]; > u32 rcntl = OPT_FRAME_SIZE | 0x04; > + u32 ecntl = 0x2; /* ETHEREN */ Also minor, but the patch looks like a good oportunity to start replacing magic values with proper defines? Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110921/1d03569b/attachment.sig> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] net/fec: add imx6q enet support 2011-09-21 11:32 ` Wolfram Sang @ 2011-09-21 11:58 ` Shawn Guo 2011-09-21 12:26 ` Wolfram Sang 0 siblings, 1 reply; 15+ messages in thread From: Shawn Guo @ 2011-09-21 11:58 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 21, 2011 at 01:32:39PM +0200, Wolfram Sang wrote: > > > +/* Controller has GBIT support */ > > +#define FEC_QUIRK_HAS_GBIT (1 << 3) > > Heh, this is not really a quirk, but a nice feature :) I think we can > drop QUIRK if we see driver_data more as "flags" instead of "quirks"? > Minor, though. > As you have told, all these FEC_QUIRK_* are just flags actually. The name was pick to keep the consistency, as they are all used for the same purpose. > > MODULE_DEVICE_TABLE(of, fec_dt_ids); > > @@ -373,6 +383,7 @@ fec_restart(struct net_device *ndev, int duplex) > > int i; > > u32 temp_mac[2]; > > u32 rcntl = OPT_FRAME_SIZE | 0x04; > > + u32 ecntl = 0x2; /* ETHEREN */ > > Also minor, but the patch looks like a good oportunity to start > replacing magic values with proper defines? > There are already so many magic numbers. It really deserves a separated patch. I heard that Uwe had a plan to do that some time ago. He gives up now? :) -- Regards, Shawn ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] net/fec: add imx6q enet support 2011-09-21 11:58 ` Shawn Guo @ 2011-09-21 12:26 ` Wolfram Sang 0 siblings, 0 replies; 15+ messages in thread From: Wolfram Sang @ 2011-09-21 12:26 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 21, 2011 at 07:58:22PM +0800, Shawn Guo wrote: > On Wed, Sep 21, 2011 at 01:32:39PM +0200, Wolfram Sang wrote: > > > > > +/* Controller has GBIT support */ > > > +#define FEC_QUIRK_HAS_GBIT (1 << 3) > > > > Heh, this is not really a quirk, but a nice feature :) I think we can > > drop QUIRK if we see driver_data more as "flags" instead of "quirks"? > > Minor, though. > > > As you have told, all these FEC_QUIRK_* are just flags actually. The > name was pick to keep the consistency, as they are all used for the > same purpose. I think introducing FEC_FEATURE_HAS_GBIT would be consistent enough, but as I said, I don't mind much. > > Also minor, but the patch looks like a good oportunity to start > > replacing magic values with proper defines? > > > There are already so many magic numbers. It really deserves a separated > patch. That is the other possibility, yes. Which sadly never happened. > I heard that Uwe had a plan to do that some time ago. He gives up > now? :) I don't know about this case. Also, it is not about blaming here. It is totally okay for you to say that you don't want to change your patch to start replacing the magic values. I mainly wanted to point out the oportunity here. Regards, Wolfram -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110921/780cd0f7/attachment.sig> ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-09-21 13:18 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-21 11:10 [PATCH v2 0/3] add fec support for imx6q Shawn Guo 2011-09-21 11:10 ` [PATCH v2 1/3] net/fec: change phy-reset-gpio request warning to debug message Shawn Guo 2011-09-21 11:25 ` Wolfram Sang 2011-09-21 12:03 ` Shawn Guo 2011-09-21 12:11 ` Wolfram Sang 2011-09-21 12:44 ` Shawn Guo 2011-09-21 12:59 ` Wolfram Sang 2011-09-21 13:18 ` Shawn Guo 2011-09-21 11:10 ` [PATCH v2 2/3] net/fec: fix fec1 check in fec_enet_mii_init() Shawn Guo 2011-09-21 11:10 ` [PATCH v2 3/3] net/fec: add imx6q enet support Shawn Guo 2011-09-21 11:07 ` Fabio Estevam 2011-09-21 11:28 ` Shawn Guo 2011-09-21 11:32 ` Wolfram Sang 2011-09-21 11:58 ` Shawn Guo 2011-09-21 12:26 ` Wolfram Sang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).