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

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],
--
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,

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.