Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Paul Grandperrin]

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

Re: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Linux regression tracking (Thorsten Leemhuis)]

(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.com/

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/1b02b1528a26f4e9b577e215c114d8c5e773ee10

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
> 

Re: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Greg KH]

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

RE: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Ricky WU]

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

Re: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Greg KH]

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

RE: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Ricky WU]

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

Re: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Greg KH]

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?

thanks,

greg k-h

RE: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Ricky WU]

> 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

Re: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Salvatore Bonaccorso]

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

Re: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Linux regression tracking (Thorsten Leemhuis)]

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

RE: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Paul Grandperrin]

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);
> +		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);
> 
> --
> 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

RE: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Ricky WU]

Hi Paul,

We have notice this issue....
https://lore.kernel.org/lkml/fa82d9dcbe83403abc644c20922b47f9@realtek.com/t/#m90373a16017f07ca32f43d6b4327164fb31bf9bb
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
> 

RE: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Jade Lovelace]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2199 bytes --]

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

RE: Regression since 6.1.46 (commit 8ee39ec): rtsx_pci from drivers/misc/cardreader breaks NVME power state, preventing system boot

[Ricky WU]

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