Re: [patch V4 06/14] PCI/MSI: Set pci_dev::msi_enabled late

Bjorn Helgaas
2025-03-19T17:08:51Z

On Wed, Mar 19, 2025 at 11:56:49AM +0100, Thomas Gleixner wrote:
> The comment claiming that pci_dev::msi_enabled has to be set across setup
> is a leftover from ancient code versions. Nothing in the setup code
> requires the flag to be set anymore.
>
> Set it in the success path and remove the extra goto label.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
> V4: New patch
> ---
> drivers/pci/msi/msi.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
> @@ -359,12 +359,8 @@ static int msi_capability_init(struct pc
> if (nvec > 1 && !pci_msi_domain_supports(dev, MSI_FLAG_MULTI_PCI_MSI, ALLOW_LEGACY))
> return 1;
>
> - /*
> - * Disable MSI during setup in the hardware, but mark it enabled
> - * so that setup code can evaluate it.
> - */
> + /* Disable MSI during setup in the hardware to erase stale state */
> pci_msi_set_enable(dev, 0);
> - dev->msi_enabled = 1;
>
> if (affd)
> masks = irq_create_affinity_masks(nvec, affd);
> @@ -372,7 +368,7 @@ static int msi_capability_init(struct pc
> msi_lock_descs(&dev->dev);
> ret = msi_setup_msi_desc(dev, nvec, masks);
> if (ret)
> - goto fail;
> + goto unlock;
>
> /* All MSIs are unmasked by default; mask them all */
> entry = msi_first_desc(&dev->dev, MSI_DESC_ALL);
> @@ -394,6 +390,7 @@ static int msi_capability_init(struct pc
> goto err;
>
> /* Set MSI enabled bits */
> + dev->msi_enabled = 1;
> pci_intx_for_msi(dev, 0);
> pci_msi_set_enable(dev, 1);
>
> @@ -404,8 +401,6 @@ static int msi_capability_init(struct pc
> err:
> pci_msi_unmask(&desc, msi_multi_mask(&desc));
> pci_free_msi_irqs(dev);
> -fail:
> - dev->msi_enabled = 0;
> unlock:
> msi_unlock_descs(&dev->dev);
> kfree(masks);
>