On Thu, Feb 18, 2021 at 11:04:57PM +0800, Dejin Zheng wrote:
> Use the new function pcim_alloc_irq_vectors() to allocate IRQ vectors,
> the pcim_alloc_irq_vectors() function, an explicit device-managed version
> of pci_alloc_irq_vectors(). If pcim_enable_device() has been called
> before, then pci_alloc_irq_vectors() is actually a device-managed
> function. It is used here as a device-managed function, So replace it
> with pcim_alloc_irq_vectors(). At the same time, Remove the
> pci_free_irq_vectors() function to simplify the error handling path.
> the freeing resources will take automatically when device is gone.
Acked-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
One side note below.
> Signed-off-by: Dejin Zheng <zhengdejin5@xxxxxxxxx>
> ---
> v3 -> v4:
> - add some commit comments.
> v2 -> v3:
> - simplify the error handling path.
> v1 -> v2:
> - Modify some commit messages.
> drivers/i2c/busses/i2c-designware-pcidrv.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index 55c83a7a24f3..620b41e373b6 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -219,7 +219,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
> if (!dev)
> return -ENOMEM;
>
> - r = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
> + r = pcim_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
> if (r < 0)
> return r;
>
> @@ -234,10 +234,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
>
> if (controller->setup) {
> r = controller->setup(pdev, controller);
> - if (r) {
> - pci_free_irq_vectors(pdev);
> + if (r)
> return r;
> - }
> }
>
> i2c_dw_adjust_bus_speed(dev);
> @@ -246,10 +244,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
> i2c_dw_acpi_configure(&pdev->dev);
>
> r = i2c_dw_validate_speed(dev);
> - if (r) {
> - pci_free_irq_vectors(pdev);
> + if (r)
> return r;
> - }
>
> i2c_dw_configure(dev);
>
> @@ -269,10 +265,8 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
> adap->nr = controller->bus_num;
>
> r = i2c_dw_probe(dev);
> - if (r) {
> - pci_free_irq_vectors(pdev);
> + if (r)
> return r;
> - }
>
> pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
> pm_runtime_use_autosuspend(&pdev->dev);
> @@ -292,7 +286,6 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev)
>
> i2c_del_adapter(&dev->adapter);
> devm_free_irq(&pdev->dev, dev->irq, dev);
> - pci_free_irq_vectors(pdev);
I'm wondering if we need explicit devm_free_irq() call now. If we don't, it can
be cleaned up in the separate change with a motivation that we got to device
managed PCI IRQ allocation and thus the ordering will be correct (needs a
double check!).
> }
>
> /* work with hotplug and coldplug */
> --
> 2.25.0
>
--
With Best Regards,
Andy Shevchenko
