stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] i2c: designware-pci: Fix BUG_ON during device removal
@ 2020-02-13 15:15 Jarkko Nikula
  2020-02-13 16:18 ` Wolfram Sang
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jarkko Nikula @ 2020-02-13 15:15 UTC (permalink / raw)
  To: linux-i2c
  Cc: Wolfram Sang, Andy Shevchenko, Mika Westerberg, Jarkko Nikula, stable

Function i2c_dw_pci_remove() -> pci_free_irq_vectors() ->
pci_disable_msi() -> free_msi_irqs() will throw a BUG_ON() for MSI
enabled device since the driver has not released the requested IRQ before
calling the pci_free_irq_vectors().

Here driver requests an IRQ using devm_request_irq() but automatic
release happens only after remove callback. Fix this by explicitly
freeing the IRQ before calling pci_free_irq_vectors().

Fixes: 21aa3983d619 ("i2c: designware-pci: Switch over to MSI interrupts")
Cc: stable@vger.kernel.org # v5.4+
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 050adda7c1bd..05b35ac33ce3 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -313,6 +313,7 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev)
 	pm_runtime_get_noresume(&pdev->dev);
 
 	i2c_del_adapter(&dev->adapter);
+	devm_free_irq(&pdev->dev, dev->irq, dev);
 	pci_free_irq_vectors(pdev);
 }
 
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] i2c: designware-pci: Fix BUG_ON during device removal
  2020-02-13 15:15 [PATCH] i2c: designware-pci: Fix BUG_ON during device removal Jarkko Nikula
@ 2020-02-13 16:18 ` Wolfram Sang
  2020-02-13 17:11   ` Andy Shevchenko
  2020-02-13 17:10 ` Andy Shevchenko
  2020-03-10  9:34 ` Wolfram Sang
  2 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2020-02-13 16:18 UTC (permalink / raw)
  To: Jarkko Nikula; +Cc: linux-i2c, Andy Shevchenko, Mika Westerberg, stable

[-- Attachment #1: Type: text/plain, Size: 574 bytes --]

On Thu, Feb 13, 2020 at 05:15:03PM +0200, Jarkko Nikula wrote:
> Function i2c_dw_pci_remove() -> pci_free_irq_vectors() ->
> pci_disable_msi() -> free_msi_irqs() will throw a BUG_ON() for MSI
> enabled device since the driver has not released the requested IRQ before
> calling the pci_free_irq_vectors().
> 
> Here driver requests an IRQ using devm_request_irq() but automatic
> release happens only after remove callback. Fix this by explicitly
> freeing the IRQ before calling pci_free_irq_vectors().

Does it make sense to keep devm for irq handling, then?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] i2c: designware-pci: Fix BUG_ON during device removal
  2020-02-13 15:15 [PATCH] i2c: designware-pci: Fix BUG_ON during device removal Jarkko Nikula
  2020-02-13 16:18 ` Wolfram Sang
@ 2020-02-13 17:10 ` Andy Shevchenko
  2020-03-10  9:34 ` Wolfram Sang
  2 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2020-02-13 17:10 UTC (permalink / raw)
  To: Jarkko Nikula; +Cc: linux-i2c, Wolfram Sang, Mika Westerberg, stable

On Thu, Feb 13, 2020 at 05:15:03PM +0200, Jarkko Nikula wrote:
> Function i2c_dw_pci_remove() -> pci_free_irq_vectors() ->
> pci_disable_msi() -> free_msi_irqs() will throw a BUG_ON() for MSI
> enabled device since the driver has not released the requested IRQ before
> calling the pci_free_irq_vectors().
> 
> Here driver requests an IRQ using devm_request_irq() but automatic
> release happens only after remove callback. Fix this by explicitly
> freeing the IRQ before calling pci_free_irq_vectors().
> 

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Fixes: 21aa3983d619 ("i2c: designware-pci: Switch over to MSI interrupts")
> Cc: stable@vger.kernel.org # v5.4+
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> ---
>  drivers/i2c/busses/i2c-designware-pcidrv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index 050adda7c1bd..05b35ac33ce3 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -313,6 +313,7 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev)
>  	pm_runtime_get_noresume(&pdev->dev);
>  
>  	i2c_del_adapter(&dev->adapter);
> +	devm_free_irq(&pdev->dev, dev->irq, dev);
>  	pci_free_irq_vectors(pdev);
>  }
>  
> -- 
> 2.25.0
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] i2c: designware-pci: Fix BUG_ON during device removal
  2020-02-13 16:18 ` Wolfram Sang
@ 2020-02-13 17:11   ` Andy Shevchenko
  2020-02-14  9:26     ` Jarkko Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2020-02-13 17:11 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Jarkko Nikula, linux-i2c, Mika Westerberg, stable

On Thu, Feb 13, 2020 at 05:18:54PM +0100, Wolfram Sang wrote:
> On Thu, Feb 13, 2020 at 05:15:03PM +0200, Jarkko Nikula wrote:
> > Function i2c_dw_pci_remove() -> pci_free_irq_vectors() ->
> > pci_disable_msi() -> free_msi_irqs() will throw a BUG_ON() for MSI
> > enabled device since the driver has not released the requested IRQ before
> > calling the pci_free_irq_vectors().
> > 
> > Here driver requests an IRQ using devm_request_irq() but automatic
> > release happens only after remove callback. Fix this by explicitly
> > freeing the IRQ before calling pci_free_irq_vectors().
> 
> Does it make sense to keep devm for irq handling, then?

Only for sake of better error handling in error path in the ->probe().

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] i2c: designware-pci: Fix BUG_ON during device removal
  2020-02-13 17:11   ` Andy Shevchenko
@ 2020-02-14  9:26     ` Jarkko Nikula
  0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Nikula @ 2020-02-14  9:26 UTC (permalink / raw)
  To: Andy Shevchenko, Wolfram Sang; +Cc: linux-i2c, Mika Westerberg, stable

On 2/13/20 7:11 PM, Andy Shevchenko wrote:
> On Thu, Feb 13, 2020 at 05:18:54PM +0100, Wolfram Sang wrote:
>> On Thu, Feb 13, 2020 at 05:15:03PM +0200, Jarkko Nikula wrote:
>>> Function i2c_dw_pci_remove() -> pci_free_irq_vectors() ->
>>> pci_disable_msi() -> free_msi_irqs() will throw a BUG_ON() for MSI
>>> enabled device since the driver has not released the requested IRQ before
>>> calling the pci_free_irq_vectors().
>>>
>>> Here driver requests an IRQ using devm_request_irq() but automatic
>>> release happens only after remove callback. Fix this by explicitly
>>> freeing the IRQ before calling pci_free_irq_vectors().
>>
>> Does it make sense to keep devm for irq handling, then?
> 
I thought about it and decided devm still saves one line of code in 
drivers/i2c/busses/i2c-designware-platdrv.c: dw_i2c_plat_remove().

> Only for sake of better error handling in error path in the ->probe().
> 
Yeah, that too.

-- 
Jarkko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] i2c: designware-pci: Fix BUG_ON during device removal
  2020-02-13 15:15 [PATCH] i2c: designware-pci: Fix BUG_ON during device removal Jarkko Nikula
  2020-02-13 16:18 ` Wolfram Sang
  2020-02-13 17:10 ` Andy Shevchenko
@ 2020-03-10  9:34 ` Wolfram Sang
  2 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2020-03-10  9:34 UTC (permalink / raw)
  To: Jarkko Nikula; +Cc: linux-i2c, Andy Shevchenko, Mika Westerberg, stable

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

On Thu, Feb 13, 2020 at 05:15:03PM +0200, Jarkko Nikula wrote:
> Function i2c_dw_pci_remove() -> pci_free_irq_vectors() ->
> pci_disable_msi() -> free_msi_irqs() will throw a BUG_ON() for MSI
> enabled device since the driver has not released the requested IRQ before
> calling the pci_free_irq_vectors().
> 
> Here driver requests an IRQ using devm_request_irq() but automatic
> release happens only after remove callback. Fix this by explicitly
> freeing the IRQ before calling pci_free_irq_vectors().
> 
> Fixes: 21aa3983d619 ("i2c: designware-pci: Switch over to MSI interrupts")
> Cc: stable@vger.kernel.org # v5.4+
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-10  9:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 15:15 [PATCH] i2c: designware-pci: Fix BUG_ON during device removal Jarkko Nikula
2020-02-13 16:18 ` Wolfram Sang
2020-02-13 17:11   ` Andy Shevchenko
2020-02-14  9:26     ` Jarkko Nikula
2020-02-13 17:10 ` Andy Shevchenko
2020-03-10  9:34 ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).