mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jie Luo <quic_luoj@quicinc.com>
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>
Subject: Re: [net-next PATCH RFC v3 1/8] dt-bindings: net: document ethernet PHY package nodes
Date: Wed, 10 Jan 2024 11:13:17 +0800	[thread overview]
Message-ID: <2de50b16-bcf7-401d-a4f9-79540ac24311@quicinc.com> (raw)
In-Reply-To: <2f2328ee-205b-4b4f-a683-2df4fbb22dde@lunn.ch>



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

  reply	other threads:[~2024-01-10  3:14 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-26  1:53 [net-next PATCH RFC v3 0/8] net: phy: Support DT PHY package Christian Marangi
2023-11-26  1:53 ` [net-next PATCH RFC v3 1/8] dt-bindings: net: document ethernet PHY package nodes Christian Marangi
2023-11-27 22:16   ` Rob Herring
2023-11-28  0:09     ` Andrew Lunn
2024-01-07 18:00   ` Sergey Ryazanov
2024-01-07 18:30     ` Christian Marangi
2024-01-07 18:44       ` Andrew Lunn
2024-01-07 21:57         ` Sergey Ryazanov
2024-01-07 21:49       ` Sergey Ryazanov
2024-01-07 21:45         ` Christian Marangi
2024-01-17  0:36         ` Christian Marangi
2024-01-17 19:39           ` Sergey Ryazanov
2024-01-17 20:03             ` Christian Marangi
2024-01-17 20:05             ` Andrew Lunn
2024-01-07 18:31     ` Andrew Lunn
2024-01-08 11:13       ` Jie Luo
2024-01-08 13:19         ` Andrew Lunn
2024-01-09 11:44           ` Jie Luo
2024-01-09 13:48             ` Andrew Lunn
2024-01-10  3:13               ` Jie Luo [this message]
2023-11-26  1:53 ` [net-next PATCH RFC v3 2/8] net: phy: add initial support for PHY package in DT Christian Marangi
2023-11-28  0:39   ` Andrew Lunn
2023-11-28 12:09     ` Christian Marangi
2023-11-26  1:53 ` [net-next PATCH RFC v3 3/8] net: phy: add support for shared priv data size " Christian Marangi
2023-11-26  1:53 ` [net-next PATCH RFC v3 4/8] net: phy: add support for driver specific PHY package probe/config Christian Marangi
2023-11-26  1:53 ` [net-next PATCH RFC v3 5/8] dt-bindings: net: add QCA807x PHY defines Christian Marangi
2023-11-27 22:36   ` Rob Herring
2023-11-26  1:53 ` [net-next PATCH RFC v3 6/8] dt-bindings: net: Document Qcom QCA807x PHY package Christian Marangi
2023-11-27 22:35   ` Rob Herring
2023-11-28  0:30   ` Andrew Lunn
2023-11-26  1:53 ` [net-next PATCH RFC v3 7/8] net: phy: add Qualcom QCA807x driver Christian Marangi
2023-11-26  1:53 ` [net-next PATCH RFC v3 8/8] net: phy: qca807x: Add support for configurable LED Christian Marangi
2023-11-28  0:20 ` [net-next PATCH RFC v3 0/8] net: phy: Support DT PHY package Andrew Lunn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2de50b16-bcf7-401d-a4f9-79540ac24311@quicinc.com \
    --to=quic_luoj@quicinc.com \
    --cc=agross@kernel.org \
    --cc=andersson@kernel.org \
    --cc=andrew@lunn.ch \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=ansuelsmth@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=konrad.dybcio@linaro.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux@armlinux.org.uk \
    --cc=matthias.bgg@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=robert.marko@sartura.hr \
    --cc=robh+dt@kernel.org \
    --cc=ryazanov.s.a@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®