[PATCH v2 0/4] *** ARM: imx6sl: add FEC support for imx6sl evk platform ***

[PATCH v2 0/4] *** ARM: imx6sl: add FEC support for imx6sl evk platform ***

[Fugang Duan]

Changelog:

V2:
 * Split iomuxc-gpr and fec phy gpio reset dts patch
 * Sort included header file

v1:
 * Add iomuxc-gpr device node
 * Add imx6sl evk fec phy gpio reset
 * Add imx6sl iomuxc-gpr fec gpr field define
 * Add fec init function to select clock source.

Fugang Duan (4):
  ARM: dts: add iomuxc-gpr device node for imx6sl
  ARM: dts: add fec phy reset for imx6sl evk board
  ARM: imx6sl: add imx6sl iomux-gpr field define
  ARM: imx6sl: config iomux-gpr1 to select clock for fec

 arch/arm/boot/dts/imx6sl-evk.dts            |    3 +++
 arch/arm/boot/dts/imx6sl.dtsi               |    5 +++++
 arch/arm/mach-imx/mach-imx6sl.c             |   21 +++++++++++++++++++++
 include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |    5 +++++
 4 files changed, 34 insertions(+), 0 deletions(-)

[PATCH v2 1/4] ARM: dts: add iomuxc-gpr device node for imx6sl

[Fugang Duan]

Add iomuxc gpr device node for imx6sl.

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

diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index c5e5da0..abd1eb9 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -504,6 +504,11 @@
 				interrupts = <0 89 0x04>;
 			};
 
+			gpr: iomuxc-gpr@020e0000 {
+				compatible = "fsl,imx6sl-iomuxc-gpr", "syscon";
+				reg = <0x020e0000 0x38>;
+			};
+
 			iomuxc: iomuxc@020e0000 {
 				compatible = "fsl,imx6sl-iomuxc";
 				reg = <0x020e0000 0x4000>;
-- 
1.7.1

Re: [PATCH v2 1/4] ARM: dts: add iomuxc-gpr device node for imx6sl

[Shawn Guo]

On Tue, Sep 03, 2013 at 12:26:22PM +0800, Fugang Duan wrote:
> Add iomuxc gpr device node for imx6sl.
> 
> Signed-off-by: Fugang Duan  <B38611@freescale.com>
                            ^^
We generally have only one space between name and email address.  You
may want to fix it in your git setup to follow the common pattern for
future patches.

Shawn

> ---
>  arch/arm/boot/dts/imx6sl.dtsi |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
> index c5e5da0..abd1eb9 100644
> --- a/arch/arm/boot/dts/imx6sl.dtsi
> +++ b/arch/arm/boot/dts/imx6sl.dtsi
> @@ -504,6 +504,11 @@
>  				interrupts = <0 89 0x04>;
>  			};
>  
> +			gpr: iomuxc-gpr@020e0000 {
> +				compatible = "fsl,imx6sl-iomuxc-gpr", "syscon";
> +				reg = <0x020e0000 0x38>;
> +			};
> +
>  			iomuxc: iomuxc@020e0000 {
>  				compatible = "fsl,imx6sl-iomuxc";
>  				reg = <0x020e0000 0x4000>;
> -- 
> 1.7.1
> 
> 

[PATCH v2 2/4] ARM: dts: add fec phy reset for imx6sl evk board

[Fugang Duan]

Add fec phy gpio reset for imx6sl evk board. imx6sl evk board
use GPIO4_21 to do phy reset, so add phy-reset-gpios and
phy-reset-duration propery initial for fec device node.

Signed-off-by: Fugang Duan  <B38611@freescale.com>
---
 arch/arm/boot/dts/imx6sl-evk.dts |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/imx6sl-evk.dts b/arch/arm/boot/dts/imx6sl-evk.dts
index 2886a59..c4d32a5 100644
--- a/arch/arm/boot/dts/imx6sl-evk.dts
+++ b/arch/arm/boot/dts/imx6sl-evk.dts
@@ -23,6 +23,8 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_fec_1>;
 	phy-mode = "rmii";
+	phy-reset-gpios = <&gpio4 21 0>; /* GPIO4_21 */
+	phy-reset-duration = <1>;
 	status = "okay";
 };
 
@@ -38,6 +40,7 @@
 				MX6SL_PAD_SD2_DAT7__GPIO5_IO00    0x17059
 				MX6SL_PAD_SD2_DAT6__GPIO4_IO29    0x17059
 				MX6SL_PAD_REF_CLK_32K__GPIO3_IO22 0x17059
+				MX6SL_PAD_FEC_TX_CLK__GPIO4_IO21  0x80000000
 			>;
 		};
 	};
-- 
1.7.1

Re: [PATCH v2 2/4] ARM: dts: add fec phy reset for imx6sl evk board

[Shawn Guo]

On Tue, Sep 03, 2013 at 12:26:23PM +0800, Fugang Duan wrote:
> Add fec phy gpio reset for imx6sl evk board. imx6sl evk board
> use GPIO4_21 to do phy reset, so add phy-reset-gpios and
> phy-reset-duration propery initial for fec device node.
> 
> Signed-off-by: Fugang Duan  <B38611@freescale.com>

I have to drop this patch for now, because it causes a regression for
me.

Before this patch, we can at least get a working fec driver in kernel if
U-Boot sets it up properly, e.g. we download kernel from network.  But
with the patch, the fec driver stops working on linux-next in the same
use case.

Shawn

> ---
>  arch/arm/boot/dts/imx6sl-evk.dts |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/imx6sl-evk.dts b/arch/arm/boot/dts/imx6sl-evk.dts
> index 2886a59..c4d32a5 100644
> --- a/arch/arm/boot/dts/imx6sl-evk.dts
> +++ b/arch/arm/boot/dts/imx6sl-evk.dts
> @@ -23,6 +23,8 @@
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_fec_1>;
>  	phy-mode = "rmii";
> +	phy-reset-gpios = <&gpio4 21 0>; /* GPIO4_21 */
> +	phy-reset-duration = <1>;
>  	status = "okay";
>  };
>  
> @@ -38,6 +40,7 @@
>  				MX6SL_PAD_SD2_DAT7__GPIO5_IO00    0x17059
>  				MX6SL_PAD_SD2_DAT6__GPIO4_IO29    0x17059
>  				MX6SL_PAD_REF_CLK_32K__GPIO3_IO22 0x17059
> +				MX6SL_PAD_FEC_TX_CLK__GPIO4_IO21  0x80000000
>  			>;
>  		};
>  	};
> -- 
> 1.7.1
> 
> 

Re: [PATCH v2 2/4] ARM: dts: add fec phy reset for imx6sl evk board

[Sascha Hauer]

On Mon, Sep 09, 2013 at 10:53:36PM +0800, Shawn Guo wrote:
> On Tue, Sep 03, 2013 at 12:26:23PM +0800, Fugang Duan wrote:
> > Add fec phy gpio reset for imx6sl evk board. imx6sl evk board
> > use GPIO4_21 to do phy reset, so add phy-reset-gpios and
> > phy-reset-duration propery initial for fec device node.
> > 
> > Signed-off-by: Fugang Duan  <B38611@freescale.com>
> 
> I have to drop this patch for now, because it causes a regression for
> me.
> 
> Before this patch, we can at least get a working fec driver in kernel if
> U-Boot sets it up properly, e.g. we download kernel from network.  But
> with the patch, the fec driver stops working on linux-next in the same
> use case.

I think the FEC driver has doesn't wait enough time after it deasserts
the phy reset before it accesses the phy. At least I once got the
impression, but I haven't digged deeper so far.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

RE: [PATCH v2 2/4] ARM: dts: add fec phy reset for imx6sl evk board

[Duan Fugang-B38611]

From: Shawn Guo [mailto:shawn.guo@linaro.org]
Sent: Monday, September 09, 2013 10:54 PM + 800

> To: Duan Fugang-B38611
> Cc: devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v2 2/4] ARM: dts: add fec phy reset for imx6sl evk
> board
> 
> On Tue, Sep 03, 2013 at 12:26:23PM +0800, Fugang Duan wrote:
> > Add fec phy gpio reset for imx6sl evk board. imx6sl evk board use
> > GPIO4_21 to do phy reset, so add phy-reset-gpios and
> > phy-reset-duration propery initial for fec device node.
> >
> > Signed-off-by: Fugang Duan  <B38611@freescale.com>
> 
> I have to drop this patch for now, because it causes a regression for me.
> 
> Before this patch, we can at least get a working fec driver in kernel if
> U-Boot sets it up properly, e.g. we download kernel from network.  But
> with the patch, the fec driver stops working on linux-next in the same use
> case.
> 
> Shawn

Shawn, you cannot drop this patch.
1. Why do you get a working fec driver in kernel if U-Boot sets it up properly ?
   Because the phy reset done by uboot, kernel cannot depend on uboot.
   And even so, the driver still have problem for unit test.
2. You see freescale internal kernel(3.10.9) work fine. There have one driver patch don't upstream.
   I will send it out to net tree for review.

Thanks,
Andy
> 
> > ---
> >  arch/arm/boot/dts/imx6sl-evk.dts |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/imx6sl-evk.dts
> > b/arch/arm/boot/dts/imx6sl-evk.dts
> > index 2886a59..c4d32a5 100644
> > --- a/arch/arm/boot/dts/imx6sl-evk.dts
> > +++ b/arch/arm/boot/dts/imx6sl-evk.dts
> > @@ -23,6 +23,8 @@
> >  	pinctrl-names = "default";
> >  	pinctrl-0 = <&pinctrl_fec_1>;
> >  	phy-mode = "rmii";
> > +	phy-reset-gpios = <&gpio4 21 0>; /* GPIO4_21 */
> > +	phy-reset-duration = <1>;
> >  	status = "okay";
> >  };
> >
> > @@ -38,6 +40,7 @@
> >  				MX6SL_PAD_SD2_DAT7__GPIO5_IO00    0x17059
> >  				MX6SL_PAD_SD2_DAT6__GPIO4_IO29    0x17059
> >  				MX6SL_PAD_REF_CLK_32K__GPIO3_IO22 0x17059
> > +				MX6SL_PAD_FEC_TX_CLK__GPIO4_IO21  0x80000000
> >  			>;
> >  		};
> >  	};
> > --
> > 1.7.1
> >
> >

Re: [PATCH v2 2/4] ARM: dts: add fec phy reset for imx6sl evk board

[Shawn Guo]

On Tue, Sep 10, 2013 at 01:26:49AM +0000, Duan Fugang-B38611 wrote:
> Shawn, you cannot drop this patch.

I have to drop the patch *for now*.  Look, with the patch landed on
linux-next, people report that fec is broken as below on imx6sl-evk.

fec 2188000.ethernet eth0: no PHY, assuming direct connection to switch
libphy: PHY fixed-0:00 not found
fec 2188000.ethernet eth0: could not attach to PHY
IP-Config: Failed to open eth0
IP-Config: No network devices available

While before the patch, we can boot the board into console with NFS when
downloading kernel image from network.  This is the most used setup for
kernel developers.

> 1. Why do you get a working fec driver in kernel if U-Boot sets it up properly ?
>    Because the phy reset done by uboot, kernel cannot depend on uboot.
>    And even so, the driver still have problem for unit test.

Yeah, it should be fixed/improved without any breakage/regression on
the current use case.

> 2. You see freescale internal kernel(3.10.9) work fine. There have one driver patch don't upstream.
>    I will send it out to net tree for review.

That's where problem comes.  You should push required driver changes
upstream before sending me this patch.  You need to resolve the
dependencies between patches properly without introducing any breakages
or regressions.

That said, I'm not saying the patch is wrong, but the timing of sending
this patch for merging is wrong.

Shawn

RE: [PATCH v2 2/4] ARM: dts: add fec phy reset for imx6sl evk board

[Duan Fugang-B38611]

From: Shawn Guo [mailto:shawn.guo@linaro.org]
Data: Tuesday, September 10, 2013 11:12 AM + 800

> To: Duan Fugang-B38611
> Cc: devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v2 2/4] ARM: dts: add fec phy reset for imx6sl evk
> board
> 
> On Tue, Sep 10, 2013 at 01:26:49AM +0000, Duan Fugang-B38611 wrote:
> > Shawn, you cannot drop this patch.
> 
> I have to drop the patch *for now*.  Look, with the patch landed on linux-
> next, people report that fec is broken as below on imx6sl-evk.
> 
> fec 2188000.ethernet eth0: no PHY, assuming direct connection to switch
> libphy: PHY fixed-0:00 not found
> fec 2188000.ethernet eth0: could not attach to PHY
> IP-Config: Failed to open eth0
> IP-Config: No network devices available
> 
> While before the patch, we can boot the board into console with NFS when
> downloading kernel image from network.  This is the most used setup for
> kernel developers.
> 
> > 1. Why do you get a working fec driver in kernel if U-Boot sets it up
> properly ?
> >    Because the phy reset done by uboot, kernel cannot depend on uboot.
> >    And even so, the driver still have problem for unit test.
> 
> Yeah, it should be fixed/improved without any breakage/regression on the
> current use case.
> 
> > 2. You see freescale internal kernel(3.10.9) work fine. There have one
> driver patch don't upstream.
> >    I will send it out to net tree for review.
> 
> That's where problem comes.  You should push required driver changes
> upstream before sending me this patch.  You need to resolve the
> dependencies between patches properly without introducing any breakages or
> regressions.
> 
Ok, you can drop the patch. After fec driver patch with phy reset applied, and then apply it.

In fact, just for kernel, there have no sequence between the patch and driver patch because
Kernel fec also cannot work for imx6sl evk if uboot don't do the initial.

> That said, I'm not saying the patch is wrong, but the timing of sending
> this patch for merging is wrong.
> 
> Shawn

Re: [PATCH v2 2/4] ARM: dts: add fec phy reset for imx6sl evk board

[Shawn Guo]

On Tue, Sep 10, 2013 at 03:23:26AM +0000, Duan Fugang-B38611 wrote:
> In fact, just for kernel, there have no sequence between the patch and driver patch because
> Kernel fec also cannot work for imx6sl evk if uboot don't do the initial.

True.  But in practice, we mostly use U-Boot to download kernel during
development.  So it's still a nice thing for kernel developers if we can
boot the board into console with NFS when FEC driver is not fully ready.

Shawn

[PATCH v2 3/4] ARM: imx6sl: add imx6sl iomux-gpr field define

[Fugang Duan]

Add imx6sl iomux-gpr register field define in "imx6q-iomuxc-gpr.h" header
file, which is not fully define all iomux-gpr registers and fields, only
add fec related macro define.

Signed-off-by: Fugang Duan  <B38611@freescale.com>
---
 include/linux/mfd/syscon/imx6q-iomuxc-gpr.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
index b6bdcd6..7086b22 100644
--- a/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx6q-iomuxc-gpr.h
@@ -363,4 +363,9 @@
 #define IMX6Q_GPR13_SATA_TX_LVL_1_240_V		(0x1f << 2)
 #define IMX6Q_GPR13_SATA_MPLL_CLK_EN		BIT(1)
 #define IMX6Q_GPR13_SATA_TX_EDGE_RATE		BIT(0)
+
+/* For imx6sl iomux gpr register field define */
+#define IMX6SL_GPR1_FEC_CLOCK_MUX1_SEL_MASK    (0x3 << 17)
+#define IMX6SL_GPR1_FEC_CLOCK_MUX2_SEL_MASK    (0x1 << 14)
+
 #endif /* __LINUX_IMX6Q_IOMUXC_GPR_H */
-- 
1.7.1

Re: [PATCH v2 3/4] ARM: imx6sl: add imx6sl iomux-gpr field define

[Otavio Salvador]

On Tue, Sep 3, 2013 at 1:26 AM, Fugang Duan <B38611@freescale.com> wrote:
> Add imx6sl iomux-gpr register field define in "imx6q-iomuxc-gpr.h" header
> file, which is not fully define all iomux-gpr registers and fields, only
> add fec related macro define.
>
> Signed-off-by: Fugang Duan  <B38611@freescale.com>

The 'q' in the header filename is misleading; wouldn't be better to
have it named imx6-iomuxc-gpr.h than?

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

RE: [PATCH v2 3/4] ARM: imx6sl: add imx6sl iomux-gpr field define

[Duan Fugang-B38611]

From: otavio.salvador@gmail.com [mailto:otavio.salvador@gmail.com] On Behalf Of Otavio Salvador
Data: Thursday, September 05, 2013 9:45 AM

> To: Duan Fugang-B38611
> Cc: Shawn Guo; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH v2 3/4] ARM: imx6sl: add imx6sl iomux-gpr field define
> 
> On Tue, Sep 3, 2013 at 1:26 AM, Fugang Duan <B38611@freescale.com> wrote:
> > Add imx6sl iomux-gpr register field define in "imx6q-iomuxc-gpr.h"
> > header file, which is not fully define all iomux-gpr registers and
> > fields, only add fec related macro define.
> >
> > Signed-off-by: Fugang Duan  <B38611@freescale.com>
> 
> The 'q' in the header filename is misleading; wouldn't be better to have
> it named imx6-iomuxc-gpr.h than?
> 
Yes, since there have many same macro define and register define, we put the iomux-gpr header file
To one. I will submit one patch to rename the header file to imx6-iomuxc-gpr.h.

> --
> Otavio Salvador                             O.S. Systems
> http://www.ossystems.com.br        http://code.ossystems.com.br
> Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH v2 3/4] ARM: imx6sl: add imx6sl iomux-gpr field define

[Otavio Salvador]

On Wed, Sep 4, 2013 at 11:01 PM, Duan Fugang-B38611
<B38611@freescale.com> wrote:
> From: otavio.salvador@gmail.com [mailto:otavio.salvador@gmail.com] On Behalf Of Otavio Salvador
> Data: Thursday, September 05, 2013 9:45 AM
>
>> To: Duan Fugang-B38611
>> Cc: Shawn Guo; devicetree@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org
>> Subject: Re: [PATCH v2 3/4] ARM: imx6sl: add imx6sl iomux-gpr field define
>>
>> On Tue, Sep 3, 2013 at 1:26 AM, Fugang Duan <B38611@freescale.com> wrote:
>> > Add imx6sl iomux-gpr register field define in "imx6q-iomuxc-gpr.h"
>> > header file, which is not fully define all iomux-gpr registers and
>> > fields, only add fec related macro define.
>> >
>> > Signed-off-by: Fugang Duan  <B38611@freescale.com>
>>
>> The 'q' in the header filename is misleading; wouldn't be better to have
>> it named imx6-iomuxc-gpr.h than?
>>
> Yes, since there have many same macro define and register define, we put the iomux-gpr header file
> To one. I will submit one patch to rename the header file to imx6-iomuxc-gpr.h.

Great; thanks! :-)

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

[PATCH v2 4/4] ARM: imx6sl: config iomux-gpr1 to select clock for fec

[Fugang Duan]

Config iomux-gpr1 to select clock source for fec system clock.
Clear gpr1[14], gpr1[18-17] bit to select the fec clock source
from internal anatop PLL.

Signed-off-by: Fugang Duan  <B38611@freescale.com>
---
 arch/arm/mach-imx/mach-imx6sl.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-imx/mach-imx6sl.c b/arch/arm/mach-imx/mach-imx6sl.c
index 132db26..96fb816 100644
--- a/arch/arm/mach-imx/mach-imx6sl.c
+++ b/arch/arm/mach-imx/mach-imx6sl.c
@@ -11,17 +11,38 @@
 #include <linux/irqchip.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
+#include <linux/regmap.h>
 #include <asm/hardware/cache-l2x0.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 
 #include "common.h"
 
+static void __init imx6sl_fec_init(void)
+{
+	struct regmap *gpr;
+
+	/* set FEC clock from internal PLL clock source */
+	gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sl-iomuxc-gpr");
+	if (!IS_ERR(gpr)) {
+		regmap_update_bits(gpr, IOMUXC_GPR1,
+			IMX6SL_GPR1_FEC_CLOCK_MUX2_SEL_MASK, 0);
+		regmap_update_bits(gpr, IOMUXC_GPR1,
+			IMX6SL_GPR1_FEC_CLOCK_MUX1_SEL_MASK, 0);
+	} else {
+		pr_err("failed to find fsl,imx6sl-iomux-gpr regmap\n");
+	}
+}
+
 static void __init imx6sl_init_machine(void)
 {
 	mxc_arch_reset_init_dt();
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+
+	imx6sl_fec_init();
 }
 
 static void __init imx6sl_init_irq(void)
-- 
1.7.1

Re: [PATCH v2 0/4] *** ARM: imx6sl: add FEC support for imx6sl evk platform ***

[Shawn Guo]

On Tue, Sep 03, 2013 at 12:26:21PM +0800, Fugang Duan wrote:
> Changelog:
> 
> V2:
>  * Split iomuxc-gpr and fec phy gpio reset dts patch
>  * Sort included header file
> 
> v1:
>  * Add iomuxc-gpr device node
>  * Add imx6sl evk fec phy gpio reset
>  * Add imx6sl iomuxc-gpr fec gpr field define
>  * Add fec init function to select clock source.
> 
> Fugang Duan (4):
>   ARM: dts: add iomuxc-gpr device node for imx6sl
>   ARM: dts: add fec phy reset for imx6sl evk board
>   ARM: imx6sl: add imx6sl iomux-gpr field define
>   ARM: imx6sl: config iomux-gpr1 to select clock for fec

Applied all, thanks.