Message-ID: <be7f0fda-ac21-4f94-a6cf-b4c3ca59630a@gmail.com>
Date: Thu, 15 May 2025 20:40:33 -0500
From: 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/15/25 10:27 AM, Lei Wei wrote:
> 
> 
> On 5/15/2025 10:32 AM, Alex G. wrote:
>> 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@...cinc.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?
>>
> 
> Yes, all the drivers are eventually for enabling the Ethernet function
> on IPQ9574.
> 
>>
>>> 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 ?
>>
> 
> May I know the source of your PPE/EDMA changes using which this issue
> is seen?

I use a mix of upstream submissions, and openwrt patches. As noted, 
using 'post-init-providers' takes care of the problem.

https://github.com/mrnuke/linux/commits/ipq95xx-devel-20250515/

> 
> The openwrt repository contains the unmerged IPQ9574 patches, Although
> this version will be updated very soon with latest code(with some 
> fixes), the version of the code in the repo currently is also functional 
> and tested.
> 
> https://github.com/CodeLinaro/openwrt/tree/main/target/linux/qualcommbe/ 
> patches-6.6


Will you be updating a clock example with IPQ9574 + QCA8084 to the repo?

Alex

>>
>>> 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