[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <45B4FFBD.40507@ru.mvista.com>
Date: Mon, 22 Jan 2007 21:17:33 +0300
From: Sergei Shtylyov <sshtylyov@...mvista.com>
To: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
Cc: linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
Hello.
Bartlomiej Zolnierkiewicz wrote:
> [PATCH] ide: fix UDMA/MWDMA/SWDMA masks
> * 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...
> 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.
> hwif->mwdma_mask = 0x07;
> hwif->swdma_mask = 0x07;
Hm, caught another nit: this driver doesn't actually support single-word
DMA modes... :-)
> 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... :-/
MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists