* [PATCH 1/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"
2024-05-13 13:53 [PATCH] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list" dev
@ 2024-05-13 13:53 ` dev
2024-05-15 17:47 ` Niklas Cassel
2024-05-15 17:19 ` [PATCH] " Niklas Cassel
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: dev @ 2024-05-13 13:53 UTC (permalink / raw)
To: dlemoal, cassel; +Cc: linux-ide
From: Jason Nader <dev@kayoway.com>
Commit b8b8b4e0c052b2c06e1c4820a8001f4e0f77900f ("ata: ahci: Add Intel
Alder Lake-P AHCI controller to low power chipsets list") enabled low
power mode for Alder Lake-P AHCI adaptors in order to reduce idle power
consumption, however this introduced a regression on at least one system.
Revert the patch until a better solution is found.
Signed-off-by: Jason Nader <dev@kayoway.com>
---
drivers/ata/ahci.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 6548f10e61d9..07d66d2c5f0d 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -429,7 +429,6 @@ static const struct pci_device_id ahci_pci_tbl[] = {
{ PCI_VDEVICE(INTEL, 0x02d7), board_ahci_pcs_quirk }, /* Comet Lake PCH RAID */
/* Elkhart Lake IDs 0x4b60 & 0x4b62 https://sata-io.org/product/8803 not tested yet */
{ PCI_VDEVICE(INTEL, 0x4b63), board_ahci_pcs_quirk }, /* Elkhart Lake AHCI */
- { PCI_VDEVICE(INTEL, 0x7ae2), board_ahci_pcs_quirk }, /* Alder Lake-P AHCI */
/* JMicron 360/1/3/5/6, match class to avoid IDE function */
{ PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
--
2.45.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"
2024-05-13 13:53 ` [PATCH 1/1] " dev
@ 2024-05-15 17:47 ` Niklas Cassel
0 siblings, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2024-05-15 17:47 UTC (permalink / raw)
To: dev; +Cc: dlemoal, linux-ide
On Mon, May 13, 2024 at 10:53:02PM +0900, dev@kayoway.com wrote:
> From: Jason Nader <dev@kayoway.com>
>
> Commit b8b8b4e0c052b2c06e1c4820a8001f4e0f77900f ("ata: ahci: Add Intel
> Alder Lake-P AHCI controller to low power chipsets list") enabled low
> power mode for Alder Lake-P AHCI adaptors in order to reduce idle power
> consumption, however this introduced a regression on at least one system.
> Revert the patch until a better solution is found.
The patch itself looks fine to me, but the commit message needs to be
rewritten.
Right now, we will enable LPM if the controller supports it
(unless the port is hot plug capable or external),
so we no longer have the "low power" board type.
Thus, it does not make sense to say that LPM is what introduced the
regression.
If v6.9 does not work, and v6.9 + this patch works, then the proper
commit message should be something like:
Commit b8b8b4e0c052 ("ata: ahci: Add Intel Alder Lake-P AHCI controller
to low power chipsets list") added Intel Alder Lake to the ahci_pci_tbl.
Because of the way that the Intel PCS quirk was implemented, having
an explicit entry in the ahci_pci_tbl caused the Intel PCS quirk to
be applied. (The quirk was not being applied if there was no explict
entry.)
Thus, entries that were added to the ahci_pci_tbl also got the Intel
PCS quirk applied.
The quirk was cleaned up in commit 7edbb6059274 ("ahci: clean up
intel_pcs_quirk"), such that it is clear which entries that actually
applies the Intel PCS quirk.
Newer Intel AHCI controllers do not need the Intel PCS quirk,
and applying it when not needed actually breaks some platforms.
Do not apply the Intel PCS quirk for Intel Alder Lake.
This is in line with how things worked before commit b8b8b4e0c052 ("ata:
ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"),
such that certain platforms using Intel Alder Lake will work once again.
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"
2024-05-13 13:53 [PATCH] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list" dev
2024-05-13 13:53 ` [PATCH 1/1] " dev
@ 2024-05-15 17:19 ` Niklas Cassel
2024-05-17 5:39 ` [PATCH v2 0/1] " dev
2024-05-21 13:36 ` [PATCH v3] ata: ahci: Do not apply Intel PCS quirk on Intel Alder Lake Jason Nader
3 siblings, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2024-05-15 17:19 UTC (permalink / raw)
To: dev; +Cc: dlemoal, linux-ide
Hello Jason,
On Mon, May 13, 2024 at 10:53:01PM +0900, dev@kayoway.com wrote:
> From: Jason Nader <dev@kayoway.com>
>
> Commit b8b8b4e0c052b2c06e1c4820a8001f4e0f77900f ("ata: ahci: Add Intel
> Alder Lake-P AHCI controller to low power chipsets list") enabled LPM for
> Alder Lake-P AHCI adaptors, however this introduced a regression on at
> least one system which causes the SATA ports to become unusable [1].
>
> The original commit stated it is for Alder Lake-P, which I understand is a
> mobile CPU, however the device ID added (0x7ae2) matches the one reported
> by my system which has an Alder Lake-S desktop CPU [2]. Searching for this
> device on other websites points to 0x7ae2 being for the desktop "-S"
> suffix [3] and not for the "-P" suffix, which is apparently 0x51d3 [4][5].
>
> Reverting this commit restores SATA port functionality on my system [6][7].
>
> [1] This Ubuntu bug report also appears to suffer from the same issue, so
> there are more affected systems out there:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2063229
>
> [2] System details:
> CPU: Intel i5-12400
> Motherboard: Biostar B660GTN
> BIOS Settings: Intel VMD off, SATA hot plug off, CSM off
> >lspci -nn -s 00:17
> 00:17.0 SATA controller [0106]: Intel Corporation Alder Lake-S PCH SATA Controller [AHCI Mode] [8086:7ae2] (rev 11)
>
> [3] https://devicehunt.com/view/type/pci/vendor/8086/device/7AE2
> [4] https://linux-hardware.org/?id=pci:8086-51d3-1462-1333
> [5] https://linux-hardware.org/?view=search&vendorid=8086&deviceid=51d3#list
>
> [6] Kernel logs before revert:
> ahci 0000:00:17.0: AHCI 0001.0301 32 slots 4 ports 6 Gbps 0xf0 impl SATA mode
> ata5: SATA max UDMA/133 abar m2048@0x80702000 port 0x80702300 irq 124 lpm-pol 3
> ata6: SATA max UDMA/133 abar m2048@0x80702000 port 0x80702380 irq 124 lpm-pol 3
> ata7: SATA max UDMA/133 abar m2048@0x80702000 port 0x80702400 irq 124 lpm-pol 3
> ata8: SATA max UDMA/133 abar m2048@0x80702000 port 0x80702480 irq 124 lpm-pol 3
> ata5: SATA link down (SStatus 4 SControl 300)
> ata6: SATA link down (SStatus 4 SControl 300)
> ata8: SATA link down (SStatus 4 SControl 300)
> ata7: SATA link down (SStatus 4 SControl 300)
>
> [7] Kernel logs after revert:
> ahci 0000:00:17.0: AHCI 0001.0301 32 slots 4 ports 6 Gbps 0xf0 impl SATA mode
> ata5: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802300 irq 125 lpm-pol 0
> ata6: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802380 irq 125 lpm-pol 0
> ata7: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802400 irq 125 lpm-pol 0
> ata8: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802480 irq 125 lpm-pol 0
> ata8: SATA link down (SStatus 0 SControl 300)
> ata7: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> ata5: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> ata6: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
These logs do not make sense to me.
Why is lpm-pol 3 before and lpm-pol 0 after removing the entry?
Are you using the same kernel config?
Could you please take:
v6.9 + the following debug print:
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 6548f10e61d9..0a14a09070ea 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1733,8 +1733,10 @@ static void ahci_update_initial_lpm_policy(struct ata_port *ap)
* Management Interaction in AHCI 1.3.1. Therefore, do not enable
* LPM if the port advertises itself as an external port.
*/
- if (ap->pflags & ATA_PFLAG_EXTERNAL)
+ if (ap->pflags & ATA_PFLAG_EXTERNAL) {
+ ata_port_info(ap, "external port, not enabling LPM\n");
return;
+ }
/* user modified policy via module param */
if (mobile_lpm_policy != -1) {
And then:
v6.9 + the debug print above + the removal of the Alder Lake-P AHCI entry.
- { PCI_VDEVICE(INTEL, 0x7ae2), board_ahci_pcs_quirk }, /* Alder Lake-P AHCI */
And paste the same kernel prints as you did in this email?
Thank you!
Kind regards,
Niklas
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 0/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"
2024-05-13 13:53 [PATCH] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list" dev
2024-05-13 13:53 ` [PATCH 1/1] " dev
2024-05-15 17:19 ` [PATCH] " Niklas Cassel
@ 2024-05-17 5:39 ` dev
2024-05-17 5:39 ` [PATCH v2 1/1] " dev
2024-05-21 12:54 ` [PATCH v2 0/1] " Niklas Cassel
2024-05-21 13:36 ` [PATCH v3] ata: ahci: Do not apply Intel PCS quirk on Intel Alder Lake Jason Nader
3 siblings, 2 replies; 12+ messages in thread
From: dev @ 2024-05-17 5:39 UTC (permalink / raw)
To: dlemoal, cassel; +Cc: linux-ide, Jason Nader
From: Jason Nader <dev@kayoway.com>
Changes - updated commit message as per the kind guidance from Niklas.
I was not sure whether to cc stable, but if so it would allow affected
users to start to be able to use kernel versions > 6.6.10.
Below are logs including the extra logging asked for by Niklas.
Kernel logs with this patch:
>journalctl --boot 0 | rg 'Linux version|SATA|external port'
Linux version linux6.9.disableAlderLakequirk@archlinux
ata4294967295: external port, not enabling LPM
ata4294967295: external port, not enabling LPM
ata4294967295: external port, not enabling LPM
ata4294967295: external port, not enabling LPM
ahci 0000:00:17.0: AHCI vers 0001.0301, 32 command slots, 6 Gbps, SATA mode
ata5: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802300 irq 125 lpm-pol 3
ata6: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802380 irq 125 lpm-pol 3
ata7: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802400 irq 125 lpm-pol 3
ata8: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802480 irq 125 lpm-pol 3
ata5: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
ata8: SATA link down (SStatus 0 SControl 300)
ata6: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
ata7: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
Kernel logs without this patch:
>journalctl --boot 0 | rg 'Linux version|SATA|external port'
Linux version linux6.9.enableAlderLakequirk@archlinux
ata4294967295: external port, not enabling LPM
ata4294967295: external port, not enabling LPM
ata4294967295: external port, not enabling LPM
ata4294967295: external port, not enabling LPM
ahci 0000:00:17.0: AHCI vers 0001.0301, 32 command slots, 6 Gbps, SATA mode
ata5: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802300 irq 125 lpm-pol 3
ata6: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802380 irq 125 lpm-pol 3
ata7: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802400 irq 125 lpm-pol 3
ata8: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802480 irq 125 lpm-pol 3
ata8: SATA link down (SStatus 4 SControl 300)
ata5: SATA link down (SStatus 4 SControl 300)
ata6: SATA link down (SStatus 4 SControl 300)
ata7: SATA link down (SStatus 4 SControl 300)
Note I also tested a patch that changes the Alder Lake entry from
board_ahci_pcs_quirk to board_ahci, and that booted fine as well.
However since it is not clear whether it is Alder Lake-S or
Alder Lake-P that was meant to be added to the list in the first place,
I have not committed that patch.
Kernel logs for board_ahci_pcs_quirk to board_ahci patch:
>journalctl --boot -1 | rg 'Linux version|SATA|external port' master|0.1s|14:07:07
Linux version linux6.9.enableAlderLakeboardahci@archlinux
ata4294967295: external port, not enabling LPM
ata4294967295: external port, not enabling LPM
ata4294967295: external port, not enabling LPM
ata4294967295: external port, not enabling LPM
ahci 0000:00:17.0: AHCI vers 0001.0301, 32 command slots, 6 Gbps, SATA mode
ata5: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802300 irq 125 lpm-pol 3
ata6: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802380 irq 125 lpm-pol 3
ata7: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802400 irq 125 lpm-pol 3
ata8: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802480 irq 125 lpm-pol 3
ata7: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
ata5: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
ata8: SATA link down (SStatus 0 SControl 300)
ata6: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
Jason Nader (1):
ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller
to low power chipsets list"
drivers/ata/ahci.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
--
2.45.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v2 1/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"
2024-05-17 5:39 ` [PATCH v2 0/1] " dev
@ 2024-05-17 5:39 ` dev
2024-05-21 12:55 ` Niklas Cassel
2024-05-21 13:13 ` Niklas Cassel
2024-05-21 12:54 ` [PATCH v2 0/1] " Niklas Cassel
1 sibling, 2 replies; 12+ messages in thread
From: dev @ 2024-05-17 5:39 UTC (permalink / raw)
To: dlemoal, cassel; +Cc: linux-ide, Jason Nader
From: Jason Nader <dev@kayoway.com>
Commit b8b8b4e0c052 ("ata: ahci: Add Intel Alder Lake-P AHCI controller
to low power chipsets list") added Intel Alder Lake to the ahci_pci_tbl.
Because of the way that the Intel PCS quirk was implemented, having
an explicit entry in the ahci_pci_tbl caused the Intel PCS quirk to
be applied. (The quirk was not being applied if there was no explicit
entry.)
Thus, entries that were added to the ahci_pci_tbl also got the Intel
PCS quirk applied.
The quirk was cleaned up in commit 7edbb6059274 ("ahci: clean up
intel_pcs_quirk"), such that it is clear which entries that actually
applies the Intel PCS quirk.
Newer Intel AHCI controllers do not need the Intel PCS quirk,
and applying it when not needed actually breaks some platforms.
Do not apply the Intel PCS quirk for Intel Alder Lake.
This is in line with how things worked before commit b8b8b4e0c052 ("ata:
ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"),
such that certain platforms using Intel Alder Lake will work once again.
Signed-off-by: Jason Nader <dev@kayoway.com>
---
drivers/ata/ahci.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 6548f10e61d9..07d66d2c5f0d 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -429,7 +429,6 @@ static const struct pci_device_id ahci_pci_tbl[] = {
{ PCI_VDEVICE(INTEL, 0x02d7), board_ahci_pcs_quirk }, /* Comet Lake PCH RAID */
/* Elkhart Lake IDs 0x4b60 & 0x4b62 https://sata-io.org/product/8803 not tested yet */
{ PCI_VDEVICE(INTEL, 0x4b63), board_ahci_pcs_quirk }, /* Elkhart Lake AHCI */
- { PCI_VDEVICE(INTEL, 0x7ae2), board_ahci_pcs_quirk }, /* Alder Lake-P AHCI */
/* JMicron 360/1/3/5/6, match class to avoid IDE function */
{ PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
--
2.45.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 1/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"
2024-05-17 5:39 ` [PATCH v2 1/1] " dev
@ 2024-05-21 12:55 ` Niklas Cassel
2024-05-21 13:13 ` Niklas Cassel
1 sibling, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2024-05-21 12:55 UTC (permalink / raw)
To: dev; +Cc: dlemoal, linux-ide
On Fri, May 17, 2024 at 02:39:02PM +0900, dev@kayoway.com wrote:
> From: Jason Nader <dev@kayoway.com>
>
> Commit b8b8b4e0c052 ("ata: ahci: Add Intel Alder Lake-P AHCI controller
> to low power chipsets list") added Intel Alder Lake to the ahci_pci_tbl.
>
> Because of the way that the Intel PCS quirk was implemented, having
> an explicit entry in the ahci_pci_tbl caused the Intel PCS quirk to
> be applied. (The quirk was not being applied if there was no explicit
> entry.)
>
> Thus, entries that were added to the ahci_pci_tbl also got the Intel
> PCS quirk applied.
>
> The quirk was cleaned up in commit 7edbb6059274 ("ahci: clean up
> intel_pcs_quirk"), such that it is clear which entries that actually
> applies the Intel PCS quirk.
>
> Newer Intel AHCI controllers do not need the Intel PCS quirk,
> and applying it when not needed actually breaks some platforms.
>
> Do not apply the Intel PCS quirk for Intel Alder Lake.
> This is in line with how things worked before commit b8b8b4e0c052 ("ata:
> ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"),
> such that certain platforms using Intel Alder Lake will work once again.
>
> Signed-off-by: Jason Nader <dev@kayoway.com>
> ---
> drivers/ata/ahci.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 6548f10e61d9..07d66d2c5f0d 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -429,7 +429,6 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x02d7), board_ahci_pcs_quirk }, /* Comet Lake PCH RAID */
> /* Elkhart Lake IDs 0x4b60 & 0x4b62 https://sata-io.org/product/8803 not tested yet */
> { PCI_VDEVICE(INTEL, 0x4b63), board_ahci_pcs_quirk }, /* Elkhart Lake AHCI */
> - { PCI_VDEVICE(INTEL, 0x7ae2), board_ahci_pcs_quirk }, /* Alder Lake-P AHCI */
>
> /* JMicron 360/1/3/5/6, match class to avoid IDE function */
> { PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> --
> 2.45.1
>
The patch looks good, I will queue it up once 6.10-rc1 is out!
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 1/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"
2024-05-17 5:39 ` [PATCH v2 1/1] " dev
2024-05-21 12:55 ` Niklas Cassel
@ 2024-05-21 13:13 ` Niklas Cassel
1 sibling, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2024-05-21 13:13 UTC (permalink / raw)
To: dev; +Cc: dlemoal, linux-ide
On Fri, May 17, 2024 at 02:39:02PM +0900, dev@kayoway.com wrote:
> From: Jason Nader <dev@kayoway.com>
>
> Commit b8b8b4e0c052 ("ata: ahci: Add Intel Alder Lake-P AHCI controller
> to low power chipsets list") added Intel Alder Lake to the ahci_pci_tbl.
>
> Because of the way that the Intel PCS quirk was implemented, having
> an explicit entry in the ahci_pci_tbl caused the Intel PCS quirk to
> be applied. (The quirk was not being applied if there was no explicit
> entry.)
>
> Thus, entries that were added to the ahci_pci_tbl also got the Intel
> PCS quirk applied.
>
> The quirk was cleaned up in commit 7edbb6059274 ("ahci: clean up
> intel_pcs_quirk"), such that it is clear which entries that actually
> applies the Intel PCS quirk.
>
> Newer Intel AHCI controllers do not need the Intel PCS quirk,
> and applying it when not needed actually breaks some platforms.
>
> Do not apply the Intel PCS quirk for Intel Alder Lake.
> This is in line with how things worked before commit b8b8b4e0c052 ("ata:
> ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"),
> such that certain platforms using Intel Alder Lake will work once again.
>
> Signed-off-by: Jason Nader <dev@kayoway.com>
> ---
> drivers/ata/ahci.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 6548f10e61d9..07d66d2c5f0d 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -429,7 +429,6 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x02d7), board_ahci_pcs_quirk }, /* Comet Lake PCH RAID */
> /* Elkhart Lake IDs 0x4b60 & 0x4b62 https://sata-io.org/product/8803 not tested yet */
> { PCI_VDEVICE(INTEL, 0x4b63), board_ahci_pcs_quirk }, /* Elkhart Lake AHCI */
> - { PCI_VDEVICE(INTEL, 0x7ae2), board_ahci_pcs_quirk }, /* Alder Lake-P AHCI */
>
> /* JMicron 360/1/3/5/6, match class to avoid IDE function */
> { PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> --
> 2.45.1
>
Thinking more about this, could you please send a v3 with:
Cc: stable@vger.kernel.org # 6.7
Fixes: b8b8b4e0c052 ("ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list")
and change the subject to something like:
ata: ahci: Do not apply Intel PCS quirk on Intel Alder Lake
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"
2024-05-17 5:39 ` [PATCH v2 0/1] " dev
2024-05-17 5:39 ` [PATCH v2 1/1] " dev
@ 2024-05-21 12:54 ` Niklas Cassel
1 sibling, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2024-05-21 12:54 UTC (permalink / raw)
To: dev; +Cc: dlemoal, linux-ide
On Fri, May 17, 2024 at 02:39:01PM +0900, dev@kayoway.com wrote:
> From: Jason Nader <dev@kayoway.com>
>
> Changes - updated commit message as per the kind guidance from Niklas.
>
> I was not sure whether to cc stable, but if so it would allow affected
> users to start to be able to use kernel versions > 6.6.10.
>
> Below are logs including the extra logging asked for by Niklas.
>
> Kernel logs with this patch:
> >journalctl --boot 0 | rg 'Linux version|SATA|external port'
> Linux version linux6.9.disableAlderLakequirk@archlinux
> ata4294967295: external port, not enabling LPM
> ata4294967295: external port, not enabling LPM
> ata4294967295: external port, not enabling LPM
> ata4294967295: external port, not enabling LPM
Since the prints below are for ata5-ata8, that most likely means that the
"external port, not enabling LPM" prints above are for port ata1-ata4,
so they are not really relevant and might even belong to a different SATA
controller.
Anyway, lpm-policy for ata5-ata8 is the same before and after this patch,
so everything looks fine.
Kind regards,
Niklas
> ahci 0000:00:17.0: AHCI vers 0001.0301, 32 command slots, 6 Gbps, SATA mode
> ata5: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802300 irq 125 lpm-pol 3
> ata6: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802380 irq 125 lpm-pol 3
> ata7: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802400 irq 125 lpm-pol 3
> ata8: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802480 irq 125 lpm-pol 3
> ata5: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> ata8: SATA link down (SStatus 0 SControl 300)
> ata6: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
> ata7: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>
> Kernel logs without this patch:
> >journalctl --boot 0 | rg 'Linux version|SATA|external port'
> Linux version linux6.9.enableAlderLakequirk@archlinux
> ata4294967295: external port, not enabling LPM
> ata4294967295: external port, not enabling LPM
> ata4294967295: external port, not enabling LPM
> ata4294967295: external port, not enabling LPM
> ahci 0000:00:17.0: AHCI vers 0001.0301, 32 command slots, 6 Gbps, SATA mode
> ata5: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802300 irq 125 lpm-pol 3
> ata6: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802380 irq 125 lpm-pol 3
> ata7: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802400 irq 125 lpm-pol 3
> ata8: SATA max UDMA/133 abar m2048@0x80802000 port 0x80802480 irq 125 lpm-pol 3
> ata8: SATA link down (SStatus 4 SControl 300)
> ata5: SATA link down (SStatus 4 SControl 300)
> ata6: SATA link down (SStatus 4 SControl 300)
> ata7: SATA link down (SStatus 4 SControl 300)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] ata: ahci: Do not apply Intel PCS quirk on Intel Alder Lake
2024-05-13 13:53 [PATCH] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list" dev
` (2 preceding siblings ...)
2024-05-17 5:39 ` [PATCH v2 0/1] " dev
@ 2024-05-21 13:36 ` Jason Nader
2024-05-27 8:12 ` Niklas Cassel
3 siblings, 1 reply; 12+ messages in thread
From: Jason Nader @ 2024-05-21 13:36 UTC (permalink / raw)
To: dlemoal, cassel; +Cc: linux-ide, Jason Nader, stable
Commit b8b8b4e0c052 ("ata: ahci: Add Intel Alder Lake-P AHCI controller
to low power chipsets list") added Intel Alder Lake to the ahci_pci_tbl.
Because of the way that the Intel PCS quirk was implemented, having
an explicit entry in the ahci_pci_tbl caused the Intel PCS quirk to
be applied. (The quirk was not being applied if there was no explict
entry.)
Thus, entries that were added to the ahci_pci_tbl also got the Intel
PCS quirk applied.
The quirk was cleaned up in commit 7edbb6059274 ("ahci: clean up
intel_pcs_quirk"), such that it is clear which entries that actually
applies the Intel PCS quirk.
Newer Intel AHCI controllers do not need the Intel PCS quirk,
and applying it when not needed actually breaks some platforms.
Do not apply the Intel PCS quirk for Intel Alder Lake.
This is in line with how things worked before commit b8b8b4e0c052 ("ata:
ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"),
such that certain platforms using Intel Alder Lake will work once again.
Cc: stable@vger.kernel.org # 6.7
Fixes: b8b8b4e0c052 ("ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list")
Signed-off-by: Jason Nader <dev@kayoway.com>
---
drivers/ata/ahci.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 6548f10e61d9..07d66d2c5f0d 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -429,7 +429,6 @@ static const struct pci_device_id ahci_pci_tbl[] = {
{ PCI_VDEVICE(INTEL, 0x02d7), board_ahci_pcs_quirk }, /* Comet Lake PCH RAID */
/* Elkhart Lake IDs 0x4b60 & 0x4b62 https://sata-io.org/product/8803 not tested yet */
{ PCI_VDEVICE(INTEL, 0x4b63), board_ahci_pcs_quirk }, /* Elkhart Lake AHCI */
- { PCI_VDEVICE(INTEL, 0x7ae2), board_ahci_pcs_quirk }, /* Alder Lake-P AHCI */
/* JMicron 360/1/3/5/6, match class to avoid IDE function */
{ PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
--
2.45.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3] ata: ahci: Do not apply Intel PCS quirk on Intel Alder Lake
2024-05-21 13:36 ` [PATCH v3] ata: ahci: Do not apply Intel PCS quirk on Intel Alder Lake Jason Nader
@ 2024-05-27 8:12 ` Niklas Cassel
2024-05-30 14:11 ` Alex
0 siblings, 1 reply; 12+ messages in thread
From: Niklas Cassel @ 2024-05-27 8:12 UTC (permalink / raw)
To: Jason Nader; +Cc: dlemoal, linux-ide, stable
On Tue, May 21, 2024 at 10:36:24PM +0900, Jason Nader wrote:
> Commit b8b8b4e0c052 ("ata: ahci: Add Intel Alder Lake-P AHCI controller
> to low power chipsets list") added Intel Alder Lake to the ahci_pci_tbl.
>
> Because of the way that the Intel PCS quirk was implemented, having
> an explicit entry in the ahci_pci_tbl caused the Intel PCS quirk to
> be applied. (The quirk was not being applied if there was no explict
> entry.)
>
> Thus, entries that were added to the ahci_pci_tbl also got the Intel
> PCS quirk applied.
>
> The quirk was cleaned up in commit 7edbb6059274 ("ahci: clean up
> intel_pcs_quirk"), such that it is clear which entries that actually
> applies the Intel PCS quirk.
>
> Newer Intel AHCI controllers do not need the Intel PCS quirk,
> and applying it when not needed actually breaks some platforms.
>
> Do not apply the Intel PCS quirk for Intel Alder Lake.
> This is in line with how things worked before commit b8b8b4e0c052 ("ata:
> ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"),
> such that certain platforms using Intel Alder Lake will work once again.
>
> Cc: stable@vger.kernel.org # 6.7
> Fixes: b8b8b4e0c052 ("ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list")
> Signed-off-by: Jason Nader <dev@kayoway.com>
> ---
> drivers/ata/ahci.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 6548f10e61d9..07d66d2c5f0d 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -429,7 +429,6 @@ static const struct pci_device_id ahci_pci_tbl[] = {
> { PCI_VDEVICE(INTEL, 0x02d7), board_ahci_pcs_quirk }, /* Comet Lake PCH RAID */
> /* Elkhart Lake IDs 0x4b60 & 0x4b62 https://sata-io.org/product/8803 not tested yet */
> { PCI_VDEVICE(INTEL, 0x4b63), board_ahci_pcs_quirk }, /* Elkhart Lake AHCI */
> - { PCI_VDEVICE(INTEL, 0x7ae2), board_ahci_pcs_quirk }, /* Alder Lake-P AHCI */
>
> /* JMicron 360/1/3/5/6, match class to avoid IDE function */
> { PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> --
> 2.45.1
>
Applied:
https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/log/?h=for-6.10-fixes
^ permalink raw reply [flat|nested] 12+ messages in thread