Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot
Hi kernel maintainers!
My computer doesn't boot with kernels newer than 6.1.45.
Here's what happens: - system boots in initramfs - detects my encrypted ZFS pool and asks for password - mount system, pivots to it, starts real init - before any daemon had time to start, the system hangs and the kernel writes on the console "nvme 0000:04:00.0: Unable to change power state from D3cold to D0, device inaccessible" - if I reboot directly without powering off (using magic sysrq or panic=10), even the UEFI complains about not finding any storage to boot from. - after a real power off, I can boot using a kernel <= 6.1.45.
The bug has been discussed here: https://bugzilla.kernel.org/show_bug.cgi?id=217705
My laptop is a Dell XPS 15 9560 (Intel 7700hq).
I bisected between 6.1.45 and 6.1.46 and found this commit
commit 8ee39ec479147e29af704639f8e55fce246ed2d9 Author: Ricky WU ricky_wu@realtek.com Date: Tue Jul 25 09:10:54 2023 +0000
misc: rtsx: judge ASPM Mode to set PETXCFG Reg
commit 101bd907b4244a726980ee67f95ed9cafab6ff7a upstream.
ASPM Mode is ASPM_MODE_CFG need to judge the value of clkreq_0 to set HIGH or LOW, if the ASPM Mode is ASPM_MODE_REG always set to HIGH during the initialization.
Cc: stable@vger.kernel.org Signed-off-by: Ricky Wu ricky_wu@realtek.com Link: https://lore.kernel.org/r/52906c6836374c8cb068225954c5543a@realtek.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
drivers/misc/cardreader/rts5227.c | 2 +- drivers/misc/cardreader/rts5228.c | 18 ------------------ drivers/misc/cardreader/rts5249.c | 3 +-- drivers/misc/cardreader/rts5260.c | 18 ------------------ drivers/misc/cardreader/rts5261.c | 18 ------------------ drivers/misc/cardreader/rtsx_pcr.c | 5 ++++- 6 files changed, 6 insertions(+), 58 deletions(-)
If I build 6.1.51 with this commit reverted, my laptop works again, confirming that this commit is to blame.
Also, blacklisting `rtsx_pci_sdmmc` and `rtsx_pci`, while preventing to use the sd card reading, allows to boot the system.
I can't try 6.4 or 6.5 because my system is dependent on ZFS..
Have a nice day, Paul Grandperrin
(CCing Greg, as he merged the culprit, and Linus, in case he wants to revert this from mainline directly as this apparently affects and annoys quite a few people)
On 12.09.23 14:29, Paul Grandperrin wrote:
My computer doesn't boot with kernels newer than 6.1.45.> Here's what happens:
- system boots in initramfs
- detects my encrypted ZFS pool and asks for password
- mount system, pivots to it, starts real init
- before any daemon had time to start, the system hangs and the kernel
writes on the console "nvme 0000:04:00.0: Unable to change power state from D3cold to D0, device inaccessible"
- if I reboot directly without powering off (using magic sysrq or
panic=10), even the UEFI complains about not finding any storage to boot from.
- after a real power off, I can boot using a kernel <= 6.1.45.
Thx for the report.
The bug has been discussed here: https://bugzilla.kernel.org/show_bug.cgi?id=217705
And recently here: https://bugzilla.kernel.org/show_bug.cgi?id=217802 https://lore.kernel.org/all/5f968b95-6b1c-4d6f-aac7-5d54f66834a8@sapience.co...
And in https://bugzilla.suse.com/show_bug.cgi?id=1214428 and a few other places iirc, too.
openSUSE Tumblewed reverted the culprit a while ago:
https://github.com/openSUSE/kernel-source/commit/1b02b1528a26f4e9b577e215c11...
I think we should do the same for mainline (and stable afterwards).
Ciao, Thorsten
My laptop is a Dell XPS 15 9560 (Intel 7700hq).
I bisected between 6.1.45 and 6.1.46 and found this commit
commit 8ee39ec479147e29af704639f8e55fce246ed2d9 Author: Ricky WU ricky_wu@realtek.com Date: Tue Jul 25 09:10:54 2023 +0000
misc: rtsx: judge ASPM Mode to set PETXCFG Reg
commit 101bd907b4244a726980ee67f95ed9cafab6ff7a upstream.
ASPM Mode is ASPM_MODE_CFG need to judge the value of clkreq_0 to set HIGH or LOW, if the ASPM Mode is ASPM_MODE_REG always set to HIGH during the initialization.
Cc: stable@vger.kernel.org Signed-off-by: Ricky Wu ricky_wu@realtek.com Link: https://lore.kernel.org/r/52906c6836374c8cb068225954c5543a@realtek.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
drivers/misc/cardreader/rts5227.c | 2 +- drivers/misc/cardreader/rts5228.c | 18 ------------------ drivers/misc/cardreader/rts5249.c | 3 +-- drivers/misc/cardreader/rts5260.c | 18 ------------------ drivers/misc/cardreader/rts5261.c | 18 ------------------ drivers/misc/cardreader/rtsx_pcr.c | 5 ++++- 6 files changed, 6 insertions(+), 58 deletions(-)
If I build 6.1.51 with this commit reverted, my laptop works again, confirming that this commit is to blame.
Also, blacklisting `rtsx_pci_sdmmc` and `rtsx_pci`, while preventing to use the sd card reading, allows to boot the system.
I can't try 6.4 or 6.5 because my system is dependent on ZFS..
Have a nice day, Paul Grandperrin
On Tue, Sep 12, 2023 at 07:10:38PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
(CCing Greg, as he merged the culprit, and Linus, in case he wants to revert this from mainline directly as this apparently affects and annoys quite a few people)
The driver authors know about this and have said they are working on a solution. Let's give them a few more days on it before reverting stuff.
thanks,
greg k-h
Hi Greg k-h，
In order to cover the those platform Power saving issue, our approach on new patch will be different from the previous patch (101bd907b4244a726980ee67f95ed9cafab6ff7a).
So we need used fixed Tag on 101bd907b4244a726980ee67f95ed9cafab6ff7a or a new patch for this problem?
Ricky
-----Original Message----- From: Greg KH gregkh@linuxfoundation.org Sent: Wednesday, September 13, 2023 1:03 PM To: Linux regressions mailing list regressions@lists.linux.dev Cc: Paul Grandperrin paul.grandperrin@gmail.com; stable@vger.kernel.org; Wei_wang wei_wang@realsil.com.cn; Roger Tseng rogerable@realtek.com; Ricky WU ricky_wu@realtek.com; Linus Torvalds torvalds@linux-foundation.org Subject: Re: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot
External mail.
On Tue, Sep 12, 2023 at 07:10:38PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
(CCing Greg, as he merged the culprit, and Linus, in case he wants to revert this from mainline directly as this apparently affects and annoys quite a few people)
The driver authors know about this and have said they are working on a solution. Let's give them a few more days on it before reverting stuff.
thanks,
greg k-h
On Tue, Sep 19, 2023 at 02:20:53AM +0000, Ricky WU wrote:
Hi Greg k-h，
In order to cover the those platform Power saving issue, our approach on new patch will be different from the previous patch (101bd907b4244a726980ee67f95ed9cafab6ff7a).
So we need used fixed Tag on 101bd907b4244a726980ee67f95ed9cafab6ff7a or a new patch for this problem?
I'm sorry, but I do not understand. I think a new change is needed here, right? Or are you wanting a different commit backported to stable trees? What about Linus's tree now?
What exactly should I do here?
confused,
greg k-h
Hi Greg k-h,
This patch is our solution for this issue... And now how can I push this?
Hi Paul，
Can you help to try this patch? Is it work for your platform?
Ricky
--- drivers/misc/cardreader/rts5227.c | 55 ++++------------------------ drivers/misc/cardreader/rts5228.c | 57 +++++++++--------------------- drivers/misc/cardreader/rts5249.c | 56 ++++------------------------- drivers/misc/cardreader/rts5260.c | 43 +++++++--------------- drivers/misc/cardreader/rts5261.c | 52 +++++++-------------------- drivers/misc/cardreader/rtsx_pcr.c | 51 +++++++++++++++++++++++--- 6 files changed, 102 insertions(+), 212 deletions(-)
diff --git a/drivers/misc/cardreader/rts5227.c b/drivers/misc/cardreader/rts5227.c index 3dae5e3a1697..cd512284bfb3 100644 --- a/drivers/misc/cardreader/rts5227.c +++ b/drivers/misc/cardreader/rts5227.c @@ -83,63 +83,20 @@ static void rts5227_fetch_vendor_settings(struct rtsx_pcr *pcr)
static void rts5227_init_from_cfg(struct rtsx_pcr *pcr) { - struct pci_dev *pdev = pcr->pci; - int l1ss; - u32 lval; struct rtsx_cr_option *option = &pcr->option;
- l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS); - if (!l1ss) - return; - - pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, &lval); - if (CHK_PCI_PID(pcr, 0x522A)) { - if (0 == (lval & 0x0F)) - rtsx_pci_enable_oobs_polling(pcr); - else + if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN + | PM_L1_1_EN | PM_L1_2_EN)) rtsx_pci_disable_oobs_polling(pcr); + else + rtsx_pci_enable_oobs_polling(pcr); }
- if (lval & PCI_L1SS_CTL1_ASPM_L1_1) - rtsx_set_dev_flag(pcr, ASPM_L1_1_EN); - else - rtsx_clear_dev_flag(pcr, ASPM_L1_1_EN); - - if (lval & PCI_L1SS_CTL1_ASPM_L1_2) - rtsx_set_dev_flag(pcr, ASPM_L1_2_EN); - else - rtsx_clear_dev_flag(pcr, ASPM_L1_2_EN); - - if (lval & PCI_L1SS_CTL1_PCIPM_L1_1) - rtsx_set_dev_flag(pcr, PM_L1_1_EN); - else - rtsx_clear_dev_flag(pcr, PM_L1_1_EN); - - if (lval & PCI_L1SS_CTL1_PCIPM_L1_2) - rtsx_set_dev_flag(pcr, PM_L1_2_EN); - else - rtsx_clear_dev_flag(pcr, PM_L1_2_EN); - if (option->ltr_en) { - u16 val; - - pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &val); - if (val & PCI_EXP_DEVCTL2_LTR_EN) { - option->ltr_enabled = true; - option->ltr_active = true; + if (option->ltr_enabled) rtsx_set_ltr_latency(pcr, option->ltr_active_latency); - } else { - option->ltr_enabled = false; - } } - - if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN - | PM_L1_1_EN | PM_L1_2_EN)) - option->force_clkreq_0 = false; - else - option->force_clkreq_0 = true; - }
static int rts5227_extra_init_hw(struct rtsx_pcr *pcr) @@ -195,7 +152,7 @@ static int rts5227_extra_init_hw(struct rtsx_pcr *pcr) } }
- if (option->force_clkreq_0 && pcr->aspm_mode == ASPM_MODE_CFG) + if (option->force_clkreq_0) rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG, FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_LOW); else diff --git a/drivers/misc/cardreader/rts5228.c b/drivers/misc/cardreader/rts5228.c index f4ab09439da7..0c7f10bcf6f1 100644 --- a/drivers/misc/cardreader/rts5228.c +++ b/drivers/misc/cardreader/rts5228.c @@ -386,59 +386,25 @@ static void rts5228_process_ocp(struct rtsx_pcr *pcr)
static void rts5228_init_from_cfg(struct rtsx_pcr *pcr) { - struct pci_dev *pdev = pcr->pci; - int l1ss; - u32 lval; struct rtsx_cr_option *option = &pcr->option;
- l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS); - if (!l1ss) - return; - - pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, &lval); - - if (0 == (lval & 0x0F)) - rtsx_pci_enable_oobs_polling(pcr); - else + if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN + | PM_L1_1_EN | PM_L1_2_EN)) rtsx_pci_disable_oobs_polling(pcr); - - if (lval & PCI_L1SS_CTL1_ASPM_L1_1) - rtsx_set_dev_flag(pcr, ASPM_L1_1_EN); - else - rtsx_clear_dev_flag(pcr, ASPM_L1_1_EN); - - if (lval & PCI_L1SS_CTL1_ASPM_L1_2) - rtsx_set_dev_flag(pcr, ASPM_L1_2_EN); - else - rtsx_clear_dev_flag(pcr, ASPM_L1_2_EN); - - if (lval & PCI_L1SS_CTL1_PCIPM_L1_1) - rtsx_set_dev_flag(pcr, PM_L1_1_EN); else - rtsx_clear_dev_flag(pcr, PM_L1_1_EN); - - if (lval & PCI_L1SS_CTL1_PCIPM_L1_2) - rtsx_set_dev_flag(pcr, PM_L1_2_EN); - else - rtsx_clear_dev_flag(pcr, PM_L1_2_EN); + rtsx_pci_enable_oobs_polling(pcr);
rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, 0xFF, 0); - if (option->ltr_en) { - u16 val;
- pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &val); - if (val & PCI_EXP_DEVCTL2_LTR_EN) { - option->ltr_enabled = true; - option->ltr_active = true; + if (option->ltr_en) { + if (option->ltr_enabled) rtsx_set_ltr_latency(pcr, option->ltr_active_latency); - } else { - option->ltr_enabled = false; - } } }
static int rts5228_extra_init_hw(struct rtsx_pcr *pcr) { + struct rtsx_cr_option *option = &pcr->option;
rtsx_pci_write_register(pcr, RTS5228_AUTOLOAD_CFG1, CD_RESUME_EN_MASK, CD_RESUME_EN_MASK); @@ -469,6 +435,17 @@ static int rts5228_extra_init_hw(struct rtsx_pcr *pcr) else rtsx_pci_write_register(pcr, PETXCFG, 0x30, 0x00);
+ /* + * If u_force_clkreq_0 is enabled, CLKREQ# PIN will be forced + * to drive low, and we forcibly request clock. + */ + if (option->force_clkreq_0) + rtsx_pci_write_register(pcr, PETXCFG, + FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_LOW); + else + rtsx_pci_write_register(pcr, PETXCFG, + FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_HIGH); + rtsx_pci_write_register(pcr, PWD_SUSPEND_EN, 0xFF, 0xFB);
if (pcr->rtd3_en) { diff --git a/drivers/misc/cardreader/rts5249.c b/drivers/misc/cardreader/rts5249.c index 47ab72a43256..6c81040e18be 100644 --- a/drivers/misc/cardreader/rts5249.c +++ b/drivers/misc/cardreader/rts5249.c @@ -86,64 +86,22 @@ static void rtsx_base_fetch_vendor_settings(struct rtsx_pcr *pcr)
static void rts5249_init_from_cfg(struct rtsx_pcr *pcr) { - struct pci_dev *pdev = pcr->pci; - int l1ss; struct rtsx_cr_option *option = &(pcr->option); - u32 lval; - - l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS); - if (!l1ss) - return; - - pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, &lval);
if (CHK_PCI_PID(pcr, PID_524A) || CHK_PCI_PID(pcr, PID_525A)) { - if (0 == (lval & 0x0F)) - rtsx_pci_enable_oobs_polling(pcr); - else + if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN + | PM_L1_1_EN | PM_L1_2_EN)) rtsx_pci_disable_oobs_polling(pcr); + else + rtsx_pci_enable_oobs_polling(pcr); }
- - if (lval & PCI_L1SS_CTL1_ASPM_L1_1) - rtsx_set_dev_flag(pcr, ASPM_L1_1_EN); - - if (lval & PCI_L1SS_CTL1_ASPM_L1_2) - rtsx_set_dev_flag(pcr, ASPM_L1_2_EN); - - if (lval & PCI_L1SS_CTL1_PCIPM_L1_1) - rtsx_set_dev_flag(pcr, PM_L1_1_EN); - - if (lval & PCI_L1SS_CTL1_PCIPM_L1_2) - rtsx_set_dev_flag(pcr, PM_L1_2_EN); - if (option->ltr_en) { - u16 val; - - pcie_capability_read_word(pdev, PCI_EXP_DEVCTL2, &val); - if (val & PCI_EXP_DEVCTL2_LTR_EN) { - option->ltr_enabled = true; - option->ltr_active = true; + if (option->ltr_enabled) rtsx_set_ltr_latency(pcr, option->ltr_active_latency); - } else { - option->ltr_enabled = false; - } } }
-static int rts5249_init_from_hw(struct rtsx_pcr *pcr) -{ - struct rtsx_cr_option *option = &(pcr->option); - - if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN - | PM_L1_1_EN | PM_L1_2_EN)) - option->force_clkreq_0 = false; - else - option->force_clkreq_0 = true; - - return 0; -} - static void rts52xa_force_power_down(struct rtsx_pcr *pcr, u8 pm_state, bool runtime) { /* Set relink_time to 0 */ @@ -276,7 +234,6 @@ static int rts5249_extra_init_hw(struct rtsx_pcr *pcr) struct rtsx_cr_option *option = &(pcr->option);
rts5249_init_from_cfg(pcr); - rts5249_init_from_hw(pcr);
rtsx_pci_init_cmd(pcr);
@@ -327,11 +284,12 @@ static int rts5249_extra_init_hw(struct rtsx_pcr *pcr) } }
+ /* * If u_force_clkreq_0 is enabled, CLKREQ# PIN will be forced * to drive low, and we forcibly request clock. */ - if (option->force_clkreq_0 && pcr->aspm_mode == ASPM_MODE_CFG) + if (option->force_clkreq_0) rtsx_pci_write_register(pcr, PETXCFG, FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_LOW); else diff --git a/drivers/misc/cardreader/rts5260.c b/drivers/misc/cardreader/rts5260.c index 79b18f6f73a8..d2d3a6ccb8f7 100644 --- a/drivers/misc/cardreader/rts5260.c +++ b/drivers/misc/cardreader/rts5260.c @@ -480,47 +480,19 @@ static void rts5260_pwr_saving_setting(struct rtsx_pcr *pcr)
static void rts5260_init_from_cfg(struct rtsx_pcr *pcr) { - struct pci_dev *pdev = pcr->pci; - int l1ss; struct rtsx_cr_option *option = &pcr->option; - u32 lval; - - l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS); - if (!l1ss) - return; - - pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, &lval); - - if (lval & PCI_L1SS_CTL1_ASPM_L1_1) - rtsx_set_dev_flag(pcr, ASPM_L1_1_EN); - - if (lval & PCI_L1SS_CTL1_ASPM_L1_2) - rtsx_set_dev_flag(pcr, ASPM_L1_2_EN); - - if (lval & PCI_L1SS_CTL1_PCIPM_L1_1) - rtsx_set_dev_flag(pcr, PM_L1_1_EN); - - if (lval & PCI_L1SS_CTL1_PCIPM_L1_2) - rtsx_set_dev_flag(pcr, PM_L1_2_EN);
rts5260_pwr_saving_setting(pcr);
if (option->ltr_en) { - u16 val; - - pcie_capability_read_word(pdev, PCI_EXP_DEVCTL2, &val); - if (val & PCI_EXP_DEVCTL2_LTR_EN) { - option->ltr_enabled = true; - option->ltr_active = true; + if (option->ltr_enabled) rtsx_set_ltr_latency(pcr, option->ltr_active_latency); - } else { - option->ltr_enabled = false; - } } }
static int rts5260_extra_init_hw(struct rtsx_pcr *pcr) { + struct rtsx_cr_option *option = &pcr->option;
/* Set mcu_cnt to 7 to ensure data can be sampled properly */ rtsx_pci_write_register(pcr, 0xFC03, 0x7F, 0x07); @@ -539,6 +511,17 @@ static int rts5260_extra_init_hw(struct rtsx_pcr *pcr)
rts5260_init_hw(pcr);
+ /* + * If u_force_clkreq_0 is enabled, CLKREQ# PIN will be forced + * to drive low, and we forcibly request clock. + */ + if (option->force_clkreq_0) + rtsx_pci_write_register(pcr, PETXCFG, + FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_LOW); + else + rtsx_pci_write_register(pcr, PETXCFG, + FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_HIGH); + rtsx_pci_write_register(pcr, pcr->reg_pm_ctrl3, 0x10, 0x00);
return 0; diff --git a/drivers/misc/cardreader/rts5261.c b/drivers/misc/cardreader/rts5261.c index 94af6bf8a25a..67252512a132 100644 --- a/drivers/misc/cardreader/rts5261.c +++ b/drivers/misc/cardreader/rts5261.c @@ -454,54 +454,17 @@ static void rts5261_init_from_hw(struct rtsx_pcr *pcr)
static void rts5261_init_from_cfg(struct rtsx_pcr *pcr) { - struct pci_dev *pdev = pcr->pci; - int l1ss; - u32 lval; struct rtsx_cr_option *option = &pcr->option;
- l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS); - if (!l1ss) - return; - - pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, &lval); - - if (lval & PCI_L1SS_CTL1_ASPM_L1_1) - rtsx_set_dev_flag(pcr, ASPM_L1_1_EN); - else - rtsx_clear_dev_flag(pcr, ASPM_L1_1_EN); - - if (lval & PCI_L1SS_CTL1_ASPM_L1_2) - rtsx_set_dev_flag(pcr, ASPM_L1_2_EN); - else - rtsx_clear_dev_flag(pcr, ASPM_L1_2_EN); - - if (lval & PCI_L1SS_CTL1_PCIPM_L1_1) - rtsx_set_dev_flag(pcr, PM_L1_1_EN); - else - rtsx_clear_dev_flag(pcr, PM_L1_1_EN); - - if (lval & PCI_L1SS_CTL1_PCIPM_L1_2) - rtsx_set_dev_flag(pcr, PM_L1_2_EN); - else - rtsx_clear_dev_flag(pcr, PM_L1_2_EN); - - rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, 0xFF, 0); if (option->ltr_en) { - u16 val; - - pcie_capability_read_word(pdev, PCI_EXP_DEVCTL2, &val); - if (val & PCI_EXP_DEVCTL2_LTR_EN) { - option->ltr_enabled = true; - option->ltr_active = true; + if (option->ltr_enabled) rtsx_set_ltr_latency(pcr, option->ltr_active_latency); - } else { - option->ltr_enabled = false; - } } }
static int rts5261_extra_init_hw(struct rtsx_pcr *pcr) { + struct rtsx_cr_option *option = &pcr->option; u32 val;
rtsx_pci_write_register(pcr, RTS5261_AUTOLOAD_CFG1, @@ -547,6 +510,17 @@ static int rts5261_extra_init_hw(struct rtsx_pcr *pcr) else rtsx_pci_write_register(pcr, PETXCFG, 0x30, 0x00);
+ /* + * If u_force_clkreq_0 is enabled, CLKREQ# PIN will be forced + * to drive low, and we forcibly request clock. + */ + if (option->force_clkreq_0) + rtsx_pci_write_register(pcr, PETXCFG, + FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_LOW); + else + rtsx_pci_write_register(pcr, PETXCFG, + FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_HIGH); + rtsx_pci_write_register(pcr, PWD_SUSPEND_EN, 0xFF, 0xFB);
if (pcr->rtd3_en) { diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c index a3f4b52bb159..a30751ad3733 100644 --- a/drivers/misc/cardreader/rtsx_pcr.c +++ b/drivers/misc/cardreader/rtsx_pcr.c @@ -1326,11 +1326,8 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr) return err; }
- if (pcr->aspm_mode == ASPM_MODE_REG) { + if (pcr->aspm_mode == ASPM_MODE_REG) rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, 0x30, 0x30); - rtsx_pci_write_register(pcr, PETXCFG, - FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_HIGH); - }
/* No CD interrupt if probing driver with card inserted. * So we need to initialize pcr->card_exist here. @@ -1345,7 +1342,9 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr)
static int rtsx_pci_init_chip(struct rtsx_pcr *pcr) { - int err; + struct rtsx_cr_option *option = &(pcr->option); + int err, l1ss; + u32 lval; u16 cfg_val; u8 val;
@@ -1430,6 +1429,48 @@ static int rtsx_pci_init_chip(struct rtsx_pcr *pcr) pcr->aspm_enabled = true; }
+ l1ss = pci_find_ext_capability(pcr->pci, PCI_EXT_CAP_ID_L1SS); + if (l1ss) { + pci_read_config_dword(pcr->pci, l1ss + PCI_L1SS_CTL1, &lval); + + if (lval & PCI_L1SS_CTL1_ASPM_L1_1) + rtsx_set_dev_flag(pcr, ASPM_L1_1_EN); + else + rtsx_clear_dev_flag(pcr, ASPM_L1_1_EN); + + if (lval & PCI_L1SS_CTL1_ASPM_L1_2) + rtsx_set_dev_flag(pcr, ASPM_L1_2_EN); + else + rtsx_clear_dev_flag(pcr, ASPM_L1_2_EN); + + if (lval & PCI_L1SS_CTL1_PCIPM_L1_1) + rtsx_set_dev_flag(pcr, PM_L1_1_EN); + else + rtsx_clear_dev_flag(pcr, PM_L1_1_EN); + + if (lval & PCI_L1SS_CTL1_PCIPM_L1_2) + rtsx_set_dev_flag(pcr, PM_L1_2_EN); + else + rtsx_clear_dev_flag(pcr, PM_L1_2_EN); + + pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &cfg_val); + if (cfg_val & PCI_EXP_DEVCTL2_LTR_EN) { + option->ltr_enabled = true; + option->ltr_active = true; + } else { + option->ltr_enabled = false; + } + + if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN + | PM_L1_1_EN | PM_L1_2_EN)) + option->force_clkreq_0 = false; + else + option->force_clkreq_0 = true; + } else { + option->ltr_enabled = false; + option->force_clkreq_0 = true; + } + if (pcr->ops->fetch_vendor_settings) pcr->ops->fetch_vendor_settings(pcr);
On Wed, Sep 20, 2023 at 07:30:00AM +0000, Ricky WU wrote:
Hi Greg k-h,
This patch is our solution for this issue... And now how can I push this?
Submit it properly like any other patch, what is preventing that from happening?
(commit 8ee39ec) some reader no longer force #CLKREQ to low when system need to enter ASPM. But some platform maybe not implement complete ASPM? I don't know..... it causes problems...
Like in the past Only the platform support L1ss we release the #CLKREQ. But new patch we move the judgment (L1ss) to probe, because we met some host will clean the config space from S3 or some power saving mode And also we think just to read config space one time when the driver start is enough
thanks,
greg k-h
Hi
[apologies if I missed some followup but was not able to find the answer]
On Wed, Sep 20, 2023 at 08:32:17AM +0000, Ricky WU wrote:
On Wed, Sep 20, 2023 at 07:30:00AM +0000, Ricky WU wrote:
Hi Greg k-h,
This patch is our solution for this issue... And now how can I push this?
Submit it properly like any other patch, what is preventing that from happening?
(commit 8ee39ec) some reader no longer force #CLKREQ to low when system need to enter ASPM. But some platform maybe not implement complete ASPM? I don't know..... it causes problems...
Like in the past Only the platform support L1ss we release the #CLKREQ. But new patch we move the judgment (L1ss) to probe, because we met some host will clean the config space from S3 or some power saving mode And also we think just to read config space one time when the driver start is enough
Is there a potential fix which is queued for this or would be the safest option to unbreak the regression to revert the commit in the stable trees temporarily?
I'm asking because in Debian we got the report at https://bugs.debian.org/1052063
(and ideally to unbreak the situation for the user I would like to include a fix in the next upload we do, but following what you as upstream will do ideally).
Regards, Salvatore
On 26.09.23 13:23, Salvatore Bonaccorso wrote:
On Wed, Sep 20, 2023 at 08:32:17AM +0000, Ricky WU wrote:
On Wed, Sep 20, 2023 at 07:30:00AM +0000, Ricky WU wrote:
This patch is our solution for this issue... And now how can I push this?
Submit it properly like any other patch, what is preventing that from happening?
(commit 8ee39ec) some reader no longer force #CLKREQ to low when system need to enter ASPM. But some platform maybe not implement complete ASPM? I don't know..... it causes problems...
Like in the past Only the platform support L1ss we release the #CLKREQ. But new patch we move the judgment (L1ss) to probe, because we met some host will clean the config space from S3 or some power saving mode And also we think just to read config space one time when the driver start is enough
Is there a potential fix which is queued for this or would be the safest option to unbreak the regression to revert the commit in the stable trees temporarily?
I'm asking because in Debian we got the report at https://bugs.debian.org/1052063
(and ideally to unbreak the situation for the user I would like to include a fix in the next upload we do, but following what you as upstream will do ideally).
A fix is out for review and will likely be merged for mainline soon: https://lore.kernel.org/all/37b1afb997f14946a8784c73d1f9a4f5@realtek.com/ https://lore.kernel.org/all/2023092522-climatic-commend-8c99@gregkh/
A few distros reverted the culprit for now in their stable trees.
HTH, Ciao, Thorsten
Hi Ricky, I'm going to test it right now! Thanks!
On Wed, Sep 20 2023 at 07:30:00 +00:00:00, Ricky WU ricky_wu@realtek.com wrote:
Hi Greg k-h,
This patch is our solution for this issue... And now how can I push this?
Hi Paul，
Can you help to try this patch? Is it work for your platform?
Ricky
drivers/misc/cardreader/rts5227.c | 55 ++++------------------------ drivers/misc/cardreader/rts5228.c | 57 +++++++++--------------------- drivers/misc/cardreader/rts5249.c | 56 ++++------------------------- drivers/misc/cardreader/rts5260.c | 43 +++++++--------------- drivers/misc/cardreader/rts5261.c | 52 +++++++-------------------- drivers/misc/cardreader/rtsx_pcr.c | 51 +++++++++++++++++++++++--- 6 files changed, 102 insertions(+), 212 deletions(-)
diff --git a/drivers/misc/cardreader/rts5227.c b/drivers/misc/cardreader/rts5227.c index 3dae5e3a1697..cd512284bfb3 100644 --- a/drivers/misc/cardreader/rts5227.c +++ b/drivers/misc/cardreader/rts5227.c @@ -83,63 +83,20 @@ static void rts5227_fetch_vendor_settings(struct rtsx_pcr *pcr)
static void rts5227_init_from_cfg(struct rtsx_pcr *pcr) {
struct pci_dev *pdev = pcr->pci;
int l1ss;
u32 lval; struct rtsx_cr_option *option = &pcr->option;
l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
if (!l1ss)
return;pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, &lval);
if (CHK_PCI_PID(pcr, 0x522A)) {
if (0 == (lval & 0x0F))rtsx_pci_enable_oobs_polling(pcr);else
if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN| PM_L1_1_EN | PM_L1_2_EN)) rtsx_pci_disable_oobs_polling(pcr);elsertsx_pci_enable_oobs_polling(pcr);
- if (lval & PCI_L1SS_CTL1_ASPM_L1_1)
rtsx_set_dev_flag(pcr, ASPM_L1_1_EN);- else
rtsx_clear_dev_flag(pcr, ASPM_L1_1_EN);- if (lval & PCI_L1SS_CTL1_ASPM_L1_2)
rtsx_set_dev_flag(pcr, ASPM_L1_2_EN);- else
rtsx_clear_dev_flag(pcr, ASPM_L1_2_EN);- if (lval & PCI_L1SS_CTL1_PCIPM_L1_1)
rtsx_set_dev_flag(pcr, PM_L1_1_EN);- else
rtsx_clear_dev_flag(pcr, PM_L1_1_EN);- if (lval & PCI_L1SS_CTL1_PCIPM_L1_2)
rtsx_set_dev_flag(pcr, PM_L1_2_EN);- else
rtsx_clear_dev_flag(pcr, PM_L1_2_EN);- if (option->ltr_en) {
u16 val;pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &val);if (val & PCI_EXP_DEVCTL2_LTR_EN) {option->ltr_enabled = true;option->ltr_active = true;
if (option->ltr_enabled) rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
} else {option->ltr_enabled = false;}- if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN
| PM_L1_1_EN | PM_L1_2_EN))option->force_clkreq_0 = false;- else
option->force_clkreq_0 = true;}
static int rts5227_extra_init_hw(struct rtsx_pcr *pcr) @@ -195,7 +152,7 @@ static int rts5227_extra_init_hw(struct rtsx_pcr *pcr) } }
- if (option->force_clkreq_0 && pcr->aspm_mode == ASPM_MODE_CFG)
- if (option->force_clkreq_0) rtsx_pci_add_cmd(pcr, WRITE_REG_CMD, PETXCFG, FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_LOW); else
diff --git a/drivers/misc/cardreader/rts5228.c b/drivers/misc/cardreader/rts5228.c index f4ab09439da7..0c7f10bcf6f1 100644 --- a/drivers/misc/cardreader/rts5228.c +++ b/drivers/misc/cardreader/rts5228.c @@ -386,59 +386,25 @@ static void rts5228_process_ocp(struct rtsx_pcr *pcr)
static void rts5228_init_from_cfg(struct rtsx_pcr *pcr) {
struct pci_dev *pdev = pcr->pci;
int l1ss;
u32 lval; struct rtsx_cr_option *option = &pcr->option;
l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
if (!l1ss)
return;pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, &lval);
if (0 == (lval & 0x0F))
rtsx_pci_enable_oobs_polling(pcr);else
- if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN
| PM_L1_1_EN | PM_L1_2_EN))
- if (lval & PCI_L1SS_CTL1_ASPM_L1_1)
rtsx_set_dev_flag(pcr, ASPM_L1_1_EN);- else
rtsx_clear_dev_flag(pcr, ASPM_L1_1_EN);- if (lval & PCI_L1SS_CTL1_ASPM_L1_2)
rtsx_set_dev_flag(pcr, ASPM_L1_2_EN);- else
rtsx_clear_dev_flag(pcr, ASPM_L1_2_EN);- if (lval & PCI_L1SS_CTL1_PCIPM_L1_1)
rtsx_set_dev_flag(pcr, PM_L1_1_EN);rtsx_clear_dev_flag(pcr, PM_L1_1_EN);- if (lval & PCI_L1SS_CTL1_PCIPM_L1_2)
rtsx_set_dev_flag(pcr, PM_L1_2_EN);- else
rtsx_clear_dev_flag(pcr, PM_L1_2_EN);
rtsx_pci_enable_oobs_polling(pcr);rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, 0xFF, 0);
if (option->ltr_en) {
u16 val;pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &val);if (val & PCI_EXP_DEVCTL2_LTR_EN) {option->ltr_enabled = true;option->ltr_active = true;
- if (option->ltr_en) {
if (option->ltr_enabled) rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
} else {option->ltr_enabled = false;}}
static int rts5228_extra_init_hw(struct rtsx_pcr *pcr) {
struct rtsx_cr_option *option = &pcr->option;
rtsx_pci_write_register(pcr, RTS5228_AUTOLOAD_CFG1, CD_RESUME_EN_MASK, CD_RESUME_EN_MASK);
@@ -469,6 +435,17 @@ static int rts5228_extra_init_hw(struct rtsx_pcr *pcr) else rtsx_pci_write_register(pcr, PETXCFG, 0x30, 0x00);
/*
* If u_force_clkreq_0 is enabled, CLKREQ# PIN will be forced* to drive low, and we forcibly request clock.*/if (option->force_clkreq_0)
rtsx_pci_write_register(pcr, PETXCFG,FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_LOW);else
rtsx_pci_write_register(pcr, PETXCFG,FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_HIGH);rtsx_pci_write_register(pcr, PWD_SUSPEND_EN, 0xFF, 0xFB);
if (pcr->rtd3_en) {
diff --git a/drivers/misc/cardreader/rts5249.c b/drivers/misc/cardreader/rts5249.c index 47ab72a43256..6c81040e18be 100644 --- a/drivers/misc/cardreader/rts5249.c +++ b/drivers/misc/cardreader/rts5249.c @@ -86,64 +86,22 @@ static void rtsx_base_fetch_vendor_settings(struct rtsx_pcr *pcr)
static void rts5249_init_from_cfg(struct rtsx_pcr *pcr) {
struct pci_dev *pdev = pcr->pci;
int l1ss; struct rtsx_cr_option *option = &(pcr->option);
u32 lval;
l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
if (!l1ss)
return;pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, &lval);
if (CHK_PCI_PID(pcr, PID_524A) || CHK_PCI_PID(pcr, PID_525A)) {
if (0 == (lval & 0x0F))rtsx_pci_enable_oobs_polling(pcr);else
if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN| PM_L1_1_EN | PM_L1_2_EN)) rtsx_pci_disable_oobs_polling(pcr);elsertsx_pci_enable_oobs_polling(pcr);
- if (lval & PCI_L1SS_CTL1_ASPM_L1_1)
rtsx_set_dev_flag(pcr, ASPM_L1_1_EN);- if (lval & PCI_L1SS_CTL1_ASPM_L1_2)
rtsx_set_dev_flag(pcr, ASPM_L1_2_EN);- if (lval & PCI_L1SS_CTL1_PCIPM_L1_1)
rtsx_set_dev_flag(pcr, PM_L1_1_EN);- if (lval & PCI_L1SS_CTL1_PCIPM_L1_2)
rtsx_set_dev_flag(pcr, PM_L1_2_EN);- if (option->ltr_en) {
u16 val;pcie_capability_read_word(pdev, PCI_EXP_DEVCTL2, &val);if (val & PCI_EXP_DEVCTL2_LTR_EN) {option->ltr_enabled = true;option->ltr_active = true;
if (option->ltr_enabled) rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
} else {option->ltr_enabled = false;}}
-static int rts5249_init_from_hw(struct rtsx_pcr *pcr) -{
- struct rtsx_cr_option *option = &(pcr->option);
- if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN
| PM_L1_1_EN | PM_L1_2_EN))option->force_clkreq_0 = false;- else
option->force_clkreq_0 = true;- return 0;
-}
static void rts52xa_force_power_down(struct rtsx_pcr *pcr, u8 pm_state, bool runtime) { /* Set relink_time to 0 */ @@ -276,7 +234,6 @@ static int rts5249_extra_init_hw(struct rtsx_pcr *pcr) struct rtsx_cr_option *option = &(pcr->option);
rts5249_init_from_cfg(pcr);
rts5249_init_from_hw(pcr);
rtsx_pci_init_cmd(pcr);
@@ -327,11 +284,12 @@ static int rts5249_extra_init_hw(struct rtsx_pcr *pcr) } }
- /*
*/
- If u_force_clkreq_0 is enabled, CLKREQ# PIN will be forced
- to drive low, and we forcibly request clock.
- if (option->force_clkreq_0 && pcr->aspm_mode == ASPM_MODE_CFG)
- if (option->force_clkreq_0) rtsx_pci_write_register(pcr, PETXCFG, FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_LOW); else
diff --git a/drivers/misc/cardreader/rts5260.c b/drivers/misc/cardreader/rts5260.c index 79b18f6f73a8..d2d3a6ccb8f7 100644 --- a/drivers/misc/cardreader/rts5260.c +++ b/drivers/misc/cardreader/rts5260.c @@ -480,47 +480,19 @@ static void rts5260_pwr_saving_setting(struct rtsx_pcr *pcr)
static void rts5260_init_from_cfg(struct rtsx_pcr *pcr) {
struct pci_dev *pdev = pcr->pci;
int l1ss; struct rtsx_cr_option *option = &pcr->option;
u32 lval;
l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
if (!l1ss)
return;pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, &lval);
if (lval & PCI_L1SS_CTL1_ASPM_L1_1)
rtsx_set_dev_flag(pcr, ASPM_L1_1_EN);if (lval & PCI_L1SS_CTL1_ASPM_L1_2)
rtsx_set_dev_flag(pcr, ASPM_L1_2_EN);if (lval & PCI_L1SS_CTL1_PCIPM_L1_1)
rtsx_set_dev_flag(pcr, PM_L1_1_EN);if (lval & PCI_L1SS_CTL1_PCIPM_L1_2)
rtsx_set_dev_flag(pcr, PM_L1_2_EN);rts5260_pwr_saving_setting(pcr);
if (option->ltr_en) {
u16 val;pcie_capability_read_word(pdev, PCI_EXP_DEVCTL2, &val);if (val & PCI_EXP_DEVCTL2_LTR_EN) {option->ltr_enabled = true;option->ltr_active = true;
if (option->ltr_enabled) rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
} else {option->ltr_enabled = false;}}
static int rts5260_extra_init_hw(struct rtsx_pcr *pcr) {
struct rtsx_cr_option *option = &pcr->option;
/* Set mcu_cnt to 7 to ensure data can be sampled properly */ rtsx_pci_write_register(pcr, 0xFC03, 0x7F, 0x07);
@@ -539,6 +511,17 @@ static int rts5260_extra_init_hw(struct rtsx_pcr *pcr)
rts5260_init_hw(pcr);
/*
* If u_force_clkreq_0 is enabled, CLKREQ# PIN will be forced* to drive low, and we forcibly request clock.*/if (option->force_clkreq_0)
rtsx_pci_write_register(pcr, PETXCFG,FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_LOW);else
rtsx_pci_write_register(pcr, PETXCFG,FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_HIGH);rtsx_pci_write_register(pcr, pcr->reg_pm_ctrl3, 0x10, 0x00);
return 0;
diff --git a/drivers/misc/cardreader/rts5261.c b/drivers/misc/cardreader/rts5261.c index 94af6bf8a25a..67252512a132 100644 --- a/drivers/misc/cardreader/rts5261.c +++ b/drivers/misc/cardreader/rts5261.c @@ -454,54 +454,17 @@ static void rts5261_init_from_hw(struct rtsx_pcr *pcr)
static void rts5261_init_from_cfg(struct rtsx_pcr *pcr) {
struct pci_dev *pdev = pcr->pci;
int l1ss;
u32 lval; struct rtsx_cr_option *option = &pcr->option;
l1ss = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_L1SS);
if (!l1ss)
return;pci_read_config_dword(pdev, l1ss + PCI_L1SS_CTL1, &lval);
if (lval & PCI_L1SS_CTL1_ASPM_L1_1)
rtsx_set_dev_flag(pcr, ASPM_L1_1_EN);else
rtsx_clear_dev_flag(pcr, ASPM_L1_1_EN);if (lval & PCI_L1SS_CTL1_ASPM_L1_2)
rtsx_set_dev_flag(pcr, ASPM_L1_2_EN);else
rtsx_clear_dev_flag(pcr, ASPM_L1_2_EN);if (lval & PCI_L1SS_CTL1_PCIPM_L1_1)
rtsx_set_dev_flag(pcr, PM_L1_1_EN);else
rtsx_clear_dev_flag(pcr, PM_L1_1_EN);if (lval & PCI_L1SS_CTL1_PCIPM_L1_2)
rtsx_set_dev_flag(pcr, PM_L1_2_EN);else
rtsx_clear_dev_flag(pcr, PM_L1_2_EN);rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, 0xFF, 0); if (option->ltr_en) {
u16 val;pcie_capability_read_word(pdev, PCI_EXP_DEVCTL2, &val);if (val & PCI_EXP_DEVCTL2_LTR_EN) {option->ltr_enabled = true;option->ltr_active = true;
if (option->ltr_enabled) rtsx_set_ltr_latency(pcr, option->ltr_active_latency);
} else {option->ltr_enabled = false;}}
static int rts5261_extra_init_hw(struct rtsx_pcr *pcr) {
struct rtsx_cr_option *option = &pcr->option; u32 val;
rtsx_pci_write_register(pcr, RTS5261_AUTOLOAD_CFG1,
@@ -547,6 +510,17 @@ static int rts5261_extra_init_hw(struct rtsx_pcr *pcr) else rtsx_pci_write_register(pcr, PETXCFG, 0x30, 0x00);
/*
* If u_force_clkreq_0 is enabled, CLKREQ# PIN will be forced* to drive low, and we forcibly request clock.*/if (option->force_clkreq_0)
rtsx_pci_write_register(pcr, PETXCFG,FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_LOW);else
rtsx_pci_write_register(pcr, PETXCFG,FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_HIGH);rtsx_pci_write_register(pcr, PWD_SUSPEND_EN, 0xFF, 0xFB);
if (pcr->rtd3_en) {
diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c index a3f4b52bb159..a30751ad3733 100644 --- a/drivers/misc/cardreader/rtsx_pcr.c +++ b/drivers/misc/cardreader/rtsx_pcr.c @@ -1326,11 +1326,8 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr) return err; }
- if (pcr->aspm_mode == ASPM_MODE_REG) {
- if (pcr->aspm_mode == ASPM_MODE_REG) rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, 0x30, 0x30);
rtsx_pci_write_register(pcr, PETXCFG,FORCE_CLKREQ_DELINK_MASK, FORCE_CLKREQ_HIGH);}
/* No CD interrupt if probing driver with card inserted.
- So we need to initialize pcr->card_exist here.
@@ -1345,7 +1342,9 @@ static int rtsx_pci_init_hw(struct rtsx_pcr *pcr)
static int rtsx_pci_init_chip(struct rtsx_pcr *pcr) {
- int err;
- struct rtsx_cr_option *option = &(pcr->option);
- int err, l1ss;
- u32 lval; u16 cfg_val; u8 val;
@@ -1430,6 +1429,48 @@ static int rtsx_pci_init_chip(struct rtsx_pcr *pcr) pcr->aspm_enabled = true; }
- l1ss = pci_find_ext_capability(pcr->pci, PCI_EXT_CAP_ID_L1SS);
- if (l1ss) {
pci_read_config_dword(pcr->pci, l1ss + PCI_L1SS_CTL1, &lval);if (lval & PCI_L1SS_CTL1_ASPM_L1_1)rtsx_set_dev_flag(pcr, ASPM_L1_1_EN);elsertsx_clear_dev_flag(pcr, ASPM_L1_1_EN);if (lval & PCI_L1SS_CTL1_ASPM_L1_2)rtsx_set_dev_flag(pcr, ASPM_L1_2_EN);elsertsx_clear_dev_flag(pcr, ASPM_L1_2_EN);if (lval & PCI_L1SS_CTL1_PCIPM_L1_1)rtsx_set_dev_flag(pcr, PM_L1_1_EN);elsertsx_clear_dev_flag(pcr, PM_L1_1_EN);if (lval & PCI_L1SS_CTL1_PCIPM_L1_2)rtsx_set_dev_flag(pcr, PM_L1_2_EN);elsertsx_clear_dev_flag(pcr, PM_L1_2_EN);pcie_capability_read_word(pcr->pci, PCI_EXP_DEVCTL2, &cfg_val);if (cfg_val & PCI_EXP_DEVCTL2_LTR_EN) {option->ltr_enabled = true;option->ltr_active = true;} else {option->ltr_enabled = false;}if (rtsx_check_dev_flag(pcr, ASPM_L1_1_EN | ASPM_L1_2_EN| PM_L1_1_EN | PM_L1_2_EN))option->force_clkreq_0 = false;elseoption->force_clkreq_0 = true;- } else {
option->ltr_enabled = false;option->force_clkreq_0 = true;- }
- if (pcr->ops->fetch_vendor_settings) pcr->ops->fetch_vendor_settings(pcr);
-- 2.25.1
-----Original Message----- From: Greg KH gregkh@linuxfoundation.org Sent: Tuesday, September 19, 2023 3:06 PM To: Ricky WU ricky_wu@realtek.com Cc: Linux regressions mailing list regressions@lists.linux.dev; Paul Grandperrin paul.grandperrin@gmail.com; stable@vger.kernel.org; Wei_wang wei_wang@realsil.com.cn; Roger Tseng rogerable@realtek.com; Linus Torvalds torvalds@linux-foundation.org Subject: Re: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot
External mail.
On Tue, Sep 19, 2023 at 02:20:53AM +0000, Ricky WU wrote:
Hi Greg k-h，
In order to cover the those platform Power saving issue, our
approach
on new patch will be different from the previous patch
(101bd907b4244a726980ee67f95ed9cafab6ff7a).
So we need used fixed Tag on
101bd907b4244a726980ee67f95ed9cafab6ff7a
or a new patch for this problem?
I'm sorry, but I do not understand. I think a new change is needed here, right? Or are you wanting a different commit backported to stable trees? What about Linus's tree now?
What exactly should I do here?
confused,
greg k-h
Hi Paul,
We have notice this issue.... https://lore.kernel.org/lkml/fa82d9dcbe83403abc644c20922b47f9@realtek.com/t/... main reason is: "it is a system power saving issue.... In the past if the BIOS(config space) not set L1-substate our driver will keep drive low CLKREQ# when HOST want to enter power saving state that make whole system not enter the power saving state. But this patch we release the CLKREQ# to HOST, make whole system can enter power saving state success when the HOST want to enter the power saving state, but I don't know why this system can not wake out success from power saving state"
This is a PCIE CLKREQ# design problem on those platform, the pcie spec allow device release the CLKREQ# to HOST, this patch only do this....
We are thinking about how to compatible with these potentially problematic platforms
Ricky
-----Original Message----- From: Paul Grandperrin paul.grandperrin@gmail.com Sent: Tuesday, September 12, 2023 8:29 PM To: stable@vger.kernel.org Cc: regressions@lists.linux.dev; Wei_wang wei_wang@realsil.com.cn; Roger Tseng rogerable@realtek.com; Ricky WU ricky_wu@realtek.com Subject: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot
External mail.
Hi kernel maintainers!
My computer doesn't boot with kernels newer than 6.1.45.
Here's what happens:
- system boots in initramfs
- detects my encrypted ZFS pool and asks for password
- mount system, pivots to it, starts real init
- before any daemon had time to start, the system hangs and the kernel
writes on the console "nvme 0000:04:00.0: Unable to change power state from D3cold to D0, device inaccessible"
- if I reboot directly without powering off (using magic sysrq or
panic=10), even the UEFI complains about not finding any storage to boot from.
- after a real power off, I can boot using a kernel <= 6.1.45.
The bug has been discussed here: https://bugzilla.kernel.org/show_bug.cgi?id=217705
My laptop is a Dell XPS 15 9560 (Intel 7700hq).
I bisected between 6.1.45 and 6.1.46 and found this commit
commit 8ee39ec479147e29af704639f8e55fce246ed2d9 Author: Ricky WU ricky_wu@realtek.com Date: Tue Jul 25 09:10:54 2023 +0000
misc: rtsx: judge ASPM Mode to set PETXCFG Reg commit 101bd907b4244a726980ee67f95ed9cafab6ff7a upstream. ASPM Mode is ASPM_MODE_CFG need to judge the value of clkreq_0 to set HIGH or LOW, if the ASPM Mode is ASPM_MODE_REG always set to HIGH during the initialization. Cc: stable@vger.kernel.org Signed-off-by: Ricky Wu <ricky_wu@realtek.com> Link:https://lore.kernel.org/r/52906c6836374c8cb068225954c5543a@realtek.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
drivers/misc/cardreader/rts5227.c | 2 +- drivers/misc/cardreader/rts5228.c | 18 ------------------ drivers/misc/cardreader/rts5249.c | 3 +-- drivers/misc/cardreader/rts5260.c | 18 ------------------ drivers/misc/cardreader/rts5261.c | 18 ------------------ drivers/misc/cardreader/rtsx_pcr.c | 5 ++++- 6 files changed, 6 insertions(+), 58 deletions(-)
If I build 6.1.51 with this commit reverted, my laptop works again, confirming that this commit is to blame.
Also, blacklisting `rtsx_pci_sdmmc` and `rtsx_pci`, while preventing to use the sd card reading, allows to boot the system.
I can't try 6.4 or 6.5 because my system is dependent on ZFS..
Have a nice day, Paul Grandperrin
Hi,
In the past if the BIOS(config space) not set L1-substate our driver will keep drive low CLKREQ# when HOST want to enter power saving state that make whole system not enter the power saving state. But this patch we release the CLKREQ# to HOST, make whole system can enter power saving state success when the HOST want to enter the power saving state, but I don't know why this system can not wake out success from power saving state"
This is a PCIE CLKREQ# design problem on those platform, the pcie spec allow device release the CLKREQ# to HOST, this patch only do this....
I spent some time debugging today but I am not a PCIe expert. I think that the card reader is actually violating the PCIe spec by not forcing CLKREQ# low on systems that don't support ASPM, as appears to be done (accidentally?) by the regressing driver change.
The kernel logs on the affected system states the following: [ 0.142326] ACPI FADT declares the system doesn't support PCIe ASPM, so disable it
The PCIe 3.0 spec states (in the description of the Link Control Register), regarding enabling clock power management:
Enable Clock Power Management – Applicable only for Upstream Ports and with form factors that support a “Clock Request” (CLKREQ#) mechanism, this bit operates as follows: 0b Clock power management is disabled and device must hold CLKREQ# signal low. 1b When this bit is Set, the device is permitted to use CLKREQ# signal to power manage Link clock according to protocol defined in appropriate form factor specification.
My reading of this is that on this system which does not support ASPM and therefore also does not support clock PM, the driver must have the device hold the line low, but I may be wrong.
It's still unclear to me based on studying the schematic of the laptop and the PCH datasheet why the one PCIe port is able to break the other one like it does. The CLKREQ# lines are simply connected directly to the SRCCLKREQ# lines of the PCH, plus a 10k pull-up to 3v3, which seems entirely reasonable; any breakage surely would be some software/firmware level misconfiguration.
Jade
Hi Jade，
I think you have some misunderstand, our set clkreg register is not go directly to control CLKREQ# after setting.... Our device keep CLKREQ# low until enter ASPM, so the system let our device to ASPM mode then we release this CLKREQ# pin
In the past if the BIOS(config space) not set L1-substate our driver will keep
drive low CLKREQ# when HOST want to enter power saving state that make whole system not enter the power saving state.
But this patch we release the CLKREQ# to HOST, make whole system can
enter power saving state success when the HOST want to enter the power saving state, but I don't know why this system can not wake out success from power saving state"
This is a PCIE CLKREQ# design problem on those platform, the pcie spec
allow device release the CLKREQ# to HOST, this patch only do this....
I spent some time debugging today but I am not a PCIe expert. I think
that the card reader is actually violating the PCIe spec by not forcing
CLKREQ# low on systems that don't support ASPM, as appears to be done
(accidentally?) by the regressing driver change.
The kernel logs on the affected system states the following:
[ 0.142326] ACPI FADT declares the system doesn't support PCIe ASPM, so disable it
The PCIe 3.0 spec states (in the description of the Link Control
Register), regarding enabling clock power management:
Enable Clock Power Management – Applicable only for Upstream Ports and
with form factors that support a “Clock Request” (CLKREQ#) mechanism, this bit operates as follows:
0b Clock power management is disabled and device must hold CLKREQ#
signal low.
1b When this bit is Set, the device is permitted to use CLKREQ# signal to
power manage Link clock
according to protocol defined in appropriate form factor specification.
My reading of this is that on this system which does not support ASPM
and therefore also does not support clock PM, the driver must have the
device hold the line low, but I may be wrong.
It's still unclear to me based on studying the schematic of the laptop
and the PCH datasheet why the one PCIe port is able to break the other
one like it does. The CLKREQ# lines are simply connected directly to the
SRCCLKREQ# lines of the PCH, plus a 10k pull-up to 3v3, which seems
entirely reasonable; any breakage surely would be some software/firmware level
misconfiguration.
Jade
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Jade Lovelace -
Linux regression tracking (Thorsten Leemhuis) -
Paul Grandperrin -
Ricky WU -
Salvatore Bonaccorso