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

[mr.nuke.me@gmail.com]

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@gmail.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?
>>
> 
> 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.
>