Message-ID: <62c98d4f-8f02-43cc-8af6-99edfa5f6c88@gmail.com>
Date: Wed, 14 May 2025 21:32:12 -0500
From: "Alex G." <mr.nuke.me@...il.com>
To: Lei Wei <quic_leiwei@...cinc.com>,
 "Russell King (Oracle)" <linux@...linux.org.uk>,
 Jakub Kicinski <kuba@...nel.org>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Andrew Lunn <andrew@...n.ch>,
 Heiner Kallweit <hkallweit1@...il.com>, netdev@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-msm@...r.kernel.org, quic_kkumarcs@...cinc.com,
 quic_suruchia@...cinc.com, quic_pavir@...cinc.com, quic_linchen@...cinc.com,
 quic_luoj@...cinc.com, srinivas.kandagatla@...aro.org,
 bartosz.golaszewski@...aro.org, vsmuthu@....qualcomm.com, john@...ozen.org,
 Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Subject: Re: [PATCH net-next v5 0/5] Add PCS support for Qualcomm IPQ9574 SoC

On 5/14/25 11:03, Lei Wei wrote:> On 5/13/2025 6:56 AM, 
mr.nuke.me@...il.com wrote:
>> On 2/19/25 4:46 AM, Lei Wei wrote:
>>
>> 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
>>
>>
> 
> Hello, thanks for bringing this to our notice. Let me try to understand 
> the reason for the probe failure:
> 
> The merged NSSCC DTS does not reference the PCS node directly in the 
> "clocks" property. It uses a placeholder phandle '<0>' for the 
> reference. Please see below patch which is merged.
> https://lore.kernel.org/all/20250313110359.242491-6-quic_mmanikan@quicinc.com/
> 
> Ideally there should be no direct dependency from NSSCC to PCS driver if
> we use this version of the NSSCC DTS.
> 
> Hence it seems that you may have a modified patch here, and DTS changes 
> have been applied to enable all the Ethernet components including PCS 
> and NSSCC, and NSSCC modified to have a direct reference to PCS? However 
> even in this case, I think the driver probe should work if the drivers 
> are built as modules. Can you please confirm if the NSSCC and PCS 
> drivers are built-in to the kernel and not built as modules

The NSSCC and PCS built-in. I also added the uniphy PCS clocks to the 
NSSCC in order to expose the issue.

I have a heavily patched tree with PPE driver and EDMA support. That's 
the final use case in order to support ethernet, right?


> For the case where the drivers are built-in to kernel, and the NSSCC DTS
> node has a direct reference to PCS node, we can use the below solution:
> [Note that the 'UNIPHY' PCS clocks are not needed for NSSCC clocks
> initialization/registration.]
> 
>      Enable 'post-init-providers' property in the NSSCC DTS node to mark
>     'UNIPHY' PCS as post-initialization providers to NSSCC. This will
>      ensure following probe order by the kernel:
> 
>      1.) NSSCC driver
>      2.) PCS driver.
> 
> Please let me know if the above suggestion can help.

I see. Adding the 'post-init-providers' property does fix the circular 
dependency. Thank you!

I have another question. Do you have a public repository with the 
unmerged IPQ9574 patches, including, PCS, PPE, EDMA, QCA8084 ?


> Later once the IPQ PCS driver is merged, we are planning to push the PCS 
> DTS changes, along with an update of the NSSCC DTS to point to the PCS 
> node and mark the "post-init-providers" property. This should work for 
> all cases.
> 
> Also, in my view, it is not suitable to move PCS MII clocks get to
> "ipq_pcs_get()" because the natural loading order for the drivers
> is as below:
> 
> 1) NSSCC driver
> 2) PCS driver
> 3) Ethernet driver.
> 
> Additionally, the community is currently working on an infrastructure to
> provide a common pcs get method. (Christian and Sean Anderson has been 
> working on this). Therefore, I expect "ipq_pcs_get" to be dropped in the 
> future and replaced with the common pcs get method once this common 
> infra is merged.

That makes sense. Thank you for clarifying.

Powered by blists - more mailing lists