Re: RFC: Advice on adding support for Qualcomm IPQ9574 SoC Ethernet

[Andrew Lunn]

On Thu, Oct 03, 2024 at 02:07:10AM +0530, Kiran Kumar C.S.K wrote:
> Hello netdev,
> 
> We are planning to publish driver patches for adding Ethernet support
> for Qualcomm's IPQ9574 SoC, and looking for some advice on the approach
> to follow. There are two new drivers (described below) split across four
> patch series, totaling to 40 patches. These two drivers depend on a
> couple of clock controller drivers which are currently in review with
> the community.
> 
> Support is currently being added only for IPQ9574 SoC. However the
> drivers are written for the Qualcomm PPE (packet process engine)
> architecture, and are easily extendable for additional IPQ SoC (Ex:
> IPQ5332) that belong to the same network architecture family.
> 
> Given the number of patches for IPQ9574, we were wondering whether it is
> preferred to publish the four series together, since having all the code
> available could help clarify the inter-workings of the code. Or whether
> it is preferred to publish the patches sequentially, depending on the
> review progress?

Sequentially. You are likely to learn about working with mainline code
from the first patch series, which will allow you to improve the
following series before posting them.

>          +---------+
>          |  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  |
>            +----------------+--------------------+ +-------+ +-------+
> 
> 
> 1.1 PPE: Internal blocks overview
> =================================
> 
> The Switch core
> ---------------
> It has maximum 8 ports, comprising 6 GMAC ports and two DMA interfaces
> (for Ethernet DMA and EIP security processor) on the IPQ9574.

How are packets from the host directed to a specific egress port? Is
there bits in the DMA descriptor of the EDMA? Or is there an
additional header in the fields? This will determine if you are
writing a DSA switch driver, or a pure switchdev driver. 

> GMAC/xGMAC
> ----------
> There are 6 GMAC and 6 XGMAC in IPQ9574. Depending on the board ethernet
> configuration, either GMAC or XGMAC is selected by the PPE driver to
> interface with the PCS. The PPE driver initializes and manages these
> GMACs, and registers one netdevice per GMAC.

That suggests you are doing a pure switchdev driver.

> 2. List of patch series and dependencies
> ========================================
> 
> Clock drivers (currently in review)
> ===================================
> 1) CMN PLL driver patch series:
> 	Currently in review with community.
> 	https://lore.kernel.org/linux-arm-msm/20240827-qcom_ipq_cmnpll-v3-0-8e009cece8b2@xxxxxxxxxxx/
> 
> 
> 2) NSS clock controller (NSSCC) driver patch series
> 	Currently in review with community.
> 	https://lore.kernel.org/linux-arm-msm/20240626143302.810632-1-quic_devipriy@xxxxxxxxxxx/
> 
> 
> Networking drivers (to be posted for review next week)
> ======================================================
> 
> The following patch series are planned to be pushed for the PPE and PCS
> drivers, to support ethernet function. These patch series are listed
> below in dependency order.
> 
> 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?

This should be a good way to start, PCS drivers are typically nice and
simple.

	Andrew