Commit 7edbb605 authored by Niklas Cassel's avatar Niklas Cassel

ahci: clean up intel_pcs_quirk

The comment in front of board_ahci_pcs7 is completely wrong.
It claims that board_ahci_pcs7 is needing the quirk, but in fact,
the logic implemented in ahci_intel_pcs_quirk() is the exact opposite,
only board_ahci_pcs7 is _excluded_ from the quirk.

This way of implementing a quirk is unconventional in several ways:
First of all because it has a board ID for which the quirk should _not_ be
applied (board_ahci_pcs7), instead of the usual way where we have a board
ID for which the quirk should be applied.

The second reason is that other than only excluding board_ahci_pcs7 from
the quirk, PCI devices that make use of the generic entry in ahci_pci_tbl
(which matches on AHCI class code) are also excluded.

This can of course lead to very subtle breakage, and did indeed do so in:
commit 104ff59a ("ata: ahci: Add Tiger Lake UP{3,4} AHCI controller"),
which added an explicit entry with board_ahci_low_power to ahci_pci_tbl.

This caused many users to complain that their SATA drives disappeared.
The logical assumption was of course that the issue was related to LPM,
and was therefore reverted in commit 6210038a ("ata: ahci: Revert
"ata: ahci: Add Tiger Lake UP{3,4} AHCI controller"").

It took a lot of time to figure out that this was all completely unrelated
to LPM, and was instead caused by an unconventional Intel quirk.

Clean up the quirk so that it behaves like other quirks, i.e. define a
board where the quirk is applied. Platforms that were using
board_ahci_pcs7 are converted to use board_ahci, this is safe since the
boards were identical, and board_ahci_pcs7 did not define any custom
port_ops.

This way, new Intel platforms can be added using the correct "board_ahci"
board, without getting any unexpected quirks applied.

This means that we currently have some modern platforms defined that are
using the Intel PCS quirk, but that is identical to the behavior that
was there before this commit.

No functional changes intended.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217114Reviewed-by: default avatarMika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: default avatarDan Williams <dan.j.williams@intel.com>
Signed-off-by: default avatarNiklas Cassel <cassel@kernel.org>
parent 7627a0ed
This diff is collapsed.
...@@ -245,6 +245,7 @@ enum { ...@@ -245,6 +245,7 @@ enum {
suspend/resume */ suspend/resume */
AHCI_HFLAG_NO_SXS = BIT(26), /* SXS not supported */ AHCI_HFLAG_NO_SXS = BIT(26), /* SXS not supported */
AHCI_HFLAG_43BIT_ONLY = BIT(27), /* 43bit DMA addr limit */ AHCI_HFLAG_43BIT_ONLY = BIT(27), /* 43bit DMA addr limit */
AHCI_HFLAG_INTEL_PCS_QUIRK = BIT(28), /* apply Intel PCS quirk */
/* ap->flags bits */ /* ap->flags bits */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment