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