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

[Ricky WU]

> -----Original Message-----
> From: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> Sent: Friday, December 3, 2021 10:17 AM
> To: Ricky WU <ricky_wu@xxxxxxxxxxx>
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; Arnd Bergmann <arnd@xxxxxxxx>;
> Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>; Yang Li
> <yang.lee@xxxxxxxxxxxxxxxxx>; open list <linux-kernel@xxxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> 
> 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.