diff mbox series

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

Message ID 20200415144833.273302-1-hws@denx.de
State Superseded
Headers show
Series [v2] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH | expand

Commit Message

Harald Seiler April 15, 2020, 2:48 p.m. UTC
Use DM_ETH instead of legacy networking.

Signed-off-by: Harald Seiler <hws at 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

Comments

Marek Vasut April 15, 2020, 2:53 p.m. UTC | #1
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 at 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)

[...]
Harald Seiler April 15, 2020, 3:17 p.m. UTC | #2
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 at 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.

> [...]
Marek Vasut April 15, 2020, 3:19 p.m. UTC | #3
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 at 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.
diff mbox series

Patch

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 at 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 at 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 at 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