Re: [PATCH] ata_piix: make DVD Drive recognisable on systems with Intel Sandybridge chipsets(v1)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

On 06-10-2011 13:27, Ming Lei wrote:

This quirk patch fixes one kind of bug inside some Intel Sandybridge
chipsets, 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
        https://bugs.launchpad.net/bugs/782389
        ......

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 some Sandybridge CPT chipsets.

Seth also tested the patch on all five affected chipsets
(pci device ID: 0x1c00, 0x1c01, 0x1d00, 0x1e00, 0x1e01), and found
the patch does fix the problem.

Tested-by: Heasley, Seth<seth.heasley@xxxxxxxxx>
Cc: Alan Cox<alan@xxxxxxxxxxxxxxx>
Signed-off-by: Ming Lei<ming.lei@xxxxxxxxxxxxx>
Signed-off-by: Tejun Heo<htejun@xxxxxxxxx>
---
  drivers/ata/ata_piix.c |   37 ++++++++++++++++++++++++++++++++-----
  include/linux/libata.h |    1 +
  2 files changed, 33 insertions(+), 5 deletions(-)

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,

   It's not clear why are you declaring a ganeric flag and then add a local
name for it...

It is just same with other flags, isn't it?

No. Read the the ata_piix.c source attentively, looking for PIIX_FLAG_* definitions.

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 */

   Please, fix the formatting. Or better totally remove this.

It is used to describe if the controller only supports 16bit PIO, and it
is introduced to fix the problem on SNB chips.

I don't yet see why it should be a generic flag: we have ata_bmdma_port_ops and ata_bmdma32_port_ops to be used for 16-bit and 32-bit PIO. You're only using this flag locally to ata_piix.c, hence it should be local to that file.

thanks,
--
Ming Lei

WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux