Received-SPF: pass (google.com: domain of linux-kernel+bounces-21681-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <2de50b16-bcf7-401d-a4f9-79540ac24311@quicinc.com>
Date: Wed, 10 Jan 2024 11:13:17 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [net-next PATCH RFC v3 1/8] dt-bindings: net: document ethernet
 PHY package nodes
Content-Language: en-US
To: Andrew Lunn <andrew@lunn.ch>
CC: Sergey Ryazanov <ryazanov.s.a@gmail.com>,
        Christian Marangi
	<ansuelsmth@gmail.com>,
        Robert Marko <robert.marko@sartura.hr>,
        Vladimir
 Oltean <olteanv@gmail.com>, Rob Herring <robh+dt@kernel.org>,
        "David S.
 Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>, Jakub
 Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley
	<conor+dt@kernel.org>, Andy Gross <agross@kernel.org>,
        Bjorn Andersson
	<andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Heiner
 Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
        <netdev@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-mediatek@lists.infradead.org>
References: <20231126015346.25208-1-ansuelsmth@gmail.com>
 <20231126015346.25208-2-ansuelsmth@gmail.com>
 <0926ea46-1ce4-4118-a04c-b6badc0b9e15@gmail.com>
 <1437d9df-2868-43f5-aebd-e0c57fe4d905@lunn.ch>
 <b75e6267-7d54-412e-8882-af4d9a0b54e6@quicinc.com>
 <841ef784-b27e-4f7a-94f2-f04f93178c61@lunn.ch>
 <07c01c11-0cc2-4837-b371-1404f2ce3359@quicinc.com>
 <2f2328ee-205b-4b4f-a683-2df4fbb22dde@lunn.ch>
From: Jie Luo <quic_luoj@quicinc.com>
In-Reply-To: <2f2328ee-205b-4b4f-a683-2df4fbb22dde@lunn.ch>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 1/9/2024 9:48 PM, Andrew Lunn wrote:
>>
>> __| |_______________| |__
>> | PCS0 |          |PCS1 |
>> |______|          |_____|
>> |_______                |<---- REF clock 50MHZ
>> |      |------------    |
>> |NSSCC |    |clks  |rsts|<---- GPIO reset
>> |______|    |      |    |
>> |           V      V    |
>> |_______________________|
>> |     |     |     |     |
>> |PHY1 |PHY2 |PHY3 |PHY4 |
>> |_____|_____|_____|_____|
> 
> Not the best of improvements. So the ref clock goes to the package,
> and then magically somehow gets to the NSSCC? Are there any more
> blocks it goes through before reaching the NSSCC? How does the GPIO
> reset get converted into multiple reset inside the package? Details,
> details, details.

The GPIO reset for the whole CHIP, like a GPIO reset on the single port
PHY qca8081, after the GPIO reset, the whole qca8084 chip is in the cold
reset state, so the initial clocks ans resets need to be initialized as
we are designing the clock model here.

As for the reference clock 50M input, this reference clock 50MHZ is
PLLed by the PCS1 block of qca8084, then there are fix 312.5MHZ is
generated to the NSSCC, NSSCC includes multiple RCG clock trees to
generate the clocks to the PHY and PCS, all these blocks(PCS and PHY)
are located in the qca8084 PHY chip, sorry for missed this diagram
info, updated the diagram as below, since the PCS generates the clocks
to NSSCC, there are initial clocks ans resets included for PCS need
to be configured before CHIP to work.

So the parent clocks are PCS0(uniphy0) and PCS1(uniphy1) for NSSCC,
as i provide the DT of NSSCC provider below, the parent clock rate
of PCS is changed according to the PHY link speed, and there are
also fix clock rate 312.5MHZ from PCS1, these parent clocks are
the root clock of RCG clock tree in NSSCC, the output clocks of
the RCG are consumed by the PHYs(PHY1-PHY4 and PCS0, PCS1).

    REF 50MHZ
       |
       |
______V__________________
| PCS1 |       ____|PCS0|
|______|      |    |____|
|  |  ________|         |
|  | |                  |<----------GPIO reset
|__V_V__                |
|      |------------    |
|NSSCC |    |clks  |rsts|
|______|    |      |    |
|           V      V    |
|_______________________|
|     |     |     |     |
|PHY1 |PHY2 |PHY3 |PHY4 |
|_____|_____|_____|_____|

> 
>> There are difference clock trees generated from NSSCC for the different
>> PHYs, all clocks and resets for qca8084 CHIP working are internally
>> provided by the NSSCC.
> 
> So show this in the block diagram.
> 
>> Yes, Andrew, the NSSCC provider driver is probed based on the MDIO
>> device, the PHY CHIP occupies the MDIO addresses, so the NSSCC is
>> registered as the MDIO device.
>>
>> DT of the NSSCC device node:
>> mdio {
>>        #address-cells = <1>;
>>        #size-cells = <0>;
>>
>>        clock-controller@18 {
>>          compatible = "qcom,qca8084-nsscc";
>>          reg = <0x18>;
>>          clocks = <&qca8k_xo>,
>>                   <&qca8k_uniphy0_rx>,
>>                   <&qca8k_uniphy0_tx>,
>>                   <&qca8k_uniphy1_rx>,
>>                   <&qca8k_uniphy1_tx>,
>>                   <&qca8k_uniphy1_rx312p5m>,
>>                   <&qca8k_uniphy1_tx312p5m>;
>>          #clock-cells = <1>;
>>          #reset-cells = <1>;
>>          #power-domain-cells = <1>;
>>        };
>>      };
>   
> This does not make any sense. You have one clock input, 50MHz. So why
> are you listing 6 consumer clocks, not one? And where are the clocks
> this clock controller provides, clock-output-names=<...>;

Hi Andrew,
Sorry for the confusion.
the DT of the NSSCC device node is from the NSSCC provider driver, which
is under review currently as the link below.
https://lore.kernel.org/lkml/20231104034858.9159-2-quic_luoj@quicinc.com/T/#m204c22d14be8f9dda7cd7f666ed726b8fc3301ef

the property "clocks" are the parent clock list of RCG clock tree in
the NSSCC driver, for qca8084 chip, there are two UNIPHY(PCS) as
mentioned in the diagram, there are also fix rate clocks 312.5MHZ,
the output clocks of NSSCC are not listed in the DT, which is same
as the GCC(IPQ SoC) DT.

The output clocks of NSSCC are provided by the DT of the clock consumer
such as the DT of qca8084 as below.
phy0 {
         reg = <0>;
         clocks = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_CLK>;
         clock-names = "gephy0_sys";
         resets = <&qca8k_nsscc NSS_CC_GEPHY0_SYS_ARES>,
                  <&qca8k_nsscc NSS_CC_GEPHY0_ARES>;
         reset-names = "gephy0_sys", "gephy0_soft";
      };

In the DT node above, the qca8k_nssc refers to the NSSCC clock provider
DT node "clock-controller@18", the clock ID "NSS_CC_GEPHY0_SYS_CLK" is
also provided by the NSSCC clock provider driver.

The different PHY(PHY1-PHY4) of qca8084 has the different clock
tree(RCG), the clock consumer driver needs to define the DT properties
clocks and clcok-names to get the clocks from the NSSCC clock provider.

> 
> I give up. Please consider this PHY driver NACKed.
> 
> Get Linaro, or some other organisation with a lot of experience with
> mainline to take over the work.
> 
> 	 Andrew