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

[Jakub Kicinski <kuba@kernel.org>]

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?

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

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

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

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