| Message ID | 20231110182304.3894319-5-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | i2c: designware: code consolidation & cleanups | expand |
On 11/10/23 20:11, Andy Shevchenko wrote: > PCI core, after pcim_enable_device(), takes care about the allocated > IRQ vectors, no need to do it explicitly and break the cleaning up > order. > > Fixes: 21aa3983d619 ("i2c: designware-pci: Switch over to MSI interrupts") > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/i2c/busses/i2c-designware-pcidrv.c | 15 +++------------ > 1 file changed, 3 insertions(+), 12 deletions(-) ... > @@ -360,9 +354,6 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev) > > pm_runtime_forbid(&pdev->dev); > pm_runtime_get_noresume(&pdev->dev); > - > - devm_free_irq(&pdev->dev, dev->irq, dev); > - pci_free_irq_vectors(pdev); Instead of commit 21aa3983d619 I guess this patch is more around my commit 9be8bc4dd617 ("i2c: designware-pci: Fix BUG_ON during device removal")? But still I don't think it qualifies Fixes tag? I was curious to see would this patch bring back above BUG_ON and fortunately not (rmmod/modprobe i2c_designware_pci on MSI enabled instance):-) Jarkko Nikula <jarkko.nikula@linux.intel.com>
On Wed, Nov 15, 2023 at 02:08:10PM +0200, Jarkko Nikula wrote: > On 11/10/23 20:11, Andy Shevchenko wrote: ... > > @@ -360,9 +354,6 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev) > > pm_runtime_forbid(&pdev->dev); > > pm_runtime_get_noresume(&pdev->dev); > > - > > - devm_free_irq(&pdev->dev, dev->irq, dev); > > - pci_free_irq_vectors(pdev); > > Instead of commit 21aa3983d619 I guess this patch is more around my commit > 9be8bc4dd617 ("i2c: designware-pci: Fix BUG_ON during device removal")? No, this one exactly what it says. Your patch is the side effect of the above mentioned change. > But > still I don't think it qualifies Fixes tag? Up to you, I can remove the fixes tag altogether, but it has the correct commit ID there. > I was curious to see would this patch bring back above BUG_ON and > fortunately not (rmmod/modprobe i2c_designware_pci on MSI enabled > instance):-) How? PCI clean up will happen on the level of pcim_enable_device(), i.e. quite after the IRQ will be freed.
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c index 35d35d7c8e23..175aa18f1a04 100644 --- a/drivers/i2c/busses/i2c-designware-pcidrv.c +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c @@ -295,10 +295,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); @@ -307,10 +305,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); @@ -330,10 +326,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; - } if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU) { dev->slave = i2c_new_ccgx_ucsi(&dev->adapter, dev->irq, &dgpu_node); @@ -360,9 +354,6 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev) pm_runtime_forbid(&pdev->dev); pm_runtime_get_noresume(&pdev->dev); - - devm_free_irq(&pdev->dev, dev->irq, dev); - pci_free_irq_vectors(pdev); } static const struct pci_device_id i2_designware_pci_ids[] = {
PCI core, after pcim_enable_device(), takes care about the allocated IRQ vectors, no need to do it explicitly and break the cleaning up order. Fixes: 21aa3983d619 ("i2c: designware-pci: Switch over to MSI interrupts") Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/i2c/busses/i2c-designware-pcidrv.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-)