[Lucid] SRU: [PATCH] UBUNTU: Make ahci ignore the controller for MBP7, 1 and fall back to ata_generic. Patch based on Tejun Heo's submitted upstream.
Andy Whitcroft
apw at canonical.com
Fri Jul 2 08:17:22 UTC 2010
On Fri, Jul 02, 2010 at 09:43:29AM +0200, Stefan Bader wrote:
> As of today, this change is upstream but as two separate patches. I took the
> liberty to do provide them as backported in your name.
> Both have the necessary tags to make them go upstream stable and should come in
> one of the next stable releases. But due to the fact that an installation is not
> possible without that changes on the affected hw, I think it is sensible to take
> them pre-stable. Changes are limited to specific device, so regression potential
> is low.
>
> -Stefan
>
> For the split up version of patches.
> Acked-by: Stefan Bader <stefan.bader at canonical.com>
>
> On 07/02/2010 01:37 AM, Dave Walker wrote:
> > The Linux kernel doesn't currently support the latest generation of the
> > Macbook Pro (7,1), and also seems to have issues with the MacBook Mini.
> > The issue presents itself as SATA support being unavailable once the
> > initrd tries to reset the SATA device.
> >
> > There is a high priority open bug[0] regarding this, and at time of
> > writing 106 Launchpad users have shared that the bug affects them (aka
> > "me too")
> >
> > The patch attached is heavily based on the work of Tejun Heo who has
> > submitted it upstream[1] (and submitted to stable). The changes to this
> > patch include some slight refactoring to which make it compatiable with
> > the 2.6.32 code, and a manual refresh to apply cleanly to the lucid git
> > tree.
> >
> > Based on looking what the patch achieves, i might suggest that the
> > regression potential is low as it essentially blacklists if the
> > following conditions are met:
> > - The vendor of the SATA device is nvidia
> > - The device is MCP89
> > - The hardware is Apple
> > - Particular subsystem id
> >
> > If the blacklist is achieved, then ahci ignores the controller and
> > allows fall back to ata_generic.
> >
> > Whilst it hasn't yet been applied to Linus' git tree (so can't send a
> > stable pull request), I would personally consider it paramount that this
> > patch is applied soon to the Lucid kernel with the hope of landing on
> > the 10.04.1 cd image.
> >
> > As it stands, Ubuntu users with this hardware are unable to install from
> > the 10.04 cd image, and are currently dependent on using an community
> > built unofficial iso to be able to install Ubuntu.
> >
> > Based on the high "me too" on Launchpad, I feel that many users are
> > highly dependent on this landing in an official cd spin, and am
> > therefore hoping that this patch is included and an SRU is created in
> > time for the 10.04.1 release.
> >
> > The original author of this patch described it as:
> > "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." -- Tejun Heo
> >
> > Thanks
> >
> > [0] http://bugs.launchpad.net/bugs/576601
> > [1] http://article.gmane.org/gmane.linux.ide/46514
> From b09867f655ad766cf2aa8032d75423d681530cea Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj at kernel.org>
> Date: Thu, 17 Jun 2010 11:42:22 +0200
> Subject: [PATCH 1/2] (pre-stable) ahci,ata_generic: let ata_generic handle new MBP w/ MCP89
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> BugLink: http://bugs.launchpad.net/bugs/576601
>
> 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.
>
> Signed-off-by: Tejun Heo <tj at kernel.org>
> Cc: Peer Chen <pchen at nvidia.com>
> Cc: stable at kernel.org
> Reported-by: Anders ??sthus <grapz666 at gmail.com>
> Reported-by: Andreas Graf <andreas_graf at csgraf.de>
> Reported-by: Benoit Gschwind <gschwind at gnu-log.net>
> Reported-by: Damien Cassou <damien.cassou at gmail.com>
> Reported-by: tixetsal at juno.com
> Signed-off-by: Jeff Garzik <jgarzik at redhat.com>
> (backported from commit b09867f655ad766cf2aa8032d75423d681530cea upstream)
> Signed-off-by: Dave Walker (Daviey) <DaveWalker at ubuntu.com>
> ---
> drivers/ata/ahci.c | 10 ++++++++++
> drivers/ata/ata_generic.c | 6 ++++++
> include/linux/pci_ids.h | 1 +
> 3 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index cb05205..e3d9816 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -3037,6 +3037,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;
> +
> /* acquire resources */
> rc = pcim_enable_device(pdev);
> if (rc)
> diff --git a/drivers/ata/ata_generic.c b/drivers/ata/ata_generic.c
> index ecfd22b..b5aa93c 100644
> --- a/drivers/ata/ata_generic.c
> +++ b/drivers/ata/ata_generic.c
> @@ -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, },
> { PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO), },
> { PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_1), },
> { PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_2), },
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 63adefe..84f60fc 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -1262,6 +1262,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
> --
> 1.7.0.4
>
> From 281f43a7aa754ad6a4eda9077548a532c147a246 Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj at kernel.org>
> Date: Tue, 22 Jun 2010 12:27:26 +0200
> Subject: [PATCH 2/2] (pre-stable) ata_generic: implement ATA_GEN_* flags and force enable DMA on MBP 7,1
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> BugLink: http://bugs.launchpad.net/bugs/576601
>
> 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.
>
> Signed-off-by: Tejun Heo <tj at kernel.org>
> Cc: Peer Chen <pchen at nvidia.com>
> Cc: stable at kernel.org
> Reported-by: Anders ??sthus <grapz666 at gmail.com>
> Reported-by: Andreas Graf <andreas_graf at csgraf.de>
> Reported-by: Benoit Gschwind <gschwind at gnu-log.net>
> Reported-by: Damien Cassou <damien.cassou at gmail.com>
> Reported-by: tixetsal at juno.com
> Signed-off-by: Jeff Garzik <jgarzik at redhat.com>
> (backported from commit 281f43a7aa754ad6a4eda9077548a532c147a246 upstream)
> Signed-off-by: Dave Walker (Daviey) <DaveWalker at ubuntu.com>
> ---
> drivers/ata/ata_generic.c | 26 +++++++++++++++++++-------
> 1 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/ata/ata_generic.c b/drivers/ata/ata_generic.c
> index b5aa93c..99e7196 100644
> --- a/drivers/ata/ata_generic.c
> +++ b/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;
I think this patch should have removed the conditional above as it is
now encoded into the driver_data field for the Cenatek devices. Though
it remaining is benign. Tejun you probabally want to kill that for the
upstream version.
> @@ -126,7 +135,7 @@ static int ata_generic_init_one(struct pci_dev *dev, const struct pci_device_id
> 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 pci_dev *dev, const struct pci_device_id
> return rc;
> pcim_pin_device(dev);
> }
> - return ata_pci_sff_init_one(dev, ppi, &generic_sht, NULL);
> + return ata_pci_sff_init_one(dev, ppi, &generic_sht, (void *)id);
> }
>
> static struct pci_device_id ata_generic[] = {
> @@ -167,18 +176,21 @@ 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 },
> { PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO), },
> { PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_1), },
> { PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_2), },
> /* 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, },
> };
Looks good for Ubuntu.
Acked-by: Andy Whitcroft <apw at canonical.com>
-apw
More information about the kernel-team
mailing list