LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
@ 2007-02-03 17:24 Bartlomiej Zolnierkiewicz
  2007-04-22 16:19 ` Sergei Shtylyov
  0 siblings, 1 reply; 17+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-03 17:24 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, Bartlomiej Zolnierkiewicz, linux-kernel

[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, cmd64x, pdc202xx_{new,old} and piix 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)

v2:
* piix: fix cable detection for 82801AA_1 and 82372FB_1
  [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
* cmd64x: use hwif->cds->udma_mask
  [ Suggested by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
* aec62xx: fix newly introduced bug - check DMA status not command register
  [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

v3:
* piix: use hwif->cds->udma_mask
  [ Suggested by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
 drivers/ide/ide.c              |    3 -
 drivers/ide/pci/aec62xx.c      |   19 +++++++++-
 drivers/ide/pci/alim15x3.c     |   13 ++++++-
 drivers/ide/pci/cmd64x.c       |   15 ++++----
 drivers/ide/pci/pdc202xx_new.c |    9 ++++-
 drivers/ide/pci/pdc202xx_old.c |    7 +++
 drivers/ide/pci/piix.c         |   73 +++++++++++++++++------------------------
 drivers/ide/pci/sis5513.c      |    5 ++
 include/linux/ide.h            |    1 
 9 files changed, 86 insertions(+), 59 deletions(-)

Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -222,9 +222,6 @@ static void init_hwif_data(ide_hwif_t *h
 	hwif->bus_state	= BUSSTATE_ON;
 
 	hwif->atapi_dma = 0;		/* disable all atapi dma */ 
-	hwif->ultra_mask = 0x80;	/* disable all ultra */
-	hwif->mwdma_mask = 0x80;	/* disable all mwdma */
-	hwif->swdma_mask = 0x80;	/* disable all swdma */
 
 	init_completion(&hwif->gendev_rel_comp);
 
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,15 @@ 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) {
+		/* check bit 0x10 of DMA status register */
+		if (inb(pci_resource_start(dev, 4) + 2) & 0x10)
+ 			hwif->ultra_mask = 0x7f; /* udma0-6 */
+	}
+
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
@@ -354,6 +364,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "AEC6260",
 		.init_setup	= init_setup_aec62xx,
@@ -363,6 +374,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= NOAUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "AEC6260R",
 		.init_setup	= init_setup_aec62xx,
@@ -373,6 +385,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= NEVER_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "AEC6X80",
 		.init_setup	= init_setup_aec6x80,
@@ -382,6 +395,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "AEC6X80R",
 		.init_setup	= init_setup_aec6x80,
@@ -392,6 +406,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
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;
 
Index: b/drivers/ide/pci/cmd64x.c
===================================================================
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -690,16 +690,13 @@ static void __devinit init_hwif_cmd64x(i
 
 	hwif->atapi_dma = 1;
 
-	hwif->ultra_mask = 0x3f;
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
-	if (dev->device == PCI_DEVICE_ID_CMD_643)
-		hwif->ultra_mask = 0x80;
-	if (dev->device == PCI_DEVICE_ID_CMD_646)
-		hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
-	if (dev->device == PCI_DEVICE_ID_CMD_648)
-		hwif->ultra_mask = 0x1f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
+
+	if (dev->device == PCI_DEVICE_ID_CMD_646 && class_rev < 5)
+		hwif->ultra_mask = 0x00;
 
 	hwif->ide_dma_check = &cmd64x_config_drive_for_dma;
 	if (!(hwif->udma_four))
@@ -733,6 +730,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x00, /* no udma */
 	},{	/* 1 */
 		.name		= "CMD646",
 		.init_chipset	= init_chipset_cmd64x,
@@ -741,6 +739,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x00,0x00,0x00}, {0x51,0x80,0x80}},
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 2 */
 		.name		= "CMD648",
 		.init_chipset	= init_chipset_cmd64x,
@@ -748,6 +747,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "CMD649",
 		.init_chipset	= init_chipset_cmd64x,
@@ -755,6 +755,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/pdc202xx_new.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_new.c
+++ b/drivers/ide/pci/pdc202xx_new.c
@@ -545,7 +545,7 @@ static void __devinit init_hwif_pdc202ne
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x7f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 
 	hwif->err_stops_fifo = 1;
@@ -621,6 +621,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 1 */
 		.name		= "PDC20269",
 		.init_setup	= init_setup_pdcnew,
@@ -629,6 +630,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 2 */
 		.name		= "PDC20270",
 		.init_setup	= init_setup_pdc20270,
@@ -637,6 +639,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 3 */
 		.name		= "PDC20271",
 		.init_setup	= init_setup_pdcnew,
@@ -645,6 +648,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 4 */
 		.name		= "PDC20275",
 		.init_setup	= init_setup_pdcnew,
@@ -653,6 +657,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 5 */
 		.name		= "PDC20276",
 		.init_setup	= init_setup_pdc20276,
@@ -661,6 +666,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 6 */
 		.name		= "PDC20277",
 		.init_setup	= init_setup_pdcnew,
@@ -669,6 +675,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	}
 };
 
Index: b/drivers/ide/pci/pdc202xx_old.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -478,7 +478,7 @@ static void __devinit init_hwif_pdc202xx
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x3f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 	hwif->atapi_dma = 1;
@@ -587,6 +587,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 16,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "PDC20262",
 		.init_setup	= init_setup_pdc202ata4,
@@ -597,6 +598,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "PDC20263",
 		.init_setup	= init_setup_pdc202ata4,
@@ -607,6 +609,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "PDC20265",
 		.init_setup	= init_setup_pdc20265,
@@ -617,6 +620,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "PDC20267",
 		.init_setup	= init_setup_pdc202xx,
@@ -627,6 +631,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/piix.c
===================================================================
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -514,26 +514,14 @@ static void __devinit init_hwif_piix(ide
 		hwif->ide_dma_clear_irq = &piix_dma_clear_irq;
 
 	hwif->atapi_dma = 1;
-	hwif->ultra_mask = 0x3f;
+
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x06;
 	hwif->swdma_mask = 0x04;
 
-	switch(hwif->pci_dev->device) {
-		case PCI_DEVICE_ID_INTEL_82371FB_0:
-		case PCI_DEVICE_ID_INTEL_82371FB_1:
-		case PCI_DEVICE_ID_INTEL_82371SB_1:
-			hwif->ultra_mask = 0x80;
-			break;
-		case PCI_DEVICE_ID_INTEL_82371AB:
-		case PCI_DEVICE_ID_INTEL_82443MX_1:
-		case PCI_DEVICE_ID_INTEL_82451NX:
-		case PCI_DEVICE_ID_INTEL_82801AB_1:
-			hwif->ultra_mask = 0x07;
-			break;
-		default:
-			if (!hwif->udma_four)
-				hwif->udma_four = piix_cable_detect(hwif);
-			break;
+	if (hwif->ultra_mask & 0x78) {
+		if (!hwif->udma_four)
+			hwif->udma_four = piix_cable_detect(hwif);
 	}
 
 	if (no_piix_dma)
@@ -547,7 +535,7 @@ static void __devinit init_hwif_piix(ide
 	hwif->drives[0].autodma = hwif->autodma;
 }
 
-#define DECLARE_PIIX_DEV(name_str) \
+#define DECLARE_PIIX_DEV(name_str, udma) \
 	{						\
 		.name		= name_str,		\
 		.init_chipset	= init_chipset_piix,	\
@@ -556,11 +544,12 @@ static void __devinit init_hwif_piix(ide
 		.autodma	= AUTODMA,		\
 		.enablebits	= {{0x41,0x80,0x80}, {0x43,0x80,0x80}}, \
 		.bootable	= ON_BOARD,		\
+		.udma_mask	= udma,			\
 	}
 
 static ide_pci_device_t piix_pci_info[] __devinitdata = {
-	/*  0 */ DECLARE_PIIX_DEV("PIIXa"),
-	/*  1 */ DECLARE_PIIX_DEV("PIIXb"),
+	/*  0 */ DECLARE_PIIX_DEV("PIIXa", 0x00),	/* no udma */
+	/*  1 */ DECLARE_PIIX_DEV("PIIXb", 0x00),	/* no udma */
 
 	/*  2 */
 	{	/*
@@ -577,28 +566,28 @@ static ide_pci_device_t piix_pci_info[] 
 		.flags		= IDEPCI_FLAG_ISA_PORTS
 	},
 
-	/*  3 */ DECLARE_PIIX_DEV("PIIX3"),
-	/*  4 */ DECLARE_PIIX_DEV("PIIX4"),
-	/*  5 */ DECLARE_PIIX_DEV("ICH0"),
-	/*  6 */ DECLARE_PIIX_DEV("PIIX4"),
-	/*  7 */ DECLARE_PIIX_DEV("ICH"),
-	/*  8 */ DECLARE_PIIX_DEV("PIIX4"),
-	/*  9 */ DECLARE_PIIX_DEV("PIIX4"),
-	/* 10 */ DECLARE_PIIX_DEV("ICH2"),
-	/* 11 */ DECLARE_PIIX_DEV("ICH2M"),
-	/* 12 */ DECLARE_PIIX_DEV("ICH3M"),
-	/* 13 */ DECLARE_PIIX_DEV("ICH3"),
-	/* 14 */ DECLARE_PIIX_DEV("ICH4"),
-	/* 15 */ DECLARE_PIIX_DEV("ICH5"),
-	/* 16 */ DECLARE_PIIX_DEV("C-ICH"),
-	/* 17 */ DECLARE_PIIX_DEV("ICH4"),
-	/* 18 */ DECLARE_PIIX_DEV("ICH5-SATA"),
-	/* 19 */ DECLARE_PIIX_DEV("ICH5"),
-	/* 20 */ DECLARE_PIIX_DEV("ICH6"),
-	/* 21 */ DECLARE_PIIX_DEV("ICH7"),
-	/* 22 */ DECLARE_PIIX_DEV("ICH4"),
-	/* 23 */ DECLARE_PIIX_DEV("ESB2"),
-	/* 24 */ DECLARE_PIIX_DEV("ICH8M"),
+	/*  3 */ DECLARE_PIIX_DEV("PIIX3", 0x00),	/* no udma */
+	/*  4 */ DECLARE_PIIX_DEV("PIIX4", 0x07),	/* udma0-2 */
+	/*  5 */ DECLARE_PIIX_DEV("ICH0",  0x07),	/* udma0-2 */
+	/*  6 */ DECLARE_PIIX_DEV("PIIX4", 0x07),	/* udma0-2 */
+	/*  7 */ DECLARE_PIIX_DEV("ICH",   0x1f),	/* udma0-4 */
+	/*  8 */ DECLARE_PIIX_DEV("PIIX4", 0x1f),	/* udma0-4 */
+	/*  9 */ DECLARE_PIIX_DEV("PIIX4", 0x07),	/* udma0-2 */
+	/* 10 */ DECLARE_PIIX_DEV("ICH2",  0x3f),	/* udma0-5 */
+	/* 11 */ DECLARE_PIIX_DEV("ICH2M", 0x3f),	/* udma0-5 */
+	/* 12 */ DECLARE_PIIX_DEV("ICH3M", 0x3f),	/* udma0-5 */
+	/* 13 */ DECLARE_PIIX_DEV("ICH3",  0x3f),	/* udma0-5 */
+	/* 14 */ DECLARE_PIIX_DEV("ICH4",  0x3f),	/* udma0-5 */
+	/* 15 */ DECLARE_PIIX_DEV("ICH5",  0x3f),	/* udma0-5 */
+	/* 16 */ DECLARE_PIIX_DEV("C-ICH", 0x3f),	/* udma0-5 */
+	/* 17 */ DECLARE_PIIX_DEV("ICH4",  0x3f),	/* udma0-5 */
+	/* 18 */ DECLARE_PIIX_DEV("ICH5-SATA", 0x3f),	/* udma0-5 */
+	/* 19 */ DECLARE_PIIX_DEV("ICH5",  0x3f),	/* udma0-5 */
+	/* 20 */ DECLARE_PIIX_DEV("ICH6",  0x3f),	/* udma0-5 */
+	/* 21 */ DECLARE_PIIX_DEV("ICH7",  0x3f),	/* udma0-5 */
+	/* 22 */ DECLARE_PIIX_DEV("ICH4",  0x3f),	/* udma0-5 */
+	/* 23 */ DECLARE_PIIX_DEV("ESB2",  0x3f),	/* udma0-5 */
+	/* 24 */ DECLARE_PIIX_DEV("ICH8M", 0x3f),	/* udma0-5 */
 };
 
 /**
Index: b/drivers/ide/pci/sis5513.c
===================================================================
--- a/drivers/ide/pci/sis5513.c
+++ b/drivers/ide/pci/sis5513.c
@@ -858,6 +858,8 @@ static unsigned int __devinit ata66_sis5
 
 static void __devinit init_hwif_sis5513 (ide_hwif_t *hwif)
 {
+	u8 udma_rates[] = { 0x00, 0x00, 0x07, 0x1f, 0x3f, 0x3f, 0x7f, 0x7f };
+
 	hwif->autodma = 0;
 
 	if (!hwif->irq)
@@ -873,7 +875,8 @@ static void __devinit init_hwif_sis5513 
 	}
 
 	hwif->atapi_dma = 1;
-	hwif->ultra_mask = 0x7f;
+
+	hwif->ultra_mask = udma_rates[chipset_family];
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1255,6 +1255,7 @@ typedef struct ide_pci_device_s {
 	unsigned int		extra;
 	struct ide_pci_device_s	*next;
 	u8			flags;
+	u8			udma_mask;
 } ide_pci_device_t;
 
 extern int ide_setup_pci_device(struct pci_dev *, ide_pci_device_t *);

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-02-03 17:24 [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
@ 2007-04-22 16:19 ` Sergei Shtylyov
  0 siblings, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2007-04-22 16:19 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

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, cmd64x, pdc202xx_{new,old} and piix 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)
> 
> v2:
> * piix: fix cable detection for 82801AA_1 and 82372FB_1
>   [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
> * cmd64x: use hwif->cds->udma_mask
>   [ Suggested by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
> * aec62xx: fix newly introduced bug - check DMA status not command register
>   [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

> v3:
> * piix: use hwif->cds->udma_mask
>   [ Suggested by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

> 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,15 @@ 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) {
> +		/* check bit 0x10 of DMA status register */
> +		if (inb(pci_resource_start(dev, 4) + 2) & 0x10)
> + 			hwif->ultra_mask = 0x7f; /* udma0-6 */
> +	}
> +
 
   IMO, this asks to be put into init_setup_aec6x80() instead but since I'm already cleaning up that function, I'll move it there myself.

MBR, Sergei

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-02-03  0:39       ` Bartlomiej Zolnierkiewicz
@ 2007-02-03 16:30         ` Sergei Shtylyov
  0 siblings, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2007-02-03 16:30 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>>Index: b/drivers/ide/pci/cmd64x.c
>>>===================================================================
>>>--- a/drivers/ide/pci/cmd64x.c
>>>+++ b/drivers/ide/pci/cmd64x.c
>>>@@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i
>>>      hwif->swdma_mask = 0x07;
>>>
>>>      if (dev->device == PCI_DEVICE_ID_CMD_643)
>>>-             hwif->ultra_mask = 0x80;
>>>+             hwif->ultra_mask = 0x00;
>>>      if (dev->device == PCI_DEVICE_ID_CMD_646)
>>>-             hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
>>>+             hwif->ultra_mask =
>>>+                     (class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00;
>>>      if (dev->device == PCI_DEVICE_ID_CMD_648)
>>>              hwif->ultra_mask = 0x1f;

>>   Hm, well, this doesn't look consistent with the changes in other drivers.
>>This driver asks for explicit hwif->cds->ultra_mask initializers, IMO...
>>   You'd only have to check for PCI-646 revisions < 5 then...

> reworked

    Thanks. :-)

>>>Index: b/drivers/ide/pci/piix.c
>>>===================================================================
>>>--- a/drivers/ide/pci/piix.c
>>>+++ b/drivers/ide/pci/piix.c
>>>              default:
>>>                      if (!hwif->udma_four)
>>>                              hwif->udma_four = piix_cable_detect(hwif);

>>   This one also certainly asks for explicit hwif->cds->ultra_mask
>>initializers... Thus almost all of this switch statement could go away...
  	
> Alas doing it now would make the nice DECLARE_PIIX_DEV() macro go away

    Why? Could add another argument to that macro...

> (=> a lot of duplicated code)... could be done in the future...

    Yes, of course.

> Thanks,
> Bart

MBR, Sergei

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-02-03 13:53 [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void Bartlomiej Zolnierkiewicz
@ 2007-02-03 13:53 ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 17+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-03 13:53 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, Bartlomiej Zolnierkiewicz, linux-kernel

[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, cmd64x, 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)

v2:
* piix: fix cable detection for 82801AA_1 and 82372FB_1
  [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
* cmd64x: use hwif->cds->udma_mask
  [ Suggested by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]
* aec62xx: fix newly introduced bug - check DMA status not command register
  [ Noticed by Sergei Shtylyov <sshtylyov@ru.mvista.com>. ]

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
 drivers/ide/ide.c              |    3 ---
 drivers/ide/pci/aec62xx.c      |   19 +++++++++++++++++--
 drivers/ide/pci/alim15x3.c     |   13 +++++++++++--
 drivers/ide/pci/cmd64x.c       |   15 ++++++++-------
 drivers/ide/pci/pdc202xx_new.c |    9 ++++++++-
 drivers/ide/pci/pdc202xx_old.c |    7 ++++++-
 drivers/ide/pci/piix.c         |    6 +++++-
 drivers/ide/pci/sis5513.c      |    5 ++++-
 include/linux/ide.h            |    1 +
 9 files changed, 60 insertions(+), 18 deletions(-)

Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -222,9 +222,6 @@ static void init_hwif_data(ide_hwif_t *h
 	hwif->bus_state	= BUSSTATE_ON;
 
 	hwif->atapi_dma = 0;		/* disable all atapi dma */ 
-	hwif->ultra_mask = 0x80;	/* disable all ultra */
-	hwif->mwdma_mask = 0x80;	/* disable all mwdma */
-	hwif->swdma_mask = 0x80;	/* disable all swdma */
 
 	init_completion(&hwif->gendev_rel_comp);
 
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,15 @@ 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) {
+		/* check bit 0x10 of DMA status register */
+		if (inb(pci_resource_start(dev, 4) + 2) & 0x10)
+ 			hwif->ultra_mask = 0x7f; /* udma0-6 */
+	}
+
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
@@ -354,6 +364,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "AEC6260",
 		.init_setup	= init_setup_aec62xx,
@@ -363,6 +374,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= NOAUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "AEC6260R",
 		.init_setup	= init_setup_aec62xx,
@@ -373,6 +385,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= NEVER_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "AEC6X80",
 		.init_setup	= init_setup_aec6x80,
@@ -382,6 +395,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "AEC6X80R",
 		.init_setup	= init_setup_aec6x80,
@@ -392,6 +406,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
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;
 
Index: b/drivers/ide/pci/cmd64x.c
===================================================================
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -690,16 +690,13 @@ static void __devinit init_hwif_cmd64x(i
 
 	hwif->atapi_dma = 1;
 
-	hwif->ultra_mask = 0x3f;
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
-	if (dev->device == PCI_DEVICE_ID_CMD_643)
-		hwif->ultra_mask = 0x80;
-	if (dev->device == PCI_DEVICE_ID_CMD_646)
-		hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
-	if (dev->device == PCI_DEVICE_ID_CMD_648)
-		hwif->ultra_mask = 0x1f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
+
+	if (dev->device == PCI_DEVICE_ID_CMD_646 && class_rev < 5)
+		hwif->ultra_mask = 0x00;
 
 	hwif->ide_dma_check = &cmd64x_config_drive_for_dma;
 	if (!(hwif->udma_four))
@@ -733,6 +730,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x00, /* no udma */
 	},{	/* 1 */
 		.name		= "CMD646",
 		.init_chipset	= init_chipset_cmd64x,
@@ -741,6 +739,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x00,0x00,0x00}, {0x51,0x80,0x80}},
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 2 */
 		.name		= "CMD648",
 		.init_chipset	= init_chipset_cmd64x,
@@ -748,6 +747,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "CMD649",
 		.init_chipset	= init_chipset_cmd64x,
@@ -755,6 +755,7 @@ static ide_pci_device_t cmd64x_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= ON_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/pdc202xx_new.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_new.c
+++ b/drivers/ide/pci/pdc202xx_new.c
@@ -545,7 +545,7 @@ static void __devinit init_hwif_pdc202ne
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x7f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 
 	hwif->err_stops_fifo = 1;
@@ -621,6 +621,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 1 */
 		.name		= "PDC20269",
 		.init_setup	= init_setup_pdcnew,
@@ -629,6 +630,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 2 */
 		.name		= "PDC20270",
 		.init_setup	= init_setup_pdc20270,
@@ -637,6 +639,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 3 */
 		.name		= "PDC20271",
 		.init_setup	= init_setup_pdcnew,
@@ -645,6 +648,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 4 */
 		.name		= "PDC20275",
 		.init_setup	= init_setup_pdcnew,
@@ -653,6 +657,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 5 */
 		.name		= "PDC20276",
 		.init_setup	= init_setup_pdc20276,
@@ -661,6 +666,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 6 */
 		.name		= "PDC20277",
 		.init_setup	= init_setup_pdcnew,
@@ -669,6 +675,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	}
 };
 
Index: b/drivers/ide/pci/pdc202xx_old.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -478,7 +478,7 @@ static void __devinit init_hwif_pdc202xx
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x3f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 	hwif->atapi_dma = 1;
@@ -587,6 +587,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 16,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "PDC20262",
 		.init_setup	= init_setup_pdc202ata4,
@@ -597,6 +598,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "PDC20263",
 		.init_setup	= init_setup_pdc202ata4,
@@ -607,6 +609,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "PDC20265",
 		.init_setup	= init_setup_pdc20265,
@@ -617,6 +620,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "PDC20267",
 		.init_setup	= init_setup_pdc202xx,
@@ -627,6 +631,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/piix.c
===================================================================
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -522,7 +522,7 @@ static void __devinit init_hwif_piix(ide
 		case PCI_DEVICE_ID_INTEL_82371FB_0:
 		case PCI_DEVICE_ID_INTEL_82371FB_1:
 		case PCI_DEVICE_ID_INTEL_82371SB_1:
-			hwif->ultra_mask = 0x80;
+			hwif->ultra_mask = 0x00;
 			break;
 		case PCI_DEVICE_ID_INTEL_82371AB:
 		case PCI_DEVICE_ID_INTEL_82443MX_1:
@@ -530,6 +530,10 @@ static void __devinit init_hwif_piix(ide
 		case PCI_DEVICE_ID_INTEL_82801AB_1:
 			hwif->ultra_mask = 0x07;
 			break;
+		case PCI_DEVICE_ID_INTEL_82801AA_1:
+		case PCI_DEVICE_ID_INTEL_82372FB_1:
+			hwif->ultra_mask = 0x1f;
+			/* fall-through */
 		default:
 			if (!hwif->udma_four)
 				hwif->udma_four = piix_cable_detect(hwif);
Index: b/drivers/ide/pci/sis5513.c
===================================================================
--- a/drivers/ide/pci/sis5513.c
+++ b/drivers/ide/pci/sis5513.c
@@ -858,6 +858,8 @@ static unsigned int __devinit ata66_sis5
 
 static void __devinit init_hwif_sis5513 (ide_hwif_t *hwif)
 {
+	u8 udma_rates[] = { 0x00, 0x00, 0x07, 0x1f, 0x3f, 0x3f, 0x7f, 0x7f };
+
 	hwif->autodma = 0;
 
 	if (!hwif->irq)
@@ -873,7 +875,8 @@ static void __devinit init_hwif_sis5513 
 	}
 
 	hwif->atapi_dma = 1;
-	hwif->ultra_mask = 0x7f;
+
+	hwif->ultra_mask = udma_rates[chipset_family];
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1255,6 +1255,7 @@ typedef struct ide_pci_device_s {
 	unsigned int		extra;
 	struct ide_pci_device_s	*next;
 	u8			flags;
+	u8			udma_mask;
 } ide_pci_device_t;
 
 extern int ide_setup_pci_device(struct pci_dev *, ide_pci_device_t *);

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
       [not found]     ` <58cb370e0702021606m4eb6f682xfa4bf769d398cf9@mail.gmail.com>
@ 2007-02-03  0:50       ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 17+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-03  0:50 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, linux-kernel


Sergei Shtylyov wrote:
> 
> 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.

Doh, fixed.  Thanks for catching this.

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

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
       [not found]     ` <58cb370e0702021606v4efa6143lf060e6aab9782c35@mail.gmail.com>
@ 2007-02-03  0:39       ` Bartlomiej Zolnierkiewicz
  2007-02-03 16:30         ` Sergei Shtylyov
  0 siblings, 1 reply; 17+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-02-03  0:39 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide, linux-kernel


Hi,

Sergei Shtylyov wrote:
> 
> 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 nit picking (-:
> 
>> Index: b/drivers/ide/pci/cmd64x.c
>> ===================================================================
>> --- a/drivers/ide/pci/cmd64x.c
>> +++ b/drivers/ide/pci/cmd64x.c
>> @@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i
>>       hwif->swdma_mask = 0x07;
>>
>>       if (dev->device == PCI_DEVICE_ID_CMD_643)
>> -             hwif->ultra_mask = 0x80;
>> +             hwif->ultra_mask = 0x00;
>>       if (dev->device == PCI_DEVICE_ID_CMD_646)
>> -             hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
>> +             hwif->ultra_mask =
>> +                     (class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00;
>>       if (dev->device == PCI_DEVICE_ID_CMD_648)
>>               hwif->ultra_mask = 0x1f;
> 
>    Hm, well, this doesn't look consistent with the changes in other drivers.
> This driver asks for explicit hwif->cds->ultra_mask initializers, IMO...
>    You'd only have to check for PCI-646 revisions < 5 then...

reworked

>> Index: b/drivers/ide/pci/piix.c
>> ===================================================================
>> --- a/drivers/ide/pci/piix.c
>> +++ b/drivers/ide/pci/piix.c
>> @@ -493,7 +493,7 @@ static void __devinit init_hwif_piix(ide
>>               case PCI_DEVICE_ID_INTEL_82371FB_0:
>>               case PCI_DEVICE_ID_INTEL_82371FB_1:
>>               case PCI_DEVICE_ID_INTEL_82371SB_1:
>> -                     hwif->ultra_mask = 0x80;
>> +                     hwif->ultra_mask = 0x00;
>>                       break;
>>               case PCI_DEVICE_ID_INTEL_82371AB:
>>               case PCI_DEVICE_ID_INTEL_82443MX_1:
>> @@ -501,6 +501,10 @@ static void __devinit init_hwif_piix(ide
>>               case PCI_DEVICE_ID_INTEL_82801AB_1:
>>                       hwif->ultra_mask = 0x07;
>>                       break;
>> +             case PCI_DEVICE_ID_INTEL_82801AA_1:
>> +             case PCI_DEVICE_ID_INTEL_82372FB_1:
>> +                     hwif->ultra_mask = 0x1f;
>> +                     break;
> 
>    Alas, I'm afraid this part is wrong!
>    At least, the cable detection should work for 82801AA the same way as for
> the 82801Bx and newer chips, if Intel's datasheet is to be trusted... I think
> we should fall thru here.

yes (extra "break:" shouldn't be there), fixed

>>               default:
>>                       if (!hwif->udma_four)
>>                               hwif->udma_four = piix_cable_detect(hwif);
> 
>    This one also certainly asks for explicit hwif->cds->ultra_mask
> initializers... Thus almost all of this switch statement could go away...
	
Alas doing it now would make the nice DECLARE_PIIX_DEV() macro go away
(=> a lot of duplicated code)... could be done in the future...

Thanks,
Bart


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-31 20:38       ` Sergei Shtylyov
@ 2007-01-31 23:24         ` Alan
  0 siblings, 0 replies; 17+ messages in thread
From: Alan @ 2007-01-31 23:24 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

> should be slightly overclocking PIO modes 0/1 (ULi docs don't shed much light 
> on how it should be calculated)... Well, this seems fixed in libata drivers.

The libata code for tuning is based upon the BIOS programmers guide. That
seemed to be the best coverage of a minimal selection. It's also got a
big "confidential" stamp on it so I can't pass it on.

Alan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 18:46     ` Alan
  2007-01-22 20:28       ` Sergei Shtylyov
@ 2007-01-31 20:38       ` Sergei Shtylyov
  2007-01-31 23:24         ` Alan
  1 sibling, 1 reply; 17+ messages in thread
From: Sergei Shtylyov @ 2007-01-31 20:38 UTC (permalink / raw)
  To: Alan; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

Hello.

Alan wrote:

>>    Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... 

> Thats long been broken. Should be correct in the libata driver

    I've looked thru the specs and it seemed to me that ULi hardware is much 
broken PIO wise: their max active time is 8 cycles even on taskfile access 
which gives 240 ns while standard requeires 290 ns for modes 0 thru 2...

    I've also noted that the tuneproc() method in both cmd64x.c and alim15x3.c 
seems to misdo recovery calculation, taking address setup into account -- that 
should be slightly overclocking PIO modes 0/1 (ULi docs don't shed much light 
on how it should be calculated)... Well, this seems fixed in libata drivers.

> Alan

MBR, Sergei

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-23 14:51           ` Sergei Shtylyov
@ 2007-01-25 15:40             ` Sergei Shtylyov
  0 siblings, 0 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2007-01-25 15:40 UTC (permalink / raw)
  To: Alan; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

Hello.

Sergei Shtylyov wrote:

>> Not a suprise to be honest. I fixed some of the ALi stuff when I did it
>> and I think that was pushed back into drivers/ide. The CMD64x hasn't had
>> much love really.

>    Another buglet found by random glancing at this driver:

> /**
>  *      cmd648_dma_stop -       DMA stop callback
>  *      @qc: Command in progress
>  *
>  *      DMA has completed.
>  */

> static void cmd648_bmdma_stop(struct ata_queued_cmd *qc)
> {
>         struct ata_port *ap = qc->ap;
>         struct pci_dev *pdev = to_pci_dev(ap->host->dev);
>         u8 dma_intr;
>         int dma_reg = ap->port_no ? ARTTIM23_INTR_CH1 : CFR_INTR_CH0;
>         int dma_mask = ap->port_no ? ARTTIM2 : CFR;
> 
>         ata_bmdma_stop(qc);
> 
>         pci_read_config_byte(pdev, dma_reg, &dma_intr);
>         pci_write_config_byte(pdev, dma_reg, dma_intr | dma_mask);
> }

>    dma_reg and dma_mask initializers must have been swapped since 
> ARTTIM2 and CFR are regster names.  So, the code reads/writes 
> semi-random regs...

    BTW, on PCI0646U2 and later chips, the interrupt status (it's not really 
DMA interrupt status but a latched INTRQ signal not "coupled" with DMA logic, 
according to the datasheets) can be read from MRDMODE reg. which is accessible 
in I/O space at BMIDE base + 1 which is certainly faster.  That's what 
drivers/ide/cmd64x.c is doing in its test_dma_irq() method (however, it's 
doign this on PCI0643 and early revs of PCI0646 which don't have these bits).
The driver's dma_end() method is acting really strange: it checks if the cjip 
is PCI-648/9 and reads the PCI config space to clear those interrupt bits 
while these chips have them in I/O mapped MRDMODE; OTOH, it ignores these bits 
on earlier chips which have them in oonfig. space only (CFR/ARTTIM23 regs)... 
go figure.  I'm going to clean this up but don't heve the h/w handy... :-/

>> Wouldn't mind the older 64x (not 640) data sheets if they are sharable.

>    Sent what I had on this machine.  Will looks for newer revision of 
> PCJ0646U2 spec elsewhere...

    Sent rev. 1.3... Hopefully gkernel.sourceforge.net/specs/ will be updated.

MBR, Sergei

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 21:31         ` Alan
  2007-01-22 22:39           ` Jeff Garzik
@ 2007-01-23 14:51           ` Sergei Shtylyov
  2007-01-25 15:40             ` Sergei Shtylyov
  1 sibling, 1 reply; 17+ messages in thread
From: Sergei Shtylyov @ 2007-01-23 14:51 UTC (permalink / raw)
  To: Alan; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

Hello.

Alan wrote:
>>>>   Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... 

>>>Thats long been broken. Should be correct in the libata driver

>>    Here's a surprise for you. pata_cmd64x copied the SW/MW DMA setup code 
>>from the IDE driver.  No way it could be working.  You may check against the 
>>PC64x datasheets (if you have them -- if you don't I think I may share) and 
>>see for yourself -- it's abolutely idiotic.

    I.e. MWDMA2 should be working due to the way the driver is written (it 
sets up PIO4 timings when auto-tuning DMA) but not the other modes since 
speedproc() method is brain-damaged in this part.

> Not a suprise to be honest. I fixed some of the ALi stuff when I did it
> and I think that was pushed back into drivers/ide. The CMD64x hasn't had
> much love really.

    Another buglet found by random glancing at this driver:

/**
  *      cmd648_dma_stop -       DMA stop callback
  *      @qc: Command in progress
  *
  *      DMA has completed.
  */

static void cmd648_bmdma_stop(struct ata_queued_cmd *qc)
{
         struct ata_port *ap = qc->ap;
         struct pci_dev *pdev = to_pci_dev(ap->host->dev);
         u8 dma_intr;
         int dma_reg = ap->port_no ? ARTTIM23_INTR_CH1 : CFR_INTR_CH0;
         int dma_mask = ap->port_no ? ARTTIM2 : CFR;

         ata_bmdma_stop(qc);

         pci_read_config_byte(pdev, dma_reg, &dma_intr);
         pci_write_config_byte(pdev, dma_reg, dma_intr | dma_mask);
}

    dma_reg and dma_mask initializers must have been swapped since ARTTIM2 and 
CFR are regster names.  So, the code reads/writes semi-random regs...

> Wouldn't mind the older 64x (not 640) data sheets if they are sharable.

    Sent what I had on this machine.  Will looks for newer revision of 
PCJ0646U2 spec elsewhere...

MBR, Sergei

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 21:31         ` Alan
@ 2007-01-22 22:39           ` Jeff Garzik
  2007-01-23 14:51           ` Sergei Shtylyov
  1 sibling, 0 replies; 17+ messages in thread
From: Jeff Garzik @ 2007-01-22 22:39 UTC (permalink / raw)
  To: Alan; +Cc: Sergei Shtylyov, Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

Alan wrote:
> Wouldn't mind the older 64x (not 640) data sheets if they are sharable.


If they are sharable, I would love to archive them at

	http://gkernel.sourceforge.net/specs/

Regards,

	Jeff



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 20:28       ` Sergei Shtylyov
@ 2007-01-22 21:31         ` Alan
  2007-01-22 22:39           ` Jeff Garzik
  2007-01-23 14:51           ` Sergei Shtylyov
  0 siblings, 2 replies; 17+ messages in thread
From: Alan @ 2007-01-22 21:31 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

> >>    Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... 
> 
> > Thats long been broken. Should be correct in the libata driver
> 
>     Here's a surprise for you. pata_cmd64x copied the SW/MW DMA setup code 
> from the IDE driver.  No way it could be working.  You may check against the 
> PC64x datasheets (if you have them -- if you don't I think I may share) and 
> see for yourself -- it's abolutely idiotic.

Not a suprise to be honest. I fixed some of the ALi stuff when I did it
and I think that was pushed back into drivers/ide. The CMD64x hasn't had
much love really.

Wouldn't mind the older 64x (not 640) data sheets if they are sharable.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 18:46     ` Alan
@ 2007-01-22 20:28       ` Sergei Shtylyov
  2007-01-22 21:31         ` Alan
  2007-01-31 20:38       ` Sergei Shtylyov
  1 sibling, 1 reply; 17+ messages in thread
From: Sergei Shtylyov @ 2007-01-22 20:28 UTC (permalink / raw)
  To: Alan; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

Hello.

Alan wrote:

>>    Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... 

> Thats long been broken. Should be correct in the libata driver

    Here's a surprise for you. pata_cmd64x copied the SW/MW DMA setup code 
from the IDE driver.  No way it could be working.  You may check against the 
PC64x datasheets (if you have them -- if you don't I think I may share) and 
see for yourself -- it's abolutely idiotic.

> Alan

WBR, Sergei

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-22 18:17   ` Sergei Shtylyov
@ 2007-01-22 18:46     ` Alan
  2007-01-22 20:28       ` Sergei Shtylyov
  2007-01-31 20:38       ` Sergei Shtylyov
       [not found]     ` <58cb370e0702021606m4eb6f682xfa4bf769d398cf9@mail.gmail.com>
  1 sibling, 2 replies; 17+ messages in thread
From: Alan @ 2007-01-22 18:46 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Bartlomiej Zolnierkiewicz, linux-ide, linux-kernel

On Mon, 22 Jan 2007 21:17:33 +0300
>     Ugh, I'm not seeing any *actual* support for MW/SW DMA in this driver... 

Thats long been broken. Should be correct in the libata driver

Alan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-19  0:32 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
  2007-01-22 16:19   ` Sergei Shtylyov
@ 2007-01-22 18:17   ` Sergei Shtylyov
  2007-01-22 18:46     ` Alan
       [not found]     ` <58cb370e0702021606m4eb6f682xfa4bf769d398cf9@mail.gmail.com>
  1 sibling, 2 replies; 17+ messages in thread
From: Sergei Shtylyov @ 2007-01-22 18:17 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-19  0:32 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
@ 2007-01-22 16:19   ` Sergei Shtylyov
       [not found]     ` <58cb370e0702021606v4efa6143lf060e6aab9782c35@mail.gmail.com>
  2007-01-22 18:17   ` Sergei Shtylyov
  1 sibling, 1 reply; 17+ messages in thread
From: Sergei Shtylyov @ 2007-01-22 16:19 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide, linux-kernel

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 nit picking (-:

> Index: b/drivers/ide/pci/cmd64x.c
> ===================================================================
> --- a/drivers/ide/pci/cmd64x.c
> +++ b/drivers/ide/pci/cmd64x.c
> @@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i
>  	hwif->swdma_mask = 0x07;
>  
>  	if (dev->device == PCI_DEVICE_ID_CMD_643)
> -		hwif->ultra_mask = 0x80;
> +		hwif->ultra_mask = 0x00;
>  	if (dev->device == PCI_DEVICE_ID_CMD_646)
> -		hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
> +		hwif->ultra_mask =
> +			(class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00;
>  	if (dev->device == PCI_DEVICE_ID_CMD_648)
>  		hwif->ultra_mask = 0x1f;

    Hm, well, this doesn't look consistent with the changes in other drivers.
This driver asks for explicit hwif->cds->ultra_mask initializers, IMO...
    You'd only have to check for PCI-646 revisions < 5 then...

> Index: b/drivers/ide/pci/piix.c
> ===================================================================
> --- a/drivers/ide/pci/piix.c
> +++ b/drivers/ide/pci/piix.c
> @@ -493,7 +493,7 @@ static void __devinit init_hwif_piix(ide
>  		case PCI_DEVICE_ID_INTEL_82371FB_0:
>  		case PCI_DEVICE_ID_INTEL_82371FB_1:
>  		case PCI_DEVICE_ID_INTEL_82371SB_1:
> -			hwif->ultra_mask = 0x80;
> +			hwif->ultra_mask = 0x00;
>  			break;
>  		case PCI_DEVICE_ID_INTEL_82371AB:
>  		case PCI_DEVICE_ID_INTEL_82443MX_1:
> @@ -501,6 +501,10 @@ static void __devinit init_hwif_piix(ide
>  		case PCI_DEVICE_ID_INTEL_82801AB_1:
>  			hwif->ultra_mask = 0x07;
>  			break;
> +		case PCI_DEVICE_ID_INTEL_82801AA_1:
> +		case PCI_DEVICE_ID_INTEL_82372FB_1:
> +			hwif->ultra_mask = 0x1f;
> +			break;

    Alas, I'm afraid this part is wrong!
    At least, the cable detection should work for 82801AA the same way as for 
the 82801Bx and newer chips, if Intel's datasheet is to be trusted... I think 
we should fall thru here.

>  		default:
>  			if (!hwif->udma_four)
>  				hwif->udma_four = piix_cable_detect(hwif);

    This one also certainly asks for explicit hwif->cds->ultra_mask 
initializers... Thus almost all of this switch statement could go away...

MBR, Sergei

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks
  2007-01-19  0:30 [PATCH 0/15] IDE quilt tree updated Bartlomiej Zolnierkiewicz
@ 2007-01-19  0:32 ` Bartlomiej Zolnierkiewicz
  2007-01-22 16:19   ` Sergei Shtylyov
  2007-01-22 18:17   ` Sergei Shtylyov
  0 siblings, 2 replies; 17+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-01-19  0:32 UTC (permalink / raw)
  To: linux-ide; +Cc: Bartlomiej Zolnierkiewicz, linux-kernel

[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)

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

---
 drivers/ide/ide.c              |    3 ---
 drivers/ide/pci/aec62xx.c      |   20 ++++++++++++++++++--
 drivers/ide/pci/alim15x3.c     |   13 +++++++++++--
 drivers/ide/pci/cmd64x.c       |    5 +++--
 drivers/ide/pci/pdc202xx_new.c |    9 ++++++++-
 drivers/ide/pci/pdc202xx_old.c |    7 ++++++-
 drivers/ide/pci/piix.c         |    6 +++++-
 drivers/ide/pci/sis5513.c      |    5 ++++-
 include/linux/ide.h            |    1 +
 9 files changed, 56 insertions(+), 13 deletions(-)

Index: b/drivers/ide/ide.c
===================================================================
--- a/drivers/ide/ide.c
+++ b/drivers/ide/ide.c
@@ -222,9 +222,6 @@ static void init_hwif_data(ide_hwif_t *h
 	hwif->bus_state	= BUSSTATE_ON;
 
 	hwif->atapi_dma = 0;		/* disable all atapi dma */ 
-	hwif->ultra_mask = 0x80;	/* disable all ultra */
-	hwif->mwdma_mask = 0x80;	/* disable all mwdma */
-	hwif->swdma_mask = 0x80;	/* disable all swdma */
 
 	init_completion(&hwif->gendev_rel_comp);
 
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 */
+	}
+
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
@@ -354,6 +365,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "AEC6260",
 		.init_setup	= init_setup_aec62xx,
@@ -363,6 +375,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= NOAUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "AEC6260R",
 		.init_setup	= init_setup_aec62xx,
@@ -373,6 +386,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= NEVER_BOARD,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "AEC6X80",
 		.init_setup	= init_setup_aec6x80,
@@ -382,6 +396,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "AEC6X80R",
 		.init_setup	= init_setup_aec6x80,
@@ -392,6 +407,7 @@ static ide_pci_device_t aec62xx_chipsets
 		.autodma	= AUTODMA,
 		.enablebits	= {{0x4a,0x02,0x02}, {0x4a,0x04,0x04}},
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
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;
 
Index: b/drivers/ide/pci/cmd64x.c
===================================================================
--- a/drivers/ide/pci/cmd64x.c
+++ b/drivers/ide/pci/cmd64x.c
@@ -695,9 +695,10 @@ static void __devinit init_hwif_cmd64x(i
 	hwif->swdma_mask = 0x07;
 
 	if (dev->device == PCI_DEVICE_ID_CMD_643)
-		hwif->ultra_mask = 0x80;
+		hwif->ultra_mask = 0x00;
 	if (dev->device == PCI_DEVICE_ID_CMD_646)
-		hwif->ultra_mask = (class_rev > 0x04) ? 0x07 : 0x80;
+		hwif->ultra_mask =
+			(class_rev == 0x05 || class_rev == 0x07) ? 0x07 : 0x00;
 	if (dev->device == PCI_DEVICE_ID_CMD_648)
 		hwif->ultra_mask = 0x1f;
 
Index: b/drivers/ide/pci/pdc202xx_new.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_new.c
+++ b/drivers/ide/pci/pdc202xx_new.c
@@ -545,7 +545,7 @@ static void __devinit init_hwif_pdc202ne
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x7f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 
 	hwif->err_stops_fifo = 1;
@@ -621,6 +621,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 1 */
 		.name		= "PDC20269",
 		.init_setup	= init_setup_pdcnew,
@@ -629,6 +630,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 2 */
 		.name		= "PDC20270",
 		.init_setup	= init_setup_pdc20270,
@@ -637,6 +639,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 3 */
 		.name		= "PDC20271",
 		.init_setup	= init_setup_pdcnew,
@@ -645,6 +648,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 4 */
 		.name		= "PDC20275",
 		.init_setup	= init_setup_pdcnew,
@@ -653,6 +657,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 5 */
 		.name		= "PDC20276",
 		.init_setup	= init_setup_pdc20276,
@@ -661,6 +666,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	},{	/* 6 */
 		.name		= "PDC20277",
 		.init_setup	= init_setup_pdcnew,
@@ -669,6 +675,7 @@ static ide_pci_device_t pdcnew_chipsets[
 		.channels	= 2,
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
+		.udma_mask	= 0x7f, /* udma0-6*/
 	}
 };
 
Index: b/drivers/ide/pci/pdc202xx_old.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -488,7 +488,7 @@ static void __devinit init_hwif_pdc202xx
 
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
-	hwif->ultra_mask = 0x3f;
+	hwif->ultra_mask = hwif->cds->udma_mask;
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 	hwif->atapi_dma = 1;
@@ -597,6 +597,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 16,
+		.udma_mask	= 0x07, /* udma0-2 */
 	},{	/* 1 */
 		.name		= "PDC20262",
 		.init_setup	= init_setup_pdc202ata4,
@@ -607,6 +608,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 2 */
 		.name		= "PDC20263",
 		.init_setup	= init_setup_pdc202ata4,
@@ -617,6 +619,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x1f, /* udma0-4 */
 	},{	/* 3 */
 		.name		= "PDC20265",
 		.init_setup	= init_setup_pdc20265,
@@ -627,6 +630,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	},{	/* 4 */
 		.name		= "PDC20267",
 		.init_setup	= init_setup_pdc202xx,
@@ -637,6 +641,7 @@ static ide_pci_device_t pdc202xx_chipset
 		.autodma	= AUTODMA,
 		.bootable	= OFF_BOARD,
 		.extra		= 48,
+		.udma_mask	= 0x3f, /* udma0-5 */
 	}
 };
 
Index: b/drivers/ide/pci/piix.c
===================================================================
--- a/drivers/ide/pci/piix.c
+++ b/drivers/ide/pci/piix.c
@@ -493,7 +493,7 @@ static void __devinit init_hwif_piix(ide
 		case PCI_DEVICE_ID_INTEL_82371FB_0:
 		case PCI_DEVICE_ID_INTEL_82371FB_1:
 		case PCI_DEVICE_ID_INTEL_82371SB_1:
-			hwif->ultra_mask = 0x80;
+			hwif->ultra_mask = 0x00;
 			break;
 		case PCI_DEVICE_ID_INTEL_82371AB:
 		case PCI_DEVICE_ID_INTEL_82443MX_1:
@@ -501,6 +501,10 @@ static void __devinit init_hwif_piix(ide
 		case PCI_DEVICE_ID_INTEL_82801AB_1:
 			hwif->ultra_mask = 0x07;
 			break;
+		case PCI_DEVICE_ID_INTEL_82801AA_1:
+		case PCI_DEVICE_ID_INTEL_82372FB_1:
+			hwif->ultra_mask = 0x1f;
+			break;
 		default:
 			if (!hwif->udma_four)
 				hwif->udma_four = piix_cable_detect(hwif);
Index: b/drivers/ide/pci/sis5513.c
===================================================================
--- a/drivers/ide/pci/sis5513.c
+++ b/drivers/ide/pci/sis5513.c
@@ -858,6 +858,8 @@ static unsigned int __devinit ata66_sis5
 
 static void __devinit init_hwif_sis5513 (ide_hwif_t *hwif)
 {
+	u8 udma_rates[] = { 0x00, 0x00, 0x07, 0x1f, 0x3f, 0x3f, 0x7f, 0x7f };
+
 	hwif->autodma = 0;
 
 	if (!hwif->irq)
@@ -873,7 +875,8 @@ static void __devinit init_hwif_sis5513 
 	}
 
 	hwif->atapi_dma = 1;
-	hwif->ultra_mask = 0x7f;
+
+	hwif->ultra_mask = udma_rates[chipset_family];
 	hwif->mwdma_mask = 0x07;
 	hwif->swdma_mask = 0x07;
 
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1255,6 +1255,7 @@ typedef struct ide_pci_device_s {
 	unsigned int		extra;
 	struct ide_pci_device_s	*next;
 	u8			flags;
+	u8			udma_mask;
 } ide_pci_device_t;
 
 extern int ide_setup_pci_device(struct pci_dev *, ide_pci_device_t *);

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2007-04-22 16:18 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-03 17:24 [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
2007-04-22 16:19 ` Sergei Shtylyov
  -- strict thread matches above, loose matches on Subject: below --
2007-02-03 13:53 [PATCH 11/15] ide: make ide_hwif_t.ide_dma_{host_off,off_quietly} void Bartlomiej Zolnierkiewicz
2007-02-03 13:53 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
2007-01-19  0:30 [PATCH 0/15] IDE quilt tree updated Bartlomiej Zolnierkiewicz
2007-01-19  0:32 ` [PATCH 13/15] ide: fix UDMA/MWDMA/SWDMA masks Bartlomiej Zolnierkiewicz
2007-01-22 16:19   ` Sergei Shtylyov
     [not found]     ` <58cb370e0702021606v4efa6143lf060e6aab9782c35@mail.gmail.com>
2007-02-03  0:39       ` Bartlomiej Zolnierkiewicz
2007-02-03 16:30         ` Sergei Shtylyov
2007-01-22 18:17   ` Sergei Shtylyov
2007-01-22 18:46     ` Alan
2007-01-22 20:28       ` Sergei Shtylyov
2007-01-22 21:31         ` Alan
2007-01-22 22:39           ` Jeff Garzik
2007-01-23 14:51           ` Sergei Shtylyov
2007-01-25 15:40             ` Sergei Shtylyov
2007-01-31 20:38       ` Sergei Shtylyov
2007-01-31 23:24         ` Alan
     [not found]     ` <58cb370e0702021606m4eb6f682xfa4bf769d398cf9@mail.gmail.com>
2007-02-03  0:50       ` Bartlomiej Zolnierkiewicz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).