[PATCH] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"

[PATCH] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"

[dev]

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)

Jason Nader (1):
  ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller
    to  low power chipsets list"

 drivers/ata/ahci.c | 1 -
 1 file changed, 1 deletion(-)

-- 
2.45.0

[PATCH 1/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"

[dev]

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

Re: [PATCH] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"

[Niklas Cassel]

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

Re: [PATCH 1/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"

[Niklas Cassel]

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

[PATCH v2 0/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"

[dev]

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

[PATCH v2 1/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"

[dev]

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

Re: [PATCH v2 0/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"

[Niklas Cassel]

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)

Re: [PATCH v2 1/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"

[Niklas Cassel]

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

Re: [PATCH v2 1/1] ata: ahci: Revert "ata: ahci: Add Intel Alder Lake-P AHCI controller to low power chipsets list"

[Niklas Cassel]

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

[PATCH v3] ata: ahci: Do not apply Intel PCS quirk on Intel Alder Lake

[Jason Nader]

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

Re: [PATCH v3] ata: ahci: Do not apply Intel PCS quirk on Intel Alder Lake

[Niklas Cassel]

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

Re: [PATCH v3] ata: ahci: Do not apply Intel PCS quirk on Intel Alder Lake

[Alex]

On 27/05/2024 10:12, Niklas Cassel wrote:

> 
> Applied:
> https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/log/?h=for-6.10-fixes

Thank you a lot for the fix, at last, i will see again my ssd :)