Re: [PATCH v4 3/9] ipack: tpci200: don't store pci_device_id
[email protected] Fri, 24 Jul 2026 22:02:02 +0000
| Newsgroups | org.kernel.vger.linux-ide,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-pci,org.kernel.vger.linux-scsi |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: Pre-existing issues: - [High] MMIO configuration registers are mapped and accessed before callin= g pci_enable_device(), which can lead to dropped writes or master aborts. -- --- Patch [3]: [PATCH v4 3/9] ipack: tpci200: don't store pci_device_id --- commit eed0a798b7390c20259769af7ca3dfe410d112f0 Author: Gary Guo <[email protected]> Subject: ipack: tpci200: don't store pci_device_id This commit removes the stored pci_device_id from the tpci200_infos structu= re. Since dynamic IDs are not guaranteed to outlive the probe function, and the stored ID is unused, removing it prevents potential issues. > diff --git a/drivers/ipack/carriers/tpci200.c b/drivers/ipack/carriers/tp= ci200.c > index 05dcb6675cd66..1cf51f7632932 100644 > --- a/drivers/ipack/carriers/tpci200.c > +++ b/drivers/ipack/carriers/tpci200.c > @@ -562,7 +562,6 @@ static int tpci200_pci_probe(struct pci_dev *pdev, > =20 > /* Save struct pci_dev pointer */ > tpci200->info->pdev =3D pdev; > - tpci200->info->id_table =3D (struct pci_device_id *)id; > =20 > /* register the device and initialize it */ > ret =3D tpci200_install(tpci200); [Severity: High] This is a pre-existing issue, but I noticed that MMIO configuration registe= rs are mapped and accessed before pci_enable_device() is called. Earlier in tpci200_pci_probe(), the driver maps the configuration memory bar and immediately accesses the registers to configure byte swapping: reg32 =3D ioread32(tpci200->info->cfg_regs + LAS1_DESC); reg32 |=3D 1 << LAS_BIT_BIGENDIAN; iowrite32(reg32, tpci200->info->cfg_regs + LAS1_DESC); However, pci_enable_device() doesn't seem to be called until later in the probe sequence, when tpci200_install() eventually calls tpci200_register(): if (pci_enable_device(tpci200->info->pdev) < 0) return -ENODEV; Since the memory enable bit in the PCI command register is not guaranteed to be set by the firmware before probe, accessing MMIO regions before pci_enable_device() can result in reads returning 0xFFFFFFFF and writes bei= ng dropped. On some architectures, this might even trigger a system-crashing PCI master abort. Should pci_enable_device() be called earlier in tpci200_pci_probe() before any MMIO access occurs? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260723-pci_id_fix= [email protected]?part=3D3