Patchwork ata: make DVD drive recognisable on systems with Sandybridge CPT chipset

login
register
mail settings
Submitter Ming Lei
Date Sept. 15, 2011, 2:27 a.m.
Message ID <1316053631-6555-1-git-send-email-ming.lei@canonical.com>
Download mbox | patch
Permalink /patch/114731/
State New
Headers show

Comments

Ming Lei - Sept. 15, 2011, 2:27 a.m.
From: Ming Lei <tom.leiming@gmail.com>

This quirk patch fixes one kind of bug inside Intel Sandybridge CPT
chipset, see reports from LP794642, LP737388, LP758433, ...

Many guys have reported the problem.

With help from upstream ata maintainer Tejun[1], the problem is found to be
caused by 32bit PIO mode, which is introduced in the commit below:

	commit 0b67c7439fe2a5d76602de36854c88e2beab00b0
	Author: Tejun Heo <tj@kernel.org>
	Date:   Mon Jan 11 17:03:11 2010 +0900

	    ata_piix: enable 32bit PIO on SATA piix

	    Commit 871af1210f13966ab911ed2166e4ab2ce775b99d enabled 32bit PIO
	    for PATA piix but didn't for SATA.  There's no reason not to use 32bit
	    PIO on SATA piix.  Enable it.

	Signed-off-by: Tejun Heo <tj@kernel.org>
	Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
	Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

so introduce the quirk patch to disable 32bit PIO on SATA piix for Sandybridge CPT
chipset, Alan Cox has agreed on the patch[2].

SRU Justification:

Impact:
        - without the patch, DVD drive can't be recognized on Dell
          optiplex 390.
Fix:
        - After applying the patch, DVD drive can be recognized well
	  on Dell optiplex 390.

BugLink: http://bugs.launchpad.net/bugs/794642

Upstream: [1]/[2]

[1], https://bugzilla.kernel.org/show_bug.cgi?id=40592
[2], http://marc.info/?t=131528297300002&r=1&w=2

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/ata/ata_piix.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)
Ming Lei - Sept. 15, 2011, 2:36 a.m.
On Thu, Sep 15, 2011 at 10:27 AM,  <ming.lei@canonical.com> wrote:
> From: Ming Lei <tom.leiming@gmail.com>
>
> This quirk patch fixes one kind of bug inside Intel Sandybridge CPT
> chipset, see reports from LP794642, LP737388, LP758433, ...
>
> Many guys have reported the problem.
>
> With help from upstream ata maintainer Tejun[1], the problem is found to be
> caused by 32bit PIO mode, which is introduced in the commit below:
>
>        commit 0b67c7439fe2a5d76602de36854c88e2beab00b0
>        Author: Tejun Heo <tj@kernel.org>
>        Date:   Mon Jan 11 17:03:11 2010 +0900
>
>            ata_piix: enable 32bit PIO on SATA piix
>
>            Commit 871af1210f13966ab911ed2166e4ab2ce775b99d enabled 32bit PIO
>            for PATA piix but didn't for SATA.  There's no reason not to use 32bit
>            PIO on SATA piix.  Enable it.
>
>        Signed-off-by: Tejun Heo <tj@kernel.org>
>        Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
>        Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
>
> so introduce the quirk patch to disable 32bit PIO on SATA piix for Sandybridge CPT
> chipset, Alan Cox has agreed on the patch[2].
>
> SRU Justification:
>
> Impact:
>        - without the patch, DVD drive can't be recognized on Dell
>          optiplex 390.
> Fix:
>        - After applying the patch, DVD drive can be recognized well
>          on Dell optiplex 390.
>
> BugLink: http://bugs.launchpad.net/bugs/794642
>
> Upstream: [1]/[2]
>
> [1], https://bugzilla.kernel.org/show_bug.cgi?id=40592
> [2], http://marc.info/?t=131528297300002&r=1&w=2
>
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>

Signed-off-by: Ming Lei <ming.lei@canonical.com>

> ---
>  drivers/ata/ata_piix.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
> index 43107e9..eb7ea56 100644
> --- a/drivers/ata/ata_piix.c
> +++ b/drivers/ata/ata_piix.c
> @@ -341,11 +341,12 @@ static struct ata_port_operations piix_sata_ops = {
>  };
>
>  static struct ata_port_operations piix_pata_ops = {
> -       .inherits               = &piix_sata_ops,
> +       .inherits               = &ata_bmdma32_port_ops,
>        .cable_detect           = ata_cable_40wire,
>        .set_piomode            = piix_set_piomode,
>        .set_dmamode            = piix_set_dmamode,
>        .prereset               = piix_pata_prereset,
> +       .sff_irq_check          = piix_irq_check,
>  };
>
>  static struct ata_port_operations piix_vmw_ops = {
> @@ -1585,6 +1586,15 @@ static int __devinit piix_init_one(struct pci_dev *pdev,
>                                "on poweroff and hibernation\n");
>        }
>
> +       /*
> +        * Sandybridge chipset H61/P67/H67 have broken 32 mode up to now
> +        * see https://bugzilla.kernel.org/show_bug.cgi?id=40592
> +        */
> +       if (pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device == 0x1c00)
> +               piix_sata_ops.inherits = &ata_bmdma_port_ops;
> +       else
> +               piix_sata_ops.inherits = &ata_bmdma32_port_ops;
> +
>        port_info[0] = piix_port_info[ent->driver_data];
>        port_info[1] = piix_port_info[ent->driver_data];
>
> --
> 1.7.4.1
>
>
Tim Gardner - Sept. 15, 2011, 1:12 p.m.
On 09/14/2011 08:27 PM, ming.lei@canonical.com wrote:
> From: Ming Lei<tom.leiming@gmail.com>
>
> This quirk patch fixes one kind of bug inside Intel Sandybridge CPT
> chipset, see reports from LP794642, LP737388, LP758433, ...
>
> Many guys have reported the problem.
>
> With help from upstream ata maintainer Tejun[1], the problem is found to be
> caused by 32bit PIO mode, which is introduced in the commit below:
>
> 	commit 0b67c7439fe2a5d76602de36854c88e2beab00b0
> 	Author: Tejun Heo<tj@kernel.org>
> 	Date:   Mon Jan 11 17:03:11 2010 +0900
>
> 	    ata_piix: enable 32bit PIO on SATA piix
>
> 	    Commit 871af1210f13966ab911ed2166e4ab2ce775b99d enabled 32bit PIO
> 	    for PATA piix but didn't for SATA.  There's no reason not to use 32bit
> 	    PIO on SATA piix.  Enable it.
>
> 	Signed-off-by: Tejun Heo<tj@kernel.org>
> 	Cc: Alan Cox<alan@lxorguk.ukuu.org.uk>
> 	Signed-off-by: Jeff Garzik<jgarzik@redhat.com>
>
> so introduce the quirk patch to disable 32bit PIO on SATA piix for Sandybridge CPT
> chipset, Alan Cox has agreed on the patch[2].
>
> SRU Justification:
>
> Impact:
>          - without the patch, DVD drive can't be recognized on Dell
>            optiplex 390.
> Fix:
>          - After applying the patch, DVD drive can be recognized well
> 	  on Dell optiplex 390.
>
> BugLink: http://bugs.launchpad.net/bugs/794642
>
> Upstream: [1]/[2]
>
> [1], https://bugzilla.kernel.org/show_bug.cgi?id=40592
> [2], http://marc.info/?t=131528297300002&r=1&w=2
>
> Signed-off-by: Ming Lei<tom.leiming@gmail.com>
> ---
>   drivers/ata/ata_piix.c |   12 +++++++++++-
>   1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
> index 43107e9..eb7ea56 100644
> --- a/drivers/ata/ata_piix.c
> +++ b/drivers/ata/ata_piix.c
> @@ -341,11 +341,12 @@ static struct ata_port_operations piix_sata_ops = {
>   };
>
>   static struct ata_port_operations piix_pata_ops = {
> -	.inherits		=&piix_sata_ops,
> +	.inherits		=&ata_bmdma32_port_ops,
>   	.cable_detect		= ata_cable_40wire,
>   	.set_piomode		= piix_set_piomode,
>   	.set_dmamode		= piix_set_dmamode,
>   	.prereset		= piix_pata_prereset,
> +	.sff_irq_check		= piix_irq_check,
>   };
>
>   static struct ata_port_operations piix_vmw_ops = {
> @@ -1585,6 +1586,15 @@ static int __devinit piix_init_one(struct pci_dev *pdev,
>   				"on poweroff and hibernation\n");
>   	}
>
> +	/*
> +	 * Sandybridge chipset H61/P67/H67 have broken 32 mode up to now
> +	 * see https://bugzilla.kernel.org/show_bug.cgi?id=40592
> +	 */
> +	if (pdev->vendor == PCI_VENDOR_ID_INTEL&&  pdev->device == 0x1c00)
> +		piix_sata_ops.inherits =&ata_bmdma_port_ops;
> +	else
> +		piix_sata_ops.inherits =&ata_bmdma32_port_ops;
> +
>   	port_info[0] = piix_port_info[ent->driver_data];
>   	port_info[1] = piix_port_info[ent->driver_data];
>

Acked-by: Tim Gardner <tim.gardner@canonical.com>
Leann Ogasawara - Sept. 15, 2011, 1:39 p.m.
On Thu, 2011-09-15 at 10:27 +0800, ming.lei@canonical.com wrote:
> From: Ming Lei <tom.leiming@gmail.com>
> 
> This quirk patch fixes one kind of bug inside Intel Sandybridge CPT
> chipset, see reports from LP794642, LP737388, LP758433, ...
> 
> Many guys have reported the problem.
> 
> With help from upstream ata maintainer Tejun[1], the problem is found to be
> caused by 32bit PIO mode, which is introduced in the commit below:
> 
> 	commit 0b67c7439fe2a5d76602de36854c88e2beab00b0
> 	Author: Tejun Heo <tj@kernel.org>
> 	Date:   Mon Jan 11 17:03:11 2010 +0900
> 
> 	    ata_piix: enable 32bit PIO on SATA piix
> 
> 	    Commit 871af1210f13966ab911ed2166e4ab2ce775b99d enabled 32bit PIO
> 	    for PATA piix but didn't for SATA.  There's no reason not to use 32bit
> 	    PIO on SATA piix.  Enable it.
> 
> 	Signed-off-by: Tejun Heo <tj@kernel.org>
> 	Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> 	Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> 
> so introduce the quirk patch to disable 32bit PIO on SATA piix for Sandybridge CPT
> chipset, Alan Cox has agreed on the patch[2].
> 
> SRU Justification:
> 
> Impact:
>         - without the patch, DVD drive can't be recognized on Dell
>           optiplex 390.
> Fix:
>         - After applying the patch, DVD drive can be recognized well
> 	  on Dell optiplex 390.
> 
> BugLink: http://bugs.launchpad.net/bugs/794642
> 
> Upstream: [1]/[2]
> 
> [1], https://bugzilla.kernel.org/show_bug.cgi?id=40592
> [2], http://marc.info/?t=131528297300002&r=1&w=2
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>

Signed-off-by: Leann Ogasawara <leann.ogasawara@canonical.com>

Given this is hw specific (ie minimal risk of regression), has positive
test feedback, and appears will make it's way upstream, I've gone ahead
and applied this as a SAUCE patch to Oneiric master-next.

Please be sure to add the SRU justification to the bug report.

Thanks,
Leann

> ---
>  drivers/ata/ata_piix.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
> index 43107e9..eb7ea56 100644
> --- a/drivers/ata/ata_piix.c
> +++ b/drivers/ata/ata_piix.c
> @@ -341,11 +341,12 @@ static struct ata_port_operations piix_sata_ops = {
>  };
>  
>  static struct ata_port_operations piix_pata_ops = {
> -	.inherits		= &piix_sata_ops,
> +	.inherits		= &ata_bmdma32_port_ops,
>  	.cable_detect		= ata_cable_40wire,
>  	.set_piomode		= piix_set_piomode,
>  	.set_dmamode		= piix_set_dmamode,
>  	.prereset		= piix_pata_prereset,
> +	.sff_irq_check		= piix_irq_check,
>  };
>  
>  static struct ata_port_operations piix_vmw_ops = {
> @@ -1585,6 +1586,15 @@ static int __devinit piix_init_one(struct pci_dev *pdev,
>  				"on poweroff and hibernation\n");
>  	}
>  
> +	/*
> +	 * Sandybridge chipset H61/P67/H67 have broken 32 mode up to now
> +	 * see https://bugzilla.kernel.org/show_bug.cgi?id=40592
> +	 */
> +	if (pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device == 0x1c00)
> +		piix_sata_ops.inherits = &ata_bmdma_port_ops;
> +	else
> +		piix_sata_ops.inherits = &ata_bmdma32_port_ops;
> +
>  	port_info[0] = piix_port_info[ent->driver_data];
>  	port_info[1] = piix_port_info[ent->driver_data];
>  
> -- 
> 1.7.4.1
> 
>
Ming Lei - Sept. 15, 2011, 4:14 p.m.
On Thu, Sep 15, 2011 at 9:39 PM, Leann Ogasawara
<leann.ogasawara@canonical.com> wrote:
> On Thu, 2011-09-15 at 10:27 +0800, ming.lei@canonical.com wrote:
>> From: Ming Lei <tom.leiming@gmail.com>
>>
>> This quirk patch fixes one kind of bug inside Intel Sandybridge CPT
>> chipset, see reports from LP794642, LP737388, LP758433, ...
>>
>> Many guys have reported the problem.
>>
>> With help from upstream ata maintainer Tejun[1], the problem is found to be
>> caused by 32bit PIO mode, which is introduced in the commit below:
>>
>>       commit 0b67c7439fe2a5d76602de36854c88e2beab00b0
>>       Author: Tejun Heo <tj@kernel.org>
>>       Date:   Mon Jan 11 17:03:11 2010 +0900
>>
>>           ata_piix: enable 32bit PIO on SATA piix
>>
>>           Commit 871af1210f13966ab911ed2166e4ab2ce775b99d enabled 32bit PIO
>>           for PATA piix but didn't for SATA.  There's no reason not to use 32bit
>>           PIO on SATA piix.  Enable it.
>>
>>       Signed-off-by: Tejun Heo <tj@kernel.org>
>>       Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
>>       Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
>>
>> so introduce the quirk patch to disable 32bit PIO on SATA piix for Sandybridge CPT
>> chipset, Alan Cox has agreed on the patch[2].
>>
>> SRU Justification:
>>
>> Impact:
>>         - without the patch, DVD drive can't be recognized on Dell
>>           optiplex 390.
>> Fix:
>>         - After applying the patch, DVD drive can be recognized well
>>         on Dell optiplex 390.
>>
>> BugLink: http://bugs.launchpad.net/bugs/794642
>>
>> Upstream: [1]/[2]
>>
>> [1], https://bugzilla.kernel.org/show_bug.cgi?id=40592
>> [2], http://marc.info/?t=131528297300002&r=1&w=2
>>
>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>
> Signed-off-by: Leann Ogasawara <leann.ogasawara@canonical.com>
>
> Given this is hw specific (ie minimal risk of regression), has positive
> test feedback, and appears will make it's way upstream, I've gone ahead
> and applied this as a SAUCE patch to Oneiric master-next.
>
> Please be sure to add the SRU justification to the bug report.

I have done it.

thanks,
--
Ming Lei

>
>> ---
>>  drivers/ata/ata_piix.c |   12 +++++++++++-
>>  1 files changed, 11 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
>> index 43107e9..eb7ea56 100644
>> --- a/drivers/ata/ata_piix.c
>> +++ b/drivers/ata/ata_piix.c
>> @@ -341,11 +341,12 @@ static struct ata_port_operations piix_sata_ops = {
>>  };
>>
>>  static struct ata_port_operations piix_pata_ops = {
>> -     .inherits               = &piix_sata_ops,
>> +     .inherits               = &ata_bmdma32_port_ops,
>>       .cable_detect           = ata_cable_40wire,
>>       .set_piomode            = piix_set_piomode,
>>       .set_dmamode            = piix_set_dmamode,
>>       .prereset               = piix_pata_prereset,
>> +     .sff_irq_check          = piix_irq_check,
>>  };
>>
>>  static struct ata_port_operations piix_vmw_ops = {
>> @@ -1585,6 +1586,15 @@ static int __devinit piix_init_one(struct pci_dev *pdev,
>>                               "on poweroff and hibernation\n");
>>       }
>>
>> +     /*
>> +      * Sandybridge chipset H61/P67/H67 have broken 32 mode up to now
>> +      * see https://bugzilla.kernel.org/show_bug.cgi?id=40592
>> +      */
>> +     if (pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device == 0x1c00)
>> +             piix_sata_ops.inherits = &ata_bmdma_port_ops;
>> +     else
>> +             piix_sata_ops.inherits = &ata_bmdma32_port_ops;
>> +
>>       port_info[0] = piix_port_info[ent->driver_data];
>>       port_info[1] = piix_port_info[ent->driver_data];
>>
>> --
>> 1.7.4.1
>>
>>
>
>
>
>

Patch

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 43107e9..eb7ea56 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -341,11 +341,12 @@  static struct ata_port_operations piix_sata_ops = {
 };
 
 static struct ata_port_operations piix_pata_ops = {
-	.inherits		= &piix_sata_ops,
+	.inherits		= &ata_bmdma32_port_ops,
 	.cable_detect		= ata_cable_40wire,
 	.set_piomode		= piix_set_piomode,
 	.set_dmamode		= piix_set_dmamode,
 	.prereset		= piix_pata_prereset,
+	.sff_irq_check		= piix_irq_check,
 };
 
 static struct ata_port_operations piix_vmw_ops = {
@@ -1585,6 +1586,15 @@  static int __devinit piix_init_one(struct pci_dev *pdev,
 				"on poweroff and hibernation\n");
 	}
 
+	/*
+	 * Sandybridge chipset H61/P67/H67 have broken 32 mode up to now
+	 * see https://bugzilla.kernel.org/show_bug.cgi?id=40592
+	 */
+	if (pdev->vendor == PCI_VENDOR_ID_INTEL && pdev->device == 0x1c00)
+		piix_sata_ops.inherits = &ata_bmdma_port_ops;
+	else
+		piix_sata_ops.inherits = &ata_bmdma32_port_ops;
+
 	port_info[0] = piix_port_info[ent->driver_data];
 	port_info[1] = piix_port_info[ent->driver_data];