Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks

Sergei Shtylyov wrote:

Hello.

Bartlomiej Zolnierkiewicz wrote:

quoted

[PATCH] ide: fix UDMA/MWDMA/SWDMA masks

quoted

* use 0x00 instead of 0x80 to disable ->{ultra,mwdma,swdma}_mask
* add udma_mask field to ide_pci_device_t and use it to initialize
  ->ultra_mask in aec62xx, pdc202xx_new and pdc202xx_old drivers
* fix UDMA masks to match with chipset specific *_ratemask()
  (alim15x3, hpt366, serverworks and siimage drivers need UDMA mask
   filtering method - done in the next patch)

   More issues with aec62xx.c driver, found after looking at the next
patch...

quoted

Index: b/drivers/ide/pci/aec62xx.c
===================================================================

--- a/drivers/ide/pci/aec62xx.c
+++ b/drivers/ide/pci/aec62xx.c

@@ -270,11 +270,13 @@ static unsigned int __devinit init_chips

 static void __devinit init_hwif_aec62xx(ide_hwif_t *hwif)
 {
+     struct pci_dev *dev = hwif->pci_dev;
+
      hwif->autodma = 0;
      hwif->tuneproc = &aec62xx_tune_drive;
      hwif->speedproc = &aec62xx_tune_chipset;

-     if (hwif->pci_dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
+     if (dev->device == PCI_DEVICE_ID_ARTOP_ATP850UF)
              hwif->serialized = hwif->channel;

      if (hwif->mate)

@@ -286,7 +288,16 @@ static void __devinit init_hwif_aec62xx(
              return;
      }

-     hwif->ultra_mask = 0x7f;
+     hwif->ultra_mask = hwif->cds->udma_mask;
+
+     /* atp865 and atp865r */
+     if (hwif->ultra_mask == 0x3f) {
+             unsigned long io = pci_resource_start(dev, 4);
+
+             if (inb(io) & 0x10)
+                     hwif->ultra_mask = 0x7f; /* udma0-6 */
+     }
+

   Looks like another intruduced buglet: you're reading DMA command, but
aec62xx_ratemask() was reading DMA status originally for this bit.

Doh, fixed.  Thanks for catching this.

quoted

      hwif->mwdma_mask = 0x07;
      hwif->swdma_mask = 0x07;

   Hm, caught another nit: this driver doesn't actually support single-word
DMA modes... :-)

added to TODO

quoted

Index: b/drivers/ide/pci/alim15x3.c
===================================================================

--- a/drivers/ide/pci/alim15x3.c
+++ b/drivers/ide/pci/alim15x3.c

@@ -765,8 +765,17 @@ static void __devinit init_hwif_common_a

      hwif->atapi_dma = 1;

-     if (m5229_revision > 0x20)
-             hwif->ultra_mask = 0x7f;
+     if (m5229_revision <= 0x20)
+             hwif->ultra_mask = 0x00; /* no udma */
+     else if (m5229_revision < 0xC2)
+             hwif->ultra_mask = 0x07; /* udma0-2 */
+     else if (m5229_revision == 0xC2 || m5229_revision == 0xC3)
+             hwif->ultra_mask = 0x1f; /* udma0-4 */
+     else if (m5229_revision == 0xC4)
+             hwif->ultra_mask = 0x3f; /* udma0-5 */
+     else
+             hwif->ultra_mask = 0x7f; /* udma0-6 */
+
      hwif->mwdma_mask = 0x07;
      hwif->swdma_mask = 0x07;

   Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver...
And PIO setting via speedproc() method is broken -- it passes to tuneproc()
method mode number not biased by -XFER_PIO_0 beforehand.  Will cook up some
patch, maybe... :-/

Please do, I added this to IDE TODO to not forget about the issue...

Thanks,
Bart