Re: [PATCH net-next v5 09/13] net: lan966x: add PCIe FDMA support

[Daniel Machon]

> On Wed, 20 May 2026 10:12:21 +0200 Daniel Machon wrote:
> > Add PCIe FDMA support for lan966x. The PCIe FDMA path uses contiguous
> > DMA buffers mapped through the endpoint's ATU, with memcpy-based frame
> > transfer instead of per-page DMA mappings.
> >
> > With PCIe FDMA, throughput increases from ~33 Mbps (register-based I/O)
> > to ~620 Mbps on an Intel x86 host with a lan966x PCIe card.
> 
> > diff --git a/drivers/net/ethernet/microchip/lan966x/Makefile b/drivers/net/ethernet/microchip/lan966x/Makefile
> > index 4cdbe263502c..ac0beceb2a0d 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/Makefile
> > +++ b/drivers/net/ethernet/microchip/lan966x/Makefile
> > @@ -18,6 +18,10 @@ lan966x-switch-objs  := lan966x_main.o lan966x_phylink.o lan966x_port.o \
> >  lan966x-switch-$(CONFIG_LAN966X_DCB) += lan966x_dcb.o
> >  lan966x-switch-$(CONFIG_DEBUG_FS) += lan966x_vcap_debugfs.o
> >
> > +ifdef CONFIG_MCHP_LAN966X_PCI
> 
> ifeq ()
> 
> would be more common in a Makefile?

Switching to ifneq ($(CONFIG_MCHP_LAN966X_PCI),) in v6.

> 
> > +lan966x-switch-y += lan966x_fdma_pci.o
> > +endif
> > +
> 
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > index cc3c7b6c65ae..7036b1d937d5 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > @@ -7,6 +7,7 @@
> >  #include <linux/ip.h>
> >  #include <linux/of.h>
> >  #include <linux/of_net.h>
> > +#include <linux/pci.h>
> >  #include <linux/phy/phy.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/reset.h>
> > @@ -49,6 +50,9 @@ struct lan966x_main_io_resource {
> >  static const struct lan966x_main_io_resource lan966x_main_iomap[] =  {
> >       { TARGET_CPU,                   0xc0000, 0 }, /* 0xe00c0000 */
> >       { TARGET_FDMA,                  0xc0400, 0 }, /* 0xe00c0400 */
> > +#if IS_ENABLED(CONFIG_MCHP_LAN966X_PCI)
> 
> why config option being enabled changes the targets?
> Can someone with a non-PCI device enable that option too
> (sure it would be useless but given that/if they can guarding
> with an #if seems like a waste of LoC)
> 

Rationale was to only map in the PCIE_DBI target if actually needed. Sure, can
drop the conditional in v6.

> > +     { TARGET_PCIE_DBI,             0x400000, 0 }, /* 0xe0400000 */
> > +#endif
> >       { TARGET_ORG,                         0, 1 }, /* 0xe2000000 */
> >       { TARGET_GCB,                    0x4000, 1 }, /* 0xe2004000 */
> >       { TARGET_QS,                     0x8000, 1 }, /* 0xe2008000 */
> > @@ -1100,6 +1104,13 @@ static int lan966x_reset_switch(struct lan966x *lan966x)
> >
> >  static const struct lan966x_fdma_ops *lan966x_get_fdma_ops(struct device *dev)
> >  {
> > +#if IS_ENABLED(CONFIG_MCHP_LAN966X_PCI)
> > +     for (struct device *p = dev->parent; p; p = p->parent) {
> > +             if (dev_is_pci(p))
> 
> If the PCIe devices also use an intermediate platform device for
> probing, can't they explicitly have some flag / state to indicate
> they are PCIe? The device walk in such a constrained env seems
> like an overkill
> 

I think it could be possible to pass some state between lan966x_pci and the
switch driver, however that seems like a much more complex solution than this.
The parent bus node already implies the same — the switch device only ever sits
under a PCI-created bus in the PCIe case, so an explicit flag/state would just
restate what the topology already tells us. The walk itself is deliberate, v1
checked dev->parent->parent directly, and I opened it up to a walk so PCIe
detection keeps working if the overlay's DT topology gains or loses an
intermediate bus node. I'd like to keep as it.

> > +                     return &lan966x_fdma_pci_ops;
> > +     }
> > +#endif
> > +
> >       return &lan966x_fdma_ops;
> >  }
> >
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > index 5f4dbeda17cd..e7fdd4447fb6 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > @@ -17,6 +17,9 @@
> >  #include <net/xdp.h>
> >
> >  #include <fdma_api.h>
> > +#if IS_ENABLED(CONFIG_MCHP_LAN966X_PCI)
> > +#include <fdma_pci.h>
> > +#endif
> 
> Conditional #includes make build testing harder, better to avoid them
> 

Ack.

> >  #include <vcap_api.h>
> >  #include <vcap_api_client.h>
> >
> > @@ -288,6 +291,10 @@ struct lan966x {
> >
> >       void __iomem *regs[NUM_TARGETS];
> >
> > +#if IS_ENABLED(CONFIG_MCHP_LAN966X_PCI)
> > +     struct fdma_pci_atu atu;
> > +#endif
> > +
> >       int shared_queue_sz;
> >
> >       u8 base_mac[ETH_ALEN];
> > @@ -586,6 +593,10 @@ void lan966x_fdma_wakeup_netdev(struct lan966x *lan966x);
> >  int lan966x_fdma_get_max_frame(struct lan966x *lan966x);
> >  int lan966x_qsys_sw_status(struct lan966x *lan966x);
> >
> > +#if IS_ENABLED(CONFIG_MCHP_LAN966X_PCI)
> > +extern const struct lan966x_fdma_ops lan966x_fdma_pci_ops;
> > +#endif
> 
> There should be no need to wrap extern in an #if

Ack.


/Daniel