linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] misc: rtsx: Avoid mangling IRQ during runtime PM
@ 2021-11-26  0:32 Kai-Heng Feng
  2021-12-03  2:17 ` Kai-Heng Feng
  0 siblings, 1 reply; 3+ messages in thread
From: Kai-Heng Feng @ 2021-11-26  0:32 UTC (permalink / raw)
  To: gregkh
  Cc: Kai-Heng Feng, Arnd Bergmann, Ricky Wu, Christophe JAILLET,
	Yang Li, open list

After commit 5b4258f6721f ("misc: rtsx: rts5249 support runtime PM"), when the
rtsx controller is runtime suspended, bring CPUs offline and back online, the
runtime resume of the controller will fail:

[   47.319391] smpboot: CPU 1 is now offline
[   47.414140] x86: Booting SMP configuration:
[   47.414147] smpboot: Booting Node 0 Processor 1 APIC 0x2
[   47.571334] smpboot: CPU 2 is now offline
[   47.686055] smpboot: Booting Node 0 Processor 2 APIC 0x4
[   47.808174] smpboot: CPU 3 is now offline
[   47.878146] smpboot: Booting Node 0 Processor 3 APIC 0x6
[   48.003679] smpboot: CPU 4 is now offline
[   48.086187] smpboot: Booting Node 0 Processor 4 APIC 0x1
[   48.239627] smpboot: CPU 5 is now offline
[   48.326059] smpboot: Booting Node 0 Processor 5 APIC 0x3
[   48.472193] smpboot: CPU 6 is now offline
[   48.574181] smpboot: Booting Node 0 Processor 6 APIC 0x5
[   48.743375] smpboot: CPU 7 is now offline
[   48.838047] smpboot: Booting Node 0 Processor 7 APIC 0x7
[   48.965447] __common_interrupt: 1.35 No irq handler for vector
[   51.174065] mmc0: error -110 doing runtime resume
[   54.978088] I/O error, dev mmcblk0, sector 21479 op 0x1:(WRITE) flags 0x0 phys_seg 11 prio class 0
[   54.978108] Buffer I/O error on dev mmcblk0p1, logical block 19431, lost async page write
[   54.978129] Buffer I/O error on dev mmcblk0p1, logical block 19432, lost async page write
[   54.978134] Buffer I/O error on dev mmcblk0p1, logical block 19433, lost async page write
[   54.978137] Buffer I/O error on dev mmcblk0p1, logical block 19434, lost async page write
[   54.978141] Buffer I/O error on dev mmcblk0p1, logical block 19435, lost async page write
[   54.978145] Buffer I/O error on dev mmcblk0p1, logical block 19436, lost async page write
[   54.978148] Buffer I/O error on dev mmcblk0p1, logical block 19437, lost async page write
[   54.978152] Buffer I/O error on dev mmcblk0p1, logical block 19438, lost async page write
[   54.978155] Buffer I/O error on dev mmcblk0p1, logical block 19439, lost async page write
[   54.978160] Buffer I/O error on dev mmcblk0p1, logical block 19440, lost async page write
[   54.978244] mmc0: card aaaa removed
[   54.978452] FAT-fs (mmcblk0p1): FAT read failed (blocknr 4257)

There's interrupt immediately raised on rtsx_pci_write_register() in
runtime resume routine, but the IRQ handler hasn't registered yet.

So we can either move rtsx_pci_write_register() after rtsx_pci_acquire_irq(),
or just stop mangling IRQ on runtime PM. Choose the latter to save some
CPU cycles.

BugLink: https://bugs.launchpad.net/bugs/1951784
Fixes: 5b4258f6721f ("misc: rtsx: rts5249 support runtime PM")
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/misc/cardreader/rtsx_pcr.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c
index 8c72eb590f79d..6ac509c1821c9 100644
--- a/drivers/misc/cardreader/rtsx_pcr.c
+++ b/drivers/misc/cardreader/rtsx_pcr.c
@@ -1803,8 +1803,6 @@ static int rtsx_pci_runtime_suspend(struct device *device)
 	mutex_lock(&pcr->pcr_mutex);
 	rtsx_pci_power_off(pcr, HOST_ENTER_S3);
 
-	free_irq(pcr->irq, (void *)pcr);
-
 	mutex_unlock(&pcr->pcr_mutex);
 
 	pcr->is_runtime_suspended = true;
@@ -1825,8 +1823,6 @@ static int rtsx_pci_runtime_resume(struct device *device)
 	mutex_lock(&pcr->pcr_mutex);
 
 	rtsx_pci_write_register(pcr, HOST_SLEEP_STATE, 0x03, 0x00);
-	rtsx_pci_acquire_irq(pcr);
-	synchronize_irq(pcr->irq);
 
 	if (pcr->ops->fetch_vendor_settings)
 		pcr->ops->fetch_vendor_settings(pcr);
-- 
2.32.0


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

* Re: [PATCH] misc: rtsx: Avoid mangling IRQ during runtime PM
  2021-11-26  0:32 [PATCH] misc: rtsx: Avoid mangling IRQ during runtime PM Kai-Heng Feng
@ 2021-12-03  2:17 ` Kai-Heng Feng
  2021-12-03  9:13   ` Ricky WU
  0 siblings, 1 reply; 3+ messages in thread
From: Kai-Heng Feng @ 2021-12-03  2:17 UTC (permalink / raw)
  To: Ricky Wu; +Cc: gregkh, Arnd Bergmann, Christophe JAILLET, Yang Li, open list

Hi Rick,

On Fri, Nov 26, 2021 at 8:33 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> After commit 5b4258f6721f ("misc: rtsx: rts5249 support runtime PM"), when the
> rtsx controller is runtime suspended, bring CPUs offline and back online, the
> runtime resume of the controller will fail:
>
> [   47.319391] smpboot: CPU 1 is now offline
> [   47.414140] x86: Booting SMP configuration:
> [   47.414147] smpboot: Booting Node 0 Processor 1 APIC 0x2
> [   47.571334] smpboot: CPU 2 is now offline
> [   47.686055] smpboot: Booting Node 0 Processor 2 APIC 0x4
> [   47.808174] smpboot: CPU 3 is now offline
> [   47.878146] smpboot: Booting Node 0 Processor 3 APIC 0x6
> [   48.003679] smpboot: CPU 4 is now offline
> [   48.086187] smpboot: Booting Node 0 Processor 4 APIC 0x1
> [   48.239627] smpboot: CPU 5 is now offline
> [   48.326059] smpboot: Booting Node 0 Processor 5 APIC 0x3
> [   48.472193] smpboot: CPU 6 is now offline
> [   48.574181] smpboot: Booting Node 0 Processor 6 APIC 0x5
> [   48.743375] smpboot: CPU 7 is now offline
> [   48.838047] smpboot: Booting Node 0 Processor 7 APIC 0x7
> [   48.965447] __common_interrupt: 1.35 No irq handler for vector
> [   51.174065] mmc0: error -110 doing runtime resume
> [   54.978088] I/O error, dev mmcblk0, sector 21479 op 0x1:(WRITE) flags 0x0 phys_seg 11 prio class 0
> [   54.978108] Buffer I/O error on dev mmcblk0p1, logical block 19431, lost async page write
> [   54.978129] Buffer I/O error on dev mmcblk0p1, logical block 19432, lost async page write
> [   54.978134] Buffer I/O error on dev mmcblk0p1, logical block 19433, lost async page write
> [   54.978137] Buffer I/O error on dev mmcblk0p1, logical block 19434, lost async page write
> [   54.978141] Buffer I/O error on dev mmcblk0p1, logical block 19435, lost async page write
> [   54.978145] Buffer I/O error on dev mmcblk0p1, logical block 19436, lost async page write
> [   54.978148] Buffer I/O error on dev mmcblk0p1, logical block 19437, lost async page write
> [   54.978152] Buffer I/O error on dev mmcblk0p1, logical block 19438, lost async page write
> [   54.978155] Buffer I/O error on dev mmcblk0p1, logical block 19439, lost async page write
> [   54.978160] Buffer I/O error on dev mmcblk0p1, logical block 19440, lost async page write
> [   54.978244] mmc0: card aaaa removed
> [   54.978452] FAT-fs (mmcblk0p1): FAT read failed (blocknr 4257)
>
> There's interrupt immediately raised on rtsx_pci_write_register() in
> runtime resume routine, but the IRQ handler hasn't registered yet.
>
> So we can either move rtsx_pci_write_register() after rtsx_pci_acquire_irq(),
> or just stop mangling IRQ on runtime PM. Choose the latter to save some
> CPU cycles.
>
> BugLink: https://bugs.launchpad.net/bugs/1951784
> Fixes: 5b4258f6721f ("misc: rtsx: rts5249 support runtime PM")
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

May I get an Ack from you?

Kai-Heng

> ---
>  drivers/misc/cardreader/rtsx_pcr.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c
> index 8c72eb590f79d..6ac509c1821c9 100644
> --- a/drivers/misc/cardreader/rtsx_pcr.c
> +++ b/drivers/misc/cardreader/rtsx_pcr.c
> @@ -1803,8 +1803,6 @@ static int rtsx_pci_runtime_suspend(struct device *device)
>         mutex_lock(&pcr->pcr_mutex);
>         rtsx_pci_power_off(pcr, HOST_ENTER_S3);
>
> -       free_irq(pcr->irq, (void *)pcr);
> -
>         mutex_unlock(&pcr->pcr_mutex);
>
>         pcr->is_runtime_suspended = true;
> @@ -1825,8 +1823,6 @@ static int rtsx_pci_runtime_resume(struct device *device)
>         mutex_lock(&pcr->pcr_mutex);
>
>         rtsx_pci_write_register(pcr, HOST_SLEEP_STATE, 0x03, 0x00);
> -       rtsx_pci_acquire_irq(pcr);
> -       synchronize_irq(pcr->irq);
>
>         if (pcr->ops->fetch_vendor_settings)
>                 pcr->ops->fetch_vendor_settings(pcr);
> --
> 2.32.0
>

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

* RE: [PATCH] misc: rtsx: Avoid mangling IRQ during runtime PM
  2021-12-03  2:17 ` Kai-Heng Feng
@ 2021-12-03  9:13   ` Ricky WU
  0 siblings, 0 replies; 3+ messages in thread
From: Ricky WU @ 2021-12-03  9:13 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: gregkh, Arnd Bergmann, Christophe JAILLET, Yang Li, open list

> -----Original Message-----
> From: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Sent: Friday, December 3, 2021 10:17 AM
> To: Ricky WU <ricky_wu@realtek.com>
> Cc: gregkh@linuxfoundation.org; Arnd Bergmann <arnd@arndb.de>;
> Christophe JAILLET <christophe.jaillet@wanadoo.fr>; Yang Li
> <yang.lee@linux.alibaba.com>; open list <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH] misc: rtsx: Avoid mangling IRQ during runtime PM
> 
> Hi Rick,
> 
> On Fri, Nov 26, 2021 at 8:33 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
> >
> > After commit 5b4258f6721f ("misc: rtsx: rts5249 support runtime PM"),
> > when the rtsx controller is runtime suspended, bring CPUs offline and
> > back online, the runtime resume of the controller will fail:
> >
> > [   47.319391] smpboot: CPU 1 is now offline
> > [   47.414140] x86: Booting SMP configuration:
> > [   47.414147] smpboot: Booting Node 0 Processor 1 APIC 0x2
> > [   47.571334] smpboot: CPU 2 is now offline
> > [   47.686055] smpboot: Booting Node 0 Processor 2 APIC 0x4
> > [   47.808174] smpboot: CPU 3 is now offline
> > [   47.878146] smpboot: Booting Node 0 Processor 3 APIC 0x6
> > [   48.003679] smpboot: CPU 4 is now offline
> > [   48.086187] smpboot: Booting Node 0 Processor 4 APIC 0x1
> > [   48.239627] smpboot: CPU 5 is now offline
> > [   48.326059] smpboot: Booting Node 0 Processor 5 APIC 0x3
> > [   48.472193] smpboot: CPU 6 is now offline
> > [   48.574181] smpboot: Booting Node 0 Processor 6 APIC 0x5
> > [   48.743375] smpboot: CPU 7 is now offline
> > [   48.838047] smpboot: Booting Node 0 Processor 7 APIC 0x7
> > [   48.965447] __common_interrupt: 1.35 No irq handler for vector
> > [   51.174065] mmc0: error -110 doing runtime resume
> > [   54.978088] I/O error, dev mmcblk0, sector 21479 op 0x1:(WRITE) flags
> 0x0 phys_seg 11 prio class 0
> > [   54.978108] Buffer I/O error on dev mmcblk0p1, logical block 19431, lost
> async page write
> > [   54.978129] Buffer I/O error on dev mmcblk0p1, logical block 19432, lost
> async page write
> > [   54.978134] Buffer I/O error on dev mmcblk0p1, logical block 19433, lost
> async page write
> > [   54.978137] Buffer I/O error on dev mmcblk0p1, logical block 19434, lost
> async page write
> > [   54.978141] Buffer I/O error on dev mmcblk0p1, logical block 19435, lost
> async page write
> > [   54.978145] Buffer I/O error on dev mmcblk0p1, logical block 19436, lost
> async page write
> > [   54.978148] Buffer I/O error on dev mmcblk0p1, logical block 19437, lost
> async page write
> > [   54.978152] Buffer I/O error on dev mmcblk0p1, logical block 19438, lost
> async page write
> > [   54.978155] Buffer I/O error on dev mmcblk0p1, logical block 19439, lost
> async page write
> > [   54.978160] Buffer I/O error on dev mmcblk0p1, logical block 19440, lost
> async page write
> > [   54.978244] mmc0: card aaaa removed
> > [   54.978452] FAT-fs (mmcblk0p1): FAT read failed (blocknr 4257)
> >
> > There's interrupt immediately raised on rtsx_pci_write_register() in
> > runtime resume routine, but the IRQ handler hasn't registered yet.
> >
> > So we can either move rtsx_pci_write_register() after
> > rtsx_pci_acquire_irq(), or just stop mangling IRQ on runtime PM.
> > Choose the latter to save some CPU cycles.
> >
> > BugLink: https://bugs.launchpad.net/bugs/1951784
> > Fixes: 5b4258f6721f ("misc: rtsx: rts5249 support runtime PM")
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> 
> May I get an Ack from you?
> 

We didn't meet this error log before, but the code maybe cause this   
If runtime_suspend() don't need to free irq, I think this change is fine...
And we tried this patch, work fine between runtime_suspend/resume 

> Kai-Heng
> 
> > ---
> >  drivers/misc/cardreader/rtsx_pcr.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/misc/cardreader/rtsx_pcr.c
> > b/drivers/misc/cardreader/rtsx_pcr.c
> > index 8c72eb590f79d..6ac509c1821c9 100644
> > --- a/drivers/misc/cardreader/rtsx_pcr.c
> > +++ b/drivers/misc/cardreader/rtsx_pcr.c
> > @@ -1803,8 +1803,6 @@ static int rtsx_pci_runtime_suspend(struct device
> *device)
> >         mutex_lock(&pcr->pcr_mutex);
> >         rtsx_pci_power_off(pcr, HOST_ENTER_S3);
> >
> > -       free_irq(pcr->irq, (void *)pcr);
> > -
> >         mutex_unlock(&pcr->pcr_mutex);
> >
> >         pcr->is_runtime_suspended = true; @@ -1825,8 +1823,6 @@
> static
> > int rtsx_pci_runtime_resume(struct device *device)
> >         mutex_lock(&pcr->pcr_mutex);
> >
> >         rtsx_pci_write_register(pcr, HOST_SLEEP_STATE, 0x03, 0x00);
> > -       rtsx_pci_acquire_irq(pcr);
> > -       synchronize_irq(pcr->irq);
> >
> >         if (pcr->ops->fetch_vendor_settings)
> >                 pcr->ops->fetch_vendor_settings(pcr);
> > --
> > 2.32.0
> >
> ------Please consider the environment before printing this e-mail.

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

end of thread, other threads:[~2021-12-03  9:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-26  0:32 [PATCH] misc: rtsx: Avoid mangling IRQ during runtime PM Kai-Heng Feng
2021-12-03  2:17 ` Kai-Heng Feng
2021-12-03  9:13   ` Ricky WU

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).