Received-SPF: pass (google.com: domain of linux-kernel+bounces-2135-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <a9798333-3105-422f-8033-76c0b1d4f439@quicinc.com>
Date: Sat, 16 Dec 2023 15:37:52 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY
 properties
To: Andrew Lunn <andrew@lunn.ch>
CC: <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>,
        <hkallweit1@gmail.com>, <linux@armlinux.org.uk>, <corbet@lwn.net>,
        <p.zabel@pengutronix.de>, <f.fainelli@gmail.com>,
        <netdev@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-doc@vger.kernel.org>
References: <20231215074005.26976-1-quic_luoj@quicinc.com>
 <20231215074005.26976-15-quic_luoj@quicinc.com>
 <60b9081c-76fa-4122-b7ae-5c3dcf7229f9@lunn.ch>
 <a65ad12d-b990-4439-b196-903f4a5f096a@quicinc.com>
 <f5c5cbce-c36e-498a-97e2-35f06d927d74@lunn.ch>
Content-Language: en-US
From: Jie Luo <quic_luoj@quicinc.com>
In-Reply-To: <f5c5cbce-c36e-498a-97e2-35f06d927d74@lunn.ch>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit


On 12/15/2023 9:31 PM, Andrew Lunn wrote:
> On Fri, Dec 15, 2023 at 08:33:00PM +0800, Jie Luo wrote:
>>
>>
>> On 12/15/2023 8:12 PM, Andrew Lunn wrote:
>>>> +  clocks:
>>>> +    items:
>>>> +      - description: APB bridge clock
>>>> +      - description: AHB clock
>>>> +      - description: Security control clock
>>>> +      - description: TLMM clock
>>>> +      - description: TLMM AHB clock
>>>> +      - description: CNOC AHB clock
>>>> +      - description: MDIO AHB clock
>>>> +      - description: MDIO master AHB clock
>>>> +      - description: PCS0 system clock
>>>> +      - description: PCS1 system clock
>>>> +      - description: EPHY0 system clock
>>>> +      - description: EPHY1 system clock
>>>> +      - description: EPHY2 system clock
>>>> +      - description: EPHY3 system clock
> 
>> Hi Andrew,
>> These clocks are for the whole PHY package including quad PHYs, since
>> these clocks & resets need to be initialized at one point, i put it
>> the previous MDIO driver code, these clocks & resets are configured
>> after GPIO hardware reset, after these clocks and resets sequences
>> configured, each PHY capabilities can be acquired correctly in the PHY
>> probe function.
> 
> I really expect the hardware is hierarchical. Its unlikely that EPHY0
> is connected to all four PHYs in the package. Its specific to one
> PHY. So it should be in the DT properties for that one specific PHY. I
> expect the resets are the same. It seems there is a soft and hard
> reset per PHY, so i would expect these to be in the node for one PHY.be 

Hi Andrew,
i understand your point, i tried putting the related clocks and resets
into each device node per PHY, which does not work, since these clocks
ans resets need to be initialized at one function pointer after GPIO 
reset on the qca8084 package.

Sorry for these confusions here, let me explain the qca8084 chip more
detail here, and i will also update this info on the cover letter.

The following is the chip package, the chip can work on the switch mode
like the existed upstream code qca8k, where PHY1-PHY4 is connected with
MAC1-MAC4 directly; The chip can also work on the PHY mode, where PHY1-
PHY4 is connected with PCS1 by 10g-qxgmii; Either switch mode or PHY 
mode, the PHY4 is optionally connected with PCS0 by SGMII, PCS0 and PCS1
are connected with the SoC(IPQ platform) PCSes.
+----------------------------------------------+
|          PCS1           PCS0                 |
|                                              |
|          MAC0           MAC5                 |
|                                              |
|                                              |
|                                              |
|      MAC1      MAC2      MAC3      MAC4      |
|                                              |
|      PHY1      PHY2      PHY3      PHY4      |
+----------------------------------------------+

After the GPIO reset on this package from the MDIO bus level, the chip
is in the cold hardware reset status, the clocks and resets mentioned
here need to be initialized before the capabilities of PHY can be
acquired correctly in the PHY probe function, that is why i put these
clocks and resets in the first PHY device tree node.
When the chip works on the PHY mode, the MAC listed in the block is
not used, the MAC is only used in the switch mode.

i know we can also put the related clocks and resets into the each
PHY and PCS device node to make the hardware hierarchical, then i can
read all initial clocks and resets in the first PHY probe function,
but there are still some package level clocks such as APB/AHB/CNOC
clocks needed for these initial configuration, so i put these clocks
and resets at one PHY device node and only needed to be called by one
time.

 From the clocks and resets name, EPHY0 is specific to the first PHY1,
which is providing clock and reset on the first PHY1, other EPHY clock
and reset is same for the corresponding PHY.

> 
> Do the two PCS instances take up two MDIO address? They can be
> considered devices on the bus, so could have a DT node, and hence you
> can place the PCS clocks on that node?

Yes, two PCS instances take up two MDIO device with different MDIO
address, which also have the DT nodes as the child node of MDIO bus node
there is also some specific clocks and resets defined in the PCS DT
node for the PCS driver.

> 
> What exactly do the two MDIO clocks do? I assume these are not for the
> MDIO bus master, but the MDIO slave block within the PHY package?
> There is one MDIO slave block shared by the four PHYs. So these are
> package properties and should be in the package node in DT.

The MDIO clocks are the qca8084 package level clock, since there
are other modules such as GCC and security control located in the
qca8084 chip, these module register is also accessed by the MDIO bus,
the MDIO AHB clock is this modules access.
there is also a MDIO master for the back pressure function in qca8084
chip, the MDIO master AHB clock is for this function, actually this
function is for the switch mode, but it is the package level clock,
so i put it together here.

No, these clocks are not for the IPQ4019 SoC MDIO bus master. Four PHYs 
are the independent MDIO slave devices.

For the switch mode, we can define these package level clocks and resets
in the DSA device node.

But for the PHY mode, All 4 PHYs is connected with PCS1 by 10g-qxgmii,
there is no package level device tree node defined.

> 
> Look at all the other clocks and decide, are they package clocks, or
> specific to one block on the MDIO bus? Do the properties go in the
> package node, or the per PHY node?
> 
> 	Andrew

The clocks and resets with prefix PCS or EPHY is per PHY/PCS node, other
clocks and resets are the qca8084 package level node.