Re: [PATCH] mx6cuboxi: Fix Ethernet after DT sync with Linux

[Josua Mayer <josua@solid-run.com>]

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.