| Message ID | 1316656512-4829-1-git-send-email-ming.lei@canonical.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
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.
>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
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>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
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];