On 10/4/2024 7:54 PM, Russell King (Oracle) wrote:
> On Thu, Oct 03, 2024 at 11:20:03PM +0530, Kiran Kumar C.S.K wrote:
>>>> +---------+
>>>> | 48MHZ |
>>>> +----+----+
>>>> |(clock)
>>>> v
>>>> +----+----+
>>>> +------| CMN PLL |
>>>> | +----+----+
>>>> | |(clock)
>>>> | v
>>>> | +----+----+ +----+----+ clock +----+----+
>>>> | +---| NSSCC | | GCC |--------->| MDIO |
>>>> | | +----+----+ +----+----+ +----+----+
>>>> | | |(clock & reset) |(clock & reset)
>>>> | | v v
>>>> | | +-----------------------------+----------+----------+---------+
>>>> | | | +-----+ |EDMA FIFO | | EIP FIFO|
>>>> | | | | SCH | +----------+ +---------+
>>>> | | | +-----+ | | |
>>>> | | | +------+ +------+ +-------------------+ |
>>>> | | | | BM | | QM | | L2/L3 Switch Core | |
>>>> | | | +------+ +------+ +-------------------+ |
>>>> | | | | |
>>>> | | | +-------+ +-------+ +-------+ +-------+ +-------+ +-------+ |
>>>> | | | | MAC0 | | MAC1 | | MAC2 | | MAC3 | | XGMAC4| |XGMAC5 | |
>>>> | | | +---+---+ +---+---+ +---+---+ +---+---+ +---+---+ +---+---+ |
>>>> | | | | | | | | | |
>>>> | | +-----+---------+---------+---------+---------+---------+-----+
>>>> | | | | | | | |
>>>> | | +---+---------+---------+---------+---+ +---+---+ +---+---+
>>>> +--+---->| PCS0 | | PCS1 | | PCS2 |
>>>> | clock +---+---------+---------+---------+---+ +---+---+ +---+---+
>>>> | | | | | | |
>>>> | +---+---------+---------+---------+---+ +---+---+ +---+---+
>>>> | clock +----------------+ | | | | |
>>>> +------->|Clock Controller| 4-port Eth PHY | | PHY4 | | PHY5 |
>>>> +----------------+--------------------+ +-------+ +-------+
> ...
>>>> 3) PCS driver patch series:
>>>> Driver for the PCS block in IPQ9574. New IPQ PCS driver will
>>>> be enabled in drivers/net/pcs/
>>>> Dependent on NSS CC patch series (2).
>>>
>>> I assume this dependency is pure at runtime? So the code will build
>>> without the NSS CC patch series?
>>
>> The MII Rx/Tx clocks are supplied from the NSS clock controller to the
>> PCS's MII channels. To represent this in the DTS, the PCS node in the
>> DTS is configured with the MII Rx/Tx clock that it consumes, using
>> macros for clocks which are exported from the NSS CC driver in a header
>> file. So, there will be a compile-time dependency for the dtbindings/DTS
>> on the NSS CC patch series. We will clearly call out this dependency in
>> the cover letter of the PCS driver. Hope that this approach is ok.
>
> Please distinguish between the clocks that are part of the connection
> between the PCS and PHY and additional clocks.
>
> For example, RGMII has its own clocks that are part of the RGMII
> interface. Despite DT having a way to describe clocks, these clocks
> are fundamental to the RGMII interface and are outside of the scope
> of DT to describe. Their description is implicit in the relationship
> between the PHY and network driver.
>
> Also, the PCS itself is a subset of the network driver, and we do
> not (as far as I know) ever describe any kind of connection between
> a PCS and PHY. That would be madness when we have situations where
> the PHY can change its serdes mode, causing the MAC to switch
> between several PCS - which PCS would one associate the PHY with in
> DT when the "mux" is embedded in the ethernet driver and may be
> effectively transparent?
>
Apologies for the delay in response. I understand that the PCS<->PHY
clocks may be out of the scope of PCS DT due to the reasons you mention.
However would like to clarify that the MII clocks referred to here, are
part of the connection between the MAC and PCS and not between PCS and PHY.
Below is a diagram that shows the sub-blocks inside the 'UNIPHY' block
of IPQ9574 which houses the PCS and the serdes, along with the clock
connectivity. The MII Rx/Tx clocks are supplied from the NSS CC, to the
GMII channels between PCS and MAC. So, it seemed appropriate to have
these clocks described as part of the PCS DT node.
+-------+ +---------+ +-------------------------+
-----------|CMN PLL| | GCC | | NSSCC (Divider) |
|25/50mhz +----+--+ +----+----+ +--+-------+--------------+
|clk | | ^ |
| 31.25M | SYS/AHB|clk RX/TX|clk +------------+
| ref clk | | | | |
| | v | MII RX|TX clk MAC| RX/TX clk
| +--+---------+----------+-------+---+ +-+---------+
| | | +----------------+ | | | | PPE |
v | | | UNIPHY0 V | | V |
+-------+ | v | +-----------+ (X)GMII| | |
| | | +---+---+ | |--------|------|-- MAC0 |
| | | | | | | (X)GMII| | |
| Quad | | |SerDes | | (X)PCS |--------|------|-- MAC1 |
| +<----+ | | | | (X)GMII| | |
|(X)GPHY| | | | | |--------|------|-- MAC2 |
| | | | | | | (X)GMII| | |
| | | +-------+ | |--------|------|-- MAC3 |
+-------+ | | | | | |
| +-----------+ | | |
+-----------------------------------+ | |
+--+---------+----------+-------+---+ | |
+-------+ | UNIPHY1 | | |
| | | +-----------+ | | |
|(X)GPHY| | +-------+ | | (X)GMII| | |
| +<----+ |SerDes | | (X)PCS |--------|------|- MAC4 |
| | | | | | | | | |
+-------+ | +-------+ | | | | |
| +-----------+ | | |
+-----------------------------------+ | |
+--+--------+-----------+-------+---+ | |
+-------+ | UNIPHY2 | | |
| | | +-----------+ | | |
|(X)GPHY| | +-------+ | | (X)GMII| | |
| +<----+ |SerDes | | (X)PCS |--------|------|- MAC5 |
| | | | | | | | | |
+-------+ | +-------+ | | | | |
| +-----------+ | | |
+-----------------------------------+ +-----------+
We had one other question on the approach used in the driver for PCS
clocks, could you please provide your comments.
As we can see from the above diagram, each serdes in the UNIPHY block
provides the clocks to the NSSCC, and the PCS block consumes the MII
Rx/Tx clocks. In our current design, the PCS/UNIPHY driver registers a
provider driver for the clocks that the serdes supplies to the NSS CC.
It also enables the MII Rx/Tx clocks which are supplied to the PCS from
the NSS CC. Would this be an acceptable design to have the PCS driver
register the clock provider driver and also consume the MII Rx/Tx
clocks? It may be worth noting that the serdes and PCS are part of the
same UNIPHY block and also share same register region.
Thank you.
