| Message ID | 20240328122116.2460855-1-festevam@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Fabio Estevam |
| Headers | show |
| Series | mx6cuboxi: Fix Ethernet after DT sync with Linux | expand |
Am 28.03.24 um 13:21 schrieb Fabio Estevam: > From: Josua Mayer <josua@solid-run.com> > > The i.MX6 Cubox-i and HummingBoards can have different PHYs at varying > addresses. U-Boot needs to auto-detect which phy is actually present, > and at which address it is responding. > > Auto-detection from multiple phy nodes specified in device-tree does not > currently work correct. As a work-around merge all three possible phys > into one node with the special address 0xffffffff which indicates to the > generic phy driver to probe all addresses. > Also fixup this fake address before booting Linux, *if* booting with > U-Boot's internal dtb. > > Signed-off-by: Josua Mayer <josua@solid-run.com> > [fabio: Added the changes to imx6qdl-sr-som-u-boot.dtsi.] > Signed-off-by: Fabio Estevam <festevam@gmail.com> > Tested-by: Christian Gmeiner <cgmeiner@igalia.com> > --- > ...qdl-hummingboard2-emmc-som-v15-u-boot.dtsi | 1 + > arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi | 40 +++++++++++++++++++ > board/solidrun/mx6cuboxi/mx6cuboxi.c | 8 +++- > 3 files changed, 48 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi > > diff --git a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi > index e9b188ed6587..358cf8abc4ff 100644 > --- a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi > +++ b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0+ > > #include "imx6qdl-u-boot.dtsi" > +#include "imx6qdl-sr-som-u-boot.dtsi" > > / { > board-detect { > diff --git a/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi b/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi > new file mode 100644 > index 000000000000..4c5f043ea92a > --- /dev/null > +++ b/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi > @@ -0,0 +1,40 @@ > +// SPDX-License-Identifier: (GPL-2.0 OR MIT) > + > +#include <dt-bindings/gpio/gpio.h> > + > +&fec { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_microsom_enet_ar8035>; > + phy-handle = <&phy>; > + phy-mode = "rgmii-id"; > + > + /* > + * The PHY seems to require a long-enough reset duration to avoid > + * some rare issues where the PHY gets stuck in an inconsistent and > + * non-functional state at boot-up. 10ms proved to be fine . > + */ > + phy-reset-duration = <10>; > + phy-reset-gpios = <&gpio4 15 GPIO_ACTIVE_LOW>; > + status = "okay"; > + > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + /delete-node/ ethernet-phy@1; > + /delete-node/ ethernet-phy@4; > + > + phy: ethernet-phy@0 { This node name is shared with upstream imx6qdl-sr-som.dtsi > + /* > + * The PHY can appear either: > + * - AR8035: at address 0 or 4 > + * - ADIN1300: at address 1 > + * Actual address being detected at runtime. > + */ > + reg = <0xffffffff>; > + qca,clk-out-frequency = <125000000>; > + qca,smarteee-tw-us-1g = <24>; > + adi,phy-output-clock = "125mhz-free-running"; > + }; > + }; > +}; > diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c b/board/solidrun/mx6cuboxi/mx6cuboxi.c > index 8edabf4404c2..fbab39e800a6 100644 > --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c > +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c > @@ -447,7 +447,7 @@ static int find_ethernet_phy(void) > */ > int ft_board_setup(void *fdt, struct bd_info *bd) > { > - int node_phy0, node_phy1, node_phy4; > + int node_phy, node_phy0, node_phy1, node_phy4; > int ret, phy; > bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = false; > enum board_type board; > @@ -479,6 +479,12 @@ int ft_board_setup(void *fdt, struct bd_info *bd) > return 0; > } > > + // update U-Boot's own unified phy node phy address, if present > + node_phy = fdt_path_offset(fdt, "/soc/bus@2100000/ethernet@2188000/mdio/phy"); This node no longer exists, unless you rename the u-boot-specific one. The u-boot node should probably have its own separate name, to ensure we do not find the upstream linux dtb ethernet-phy@0 node by mistake. > + ret = fdt_setprop_u32(fdt, node_phy, "reg", phy); > + if (ret < 0) > + pr_err("%s: failed to update unified PHY node address\n", __func__); > + > // update all phy nodes status > node_phy0 = fdt_path_offset(fdt, "/soc/bus@2100000/ethernet@2188000/mdio/ethernet-phy@0"); > ret = fdt_setprop_string(fdt, node_phy0, "status", enable_phy0 ? "okay" : "disabled");
Am 28.03.24 um 13:51 schrieb Josua Mayer: > Am 28.03.24 um 13:21 schrieb Fabio Estevam: >> From: Josua Mayer <josua@solid-run.com> >> >> The i.MX6 Cubox-i and HummingBoards can have different PHYs at varying >> addresses. U-Boot needs to auto-detect which phy is actually present, >> and at which address it is responding. >> >> Auto-detection from multiple phy nodes specified in device-tree does not >> currently work correct. As a work-around merge all three possible phys >> into one node with the special address 0xffffffff which indicates to the >> generic phy driver to probe all addresses. >> Also fixup this fake address before booting Linux, *if* booting with >> U-Boot's internal dtb. >> >> Signed-off-by: Josua Mayer <josua@solid-run.com> >> [fabio: Added the changes to imx6qdl-sr-som-u-boot.dtsi.] >> Signed-off-by: Fabio Estevam <festevam@gmail.com> >> Tested-by: Christian Gmeiner <cgmeiner@igalia.com> >> --- >> ...qdl-hummingboard2-emmc-som-v15-u-boot.dtsi | 1 + >> arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi | 40 +++++++++++++++++++ >> board/solidrun/mx6cuboxi/mx6cuboxi.c | 8 +++- >> 3 files changed, 48 insertions(+), 1 deletion(-) >> create mode 100644 arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi >> >> diff --git a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi >> index e9b188ed6587..358cf8abc4ff 100644 >> --- a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi >> +++ b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi >> @@ -1,6 +1,7 @@ >> // SPDX-License-Identifier: GPL-2.0+ >> >> #include "imx6qdl-u-boot.dtsi" >> +#include "imx6qdl-sr-som-u-boot.dtsi" >> >> / { >> board-detect { >> diff --git a/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi b/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi >> new file mode 100644 >> index 000000000000..4c5f043ea92a >> --- /dev/null >> +++ b/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi >> @@ -0,0 +1,40 @@ >> +// SPDX-License-Identifier: (GPL-2.0 OR MIT) >> + >> +#include <dt-bindings/gpio/gpio.h> >> + >> +&fec { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_microsom_enet_ar8035>; >> + phy-handle = <&phy>; >> + phy-mode = "rgmii-id"; >> + >> + /* >> + * The PHY seems to require a long-enough reset duration to avoid >> + * some rare issues where the PHY gets stuck in an inconsistent and >> + * non-functional state at boot-up. 10ms proved to be fine . >> + */ >> + phy-reset-duration = <10>; >> + phy-reset-gpios = <&gpio4 15 GPIO_ACTIVE_LOW>; >> + status = "okay"; >> + >> + mdio { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + /delete-node/ ethernet-phy@1; >> + /delete-node/ ethernet-phy@4; I suggest changing their status to disabled, and keeping the nodes. >> + >> + phy: ethernet-phy@0 { > This node name is shared with upstream imx6qdl-sr-som.dtsi Give this one a u-boot-only internal name, maybe ethernet-phy@ff >> + /* >> + * The PHY can appear either: >> + * - AR8035: at address 0 or 4 >> + * - ADIN1300: at address 1 >> + * Actual address being detected at runtime. >> + */ >> + reg = <0xffffffff>; >> + qca,clk-out-frequency = <125000000>; >> + qca,smarteee-tw-us-1g = <24>; >> + adi,phy-output-clock = "125mhz-free-running"; >> + }; >> + }; >> +}; >> diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c b/board/solidrun/mx6cuboxi/mx6cuboxi.c >> index 8edabf4404c2..fbab39e800a6 100644 >> --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c >> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c >> @@ -447,7 +447,7 @@ static int find_ethernet_phy(void) >> */ >> int ft_board_setup(void *fdt, struct bd_info *bd) >> { >> - int node_phy0, node_phy1, node_phy4; >> + int node_phy, node_phy0, node_phy1, node_phy4; >> int ret, phy; >> bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = false; >> enum board_type board; >> @@ -479,6 +479,12 @@ int ft_board_setup(void *fdt, struct bd_info *bd) >> return 0; >> } >> >> + // update U-Boot's own unified phy node phy address, if present >> + node_phy = fdt_path_offset(fdt, "/soc/bus@2100000/ethernet@2188000/mdio/phy"); > This node no longer exists, unless you rename the u-boot-specific one. > The u-boot node should probably have its own separate name, to ensure > we do not find the upstream linux dtb ethernet-phy@0 node by mistake. Look-up the u-boot-internal phy node name here >> + ret = fdt_setprop_u32(fdt, node_phy, "reg", phy); >> + if (ret < 0) >> + pr_err("%s: failed to update unified PHY node address\n", __func__); >> + >> // update all phy nodes status >> node_phy0 = fdt_path_offset(fdt, "/soc/bus@2100000/ethernet@2188000/mdio/ethernet-phy@0"); >> ret = fdt_setprop_string(fdt, node_phy0, "status", enable_phy0 ? "okay" : "disabled"); Just disable the u-boot-internal phy node unconditionally, or delete it. Because u-boot DTB is synced with upstream, the code below will update status properties for the standard etherent-phy@[0,1,4] nodes used by Linux.
Hi Josua, On Thu, Mar 28, 2024 at 10:03 AM Josua Mayer <josua@solid-run.com> wrote: > I suggest changing their status to disabled, and keeping the nodes. > >> + > >> + phy: ethernet-phy@0 { > > This node name is shared with upstream imx6qdl-sr-som.dtsi > Give this one a u-boot-only internal name, maybe ethernet-phy@ff ... > Just disable the u-boot-internal phy node unconditionally, or delete it. > Because u-boot DTB is synced with upstream, the code below will update status properties > for the standard etherent-phy@[0,1,4] nodes used by Linux. Thanks for the suggestions. My imx6-cuboxi board does not turn on anymore, unfortunately. As I cannot test it, I am not comfortable doing the changes. I hope you or Christian can submit a proper fix.
diff --git a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi index e9b188ed6587..358cf8abc4ff 100644 --- a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi +++ b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+ #include "imx6qdl-u-boot.dtsi" +#include "imx6qdl-sr-som-u-boot.dtsi" / { board-detect { diff --git a/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi b/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi new file mode 100644 index 000000000000..4c5f043ea92a --- /dev/null +++ b/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) + +#include <dt-bindings/gpio/gpio.h> + +&fec { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_microsom_enet_ar8035>; + phy-handle = <&phy>; + phy-mode = "rgmii-id"; + + /* + * The PHY seems to require a long-enough reset duration to avoid + * some rare issues where the PHY gets stuck in an inconsistent and + * non-functional state at boot-up. 10ms proved to be fine . + */ + phy-reset-duration = <10>; + phy-reset-gpios = <&gpio4 15 GPIO_ACTIVE_LOW>; + status = "okay"; + + mdio { + #address-cells = <1>; + #size-cells = <0>; + + /delete-node/ ethernet-phy@1; + /delete-node/ ethernet-phy@4; + + phy: ethernet-phy@0 { + /* + * The PHY can appear either: + * - AR8035: at address 0 or 4 + * - ADIN1300: at address 1 + * Actual address being detected at runtime. + */ + reg = <0xffffffff>; + qca,clk-out-frequency = <125000000>; + qca,smarteee-tw-us-1g = <24>; + adi,phy-output-clock = "125mhz-free-running"; + }; + }; +}; diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c b/board/solidrun/mx6cuboxi/mx6cuboxi.c index 8edabf4404c2..fbab39e800a6 100644 --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c @@ -447,7 +447,7 @@ static int find_ethernet_phy(void) */ int ft_board_setup(void *fdt, struct bd_info *bd) { - int node_phy0, node_phy1, node_phy4; + int node_phy, node_phy0, node_phy1, node_phy4; int ret, phy; bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = false; enum board_type board; @@ -479,6 +479,12 @@ int ft_board_setup(void *fdt, struct bd_info *bd) return 0; } + // update U-Boot's own unified phy node phy address, if present + node_phy = fdt_path_offset(fdt, "/soc/bus@2100000/ethernet@2188000/mdio/phy"); + ret = fdt_setprop_u32(fdt, node_phy, "reg", phy); + if (ret < 0) + pr_err("%s: failed to update unified PHY node address\n", __func__); + // update all phy nodes status node_phy0 = fdt_path_offset(fdt, "/soc/bus@2100000/ethernet@2188000/mdio/ethernet-phy@0"); ret = fdt_setprop_string(fdt, node_phy0, "status", enable_phy0 ? "okay" : "disabled");