On 5/13/2025 6:56 AM, mr.nuke.me@gmail.com wrote:

On 2/19/25 4:46 AM, Lei Wei wrote:

quoted

On 2/12/2025 6:19 PM, Russell King (Oracle) wrote:

quoted

On Tue, Feb 11, 2025 at 07:59:34PM -0800, Jakub Kicinski wrote:

quoted

On Fri, 7 Feb 2025 23:53:11 +0800 Lei Wei wrote:

quoted

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

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/ (local)

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?

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.

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.

This is the post-init-providers dtschma:
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/post-init-providers.yaml

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>;
             ...
         };