Re: [PATCH net-next v8 14/15] misc: lan966x-pci: dts: extend cpu reg to cover PCIE DBI space

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Concern: nothing enforces the contract between the DT "cpu" reg
  size and the driver's hard-coded TARGET_PCIE_DBI offset (0x400000)…

--

misc: lan966x-pci: dts: extend cpu reg to cover PCIE DBI space

This patch grows the "cpu" reg of the switch node in the lan966x PCI
overlay from 1MB to 8MB. The FDMA engine programs its ATU outbound windows
through registers at offset 0x400000 and above, and the old mapping did
not reach them.

> diff --git a/drivers/misc/lan966x_pci.dtso b/drivers/misc/lan966x_pci.dtso
> index 7b196b0a0eb65..7bb726550cafd 100644
> --- a/drivers/misc/lan966x_pci.dtso
> +++ b/drivers/misc/lan966x_pci.dtso
> @@ -135,7 +135,7 @@ lan966x_phy1: ethernet-lan966x_phy@2 {
>  
>  				switch: switch@e0000000 {
>  					compatible = "microchip,lan966x-switch";
> -					reg = <0xe0000000 0x0100000>,
> +					reg = <0xe0000000 0x0800000>,

[Severity: Low]
This isn't a bug, but the driver only works if the size here is large
enough, and nothing in the driver checks that.

The driver computes the DBI target from a fixed offset into the "cpu"
resource:

drivers/net/ethernet/microchip/lan966x/lan966x_main.c:
	{ TARGET_PCIE_DBI,             0x400000, 0 }, /* 0xe0400000 */

lan966x_create_targets() then adds that offset to the ioremapped base
without comparing it to resource_size():

	lan966x->regs[iomap->id] = begin[iomap->range] + iomap->offset;

fdma_pci_atu_addr_get() in drivers/net/ethernet/microchip/fdma/fdma_pci.c
adds a further FDMA_PCI_ATU_ADDR (0x300000) + idx * 0x200 + offset:

	return addr + FDMA_PCI_ATU_ADDR + FDMA_PCI_ATU_IDX_SIZE * idx + offset;

With this patch, the in-tree overlay covers the highest register touched
(about 0x700A18 into the cpu range). The native lan966x.dtsi still uses a
1MB cpu reg, but it never reaches this path, because lan966x_is_pci() is
false there and lan966x_shutdown() returns early.

So there is no in-tree problem. Would it still make sense to check in
lan966x_create_targets() that TARGET_PCIE_DBI plus the ATU range fits
within the "cpu" resource on PCI, rather than depending on the DT size
alone?

>  					      <0xe2000000 0x0800000>;
>  					reg-names = "cpu", "gcb";
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260924-lan966x-pci-fdma-v8-0-201c7b707d8b%40microchip.com