Possible bug in drivers/misc/cardreader/rtsx_pcr.c

Possible bug in drivers/misc/cardreader/rtsx_pcr.c

[Klaus Doth]

Hi,


As per the info from kernelnewbies IRC, I'm sending this also to the PCI
list.


There seems to be a bug in the power saving functions of the drivers
forthe RTS5260 PCIe Card Reader. Reading a card basically works, however
the first command issued to the card after the chip goes into power
saving fails. Due to the command timeout of 10 seconds inside the code,
every new access takes 10 seconds before it is processed.

It applies to at least PCI ID 10ec:5260 (rev 01), in case there are
several revsions of the chip.

I tested this on Kernel 5.3.0, 5.6.2, on a default Ubuntu 19.10, and a
custom Gentoo installation. Both versions, Ubuntu using 5.3.0 kernel,
and Gentoo using Kernel 5.6.2 have the same issue on both several Dell
Precision 7530 and 7540 laptops.


How to reproduce:

1) Insert SD-Card

2) Wait for one second or more where there is no access to the card

3) dd if=/dev/mmblk0p1 of=/dev/null

4) The dd command will hang for 10 seconds without transfering a single
byte, and then resume. After resuming, full (correct) transfer speed is
reached, until repeating from 2). The card can stay inserted for
repeated reproduction of issue.



The issue in detail:

If the rtsx_pci_idle_work() function is called, most of the time (like
90% or more), the next call to rtsx_pci_dma_transfer (i.e. reading or
writing to the card) runs into its timeout and fails.

After that, the next command always succeeds and data transfer from and
to the card is resumed and works correctly, until the next
rtsx_pic_idle_work() call is issued. After that the same timeout happens
again.

It does not matter if reading or writing occurs using a file system or
direct access to the block device using dd or similar.

After some testing it looks like something how the driver or the chip
itself handle leaving the power saving mode is wrong.

If I modify the rtsx_comm_pm_full_on() function and add msleep(1);
directly after the rtsx_disable_aspm(pcr); call, leaving power save
always works, and the card does not fail on the first command after
power saving mode.

This is likely not the proper solution to the issue, however I hope it
might help the current developer in identifying the correct solution. It
looks like something needs a little more time to wake up before issuing
a new DMA access.


If I can help further in any way in debugging this issue, let me know.



Attached are the changed function in rtsx_pcr.c, and kernel logs with
and without the modification:


--------------------------------------------------------------------------

Changed funcion in rtsx_pcr.c (lines 139 - 151)

static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
{
        struct rtsx_cr_option *option = &pcr->option;

        rtsx_disable_aspm(pcr);
+      msleep(1);

        if (option->ltr_enabled)
                rtsx_set_ltr_latency(pcr, option->ltr_active_latency);

        if (rtsx_check_dev_flag(pcr, LTR_L1SS_PWR_GATE_EN))
                rtsx_set_l1off_sub_cfg_d0(pcr, 1);
}


--------------------------------------------------------------------------

Kernel 5.6.2 without modification:

Apr  5 21:38:25 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: pre dma sg: 4
Apr  5 21:38:25 - kernel: rtsx_pci 0000:6f:00.0: Switch card clock to 50MHz
Apr  5 21:38:25 - kernel: rtsx_pci 0000:6f:00.0: Internal SSC clock:
100MHz (cur_clock = 100)
Apr  5 21:38:25 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0:
sd_read_long_data: SD/MMC CMD 18, arg = 0x00002000
Apr  5 21:38:25 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xff054000,
Len: 0x1000
Apr  5 21:38:25 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xff055000,
Len: 0x1000
Apr  5 21:38:25 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xff056000,
Len: 0x1000
Apr  5 21:38:25 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xff057000,
Len: 0x1000
Apr  5 21:38:25 - kernel: rtsx_pci 0000:6f:00.0: --> rtsx_pci_idle_work
--- waiting for 10 seconds here
Apr  5 21:38:35 - kernel: rtsx_pci 0000:6f:00.0: Timeout
(rtsx_pci_dma_transfer 538)
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFDA0(8): 11
01 04 08 00 1f 00 08
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFDA8(8): 10
0c 00 00 0b 00 7f 00
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFDB0(4): 02
00 00 68 00 00 00 00
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFD52(8): 04
41 00 04 0f 00 f0 00
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFD5A(8): 94
1f 52 ff 94 bb 66 aa
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFD62(8): e9
aa aa aa 24 00 04 04
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0:
sd_send_cmd_get_rsp: SD/MMC CMD 12, arg = 0x00000000
Apr  5 21:38:35 - kernel: rtsx_pci 0000:6f:00.0: Timeout
(rtsx_pci_send_cmd 401)
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFDA0(8): 11
01 04 08 00 1f 00 08
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFDA8(8): 10
ff ff ff ff ff 7f 00
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFDB0(4): 02
00 00 08 00 00 00 00
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFD52(8): 04
41 00 04 0f 00 f0 00
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFD5A(8): 94
1f 52 ff 94 bb 66 aa
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: 0xFD62(8): e9
aa aa aa 24 00 04 04
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0:
rtsx_pci_send_cmd error (err = -110)
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: CMD 18
0x00002000 error(-110)
Apr  5 21:38:35 - kernel: rtsx_pci 0000:6f:00.0: Switch card clock to 50MHz
Apr  5 21:38:35 - kernel: rtsx_pci 0000:6f:00.0: Internal SSC clock:
100MHz (cur_clock = 100)
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0:
sd_send_cmd_get_rsp: SD/MMC CMD 13, arg = 0xe6240000
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: cmd->resp[0]
= 0x00400900
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: pre dma sg: 4
Apr  5 21:38:35 - kernel: rtsx_pci 0000:6f:00.0: Switch card clock to 50MHz
Apr  5 21:38:35 - kernel: rtsx_pci 0000:6f:00.0: Internal SSC clock:
100MHz (cur_clock = 100)
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0:
sd_read_long_data: SD/MMC CMD 18, arg = 0x00002000
Apr  5 21:38:35 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xff050000,
Len: 0x1000
Apr  5 21:38:35 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xff051000,
Len: 0x1000
Apr  5 21:38:35 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xff052000,
Len: 0x1000
Apr  5 21:38:35 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xff053000,
Len: 0x1000
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0:
sd_send_cmd_get_rsp: SD/MMC CMD 12, arg = 0x00000000
Apr  5 21:38:35 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: cmd->resp[0]
= 0x00000b00
Apr  5 21:38:35 - kernel: rtsx_pci 0000:6f:00.0: --> rtsx_pci_idle_work

--------------------------------------------------------------------------

Kernel 5.6.2 with msleep(1); like described above:

For testing i tried to read some bytes from the SD card using DD. dd is
called every 2 seconds, and reads 10 bytes from the SD card. The delay
between the reads is inserted, to allow the driver to go into power
saving and issue a rtsx_pci_idle_work and trigger the issue.

The 2 seconds delay between rtsx_pci_idle_work and the next log entry
are therefore not from the issue itself, but from the way I read data.
Access in file system mode, dd and everything else does not experience
any noticeable delay anymore.


Apr  5 21:42:46 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: pre dma sg: 4
Apr  5 21:42:46 - kernel: rtsx_pci 0000:6f:00.0: Switch card clock to 50MHz
Apr  5 21:42:46 - kernel: rtsx_pci 0000:6f:00.0: Internal SSC clock:
100MHz (cur_clock = 100)
Apr  5 21:42:46 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0:
sd_read_long_data: SD/MMC CMD 18, arg = 0x00002000
Apr  5 21:42:46 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xffbbc000,
Len: 0x1000
Apr  5 21:42:46 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xffbbd000,
Len: 0x1000
Apr  5 21:42:46 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xffbbe000,
Len: 0x1000
Apr  5 21:42:46 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xffbbf000,
Len: 0x1000
Apr  5 21:42:46 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0:
sd_send_cmd_get_rsp: SD/MMC CMD 12, arg = 0x00000000
Apr  5 21:42:46 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: cmd->resp[0]
= 0x00000b00
Apr  5 21:42:46 - kernel: rtsx_pci 0000:6f:00.0: --> rtsx_pci_idle_work
-- no delay anymore
Apr  5 21:42:48 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: pre dma sg: 4
Apr  5 21:42:48 - kernel: rtsx_pci 0000:6f:00.0: Switch card clock to 50MHz
Apr  5 21:42:48 - kernel: rtsx_pci 0000:6f:00.0: Internal SSC clock:
100MHz (cur_clock = 100)
Apr  5 21:42:48 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0:
sd_read_long_data: SD/MMC CMD 18, arg = 0x00002000
Apr  5 21:42:48 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xffc3c000,
Len: 0x1000
Apr  5 21:42:48 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xffc3d000,
Len: 0x1000
Apr  5 21:42:48 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xffc3e000,
Len: 0x1000
Apr  5 21:42:48 - kernel: rtsx_pci 0000:6f:00.0: DMA addr: 0xffc3f000,
Len: 0x1000
Apr  5 21:42:48 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0:
sd_send_cmd_get_rsp: SD/MMC CMD 12, arg = 0x00000000
Apr  5 21:42:48 - kernel: rtsx_pci_sdmmc rtsx_pci_sdmmc.0: cmd->resp[0]
= 0x00000b00
Apr  5 21:42:48 - kernel: rtsx_pci 0000:6f:00.0: --> rtsx_pci_idle_work

Re: Possible bug in drivers/misc/cardreader/rtsx_pcr.c

[Greg Kroah-Hartman]

On Tue, May 19, 2020 at 07:04:06PM +0200, Klaus Doth wrote:
> Hi,
> 
> 
> As per the info from kernelnewbies IRC, I'm sending this also to the PCI
> list.

<snip>

Can you submit a proposed patch in a format that it can be tested and
possibly submitted in so that we can review this easier?

Also try cc:ing the author of changes in that code, Rui Feng
<rui_feng@realsil.com.cn>, as well, as they are the best one to review
and comment on your issue.

thanks,

greg k-h

Re: Possible bug in drivers/misc/cardreader/rtsx_pcr.c

[Klaus Doth]

On 5/21/20 10:52 AM, Greg Kroah-Hartman wrote:
> On Tue, May 19, 2020 at 07:04:06PM +0200, Klaus Doth wrote:
>> Hi,
>>
>>
>> As per the info from kernelnewbies IRC, I'm sending this also to the PCI
>> list.
> <snip>
>
> Can you submit a proposed patch in a format that it can be tested and
> possibly submitted in so that we can review this easier?
>
> Also try cc:ing the author of changes in that code, Rui Feng
> <rui_feng@realsil.com.cn>, as well, as they are the best one to review
> and comment on your issue.
>
> thanks,
>
> greg k-h


DMA transfers to and from the SD card stall for 10 seconds and run into
timeout on RTS5260 card readers after ASPM was enabled.

Adding a short msleep after disabling ASPM fixes the issue on several
Dell Precision 7530/7540 systems I tested.

This function is only called when waking up after the chip went into
powersave after not transferring data for a few seconds. The added
msleep does therefore not change anything in data transfer speed or
induce any excessive waiting while data transfers are running, or the
chip is sleeping. Only the transistion from sleep to active is affected.


Signed-off-by: Klaus Doth <kdlnx@doth.eu>

---
 drivers/misc/cardreader/rtsx_pcr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/misc/cardreader/rtsx_pcr.c
b/drivers/misc/cardreader/rtsx_pcr.c
index 06038b325b02..8b0799cd88c2 100644
--- a/drivers/misc/cardreader/rtsx_pcr.c
+++ b/drivers/misc/cardreader/rtsx_pcr.c
@@ -141,6 +141,7 @@ static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
     struct rtsx_cr_option *option = &pcr->option;
 
     rtsx_disable_aspm(pcr);
+    msleep(1);
 
     if (option->ltr_enabled)
         rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
-- 
2.26.2

Re: Possible bug in drivers/misc/cardreader/rtsx_pcr.c

[Bjorn Helgaas]

On Thu, May 21, 2020 at 01:50:02PM +0200, Klaus Doth wrote:
> On 5/21/20 10:52 AM, Greg Kroah-Hartman wrote:
> > On Tue, May 19, 2020 at 07:04:06PM +0200, Klaus Doth wrote:
> >> Hi,
> >>
> >> As per the info from kernelnewbies IRC, I'm sending this also to the PCI
> >> list.
> > <snip>
> >
> > Can you submit a proposed patch in a format that it can be tested and
> > possibly submitted in so that we can review this easier?
> >
> > Also try cc:ing the author of changes in that code, Rui Feng
> > <rui_feng@realsil.com.cn>, as well, as they are the best one to review
> > and comment on your issue.
> >
> > thanks,
> >
> > greg k-h
> 
> DMA transfers to and from the SD card stall for 10 seconds and run into
> timeout on RTS5260 card readers after ASPM was enabled.
> 
> Adding a short msleep after disabling ASPM fixes the issue on several
> Dell Precision 7530/7540 systems I tested.
> 
> This function is only called when waking up after the chip went into
> powersave after not transferring data for a few seconds. The added
> msleep does therefore not change anything in data transfer speed or
> induce any excessive waiting while data transfers are running, or the
> chip is sleeping. Only the transistion from sleep to active is affected.

s/transistion/transition/

I don't see anything in the PCIe spec that would require a delay here.
In general, drivers should not be configuration ASPM directly because
doing it correctly requires knowledge of the upstream component, i.e.,
a root port or a switch downstream port.

But these chips seem to be pretty buggy in terms of ASPM, so it
doesn't really surprise me if they're broken here.

That's all to say that I don't know *why* this would be needed, but I
don't object to it.

> Signed-off-by: Klaus Doth <kdlnx@doth.eu>
> 
> ---
>  drivers/misc/cardreader/rtsx_pcr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/misc/cardreader/rtsx_pcr.c
> b/drivers/misc/cardreader/rtsx_pcr.c
> index 06038b325b02..8b0799cd88c2 100644
> --- a/drivers/misc/cardreader/rtsx_pcr.c
> +++ b/drivers/misc/cardreader/rtsx_pcr.c
> @@ -141,6 +141,7 @@ static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
>      struct rtsx_cr_option *option = &pcr->option;
>  
>      rtsx_disable_aspm(pcr);
> +    msleep(1);
>  
>      if (option->ltr_enabled)
>          rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
> -- 
> 2.26.2
> 
> 

Re: Possible bug in drivers/misc/cardreader/rtsx_pcr.c

[Greg Kroah-Hartman]

On Thu, May 21, 2020 at 01:50:02PM +0200, Klaus Doth wrote:
> On 5/21/20 10:52 AM, Greg Kroah-Hartman wrote:
> > On Tue, May 19, 2020 at 07:04:06PM +0200, Klaus Doth wrote:
> >> Hi,
> >>
> >>
> >> As per the info from kernelnewbies IRC, I'm sending this also to the PCI
> >> list.
> > <snip>
> >
> > Can you submit a proposed patch in a format that it can be tested and
> > possibly submitted in so that we can review this easier?
> >
> > Also try cc:ing the author of changes in that code, Rui Feng
> > <rui_feng@realsil.com.cn>, as well, as they are the best one to review
> > and comment on your issue.
> >
> > thanks,
> >
> > greg k-h
> 
> 
> DMA transfers to and from the SD card stall for 10 seconds and run into
> timeout on RTS5260 card readers after ASPM was enabled.
> 
> Adding a short msleep after disabling ASPM fixes the issue on several
> Dell Precision 7530/7540 systems I tested.
> 
> This function is only called when waking up after the chip went into
> powersave after not transferring data for a few seconds. The added
> msleep does therefore not change anything in data transfer speed or
> induce any excessive waiting while data transfers are running, or the
> chip is sleeping. Only the transistion from sleep to active is affected.
> 
> 
> Signed-off-by: Klaus Doth <kdlnx@doth.eu>
> 
> ---
>  drivers/misc/cardreader/rtsx_pcr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/misc/cardreader/rtsx_pcr.c
> b/drivers/misc/cardreader/rtsx_pcr.c
> index 06038b325b02..8b0799cd88c2 100644
> --- a/drivers/misc/cardreader/rtsx_pcr.c
> +++ b/drivers/misc/cardreader/rtsx_pcr.c
> @@ -141,6 +141,7 @@ static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
>      struct rtsx_cr_option *option = &pcr->option;
>  
>      rtsx_disable_aspm(pcr);
> +    msleep(1);
>  
>      if (option->ltr_enabled)
>          rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
> -- 
> 2.26.2

Looks sane, can you resend it in a format I can apply it in (i.e. as a
stand-alone patch with a correct subject line?)

thanks,

greg k-h

[PATCH] misc: rtsx: Add short delay after exit from ASPM

[Klaus Doth]

From: Klaus Doth <kdlnx@doth.eu>

DMA transfers to and from the SD card stall for 10 seconds and run into
timeout on RTS5260 card readers after ASPM was enabled.

Adding a short msleep after disabling ASPM fixes the issue on several
Dell Precision 7530/7540 systems I tested.

This function is only called when waking up after the chip went into
power-save after not transferring data for a few seconds. The added
msleep does therefore not change anything in data transfer speed or
induce any excessive waiting while data transfers are running, or the
chip is sleeping. Only the transition from sleep to active is affected.

Signed-off-by: Klaus Doth <kdlnx@doth.eu>
---
 drivers/misc/cardreader/rtsx_pcr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/misc/cardreader/rtsx_pcr.c
b/drivers/misc/cardreader/rtsx_pcr.c
index 06038b325b02..8b0799cd88c2 100644
--- a/drivers/misc/cardreader/rtsx_pcr.c
+++ b/drivers/misc/cardreader/rtsx_pcr.c
@@ -141,6 +141,7 @@ static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
     struct rtsx_cr_option *option = &pcr->option;
 
     rtsx_disable_aspm(pcr);
+    msleep(1);
 
     if (option->ltr_enabled)
         rtsx_set_ltr_latency(pcr, option->ltr_active_latency);

-- 
2.26.2

Re: [PATCH] misc: rtsx: Add short delay after exit from ASPM

[Arnd Bergmann]

On Fri, May 22, 2020 at 10:23 AM Klaus Doth <kdlnx@doth.eu> wrote:
>
> From: Klaus Doth <kdlnx@doth.eu>
>
> DMA transfers to and from the SD card stall for 10 seconds and run into
> timeout on RTS5260 card readers after ASPM was enabled.
>
> Adding a short msleep after disabling ASPM fixes the issue on several
> Dell Precision 7530/7540 systems I tested.
>
> This function is only called when waking up after the chip went into
> power-save after not transferring data for a few seconds. The added
> msleep does therefore not change anything in data transfer speed or
> induce any excessive waiting while data transfers are running, or the
> chip is sleeping. Only the transition from sleep to active is affected.
>
> Signed-off-by: Klaus Doth <kdlnx@doth.eu>
> ---
>  drivers/misc/cardreader/rtsx_pcr.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/misc/cardreader/rtsx_pcr.c
> b/drivers/misc/cardreader/rtsx_pcr.c
> index 06038b325b02..8b0799cd88c2 100644
> --- a/drivers/misc/cardreader/rtsx_pcr.c
> +++ b/drivers/misc/cardreader/rtsx_pcr.c
> @@ -141,6 +141,7 @@ static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
>      struct rtsx_cr_option *option = &pcr->option;
>
>      rtsx_disable_aspm(pcr);
> +    msleep(1);

Your patch looks fine to me, but I think you should put a short version
of that the changelog text into a code comment next to the msleep().

       Arnd

[PATCH v2] misc: rtsx: Add short delay after exit from ASPM

[Klaus Doth]

From: Klaus Doth <kdlnx@doth.eu>

DMA transfers to and from the SD card stall for 10 seconds and run into
timeout on RTS5260 card readers after ASPM was enabled.

Adding a short msleep after disabling ASPM fixes the issue on several
Dell Precision 7530/7540 systems I tested.

This function is only called when waking up after the chip went into
power-save after not transferring data for a few seconds. The added
msleep does therefore not change anything in data transfer speed or
induce any excessive waiting while data transfers are running, or the
chip is sleeping. Only the transition from sleep to active is affected.

Signed-off-by: Klaus Doth <kdlnx@doth.eu>
---
 drivers/misc/cardreader/rtsx_pcr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/misc/cardreader/rtsx_pcr.c
b/drivers/misc/cardreader/rtsx_pcr.c
index 06038b325b02..3a6a6988cf80 100644
--- a/drivers/misc/cardreader/rtsx_pcr.c
+++ b/drivers/misc/cardreader/rtsx_pcr.c
@@ -141,6 +141,9 @@ static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
     struct rtsx_cr_option *option = &pcr->option;
 
     rtsx_disable_aspm(pcr);
+   
+    /* Fixes DMA transfer timeout issue after disabling ASPM on RTS5260 */
+    msleep(1);
 
     if (option->ltr_enabled)
         rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
-- 
2.26.2

Re: [PATCH v2] misc: rtsx: Add short delay after exit from ASPM

[Greg Kroah-Hartman]

On Fri, May 22, 2020 at 11:18:24AM +0200, Klaus Doth wrote:
> From: Klaus Doth <kdlnx@doth.eu>
> 
> DMA transfers to and from the SD card stall for 10 seconds and run into
> timeout on RTS5260 card readers after ASPM was enabled.
> 
> Adding a short msleep after disabling ASPM fixes the issue on several
> Dell Precision 7530/7540 systems I tested.
> 
> This function is only called when waking up after the chip went into
> power-save after not transferring data for a few seconds. The added
> msleep does therefore not change anything in data transfer speed or
> induce any excessive waiting while data transfers are running, or the
> chip is sleeping. Only the transition from sleep to active is affected.
> 
> Signed-off-by: Klaus Doth <kdlnx@doth.eu>
> ---
>  drivers/misc/cardreader/rtsx_pcr.c | 3 +++
>  1 file changed, 3 insertions(+)

What changed from v1?  Always put that below the --- line like the
documentation says to do so.

> 
> diff --git a/drivers/misc/cardreader/rtsx_pcr.c
> b/drivers/misc/cardreader/rtsx_pcr.c
> index 06038b325b02..3a6a6988cf80 100644
> --- a/drivers/misc/cardreader/rtsx_pcr.c
> +++ b/drivers/misc/cardreader/rtsx_pcr.c
> @@ -141,6 +141,9 @@ static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
>      struct rtsx_cr_option *option = &pcr->option;
>  
>      rtsx_disable_aspm(pcr);
> +   
> +    /* Fixes DMA transfer timeout issue after disabling ASPM on RTS5260 */
> +    msleep(1);
>  
>      if (option->ltr_enabled)
>          rtsx_set_ltr_latency(pcr, option->ltr_active_latency);

All tabs are gone and replaced with spaces, making this impossible to
apply :(

thanks,

greg k-h

[PATCH v3] misc: rtsx: Add short delay after exit from ASPM

[Klaus Doth]

From: Klaus Doth <kdlnx@doth.eu>

DMA transfers to and from the SD card stall for 10 seconds and run into
timeout on RTS5260 card readers after ASPM was enabled.

Adding a short msleep after disabling ASPM fixes the issue on several
Dell Precision 7530/7540 systems I tested.

This function is only called when waking up after the chip went into
power-save after not transferring data for a few seconds. The added
msleep does therefore not change anything in data transfer speed or
induce any excessive waiting while data transfers are running, or the
chip is sleeping. Only the transition from sleep to active is affected.

Signed-off-by: Klaus Doth <kdlnx@doth.eu>
---
Changes from v2:
  - Added this changelog. Tabs should now be tabs instead of spaces.

Changes from v1:
  - Added comment explaining why the msleep is required

 drivers/misc/cardreader/rtsx_pcr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c
index 06038b325b02..3a6a6988cf80 100644
--- a/drivers/misc/cardreader/rtsx_pcr.c
+++ b/drivers/misc/cardreader/rtsx_pcr.c
@@ -141,6 +141,9 @@ static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
 	struct rtsx_cr_option *option = &pcr->option;
 
 	rtsx_disable_aspm(pcr);
+	
+	/* Fixes DMA transfer timout issue after disabling ASPM on RTS5260 */
+	msleep(1);
 
 	if (option->ltr_enabled)
 		rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
-- 
2.26.2

Re: [PATCH v3] misc: rtsx: Add short delay after exit from ASPM

[Greg Kroah-Hartman]

On Fri, May 22, 2020 at 12:56:04PM +0200, Klaus Doth wrote:
> From: Klaus Doth <kdlnx@doth.eu>
> 
> DMA transfers to and from the SD card stall for 10 seconds and run into
> timeout on RTS5260 card readers after ASPM was enabled.
> 
> Adding a short msleep after disabling ASPM fixes the issue on several
> Dell Precision 7530/7540 systems I tested.
> 
> This function is only called when waking up after the chip went into
> power-save after not transferring data for a few seconds. The added
> msleep does therefore not change anything in data transfer speed or
> induce any excessive waiting while data transfers are running, or the
> chip is sleeping. Only the transition from sleep to active is affected.
> 
> Signed-off-by: Klaus Doth <kdlnx@doth.eu>
> Cc: stable <stable@vger.kernel.org>
> ---
> Changes from v2:
>   - Added this changelog. Tabs should now be tabs instead of spaces.
> 
> Changes from v1:
>   - Added comment explaining why the msleep is required
> 
>  drivers/misc/cardreader/rtsx_pcr.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c
> index 06038b325b02..3a6a6988cf80 100644
> --- a/drivers/misc/cardreader/rtsx_pcr.c
> +++ b/drivers/misc/cardreader/rtsx_pcr.c
> @@ -141,6 +141,9 @@ static void rtsx_comm_pm_full_on(struct rtsx_pcr *pcr)
>  	struct rtsx_cr_option *option = &pcr->option;
>  
>  	rtsx_disable_aspm(pcr);
> +	

That's trailing whitespace :(

I'll fix it up by hand, but next time, always run your patches through
scripts/checkpatch.pl to catch these things.

thanks,

greg k-h