RE: [PATCH 1/1] Revert net: fec: Ensure clocks are enabled while using mdio bus
From: Peter Chen peter.c...@freescale.com Sent: Friday, August 14, 2015 1:48 PM To: da...@davemloft.net Cc: Chen Peter-B29397; netdev@vger.kernel.org; Duan Fugang-B38611; shawn@linaro.org; Estevam Fabio-R49496; tyler.ba...@linaro.org; Lucas Stach; Andrew Lunn Subject: [PATCH 1/1] Revert net: fec: Ensure clocks are enabled while using mdio bus It causes the i.mx6sx sdb board hang when using nfsroot during boots up at v4.2-rc6. This reverts commit 8fff755e9f8d0f70a595e79f248695ce6aef5cc3. Cc: netdev@vger.kernel.org Cc: Fugang Duan b38...@freescale.com Cc: shawn@linaro.org Cc: fabio.este...@freescale.com Cc: tyler.ba...@linaro.org Cc: Lucas Stach l.st...@pengutronix.de Cc: Andrew Lunn and...@lunn.ch Signed-off-by: Peter Chen peter.c...@freescale.com --- According to Fugang Duan, the i.mx series has different clock control sequence among SoCs, this patch may only consider certain SoCs. drivers/net/ethernet/freescale/fec_main.c | 89 + -- 1 file changed, 13 insertions(+), 76 deletions(-) I suggest to revert the patch. The current patch doesn't consider i.MX6sx/i.MX7d... chips. As somebody/customer's requirement that want to use MDIO bus is independent of MAC itself, I will submit one mdio driver to separate MDIO bus and MAC driver. Regards, Andy -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Revert net: fec: Ensure clocks are enabled while using mdio bus
Am Freitag, den 14.08.2015, 08:25 + schrieb Peter Chen:
Am Freitag, den 14.08.2015, 13:47 +0800 schrieb Peter Chen:
It causes the i.mx6sx sdb board hang when using nfsroot during boots
up at v4.2-rc6.
This reverts commit 8fff755e9f8d0f70a595e79f248695ce6aef5cc3.
Cc: netdev@vger.kernel.org
Cc: Fugang Duan b38...@freescale.com
Cc: shawn@linaro.org
Cc: fabio.este...@freescale.com
Cc: tyler.ba...@linaro.org
Cc: Lucas Stach l.st...@pengutronix.de
Cc: Andrew Lunn and...@lunn.ch
Signed-off-by: Peter Chen peter.c...@freescale.com
---
According to Fugang Duan, the i.mx series has different clock control
sequence among SoCs, this patch may only consider certain SoCs.
Sorry, but NACK.
Please test current mainline (what will become v4.2-rc7). There is already a
patch in that fixes i.MX27 and probably fixes the same problem on i.MX6SX.
Would you help point to me which commit and at which tree?
Mainline, so Linus Torvalds tree.
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=14d2b7c1a96ef37eb571599c73d4a1a606b964d6
Regards,
Lucas
Peter
drivers/net/ethernet/freescale/fec_main.c | 89
+--
1 file changed, 13 insertions(+), 76 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c
b/drivers/net/ethernet/freescale/fec_main.c
index 32e3807c..5e8b837 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -24,7 +24,6 @@
#include linux/module.h
#include linux/kernel.h
#include linux/string.h
-#include linux/pm_runtime.h
#include linux/ptrace.h
#include linux/errno.h
#include linux/ioport.h
@@ -78,7 +77,6 @@ static void fec_enet_itr_coal_init(struct net_device
*ndev);
#define FEC_ENET_RAEM_V 0x8
#define FEC_ENET_RAFL_V 0x8
#define FEC_ENET_OPD_V 0xFFF0
-#define FEC_MDIO_PM_TIMEOUT 100 /* ms */
static struct platform_device_id fec_devtype[] = {
{
@@ -1769,13 +1767,7 @@ static void fec_enet_adjust_link(struct
net_device *ndev) static int fec_enet_mdio_read(struct mii_bus *bus,
int mii_id, int regnum) {
struct fec_enet_private *fep = bus-priv;
- struct device *dev = fep-pdev-dev;
unsigned long time_left;
- int ret = 0;
-
- ret = pm_runtime_get_sync(dev);
- if (IS_ERR_VALUE(ret))
- return ret;
fep-mii_timeout = 0;
init_completion(fep-mdio_done);
@@ -1791,30 +1783,18 @@ static int fec_enet_mdio_read(struct mii_bus
*bus, int mii_id, int regnum)
if (time_left == 0) {
fep-mii_timeout = 1;
netdev_err(fep-netdev, MDIO read timeout\n);
- ret = -ETIMEDOUT;
- goto out;
+ return -ETIMEDOUT;
}
- ret = FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
-
-out:
- pm_runtime_mark_last_busy(dev);
- pm_runtime_put_autosuspend(dev);
-
- return ret;
+ /* return value */
+ return FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
}
static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int
regnum,
u16 value)
{
struct fec_enet_private *fep = bus-priv;
- struct device *dev = fep-pdev-dev;
unsigned long time_left;
- int ret = 0;
-
- ret = pm_runtime_get_sync(dev);
- if (IS_ERR_VALUE(ret))
- return ret;
fep-mii_timeout = 0;
init_completion(fep-mdio_done);
@@ -1831,13 +1811,10 @@ static int fec_enet_mdio_write(struct mii_bus
*bus, int mii_id, int regnum,
if (time_left == 0) {
fep-mii_timeout = 1;
netdev_err(fep-netdev, MDIO write timeout\n);
- ret = -ETIMEDOUT;
+ return -ETIMEDOUT;
}
- pm_runtime_mark_last_busy(dev);
- pm_runtime_put_autosuspend(dev);
-
- return ret;
+ return 0;
}
static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
@@ -1849,6 +1826,9 @@ static int fec_enet_clk_enable(struct net_device
*ndev, bool enable)
ret = clk_prepare_enable(fep-clk_ahb);
if (ret)
return ret;
+ ret = clk_prepare_enable(fep-clk_ipg);
+ if (ret)
+ goto failed_clk_ipg;
if (fep-clk_enet_out) {
ret = clk_prepare_enable(fep-clk_enet_out);
if (ret)
@@ -1872,6 +1852,7 @@ static int fec_enet_clk_enable(struct net_device
*ndev, bool enable)
}
} else {
clk_disable_unprepare(fep-clk_ahb);
+ clk_disable_unprepare(fep-clk_ipg);
if (fep-clk_enet_out)
clk_disable_unprepare(fep-clk_enet_out);
if (fep-clk_ptp) {
@@ -1893,6 +1874,8 @@ failed_clk_ptp:
if (fep-clk_enet_out)
clk_disable_unprepare(fep-clk_enet_out);
failed_clk_enet_out:
+
Re: [PATCH 1/1] Revert net: fec: Ensure clocks are enabled while using mdio bus
Am Freitag, den 14.08.2015, 13:47 +0800 schrieb Peter Chen:
It causes the i.mx6sx sdb board hang when using nfsroot during boots
up at v4.2-rc6.
This reverts commit 8fff755e9f8d0f70a595e79f248695ce6aef5cc3.
Cc: netdev@vger.kernel.org
Cc: Fugang Duan b38...@freescale.com
Cc: shawn@linaro.org
Cc: fabio.este...@freescale.com
Cc: tyler.ba...@linaro.org
Cc: Lucas Stach l.st...@pengutronix.de
Cc: Andrew Lunn and...@lunn.ch
Signed-off-by: Peter Chen peter.c...@freescale.com
---
According to Fugang Duan, the i.mx series has different clock control
sequence among SoCs, this patch may only consider certain SoCs.
Sorry, but NACK.
Please test current mainline (what will become v4.2-rc7). There is
already a patch in that fixes i.MX27 and probably fixes the same problem
on i.MX6SX.
drivers/net/ethernet/freescale/fec_main.c | 89
+--
1 file changed, 13 insertions(+), 76 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c
b/drivers/net/ethernet/freescale/fec_main.c
index 32e3807c..5e8b837 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -24,7 +24,6 @@
#include linux/module.h
#include linux/kernel.h
#include linux/string.h
-#include linux/pm_runtime.h
#include linux/ptrace.h
#include linux/errno.h
#include linux/ioport.h
@@ -78,7 +77,6 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
#define FEC_ENET_RAEM_V 0x8
#define FEC_ENET_RAFL_V 0x8
#define FEC_ENET_OPD_V 0xFFF0
-#define FEC_MDIO_PM_TIMEOUT 100 /* ms */
static struct platform_device_id fec_devtype[] = {
{
@@ -1769,13 +1767,7 @@ static void fec_enet_adjust_link(struct net_device
*ndev)
static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
{
struct fec_enet_private *fep = bus-priv;
- struct device *dev = fep-pdev-dev;
unsigned long time_left;
- int ret = 0;
-
- ret = pm_runtime_get_sync(dev);
- if (IS_ERR_VALUE(ret))
- return ret;
fep-mii_timeout = 0;
init_completion(fep-mdio_done);
@@ -1791,30 +1783,18 @@ static int fec_enet_mdio_read(struct mii_bus *bus,
int mii_id, int regnum)
if (time_left == 0) {
fep-mii_timeout = 1;
netdev_err(fep-netdev, MDIO read timeout\n);
- ret = -ETIMEDOUT;
- goto out;
+ return -ETIMEDOUT;
}
- ret = FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
-
-out:
- pm_runtime_mark_last_busy(dev);
- pm_runtime_put_autosuspend(dev);
-
- return ret;
+ /* return value */
+ return FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
}
static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
u16 value)
{
struct fec_enet_private *fep = bus-priv;
- struct device *dev = fep-pdev-dev;
unsigned long time_left;
- int ret = 0;
-
- ret = pm_runtime_get_sync(dev);
- if (IS_ERR_VALUE(ret))
- return ret;
fep-mii_timeout = 0;
init_completion(fep-mdio_done);
@@ -1831,13 +1811,10 @@ static int fec_enet_mdio_write(struct mii_bus *bus,
int mii_id, int regnum,
if (time_left == 0) {
fep-mii_timeout = 1;
netdev_err(fep-netdev, MDIO write timeout\n);
- ret = -ETIMEDOUT;
+ return -ETIMEDOUT;
}
- pm_runtime_mark_last_busy(dev);
- pm_runtime_put_autosuspend(dev);
-
- return ret;
+ return 0;
}
static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
@@ -1849,6 +1826,9 @@ static int fec_enet_clk_enable(struct net_device *ndev,
bool enable)
ret = clk_prepare_enable(fep-clk_ahb);
if (ret)
return ret;
+ ret = clk_prepare_enable(fep-clk_ipg);
+ if (ret)
+ goto failed_clk_ipg;
if (fep-clk_enet_out) {
ret = clk_prepare_enable(fep-clk_enet_out);
if (ret)
@@ -1872,6 +1852,7 @@ static int fec_enet_clk_enable(struct net_device *ndev,
bool enable)
}
} else {
clk_disable_unprepare(fep-clk_ahb);
+ clk_disable_unprepare(fep-clk_ipg);
if (fep-clk_enet_out)
clk_disable_unprepare(fep-clk_enet_out);
if (fep-clk_ptp) {
@@ -1893,6 +1874,8 @@ failed_clk_ptp:
if (fep-clk_enet_out)
clk_disable_unprepare(fep-clk_enet_out);
failed_clk_enet_out:
+ clk_disable_unprepare(fep-clk_ipg);
+failed_clk_ipg:
clk_disable_unprepare(fep-clk_ahb);
return ret;
@@ -2864,14 +2847,10 @@ fec_enet_open(struct net_device *ndev)
struct fec_enet_private *fep = netdev_priv(ndev);
int ret;
- ret = pm_runtime_get_sync(fep-pdev-dev);
-
RE: [PATCH 1/1] Revert net: fec: Ensure clocks are enabled while using mdio bus
Am Freitag, den 14.08.2015, 13:47 +0800 schrieb Peter Chen:
It causes the i.mx6sx sdb board hang when using nfsroot during boots
up at v4.2-rc6.
This reverts commit 8fff755e9f8d0f70a595e79f248695ce6aef5cc3.
Cc: netdev@vger.kernel.org
Cc: Fugang Duan b38...@freescale.com
Cc: shawn@linaro.org
Cc: fabio.este...@freescale.com
Cc: tyler.ba...@linaro.org
Cc: Lucas Stach l.st...@pengutronix.de
Cc: Andrew Lunn and...@lunn.ch
Signed-off-by: Peter Chen peter.c...@freescale.com
---
According to Fugang Duan, the i.mx series has different clock control
sequence among SoCs, this patch may only consider certain SoCs.
Sorry, but NACK.
Please test current mainline (what will become v4.2-rc7). There is already a
patch in that fixes i.MX27 and probably fixes the same problem on i.MX6SX.
Would you help point to me which commit and at which tree?
Peter
drivers/net/ethernet/freescale/fec_main.c | 89
+--
1 file changed, 13 insertions(+), 76 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c
b/drivers/net/ethernet/freescale/fec_main.c
index 32e3807c..5e8b837 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -24,7 +24,6 @@
#include linux/module.h
#include linux/kernel.h
#include linux/string.h
-#include linux/pm_runtime.h
#include linux/ptrace.h
#include linux/errno.h
#include linux/ioport.h
@@ -78,7 +77,6 @@ static void fec_enet_itr_coal_init(struct net_device
*ndev);
#define FEC_ENET_RAEM_V0x8
#define FEC_ENET_RAFL_V0x8
#define FEC_ENET_OPD_V 0xFFF0
-#define FEC_MDIO_PM_TIMEOUT 100 /* ms */
static struct platform_device_id fec_devtype[] = {
{
@@ -1769,13 +1767,7 @@ static void fec_enet_adjust_link(struct
net_device *ndev) static int fec_enet_mdio_read(struct mii_bus *bus,
int mii_id, int regnum) {
struct fec_enet_private *fep = bus-priv;
- struct device *dev = fep-pdev-dev;
unsigned long time_left;
- int ret = 0;
-
- ret = pm_runtime_get_sync(dev);
- if (IS_ERR_VALUE(ret))
- return ret;
fep-mii_timeout = 0;
init_completion(fep-mdio_done);
@@ -1791,30 +1783,18 @@ static int fec_enet_mdio_read(struct mii_bus
*bus, int mii_id, int regnum)
if (time_left == 0) {
fep-mii_timeout = 1;
netdev_err(fep-netdev, MDIO read timeout\n);
- ret = -ETIMEDOUT;
- goto out;
+ return -ETIMEDOUT;
}
- ret = FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
-
-out:
- pm_runtime_mark_last_busy(dev);
- pm_runtime_put_autosuspend(dev);
-
- return ret;
+ /* return value */
+ return FEC_MMFR_DATA(readl(fep-hwp + FEC_MII_DATA));
}
static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
u16 value)
{
struct fec_enet_private *fep = bus-priv;
- struct device *dev = fep-pdev-dev;
unsigned long time_left;
- int ret = 0;
-
- ret = pm_runtime_get_sync(dev);
- if (IS_ERR_VALUE(ret))
- return ret;
fep-mii_timeout = 0;
init_completion(fep-mdio_done);
@@ -1831,13 +1811,10 @@ static int fec_enet_mdio_write(struct mii_bus
*bus, int mii_id, int regnum,
if (time_left == 0) {
fep-mii_timeout = 1;
netdev_err(fep-netdev, MDIO write timeout\n);
- ret = -ETIMEDOUT;
+ return -ETIMEDOUT;
}
- pm_runtime_mark_last_busy(dev);
- pm_runtime_put_autosuspend(dev);
-
- return ret;
+ return 0;
}
static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
@@ -1849,6 +1826,9 @@ static int fec_enet_clk_enable(struct net_device
*ndev, bool enable)
ret = clk_prepare_enable(fep-clk_ahb);
if (ret)
return ret;
+ ret = clk_prepare_enable(fep-clk_ipg);
+ if (ret)
+ goto failed_clk_ipg;
if (fep-clk_enet_out) {
ret = clk_prepare_enable(fep-clk_enet_out);
if (ret)
@@ -1872,6 +1852,7 @@ static int fec_enet_clk_enable(struct net_device
*ndev, bool enable)
}
} else {
clk_disable_unprepare(fep-clk_ahb);
+ clk_disable_unprepare(fep-clk_ipg);
if (fep-clk_enet_out)
clk_disable_unprepare(fep-clk_enet_out);
if (fep-clk_ptp) {
@@ -1893,6 +1874,8 @@ failed_clk_ptp:
if (fep-clk_enet_out)
clk_disable_unprepare(fep-clk_enet_out);
failed_clk_enet_out:
+ clk_disable_unprepare(fep-clk_ipg);
+failed_clk_ipg:
clk_disable_unprepare(fep-clk_ahb);
return ret;
@@ -2864,14 +2847,10 @@ fec_enet_open(struct net_device *ndev)
struct fec_enet_private *fep = netdev_priv(ndev);
int ret;
-
Re: [PATCH 1/1] Revert net: fec: Ensure clocks are enabled while using mdio bus
On Fri, Aug 14, 2015 at 10:27:33AM +0200, Lucas Stach wrote: Am Freitag, den 14.08.2015, 08:25 + schrieb Peter Chen: Am Freitag, den 14.08.2015, 13:47 +0800 schrieb Peter Chen: It causes the i.mx6sx sdb board hang when using nfsroot during boots up at v4.2-rc6. This reverts commit 8fff755e9f8d0f70a595e79f248695ce6aef5cc3. Cc: netdev@vger.kernel.org Cc: Fugang Duan b38...@freescale.com Cc: shawn@linaro.org Cc: fabio.este...@freescale.com Cc: tyler.ba...@linaro.org Cc: Lucas Stach l.st...@pengutronix.de Cc: Andrew Lunn and...@lunn.ch Signed-off-by: Peter Chen peter.c...@freescale.com --- According to Fugang Duan, the i.mx series has different clock control sequence among SoCs, this patch may only consider certain SoCs. Sorry, but NACK. Please test current mainline (what will become v4.2-rc7). There is already a patch in that fixes i.MX27 and probably fixes the same problem on i.MX6SX. Would you help point to me which commit and at which tree? Mainline, so Linus Torvalds tree. http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=14d2b7c1a96ef37eb571599c73d4a1a606b964d6 It fixes my imx6sx-sdb board. It is interesting that there was no problem for some platforms, but with problem for others. Your fix is a common runtime PM fix. Again, why we need this as a bug-fix, not but as new feature for next rc1? -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Revert net: fec: Ensure clocks are enabled while using mdio bus
Am Freitag, den 14.08.2015, 16:14 +0800 schrieb Peter Chen: On Fri, Aug 14, 2015 at 10:27:33AM +0200, Lucas Stach wrote: Am Freitag, den 14.08.2015, 08:25 + schrieb Peter Chen: Am Freitag, den 14.08.2015, 13:47 +0800 schrieb Peter Chen: It causes the i.mx6sx sdb board hang when using nfsroot during boots up at v4.2-rc6. This reverts commit 8fff755e9f8d0f70a595e79f248695ce6aef5cc3. Cc: netdev@vger.kernel.org Cc: Fugang Duan b38...@freescale.com Cc: shawn@linaro.org Cc: fabio.este...@freescale.com Cc: tyler.ba...@linaro.org Cc: Lucas Stach l.st...@pengutronix.de Cc: Andrew Lunn and...@lunn.ch Signed-off-by: Peter Chen peter.c...@freescale.com --- According to Fugang Duan, the i.mx series has different clock control sequence among SoCs, this patch may only consider certain SoCs. Sorry, but NACK. Please test current mainline (what will become v4.2-rc7). There is already a patch in that fixes i.MX27 and probably fixes the same problem on i.MX6SX. Would you help point to me which commit and at which tree? Mainline, so Linus Torvalds tree. http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=14d2b7c1a96ef37eb571599c73d4a1a606b964d6 It fixes my imx6sx-sdb board. It is interesting that there was no problem for some platforms, but with problem for others. Your fix is a common runtime PM fix. That's because on i.MX6Q/DL IPG and AHB clock are the same clock, on i.MX27 and apparently i.MX6SX they are different, so those chips fail if RPM is disabling the clock at the wrong point in time. Again, why we need this as a bug-fix, not but as new feature for next rc1? It fixes MDIO attached switches on Vybrid, but you are right this probably could have waited until the next merge window. But this is the wrong question to ask after we got in all the fixes to keep things from regressing. Seeing that people test those things pretty late (the original broken patch got in with -rc2!) moving things to the next merge window would just have people complaining during the v4.3 RC phase, instead of now during the 4.2 RC phase. Takeaway for everyone involved: test things more thoroughly and earlier. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
