Re: [PATCH 2/2] misc: tc9564: introduce base PCI driver
Alex Elder <[email protected]>
| Newsgroups | org.kernel.vger.linux-pci,dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
On 8/13/26 8:59 PM, [email protected] wrote: > 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. I will investigate this. I don't have a good answer at the moment. -Alex > 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"); >> +} > > [ ... ] >