Re: [PATCH net-next v5 0/5] Add PCS support for Qualcomm IPQ9574 SoC

[mr.nuke.me@gmail.com]

On 2/19/25 4:46 AM, Lei Wei wrote:
> 
> On 2/12/2025 6:19 PM, Russell King (Oracle) wrote:
>> On Tue, Feb 11, 2025 at 07:59:34PM -0800, Jakub Kicinski wrote:
>>> On Fri, 7 Feb 2025 23:53:11 +0800 Lei Wei wrote:
>>>> The 'UNIPHY' PCS block in the Qualcomm IPQ9574 SoC provides Ethernet
>>>> PCS and SerDes functions. It supports 1Gbps mode PCS and 10-Gigabit
>>>> mode PCS (XPCS) functions, and supports various interface modes for
>>>> the connectivity between the Ethernet MAC and the external PHYs/Switch.
>>>> There are three UNIPHY (PCS) instances in IPQ9574, supporting the six
>>>> Ethernet ports.
>>>>
>>>> This patch series adds base driver support for initializing the PCS,
>>>> and PCS phylink ops for managing the PCS modes/states. Support for
>>>> SGMII/QSGMII (PCS) and USXGMII (XPCS) modes is being added initially.
>>>>
>>>> The Ethernet driver which handles the MAC operations will create the
>>>> PCS instances and phylink for the MAC, by utilizing the API exported
>>>> by this driver.
>>>>
>>>> While support is being added initially for IPQ9574, the driver is
>>>> expected to be easily extendable later for other SoCs in the IPQ
>>>> family such as IPQ5332.
>>>
>>> Could someone with PHY, or even, dare I say, phylink expertise
>>> take a look here?
>>
>> I've not had the time, sorry. Looking at it now, I have lots of
>> questions over this.
>>
>> 1) clocks.
>>
>> - Patch 2 provides clocks from this driver which are exported to the
>>    NSCCC block that are then used to provide the MII clocks.
>> - Patch 3 consumes clocks from the NSCCC block for use with each PCS.
>>
>> Surely this leads to a circular dependency, where the MSCCC driver
>> can't get the clocks it needs until this driver has initialised, but
>> this driver can't get the clocks it needs for each PCS from the NSCCC
>> because the MSCCC driver needs this driver to initialise.
>>
> 
> Sorry for the delay in response. Below is a description of the 
> dependencies between the PCS/NSSCC drivers during initialization time 
> and how the clock relationships are set up. Based on this, there should 
> not any issue due to circular dependency, but please let me know if any 
> improvement is possible here given the hardware clock dependency. The 
> module loading order is as follows:
> 
> Step 1.) NSCC driver module
> Step 2.) PCS driver module
> Step 3.) Ethernet driver module
> 
> The 'UNIPHY' PCS clocks (from Serdes to NSSCC) are not needed to be 
> available at the time of registration of PCS MII clocks (NSSCC to PCS 
> MII) by the NSSCC driver (Step 1). The PCS MII clocks is registered 
> before 'UNIPHY' PCS clock is registered, since by default the parent is 
> initialized to 'xo' clock. Below is the output of clock tree on the 
> board before the PCS driver is loaded.
> 
> xo-board-clk
>      nss_cc_port1_rx_clk_src
>          nss_cc_port1_rx_div_clk_src
>              nss_cc_uniphy_port1_rx_clk
>              nss_cc_port1_rx_clk
> 
> The 'UNIPHY' PCS clock is later configured as a parent to the PCS MII 
> clock at the time when the Ethernet and PCS drivers are enabled (step3) 
> and the MAC links up. At link up time, the NSSCC driver sets the NSSCC 
> port clock rate (by configuring the divider) based on the link speed, 
> during which time the NSSCC port clock's parent is switched to 'UNIPHY' 
> PCS clock. Below is the clock tree dump after this step.
> 
> 7a00000.ethernet-pcs::rx_clk
>      nss_cc_port1_rx_clk_src
>          nss_cc_port1_rx_div_clk_src
>              nss_cc_uniphy_port1_rx_clk
>              nss_cc_port1_rx_clk
> 

I tried this PCS driver, and I am seeing a circular dependency in the 
clock init. If the clock tree is:
     GCC -> NSSCC -> PCS(uniphy) -> NSSCC -> PCS(mii)

The way I understand it, the UNIPHY probe depends on the MII probe. If 
MII .probe() returns -EPROBE_DEFER, then so will the UNIPHY .probe(). 
But the MII cannot probe until the UNIPHY is done, due to the clock 
dependency. How is it supposed to work?

The way I found to resolve this is to move the probing of the MII clocks 
to ipq_pcs_get().

This is the kernel log that I see:

[   12.008754] platform 39b00000.clock-controller: deferred probe 
pending: platform: supplier 7a00000.ethernet-pcs not ready
[   12.008788] mdio_bus 90000.mdio-1:18: deferred probe pending: 
mdio_bus: supplier 7a20000.ethernet-pcs not ready
[   12.018704] mdio_bus 90000.mdio-1:00: deferred probe pending: 
mdio_bus: supplier 90000.mdio-1:18 not ready
[   12.028588] mdio_bus 90000.mdio-1:01: deferred probe pending: 
mdio_bus: supplier 90000.mdio-1:18 not ready
[   12.038310] mdio_bus 90000.mdio-1:02: deferred probe pending: 
mdio_bus: supplier 90000.mdio-1:18 not ready
[   12.047943] mdio_bus 90000.mdio-1:03: deferred probe pending: 
mdio_bus: supplier 90000.mdio-1:18 not ready
[   12.057579] platform 7a00000.ethernet-pcs: deferred probe pending: 
ipq9574_pcs: Failed to get MII 0 RX clock
[   12.067209] platform 7a20000.ethernet-pcs: deferred probe pending: 
ipq9574_pcs: Failed to get MII 0 RX clock
[   12.077200] platform 3a000000.qcom-ppe: deferred probe pending: 
platform: supplier 39b00000.clock-controller not ready


PHY:
&mdio {
	qca8k_nsscc: clock-controller@18 {
		compatible = "qcom,qca8084-nsscc";
		...
	};

	ethernet-phy-package@0 {
		compatible = "qcom,qca8084-package";
		...

		qca8084_0: ethernet-phy@0 {
			compatible = "ethernet-phy-id004d.d180";
			reg = <0>;
			clocks = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>;
			resets = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>;
		};
		qca8084_1: ethernet-phy@1 {
			compatible = "ethernet-phy-id004d.d180";
			reg = <1>;
			clocks = <&qca8k_nsscc NSS_CC_GEPHY1_SYS_CLK>;
			resets = <&qca8k_nsscc NSS_CC_GEPHY1_SYS_ARES>;
		};
		qca8084_2: ethernet-phy@2 {
			compatible = "ethernet-phy-id004d.d180";
			reg = <2>;
			clocks = <&qca8k_nsscc NSS_CC_GEPHY2_SYS_CLK>;
			resets = <&qca8k_nsscc NSS_CC_GEPHY2_SYS_ARES>;
		};
		qca8084_3: ethernet-phy@3 {
			compatible = "ethernet-phy-id004d.d180";
			reg = <3>;
			clocks = <&qca8k_nsscc NSS_CC_GEPHY3_SYS_CLK>;
			resets = <&qca8k_nsscc NSS_CC_GEPHY3_SYS_ARES>;
		};
	};

	qca8081_12: ethernet-phy@12 {
		reset-gpios = <&tlmm 36 GPIO_ACTIVE_LOW>;
		reg = <12>;
	};

PCS:
	pcs_uniphy0: ethernet-pcs@7a00000 {
		compatible = "qcom,ipq9574-pcs";
		...
		pcsuniphy0_ch0: pcs-mii@0 {
			reg = <0>;
			clocks = <&nsscc NSS_CC_UNIPHY_PORT1_RX_CLK>,
				 <&nsscc NSS_CC_UNIPHY_PORT1_TX_CLK>;
			clock-names = "rx",
				      "tx";
		};
		...

MAC:
		port@1 {
			reg = <1>;
			phy-mode = "usxgmii";
			managed = "in-band-status";
			phy-handle = <&qca8084_0>;
			pcs-handle = <&pcsuniphy0_ch0>;
			...
		};