Re: [PATCH net-next v3 0/3] net: stmmac: Add PCI driver support for BCM8958x

[Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>]

On Mon, Aug 5, 2024 at 3:43 PM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> On Fri, Aug 02, 2024 at 03:06:05PM -0700, Jitendra Vegiraju wrote:
> > On Fri, Aug 2, 2024 at 3:02 AM Serge Semin <fancer.lancer@gmail.com> wrote:
> > >
> > > Hi Jitendra
> > >
> > > On Thu, Aug 01, 2024 at 08:18:19PM -0700, jitendra.vegiraju@broadcom.com wrote:
> > > > From: Jitendra Vegiraju <jitendra.vegiraju@broadcom.com>
> > > >
> > > > This patchset adds basic PCI ethernet device driver support for Broadcom
> > > > BCM8958x Automotive Ethernet switch SoC devices.
> > > >
> > > > This SoC device has PCIe ethernet MAC attached to an integrated ethernet
> > > > switch using XGMII interface. The PCIe ethernet controller is presented to
> > > > the Linux host as PCI network device.
> > > >
> > > > The following block diagram gives an overview of the application.
> > > >              +=================================+
> > > >              |       Host CPU/Linux            |
> > > >              +=================================+
> > > >                         || PCIe
> > > >                         ||
> > > >         +==========================================+
> > > >         |           +--------------+               |
> > > >         |           | PCIE Endpoint|               |
> > > >         |           | Ethernet     |               |
> > > >         |           | Controller   |               |
> > > >         |           |   DMA        |               |
> > > >         |           +--------------+               |
> > > >         |           |   MAC        |   BCM8958X    |
> > > >         |           +--------------+   SoC         |
> > > >         |               || XGMII                   |
> > > >         |               ||                         |
> > > >         |           +--------------+               |
> > > >         |           | Ethernet     |               |
> > > >         |           | switch       |               |
> > > >         |           +--------------+               |
> > > >         |             || || || ||                  |
> > > >         +==========================================+
> > > >                       || || || || More external interfaces
> > > >
> > > > The MAC block on BCM8958x is based on Synopsis XGMAC 4.00a core. This
> > > > driver uses common dwxgmac2 code where applicable.
> > >
> > > Thanks for submitting the series.
> > >
> > > I am curious how come Broadcom got to use an IP-core which hasn't
> > > been even announced by Synopsys. AFAICS the most modern DW XGMAC
> > > IP-core is of v3.xxa version:
> > >
> > > https://www.synopsys.com/dw/ipdir.php?ds=dwc_ether_xgmac
> > >
>
> > I am not sure why the 4.00a IP-code is not announced for general
> > availability yet.
> > The Synopsis documentation for this IP mentions 3.xx IP as reference
> > for this design and lists
> > new features for 4.00a.
> >
> > > Are you sure that your device isn't equipped with some another DW MAC
> > > IP-core, like DW 25G Ethernet MAC? (which BTW is equipped with a new
> > > Hyper DMA engine with a capability to have up to 128/256 channels with
> > > likely indirect addressing.) Do I miss something?
> > >
> > Yes, I briefly mentioned the new DMA architecture in the commit log
> > for patch 1/3.
> > You are correct, the name for the new DMA engine is Hyper DMA. It
> > probably started with some 3.xx IP-Core.
> > This DW-MAC is capable of 25G, but this SOC is not using 25G support.
>
> Then what you have is likely the DW 25GMAC since just DW XGMAC hasn't
> been announced to have neither 25G speed nor the Hyper-DMA with the
> virtualization channels capabilities. Meanwhile the former IP-core
> does have these features:
> https://www.synopsys.com/dw/ipdir.php?ds=dwc_25g_ethernet_mac_ip
>
> Alas I don't have the DW 25GMAC IP-core databook to say for sure, but
> that's the only explanation of why you have the 0x40 Synopsys ID and
> the IP-core version of v4.00a, and the 25G capability of the MAC.
>
> Seeing Synopsys tends to re-use the CSRs mapping even across the major
> IP-core releases it isn't that surprising that the DW XGMAC 3.xx
> IP-core was referenced in the doc. (See the driver, DW XLGMAC is
> almost fully compatible with the DW XGMAC CSRs mapping.)
>
> Moreover the most DMA-capable device currently supported by the
> STMMAC-driver is DW XGMAC/XLGMAC and it can't have more than 16
> DMA-channels. That allows to directly map all the channels CSRs to the
> system memory. But your case is different. The DW 25GMAC IP-core is
> announced to support virtualization up to 128/256 channels, for which
> the direct CSRs mapping could require 16-times more memory. That's
> likely why the indirect addressing was implemented to access the
> settings of all the possible channels. That's also implicitly proofs
> that you have the DW 25GMAC IP-core.
Hi Serge(y)

Thanks for reviewing the patch series.
Sorry for the delay in my response. We waited for a clarification on
the IP version.
Its confirmed that we got an early adapter version of 25GMAC IP-Core.
Added more details in the context of other questions in Patch 1,2.
>
> -Serge(y)
>
> >
> > > * I'll join the patch set review after the weekend, sometime on the
> > > next week.
> > >
> > > -Serge(y)
> > >
> > > > Driver functionality specific to this MAC is implemented in dwxgmac4.c.
> > > > Management of integrated ethernet switch on this SoC is not handled by
> > > > the PCIe interface.
> > > > This SoC device has PCIe ethernet MAC directly attached to an integrated
> > > > ethernet switch using XGMII interface.
> > > >
> > > > v2->v3:
> > > >    Addressed v2 comments from Andrew, Jakub, Russel and Simon.
> > > >    Based on suggestion by Russel and Andrew, added software node to create
> > > >    phylink in fixed-link mode.
> > > >    Moved dwxgmac4 specific functions to new files dwxgmac4.c and dwxgmac4.h
> > > >    in stmmac core module.
> > > >    Reorganized the code to use the existing glue logic support for xgmac in
> > > >    hwif.c and override ops functions for dwxgmac4 specific functions.
> > > >    The patch is split into three parts.
> > > >      Patch#1 Adds dma_ops for dwxgmac4 in stmmac core
> > > >      Patch#2 Hooks in the hardware interface handling for dwxgmac4
> > > >      Patch#3 Adds PCI driver for BCM8958x device
> > > >
> > > > v1->v2:
> > > >    Minor fixes to address coding style issues.
> > > >    Sent v2 too soon by mistake, without waiting for review comments.
> > > >    Received feedback on this version.
> > > >    https://lore.kernel.org/netdev/20240511015924.41457-1-jitendra.vegiraju@broadcom.com/
> > > >
> > > > v1:
> > > >    https://lore.kernel.org/netdev/20240510000331.154486-1-jitendra.vegiraju@broadcom.com/
> > > >
> > > > Jitendra Vegiraju (3):
> > > >   Add basic dwxgmac4 support to stmmac core
> > > >   Integrate dwxgmac4 into stmmac hwif handling
> > > >   Add PCI driver support for BCM8958x
> > > >
> > > >  MAINTAINERS                                   |   8 +
> > > >  drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 +
> > > >  drivers/net/ethernet/stmicro/stmmac/Makefile  |   3 +-
> > > >  drivers/net/ethernet/stmicro/stmmac/common.h  |   4 +
> > > >  .../net/ethernet/stmicro/stmmac/dwmac-brcm.c  | 517 ++++++++++++++++++
> > > >  .../ethernet/stmicro/stmmac/dwxgmac2_dma.c    |  31 ++
> > > >  .../net/ethernet/stmicro/stmmac/dwxgmac4.c    | 142 +++++
> > > >  .../net/ethernet/stmicro/stmmac/dwxgmac4.h    |  84 +++
> > > >  drivers/net/ethernet/stmicro/stmmac/hwif.c    |  26 +-
> > > >  drivers/net/ethernet/stmicro/stmmac/hwif.h    |   1 +
> > > >  10 files changed, 825 insertions(+), 2 deletions(-)
> > > >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-brcm.c
> > > >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwxgmac4.c
> > > >  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwxgmac4.h
> > > >
> > > > --
> > > > 2.34.1
> > > >
> > > >