2021-11-26 00:35:41

by Kai-Heng Feng

[permalink] [raw]
Subject: [PATCH] misc: rtsx: Avoid mangling IRQ during runtime PM

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 <[email protected]>
---
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



2021-12-03 02:17:40

by Kai-Heng Feng

[permalink] [raw]
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
<[email protected]> 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 <[email protected]>

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
>

2021-12-03 09:14:10

by Ricky Wu

[permalink] [raw]
Subject: RE: [PATCH] misc: rtsx: Avoid mangling IRQ during runtime PM

> -----Original Message-----
> From: Kai-Heng Feng <[email protected]>
> Sent: Friday, December 3, 2021 10:17 AM
> To: Ricky WU <[email protected]>
> Cc: [email protected]; Arnd Bergmann <[email protected]>;
> Christophe JAILLET <[email protected]>; Yang Li
> <[email protected]>; open list <[email protected]>
> 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
> <[email protected]> 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 <[email protected]>
>
> 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.