Hi,

On Tuesday 03 July 2007, Alan Cox wrote:
> The HPT343/345 (aka 363) is a bit of a warped device. For many setups you
> need to access the other registers via BAR4 offsets. PIO is now rock

If you happen to have HPT363 you may want to check how BIOS does the DMA
configuration.  I wouldn't be surprised if it turns out that _both_ drivers
do it wrongly currently.

> solid, DMA isn't. Unfortunately the drivers/ide hpt34x driver is
> completely broken so doesn't help further debug.

Translation:

The new improved driver is not really better than the old one because
the old one is broken. :)

Old driver does identical configuration when it comes to PIO modes.

For DMA modes it seem to have bug in setting DMA transfer type bits but DMA
is _never_ enabled unless CONFIG_HPT34X_AUTODMA is set and this config option
is marked EXPERIMENTAL with the help entry stating that enabling DMA is a
dangerous thing to do.

Old driver is a source of PCI quirk which is now propagated to the new driver.

Thanks,
Bart

> Signed-off-by: Alan Cox <[EMAIL PROTECTED]>
> 
> diff -u --new-file --exclude-from /usr/src/exclude --recursive 
> linux.vanilla-2.6.22-rc6-mm1/drivers/ata/pata_hpt3x3.c 
> linux-2.6.22-rc6-mm1/drivers/ata/pata_hpt3x3.c
> --- linux.vanilla-2.6.22-rc6-mm1/drivers/ata/pata_hpt3x3.c    2007-07-02 
> 20:50:11.000000000 +0100
> +++ linux-2.6.22-rc6-mm1/drivers/ata/pata_hpt3x3.c    2007-07-02 
> 21:03:32.000000000 +0100
> @@ -23,7 +23,7 @@
>  #include <linux/libata.h>
>  
>  #define DRV_NAME     "pata_hpt3x3"
> -#define DRV_VERSION  "0.4.3"
> +#define DRV_VERSION  "0.5.3"
>  
>  /**
>   *   hpt3x3_set_piomode              -       PIO setup
> @@ -59,6 +59,9 @@
>   *
>   *   Set up the channel for MWDMA or UDMA modes. Much the same as with
>   *   PIO, load the mode number and then set MWDMA or UDMA flag.
> + *
> + *   0x44 : bit 0-2 master mode, 3-5 slave mode, etc 
> + *   0x48 : bit 4/0 DMA/UDMA bit 5/1 for slave etc
>   */
>  
>  static void hpt3x3_set_dmamode(struct ata_port *ap, struct ata_device *adev)
> @@ -76,14 +79,26 @@
>       r2 &= ~(0x11 << dn);    /* Clear MWDMA and UDMA bits */
>  
>       if (adev->dma_mode >= XFER_UDMA_0)
> -             r2 |= 0x01 << dn;       /* Ultra mode */
> +             r2 |= (0x10 << dn);     /* Ultra mode */
>       else
> -             r2 |= 0x10 << dn;       /* MWDMA */
> +             r2 |= (0x01 << dn);     /* MWDMA */
>  
>       pci_write_config_dword(pdev, 0x44, r1);
>       pci_write_config_dword(pdev, 0x48, r2);
>  }
>  
> +/**
> + *   hpt3x3_atapi_dma        -       ATAPI DMA check
> + *   @qc: Queued command
> + *
> + *   Just say no - we don't do ATAPI DMA
> + */
> +
> +static int hpt3x3_atapi_dma(struct ata_queued_cmd *qc)
> +{
> +     return 1;
> +}
> +
>  static struct scsi_host_template hpt3x3_sht = {
>       .module                 = THIS_MODULE,
>       .name                   = DRV_NAME,
> @@ -105,7 +120,6 @@
>  static struct ata_port_operations hpt3x3_port_ops = {
>       .port_disable   = ata_port_disable,
>       .set_piomode    = hpt3x3_set_piomode,
> -     .set_dmamode    = hpt3x3_set_dmamode,
>       .mode_filter    = ata_pci_default_filter,
>  
>       .tf_load        = ata_tf_load,
> @@ -124,8 +138,9 @@
>       .bmdma_start    = ata_bmdma_start,
>       .bmdma_stop     = ata_bmdma_stop,
>       .bmdma_status   = ata_bmdma_status,
> +     .check_atapi_dma= hpt3x3_atapi_dma,
>  
> -     .qc_prep        = ata_qc_prep,
> +     .qc_prep        = ata_qc_prep,
>       .qc_issue       = ata_qc_issue_prot,
>  
>       .data_xfer      = ata_data_xfer,
> @@ -158,32 +173,79 @@
>               pci_write_config_byte(dev, PCI_LATENCY_TIMER, 0x20);
>  }
>  
> -
>  /**
>   *   hpt3x3_init_one         -       Initialise an HPT343/363
> - *   @dev: PCI device
> + *   @pdev: PCI device
>   *   @id: Entry in match table
>   *
> - *   Perform basic initialisation. The chip has a quirk that it won't
> - *   function unless it is at XX00. The old ATA driver touched this up
> - *   but we leave it for pci quirks to do properly.
> + *   Perform basic initialisation. We set the device up so we access all
> + *   ports via BAR4. This is neccessary to work around errata.
>   */
>  
> -static int hpt3x3_init_one(struct pci_dev *dev, const struct pci_device_id 
> *id)
> +static int hpt3x3_init_one(struct pci_dev *pdev, const struct pci_device_id 
> *id)
>  {
> +     static int printed_version;
>       static const struct ata_port_info info = {
>               .sht = &hpt3x3_sht,
>               .flags = ATA_FLAG_SLAVE_POSS,
>               .pio_mask = 0x1f,
> +#if defined(CONFIG_PATA_HPT3X3_DMA)
> +             /* Further debug needed */              
>               .mwdma_mask = 0x07,
>               .udma_mask = 0x07,
> +#endif               
>               .port_ops = &hpt3x3_port_ops
>       };
> +     /* Register offsets of taskfiles in BAR4 area */
> +     static const u8 offset_cmd[2] = { 0x20, 0x28 };
> +     static const u8 offset_ctl[2] = { 0x36, 0x3E };
>       const struct ata_port_info *ppi[] = { &info, NULL };
> -
> -     hpt3x3_init_chipset(dev);
> -     /* Now kick off ATA set up */
> -     return ata_pci_init_one(dev, ppi);
> +     struct ata_host *host;
> +     int i, rc;
> +     void __iomem *base;
> +
> +     hpt3x3_init_chipset(pdev);
> +
> +     if (!printed_version++)
> +             dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
> +
> +     host = ata_host_alloc_pinfo(&pdev->dev, ppi, 2);
> +     if (!host)
> +             return -ENOMEM;
> +     /* acquire resources and fill host */
> +     rc = pcim_enable_device(pdev);
> +     if (rc)
> +             return rc;
> +
> +     /* Everything is relative to BAR4 if we set up this way */
> +     rc = pcim_iomap_regions(pdev, 1 << 4, DRV_NAME);
> +     if (rc == -EBUSY)
> +             pcim_pin_device(pdev);
> +     if (rc)
> +             return rc;
> +     host->iomap = pcim_iomap_table(pdev);
> +     rc = pci_set_dma_mask(pdev, ATA_DMA_MASK);
> +     if (rc)
> +             return rc;
> +     rc = pci_set_consistent_dma_mask(pdev, ATA_DMA_MASK);
> +     if (rc)
> +             return rc;
> +
> +     base = host->iomap[4];  /* Bus mastering base */
> +
> +     for (i = 0; i < host->n_ports; i++) {
> +             struct ata_ioports *ioaddr = &host->ports[i]->ioaddr;
> +
> +             ioaddr->cmd_addr = base + offset_cmd[i];
> +             ioaddr->altstatus_addr =
> +             ioaddr->ctl_addr = base + offset_ctl[i];
> +             ioaddr->scr_addr = NULL;
> +             ata_std_ports(ioaddr);
> +             ioaddr->bmdma_addr = base + 8 * i;
> +     }
> +     pci_set_master(pdev);
> +     return ata_host_activate(host, pdev->irq, ata_interrupt, IRQF_SHARED,
> +                              &hpt3x3_sht);
>  }
>  
>  #ifdef CONFIG_PM
> diff -u --new-file --exclude-from /usr/src/exclude --recursive 
> linux.vanilla-2.6.22-rc6-mm1/drivers/ata/Kconfig 
> linux-2.6.22-rc6-mm1/drivers/ata/Kconfig
> --- linux.vanilla-2.6.22-rc6-mm1/drivers/ata/Kconfig  2007-07-02 
> 20:50:11.000000000 +0100
> +++ linux-2.6.22-rc6-mm1/drivers/ata/Kconfig  2007-07-02 21:02:10.000000000 
> +0100
> @@ -313,7 +313,7 @@
>         If unsure, say N.
>  
>  config PATA_HPT3X3
> -     tristate "HPT 343/363 PATA support (Experimental)"
> +     tristate "HPT 343/363 PATA support"
>       depends on PCI
>       help
>         This option enables support for the HPT 343/363
> @@ -321,6 +321,14 @@
>  
>         If unsure, say N.
>  
> +config PATA_HPT3X3_DMA
> +     bool "HPT 343/363 DMA support (Experimental)"
> +     depends on PATA_HPT3X3
> +     help
> +       This option enables DMA support for the HPT343/363
> +       controllers. Enable with care as there are still some
> +       problems with DMA on this chipset.
> +
>  config PATA_ISAPNP
>       tristate "ISA Plug and Play PATA support (Experimental)"
>       depends on EXPERIMENTAL && ISAPNP
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to