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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing PCI IRQ vector allocation prevents MSI/MSI-X usage for high-speed child devices.
--

--- Patch [2]: [PATCH 2/2] misc: tc9564: introduce base PCI driver ---
commit e66bfe4041c21f48829a22eb54cde73756806870
Author: Alex Elder <[email protected]>
misc: tc9564: introduce base PCI driver

This commit introduces a base PCI driver for the Toshiba TC9564 SoC which
implements a PCIe switch and Ethernet AVB/TSN bridge. The driver binds to the
embedded PCI endpoint functions, performs basic PCI initialization, and uses
devicetree to populate any endpoint bus children.

[ ... ]

> diff --git a/drivers/misc/tc9564-pci.c b/drivers/misc/tc9564-pci.c
> new file mode 100644
> index 0000000000000..d3ffcac1c522f
> --- /dev/null
> +++ b/drivers/misc/tc9564-pci.c

[ ... ]

> +static int
> +tc9564_function_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np;
> +	int ret;
> +
> +	/* Despite being a PCI device, we require devicetree */
> +	np = dev_of_node(dev);
> +	if (!np)
> +		return dev_err_probe(dev, -EINVAL, "no devicetree node\n");
> +
> +	ret = pcim_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	pci_set_master(pdev);
> +
> +	/* Scan for pci-ep-bus nodes and probe their sub-devices */

[Severity: High]
Does this setup sequence omit PCI IRQ vector allocation? 

Without calling pci_alloc_irq_vectors() in tc9564_function_probe() before
populating the child devices, the Linux PCI core won't configure the device's
MSI/MSI-X capabilities with host addresses and data payloads. 

Since child platform drivers cannot safely allocate PCI IRQ vectors on behalf
of their parent PCI device, will the high-speed child devices (like the 10Gbps
Ethernet XGMACs) be forced to share a single legacy INTx interrupt, or fail
entirely if the host platform does not support legacy PCI interrupts?

> +	ret = of_platform_default_populate(np, NULL, dev);
> +	if (ret)
> +		goto err_clear_master;
> +
> +	return 0;
> +
> +err_clear_master:
> +	pci_clear_master(pdev);
> +
> +	return dev_err_probe(dev, ret, "failed to populate platform bus\n");
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2