Enable AHCI on certain ich chipsets

PATCH In review Joerg Dorchain 3 participants 9 messages Feb 9, 2011 – Feb 14, 2011

Subsystems (2)

PCI libata

lore

Share & tools

lore

Joerg Dorchain

author

2011-02-09 12:07 UTC #0 lore raw

PATCH 1/1 Enable AHCI on certain ich chipsets

Hello all,

Disclaimer: this patch is based on http://mjg59.livejournal.com/85504.html
It is in works-for-me state.

The patch to ahci.c is required for suspend/resume.

Signed-off-by joerg Dorchain <[email protected]>

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +73 -0

@@ -2684,6 +2684,79 @@
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HINT, 0x0020, quirk_hotplug_bridge);
 
 /*
+ * Force ICH7/8/9 into AHCI mode.  This is needed because some
+ * BIOSes do not make AHCI-mode operation available to the user.
+ * As the Intel documentation states that the OS should not carry
+ * out the operation - the user must force this on the kernel
+ * commandline using quirk_ich_force_ahci
+ *
+ * As this quirk gets called whilst the PCI subsystem is
+ * walking the PCI bus, we declare this quirk against the LPC
+ * (device 00:1f.0), so that we can frob 00:1f.2 before the PCI
+ * code has scanned it.
+ * Note: the pci id might change due to this (e.g. from 27c4 to 27c5)
+ *
+ */
+
+static bool ich_force_ahci_mode = false;
+static bool ich_ahci_mode_forced = false;
+
+static int __init ich789_force_ahci_mode_setup(char *str)
+{
+	ich_force_ahci_mode = true;
+	return 0;
+}
+early_param("quirk_ich_force_ahci", ich789_force_ahci_mode_setup);
+
+static void ich789_force_ahci_mode(struct pci_dev *pdev)
+{
+        u8 amrval;
+        u8 sclkgc;
+        const int ich89_address_map_reg = 0x90;
+        const int ich89_sata_clock_gen_config_reg = 0x9c;
+        /* const u32 ich89_func_disable_reg_offset = 0x3418; */
+
+	if (!ich_force_ahci_mode)
+		return;
+
+        /* ICH8 datasheet section 12.1.33 */
+        if (!pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2),
+                                ich89_address_map_reg, &amrval))
+{
+                if (amrval & (BIT(6) | BIT(7))) {
+                        dev_printk(KERN_DEBUG, &pdev->dev,
+                                        "ICH7/8/9 SATA controller not in IDE mode.  Not modifying.\n");
+                        return;
+                }
+                if (amrval & (BIT(0) | BIT(1)))
+                        dev_printk(KERN_DEBUG, &pdev->dev,
+                                        "ICH7/8/9 in SATA/PATA combined mode.  Untested.\n");
+                /* AHCI mode */
+                amrval |= BIT(6);
+                amrval &= ~BIT(7);
+                pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2), 
+                                ich89_sata_clock_gen_config_reg, &sclkgc);
+                dev_printk(KERN_DEBUG, &pdev->dev, "sclkgc is %#0x\n", sclkgc);
+                pci_bus_write_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2), 
+                                ich89_address_map_reg, amrval);
+                ich_ahci_mode_forced = true;
+                dev_printk(KERN_DEBUG, &pdev->dev, "Forced ICH7/8/9 mode PIIX->AHCI\n");
+        }
+
+}
+/* ICH7 */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x27b9, ich789_force_ahci_mode);
+/* ICH8 */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH8_0, ich789_force_ahci_mode);
+/* ICH9R LPC */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2916, ich789_force_ahci_mode);
+/* ICH9M LPC */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2917, ich789_force_ahci_mode);
+/* ICH9M-E LPC */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2919, ich789_force_ahci_mode);
+
+
+/*
  * This is a quirk for the Ricoh MMC controller found as a part of
  * some mulifunction chips.

drivers/ata/ahci.c renamed from drivers/ata/ahci.c.orig +2 -0

@@ -640,6 +640,8 @@
 	struct ata_host *host = dev_get_drvdata(&pdev->dev);
 	int rc;
 
+	// override check to see if PCI config space is already restored in pci_restore_state
+	pdev->state_saved = true;
 	rc = ata_pci_device_do_resume(pdev);
 	if (rc)
 		return rc;

Joerg Dorchain

author

2011-02-10 19:23 UTC #2 lore raw

Hello all,

On Wed, Feb 09, 2011 at 03:56:32PM +0300, Sergei Shtylyov wrote:
[Several formal corrections]

Should be addressed by this try.

Quoted 3 lines

Quoted 1 line
The patch to ahci.c is required for suspend/resume.
Need a better description than thatl, I think...

Well, then it is a bit longer story.

During resume from suspend to ram, the kernel pci layer restores the registers for the SATA controller once, then says okay, and sets dev->state_saved = false. However, since the restore goes from highest address (the BARs [base address registers]) to lowest register, some of the higher registers are set as RO because according to the lower registers controller is in PIIX mode. This patch introduces a workaround for
this problem, hacking around the PCI API by setting pdev->state_saved = true before we do the restore.

Admittingly, more testing would be welcome.

Signed-off-by joerg Dorchain <[email protected]>

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +70 -0

@@ -2684,6 +2684,76 @@
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HINT, 0x0020, quirk_hotplug_bridge);
 
 /*
+ * Force ICH7/8/9 into AHCI mode.  This is needed because some
+ * BIOSes do not make AHCI-mode operation available to the user.
+ * As the Intel documentation states that the OS should not carry
+ * out the operation - the user must force this on the kernel
+ * commandline using quirk_ich_force_ahci
+ *
+ * As this quirk gets called whilst the PCI subsystem is
+ * walking the PCI bus, we declare this quirk against the LPC
+ * (device 00:1f.0), so that we can frob 00:1f.2 before the PCI
+ * code has scanned it.
+ * Note: the pci id might change due to this (e.g. from 27c4 to 27c5)
+ *
+ */
+
+static bool ich_force_ahci_mode = false;
+
+static int __init ich789_force_ahci_mode_setup(char *str)
+{
+	ich_force_ahci_mode = true;
+	return 0;
+}
+early_param("quirk_ich_force_ahci", ich789_force_ahci_mode_setup);
+
+static void ich789_force_ahci_mode(struct pci_dev *pdev)
+{
+        u8 amrval;
+        u8 sclkgc;
+        const int ich89_address_map_reg = 0x90;
+        const int ich89_sata_clock_gen_config_reg = 0x9c;
+
+	if (!ich_force_ahci_mode)
+		return;
+
+        /* ICH8 datasheet section 12.1.33 */
+        if (!pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2),
+        	ich89_address_map_reg, &amrval))
+	{
+                if (amrval & (BIT(6) | BIT(7))) {
+                        dev_printk(KERN_DEBUG, &pdev->dev,
+                                        "ICH7/8/9 SATA controller not in IDE mode.  Not modifying.\n");
+                        return;
+                }
+                if (amrval & (BIT(0) | BIT(1)))
+                        dev_printk(KERN_DEBUG, &pdev->dev,
+                                        "ICH7/8/9 in SATA/PATA combined mode.  Untested.\n");
+                /* AHCI mode */
+                amrval |= BIT(6);
+                amrval &= ~BIT(7);
+                pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2), 
+                                ich89_sata_clock_gen_config_reg, &sclkgc);
+                dev_printk(KERN_DEBUG, &pdev->dev, "sclkgc is %#0x\n", sclkgc);
+                pci_bus_write_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2), 
+                                ich89_address_map_reg, amrval);
+                dev_printk(KERN_DEBUG, &pdev->dev, "Forced ICH7/8/9 mode PIIX->AHCI\n");
+        }
+
+}
+/* ICH7 */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x27b9, ich789_force_ahci_mode);
+/* ICH8 */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH8_0, ich789_force_ahci_mode);
+/* ICH9R LPC */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2916, ich789_force_ahci_mode);
+/* ICH9M LPC */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2917, ich789_force_ahci_mode);
+/* ICH9M-E LPC */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2919, ich789_force_ahci_mode);
+
+
+/*
  * This is a quirk for the Ricoh MMC controller found as a part of
  * some mulifunction chips.

drivers/ata/ahci.c renamed from drivers/ata/ahci.c.orig +3 -0

@@ -640,6 +640,9 @@
 	struct ata_host *host = dev_get_drvdata(&pdev->dev);
 	int rc;
 
+	/* override check to see if PCI config space is already
+	 * restored in pci_restore_state */
+	pdev->state_saved = true;
 	rc = ata_pci_device_do_resume(pdev);
 	if (rc)
 		return rc;

Sergei Shtylyov

2011-02-11 12:28 UTC #3 lore raw

Hello.

Quoted 21 lines · Joerg Dorchain

On Wed, Feb 09, 2011 at 03:56:32PM +0300, Sergei Shtylyov wrote:
[Several formal corrections]
Should be addressed by this try.
Quoted 3 lines
Quoted 1 line
The patch to ahci.c is required for suspend/resume.
Need a better description than thatl, I think...
Well, then it is a bit longer story.
During resume from suspend to ram, the kernel pci layer restores the registers for the SATA controller once, then says okay, and sets dev->state_saved = false. However, since the restore goes from highest address (the BARs [base address registers]) to lowest register, some of the higher registers are set as RO because according to the lower registers controller is in PIIX mode. This patch introduces a workaround for
this problem, hacking around the PCI API by setting pdev->state_saved = true before we do the restore.

Quoted 3 lines

Admittingly, more testing would be welcome.
Signed-off-by joerg Dorchain<[email protected]>

This only describes drivers/ata/ahci.c change. And looks like it should be
in a patch of its own...

Quoted 3 lines

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +0 -0
@@ -2684,6 +2684,76 @@

[...]

drivers/pci/quirks.c line 2715

Quoted 6 lines

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +6 -0

+static void ich789_force_ahci_mode(struct pci_dev *pdev)
+{
+        u8 amrval;
+        u8 sclkgc;
+        const int ich89_address_map_reg = 0x90;
+        const int ich89_sata_clock_gen_config_reg = 0x9c;

You indent with spaces instead of tab here.

drivers/pci/quirks.c line 2722

Quoted 7 lines

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +7 -0

+
+	if (!ich_force_ahci_mode)
+		return;
+
+        /* ICH8 datasheet section 12.1.33 */
+        if (!pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2),
+        	ich89_address_map_reg, &amrval))

You again indent with spaces...

drivers/pci/quirks.c

Quoted 1 line

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +1 -0
+	{

I've clearly said that { should be on the same line as the end of *if*
statement.

Quoted 8 lines

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +8 -0

+                if (amrval & (BIT(6) | BIT(7))) {
+                        dev_printk(KERN_DEBUG, &pdev->dev,
+                                        "ICH7/8/9 SATA controller not in IDE mode.  Not modifying.\n");
+                        return;
+                }
+                if (amrval & (BIT(0) | BIT(1)))
+                        dev_printk(KERN_DEBUG, &pdev->dev,
+                                        "ICH7/8/9 in SATA/PATA combined mode.  Untested.\n");

drivers/pci/quirks.c line 2740

Quoted 10 lines

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +10 -0

+                /* AHCI mode */
+                amrval |= BIT(6);
+                amrval &= ~BIT(7);
+                pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2),
+                                ich89_sata_clock_gen_config_reg, &sclkgc);
+                dev_printk(KERN_DEBUG, &pdev->dev, "sclkgc is %#0x\n", sclkgc);
+                pci_bus_write_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2),
+                                ich89_address_map_reg, amrval);
+                ich_ahci_mode_forced = true;
+                dev_printk(KERN_DEBUG, &pdev->dev, "Forced ICH7/8/9 mode PIIX->AHCI\n");

Again indented with spaces...

drivers/pci/quirks.c

Quoted 2 lines

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +2 -0
+        }
+

Empty line not needed...

drivers/ata/ahci.c line 644

Quoted 10 lines

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +1 -0
+}
drivers/ata/ahci.c renamed from drivers/ata/ahci.c.orig +2 -0
@@ -640,6 +640,9 @@
  	struct ata_host *host = dev_get_drvdata(&pdev->dev);
  	int rc;
 
+	/* override check to see if PCI config space is already
+	 * restored in pci_restore_state */

The preferred style for the multi-line comments is this:

/*
* bla
* bla
*/

WBR, Sergei

Joerg Dorchain

author

2011-02-11 17:36 UTC #4 lore raw

Quoted 3 lines · Sergei Shtylyov

Quoted 3 lines
[Several formal corrections]
Should be addressed by this try.

Thank you for bearing with me. Next try is below.

Quoted 13 lines

Quoted 10 lines
During resume from suspend to ram, the kernel pci layer restores the registers for the SATA controller once, then says okay, and sets dev->state_saved = false. However, since the restore goes from highest address (the BARs [base address registers]) to lowest register, some of the higher registers are set as RO because according to the lower registers controller is in PIIX mode. This patch introduces a workaround for
this problem, hacking around the PCI API by setting pdev->state_saved = true before we do the restore.

Quoted 1 line

This only describes drivers/ata/ahci.c change.

Well, the functionality of the patch to quirks.c is described in the comments on the top of it. Should that be repeated?

Quoted 2 lines

And looks like it
should be in a patch of its own...

I need both parts in order to use the AHCI driver and having suspend/resume work, hence they are together.

Bye,

Joerg

Signed-off-by joerg Dorchain<[email protected]>

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +68 -0

@@ -2684,6 +2684,74 @@
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HINT, 0x0020, quirk_hotplug_bridge);
 
 /*
+ * Force ICH7/8/9 into AHCI mode.  This is needed because some
+ * BIOSes do not make AHCI-mode operation available to the user.
+ * As the Intel documentation states that the OS should not carry
+ * out the operation - the user must force this on the kernel
+ * commandline using quirk_ich_force_ahci
+ *
+ * As this quirk gets called whilst the PCI subsystem is
+ * walking the PCI bus, we declare this quirk against the LPC
+ * (device 00:1f.0), so that we can frob 00:1f.2 before the PCI
+ * code has scanned it.
+ * Note: the pci id might change due to this (e.g. from 27c4 to 27c5)
+ *
+ */
+
+static bool ich_force_ahci_mode = false;
+
+static int __init ich789_force_ahci_mode_setup(char *str)
+{
+	ich_force_ahci_mode = true;
+	return 0;
+}
+early_param("quirk_ich_force_ahci", ich789_force_ahci_mode_setup);
+
+static void ich789_force_ahci_mode(struct pci_dev *pdev)
+{
+	u8 amrval;
+	u8 sclkgc;
+	const int ich89_address_map_reg = 0x90;
+	const int ich89_sata_clock_gen_config_reg = 0x9c;
+
+	if (!ich_force_ahci_mode)
+		return;
+
+	/* ICH8 datasheet section 12.1.33 */
+	if (!pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2),
+        	ich89_address_map_reg, &amrval)) {
+
+		if (amrval & (BIT(6) | BIT(7))) {
+			dev_printk(KERN_DEBUG, &pdev->dev,
+				"ICH7/8/9 SATA controller not in IDE mode.  Not modifying.\n");
+			return;
+		}
+		if (amrval & (BIT(0) | BIT(1)))
+			dev_printk(KERN_DEBUG, &pdev->dev,
+				"ICH7/8/9 in SATA/PATA combined mode.  Untested.\n");
+		/* AHCI mode */
+		amrval |= BIT(6);
+		amrval &= ~BIT(7);
+		pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2), 
+			ich89_sata_clock_gen_config_reg, &sclkgc);
+		dev_printk(KERN_DEBUG, &pdev->dev, "sclkgc is %#0x\n", sclkgc);
+		pci_bus_write_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2), 
+			ich89_address_map_reg, amrval);
+		dev_printk(KERN_DEBUG, &pdev->dev, "Forced ICH7/8/9 mode PIIX->AHCI\n");
+	}
+}
+/* ICH7 */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x27b9, ich789_force_ahci_mode);
+/* ICH8 */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_ICH8_0, ich789_force_ahci_mode);
+/* ICH9R LPC */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2916, ich789_force_ahci_mode);
+/* ICH9M LPC */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2917, ich789_force_ahci_mode);
+/* ICH9M-E LPC */
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x2919, ich789_force_ahci_mode);
+
+/*
  * This is a quirk for the Ricoh MMC controller found as a part of
  * some mulifunction chips.

drivers/ata/ahci.c renamed from drivers/ata/ahci.c.orig +5 -0

@@ -640,6 +640,11 @@
 	struct ata_host *host = dev_get_drvdata(&pdev->dev);
 	int rc;
 
+	/*
+	 * override check to see if PCI config space is already
+	 * restored in pci_restore_state
+	 */
+	pdev->state_saved = true;
 	rc = ata_pci_device_do_resume(pdev);
 	if (rc)
 		return rc;

Sergei Shtylyov

2011-02-11 20:51 UTC #5 lore raw

Hello.

Quoted 10 lines · Joerg Dorchain

Quoted 9 lines
Quoted 9 lines
During resume from suspend to ram, the kernel pci layer restores the registers for the SATA controller once, then says okay, and sets dev->state_saved = false. However, since the restore goes from highest address (the BARs [base address registers]) to lowest register, some of the higher registers are set as RO because according to the lower registers controller is in PIIX mode. This patch introduces a workaround for
this problem, hacking around the PCI API by setting pdev->state_saved = true before we do the restore.

Quoted 4 lines

Quoted 1 line
This only describes drivers/ata/ahci.c change.
Well, the functionality of the patch to quirks.c is described in the comments on the top of it. Should that be repeated?

Yes, certainly.

Quoted 5 lines

Quoted 2 lines
And looks like it
should be in a patch of its own...
I need both parts in order to use the AHCI driver and having suspend/resume work, hence they are together.

They are solving two different issues, as far as I understand, hence should
be in two different patches. One issue per patch is the basic rule.

Quoted 9 lines

Bye,
Joerg
Signed-off-by joerg Dorchain<[email protected]>
drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +0 -0
@@ -2684,6 +2684,74 @@

[...]

drivers/pci/quirks.c line 2722

Quoted 13 lines

drivers/pci/quirks.c renamed from drivers/pci/quirks.c.orig +13 -0

+static void ich789_force_ahci_mode(struct pci_dev *pdev)
+{
+	u8 amrval;
+	u8 sclkgc;
+	const int ich89_address_map_reg = 0x90;
+	const int ich89_sata_clock_gen_config_reg = 0x9c;
+
+	if (!ich_force_ahci_mode)
+		return;
+
+	/* ICH8 datasheet section 12.1.33 */
+	if (!pci_bus_read_config_byte(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 2),
+        	ich89_address_map_reg, &amrval)) {

Still spaces at the start of line... run your patches thru
scripts/checkpatch.pl -- it notices such style violations.

WBR, Sergei

Joerg Dorchain

author

2011-02-12 06:27 UTC #6 lore raw

Quoted 5 lines · Sergei Shtylyov

Quoted 3 lines
Well, the functionality of the patch to quirks.c is described in the comments on the top of it. Should that be repeated?
Yes, certainly.

Ok, will do so.

Quoted 11 lines

Quoted 6 lines
Quoted 3 lines
And looks like it
should be in a patch of its own...
I need both parts in order to use the AHCI driver and having suspend/resume work, hence they are together.
They are solving two different issues, as far as I understand, hence should be in two different patches. One issue per patch is the basic rule.

Here I am not sure. Suspend/resume works fine (for me) without any of the patches. The first patch fiddles with pci config space in order to achieve something that which should be done by the BIOS, hence it is in quirks, together with other workarounds.

The second part just makes sure that the config space change is kept after a resume. Otherwise it comes up as a different PCI id, the AHCI driver finds nothing to work with, the harddisk is gone after resume, not good.

You could also see it as it triggers a general fault, and should be fixed anyway indepent of the trigger case.

Whereas I am quite sure about the quirks part, I'd be glad for some functional review of the ahci part, to avoid undesired side effects.

Quoted 3 lines

scripts/checkpatch.pl -- it notices such style violations.

Thank you for that hint.

Bye,

Joerg

Bartlomiej Zolnierkiewicz

2011-02-12 12:09 UTC #7 lore raw

Hi,

Quoted 4 lines · Joerg Dorchain

The second part just makes sure that the config space change is kept after a resume. Otherwise it comes up as a different PCI id, the AHCI driver finds nothing to work with, the harddisk is gone after resume, not good.

Shouldn't therefore the quirk be applied also during resume (by additional use of DECLARE_PCI_FIXUP_RESUME_EARLY in addition to existing DECLARE_PCI_FIXUP_EARLY one)?

Thanks,
Bartomiej

Joerg Dorchain

author

2011-02-14 07:41 UTC #8 lore raw

Quoted 9 lines · Bartlomiej Zolnierkiewicz

Quoted 5 lines
The second part just makes sure that the config space change is kept after a resume. Otherwise it comes up as a different PCI id, the AHCI driver finds nothing to work with, the harddisk is gone after resume, not good.
Shouldn't therefore the quirk be applied also during resume (by additional use of DECLARE_PCI_FIXUP_RESUME_EARLY in addition to existing DECLARE_PCI_FIXUP_EARLY one)?

I have considered that, but just not touching the config space for having the desired effect seems easier to me.

Actually this is a reason why I am looking for feedback from other people with the chipsets listed in the patch. Is it only my system for keeps pci config during suspend/resume or does it work for others, too?
On the other hand, if I understand Sergei correctly, it could be a more general effect with it, which is why I'd also appreciated some review from an architectural perspective.

If not, DECLARE_PCI_FIXUP_RESUME_EARLY is fine with me. My main objective is to use the ahci driver, and I hope others like to do so as well.

Bye,

Joerg