Re: [PATCH v2 3/5] ARM: dts: imx6qdl-sr-som: add support for alternate phy addresses

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

Hi Mark,

Am 19.05.22 um 13:21 schrieb Mark Kettenis:
>> From: Josua Mayer <josua@solid-run.com>
>> Date: Thu, 19 May 2022 12:31:58 +0300
>>
>> The Cubox has an unstable phy address - which can appear at either
>> address 0 (intended) or 4 (unintended).
>>
>> SoM revision 1.9 has replaced the ar8035 phy with an adin1300, which
>> will always appear at address 1.
>>
>> Change the reg property of the phy node to the magic value 0xffffffff,
>> which indicates to the generic phy driver that all addresses should be
>> probed. That allows the same node (which is pinned by phy-handle) to match
>> either the AR8035 PHY at both possible addresses, as well as the new one
>> at address 1.
>> Also add the new adi,phy-output-clock property for enabling the 125MHz
>> clock used by the fec ethernet controller, as submitted to Linux [1].
>>
>> Linux solves this problem differently:
>> For the ar8035 phy it will probe both phy nodes in device-tree in order,
>> and use the one that succeeds. For the new adin1300 it expects U-Boot to
>> patch the status field in the DTB before booting
> 
> And U-Boot should do the same thing for its own device tree such that
> the built-in DTB can simply be passed to the OS.  There should be no
> difference between the U-Boot device tree and the Linux device tree.
Should, but that does not currently work.
I actually tried doing it before submitting v1, and I found that the phy 
driver would not find the device-tree properties I was adding to the 
phy@1 node.
So it did not enable the required clock output, hence the ethernet port 
was not functional.
We also have a non-Linux phy-handle property, so I suspect there were 
reasons to not use the Linux dts for i.MX6 ethernet.

This is why instead I added to the one single phy node.
> 
>> While at it also sync the reset-delay with the upstream Linux dtb.
>>
>> [1] https://patchwork.kernel.org/project/netdevbpf/patch/20220428082848.12191-4-josua@solid-run.com/
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> ---
>>   arch/arm/dts/imx6qdl-sr-som.dtsi | 17 +++++++++++++++--
>>   1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/dts/imx6qdl-sr-som.dtsi b/arch/arm/dts/imx6qdl-sr-som.dtsi
>> index b06577808f..c20bed2721 100644
>> --- a/arch/arm/dts/imx6qdl-sr-som.dtsi
>> +++ b/arch/arm/dts/imx6qdl-sr-som.dtsi
>> @@ -55,7 +55,13 @@
>>   	pinctrl-0 = <&pinctrl_microsom_enet_ar8035>;
>>   	phy-handle = <&phy>;
>>   	phy-mode = "rgmii-id";
>> -	phy-reset-duration = <2>;
>> +
>> +	/*
>> +	 * 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";
>>   
>> @@ -64,8 +70,15 @@
>>   		#size-cells = <0>;
>>   
>>   		phy: ethernet-phy@0 {
>> -			reg = <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>;
>> +			adi,phy-output-clock = "125mhz-free-running";
>>   		};
>>   	};
>>   };
>> -- 
>> 2.35.3
>>
>>