Skip to contentSkip to conversation

ahci,ata_generic: let ata_generic handle new MBP w/ MCP89

PATCH In review Tejun Heo 2 participants 3 messages Jun 17, 2010 – Jul 1, 2010

Subsystems (2)
lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

PATCH 1/1 ahci,ata_generic: let ata_generic handle new MBP w/ MCP89

For yet unknown reason, MCP89 on MBP 7,1 doesn't work w/ ahci under linux but the controller doesn't require explicit mode setting and works fine with ata_generic. Make ahci ignore the controller on MBP 7,1 and let ata_generic take it for now.

Reported in bko#15923.

https://bugzilla.kernel.org/show_bug.cgi?id=15923

NVIDIA is investigating why ahci mode doesn't work.

---
 drivers/ata/ahci.c        |   10 ++++++++++
 drivers/ata/ata_generic.c |    6 ++++++
 include/linux/pci_ids.h   |    1 +
 3 files changed, 17 insertions(+)
drivers/ata/ahci.c +10 -0
index 8ca16f5..f252253 100644
@@ -1053,6 +1053,16 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (pdev->vendor == PCI_VENDOR_ID_MARVELL && !marvell_enable)
 		return -ENODEV;
 
+	/*
+	 * For some reason, MCP89 on MacBook 7,1 doesn't work with
+	 * ahci, use ata_generic instead.
+	 */
+	if (pdev->vendor == PCI_VENDOR_ID_NVIDIA &&
+	    pdev->device == PCI_DEVICE_ID_NVIDIA_NFORCE_MCP89_SATA &&
+	    pdev->subsystem_vendor == PCI_VENDOR_ID_APPLE &&
+	    pdev->subsystem_device == 0xcb89)
+		return -ENODEV;
+
 	/* Promise's PDC42819 is a SAS/SATA controller that has an AHCI mode.
 	 * At the moment, we can only use the AHCI mode. Let the users know
 	 * that for SAS drives they're out of luck.
drivers/ata/ata_generic.c +6 -0
index 573158a..d4ccf74 100644
@@ -168,6 +168,12 @@ static struct pci_device_id ata_generic[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_VIA,    PCI_DEVICE_ID_VIA_82C561), },
 	{ PCI_DEVICE(PCI_VENDOR_ID_OPTI,   PCI_DEVICE_ID_OPTI_82C558), },
 	{ PCI_DEVICE(PCI_VENDOR_ID_CENATEK,PCI_DEVICE_ID_CENATEK_IDE), },
+	/*
+	 * For some reason, MCP89 on MacBook 7,1 doesn't work with
+	 * ahci, use ata_generic instead.
+	 */
+	{ PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP89_SATA,
+	  PCI_VENDOR_ID_APPLE, 0xcb89, },
 #if !defined(CONFIG_PATA_TOSHIBA) && !defined(CONFIG_PATA_TOSHIBA_MODULE)
 	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_1), },
 	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_2),  },
include/linux/pci_ids.h +1 -0
index 4eb4679..3bedcc1 100644
@@ -1261,6 +1261,7 @@
 #define PCI_DEVICE_ID_NVIDIA_NFORCE_MCP77_IDE       0x0759
 #define PCI_DEVICE_ID_NVIDIA_NFORCE_MCP73_SMBUS     0x07D8
 #define PCI_DEVICE_ID_NVIDIA_NFORCE_MCP79_SMBUS     0x0AA2
+#define PCI_DEVICE_ID_NVIDIA_NFORCE_MCP89_SATA	    0x0D85
 
 #define PCI_VENDOR_ID_IMS		0x10e0
 #define PCI_DEVICE_ID_IMS_TT128		0x9128
author
#1 lore raw

PATCH 2/2 ata_generic: implement ATA_GEN_* flags and force enable DMA on MBP 7,1

IDE mode of MCP89 on MBP 7,1 doesn't set DMA enable bits in the BMDMA status register. Make the following changes to work around the problem.

* Instead of using hard coded 1 in id->driver_data as class code match, use ATA_GEN_CLASS_MATCH and carry the matched id in host->private_data.

* Instead of matching PCI_VENDOR_ID_CENATEK, use ATA_GEN_FORCE_DMA flag in id instead.

* Add ATA_GEN_FORCE_DMA to the id entry of MBP 7,1.

---
 drivers/ata/ata_generic.c |   26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)
drivers/ata/ata_generic.c +19 -7
Index: ata/drivers/ata/ata_generic.c
===================================================================
@@ -32,6 +32,11 @@
  *	A generic parallel ATA driver using libata
  */
 
+enum {
+	ATA_GEN_CLASS_MATCH		= (1 << 0),
+	ATA_GEN_FORCE_DMA		= (1 << 1),
+};
+
 /**
  *	generic_set_mode	-	mode setting
  *	@link: link to set up
@@ -46,13 +51,17 @@
 static int generic_set_mode(struct ata_link *link, struct ata_device **unused)
 {
 	struct ata_port *ap = link->ap;
+	const struct pci_device_id *id = ap->host->private_data;
 	int dma_enabled = 0;
 	struct ata_device *dev;
 	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
 
-	/* Bits 5 and 6 indicate if DMA is active on master/slave */
-	if (ap->ioaddr.bmdma_addr)
+	if (id->driver_data & ATA_GEN_FORCE_DMA) {
+		dma_enabled = 0xff;
+	} else if (ap->ioaddr.bmdma_addr) {
+		/* Bits 5 and 6 indicate if DMA is active on master/slave */
 		dma_enabled = ioread8(ap->ioaddr.bmdma_addr + ATA_DMA_STATUS);
+	}
 
 	if (pdev->vendor == PCI_VENDOR_ID_CENATEK)
 		dma_enabled = 0xFF;
@@ -126,7 +135,7 @@ static int ata_generic_init_one(struct p
 	const struct ata_port_info *ppi[] = { &info, NULL };
 
 	/* Don't use the generic entry unless instructed to do so */
-	if (id->driver_data == 1 && all_generic_ide == 0)
+	if ((id->driver_data & ATA_GEN_CLASS_MATCH) && all_generic_ide == 0)
 		return -ENODEV;
 
 	/* Devices that need care */
@@ -155,7 +164,7 @@ static int ata_generic_init_one(struct p
 			return rc;
 		pcim_pin_device(dev);
 	}
-	return ata_pci_bmdma_init_one(dev, ppi, &generic_sht, NULL, 0);
+	return ata_pci_bmdma_init_one(dev, ppi, &generic_sht, (void *)id, 0);
 }
 
 static struct pci_device_id ata_generic[] = {
@@ -167,13 +176,15 @@ static struct pci_device_id ata_generic[
 	{ PCI_DEVICE(PCI_VENDOR_ID_HINT,   PCI_DEVICE_ID_HINT_VXPROII_IDE), },
 	{ PCI_DEVICE(PCI_VENDOR_ID_VIA,    PCI_DEVICE_ID_VIA_82C561), },
 	{ PCI_DEVICE(PCI_VENDOR_ID_OPTI,   PCI_DEVICE_ID_OPTI_82C558), },
-	{ PCI_DEVICE(PCI_VENDOR_ID_CENATEK,PCI_DEVICE_ID_CENATEK_IDE), },
+	{ PCI_DEVICE(PCI_VENDOR_ID_CENATEK,PCI_DEVICE_ID_CENATEK_IDE),
+	  .driver_data = ATA_GEN_FORCE_DMA },
 	/*
 	 * For some reason, MCP89 on MacBook 7,1 doesn't work with
 	 * ahci, use ata_generic instead.
 	 */
 	{ PCI_VENDOR_ID_NVIDIA, PCI_DEVICE_ID_NVIDIA_NFORCE_MCP89_SATA,
-	  PCI_VENDOR_ID_APPLE, 0xcb89, },
+	  PCI_VENDOR_ID_APPLE, 0xcb89,
+	  .driver_data = ATA_GEN_FORCE_DMA },
 #if !defined(CONFIG_PATA_TOSHIBA) && !defined(CONFIG_PATA_TOSHIBA_MODULE)
 	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_1), },
 	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_2),  },
@@ -181,7 +192,8 @@ static struct pci_device_id ata_generic[
 	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_5),  },
 #endif	
 	/* Must come last. If you add entries adjust this table appropriately */
-	{ PCI_ANY_ID,		PCI_ANY_ID,			   PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_STORAGE_IDE << 8, 0xFFFFFF00UL, 1},
+	{ PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_IDE << 8, 0xFFFFFF00UL),
+	  .driver_data = ATA_GEN_CLASS_MATCH },
 	{ 0, },
 };
 
#2 lore raw
In reply to Tejun Heo
Quoted 19 lines · Tejun Heo

For yet unknown reason, MCP89 on MBP 7,1 doesn't work w/ ahci under linux but the controller doesn't require explicit mode setting and works fine with ata_generic. Make ahci ignore the controller on MBP 7,1 and let ata_generic take it for now.

Reported in bko#15923.

   https://bugzilla.kernel.org/show_bug.cgi?id=15923

NVIDIA is investigating why ahci mode doesn't work.

Quoted 5 lines
---
  drivers/ata/ahci.c        |   10 ++++++++++
  drivers/ata/ata_generic.c |    6 ++++++
  include/linux/pci_ids.h   |    1 +
  3 files changed, 17 insertions(+)

applied 1-2