[PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[ming.lei]

From: Ming Lei <ming.lei@canonical.com>

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

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

Many guys also have reported the problem before:

	https://bugs.launchpad.net/bugs/737388
	https://bugs.launchpad.net/bugs/794642
	......

With help from Tejun, the problem is found to be caused by 32bit PIO
mode, so introduce the quirk patch to disable 32bit PIO on SATA piix
for Sandybridge CPT chipset.

Signed-off-by: Tejun Heo <htejun@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.5.4

Re: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Tejun Heo]

Hello,

(cc'ing Jeff)

On Thu, Sep 22, 2011 at 09:55:12AM +0800, ming.lei@canonical.com wrote:
> From: Ming Lei <ming.lei@canonical.com>
> 
> This quirk patch fixes one kind of bug inside Intel Sandybridge CPT
> chipset, see reports from
> 
> 	https://bugzilla.kernel.org/show_bug.cgi?id=40592.
> 
> Many guys also have reported the problem before:
> 
> 	https://bugs.launchpad.net/bugs/737388
> 	https://bugs.launchpad.net/bugs/794642
> 	......
> 
> With help from Tejun, the problem is found to be caused by 32bit PIO
> mode, so introduce the quirk patch to disable 32bit PIO on SATA piix
> for Sandybridge CPT chipset.

Have we successfully localized the problem to SNB?  If so, great.

>  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;
> +

No, this wouldn't work.  Ops inheritance isn't dynamic.

Please define a separate ata_port_operations for controller which
require 16bit PIO - piix_pata16_ops, create a new controller id (say,
ich_snb_pata), add an accompanying port_info entry and device_id
entry.

Thank you.

-- 
tejun

RE: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Heasley, Seth]

>Have we successfully localized the problem to SNB?  If so, great.

No, we haven't.  I've reproduced the issue on two newer Intel chipsets.  In IDE mode, ATAPI just isn't working on SATA3 ports.  With the provided patch, the issue is resolved.  At what cost, I can't say.  But if a patch will go in for the 6 Series, we need to apply it to the other platforms as well.  I can provide the DeviceIDs.

--Seth

Re: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Ming Lei]

Hi,

On Sat, Sep 24, 2011 at 9:58 AM, Tejun Heo <htejun@gmail.com> wrote:
> Hello,
> No, this wouldn't work.  Ops inheritance isn't dynamic.
>

I am sure that I have tested the patch and it does work.

> Please define a separate ata_port_operations for controller which
> require 16bit PIO - piix_pata16_ops, create a new controller id (say,
> ich_snb_pata), add an accompanying port_info entry and device_id
> entry.

In fact, I am not familiar with sata, but just want to fix the problem.
If you have a better patch, please ignore mine and apply yours.


On Sat, Sep 24, 2011 at 12:28 PM, Heasley, Seth <seth.heasley@intel.com> wrote:
>>Have we successfully localized the problem to SNB?  If so, great.
>
> No, we haven't.  I've reproduced the issue on two newer Intel chipsets.  In > IDE mode, ATAPI just isn't working on SATA3 ports.  With the provided
> patch, the issue is resolved.  At what cost, I can't say.  But if a patch will
> go in for the 6 Series, we need to apply it to the other platforms as well.  I > can provide the DeviceIDs.

I have seen someone reported the same problem on the device with
pci device id of 0x1c01[1]. I have asked them to test the patch but without
any response, so I had to not include the dev id in the patch.


[1], https://bugs.launchpad.net/bugs/782389

thanks,
--
Ming Lei

Re: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Tejun Heo]

Hello,

On Sat, Sep 24, 2011 at 09:34:09PM +0800, Ming Lei wrote:
> On Sat, Sep 24, 2011 at 9:58 AM, Tejun Heo <htejun@gmail.com> wrote:
> > Hello,
> > No, this wouldn't work.  Ops inheritance isn't dynamic.
> 
> I am sure that I have tested the patch and it does work.

Yeah, but not by design.  That field is assumed to be static.
Inheritance currently is finalized during the first use of the
operation structure, where the first use also includes being inherited
by other ops structure, so doing it like that is asking for obscure
bugs.

> > Please define a separate ata_port_operations for controller which
> > require 16bit PIO - piix_pata16_ops, create a new controller id (say,
> > ich_snb_pata), add an accompanying port_info entry and device_id
> > entry.
> 
> In fact, I am not familiar with sata, but just want to fix the problem.
> If you have a better patch, please ignore mine and apply yours.

Sure I can do that but it would be better if you can revise your
patch.  Please take a look at how different ops are mapped to
different device IDs.  You just need to create another variant to be
mapped to the problematic device IDs.

> On Sat, Sep 24, 2011 at 12:28 PM, Heasley, Seth <seth.heasley@intel.com> wrote:
> >>Have we successfully localized the problem to SNB?  If so, great.
> >
> > No, we haven't.  I've reproduced the issue on two newer Intel chipsets.  In > IDE mode, ATAPI just isn't working on SATA3 ports.  With the provided
> > patch, the issue is resolved.  At what cost, I can't say.  But if a patch will
> > go in for the 6 Series, we need to apply it to the other platforms as well.  I > can provide the DeviceIDs.
> 
> I have seen someone reported the same problem on the device with
> pci device id of 0x1c01[1]. I have asked them to test the patch but without
> any response, so I had to not include the dev id in the patch.

Developing partial blacklist w/o knowing what's going on is messy.  If
we discover that something wasn't quite what we suspected it was and
had to revise, it'll be tricky to verify whicn ones need to remain.
Alan, can someone from intel verify the issue?  Is there an errata we
can look at?

Thanks.

-- 
tejun

Re: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Ming Lei]

Hi,

On Sun, Sep 25, 2011 at 8:03 AM, Tejun Heo <htejun@gmail.com> wrote:
> Sure I can do that but it would be better if you can revise your
> patch.  Please take a look at how different ops are mapped to
> different device IDs.  You just need to create another variant to be
> mapped to the problematic device IDs.

OK， I will do it.

>
>> On Sat, Sep 24, 2011 at 12:28 PM, Heasley, Seth <seth.heasley@intel.com> wrote:
>> >>Have we successfully localized the problem to SNB?  If so, great.
>> >
>> > No, we haven't.  I've reproduced the issue on two newer Intel chipsets.  In > IDE mode, ATAPI just isn't working on SATA3 ports.  With the provided
>> > patch, the issue is resolved.  At what cost, I can't say.  But if a patch will
>> > go in for the 6 Series, we need to apply it to the other platforms as well.  I > can provide the DeviceIDs.
>>

Seth, could you provide the DeviceIDs so that we can make a quirk for them
now?

>> I have seen someone reported the same problem on the device with
>> pci device id of 0x1c01[1]. I have asked them to test the patch but without
>> any response, so I had to not include the dev id in the patch.
>
> Developing partial blacklist w/o knowing what's going on is messy.  If
> we discover that something wasn't quite what we suspected it was and
> had to revise, it'll be tricky to verify whicn ones need to remain.
> Alan, can someone from intel verify the issue?  Is there an errata we
> can look at?

thanks,
---
Ming Lei

Re: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Ming Lei]

Hi Tejun,

On Mon, Sep 26, 2011 at 2:51 PM, Ming Lei <ming.lei@canonical.com> wrote:
> Hi,
>
> On Sun, Sep 25, 2011 at 8:03 AM, Tejun Heo <htejun@gmail.com> wrote:
>> Sure I can do that but it would be better if you can revise your
>> patch.  Please take a look at how different ops are mapped to
>> different device IDs.  You just need to create another variant to be
>> mapped to the problematic device IDs.
>
> OK， I will do it.

How about this one below?

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 43107e9..b35086b 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -147,6 +147,7 @@ enum piix_controller_ids {
 	ich8m_apple_sata,	/* locks up on second port enable */
 	tolapai_sata,
 	piix_pata_vmw,			/* PIIX4 for VMware, spurious DMA_ERR */
+	ich8_sata_snb,
 };

 struct piix_map_db {
@@ -298,7 +299,7 @@ static const struct pci_device_id piix_pci_tbl[] = {
 	/* SATA Controller IDE (PCH) */
 	{ 0x8086, 0x3b2e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
 	/* SATA Controller IDE (CPT) */
-	{ 0x8086, 0x1c00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
+	{ 0x8086, 0x1c00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
 	/* SATA Controller IDE (CPT) */
 	{ 0x8086, 0x1c01, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
 	/* SATA Controller IDE (CPT) */
@@ -335,6 +336,11 @@ static struct scsi_host_template piix_sht = {
 	ATA_BMDMA_SHT(DRV_NAME),
 };

+static struct ata_port_operations piix_sata_snb_ops = {
+	.inherits		= &ata_bmdma_port_ops,
+	.sff_irq_check		= piix_irq_check,
+};
+
 static struct ata_port_operations piix_sata_ops = {
 	.inherits		= &ata_bmdma32_port_ops,
 	.sff_irq_check		= piix_irq_check,
@@ -478,6 +484,7 @@ static const struct piix_map_db *piix_map_db_table[] = {
 	[ich8_2port_sata]	= &ich8_2port_map_db,
 	[ich8m_apple_sata]	= &ich8m_apple_map_db,
 	[tolapai_sata]		= &tolapai_map_db,
+	[ich8_sata_snb]		= &ich8_map_db,
 };

 static struct ata_port_info piix_port_info[] = {
@@ -606,6 +613,15 @@ static struct ata_port_info piix_port_info[] = {
 		.port_ops	= &piix_vmw_ops,
 	},

+	[ich8_sata_snb] =
+	{
+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR,
+		.pio_mask	= ATA_PIO4,
+		.mwdma_mask	= ATA_MWDMA2,
+		.udma_mask	= ATA_UDMA6,
+		.port_ops	= &piix_sata_snb_ops,
+	},
+
 };

 static struct pci_bits piix_enable_bits[] = {


thanks,
--
Ming Lei

Re: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Alan Cox]

> Developing partial blacklist w/o knowing what's going on is messy.  If
> we discover that something wasn't quite what we suspected it was and
> had to revise, it'll be tricky to verify whicn ones need to remain.
> Alan, can someone from intel verify the issue? 

Seth is "someone from Intel" 8)

Alan

RE: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Heasley, Seth]

>Seth, could you provide the DeviceIDs so that we can make a quirk for
>them
>now?

The DeviceIDs I've reproduced the issue with are the following:

1c00, 1c01 (6 Series/Cougar Point)
1d00 (Patsburg)
1e00, 1e01 (Panther Point)

>> Developing partial blacklist w/o knowing what's going on is messy.  If
>> we discover that something wasn't quite what we suspected it was and
>> had to revise, it'll be tricky to verify whicn ones need to remain.
>> Alan, can someone from intel verify the issue?  Is there an errata we
>> can look at?

Adding Kristin Accardi to the thread.  I've repro'd the issue on several systems and can verify any patch that's proposed, but I don't have the SATA expertise to provide much else.

-Seth	

Re: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Ming Lei]

Hi Tejun,

On Mon, Sep 26, 2011 at 3:23 PM, Ming Lei <ming.lei@canonical.com> wrote:

> How about this one below?
>
> diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
> index 43107e9..b35086b 100644
> --- a/drivers/ata/ata_piix.c
> +++ b/drivers/ata/ata_piix.c
> @@ -147,6 +147,7 @@ enum piix_controller_ids {
>        ich8m_apple_sata,       /* locks up on second port enable */
>        tolapai_sata,
>        piix_pata_vmw,                  /* PIIX4 for VMware, spurious DMA_ERR */
> +       ich8_sata_snb,
>  };
>
>  struct piix_map_db {
> @@ -298,7 +299,7 @@ static const struct pci_device_id piix_pci_tbl[] = {
>        /* SATA Controller IDE (PCH) */
>        { 0x8086, 0x3b2e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
>        /* SATA Controller IDE (CPT) */
> -       { 0x8086, 0x1c00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
> +       { 0x8086, 0x1c00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
>        /* SATA Controller IDE (CPT) */
>        { 0x8086, 0x1c01, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
>        /* SATA Controller IDE (CPT) */
> @@ -335,6 +336,11 @@ static struct scsi_host_template piix_sht = {
>        ATA_BMDMA_SHT(DRV_NAME),
>  };
>
> +static struct ata_port_operations piix_sata_snb_ops = {
> +       .inherits               = &ata_bmdma_port_ops,
> +       .sff_irq_check          = piix_irq_check,
> +};
> +
>  static struct ata_port_operations piix_sata_ops = {
>        .inherits               = &ata_bmdma32_port_ops,
>        .sff_irq_check          = piix_irq_check,
> @@ -478,6 +484,7 @@ static const struct piix_map_db *piix_map_db_table[] = {
>        [ich8_2port_sata]       = &ich8_2port_map_db,
>        [ich8m_apple_sata]      = &ich8m_apple_map_db,
>        [tolapai_sata]          = &tolapai_map_db,
> +       [ich8_sata_snb]         = &ich8_map_db,
>  };
>
>  static struct ata_port_info piix_port_info[] = {
> @@ -606,6 +613,15 @@ static struct ata_port_info piix_port_info[] = {
>                .port_ops       = &piix_vmw_ops,
>        },
>
> +       [ich8_sata_snb] =
> +       {
> +               .flags          = PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR,
> +               .pio_mask       = ATA_PIO4,
> +               .mwdma_mask     = ATA_MWDMA2,
> +               .udma_mask      = ATA_UDMA6,
> +               .port_ops       = &piix_sata_snb_ops,
> +       },
> +
>  };
>
>  static struct pci_bits piix_enable_bits[] = {

After testing, I found the patch above does not make DVD drive work.
But plus the below[1], dvd drive starts to working. Since piix_sidpr_sata_ops
is hardcode in ata_piix.c, looks like it is a bit difficult to figure
out a clean fix.

Tejun, could you give some comments about it?

thanks,
--
Ming Lei

[1],
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 150d286..3b3785d 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -376,7 +376,7 @@ static struct scsi_host_template piix_sidpr_sht = {
 };

 static struct ata_port_operations piix_sidpr_sata_ops = {
-	.inherits		= &piix_sata_ops,
+	.inherits		= &piix_sata_snb_ops,
 	.hardreset		= sata_std_hardreset,
 	.scr_read		= piix_sidpr_scr_read,
 	.scr_write		= piix_sidpr_scr_write,

Re: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Tejun Heo]

Hello,

Sorry about the delay.

On Wed, Sep 28, 2011 at 12:58:25PM +0800, Ming Lei wrote:
> On Mon, Sep 26, 2011 at 3:23 PM, Ming Lei <ming.lei@canonical.com> wrote:
> 
> > How about this one below?

The patch itself looks good but please read on.

> After testing, I found the patch above does not make DVD drive work.
> But plus the below[1], dvd drive starts to working. Since piix_sidpr_sata_ops
> is hardcode in ata_piix.c, looks like it is a bit difficult to figure
> out a clean fix.

Sorry about that.  I forgot that sidpr ops selection was dynamic. :(
Hmmm... it seems we already have dynamic switch to control 32bit PIO -
ATA_PFLAG_PIO32.  How about the following then?

* define PIIX_FLAG_PIO16 and set it in the port_info for SNBs.

* define piix_port_start() which sets PFLAG_PIO32 iff !PIIX_FLAG_PIO16
  and then call ata_bmdma_port_start() and use it in piix_sata_ops.

Thank you.

-- 
tejun

Re: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Ming Lei]

Hi,

On Fri, Sep 30, 2011 at 11:09 AM, Tejun Heo <htejun@gmail.com> wrote:

>
> * define PIIX_FLAG_PIO16 and set it in the port_info for SNBs.
>
> * define piix_port_start() which sets PFLAG_PIO32 iff !PIIX_FLAG_PIO16
>  and then call ata_bmdma_port_start() and use it in piix_sata_ops.

Looks It is really a nice and clean idea to fix the problem, follows
the patch, which
has been tested OK to recognize DVD in machine with device ID 0x1c00
chipset.

Seth, could you help to test the patch on your machines with other pci device
IDs(0x1c01, 0x1d00, 0x1e00, 0x1e01)?

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 43107e9..70095be 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -113,6 +113,8 @@ enum {
 	PIIX_PATA_FLAGS		= ATA_FLAG_SLAVE_POSS,
 	PIIX_SATA_FLAGS		= ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR,

+	PIIX_FLAG_PIO16		= ATA_FLAG_PIO16,
+
 	PIIX_80C_PRI		= (1 << 5) | (1 << 4),
 	PIIX_80C_SEC		= (1 << 7) | (1 << 6),

@@ -147,6 +149,7 @@ enum piix_controller_ids {
 	ich8m_apple_sata,	/* locks up on second port enable */
 	tolapai_sata,
 	piix_pata_vmw,			/* PIIX4 for VMware, spurious DMA_ERR */
+	ich8_sata_snb,
 };

 struct piix_map_db {
@@ -177,6 +180,7 @@ static int piix_sidpr_scr_write(struct ata_link *link,
 static int piix_sidpr_set_lpm(struct ata_link *link, enum
ata_lpm_policy policy,
 			      unsigned hints);
 static bool piix_irq_check(struct ata_port *ap);
+static int piix_port_start(struct ata_port *ap);
 #ifdef CONFIG_PM
 static int piix_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
 static int piix_pci_device_resume(struct pci_dev *pdev);
@@ -298,21 +302,21 @@ static const struct pci_device_id piix_pci_tbl[] = {
 	/* SATA Controller IDE (PCH) */
 	{ 0x8086, 0x3b2e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
 	/* SATA Controller IDE (CPT) */
-	{ 0x8086, 0x1c00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
+	{ 0x8086, 0x1c00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
 	/* SATA Controller IDE (CPT) */
-	{ 0x8086, 0x1c01, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
+	{ 0x8086, 0x1c01, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
 	/* SATA Controller IDE (CPT) */
 	{ 0x8086, 0x1c08, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_2port_sata },
 	/* SATA Controller IDE (CPT) */
 	{ 0x8086, 0x1c09, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_2port_sata },
 	/* SATA Controller IDE (PBG) */
-	{ 0x8086, 0x1d00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
+	{ 0x8086, 0x1d00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
 	/* SATA Controller IDE (PBG) */
 	{ 0x8086, 0x1d08, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_2port_sata },
 	/* SATA Controller IDE (Panther Point) */
-	{ 0x8086, 0x1e00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
+	{ 0x8086, 0x1e00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
 	/* SATA Controller IDE (Panther Point) */
-	{ 0x8086, 0x1e01, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
+	{ 0x8086, 0x1e01, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
 	/* SATA Controller IDE (Panther Point) */
 	{ 0x8086, 0x1e08, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_2port_sata },
 	/* SATA Controller IDE (Panther Point) */
@@ -338,6 +342,7 @@ static struct scsi_host_template piix_sht = {
 static struct ata_port_operations piix_sata_ops = {
 	.inherits		= &ata_bmdma32_port_ops,
 	.sff_irq_check		= piix_irq_check,
+	.port_start		= piix_port_start,
 };

 static struct ata_port_operations piix_pata_ops = {
@@ -478,6 +483,7 @@ static const struct piix_map_db *piix_map_db_table[] = {
 	[ich8_2port_sata]	= &ich8_2port_map_db,
 	[ich8m_apple_sata]	= &ich8m_apple_map_db,
 	[tolapai_sata]		= &tolapai_map_db,
+	[ich8_sata_snb]		= &ich8_map_db,
 };

 static struct ata_port_info piix_port_info[] = {
@@ -606,6 +612,19 @@ static struct ata_port_info piix_port_info[] = {
 		.port_ops	= &piix_vmw_ops,
 	},

+	/*
+	 * some Sandybridge chipsets have broken 32 mode up to now,
+	 * see https://bugzilla.kernel.org/show_bug.cgi?id=40592
+	 */
+	[ich8_sata_snb] =
+	{
+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR | PIIX_FLAG_PIO16,
+		.pio_mask	= ATA_PIO4,
+		.mwdma_mask	= ATA_MWDMA2,
+		.udma_mask	= ATA_UDMA6,
+		.port_ops	= &piix_sata_ops,
+	},
+
 };

 static struct pci_bits piix_enable_bits[] = {
@@ -649,6 +668,14 @@ static const struct ich_laptop ich_laptop[] = {
 	{ 0, }
 };

+static int piix_port_start(struct ata_port *ap)
+{
+	if (!(ap->flags & PIIX_FLAG_PIO16))
+		ap->pflags |= ATA_PFLAG_PIO32 | ATA_PFLAG_PIO32CHANGE;
+
+	return ata_bmdma_port_start(ap);
+}
+
 /**
  *	ich_pata_cable_detect - Probe host controller cable detect info
  *	@ap: Port for which cable detect info is desired
diff --git a/include/linux/libata.h b/include/linux/libata.h
index efd6f98..dc68de5 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -207,6 +207,7 @@ enum {
 	ATA_FLAG_SW_ACTIVITY	= (1 << 22), /* driver supports sw activity
 					      * led */
 	ATA_FLAG_NO_DIPM	= (1 << 23), /* host not happy with DIPM */
+	ATA_FLAG_PIO16		= (1 << 24),  /*16bit PIO */

 	/* bits 24:31 of ap->flags are reserved for LLD specific flags */

--
Ming Lei

RE: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Heasley, Seth]

>Looks It is really a nice and clean idea to fix the problem, follows
>the patch, which
>has been tested OK to recognize DVD in machine with device ID 0x1c00
>chipset.
>
>Seth, could you help to test the patch on your machines with other pci
>device
>IDs(0x1c01, 0x1d00, 0x1e00, 0x1e01)?

I've tested it on two of the three platforms.  Everything looks good.  I'll try to get all DeviceIDs tested by the end of the day.

-Seth

>
>diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
>index 43107e9..70095be 100644
>--- a/drivers/ata/ata_piix.c
>+++ b/drivers/ata/ata_piix.c
>@@ -113,6 +113,8 @@ enum {
> 	PIIX_PATA_FLAGS		= ATA_FLAG_SLAVE_POSS,
> 	PIIX_SATA_FLAGS		= ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR,
>
>+	PIIX_FLAG_PIO16		= ATA_FLAG_PIO16,
>+
> 	PIIX_80C_PRI		= (1 << 5) | (1 << 4),
> 	PIIX_80C_SEC		= (1 << 7) | (1 << 6),
>
>@@ -147,6 +149,7 @@ enum piix_controller_ids {
> 	ich8m_apple_sata,	/* locks up on second port enable */
> 	tolapai_sata,
> 	piix_pata_vmw,			/* PIIX4 for VMware, spurious
>DMA_ERR */
>+	ich8_sata_snb,
> };
>
> struct piix_map_db {
>@@ -177,6 +180,7 @@ static int piix_sidpr_scr_write(struct ata_link
>*link,
> static int piix_sidpr_set_lpm(struct ata_link *link, enum
>ata_lpm_policy policy,
> 			      unsigned hints);
> static bool piix_irq_check(struct ata_port *ap);
>+static int piix_port_start(struct ata_port *ap);
> #ifdef CONFIG_PM
> static int piix_pci_device_suspend(struct pci_dev *pdev, pm_message_t
>mesg);
> static int piix_pci_device_resume(struct pci_dev *pdev);
>@@ -298,21 +302,21 @@ static const struct pci_device_id piix_pci_tbl[] =
>{
> 	/* SATA Controller IDE (PCH) */
> 	{ 0x8086, 0x3b2e, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
> 	/* SATA Controller IDE (CPT) */
>-	{ 0x8086, 0x1c00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
>+	{ 0x8086, 0x1c00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
> 	/* SATA Controller IDE (CPT) */
>-	{ 0x8086, 0x1c01, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
>+	{ 0x8086, 0x1c01, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
> 	/* SATA Controller IDE (CPT) */
> 	{ 0x8086, 0x1c08, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_2port_sata },
> 	/* SATA Controller IDE (CPT) */
> 	{ 0x8086, 0x1c09, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_2port_sata },
> 	/* SATA Controller IDE (PBG) */
>-	{ 0x8086, 0x1d00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
>+	{ 0x8086, 0x1d00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
> 	/* SATA Controller IDE (PBG) */
> 	{ 0x8086, 0x1d08, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_2port_sata },
> 	/* SATA Controller IDE (Panther Point) */
>-	{ 0x8086, 0x1e00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
>+	{ 0x8086, 0x1e00, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
> 	/* SATA Controller IDE (Panther Point) */
>-	{ 0x8086, 0x1e01, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata },
>+	{ 0x8086, 0x1e01, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_sata_snb },
> 	/* SATA Controller IDE (Panther Point) */
> 	{ 0x8086, 0x1e08, PCI_ANY_ID, PCI_ANY_ID, 0, 0, ich8_2port_sata },
> 	/* SATA Controller IDE (Panther Point) */
>@@ -338,6 +342,7 @@ static struct scsi_host_template piix_sht = {
> static struct ata_port_operations piix_sata_ops = {
> 	.inherits		= &ata_bmdma32_port_ops,
> 	.sff_irq_check		= piix_irq_check,
>+	.port_start		= piix_port_start,
> };
>
> static struct ata_port_operations piix_pata_ops = {
>@@ -478,6 +483,7 @@ static const struct piix_map_db *piix_map_db_table[]
>= {
> 	[ich8_2port_sata]	= &ich8_2port_map_db,
> 	[ich8m_apple_sata]	= &ich8m_apple_map_db,
> 	[tolapai_sata]		= &tolapai_map_db,
>+	[ich8_sata_snb]		= &ich8_map_db,
> };
>
> static struct ata_port_info piix_port_info[] = {
>@@ -606,6 +612,19 @@ static struct ata_port_info piix_port_info[] = {
> 		.port_ops	= &piix_vmw_ops,
> 	},
>
>+	/*
>+	 * some Sandybridge chipsets have broken 32 mode up to now,
>+	 * see https://bugzilla.kernel.org/show_bug.cgi?id=40592
>+	 */
>+	[ich8_sata_snb] =
>+	{
>+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR |
>PIIX_FLAG_PIO16,
>+		.pio_mask	= ATA_PIO4,
>+		.mwdma_mask	= ATA_MWDMA2,
>+		.udma_mask	= ATA_UDMA6,
>+		.port_ops	= &piix_sata_ops,
>+	},
>+
> };
>
> static struct pci_bits piix_enable_bits[] = {
>@@ -649,6 +668,14 @@ static const struct ich_laptop ich_laptop[] = {
> 	{ 0, }
> };
>
>+static int piix_port_start(struct ata_port *ap)
>+{
>+	if (!(ap->flags & PIIX_FLAG_PIO16))
>+		ap->pflags |= ATA_PFLAG_PIO32 | ATA_PFLAG_PIO32CHANGE;
>+
>+	return ata_bmdma_port_start(ap);
>+}
>+
> /**
>  *	ich_pata_cable_detect - Probe host controller cable detect info
>  *	@ap: Port for which cable detect info is desired
>diff --git a/include/linux/libata.h b/include/linux/libata.h
>index efd6f98..dc68de5 100644
>--- a/include/linux/libata.h
>+++ b/include/linux/libata.h
>@@ -207,6 +207,7 @@ enum {
> 	ATA_FLAG_SW_ACTIVITY	= (1 << 22), /* driver supports sw
>activity
> 					      * led */
> 	ATA_FLAG_NO_DIPM	= (1 << 23), /* host not happy with DIPM */
>+	ATA_FLAG_PIO16		= (1 << 24),  /*16bit PIO */
>
> 	/* bits 24:31 of ap->flags are reserved for LLD specific flags */
>
>--
>Ming Lei

Re: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Tejun Heo]

Hello,

On Fri, Sep 30, 2011 at 04:39:27PM +0800, Ming Lei wrote:
> On Fri, Sep 30, 2011 at 11:09 AM, Tejun Heo <htejun@gmail.com> wrote:
> Looks It is really a nice and clean idea to fix the problem, follows
> the patch, which
> has been tested OK to recognize DVD in machine with device ID 0x1c00
> chipset.
> 
> Seth, could you help to test the patch on your machines with other pci device
> IDs(0x1c01, 0x1d00, 0x1e00, 0x1e01)?

Yeah, looks good to me.  Please feel free to add

  Acked-by: Tejun Heo <tj@kernel.org>

Thank you.

-- 
tejun

RE: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Heasley, Seth]

>Seth, could you help to test the patch on your machines with other pci
>device
>IDs(0x1c01, 0x1d00, 0x1e00, 0x1e01)?

I've now tested the patch on all five DeviceIDs, including 0x1c00.  It resolves the issue.

-Seth

Re: [PATCH] ata: make DVD drive recognisable on systems with Intel Sandybridge CPT chipset

[Jeff Garzik]

On 09/30/2011 08:02 PM, Heasley, Seth wrote:
>> Seth, could you help to test the patch on your machines with other pci
>> device
>> IDs(0x1c01, 0x1d00, 0x1e00, 0x1e01)?
>
> I've now tested the patch on all five DeviceIDs, including 0x1c00.  It resolves the issue.

Great, thanks.

Can someone please resend the patch to me?  Ming Lei's patch from today 
is word-wrapped.

	Jeff