Message-ID: <c7d8109d-8f88-4f4c-abb7-6ebfa1f1daa3@quicinc.com>
Date: Thu, 3 Oct 2024 23:20:03 +0530
From: Kiran Kumar C.S.K <quic_kkumarcs@...cinc.com>
To: Andrew Lunn <andrew@...n.ch>
CC: <netdev@...r.kernel.org>, Andy Gross <agross@...nel.org>,
        Bjorn Andersson
	<andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        "David S.
 Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        "Jakub
 Kicinski" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
        Rob Herring
	<robh+dt@...nel.org>,
        Krzysztof Kozlowski
	<krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Russell King <linux@...linux.org.uk>,
        Jacob Keller <jacob.e.keller@...el.com>,
        Bhupesh Sharma
	<bhupesh.sharma@...aro.org>,
        <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <vsmuthu@....qualcomm.com>,
        <arastogi@....qualcomm.com>, <linchen@....qualcomm.com>,
        <john@...ozen.org>, Luo Jie
	<quic_luoj@...cinc.com>,
        Pavithra R <quic_pavir@...cinc.com>,
        "Suruchi
 Agarwal (QUIC)" <quic_suruchia@...cinc.com>,
        "Lei Wei (QUIC)"
	<quic_leiwei@...cinc.com>
Subject: Re: RFC: Advice on adding support for Qualcomm IPQ9574 SoC Ethernet



On 10/3/2024 2:58 AM, Andrew Lunn wrote:
> 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.
> 

OK, thanks.

>>          +---------+
>>          |  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. 

The DMA descriptor carries the information on the destination port.
There is no additional header required in the packet.

> 
>> 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.
> 

Agree that switchdev is the right model for this device. We were
planning to enable base Ethernet functionality using regular
(non-switchdev) netdevice representation for the ports initially,
without offload support. As the next step, L2/VLAN offload support using
switchdev will be enabled on top. Hope this phased approach is fine.

>> 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@quicinc.com/
>>
>>
>> 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@quicinc.com/
>>
>>
>> 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?

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.

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

Sure, thanks for the inputs.

> 	Andrew

Powered by blists - more mailing lists