All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support
@ 2014-12-24  9:30 Fugang Duan
  2014-12-24  9:30 ` [PATCH net-next v1 1/3] " Fugang Duan
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Fugang Duan @ 2014-12-24  9:30 UTC (permalink / raw)
  To: shawn.guo, davem; +Cc: netdev, bhutchings, stephen, b38611

The patch series enable FEC Wake-on-LAN feature for i.MX6q/dl and i.MX6SX SOCs.
FEC HW support sleep mode, when system in suspend status with FEC all clock gate
off, magic packet can wake up system. For different SOCs, there have special SOC
GPR register to let FEC enter sleep mode or exit sleep mode, add these to platform
callback for driver' call.

Patch#1: add WOL interface supports.
Patch#2: add SOC special sleep of/off operations for driver's sleep callback.
Patch#3: add magic pattern support for devicetree.

Fugang Duan (3):
  net: fec: add Wake-on-LAN support
  ARM: imx: add FEC sleep mode callback function
  ARM: dts: imx6qdl: enable FEC magic-packet feature

 Documentation/devicetree/bindings/net/fsl-fec.txt |    2 +
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi          |    1 +
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi            |    1 +
 arch/arm/mach-imx/mach-imx6q.c                    |   41 ++++++++-
 arch/arm/mach-imx/mach-imx6sx.c                   |   50 ++++++++++
 drivers/net/ethernet/freescale/fec.h              |    2 +
 drivers/net/ethernet/freescale/fec_main.c         |  104 +++++++++++++++++++--
 include/linux/fec.h                               |    1 +
 8 files changed, 191 insertions(+), 11 deletions(-)

-- 
1.7.8

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH net-next v1 1/3] net: fec: add Wake-on-LAN support
  2014-12-24  9:30 [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support Fugang Duan
@ 2014-12-24  9:30 ` Fugang Duan
  2014-12-24  9:30 ` [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function Fugang Duan
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Fugang Duan @ 2014-12-24  9:30 UTC (permalink / raw)
  To: shawn.guo, davem; +Cc: netdev, bhutchings, stephen, b38611

Support for Wake-on-LAN using Magic Packet. ENET IP supports sleep mode
in low power status, when system enter suspend status, Magic packet can
wake up system even if all SOC clocks are gate. The patch doing below things:
- flagging the device as a wakeup source for the system, as well as
  its Wake-on-LAN interrupt
- prepare the hardware for entering WoL mode
- add standard ethtool WOL interface
- enable the ENET interrupt to wake us

Tested on i.MX6q/dl sabresd, sabreauto boards, i.MX6SX arm2 boards.

Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 Documentation/devicetree/bindings/net/fsl-fec.txt |    2 +
 drivers/net/ethernet/freescale/fec.h              |    2 +
 drivers/net/ethernet/freescale/fec_main.c         |  104 +++++++++++++++++++--
 include/linux/fec.h                               |    1 +
 4 files changed, 99 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 0c8775c..a9eb611 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -22,6 +22,8 @@ Optional properties:
 - fsl,num-rx-queues : The property is valid for enet-avb IP, which supports
   hw multi queues. Should specify the rx queue number, otherwise set rx queue
   number to 1.
+- fsl,magic-packet : If present, indicates that the hardware supports waking
+  up via magic packet.
 
 Optional subnodes:
 - mdio : specifies the mdio bus in the FEC, used as a container for phy nodes
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 469691a..e51ab5d 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -356,6 +356,7 @@ struct bufdesc_ex {
 #define FEC_ENET_RXB    ((uint)0x01000000)      /* A buffer was received */
 #define FEC_ENET_MII    ((uint)0x00800000)      /* MII interrupt */
 #define FEC_ENET_EBERR  ((uint)0x00400000)      /* SDMA bus error */
+#define FEC_ENET_WAKEUP	((uint)0x00020000)	/* Wakeup request */
 #define FEC_ENET_TXF	(FEC_ENET_TXF_0 | FEC_ENET_TXF_1 | FEC_ENET_TXF_2)
 #define FEC_ENET_RXF	(FEC_ENET_RXF_0 | FEC_ENET_RXF_1 | FEC_ENET_RXF_2)
 #define FEC_ENET_TS_AVAIL       ((uint)0x00010000)
@@ -511,6 +512,7 @@ struct fec_enet_private {
 	int	irq[FEC_IRQ_NUM];
 	bool	bufdesc_ex;
 	int	pause_flag;
+	int	wol_flag;
 	u32	quirks;
 
 	struct	napi_struct napi;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5c4a8bd..615b51d 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -187,6 +187,9 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 #define FEC_MMFR_RA(v)		((v & 0x1f) << 18)
 #define FEC_MMFR_TA		(2 << 16)
 #define FEC_MMFR_DATA(v)	(v & 0xffff)
+/* FEC ECR bits definition */
+#define FEC_ECR_MAGICEN		(1 << 2)
+#define FEC_ECR_SLEEP		(1 << 3)
 
 #define FEC_MII_TIMEOUT		30000 /* us */
 
@@ -195,6 +198,9 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 
 #define FEC_PAUSE_FLAG_AUTONEG	0x1
 #define FEC_PAUSE_FLAG_ENABLE	0x2
+#define FEC_WOL_HAS_MAGIC_PACKET	(0x1 << 0)
+#define FEC_WOL_FLAG_ENABLE		(0x1 << 1)
+#define FEC_WOL_FLAG_SLEEP_ON		(0x1 << 2)
 
 #define COPYBREAK_DEFAULT	256
 
@@ -1089,7 +1095,9 @@ static void
 fec_stop(struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
+	struct fec_platform_data *pdata = fep->pdev->dev.platform_data;
 	u32 rmii_mode = readl(fep->hwp + FEC_R_CNTRL) & (1 << 8);
+	u32 val;
 
 	/* We cannot expect a graceful transmit stop without link !!! */
 	if (fep->link) {
@@ -1103,17 +1111,28 @@ fec_stop(struct net_device *ndev)
 	 * For i.MX6SX SOC, enet use AXI bus, we use disable MAC
 	 * instead of reset MAC itself.
 	 */
-	if (fep->quirks & FEC_QUIRK_HAS_AVB) {
-		writel(0, fep->hwp + FEC_ECNTRL);
+	if (!(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) {
+		if (fep->quirks & FEC_QUIRK_HAS_AVB) {
+			writel(0, fep->hwp + FEC_ECNTRL);
+		} else {
+			writel(1, fep->hwp + FEC_ECNTRL);
+			udelay(10);
+		}
+		writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
 	} else {
-		writel(1, fep->hwp + FEC_ECNTRL);
-		udelay(10);
+		writel(FEC_DEFAULT_IMASK | FEC_ENET_WAKEUP, fep->hwp + FEC_IMASK);
+		val = readl(fep->hwp + FEC_ECNTRL);
+		val |= (FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
+		writel(val, fep->hwp + FEC_ECNTRL);
+
+		if (pdata && pdata->sleep_mode_enable)
+			pdata->sleep_mode_enable(true);
 	}
 	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 (fep->quirks & FEC_QUIRK_ENET_MAC) {
+	if (fep->quirks & FEC_QUIRK_ENET_MAC &&
+		!(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) {
 		writel(2, fep->hwp + FEC_ECNTRL);
 		writel(rmii_mode, fep->hwp + FEC_R_CNTRL);
 	}
@@ -2427,6 +2446,44 @@ static int fec_enet_set_tunable(struct net_device *netdev,
 	return ret;
 }
 
+static void
+fec_enet_get_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+
+	if (fep->wol_flag & FEC_WOL_HAS_MAGIC_PACKET) {
+		wol->supported = WAKE_MAGIC;
+		wol->wolopts = fep->wol_flag & FEC_WOL_FLAG_ENABLE ? WAKE_MAGIC : 0;
+	} else {
+		wol->supported = wol->wolopts = 0;
+	}
+}
+
+static int
+fec_enet_set_wol(struct net_device *ndev, struct ethtool_wolinfo *wol)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+
+	if (!(fep->wol_flag & FEC_WOL_HAS_MAGIC_PACKET))
+		return -EINVAL;
+
+	if (wol->wolopts & ~WAKE_MAGIC)
+		return -EINVAL;
+
+	device_set_wakeup_enable(&ndev->dev, wol->wolopts & WAKE_MAGIC);
+	if (device_may_wakeup(&ndev->dev)) {
+		fep->wol_flag |= FEC_WOL_FLAG_ENABLE;
+		if (fep->irq[0] > 0)
+			enable_irq_wake(fep->irq[0]);
+	} else {
+		fep->wol_flag &= (~FEC_WOL_FLAG_ENABLE);
+		if (fep->irq[0] > 0)
+			disable_irq_wake(fep->irq[0]);
+	}
+
+	return 0;
+}
+
 static const struct ethtool_ops fec_enet_ethtool_ops = {
 	.get_settings		= fec_enet_get_settings,
 	.set_settings		= fec_enet_set_settings,
@@ -2445,6 +2502,8 @@ static const struct ethtool_ops fec_enet_ethtool_ops = {
 	.get_ts_info		= fec_enet_get_ts_info,
 	.get_tunable		= fec_enet_get_tunable,
 	.set_tunable		= fec_enet_set_tunable,
+	.get_wol		= fec_enet_get_wol,
+	.set_wol		= fec_enet_set_wol,
 };
 
 static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
@@ -2705,6 +2764,9 @@ fec_enet_open(struct net_device *ndev)
 	phy_start(fep->phy_dev);
 	netif_tx_start_all_queues(ndev);
 
+	device_set_wakeup_enable(&ndev->dev, fep->wol_flag &
+				 FEC_WOL_FLAG_ENABLE);
+
 	return 0;
 
 err_enet_mii_probe:
@@ -3153,6 +3215,9 @@ fec_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, ndev);
 
+	if (of_get_property(np, "fsl,magic-packet", NULL))
+		fep->wol_flag |= FEC_WOL_HAS_MAGIC_PACKET;
+
 	phy_node = of_parse_phandle(np, "phy-handle", 0);
 	if (!phy_node && of_phy_is_fixed_link(np)) {
 		ret = of_phy_register_fixed_link(np);
@@ -3247,6 +3312,8 @@ fec_probe(struct platform_device *pdev)
 				       0, pdev->name, ndev);
 		if (ret)
 			goto failed_irq;
+
+		fep->irq[i] = irq;
 	}
 
 	init_completion(&fep->mdio_done);
@@ -3263,6 +3330,9 @@ fec_probe(struct platform_device *pdev)
 	if (ret)
 		goto failed_register;
 
+	device_init_wakeup(&ndev->dev, fep->wol_flag &
+			   FEC_WOL_HAS_MAGIC_PACKET);
+
 	if (fep->bufdesc_ex && fep->ptp_clock)
 		netdev_info(ndev, "registered PHC device %d\n", fep->dev_id);
 
@@ -3316,6 +3386,8 @@ static int __maybe_unused fec_suspend(struct device *dev)
 
 	rtnl_lock();
 	if (netif_running(ndev)) {
+		if (fep->wol_flag & FEC_WOL_FLAG_ENABLE)
+			fep->wol_flag |= FEC_WOL_FLAG_SLEEP_ON;
 		phy_stop(fep->phy_dev);
 		napi_disable(&fep->napi);
 		netif_tx_lock_bh(ndev);
@@ -3323,11 +3395,12 @@ static int __maybe_unused fec_suspend(struct device *dev)
 		netif_tx_unlock_bh(ndev);
 		fec_stop(ndev);
 		fec_enet_clk_enable(ndev, false);
-		pinctrl_pm_select_sleep_state(&fep->pdev->dev);
+		if (!(fep->wol_flag & FEC_WOL_FLAG_ENABLE))
+			pinctrl_pm_select_sleep_state(&fep->pdev->dev);
 	}
 	rtnl_unlock();
 
-	if (fep->reg_phy)
+	if (fep->reg_phy && !(fep->wol_flag & FEC_WOL_FLAG_ENABLE))
 		regulator_disable(fep->reg_phy);
 
 	/* SOC supply clock to phy, when clock is disabled, phy link down
@@ -3343,9 +3416,11 @@ static int __maybe_unused fec_resume(struct device *dev)
 {
 	struct net_device *ndev = dev_get_drvdata(dev);
 	struct fec_enet_private *fep = netdev_priv(ndev);
+	struct fec_platform_data *pdata = fep->pdev->dev.platform_data;
 	int ret;
+	int val;
 
-	if (fep->reg_phy) {
+	if (fep->reg_phy && !(fep->wol_flag & FEC_WOL_FLAG_ENABLE)) {
 		ret = regulator_enable(fep->reg_phy);
 		if (ret)
 			return ret;
@@ -3353,12 +3428,21 @@ static int __maybe_unused fec_resume(struct device *dev)
 
 	rtnl_lock();
 	if (netif_running(ndev)) {
-		pinctrl_pm_select_default_state(&fep->pdev->dev);
 		ret = fec_enet_clk_enable(ndev, true);
 		if (ret) {
 			rtnl_unlock();
 			goto failed_clk;
 		}
+		if (fep->wol_flag & FEC_WOL_FLAG_ENABLE) {
+			if (pdata && pdata->sleep_mode_enable)
+				pdata->sleep_mode_enable(false);
+			val = readl(fep->hwp + FEC_ECNTRL);
+			val &= ~(FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
+			writel(val, fep->hwp + FEC_ECNTRL);
+			fep->wol_flag &= ~FEC_WOL_FLAG_SLEEP_ON;
+		} else {
+			pinctrl_pm_select_default_state(&fep->pdev->dev);
+		}
 		fec_restart(ndev);
 		netif_tx_lock_bh(ndev);
 		netif_device_attach(ndev);
diff --git a/include/linux/fec.h b/include/linux/fec.h
index bcff455..1454a50 100644
--- a/include/linux/fec.h
+++ b/include/linux/fec.h
@@ -19,6 +19,7 @@
 struct fec_platform_data {
 	phy_interface_t phy;
 	unsigned char mac[ETH_ALEN];
+	void (*sleep_mode_enable)(int enabled);
 };
 
 #endif
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function
  2014-12-24  9:30 [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support Fugang Duan
  2014-12-24  9:30 ` [PATCH net-next v1 1/3] " Fugang Duan
@ 2014-12-24  9:30 ` Fugang Duan
  2015-01-06 11:48   ` Shawn Guo
  2014-12-24  9:30 ` [PATCH net-next v1 3/3] ARM: dts: imx6qdl: enable FEC magic-packet feature Fugang Duan
  2014-12-31 18:07 ` [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support David Miller
  3 siblings, 1 reply; 17+ messages in thread
From: Fugang Duan @ 2014-12-24  9:30 UTC (permalink / raw)
  To: shawn.guo, davem; +Cc: netdev, bhutchings, stephen, b38611

i.MX6q/dl, i.MX6SX SOCs enet support sleep mode that magic packet can
wake up system in suspend status. For different SOCs, there have some
SOC specifical GPR register to set sleep on/off mode. So add these to
callback function for driver.

Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 arch/arm/mach-imx/mach-imx6q.c  |   41 +++++++++++++++++++++++++++++++-
 arch/arm/mach-imx/mach-imx6sx.c |   50 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 5057d61..2f76168 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -31,6 +31,8 @@
 #include <linux/micrel_phy.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
+#include <linux/fec.h>
+#include <linux/netdevice.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 #include <asm/system_misc.h>
@@ -39,6 +41,35 @@
 #include "cpuidle.h"
 #include "hardware.h"
 
+static struct fec_platform_data fec_pdata;
+
+static void imx6q_fec_sleep_enable(int enabled)
+{
+	struct regmap *gpr;
+
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+	if (!IS_ERR(gpr)) {
+		if (enabled)
+			regmap_update_bits(gpr, IOMUXC_GPR13,
+					   IMX6Q_GPR13_ENET_STOP_REQ,
+					   IMX6Q_GPR13_ENET_STOP_REQ);
+
+		else
+			regmap_update_bits(gpr, IOMUXC_GPR13,
+					   IMX6Q_GPR13_ENET_STOP_REQ, 0);
+	} else
+		pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");
+}
+
+static void __init imx6q_enet_plt_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@02188000");
+	if (np && of_get_property(np, "fsl,magic-packet", NULL))
+		fec_pdata.sleep_mode_enable = imx6q_fec_sleep_enable;
+}
+
 /* For imx6q sabrelite board: set KSZ9021RN RGMII pad skew */
 static int ksz9021rn_phy_fixup(struct phy_device *phydev)
 {
@@ -261,6 +292,12 @@ static void __init imx6q_axi_init(void)
 	}
 }
 
+/* Add auxdata to pass platform data */
+static const struct of_dev_auxdata imx6q_auxdata_lookup[] __initconst = {
+	OF_DEV_AUXDATA("fsl,imx6q-fec", 0x02188000, NULL, &fec_pdata),
+	{ /* sentinel */ }
+};
+
 static void __init imx6q_init_machine(void)
 {
 	struct device *parent;
@@ -274,11 +311,13 @@ static void __init imx6q_init_machine(void)
 
 	imx6q_enet_phy_init();
 
-	of_platform_populate(NULL, of_default_bus_match_table, NULL, parent);
+	of_platform_populate(NULL, of_default_bus_match_table,
+			     imx6q_auxdata_lookup, parent);
 
 	imx_anatop_init();
 	cpu_is_imx6q() ?  imx6q_pm_init() : imx6dl_pm_init();
 	imx6q_1588_init();
+	imx6q_enet_plt_init();
 	imx6q_axi_init();
 }
 
diff --git a/arch/arm/mach-imx/mach-imx6sx.c b/arch/arm/mach-imx/mach-imx6sx.c
index 7a96c65..747b012 100644
--- a/arch/arm/mach-imx/mach-imx6sx.c
+++ b/arch/arm/mach-imx/mach-imx6sx.c
@@ -12,12 +12,62 @@
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
+#include <linux/fec.h>
+#include <linux/netdevice.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 
 #include "common.h"
 #include "cpuidle.h"
 
+static struct fec_platform_data fec_pdata[2];
+
+static void imx6sx_fec1_sleep_enable(int enabled)
+{
+	struct regmap *gpr;
+
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sx-iomuxc-gpr");
+	if (!IS_ERR(gpr)) {
+		if (enabled)
+			regmap_update_bits(gpr, IOMUXC_GPR4,
+					   IMX6SX_GPR4_FEC_ENET1_STOP_REQ,
+					   IMX6SX_GPR4_FEC_ENET1_STOP_REQ);
+		else
+			regmap_update_bits(gpr, IOMUXC_GPR4,
+					   IMX6SX_GPR4_FEC_ENET1_STOP_REQ, 0);
+	} else
+		pr_err("failed to find fsl,imx6sx-iomux-gpr regmap\n");
+}
+
+static void imx6sx_fec2_sleep_enable(int enabled)
+{
+	struct regmap *gpr;
+
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sx-iomuxc-gpr");
+	if (!IS_ERR(gpr)) {
+		if (enabled)
+			regmap_update_bits(gpr, IOMUXC_GPR4,
+					   IMX6SX_GPR4_FEC_ENET2_STOP_REQ,
+					   IMX6SX_GPR4_FEC_ENET2_STOP_REQ);
+		else
+			regmap_update_bits(gpr, IOMUXC_GPR4,
+					   IMX6SX_GPR4_FEC_ENET2_STOP_REQ, 0);
+	} else
+		pr_err("failed to find fsl,imx6sx-iomux-gpr regmap\n");
+}
+
+static void __init imx6sx_enet_plt_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@02188000");
+	if (np && of_get_property(np, "fsl,magic-packet", NULL))
+		fec_pdata[0].sleep_mode_enable = imx6sx_fec1_sleep_enable;
+	np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@021b4000");
+	if (np && of_get_property(np, "fsl,magic-packet", NULL))
+		fec_pdata[1].sleep_mode_enable = imx6sx_fec2_sleep_enable;
+}
+
 static int ar8031_phy_fixup(struct phy_device *dev)
 {
 	u16 val;
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v1 3/3] ARM: dts: imx6qdl: enable FEC magic-packet feature
  2014-12-24  9:30 [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support Fugang Duan
  2014-12-24  9:30 ` [PATCH net-next v1 1/3] " Fugang Duan
  2014-12-24  9:30 ` [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function Fugang Duan
@ 2014-12-24  9:30 ` Fugang Duan
  2014-12-31 18:07 ` [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support David Miller
  3 siblings, 0 replies; 17+ messages in thread
From: Fugang Duan @ 2014-12-24  9:30 UTC (permalink / raw)
  To: shawn.guo, davem; +Cc: netdev, bhutchings, stephen, b38611

Add FEC magic-packet feature support.

Signed-off-by: Fugang Duan <B38611@freescale.com>
---
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi |    1 +
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi   |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd6..327d362 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -67,6 +67,7 @@
 	phy-mode = "rgmii";
 	interrupts-extended = <&gpio1 6 IRQ_TYPE_LEVEL_HIGH>,
 			      <&intc 0 119 IRQ_TYPE_LEVEL_HIGH>;
+	fsl,magic-packet;
 	status = "okay";
 };
 
diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index f1cd214..6bfd0bc 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -160,6 +160,7 @@
 	pinctrl-0 = <&pinctrl_enet>;
 	phy-mode = "rgmii";
 	phy-reset-gpios = <&gpio1 25 0>;
+	fsl,magic-packet;
 	status = "okay";
 };
 
-- 
1.7.8

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support
  2014-12-24  9:30 [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support Fugang Duan
                   ` (2 preceding siblings ...)
  2014-12-24  9:30 ` [PATCH net-next v1 3/3] ARM: dts: imx6qdl: enable FEC magic-packet feature Fugang Duan
@ 2014-12-31 18:07 ` David Miller
  2014-12-31 19:03   ` Fabio Estevam
  2015-01-06 11:21   ` Shawn Guo
  3 siblings, 2 replies; 17+ messages in thread
From: David Miller @ 2014-12-31 18:07 UTC (permalink / raw)
  To: b38611; +Cc: shawn.guo, netdev, bhutchings, stephen

From: Fugang Duan <b38611@freescale.com>
Date: Wed, 24 Dec 2014 17:30:38 +0800

> The patch series enable FEC Wake-on-LAN feature for i.MX6q/dl and i.MX6SX SOCs.
> FEC HW support sleep mode, when system in suspend status with FEC all clock gate
> off, magic packet can wake up system. For different SOCs, there have special SOC
> GPR register to let FEC enter sleep mode or exit sleep mode, add these to platform
> callback for driver' call.
> 
> Patch#1: add WOL interface supports.
> Patch#2: add SOC special sleep of/off operations for driver's sleep callback.
> Patch#3: add magic pattern support for devicetree.

Series applied, thanks.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support
  2014-12-31 18:07 ` [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support David Miller
@ 2014-12-31 19:03   ` Fabio Estevam
  2014-12-31 19:20     ` David Miller
  2015-01-06 11:21   ` Shawn Guo
  1 sibling, 1 reply; 17+ messages in thread
From: Fabio Estevam @ 2014-12-31 19:03 UTC (permalink / raw)
  To: David Miller
  Cc: Duan Fugang-B38611, Shawn Guo, netdev, Ben Hutchings, Stephen Hemminger

Hi Fugang,

Just a minor comment about your patches:

On Wed, Dec 31, 2014 at 4:07 PM, David Miller <davem@davemloft.net> wrote:
> From: Fugang Duan <b38611@freescale.com>

Here we see the From field as Fugang Duan.

> Date: Wed, 24 Dec 2014 17:30:38 +0800
>
>> The patch series enable FEC Wake-on-LAN feature for i.MX6q/dl and i.MX6SX SOCs.
>> FEC HW support sleep mode, when system in suspend status with FEC all clock gate
>> off, magic packet can wake up system. For different SOCs, there have special SOC
>> GPR register to let FEC enter sleep mode or exit sleep mode, add these to platform
>> callback for driver' call.
>>
>> Patch#1: add WOL interface supports.
>> Patch#2: add SOC special sleep of/off operations for driver's sleep callback.
>> Patch#3: add magic pattern support for devicetree.
>
> Series applied, thanks.

,but when we see your patches applied they appear with Nimrod Andy as
the author instead:
https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=de40ed31b3c577cefd7b54972365a272ecbe9dd6

It would be nice if you could use the From field to match the name in
the Signed-off tag.

Regards,

Fabio Estevam

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support
  2014-12-31 19:03   ` Fabio Estevam
@ 2014-12-31 19:20     ` David Miller
  2014-12-31 19:22       ` Fabio Estevam
  0 siblings, 1 reply; 17+ messages in thread
From: David Miller @ 2014-12-31 19:20 UTC (permalink / raw)
  To: festevam; +Cc: b38611, shawn.guo, netdev, bhutchings, stephen

From: Fabio Estevam <festevam@gmail.com>
Date: Wed, 31 Dec 2014 17:03:28 -0200

> ,but when we see your patches applied they appear with Nimrod Andy as
> the author instead:
> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=de40ed31b3c577cefd7b54972365a272ecbe9dd6
> 
> It would be nice if you could use the From field to match the name in
> the Signed-off tag.

It's his business to setup things properly so they match, not
mine.  I just take what is in the patch as-is.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support
  2014-12-31 19:20     ` David Miller
@ 2014-12-31 19:22       ` Fabio Estevam
  2015-01-04  1:39         ` fugang.duan
  0 siblings, 1 reply; 17+ messages in thread
From: Fabio Estevam @ 2014-12-31 19:22 UTC (permalink / raw)
  To: David Miller
  Cc: Duan Fugang-B38611, Shawn Guo, netdev, Ben Hutchings, Stephen Hemminger

On Wed, Dec 31, 2014 at 5:20 PM, David Miller <davem@davemloft.net> wrote:
> From: Fabio Estevam <festevam@gmail.com>
> Date: Wed, 31 Dec 2014 17:03:28 -0200
>
>> ,but when we see your patches applied they appear with Nimrod Andy as
>> the author instead:
>> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=de40ed31b3c577cefd7b54972365a272ecbe9dd6
>>
>> It would be nice if you could use the From field to match the name in
>> the Signed-off tag.
>
> It's his business to setup things properly so they match, not
> mine.  I just take what is in the patch as-is.

Exactly. This was what I suggested him to do.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support
  2014-12-31 19:22       ` Fabio Estevam
@ 2015-01-04  1:39         ` fugang.duan
  0 siblings, 0 replies; 17+ messages in thread
From: fugang.duan @ 2015-01-04  1:39 UTC (permalink / raw)
  To: Fabio Estevam, David Miller
  Cc: Shawn Guo, netdev, Ben Hutchings, Stephen Hemminger

From: Fabio Estevam <festevam@gmail.com> Sent: Thursday, January 01, 2015 3:22 AM
> To: David Miller
> Cc: Duan Fugang-B38611; Shawn Guo; netdev@vger.kernel.org; Ben Hutchings;
> Stephen Hemminger
> Subject: Re: [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support
> 
> On Wed, Dec 31, 2014 at 5:20 PM, David Miller <davem@davemloft.net> wrote:
> > From: Fabio Estevam <festevam@gmail.com>
> > Date: Wed, 31 Dec 2014 17:03:28 -0200
> >
> >> ,but when we see your patches applied they appear with Nimrod Andy as
> >> the author instead:
> >> https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commi
> >> t/?id=de40ed31b3c577cefd7b54972365a272ecbe9dd6
> >>
> >> It would be nice if you could use the From field to match the name in
> >> the Signed-off tag.
> >
> > It's his business to setup things properly so they match, not mine.  I
> > just take what is in the patch as-is.
> 
> Exactly. This was what I suggested him to do.

Thanks for your suggestion.
I have these two English name. Sorry I will sync one name in later patch.

Regards,
Andy

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support
  2014-12-31 18:07 ` [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support David Miller
  2014-12-31 19:03   ` Fabio Estevam
@ 2015-01-06 11:21   ` Shawn Guo
  1 sibling, 0 replies; 17+ messages in thread
From: Shawn Guo @ 2015-01-06 11:21 UTC (permalink / raw)
  To: David Miller; +Cc: b38611, netdev, bhutchings, stephen

On Wed, Dec 31, 2014 at 01:07:27PM -0500, David Miller wrote:
> From: Fugang Duan <b38611@freescale.com>
> Date: Wed, 24 Dec 2014 17:30:38 +0800
> 
> > The patch series enable FEC Wake-on-LAN feature for i.MX6q/dl and i.MX6SX SOCs.
> > FEC HW support sleep mode, when system in suspend status with FEC all clock gate
> > off, magic packet can wake up system. For different SOCs, there have special SOC
> > GPR register to let FEC enter sleep mode or exit sleep mode, add these to platform
> > callback for driver' call.
> > 
> > Patch#1: add WOL interface supports.
> > Patch#2: add SOC special sleep of/off operations for driver's sleep callback.
> > Patch#3: add magic pattern support for devicetree.
> 
> Series applied, thanks.

I'm not sure you want to take the last two patches.  Doing so will
likely causes merge conflict between net and arm-soc tree.

Shawn

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function
  2014-12-24  9:30 ` [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function Fugang Duan
@ 2015-01-06 11:48   ` Shawn Guo
  2015-01-07  1:41     ` fugang.duan
  0 siblings, 1 reply; 17+ messages in thread
From: Shawn Guo @ 2015-01-06 11:48 UTC (permalink / raw)
  To: Fugang Duan; +Cc: davem, netdev, bhutchings, stephen

On Wed, Dec 24, 2014 at 05:30:40PM +0800, Fugang Duan wrote:
> i.MX6q/dl, i.MX6SX SOCs enet support sleep mode that magic packet can
> wake up system in suspend status. For different SOCs, there have some
> SOC specifical GPR register to set sleep on/off mode. So add these to
> callback function for driver.
> 
> Signed-off-by: Fugang Duan <B38611@freescale.com>

I do not like this patch.  In the end, this is just a GRP register bit
setup per FEC driver need.  Rather than messing up platform code for
each SoC with the same pattern, I do not see why this can not be done by
FEC driver itself.  

You can take a look at LDB driver (drivers/gpu/drm/imx/imx-ldb.c) to see
how this can be done.

Shawn

> ---
>  arch/arm/mach-imx/mach-imx6q.c  |   41 +++++++++++++++++++++++++++++++-
>  arch/arm/mach-imx/mach-imx6sx.c |   50 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 90 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index 5057d61..2f76168 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -31,6 +31,8 @@
>  #include <linux/micrel_phy.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> +#include <linux/fec.h>
> +#include <linux/netdevice.h>
>  #include <asm/mach/arch.h>
>  #include <asm/mach/map.h>
>  #include <asm/system_misc.h>
> @@ -39,6 +41,35 @@
>  #include "cpuidle.h"
>  #include "hardware.h"
>  
> +static struct fec_platform_data fec_pdata;
> +
> +static void imx6q_fec_sleep_enable(int enabled)
> +{
> +	struct regmap *gpr;
> +
> +	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
> +	if (!IS_ERR(gpr)) {
> +		if (enabled)
> +			regmap_update_bits(gpr, IOMUXC_GPR13,
> +					   IMX6Q_GPR13_ENET_STOP_REQ,
> +					   IMX6Q_GPR13_ENET_STOP_REQ);
> +
> +		else
> +			regmap_update_bits(gpr, IOMUXC_GPR13,
> +					   IMX6Q_GPR13_ENET_STOP_REQ, 0);
> +	} else
> +		pr_err("failed to find fsl,imx6q-iomux-gpr regmap\n");
> +}
> +
> +static void __init imx6q_enet_plt_init(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@02188000");
> +	if (np && of_get_property(np, "fsl,magic-packet", NULL))
> +		fec_pdata.sleep_mode_enable = imx6q_fec_sleep_enable;
> +}
> +
>  /* For imx6q sabrelite board: set KSZ9021RN RGMII pad skew */
>  static int ksz9021rn_phy_fixup(struct phy_device *phydev)
>  {
> @@ -261,6 +292,12 @@ static void __init imx6q_axi_init(void)
>  	}
>  }
>  
> +/* Add auxdata to pass platform data */
> +static const struct of_dev_auxdata imx6q_auxdata_lookup[] __initconst = {
> +	OF_DEV_AUXDATA("fsl,imx6q-fec", 0x02188000, NULL, &fec_pdata),
> +	{ /* sentinel */ }
> +};
> +
>  static void __init imx6q_init_machine(void)
>  {
>  	struct device *parent;
> @@ -274,11 +311,13 @@ static void __init imx6q_init_machine(void)
>  
>  	imx6q_enet_phy_init();
>  
> -	of_platform_populate(NULL, of_default_bus_match_table, NULL, parent);
> +	of_platform_populate(NULL, of_default_bus_match_table,
> +			     imx6q_auxdata_lookup, parent);
>  
>  	imx_anatop_init();
>  	cpu_is_imx6q() ?  imx6q_pm_init() : imx6dl_pm_init();
>  	imx6q_1588_init();
> +	imx6q_enet_plt_init();
>  	imx6q_axi_init();
>  }
>  
> diff --git a/arch/arm/mach-imx/mach-imx6sx.c b/arch/arm/mach-imx/mach-imx6sx.c
> index 7a96c65..747b012 100644
> --- a/arch/arm/mach-imx/mach-imx6sx.c
> +++ b/arch/arm/mach-imx/mach-imx6sx.c
> @@ -12,12 +12,62 @@
>  #include <linux/regmap.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> +#include <linux/fec.h>
> +#include <linux/netdevice.h>
>  #include <asm/mach/arch.h>
>  #include <asm/mach/map.h>
>  
>  #include "common.h"
>  #include "cpuidle.h"
>  
> +static struct fec_platform_data fec_pdata[2];
> +
> +static void imx6sx_fec1_sleep_enable(int enabled)
> +{
> +	struct regmap *gpr;
> +
> +	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sx-iomuxc-gpr");
> +	if (!IS_ERR(gpr)) {
> +		if (enabled)
> +			regmap_update_bits(gpr, IOMUXC_GPR4,
> +					   IMX6SX_GPR4_FEC_ENET1_STOP_REQ,
> +					   IMX6SX_GPR4_FEC_ENET1_STOP_REQ);
> +		else
> +			regmap_update_bits(gpr, IOMUXC_GPR4,
> +					   IMX6SX_GPR4_FEC_ENET1_STOP_REQ, 0);
> +	} else
> +		pr_err("failed to find fsl,imx6sx-iomux-gpr regmap\n");
> +}
> +
> +static void imx6sx_fec2_sleep_enable(int enabled)
> +{
> +	struct regmap *gpr;
> +
> +	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sx-iomuxc-gpr");
> +	if (!IS_ERR(gpr)) {
> +		if (enabled)
> +			regmap_update_bits(gpr, IOMUXC_GPR4,
> +					   IMX6SX_GPR4_FEC_ENET2_STOP_REQ,
> +					   IMX6SX_GPR4_FEC_ENET2_STOP_REQ);
> +		else
> +			regmap_update_bits(gpr, IOMUXC_GPR4,
> +					   IMX6SX_GPR4_FEC_ENET2_STOP_REQ, 0);
> +	} else
> +		pr_err("failed to find fsl,imx6sx-iomux-gpr regmap\n");
> +}
> +
> +static void __init imx6sx_enet_plt_init(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@02188000");
> +	if (np && of_get_property(np, "fsl,magic-packet", NULL))
> +		fec_pdata[0].sleep_mode_enable = imx6sx_fec1_sleep_enable;
> +	np = of_find_node_by_path("/soc/aips-bus@02100000/ethernet@021b4000");
> +	if (np && of_get_property(np, "fsl,magic-packet", NULL))
> +		fec_pdata[1].sleep_mode_enable = imx6sx_fec2_sleep_enable;
> +}
> +
>  static int ar8031_phy_fixup(struct phy_device *dev)
>  {
>  	u16 val;
> -- 
> 1.7.8
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function
  2015-01-06 11:48   ` Shawn Guo
@ 2015-01-07  1:41     ` fugang.duan
  2015-01-07  2:33       ` Shawn Guo
  0 siblings, 1 reply; 17+ messages in thread
From: fugang.duan @ 2015-01-07  1:41 UTC (permalink / raw)
  To: Shawn Guo; +Cc: davem, netdev, bhutchings, stephen

From: Shawn Guo <shawn.guo@linaro.org> Sent: Tuesday, January 06, 2015 7:48 PM
> To: Duan Fugang-B38611
> Cc: davem@davemloft.net; netdev@vger.kernel.org;
> bhutchings@solarflare.com; stephen@networkplumber.org
> Subject: Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode
> callback function
> 
> On Wed, Dec 24, 2014 at 05:30:40PM +0800, Fugang Duan wrote:
> > i.MX6q/dl, i.MX6SX SOCs enet support sleep mode that magic packet can
> > wake up system in suspend status. For different SOCs, there have some
> > SOC specifical GPR register to set sleep on/off mode. So add these to
> > callback function for driver.
> >
> > Signed-off-by: Fugang Duan <B38611@freescale.com>
> 
> I do not like this patch.  In the end, this is just a GRP register bit
> setup per FEC driver need.  Rather than messing up platform code for each
> SoC with the same pattern, I do not see why this can not be done by FEC
> driver itself.
> 
> You can take a look at LDB driver (drivers/gpu/drm/imx/imx-ldb.c) to see
> how this can be done.
> 
> Shawn
> 

Hi, Shawn,

It is SOC related setting, not fec IP itself setting, and different SOC GPR setting is not different.
So I think it better to put it to platform code.

Regards,
Andy

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function
  2015-01-07  1:41     ` fugang.duan
@ 2015-01-07  2:33       ` Shawn Guo
  2015-01-07  2:40         ` fugang.duan
  2015-01-07  3:44         ` Fabio Estevam
  0 siblings, 2 replies; 17+ messages in thread
From: Shawn Guo @ 2015-01-07  2:33 UTC (permalink / raw)
  To: fugang.duan; +Cc: davem, netdev, bhutchings, stephen

On Wed, Jan 07, 2015 at 01:41:33AM +0000, fugang.duan@freescale.com wrote:
> From: Shawn Guo <shawn.guo@linaro.org> Sent: Tuesday, January 06, 2015 7:48 PM
> > To: Duan Fugang-B38611
> > Cc: davem@davemloft.net; netdev@vger.kernel.org;
> > bhutchings@solarflare.com; stephen@networkplumber.org
> > Subject: Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode
> > callback function
> > 
> > On Wed, Dec 24, 2014 at 05:30:40PM +0800, Fugang Duan wrote:
> > > i.MX6q/dl, i.MX6SX SOCs enet support sleep mode that magic packet can
> > > wake up system in suspend status. For different SOCs, there have some
> > > SOC specifical GPR register to set sleep on/off mode. So add these to
> > > callback function for driver.
> > >
> > > Signed-off-by: Fugang Duan <B38611@freescale.com>
> > 
> > I do not like this patch.  In the end, this is just a GRP register bit
> > setup per FEC driver need.  Rather than messing up platform code for each
> > SoC with the same pattern, I do not see why this can not be done by FEC
> > driver itself.
> > 
> > You can take a look at LDB driver (drivers/gpu/drm/imx/imx-ldb.c) to see
> > how this can be done.
> > 
> > Shawn
> > 
> 
> Hi, Shawn,
> 
> It is SOC related setting, not fec IP itself setting, and different SOC GPR setting is not different.
> So I think it better to put it to platform code.

The GPR difference between SoCs can be encoded in device tree as well.
It's pointless to repeat the same code pattern for every single
platform, that need to set up GPR bits for enabling magic packet wake
up, while the only difference is the register and bit offset.

The platform code will become quite messy and unmaintainable if every
device driver dump their GPR register setup code into platform.

Sorry, but it's NACK from me.

Shawn

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function
  2015-01-07  2:33       ` Shawn Guo
@ 2015-01-07  2:40         ` fugang.duan
  2015-01-07  3:17           ` Shawn Guo
  2015-01-07  3:44         ` Fabio Estevam
  1 sibling, 1 reply; 17+ messages in thread
From: fugang.duan @ 2015-01-07  2:40 UTC (permalink / raw)
  To: Shawn Guo; +Cc: davem, netdev, bhutchings, stephen

From: Shawn Guo <shawn.guo@linaro.org> Sent: Wednesday, January 07, 2015 10:33 AM
> To: Duan Fugang-B38611
> Cc: davem@davemloft.net; netdev@vger.kernel.org;
> bhutchings@solarflare.com; stephen@networkplumber.org
> Subject: Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode
> callback function
> 
> On Wed, Jan 07, 2015 at 01:41:33AM +0000, fugang.duan@freescale.com wrote:
> > From: Shawn Guo <shawn.guo@linaro.org> Sent: Tuesday, January 06, 2015
> > 7:48 PM
> > > To: Duan Fugang-B38611
> > > Cc: davem@davemloft.net; netdev@vger.kernel.org;
> > > bhutchings@solarflare.com; stephen@networkplumber.org
> > > Subject: Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode
> > > callback function
> > >
> > > On Wed, Dec 24, 2014 at 05:30:40PM +0800, Fugang Duan wrote:
> > > > i.MX6q/dl, i.MX6SX SOCs enet support sleep mode that magic packet
> > > > can wake up system in suspend status. For different SOCs, there
> > > > have some SOC specifical GPR register to set sleep on/off mode. So
> > > > add these to callback function for driver.
> > > >
> > > > Signed-off-by: Fugang Duan <B38611@freescale.com>
> > >
> > > I do not like this patch.  In the end, this is just a GRP register
> > > bit setup per FEC driver need.  Rather than messing up platform code
> > > for each SoC with the same pattern, I do not see why this can not be
> > > done by FEC driver itself.
> > >
> > > You can take a look at LDB driver (drivers/gpu/drm/imx/imx-ldb.c) to
> > > see how this can be done.
> > >
> > > Shawn
> > >
> >
> > Hi, Shawn,
> >
> > It is SOC related setting, not fec IP itself setting, and different SOC
> GPR setting is not different.
> > So I think it better to put it to platform code.
> 
> The GPR difference between SoCs can be encoded in device tree as well.
> It's pointless to repeat the same code pattern for every single platform,
> that need to set up GPR bits for enabling magic packet wake up, while the
> only difference is the register and bit offset.
> 
> The platform code will become quite messy and unmaintainable if every
> device driver dump their GPR register setup code into platform.
> 
> Sorry, but it's NACK from me.
> 
> Shawn

Yes, the platform code will become quite messy and tremendous.
I agree with your thinking.

But David applied the patches, how can I do it now ?

Thanks,
Andy

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function
  2015-01-07  2:40         ` fugang.duan
@ 2015-01-07  3:17           ` Shawn Guo
  2015-01-07 12:02             ` David Miller
  0 siblings, 1 reply; 17+ messages in thread
From: Shawn Guo @ 2015-01-07  3:17 UTC (permalink / raw)
  To: fugang.duan, davem; +Cc: netdev, bhutchings, stephen

On Wed, Jan 07, 2015 at 02:40:33AM +0000, fugang.duan@freescale.com wrote:
> Yes, the platform code will become quite messy and tremendous.
> I agree with your thinking.
> 
> But David applied the patches, how can I do it now ?

David,

Can you please drop the last two arch/arm/ patches, or revert them if
your tree is non-rebasable?

Shawn

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function
  2015-01-07  2:33       ` Shawn Guo
  2015-01-07  2:40         ` fugang.duan
@ 2015-01-07  3:44         ` Fabio Estevam
  1 sibling, 0 replies; 17+ messages in thread
From: Fabio Estevam @ 2015-01-07  3:44 UTC (permalink / raw)
  To: Shawn Guo; +Cc: fugang.duan, davem, netdev, bhutchings, stephen

On Wed, Jan 7, 2015 at 12:33 AM, Shawn Guo <shawn.guo@linaro.org> wrote:

> The GPR difference between SoCs can be encoded in device tree as well.
> It's pointless to repeat the same code pattern for every single
> platform, that need to set up GPR bits for enabling magic packet wake
> up, while the only difference is the register and bit offset.
>
> The platform code will become quite messy and unmaintainable if every
> device driver dump their GPR register setup code into platform.
>
> Sorry, but it's NACK from me.

This patch has already reached linux-next via Dave's tree as commit
456062b3ec6f5.

Should a revert patch be sent?

Also, this commit causes a build warning in linux-next:
arch/arm/mach-imx/mach-imx6sx.c:59:20: warning: 'imx6sx_enet_plt_init'
defined but not used [-Wunused-function]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function
  2015-01-07  3:17           ` Shawn Guo
@ 2015-01-07 12:02             ` David Miller
  0 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2015-01-07 12:02 UTC (permalink / raw)
  To: shawn.guo; +Cc: fugang.duan, netdev, bhutchings, stephen

From: Shawn Guo <shawn.guo@linaro.org>
Date: Wed, 7 Jan 2015 11:17:00 +0800

> On Wed, Jan 07, 2015 at 02:40:33AM +0000, fugang.duan@freescale.com wrote:
>> Yes, the platform code will become quite messy and tremendous.
>> I agree with your thinking.
>> 
>> But David applied the patches, how can I do it now ?
> 
> David,
> 
> Can you please drop the last two arch/arm/ patches, or revert them if
> your tree is non-rebasable?

My tree is non-rebasable, someone will need to send me a revert patch
which has a commit message explains why the revert is happening.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-01-07 12:03 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-24  9:30 [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support Fugang Duan
2014-12-24  9:30 ` [PATCH net-next v1 1/3] " Fugang Duan
2014-12-24  9:30 ` [PATCH net-next v1 2/3] ARM: imx: add FEC sleep mode callback function Fugang Duan
2015-01-06 11:48   ` Shawn Guo
2015-01-07  1:41     ` fugang.duan
2015-01-07  2:33       ` Shawn Guo
2015-01-07  2:40         ` fugang.duan
2015-01-07  3:17           ` Shawn Guo
2015-01-07 12:02             ` David Miller
2015-01-07  3:44         ` Fabio Estevam
2014-12-24  9:30 ` [PATCH net-next v1 3/3] ARM: dts: imx6qdl: enable FEC magic-packet feature Fugang Duan
2014-12-31 18:07 ` [PATCH net-next v1 0/3] net: fec: add Wake-on-LAN support David Miller
2014-12-31 19:03   ` Fabio Estevam
2014-12-31 19:20     ` David Miller
2014-12-31 19:22       ` Fabio Estevam
2015-01-04  1:39         ` fugang.duan
2015-01-06 11:21   ` Shawn Guo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.