[PATCH net v2 0/4] net: ethernet: fec: move GPR reigster offset and bit into DT

[PATCH net v2 0/4] net: ethernet: fec: move GPR reigster offset and bit into DT

[fugang.duan]

From: Fugang Duan <fugang.duan@nxp.com>

The commit da722186f654 (net: fec: set GPR bit on suspend by DT configuration) set the GPR reigster offset and bit in driver for wol feature.

It bring trouble to enable wol feature on imx6sx/imx6ul/imx7d platforms that have multiple ethernet instances with different GPR bit for stop mode control. So the patch set is to move GPR reigster offset and bit define into DT, and enable imx6q/imx6dl/imx6sx/imx6ul/imx7d stop mode support.

Currently, below NXP i.MX boards support wol:
- imx6q/imx6dl sabresd
- imx6sx sabreauto
- imx7d sdb

imx6q/imx6dl sarebsd board dts file miss the property "fsl,magic-packet;", so patch#4 is to add the property for stop mode support.


v1 -> v2:
 - driver: switch back to store the quirks bitmask in driver_data
 - dt-bindings: rename 'gpr' property string to 'fsl,stop-mode'
 - imx6/7 dtsi: add imx6sx/imx6ul/imx7d ethernet stop mode property

Thanks Martin and Andrew for the review.


Fugang Duan (4):
  net: ethernet: fec: move GPR register offset and bit into DT
  dt-bindings: fec: update the gpr property
  ARM: dts: imx: add ethernet stop mode property
  ARM: dts: imx6qdl-sabresd: enable fec wake-on-lan

 Documentation/devicetree/bindings/net/fsl-fec.txt |   7 +-
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi            |   1 +
 arch/arm/boot/dts/imx6qdl.dtsi                    |   2 +-
 arch/arm/boot/dts/imx6sx.dtsi                     |   2 +
 arch/arm/boot/dts/imx6ul.dtsi                     |   2 +
 arch/arm/boot/dts/imx7d.dtsi                      |   1 +
 arch/arm/boot/dts/imx7s.dtsi                      |   1 +
 drivers/net/ethernet/freescale/fec_main.c         | 103 +++++++---------------
 8 files changed, 47 insertions(+), 72 deletions(-)

-- 
2.7.4

[PATCH net v2 1/4] net: ethernet: fec: move GPR register offset and bit into DT

[fugang.duan]

From: Fugang Duan <fugang.duan@nxp.com>

The commit da722186f654 (net: fec: set GPR bit on suspend by DT
configuration) set the GPR reigster offset and bit in driver for
wake on lan feature.

But it introduces two issues here:
- one SOC has two instances, they have different bit
- different SOCs may have different offset and bit

So to support wake-on-lan feature on other i.MX platforms, it should
configure the GPR reigster offset and bit from DT.

So the patch is to improve the commit da722186f654 (net: fec: set GPR
bit on suspend by DT configuration) to support multiple ethernet
instances on i.MX series.

v2:
 * switch back to store the quirks bitmask in driver_data

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 103 ++++++++++--------------------
 1 file changed, 34 insertions(+), 69 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 2e20914..4f55d30 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -86,56 +86,6 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
 #define FEC_ENET_OPD_V	0xFFF0
 #define FEC_MDIO_PM_TIMEOUT  100 /* ms */
 
-struct fec_devinfo {
-	u32 quirks;
-	u8 stop_gpr_reg;
-	u8 stop_gpr_bit;
-};
-
-static const struct fec_devinfo fec_imx25_info = {
-	.quirks = FEC_QUIRK_USE_GASKET | FEC_QUIRK_MIB_CLEAR |
-		  FEC_QUIRK_HAS_FRREG,
-};
-
-static const struct fec_devinfo fec_imx27_info = {
-	.quirks = FEC_QUIRK_MIB_CLEAR | FEC_QUIRK_HAS_FRREG,
-};
-
-static const struct fec_devinfo fec_imx28_info = {
-	.quirks = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME |
-		  FEC_QUIRK_SINGLE_MDIO | FEC_QUIRK_HAS_RACC |
-		  FEC_QUIRK_HAS_FRREG,
-};
-
-static const struct fec_devinfo fec_imx6q_info = {
-	.quirks = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
-		  FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
-		  FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR006358 |
-		  FEC_QUIRK_HAS_RACC,
-	.stop_gpr_reg = 0x34,
-	.stop_gpr_bit = 27,
-};
-
-static const struct fec_devinfo fec_mvf600_info = {
-	.quirks = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_RACC,
-};
-
-static const struct fec_devinfo fec_imx6x_info = {
-	.quirks = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
-		  FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
-		  FEC_QUIRK_HAS_VLAN | FEC_QUIRK_HAS_AVB |
-		  FEC_QUIRK_ERR007885 | FEC_QUIRK_BUG_CAPTURE |
-		  FEC_QUIRK_HAS_RACC | FEC_QUIRK_HAS_COALESCE,
-};
-
-static const struct fec_devinfo fec_imx6ul_info = {
-	.quirks = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
-		  FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
-		  FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR007885 |
-		  FEC_QUIRK_BUG_CAPTURE | FEC_QUIRK_HAS_RACC |
-		  FEC_QUIRK_HAS_COALESCE,
-};
-
 static struct platform_device_id fec_devtype[] = {
 	{
 		/* keep it for coldfire */
@@ -143,25 +93,39 @@ static struct platform_device_id fec_devtype[] = {
 		.driver_data = 0,
 	}, {
 		.name = "imx25-fec",
-		.driver_data = (kernel_ulong_t)&fec_imx25_info,
+		.driver_data = FEC_QUIRK_USE_GASKET | FEC_QUIRK_MIB_CLEAR |
+			       FEC_QUIRK_HAS_FRREG,
 	}, {
 		.name = "imx27-fec",
-		.driver_data = (kernel_ulong_t)&fec_imx27_info,
+		.driver_data = FEC_QUIRK_MIB_CLEAR | FEC_QUIRK_HAS_FRREG,
 	}, {
 		.name = "imx28-fec",
-		.driver_data = (kernel_ulong_t)&fec_imx28_info,
+		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_SWAP_FRAME |
+				FEC_QUIRK_SINGLE_MDIO | FEC_QUIRK_HAS_RACC |
+				FEC_QUIRK_HAS_FRREG,
 	}, {
 		.name = "imx6q-fec",
-		.driver_data = (kernel_ulong_t)&fec_imx6q_info,
+		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
+				FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
+				FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR006358 |
+				FEC_QUIRK_HAS_RACC,
 	}, {
 		.name = "mvf600-fec",
-		.driver_data = (kernel_ulong_t)&fec_mvf600_info,
+		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_RACC,
 	}, {
 		.name = "imx6sx-fec",
-		.driver_data = (kernel_ulong_t)&fec_imx6x_info,
+		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
+				FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
+				FEC_QUIRK_HAS_VLAN | FEC_QUIRK_HAS_AVB |
+				FEC_QUIRK_ERR007885 | FEC_QUIRK_BUG_CAPTURE |
+				FEC_QUIRK_HAS_RACC | FEC_QUIRK_HAS_COALESCE,
 	}, {
 		.name = "imx6ul-fec",
-		.driver_data = (kernel_ulong_t)&fec_imx6ul_info,
+		.driver_data = FEC_QUIRK_ENET_MAC | FEC_QUIRK_HAS_GBIT |
+				FEC_QUIRK_HAS_BUFDESC_EX | FEC_QUIRK_HAS_CSUM |
+				FEC_QUIRK_HAS_VLAN | FEC_QUIRK_ERR007885 |
+				FEC_QUIRK_BUG_CAPTURE | FEC_QUIRK_HAS_RACC |
+				FEC_QUIRK_HAS_COALESCE,
 	}, {
 		/* sentinel */
 	}
@@ -3476,19 +3440,23 @@ static int fec_enet_get_irq_cnt(struct platform_device *pdev)
 }
 
 static int fec_enet_init_stop_mode(struct fec_enet_private *fep,
-				   struct fec_devinfo *dev_info,
 				   struct device_node *np)
 {
 	struct device_node *gpr_np;
+	u32 out_val[3];
 	int ret = 0;
 
-	if (!dev_info)
-		return 0;
-
-	gpr_np = of_parse_phandle(np, "gpr", 0);
+	gpr_np = of_parse_phandle(np, "fsl,stop-mode", 0);
 	if (!gpr_np)
 		return 0;
 
+	ret = of_property_read_u32_array(np, "fsl,stop-mode", out_val,
+					 ARRAY_SIZE(out_val));
+	if (ret) {
+		dev_dbg(&fep->pdev->dev, "no stop mode property\n");
+		return ret;
+	}
+
 	fep->stop_gpr.gpr = syscon_node_to_regmap(gpr_np);
 	if (IS_ERR(fep->stop_gpr.gpr)) {
 		dev_err(&fep->pdev->dev, "could not find gpr regmap\n");
@@ -3497,8 +3465,8 @@ static int fec_enet_init_stop_mode(struct fec_enet_private *fep,
 		goto out;
 	}
 
-	fep->stop_gpr.reg = dev_info->stop_gpr_reg;
-	fep->stop_gpr.bit = dev_info->stop_gpr_bit;
+	fep->stop_gpr.reg = out_val[1];
+	fep->stop_gpr.bit = out_val[2];
 
 out:
 	of_node_put(gpr_np);
@@ -3521,7 +3489,6 @@ fec_probe(struct platform_device *pdev)
 	int num_rx_qs;
 	char irq_name[8];
 	int irq_cnt;
-	struct fec_devinfo *dev_info;
 
 	fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
 
@@ -3539,9 +3506,7 @@ fec_probe(struct platform_device *pdev)
 	of_id = of_match_device(fec_dt_ids, &pdev->dev);
 	if (of_id)
 		pdev->id_entry = of_id->data;
-	dev_info = (struct fec_devinfo *)pdev->id_entry->driver_data;
-	if (dev_info)
-		fep->quirks = dev_info->quirks;
+	fep->quirks = pdev->id_entry->driver_data;
 
 	fep->netdev = ndev;
 	fep->num_rx_queues = num_rx_qs;
@@ -3575,7 +3540,7 @@ fec_probe(struct platform_device *pdev)
 	if (of_get_property(np, "fsl,magic-packet", NULL))
 		fep->wol_flag |= FEC_WOL_HAS_MAGIC_PACKET;
 
-	ret = fec_enet_init_stop_mode(fep, dev_info, np);
+	ret = fec_enet_init_stop_mode(fep, np);
 	if (ret)
 		goto failed_stop_mode;
 
-- 
2.7.4

[PATCH net v2 2/4] dt-bindings: fec: update the gpr property

[fugang.duan]

From: Fugang Duan <fugang.duan@nxp.com>

- rename the 'gpr' property string to 'fsl,stop-mode'.
- Update the property to define gpr register offset and
bit in DT, since different instance have different gpr bit.

v2:
 * rename 'gpr' property string to 'fsl,stop-mode'.

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
 Documentation/devicetree/bindings/net/fsl-fec.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 26c492a..9b54378 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -22,8 +22,11 @@ Optional properties:
 - fsl,err006687-workaround-present: If present indicates that the system has
   the hardware workaround for ERR006687 applied and does not need a software
   workaround.
-- gpr: phandle of SoC general purpose register mode. Required for wake on LAN
-  on some SoCs
+- fsl,stop-mode: register bits of stop mode control, the format is
+		 <&gpr req_gpr req_bit>.
+		 gpr is the phandle to general purpose register node.
+		 req_gpr is the gpr register offset for ENET stop request.
+		 req_bit is the gpr bit offset for ENET stop request.
  -interrupt-names:  names of the interrupts listed in interrupts property in
   the same order. The defaults if not specified are
   __Number of interrupts__   __Default__
-- 
2.7.4

[PATCH net v2 3/4] ARM: dts: imx: add ethernet stop mode property

[fugang.duan]

From: Fugang Duan <fugang.duan@nxp.com>

- Update the imx6qdl gpr property to define gpr register
  offset and bit in DT.
- Add imx6sx/imx6ul/imx7d ethernet stop mode property.

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
 arch/arm/boot/dts/imx6qdl.dtsi | 2 +-
 arch/arm/boot/dts/imx6sx.dtsi  | 2 ++
 arch/arm/boot/dts/imx6ul.dtsi  | 2 ++
 arch/arm/boot/dts/imx7d.dtsi   | 1 +
 arch/arm/boot/dts/imx7s.dtsi   | 1 +
 5 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 98da446..48f5016 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1045,7 +1045,7 @@
 					 <&clks IMX6QDL_CLK_ENET>,
 					 <&clks IMX6QDL_CLK_ENET_REF>;
 				clock-names = "ipg", "ahb", "ptp";
-				gpr = <&gpr>;
+				fsl,stop-mode = <&gpr 0x34 27>;
 				status = "disabled";
 			};
 
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index d6f8317..09f21aa 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -930,6 +930,7 @@
 					      "enet_clk_ref", "enet_out";
 				fsl,num-tx-queues = <3>;
 				fsl,num-rx-queues = <3>;
+				fsl,stop-mode = <&gpr 0x10 3>;
 				status = "disabled";
 			};
 
@@ -1039,6 +1040,7 @@
 					 <&clks IMX6SX_CLK_ENET_PTP>;
 				clock-names = "ipg", "ahb", "ptp",
 					      "enet_clk_ref", "enet_out";
+				fsl,stop-mode = <&gpr 0x10 4>;
 				status = "disabled";
 			};
 
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 2ccf67c..345ae9b 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -520,6 +520,7 @@
 					      "enet_clk_ref", "enet_out";
 				fsl,num-tx-queues = <1>;
 				fsl,num-rx-queues = <1>;
+				fsl,stop-mode = <&gpr 0x10 4>;
 				status = "disabled";
 			};
 
@@ -856,6 +857,7 @@
 					      "enet_clk_ref", "enet_out";
 				fsl,num-tx-queues = <1>;
 				fsl,num-rx-queues = <1>;
+				fsl,stop-mode = <&gpr 0x10 3>;
 				status = "disabled";
 			};
 
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 4c22828..cff875b 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -153,6 +153,7 @@
 			"enet_clk_ref", "enet_out";
 		fsl,num-tx-queues = <3>;
 		fsl,num-rx-queues = <3>;
+		fsl,stop-mode = <&gpr 0x10 4>;
 		status = "disabled";
 	};
 
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 76e3ffb..5bf0b39 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -1190,6 +1190,7 @@
 					"enet_clk_ref", "enet_out";
 				fsl,num-tx-queues = <3>;
 				fsl,num-rx-queues = <3>;
+				fsl,stop-mode = <&gpr 0x10 3>;
 				status = "disabled";
 			};
 		};
-- 
2.7.4

[PATCH net v2 4/4] ARM: dts: imx6qdl-sabresd: enable fec wake-on-lan

[fugang.duan]

From: Fugang Duan <fugang.duan@nxp.com>

Enable ethernet wake-on-lan feature for imx6q/dl/qp sabresd
boards since the PHY clock is supplied by exteranl osc.

Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
---
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
index fe59dde..28b35cc 100644
--- a/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabresd.dtsi
@@ -204,6 +204,7 @@
 	pinctrl-0 = <&pinctrl_enet>;
 	phy-mode = "rgmii-id";
 	phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;
+	fsl,magic-packet;
 	status = "okay";
 };
 
-- 
2.7.4

Re: [PATCH net v2 1/4] net: ethernet: fec: move GPR register offset and bit into DT

[Sascha Hauer]

On Mon, May 25, 2020 at 03:09:26PM +0800, fugang.duan@nxp.com wrote:
> From: Fugang Duan <fugang.duan@nxp.com>
> 
> The commit da722186f654 (net: fec: set GPR bit on suspend by DT
> configuration) set the GPR reigster offset and bit in driver for
> wake on lan feature.
> 
> But it introduces two issues here:
> - one SOC has two instances, they have different bit
> - different SOCs may have different offset and bit
> 
> So to support wake-on-lan feature on other i.MX platforms, it should
> configure the GPR reigster offset and bit from DT.
> 
> So the patch is to improve the commit da722186f654 (net: fec: set GPR
> bit on suspend by DT configuration) to support multiple ethernet
> instances on i.MX series.
> 
> v2:
>  * switch back to store the quirks bitmask in driver_data
> 
> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 103 ++++++++++--------------------
>  1 file changed, 34 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 2e20914..4f55d30 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -86,56 +86,6 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
>  #define FEC_ENET_OPD_V	0xFFF0
>  #define FEC_MDIO_PM_TIMEOUT  100 /* ms */
>  
> -struct fec_devinfo {
> -	u32 quirks;
> -	u8 stop_gpr_reg;
> -	u8 stop_gpr_bit;
> -};

Honestly I like the approach of having a struct fec_devinfo for
abstracting differences between different hardware variants. It gives
you more freedom to describe the differences. Converting this back to a
single bitfield is a step backward, even when currently struct
fec_devinfo only contains a single value.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net v2 3/4] ARM: dts: imx: add ethernet stop mode property

[Andrew Lunn]

On Mon, May 25, 2020 at 03:09:28PM +0800, fugang.duan@nxp.com wrote:
> From: Fugang Duan <fugang.duan@nxp.com>
> 
> - Update the imx6qdl gpr property to define gpr register
>   offset and bit in DT.
> - Add imx6sx/imx6ul/imx7d ethernet stop mode property.
> 
> Signed-off-by: Fugang Duan <fugang.duan@nxp.com>

Thanks for adding a user.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net v2 4/4] ARM: dts: imx6qdl-sabresd: enable fec wake-on-lan

[Andrew Lunn]

On Mon, May 25, 2020 at 03:09:29PM +0800, fugang.duan@nxp.com wrote:
> From: Fugang Duan <fugang.duan@nxp.com>
> 
> Enable ethernet wake-on-lan feature for imx6q/dl/qp sabresd
> boards since the PHY clock is supplied by exteranl osc.

external

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net v2 0/4] net: ethernet: fec: move GPR reigster offset and bit into DT

[Andrew Lunn]

On Mon, May 25, 2020 at 03:09:25PM +0800, fugang.duan@nxp.com wrote:
> From: Fugang Duan <fugang.duan@nxp.com>
> 
> The commit da722186f654 (net: fec: set GPR bit on suspend by DT configuration) set the GPR reigster offset and bit in driver for wol feature.

The cover letter gets committed as the merge commit message. So please
wrap long longs.

> It bring trouble to enable wol feature on imx6sx/imx6ul/imx7d
> platforms that have multiple ethernet instances with different GPR
> bit for stop mode control. So the patch set is to move GPR reigster

register

> offset and bit define into DT, and enable
> imx6q/imx6dl/imx6sx/imx6ul/imx7d stop mode support.


> 
> Currently, below NXP i.MX boards support wol:
> - imx6q/imx6dl sabresd
> - imx6sx sabreauto
> - imx7d sdb
> 
> imx6q/imx6dl sarebsd board dts file miss the property "fsl,magic-packet;", so patch#4 is to add the property for stop mode support.

sabresd?

	Andrew

RE: [EXT] Re: [PATCH net v2 1/4] net: ethernet: fec: move GPR register offset and bit into DT

[Andy Duan]

From: Sascha Hauer <s.hauer@pengutronix.de> Sent: Monday, May 25, 2020 6:49 PM
> On Mon, May 25, 2020 at 03:09:26PM +0800, fugang.duan@nxp.com wrote:
> > From: Fugang Duan <fugang.duan@nxp.com>
> >
> > The commit da722186f654 (net: fec: set GPR bit on suspend by DT
> > configuration) set the GPR reigster offset and bit in driver for wake
> > on lan feature.
> >
> > But it introduces two issues here:
> > - one SOC has two instances, they have different bit
> > - different SOCs may have different offset and bit
> >
> > So to support wake-on-lan feature on other i.MX platforms, it should
> > configure the GPR reigster offset and bit from DT.
> >
> > So the patch is to improve the commit da722186f654 (net: fec: set GPR
> > bit on suspend by DT configuration) to support multiple ethernet
> > instances on i.MX series.
> >
> > v2:
> >  * switch back to store the quirks bitmask in driver_data
> >
> > Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> > ---
> >  drivers/net/ethernet/freescale/fec_main.c | 103
> > ++++++++++--------------------
> >  1 file changed, 34 insertions(+), 69 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index 2e20914..4f55d30 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -86,56 +86,6 @@ static void fec_enet_itr_coal_init(struct net_device
> *ndev);
> >  #define FEC_ENET_OPD_V       0xFFF0
> >  #define FEC_MDIO_PM_TIMEOUT  100 /* ms */
> >
> > -struct fec_devinfo {
> > -     u32 quirks;
> > -     u8 stop_gpr_reg;
> > -     u8 stop_gpr_bit;
> > -};
> 
> Honestly I like the approach of having a struct fec_devinfo for abstracting
> differences between different hardware variants. It gives you more freedom
> to describe the differences. Converting this back to a single bitfield is a step
> backward, even when currently struct fec_devinfo only contains a single
> value.
> 
> Sascha
> 
Sascha, thanks for your review.
v1 patch is doing like this by using a struct fec_devinfo for abstracting differences.
In fact, I also like v1 method.

I will send the v3 version by using v1 patch.

> --
> Pengutronix e.K.                           |
> |
> Steuerwalder Str. 21                       |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.p
> engutronix.de%2F&amp;data=02%7C01%7Cfugang.duan%40nxp.com%7C2ae
> 2657af251492ced5608d80099384e%7C686ea1d3bc2b4c6fa92cd99c5c30163
> 5%7C0%7C0%7C637260005356084997&amp;sdata=sarhmRepUf1o4hCZ8WH
> oSBrg%2Fl128jz%2BNXxHRGAM%2FL0%3D&amp;reserved=0  |
> 31137 Hildesheim, Germany                  | Phone:
> +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |

RE: [EXT] Re: [PATCH net v2 3/4] ARM: dts: imx: add ethernet stop mode property

[Andy Duan]

From: Andrew Lunn <andrew@lunn.ch> Sent: Monday, May 25, 2020 9:51 PM
> On Mon, May 25, 2020 at 03:09:28PM +0800, fugang.duan@nxp.com wrote:
> > From: Fugang Duan <fugang.duan@nxp.com>
> >
> > - Update the imx6qdl gpr property to define gpr register
> >   offset and bit in DT.
> > - Add imx6sx/imx6ul/imx7d ethernet stop mode property.
> >
> > Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
> 
> Thanks for adding a user.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew

Andrew, thanks for your review.

RE: [EXT] Re: [PATCH net v2 4/4] ARM: dts: imx6qdl-sabresd: enable fec wake-on-lan

[Andy Duan]

From: Andrew Lunn <andrew@lunn.ch> Sent: Monday, May 25, 2020 9:52 PM
> On Mon, May 25, 2020 at 03:09:29PM +0800, fugang.duan@nxp.com wrote:
> > From: Fugang Duan <fugang.duan@nxp.com>
> >
> > Enable ethernet wake-on-lan feature for imx6q/dl/qp sabresd boards
> > since the PHY clock is supplied by exteranl osc.
> 
> external
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew

Andrew, thanks for your review.
Will send v3 by correct the typo.

RE: [EXT] Re: [PATCH net v2 0/4] net: ethernet: fec: move GPR reigster offset and bit into DT

[Andy Duan]

From: Andrew Lunn <andrew@lunn.ch> Sent: Monday, May 25, 2020 9:55 PM
> On Mon, May 25, 2020 at 03:09:25PM +0800, fugang.duan@nxp.com wrote:
> > From: Fugang Duan <fugang.duan@nxp.com>
> >
> > The commit da722186f654 (net: fec: set GPR bit on suspend by DT
> configuration) set the GPR reigster offset and bit in driver for wol feature.
> 
> The cover letter gets committed as the merge commit message. So please
> wrap long longs.
> 
> > It bring trouble to enable wol feature on imx6sx/imx6ul/imx7d
> > platforms that have multiple ethernet instances with different GPR bit
> > for stop mode control. So the patch set is to move GPR reigster
> 
> register
> 
Got it, will correct the typo in v3.

> > offset and bit define into DT, and enable
> > imx6q/imx6dl/imx6sx/imx6ul/imx7d stop mode support.
> 
> 
> >
> > Currently, below NXP i.MX boards support wol:
> > - imx6q/imx6dl sabresd
> > - imx6sx sabreauto
> > - imx7d sdb
> >
> > imx6q/imx6dl sarebsd board dts file miss the property "fsl,magic-packet;",
> so patch#4 is to add the property for stop mode support.
> 
> sabresd?
> 
>         Andrew

Thanks, I will correct the typo in v3.