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 |
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) [...]
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. > [...]
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 --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
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