[PATCH] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[PATCH] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Harald Seiler]

Use DM_ETH instead of legacy networking.

Signed-off-by: Harald Seiler <hws@denx.de>
---

Notes:
    I am unsure whether I converted 'enough' of the board_eth_init()
    function to DM.  I think the reset GPIO in particular could be handled
    by the DM driver if I add the following to the device-tree (did not try
    it yet):
    
    	phy-reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
    	phy-reset-duration = <1>;
    	phy-reset-post-delay = <10>;
    
    Is it preferred to change the device-tree here or should I keep the reset
    from board_init() like I have it now?  If I should use the dt, is there
    a similar way for dealing with VIO?

 board/dhelectronics/dh_imx6/dh_imx6.c | 26 +++++---------------------
 configs/dh_imx6_defconfig             |  2 ++
 2 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/board/dhelectronics/dh_imx6/dh_imx6.c b/board/dhelectronics/dh_imx6/dh_imx6.c
index 33ce7e8ff115..aafb9f1b341f 100644
--- a/board/dhelectronics/dh_imx6/dh_imx6.c
+++ b/board/dhelectronics/dh_imx6/dh_imx6.c
@@ -28,10 +28,7 @@
 #include <fsl_esdhc_imx.h>
 #include <fuse.h>
 #include <i2c_eeprom.h>
-#include <miiphy.h>
 #include <mmc.h>
-#include <net.h>
-#include <netdev.h>
 #include <usb.h>
 #include <usb/ehci-ci.h>
 
@@ -80,31 +77,14 @@ static int setup_fec_clock(void)
 	return enable_fec_anatop_clock(0, ENET_50MHZ);
 }
 
-int board_eth_init(bd_t *bis)
+static void setup_fec(void)
 {
-	uint32_t base = IMX_FEC_BASE;
-	struct mii_dev *bus = NULL;
-	struct phy_device *phydev = NULL;
-
 	gpio_request(IMX_GPIO_NR(5, 0), "PHY-reset");
 	gpio_request(IMX_GPIO_NR(1, 7), "VIO");
 
 	setup_fec_clock();
 
 	eth_phy_reset();
-
-	bus = fec_get_miibus(base, -1);
-	if (!bus)
-		return -EINVAL;
-
-	/* Scan PHY 0 */
-	phydev = phy_find_by_mask(bus, 0xf, PHY_INTERFACE_MODE_RGMII);
-	if (!phydev) {
-		printf("Ethernet PHY not found!\n");
-		return -EINVAL;
-	}
-
-	return fec_probe(bis, -1, base, bus, phydev);
 }
 #endif
 
@@ -190,6 +170,10 @@ int board_init(void)
 
 	setup_dhcom_mac_from_fuse();
 
+#ifdef CONFIG_FEC_MXC
+	setup_fec();
+#endif
+
 	return 0;
 }
 
diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig
index cbfc3c394e7d..40de1d82031b 100644
--- a/configs/dh_imx6_defconfig
+++ b/configs/dh_imx6_defconfig
@@ -74,6 +74,8 @@ CONFIG_SPI_FLASH_MTD=y
 CONFIG_PHYLIB=y
 CONFIG_PHY_MICREL=y
 CONFIG_PHY_MICREL_KSZ90X1=y
+CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
 CONFIG_FEC_MXC=y
 CONFIG_MII=y
 CONFIG_PINCTRL=y
-- 
2.26.0

[PATCH] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Marek Vasut]

On 4/15/20 4:01 PM, Harald Seiler wrote:
> Use DM_ETH instead of legacy networking.
> 
> Signed-off-by: Harald Seiler <hws@denx.de>
> ---
> 
> Notes:
>     I am unsure whether I converted 'enough' of the board_eth_init()
>     function to DM.  I think the reset GPIO in particular could be handled
>     by the DM driver if I add the following to the device-tree (did not try
>     it yet):
>     
>     	phy-reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
>     	phy-reset-duration = <1>;
>     	phy-reset-post-delay = <10>;

That's how PHY reset should be done, yes.

>     Is it preferred to change the device-tree here or should I keep the reset
>     from board_init() like I have it now?  If I should use the dt, is there
>     a similar way for dealing with VIO?

VIO is likely a fixed regulator-always-on regulator for now.

>  board/dhelectronics/dh_imx6/dh_imx6.c | 26 +++++---------------------
>  configs/dh_imx6_defconfig             |  2 ++
>  2 files changed, 7 insertions(+), 21 deletions(-)
> 
> diff --git a/board/dhelectronics/dh_imx6/dh_imx6.c b/board/dhelectronics/dh_imx6/dh_imx6.c
> index 33ce7e8ff115..aafb9f1b341f 100644
> --- a/board/dhelectronics/dh_imx6/dh_imx6.c
> +++ b/board/dhelectronics/dh_imx6/dh_imx6.c
> @@ -28,10 +28,7 @@
>  #include <fsl_esdhc_imx.h>
>  #include <fuse.h>
>  #include <i2c_eeprom.h>
> -#include <miiphy.h>
>  #include <mmc.h>
> -#include <net.h>
> -#include <netdev.h>
>  #include <usb.h>
>  #include <usb/ehci-ci.h>
>  
> @@ -80,31 +77,14 @@ static int setup_fec_clock(void)
>  	return enable_fec_anatop_clock(0, ENET_50MHZ);
>  }
>  
> -int board_eth_init(bd_t *bis)
> +static void setup_fec(void)
>  {
> -	uint32_t base = IMX_FEC_BASE;
> -	struct mii_dev *bus = NULL;
> -	struct phy_device *phydev = NULL;
> -
>  	gpio_request(IMX_GPIO_NR(5, 0), "PHY-reset");
>  	gpio_request(IMX_GPIO_NR(1, 7), "VIO");
>  
>  	setup_fec_clock();
>  
>  	eth_phy_reset();
> -
> -	bus = fec_get_miibus(base, -1);
> -	if (!bus)
> -		return -EINVAL;
> -
> -	/* Scan PHY 0 */
> -	phydev = phy_find_by_mask(bus, 0xf, PHY_INTERFACE_MODE_RGMII);
> -	if (!phydev) {
> -		printf("Ethernet PHY not found!\n");
> -		return -EINVAL;
> -	}
> -
> -	return fec_probe(bis, -1, base, bus, phydev);
>  }
>  #endif
>  
> @@ -190,6 +170,10 @@ int board_init(void)
>  
>  	setup_dhcom_mac_from_fuse();
>  
> +#ifdef CONFIG_FEC_MXC

You probably want to init the clock either way, not only if the FEC is
enabled, so drop the ifdef.

> +	setup_fec();
> +#endif

[...]

[PATCH v2] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Harald Seiler]

Use DM_ETH instead of legacy networking.

Signed-off-by: Harald Seiler <hws@denx.de>
---

Notes:
    Changes in v2:
      - Move reset and VIO to device-tree
      - Always enable the clock, not just if CONFIG_FEC_MXC=y

 arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi |  6 +++
 arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi  |  2 +
 arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi     | 22 ++++++++++
 board/dhelectronics/dh_imx6/dh_imx6.c      | 51 +---------------------
 configs/dh_imx6_defconfig                  |  2 +
 5 files changed, 34 insertions(+), 49 deletions(-)
 create mode 100644 arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
 create mode 100644 arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi

diff --git a/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
new file mode 100644
index 000000000000..50bcb0419c9c
--- /dev/null
+++ b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2020 Harald Seiler <hws@denx.de>
+ */
+
+#include "imx6qdl-dhcom-u-boot.dtsi"
diff --git a/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi b/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
index b94231edb3fb..3060ee84f463 100644
--- a/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
+++ b/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
@@ -3,6 +3,8 @@
  * Copyright (C) 2019 Claudius Heine <ch@denx.de>
  */
 
+#include "imx6qdl-dhcom-u-boot.dtsi"
+
 / {
 	wdt-reboot {
 		compatible = "wdt-reboot";
diff --git a/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
new file mode 100644
index 000000000000..88840bb45920
--- /dev/null
+++ b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2020 Harald Seiler <hws@denx.de>
+ */
+
+/ {
+	fec_vio: regulator-fec {
+		compatible = "regulator-fixed";
+
+		regulator-name = "fec-vio";
+		gpio = <&gpio1 7 GPIO_ACTIVE_HIGH>;
+		regulator-always-on;
+	};
+};
+
+&fec {
+	phy-reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
+	phy-reset-duration = <1>;
+	phy-reset-post-delay = <10>;
+
+	phy-supply = <&fec_vio>;
+};
diff --git a/board/dhelectronics/dh_imx6/dh_imx6.c b/board/dhelectronics/dh_imx6/dh_imx6.c
index 33ce7e8ff115..b6f8b11a10cc 100644
--- a/board/dhelectronics/dh_imx6/dh_imx6.c
+++ b/board/dhelectronics/dh_imx6/dh_imx6.c
@@ -28,10 +28,7 @@
 #include <fsl_esdhc_imx.h>
 #include <fuse.h>
 #include <i2c_eeprom.h>
-#include <miiphy.h>
 #include <mmc.h>
-#include <net.h>
-#include <netdev.h>
 #include <usb.h>
 #include <usb/ehci-ci.h>
 
@@ -52,24 +49,6 @@ int overwrite_console(void)
 	return 1;
 }
 
-#ifdef CONFIG_FEC_MXC
-static void eth_phy_reset(void)
-{
-	/* Reset PHY */
-	gpio_direction_output(IMX_GPIO_NR(5, 0) , 0);
-	udelay(500);
-	gpio_set_value(IMX_GPIO_NR(5, 0), 1);
-
-	/* Enable VIO */
-	gpio_direction_output(IMX_GPIO_NR(1, 7) , 0);
-
-	/*
-	 * KSZ9021 PHY needs at least 10 mSec after PHY reset
-	 * is released to stabilize
-	 */
-	mdelay(10);
-}
-
 static int setup_fec_clock(void)
 {
 	struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR;
@@ -80,34 +59,6 @@ static int setup_fec_clock(void)
 	return enable_fec_anatop_clock(0, ENET_50MHZ);
 }
 
-int board_eth_init(bd_t *bis)
-{
-	uint32_t base = IMX_FEC_BASE;
-	struct mii_dev *bus = NULL;
-	struct phy_device *phydev = NULL;
-
-	gpio_request(IMX_GPIO_NR(5, 0), "PHY-reset");
-	gpio_request(IMX_GPIO_NR(1, 7), "VIO");
-
-	setup_fec_clock();
-
-	eth_phy_reset();
-
-	bus = fec_get_miibus(base, -1);
-	if (!bus)
-		return -EINVAL;
-
-	/* Scan PHY 0 */
-	phydev = phy_find_by_mask(bus, 0xf, PHY_INTERFACE_MODE_RGMII);
-	if (!phydev) {
-		printf("Ethernet PHY not found!\n");
-		return -EINVAL;
-	}
-
-	return fec_probe(bis, -1, base, bus, phydev);
-}
-#endif
-
 #ifdef CONFIG_USB_EHCI_MX6
 static void setup_usb(void)
 {
@@ -190,6 +141,8 @@ int board_init(void)
 
 	setup_dhcom_mac_from_fuse();
 
+	setup_fec_clock();
+
 	return 0;
 }
 
diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig
index cbfc3c394e7d..40de1d82031b 100644
--- a/configs/dh_imx6_defconfig
+++ b/configs/dh_imx6_defconfig
@@ -74,6 +74,8 @@ CONFIG_SPI_FLASH_MTD=y
 CONFIG_PHYLIB=y
 CONFIG_PHY_MICREL=y
 CONFIG_PHY_MICREL_KSZ90X1=y
+CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
 CONFIG_FEC_MXC=y
 CONFIG_MII=y
 CONFIG_PINCTRL=y
-- 
2.26.0

[PATCH v2] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Marek Vasut]

On 4/15/20 4:48 PM, Harald Seiler wrote:
> Use DM_ETH instead of legacy networking.

Some more descriptive commit message would help.

[...]

> diff --git a/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
> new file mode 100644
> index 000000000000..88840bb45920
> --- /dev/null
> +++ b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: (GPL-2.0+)
> +/*
> + * Copyright (C) 2020 Harald Seiler <hws@denx.de>
> + */
> +
> +/ {
> +	fec_vio: regulator-fec {
> +		compatible = "regulator-fixed";
> +
> +		regulator-name = "fec-vio";
> +		gpio = <&gpio1 7 GPIO_ACTIVE_HIGH>;
> +		regulator-always-on;
> +	};
> +};

The VIO regulator is on the pdk2, so it should be in the PDK2 U-Boot extras.

> +&fec {
> +	phy-reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
> +	phy-reset-duration = <1>;
> +	phy-reset-post-delay = <10>;

So is the PHY, so this should also be in the PDK2 extras.

(and it should be fixed in Linux too eventually, if it's not done yet)

[...]

[PATCH v2] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Harald Seiler]

Hello Marek,

On Wed, 2020-04-15 at 16:53 +0200, Marek Vasut wrote:
> On 4/15/20 4:48 PM, Harald Seiler wrote:
> > Use DM_ETH instead of legacy networking.
> 
> Some more descriptive commit message would help.
> 
> [...]
> 
> > diff --git a/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
> > new file mode 100644
> > index 000000000000..88840bb45920
> > --- /dev/null
> > +++ b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
> > @@ -0,0 +1,22 @@
> > +// SPDX-License-Identifier: (GPL-2.0+)
> > +/*
> > + * Copyright (C) 2020 Harald Seiler <hws@denx.de>
> > + */
> > +
> > +/ {
> > +	fec_vio: regulator-fec {
> > +		compatible = "regulator-fixed";
> > +
> > +		regulator-name = "fec-vio";
> > +		gpio = <&gpio1 7 GPIO_ACTIVE_HIGH>;
> > +		regulator-always-on;
> > +	};
> > +};
> 
> The VIO regulator is on the pdk2, so it should be in the PDK2 U-Boot extras.
> 
> > +&fec {
> > +	phy-reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
> > +	phy-reset-duration = <1>;
> > +	phy-reset-post-delay = <10>;
> 
> So is the PHY, so this should also be in the PDK2 extras.
> 
> (and it should be fixed in Linux too eventually, if it's not done yet)

I think Linux handles this a bit different:  The node for the PHY contains
almost the same properties already so I believe that is what's used in the
kernel:

	ethphy0: ethernet-phy at 0 {
		reg = <0>;
		max-speed = <100>;
		reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
		reset-delay-us = <1000>;
		reset-post-delay-us = <1000>;
	};

Not sure why U-Boot uses a different set of properties, maybe it makes
sense at some point to start using those instead.

Also, this was the reason why I put it into the general dhcom dtsi.  I was
thinking that, if the existing properties are this general, mine should
probably be, too.

> [...]
-- 
Harald

[PATCH v2] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Marek Vasut]

On 4/15/20 5:17 PM, Harald Seiler wrote:
> Hello Marek,
> 
> On Wed, 2020-04-15 at 16:53 +0200, Marek Vasut wrote:
>> On 4/15/20 4:48 PM, Harald Seiler wrote:
>>> Use DM_ETH instead of legacy networking.
>>
>> Some more descriptive commit message would help.
>>
>> [...]
>>
>>> diff --git a/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
>>> new file mode 100644
>>> index 000000000000..88840bb45920
>>> --- /dev/null
>>> +++ b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
>>> @@ -0,0 +1,22 @@
>>> +// SPDX-License-Identifier: (GPL-2.0+)
>>> +/*
>>> + * Copyright (C) 2020 Harald Seiler <hws@denx.de>
>>> + */
>>> +
>>> +/ {
>>> +	fec_vio: regulator-fec {
>>> +		compatible = "regulator-fixed";
>>> +
>>> +		regulator-name = "fec-vio";
>>> +		gpio = <&gpio1 7 GPIO_ACTIVE_HIGH>;
>>> +		regulator-always-on;
>>> +	};
>>> +};
>>
>> The VIO regulator is on the pdk2, so it should be in the PDK2 U-Boot extras.
>>
>>> +&fec {
>>> +	phy-reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
>>> +	phy-reset-duration = <1>;
>>> +	phy-reset-post-delay = <10>;
>>
>> So is the PHY, so this should also be in the PDK2 extras.
>>
>> (and it should be fixed in Linux too eventually, if it's not done yet)
> 
> I think Linux handles this a bit different:  The node for the PHY contains
> almost the same properties already so I believe that is what's used in the
> kernel:
> 
> 	ethphy0: ethernet-phy at 0 {
> 		reg = <0>;
> 		max-speed = <100>;
> 		reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
> 		reset-delay-us = <1000>;
> 		reset-post-delay-us = <1000>;
> 	};
> 
> Not sure why U-Boot uses a different set of properties, maybe it makes
> sense at some point to start using those instead.
> 
> Also, this was the reason why I put it into the general dhcom dtsi.  I was
> thinking that, if the existing properties are this general, mine should
> probably be, too.

I recently had a look into the MDIO subsystem in Linux and the reset
GPIO can be either MDIO-bus level OR PHY-level, so that's why there are
two sets of properties at different location. Look at:

linux-2.6$ git grep gpio.*reset drivers/net/phy/

I _think_ U-Boot only implements one of those two options, but I might
be wrong there.

[PATCH v3] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Harald Seiler]

Use DM_ETH instead of legacy networking.  Add VIO as a fixed regulator
to the relevant device-trees and augment the FEC node with properties
for the reset GPIO.

It should be noted that the relevant properties for the reset GPIO
already exist in the PHY node but U-Boot currently ignores those and
only supports the bus-level reset properties in the FEC node.

Signed-off-by: Harald Seiler <hws@denx.de>
---

Notes:
    Changes in v2:
      - Move reset and VIO to device-tree.
      - Always enable the clock, not just if CONFIG_FEC_MXC=y.
    
    Changes in v3:
      - Rename the dt file to imx6qdl-dhcom-pdk2-u-boot.dtsi because the PHY is
        pdk2 specific.
      - More verbose commit message.

 arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi  |  6 +++
 arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi   |  2 +
 arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi | 22 +++++++++
 board/dhelectronics/dh_imx6/dh_imx6.c       | 51 +--------------------
 configs/dh_imx6_defconfig                   |  2 +
 5 files changed, 34 insertions(+), 49 deletions(-)
 create mode 100644 arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
 create mode 100644 arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi

diff --git a/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
new file mode 100644
index 000000000000..fc7dffea2a69
--- /dev/null
+++ b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2020 Harald Seiler <hws@denx.de>
+ */
+
+#include "imx6qdl-dhcom-pdk2-u-boot.dtsi"
diff --git a/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi b/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
index b94231edb3fb..026342df5a4a 100644
--- a/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
+++ b/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
@@ -3,6 +3,8 @@
  * Copyright (C) 2019 Claudius Heine <ch@denx.de>
  */
 
+#include "imx6qdl-dhcom-pdk2-u-boot.dtsi"
+
 / {
 	wdt-reboot {
 		compatible = "wdt-reboot";
diff --git a/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi b/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
new file mode 100644
index 000000000000..88840bb45920
--- /dev/null
+++ b/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2020 Harald Seiler <hws@denx.de>
+ */
+
+/ {
+	fec_vio: regulator-fec {
+		compatible = "regulator-fixed";
+
+		regulator-name = "fec-vio";
+		gpio = <&gpio1 7 GPIO_ACTIVE_HIGH>;
+		regulator-always-on;
+	};
+};
+
+&fec {
+	phy-reset-gpios = <&gpio5 0 GPIO_ACTIVE_LOW>;
+	phy-reset-duration = <1>;
+	phy-reset-post-delay = <10>;
+
+	phy-supply = <&fec_vio>;
+};
diff --git a/board/dhelectronics/dh_imx6/dh_imx6.c b/board/dhelectronics/dh_imx6/dh_imx6.c
index 33ce7e8ff115..b6f8b11a10cc 100644
--- a/board/dhelectronics/dh_imx6/dh_imx6.c
+++ b/board/dhelectronics/dh_imx6/dh_imx6.c
@@ -28,10 +28,7 @@
 #include <fsl_esdhc_imx.h>
 #include <fuse.h>
 #include <i2c_eeprom.h>
-#include <miiphy.h>
 #include <mmc.h>
-#include <net.h>
-#include <netdev.h>
 #include <usb.h>
 #include <usb/ehci-ci.h>
 
@@ -52,24 +49,6 @@ int overwrite_console(void)
 	return 1;
 }
 
-#ifdef CONFIG_FEC_MXC
-static void eth_phy_reset(void)
-{
-	/* Reset PHY */
-	gpio_direction_output(IMX_GPIO_NR(5, 0) , 0);
-	udelay(500);
-	gpio_set_value(IMX_GPIO_NR(5, 0), 1);
-
-	/* Enable VIO */
-	gpio_direction_output(IMX_GPIO_NR(1, 7) , 0);
-
-	/*
-	 * KSZ9021 PHY needs at least 10 mSec after PHY reset
-	 * is released to stabilize
-	 */
-	mdelay(10);
-}
-
 static int setup_fec_clock(void)
 {
 	struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR;
@@ -80,34 +59,6 @@ static int setup_fec_clock(void)
 	return enable_fec_anatop_clock(0, ENET_50MHZ);
 }
 
-int board_eth_init(bd_t *bis)
-{
-	uint32_t base = IMX_FEC_BASE;
-	struct mii_dev *bus = NULL;
-	struct phy_device *phydev = NULL;
-
-	gpio_request(IMX_GPIO_NR(5, 0), "PHY-reset");
-	gpio_request(IMX_GPIO_NR(1, 7), "VIO");
-
-	setup_fec_clock();
-
-	eth_phy_reset();
-
-	bus = fec_get_miibus(base, -1);
-	if (!bus)
-		return -EINVAL;
-
-	/* Scan PHY 0 */
-	phydev = phy_find_by_mask(bus, 0xf, PHY_INTERFACE_MODE_RGMII);
-	if (!phydev) {
-		printf("Ethernet PHY not found!\n");
-		return -EINVAL;
-	}
-
-	return fec_probe(bis, -1, base, bus, phydev);
-}
-#endif
-
 #ifdef CONFIG_USB_EHCI_MX6
 static void setup_usb(void)
 {
@@ -190,6 +141,8 @@ int board_init(void)
 
 	setup_dhcom_mac_from_fuse();
 
+	setup_fec_clock();
+
 	return 0;
 }
 
diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig
index cbfc3c394e7d..40de1d82031b 100644
--- a/configs/dh_imx6_defconfig
+++ b/configs/dh_imx6_defconfig
@@ -74,6 +74,8 @@ CONFIG_SPI_FLASH_MTD=y
 CONFIG_PHYLIB=y
 CONFIG_PHY_MICREL=y
 CONFIG_PHY_MICREL_KSZ90X1=y
+CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
 CONFIG_FEC_MXC=y
 CONFIG_MII=y
 CONFIG_PINCTRL=y
-- 
2.26.0

[PATCH v3] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Fabio Estevam]

Hi Harald,

On Wed, Apr 15, 2020 at 12:54 PM Harald Seiler <hws@denx.de> wrote:

> +/ {
> +       fec_vio: regulator-fec {
> +               compatible = "regulator-fixed";
> +
> +               regulator-name = "fec-vio";
> +               gpio = <&gpio1 7 GPIO_ACTIVE_HIGH>;

By looking at your board code, this should be GPIO_ACTIVE_LOW instead.

> +               regulator-always-on;

This one could be removed since it has the FEC as a consumer.

[PATCH v3] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Marek Vasut]

On 4/15/20 5:54 PM, Harald Seiler wrote:
> Use DM_ETH instead of legacy networking.  Add VIO as a fixed regulator
> to the relevant device-trees and augment the FEC node with properties
> for the reset GPIO.
> 
> It should be noted that the relevant properties for the reset GPIO
> already exist in the PHY node but U-Boot currently ignores those and
> only supports the bus-level reset properties in the FEC node.
> 
> Signed-off-by: Harald Seiler <hws@denx.de>
> ---
> 
> Notes:
>     Changes in v2:
>       - Move reset and VIO to device-tree.
>       - Always enable the clock, not just if CONFIG_FEC_MXC=y.
>     
>     Changes in v3:
>       - Rename the dt file to imx6qdl-dhcom-pdk2-u-boot.dtsi because the PHY is
>         pdk2 specific.
>       - More verbose commit message.
> 
>  arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi  |  6 +++
>  arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi   |  2 +
>  arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi | 22 +++++++++
>  board/dhelectronics/dh_imx6/dh_imx6.c       | 51 +--------------------
>  configs/dh_imx6_defconfig                   |  2 +
>  5 files changed, 34 insertions(+), 49 deletions(-)
>  create mode 100644 arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
>  create mode 100644 arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
> 
> diff --git a/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
> new file mode 100644
> index 000000000000..fc7dffea2a69
> --- /dev/null
> +++ b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi

Do we really need a separate DT for DL ? If so, this should be a
separate patch.

[PATCH v3] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Harald Seiler]

Hello Fabio,

On Wed, 2020-04-15 at 13:15 -0300, Fabio Estevam wrote:
> Hi Harald,
> 
> On Wed, Apr 15, 2020 at 12:54 PM Harald Seiler <hws@denx.de> wrote:
> 
> > +/ {
> > +       fec_vio: regulator-fec {
> > +               compatible = "regulator-fixed";
> > +
> > +               regulator-name = "fec-vio";
> > +               gpio = <&gpio1 7 GPIO_ACTIVE_HIGH>;
> 
> By looking at your board code, this should be GPIO_ACTIVE_LOW instead.

Yes, you are right, I will change this in v4.  Interestingly, it works
with both ACTIVE_LOW and ACTIVE_HIGH but removing the regulator entirely
breaks it.  Seems a bit weird to me ...

> > +               regulator-always-on;
> 
> This one could be removed since it has the FEC as a consumer.

I see, thanks!
-- 
Harald

[PATCH v3] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Fabio Estevam]

Hi Harald,

On Wed, Apr 15, 2020 at 2:32 PM Harald Seiler <hws@denx.de> wrote:

> Yes, you are right, I will change this in v4.  Interestingly, it works
> with both ACTIVE_LOW and ACTIVE_HIGH but removing the regulator entirely
> breaks it.  Seems a bit weird to me ...

Due to historical reasons, the GPIO polarity is not read from device
tree and it is considered to be active low.

To describe an active high polarity for the GPIO regulator, the
"enable-active-high" property needs to be passed.

Anyway, it is better to pass active low in your case to reflect the reality.

Thanks

[PATCH v3] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Harald Seiler]

Hello Marek,

On Wed, 2020-04-15 at 19:02 +0200, Marek Vasut wrote:
> On 4/15/20 5:54 PM, Harald Seiler wrote:
> > Use DM_ETH instead of legacy networking.  Add VIO as a fixed regulator
> > to the relevant device-trees and augment the FEC node with properties
> > for the reset GPIO.
> > 
> > It should be noted that the relevant properties for the reset GPIO
> > already exist in the PHY node but U-Boot currently ignores those and
> > only supports the bus-level reset properties in the FEC node.
> > 
> > Signed-off-by: Harald Seiler <hws@denx.de>
> > ---
> > 
> > Notes:
> >     Changes in v2:
> >       - Move reset and VIO to device-tree.
> >       - Always enable the clock, not just if CONFIG_FEC_MXC=y.
> >     
> >     Changes in v3:
> >       - Rename the dt file to imx6qdl-dhcom-pdk2-u-boot.dtsi because the PHY is
> >         pdk2 specific.
> >       - More verbose commit message.
> > 
> >  arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi  |  6 +++
> >  arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi   |  2 +
> >  arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi | 22 +++++++++
> >  board/dhelectronics/dh_imx6/dh_imx6.c       | 51 +--------------------
> >  configs/dh_imx6_defconfig                   |  2 +
> >  5 files changed, 34 insertions(+), 49 deletions(-)
> >  create mode 100644 arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
> >  create mode 100644 arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
> > 
> > diff --git a/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
> > new file mode 100644
> > index 000000000000..fc7dffea2a69
> > --- /dev/null
> > +++ b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
> 
> Do we really need a separate DT for DL ? If so, this should be a
> separate patch.

I can't really comment on the reason for the two separate device-trees but
it looks like they were introduced for commit 8039211a8a9c ("ARM: imx6:
DHCOM i.MX6 PDK: config SPL to load U-Boot fitImage with mulitple DTs").

I'm not sure I understand why you want two separate patches here.
Wouldn't it make more sense to fix both device-trees in one go so we don't
have a broken U-Boot for the DL version from just the first patch (which
would e.g. hurt bisecting)?

-- 
Harald

[PATCH v3] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH

[Marek Vasut]

On 4/15/20 7:53 PM, Harald Seiler wrote:
> Hello Marek,
> 
> On Wed, 2020-04-15 at 19:02 +0200, Marek Vasut wrote:
>> On 4/15/20 5:54 PM, Harald Seiler wrote:
>>> Use DM_ETH instead of legacy networking.  Add VIO as a fixed regulator
>>> to the relevant device-trees and augment the FEC node with properties
>>> for the reset GPIO.
>>>
>>> It should be noted that the relevant properties for the reset GPIO
>>> already exist in the PHY node but U-Boot currently ignores those and
>>> only supports the bus-level reset properties in the FEC node.
>>>
>>> Signed-off-by: Harald Seiler <hws@denx.de>
>>> ---
>>>
>>> Notes:
>>>     Changes in v2:
>>>       - Move reset and VIO to device-tree.
>>>       - Always enable the clock, not just if CONFIG_FEC_MXC=y.
>>>     
>>>     Changes in v3:
>>>       - Rename the dt file to imx6qdl-dhcom-pdk2-u-boot.dtsi because the PHY is
>>>         pdk2 specific.
>>>       - More verbose commit message.
>>>
>>>  arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi  |  6 +++
>>>  arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi   |  2 +
>>>  arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi | 22 +++++++++
>>>  board/dhelectronics/dh_imx6/dh_imx6.c       | 51 +--------------------
>>>  configs/dh_imx6_defconfig                   |  2 +
>>>  5 files changed, 34 insertions(+), 49 deletions(-)
>>>  create mode 100644 arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
>>>  create mode 100644 arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
>>>
>>> diff --git a/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
>>> new file mode 100644
>>> index 000000000000..fc7dffea2a69
>>> --- /dev/null
>>> +++ b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
>>
>> Do we really need a separate DT for DL ? If so, this should be a
>> separate patch.
> 
> I can't really comment on the reason for the two separate device-trees but
> it looks like they were introduced for commit 8039211a8a9c ("ARM: imx6:
> DHCOM i.MX6 PDK: config SPL to load U-Boot fitImage with mulitple DTs").

Oh, so we already have two DTs, OK.

> I'm not sure I understand why you want two separate patches here.
> Wouldn't it make more sense to fix both device-trees in one go so we don't
> have a broken U-Boot for the DL version from just the first patch (which
> would e.g. hurt bisecting)?

I didn't realize we already have two DTs, sorry, please ignore.