Re: [PATCH v4 2/3] misc: tc9564: introduce base PCI driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] pci: of_pci_make_dev_node quirk fails to update ranges of existing DT node
- [Medium] kconfig: TC9564_PCI selects PCI_DYNAMIC_OF_NODES without inheriting OF_IRQ dependency
--

commit d693efad7a099f6326c4b836ade779f0f07366be
Author: Alex Elder <elder@riscstar.com>

misc: tc9564: introduce base PCI driver

The Toshiba TC9564 is small and highly-specialized SoC that implements
a PCIe switch as well as an Ethernet AVB/TSN bridge. Create a new PCI
driver under drivers/misc that binds with the embedded PCI endpoint
functions within the TC9564 SoC. Use the PCI quirks mechanism to update
endpoint device nodes dynamically.

> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 7364931dad3a1..9c5b1e9d672e8 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig

[ ... ]

> @@ -568,6 +568,21 @@ config MCHP_LAN966X_PCI
>  	    - lan966x-miim (MDIO_MSCC_MIIM)
>  	    - lan966x-switch (LAN966X_SWITCH)
>  
> +config TC9564_PCI
> +	tristate "Toshiba TC9564 PCI function support"
> +	depends on OF
> +	depends on PCI
> +	select PCI_DYNAMIC_OF_NODES

[Severity: Medium]
Can this cause a Kconfig unmet direct dependency warning?

PCI_DYNAMIC_OF_NODES explicitly defines 'depends on OF_IRQ' in
drivers/pci/Kconfig. Selecting it here without inheriting its dependencies
could cause unmet direct dependency warnings and potential build failures on
architectures lacking OF_IRQ support.

> +	select PCI_QUIRKS
> +	default m
> +	help

[ ... ]

> diff --git a/drivers/misc/tc9564-pci.c b/drivers/misc/tc9564-pci.c
> index 0000000000000..e4d00dfecfca9 100644
> --- /dev/null
> +++ b/drivers/misc/tc9564-pci.c

[ ... ]

> +	/* Scan for pci-ep-bus (simple-bus) nodes and probe their sub-devices */
> +	ret = of_platform_default_populate(np, NULL, dev);

[ ... ]

> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index de9bbccda21fd..54e310c4153df 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -6391,6 +6391,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5020, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_XILINX, 0x5021, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_REDHAT, 0x0005, of_pci_make_dev_node);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_EFAR, 0x9660, of_pci_make_dev_node);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_TOSHIBA, 0x0220, of_pci_make_dev_node);

[Severity: High]
Will this quirk successfully update the ranges property of the existing
devicetree node?

Because the tc9564 device has a static device tree node, pci_set_of_node()
attaches the static node to pdev->dev.of_node before this fixup runs.

Inside of_pci_make_dev_node(), the early check for an existing node:

    if (pci_device_to_OF_node(pdev))
        return;

evaluates to true, causing it to return immediately. Consequently,
of_pci_prop_ranges() is never executed.

This means endpoint platform devices populated by of_platform_default_populate()
in tc9564_function_probe() will fail to map their MMIO addresses correctly,
translating addresses using the placeholder static 'ranges' property rather
than dynamically assigned PCI BAR addresses.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918152659.413268-1-elder@riscstar.com?part=2