[PATCH 0/6] ATA driver updates

[PATCH 0/6] ATA driver updates

[Alan Cox]

(Cc'd Davem as the piccolo driver touches the drivers/ide/ide-pci-generic.c
 to update the PCI identifiers and keep them all in sync with the name
 corrections as pointed out by Bartlomiej)

---

Alan Cox (6):
      pata_piccolo: Driver for old Toshiba chipsets
      pata_ali: Fix regression with old devices
      pata: Update experimental tags
      cmd64x: implement serialization as per notes
      pata_sis: Implement MWDMA for the UDMA 133 capable chips
      pata_via: Blacklist some combinations of Transcend Flash and via


 drivers/ata/Kconfig           |   39 +++++++----
 drivers/ata/Makefile          |    1 
 drivers/ata/ata_generic.c     |    5 +
 drivers/ata/pata_ali.c        |    4 +
 drivers/ata/pata_cmd64x.c     |  116 ++++++++++++++++++++++++++++++++--
 drivers/ata/pata_piccolo.c    |  140 +++++++++++++++++++++++++++++++++++++++++
 drivers/ata/pata_sis.c        |   91 ++++++++++++++++++++-------
 drivers/ata/pata_via.c        |   27 ++++++++
 drivers/ide/ide-pci-generic.c |    3 +
 include/linux/pci_ids.h       |    7 +-
 10 files changed, 382 insertions(+), 51 deletions(-)
 create mode 100644 drivers/ata/pata_piccolo.c

-- 
"The IETF already has more than enough RFCs that codify the obvious, make
stupidity illegal, support truth, justice, and the IETF way, and generally
demonstrate the author is a brilliant and valuable Contributor to The
Standards Process." -- Vernon Schryver

[PATCH 1/6] pata_via: Blacklist some combinations of Transcend Flash and via

[Alan Cox]

Reported by Mikulas Patocka.

VIA VT82C586B + Transcend TS64GSSD25-M v0826 does not work in UDMA mode

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/ata/pata_via.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)


diff --git a/drivers/ata/pata_via.c b/drivers/ata/pata_via.c
index 520d5a3..78eac27 100644
--- a/drivers/ata/pata_via.c
+++ b/drivers/ata/pata_via.c
@@ -337,6 +337,32 @@ static void via_set_dmamode(struct ata_port *ap, struct ata_device *adev)
 }
 
 /**
+ *	via_mode_filter		-	filter buggy device/mode pairs
+ *	@dev: ATA device
+ *	@mask: Mode bitmask
+ *
+ *	We need to apply some minimal filtering for old controllers and at least
+ *	one breed of Transcend SSD. Return the updated mask.
+ */
+
+static unsigned long via_mode_filter(struct ata_device *dev, unsigned long mask)
+{
+	struct ata_host *host = dev->link->ap->host;
+	const struct via_isa_bridge *config = host->private_data;
+	unsigned char model_num[ATA_ID_PROD_LEN + 1];
+
+	if (config->id == PCI_DEVICE_ID_VIA_82C586_0) {
+		ata_id_c_string(dev->id, model_num, ATA_ID_PROD, sizeof(model_num));
+		if (strcmp(model_num, "TS64GSSD25-M") == 0) {
+			ata_dev_printk(dev, KERN_WARNING,
+	"disabling UDMA mode due to reported lockups with this device.\n");
+			mask &= ~ ATA_MASK_UDMA;
+		}
+	}
+	return ata_bmdma_mode_filter(dev, mask);
+}
+
+/**
  *	via_tf_load - send taskfile registers to host controller
  *	@ap: Port to which output is sent
  *	@tf: ATA taskfile register set
@@ -427,6 +453,7 @@ static struct ata_port_operations via_port_ops = {
 	.prereset	= via_pre_reset,
 	.sff_tf_load	= via_tf_load,
 	.port_start	= via_port_start,
+	.mode_filter	= via_mode_filter,
 };
 
 static struct ata_port_operations via_port_ops_noirq = {

[PATCH 2/6] pata_sis: Implement MWDMA for the UDMA 133 capable chips

[Alan Cox]

Bartlomiej pointed out that while this got fixed in the old driver whoever
did it didn't port it across.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/ata/pata_sis.c |   91 ++++++++++++++++++++++++++++++++++++------------
 1 files changed, 69 insertions(+), 22 deletions(-)


diff --git a/drivers/ata/pata_sis.c b/drivers/ata/pata_sis.c
index 488e77b..d70ecec 100644
--- a/drivers/ata/pata_sis.c
+++ b/drivers/ata/pata_sis.c
@@ -252,24 +252,25 @@ static void sis_100_set_piomode (struct ata_port *ap, struct ata_device *adev)
 }
 
 /**
- *	sis_133_set_piomode - Initialize host controller PATA PIO timings
+ *	sis_133_do_piomode - Initialize host controller PATA PIO/DMA timings
  *	@ap: Port whose timings we are configuring
  *	@adev: Device we are configuring for.
  *
  *	Set PIO mode for device, in host controller PCI config space. This
- *	function handles PIO set up for the later ATA133 devices.
+ *	function handles PIO set up for the later ATA133 devices. The same
+ *	timings are used for MWDMA.
  *
  *	LOCKING:
  *	None (inherited from caller).
  */
 
-static void sis_133_set_piomode (struct ata_port *ap, struct ata_device *adev)
+static void sis_133_do_piomode(struct ata_port *ap, struct ata_device *adev,
+					int speed)
 {
 	struct pci_dev *pdev	= to_pci_dev(ap->host->dev);
 	int port = 0x40;
 	u32 t1;
 	u32 reg54;
-	int speed = adev->pio_mode - XFER_PIO_0;
 
 	const u32 timing133[] = {
 		0x28269000,	/* Recovery << 24 | Act << 16 | Ini << 12 */
@@ -305,6 +306,42 @@ static void sis_133_set_piomode (struct ata_port *ap, struct ata_device *adev)
 }
 
 /**
+ *	sis_133_set_piomode - Initialize host controller PATA PIO timings
+ *	@ap: Port whose timings we are configuring
+ *	@adev: Device we are configuring for.
+ *
+ *	Set PIO mode for device, in host controller PCI config space. This
+ *	function handles PIO set up for the later ATA133 devices.
+ *
+ *	LOCKING:
+ *	None (inherited from caller).
+ */
+
+static void sis_133_set_piomode (struct ata_port *ap, struct ata_device *adev)
+{
+
+	sis_133_do_piomode(ap, adev, adev->pio_mode - XFER_PIO_0);
+}
+
+/**
+ *	mwdma_clip_to_pio		-	clip MWDMA mode
+ *	@adev: device
+ *
+ *	As the SiS shared MWDMA and PIO timings we must program the equivalent
+ *	PIO timing for the MWDMA mode but we must not program one higher than
+ *	the permitted PIO timing of the device.
+ */
+
+static int mwdma_clip_to_pio(struct ata_device *adev)
+{
+	const int mwdma_to_pio[3] = {
+		XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
+	};
+	return min(mwdma_to_pio[adev->dma_mode - XFER_MW_DMA_0],
+				adev->pio_mode - XFER_PIO_0);
+}
+
+/**
  *	sis_old_set_dmamode - Initialize host controller PATA DMA timings
  *	@ap: Port whose timings we are configuring
  *	@adev: Device to program
@@ -332,6 +369,7 @@ static void sis_old_set_dmamode (struct ata_port *ap, struct ata_device *adev)
 	if (adev->dma_mode < XFER_UDMA_0) {
 		/* bits 3-0 hold recovery timing bits 8-10 active timing and
 		   the higher bits are dependant on the device */
+		speed = mwdma_clip_to_pio(adev);
 		timing &= ~0x870F;
 		timing |= mwdma_bits[speed];
 	} else {
@@ -372,6 +410,7 @@ static void sis_66_set_dmamode (struct ata_port *ap, struct ata_device *adev)
 	if (adev->dma_mode < XFER_UDMA_0) {
 		/* bits 3-0 hold recovery timing bits 8-10 active timing and
 		   the higher bits are dependant on the device, bit 15 udma */
+		speed = mwdma_clip_to_pio(adev);
 		timing &= ~0x870F;
 		timing |= mwdma_bits[speed];
 	} else {
@@ -389,7 +428,7 @@ static void sis_66_set_dmamode (struct ata_port *ap, struct ata_device *adev)
  *	@adev: Device to program
  *
  *	Set UDMA/MWDMA mode for device, in host controller PCI config space.
- *	Handles UDMA66 and early UDMA100 devices.
+ *	Handles later UDMA100 devices.
  *
  *	LOCKING:
  *	None (inherited from caller).
@@ -400,21 +439,25 @@ static void sis_100_set_dmamode (struct ata_port *ap, struct ata_device *adev)
 	struct pci_dev *pdev	= to_pci_dev(ap->host->dev);
 	int speed = adev->dma_mode - XFER_MW_DMA_0;
 	int drive_pci = sis_old_port_base(adev);
-	u8 timing;
+	u16 timing;
 
-	const u8 udma_bits[]  = { 0x8B, 0x87, 0x85, 0x83, 0x82, 0x81};
+	const u16 udma_bits[]  = {
+		0x8B00, 0x8700, 0x8500, 0x8300, 0x8200, 0x8100};
+	const u8 mwdma_bits[] = { 0x08, 0x32, 0x31 };
 
-	pci_read_config_byte(pdev, drive_pci + 1, &timing);
+	pci_read_config_word(pdev, drive_pci, &timing);
 
 	if (adev->dma_mode < XFER_UDMA_0) {
-		/* NOT SUPPORTED YET: NEED DATA SHEET. DITTO IN OLD DRIVER */
+		speed = mwdma_clip_to_pio(adev);
+		timing &= ~0x80FF;
+		timing |= mwdma_bits[speed];
 	} else {
 		/* Bit 7 is UDMA on/off, bit 0-3 are cycle time */
 		speed = adev->dma_mode - XFER_UDMA_0;
-		timing &= ~0x8F;
+		timing &= ~0x8F00;
 		timing |= udma_bits[speed];
 	}
-	pci_write_config_byte(pdev, drive_pci + 1, timing);
+	pci_write_config_word(pdev, drive_pci, timing);
 }
 
 /**
@@ -434,21 +477,26 @@ static void sis_133_early_set_dmamode (struct ata_port *ap, struct ata_device *a
 	struct pci_dev *pdev	= to_pci_dev(ap->host->dev);
 	int speed = adev->dma_mode - XFER_MW_DMA_0;
 	int drive_pci = sis_old_port_base(adev);
-	u8 timing;
-	/* Low 4 bits are timing */
-	static const u8 udma_bits[]  = { 0x8F, 0x8A, 0x87, 0x85, 0x83, 0x82, 0x81};
+	u16 timing;
+	/* Bits 15-12  are timing */
+	static const u16 udma_bits[]  = {
+		0x8F00, 0x8A00, 0x8700, 0x8500, 0x8300, 0x8200, 0x8100
+	};
+	static const u8 mwdma_bits[] = { 0x08, 0x32, 0x31 };
 
-	pci_read_config_byte(pdev, drive_pci + 1, &timing);
+	pci_read_config_word(pdev, drive_pci, &timing);
 
 	if (adev->dma_mode < XFER_UDMA_0) {
-		/* NOT SUPPORTED YET: NEED DATA SHEET. DITTO IN OLD DRIVER */
+		speed = mwdma_clip_to_pio(adev);
+		timing &= ~0x80FF;
+		timing = mwdma_bits[speed];
 	} else {
 		/* Bit 7 is UDMA on/off, bit 0-3 are cycle time */
 		speed = adev->dma_mode - XFER_UDMA_0;
-		timing &= ~0x8F;
+		timing &= ~0x8F00;
 		timing |= udma_bits[speed];
 	}
-	pci_write_config_byte(pdev, drive_pci + 1, timing);
+	pci_write_config_word(pdev, drive_pci, timing);
 }
 
 /**
@@ -479,13 +527,12 @@ static void sis_133_set_dmamode (struct ata_port *ap, struct ata_device *adev)
 	if (reg54 & 0x40000000)
 		port = 0x70;
 	port += (8 * ap->port_no) +  (4 * adev->devno);
-
 	pci_read_config_dword(pdev, port, &t1);
 
 	if (adev->dma_mode < XFER_UDMA_0) {
-		t1 &= ~0x00000004;
-		/* FIXME: need data sheet to add MWDMA here. Also lacking on
-		   ide/pci driver */
+		speed = mwdma_clip_to_pio(adev);
+		sis_133_do_piomode(ap, adev, speed);
+		t1 &= ~4;	/* UDMA off */
 	} else {
 		speed = adev->dma_mode - XFER_UDMA_0;
 		/* if & 8 no UDMA133 - need info for ... */

[PATCH 3/6] cmd64x: implement serialization as per notes

[Alan Cox]

Daniela Engert pointed out that there are some implementation notes for the
643 and 646 that deal with certain serialization rules. In theory we don't
need them because they apply when the motherboard decides not to retry PCI
requests for long enough and the chip is busy doing a DMA transfer on the
other channel.

The rule basically is "don't touch the taskfile of the other channel while
a DMA is in progress". To implement that we need to

- not issue a command on a channel when there is a DMA command queued
- not issue a DMA command on a channel when there are PIO commands queued
- use the alternative access to the interrupt source so that we do not
  touch altstatus or status on shared IRQ.

Updated to remote extra conditional check Bartlomiej noted and to remove
the variables for irq checks as the CMD648 doesn't have the underlying problem.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/ata/pata_cmd64x.c |  116 ++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 108 insertions(+), 8 deletions(-)


diff --git a/drivers/ata/pata_cmd64x.c b/drivers/ata/pata_cmd64x.c
index f98dffe..e4d3a1e 100644
--- a/drivers/ata/pata_cmd64x.c
+++ b/drivers/ata/pata_cmd64x.c
@@ -31,7 +31,7 @@
 #include <linux/libata.h>
 
 #define DRV_NAME "pata_cmd64x"
-#define DRV_VERSION "0.2.5"
+#define DRV_VERSION "0.3.1"
 
 /*
  * CMD64x specific registers definition.
@@ -254,17 +254,109 @@ static void cmd648_bmdma_stop(struct ata_queued_cmd *qc)
 }
 
 /**
- *	cmd646r1_dma_stop	-	DMA stop callback
+ *	cmd64x_bmdma_stop	-	DMA stop callback
  *	@qc: Command in progress
  *
- *	Stub for now while investigating the r1 quirk in the old driver.
+ *	Track the completion of live DMA commands and clear the
+ *	host->private_data DMA tracking flag as we do.
  */
 
-static void cmd646r1_bmdma_stop(struct ata_queued_cmd *qc)
+static void cmd64x_bmdma_stop(struct ata_queued_cmd *qc)
 {
+	struct ata_port *ap = qc->ap;
 	ata_bmdma_stop(qc);
+	WARN_ON(ap->host->private_data != ap);
+	ap->host->private_data = NULL;
+}
+
+/**
+ *	cmd64x_qc_defer		-	Defer logic for chip limits
+ *	@qc: queued command
+ *
+ *	Decide whether we can issue the command. Called under the host lock.
+ */
+
+static int cmd64x_qc_defer(struct ata_queued_cmd *qc)
+{
+	struct ata_host *host = qc->ap->host;
+	struct ata_port *alt = host->ports[1 ^ qc->ap->port_no];
+	int rc;
+	int dma = 0;
+
+	/* Apply the ATA rules first */
+	rc = ata_std_qc_defer(qc);
+	if (rc)
+		return rc;
+
+	if (qc->tf.protocol == ATAPI_PROT_DMA ||
+			qc->tf.protocol == ATA_PROT_DMA)
+		dma = 1;
+
+	/* If the other port is not live then issue the command */
+	if (alt == NULL || !alt->qc_active) {
+		if (dma)
+			host->private_data = qc->ap;
+		return 0;
+	}
+	/* If there is a live DMA command then wait */
+	if (host->private_data != NULL)
+		return 	ATA_DEFER_PORT;
+	if (dma)
+		/* Cannot overlap our DMA command */
+		return ATA_DEFER_PORT;
+	return 0;
 }
 
+/**
+ *	cmd64x_interrupt - ATA host interrupt handler
+ *	@irq: irq line (unused)
+ *	@dev_instance: pointer to our ata_host information structure
+ *
+ *	Our interrupt handler for PCI IDE devices.  Calls
+ *	ata_sff_host_intr() for each port that is flagging an IRQ. We cannot
+ *	use the defaults as we need to avoid touching status/altstatus during
+ *	a DMA.
+ *
+ *	LOCKING:
+ *	Obtains host lock during operation.
+ *
+ *	RETURNS:
+ *	IRQ_NONE or IRQ_HANDLED.
+ */
+irqreturn_t cmd64x_interrupt(int irq, void *dev_instance)
+{
+	struct ata_host *host = dev_instance;
+	struct pci_dev *pdev = to_pci_dev(host->dev);
+	unsigned int i;
+	unsigned int handled = 0;
+	unsigned long flags;
+	static const u8 irq_reg[2] = { CFR, ARTTIM23 };
+	static const u8 irq_mask[2] = { 1 << 2, 1 << 4 };
+
+	/* TODO: make _irqsave conditional on x86 PCI IDE legacy mode */
+	spin_lock_irqsave(&host->lock, flags);
+
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap;
+		u8 reg;
+
+		pci_read_config_byte(pdev, irq_reg[i], &reg);
+		ap = host->ports[i];
+		if (ap && (reg & irq_mask[i]) &&
+		    !(ap->flags & ATA_FLAG_DISABLED)) {
+			struct ata_queued_cmd *qc;
+
+			qc = ata_qc_from_tag(ap, ap->link.active_tag);
+			if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING)) &&
+			    (qc->flags & ATA_QCFLAG_ACTIVE))
+				handled |= ata_sff_host_intr(ap, qc);
+		}
+	}
+
+	spin_unlock_irqrestore(&host->lock, flags);
+
+	return IRQ_RETVAL(handled);
+}
 static struct scsi_host_template cmd64x_sht = {
 	ATA_BMDMA_SHT(DRV_NAME),
 };
@@ -273,6 +365,8 @@ static const struct ata_port_operations cmd64x_base_ops = {
 	.inherits	= &ata_bmdma_port_ops,
 	.set_piomode	= cmd64x_set_piomode,
 	.set_dmamode	= cmd64x_set_dmamode,
+	.bmdma_stop	= cmd64x_bmdma_stop,
+	.qc_defer	= cmd64x_qc_defer,
 };
 
 static struct ata_port_operations cmd64x_port_ops = {
@@ -282,7 +376,6 @@ static struct ata_port_operations cmd64x_port_ops = {
 
 static struct ata_port_operations cmd646r1_port_ops = {
 	.inherits	= &cmd64x_base_ops,
-	.bmdma_stop	= cmd646r1_bmdma_stop,
 	.cable_detect	= ata_cable_40wire,
 };
 
@@ -290,6 +383,7 @@ static struct ata_port_operations cmd648_port_ops = {
 	.inherits	= &cmd64x_base_ops,
 	.bmdma_stop	= cmd648_bmdma_stop,
 	.cable_detect	= cmd648_cable_detect,
+	.qc_defer	= ata_std_qc_defer
 };
 
 static int cmd64x_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
@@ -340,6 +434,7 @@ static int cmd64x_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	const struct ata_port_info *ppi[] = { &cmd_info[id->driver_data], NULL };
 	u8 mrdmode;
 	int rc;
+	struct ata_host *host;
 
 	rc = pcim_enable_device(pdev);
 	if (rc)
@@ -360,20 +455,25 @@ static int cmd64x_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 			ppi[0] = &cmd_info[3];
 	}
 
+
 	pci_write_config_byte(pdev, PCI_LATENCY_TIMER, 64);
 	pci_read_config_byte(pdev, MRDMODE, &mrdmode);
 	mrdmode &= ~ 0x30;	/* IRQ set up */
 	mrdmode |= 0x02;	/* Memory read line enable */
 	pci_write_config_byte(pdev, MRDMODE, mrdmode);
 
-	/* Force PIO 0 here.. */
-
 	/* PPC specific fixup copied from old driver */
 #ifdef CONFIG_PPC
 	pci_write_config_byte(pdev, UDIDETCR0, 0xF0);
 #endif
+	rc = ata_pci_sff_prepare_host(pdev, ppi, &host);
+	if (rc)
+		return rc;
+	/* We use this pointer to track the AP which has DMA running */
+	host->private_data = NULL;
 
-	return ata_pci_sff_init_one(pdev, ppi, &cmd64x_sht, NULL);
+	pci_set_master(pdev);
+	return ata_pci_sff_activate_host(host, cmd64x_interrupt, &cmd64x_sht);
 }
 
 #ifdef CONFIG_PM

[PATCH 4/6] pata: Update experimental tags

[Alan Cox]

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/ata/Kconfig |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)


diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 33133c6..a47ca75 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -374,8 +374,8 @@ config PATA_HPT366
 	  If unsure, say N.
 
 config PATA_HPT37X
-	tristate "HPT 370/370A/371/372/374/302 PATA support (Experimental)"
-	depends on PCI && EXPERIMENTAL
+	tristate "HPT 370/370A/371/372/374/302 PATA support"
+	depends on PCI
 	help
 	  This option enables support for the majority of the later HPT
 	  PATA controllers via the new ATA layer.
@@ -383,8 +383,8 @@ config PATA_HPT37X
 	  If unsure, say N.
 
 config PATA_HPT3X2N
-	tristate "HPT 372N/302N PATA support (Experimental)"
-	depends on PCI && EXPERIMENTAL
+	tristate "HPT 372N/302N PATA support"
+	depends on PCI
 	help
 	  This option enables support for the N variant HPT PATA
 	  controllers via the new ATA layer
@@ -401,7 +401,7 @@ config PATA_HPT3X3
 	  If unsure, say N.
 
 config PATA_HPT3X3_DMA
-	bool "HPT 343/363 DMA support (Experimental)"
+	bool "HPT 343/363 DMA support"
 	depends on PATA_HPT3X3
 	help
 	  This option enables DMA support for the HPT343/363
@@ -510,8 +510,8 @@ config PATA_NETCELL
 	  If unsure, say N.
 
 config PATA_NINJA32
-	tristate "Ninja32/Delkin Cardbus ATA support (Experimental)"
-	depends on PCI && EXPERIMENTAL
+	tristate "Ninja32/Delkin Cardbus ATA support"
+	depends on PCI
 	help
 	  This option enables support for the Ninja32, Delkin and
 	  possibly other brands of Cardbus ATA adapter
@@ -573,6 +573,14 @@ config PATA_PCMCIA
 
 	  If unsure, say N.
 
+config PATA_PDC2027X
+	tristate "Promise PATA 2027x support"
+	depends on PCI
+	help
+	  This option enables support for Promise PATA pdc20268 to pdc20277 host adapters.
+
+	  If unsure, say N.
+
 config PATA_PDC_OLD
 	tristate "Older Promise PATA controller support"
 	depends on PCI
@@ -643,14 +651,6 @@ config PATA_SERVERWORKS
 
 	  If unsure, say N.
 
-config PATA_PDC2027X
-	tristate "Promise PATA 2027x support"
-	depends on PCI
-	help
-	  This option enables support for Promise PATA pdc20268 to pdc20277 host adapters.
-
-	  If unsure, say N.
-
 config PATA_SIL680
 	tristate "CMD / Silicon Image 680 PATA support"
 	depends on PCI

[PATCH 5/6] pata_ali: Fix regression with old devices

[Alan Cox]

Making the new stuff work broke some of the old chipsets. We need to go
back to the old set up values for these it seems. Unfortunately even with
documentation this is basically a mix of cargoculting and guesswork.

Chased down to the exact line by Gianluca.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/ata/pata_ali.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)


diff --git a/drivers/ata/pata_ali.c b/drivers/ata/pata_ali.c
index 1432dc9..9434114 100644
--- a/drivers/ata/pata_ali.c
+++ b/drivers/ata/pata_ali.c
@@ -453,7 +453,9 @@ static void ali_init_chipset(struct pci_dev *pdev)
 			/* Clear CD-ROM DMA write bit */
 			tmp &= 0x7F;
 		/* Cable and UDMA */
-		pci_write_config_byte(pdev, 0x4B, tmp | 0x09);
+		if (pdev->revision >= 0xc2)
+			tmp |= 0x01;
+		pci_write_config_byte(pdev, 0x4B, tmp | 0x08);
 		/*
 		 * CD_ROM DMA on (0x53 bit 0). Enable this even if we want
 		 * to use PIO. 0x53 bit 1 (rev 20 only) - enable FIFO control

[PATCH 6/6] pata_piccolo: Driver for old Toshiba chipsets

[Alan Cox]

We were never able to get docs for this out of Toshiba for years. Dave
Barnes produced a NetBSD driver however and from that we can fill in the
needed tables.

As we correct the PCI identifiers a bit also update the old ide generic driver
at the same time so it stays compiling.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/ata/Kconfig           |    9 +++
 drivers/ata/Makefile          |    1 
 drivers/ata/ata_generic.c     |    5 +
 drivers/ata/pata_piccolo.c    |  140 +++++++++++++++++++++++++++++++++++++++++
 drivers/ide/ide-pci-generic.c |    3 +
 include/linux/pci_ids.h       |    7 +-
 6 files changed, 160 insertions(+), 5 deletions(-)
 create mode 100644 drivers/ata/pata_piccolo.c


diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index a47ca75..676f08b 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -667,6 +667,15 @@ config PATA_SIS
 
 	  If unsure, say N.
 
+config PATA_TOSHIBA
+	tristate "Toshiba Piccolo support (Experimental)"
+	depends on PCI && EXPERIMENTAL
+	help
+	  Support for the Toshiba Piccolo controllers. Currently only the
+	  primary channel is supported by this driver.
+
+	  If unsure, say N.
+
 config PATA_VIA
 	tristate "VIA PATA support"
 	depends on PCI
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 01e126f..d909435 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_PATA_RZ1000)	+= pata_rz1000.o
 obj-$(CONFIG_PATA_SC1200)	+= pata_sc1200.o
 obj-$(CONFIG_PATA_SERVERWORKS)	+= pata_serverworks.o
 obj-$(CONFIG_PATA_SIL680)	+= pata_sil680.o
+obj-$(CONFIG_PATA_TOSHIBA)	+= pata_piccolo.o
 obj-$(CONFIG_PATA_VIA)		+= pata_via.o
 obj-$(CONFIG_PATA_WINBOND)	+= pata_sl82c105.o
 obj-$(CONFIG_PATA_WINBOND_VLB)	+= pata_winbond.o
diff --git a/drivers/ata/ata_generic.c b/drivers/ata/ata_generic.c
index ecfd22b..12e26c3 100644
--- a/drivers/ata/ata_generic.c
+++ b/drivers/ata/ata_generic.c
@@ -168,9 +168,12 @@ static struct pci_device_id ata_generic[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_VIA,    PCI_DEVICE_ID_VIA_82C561), },
 	{ PCI_DEVICE(PCI_VENDOR_ID_OPTI,   PCI_DEVICE_ID_OPTI_82C558), },
 	{ PCI_DEVICE(PCI_VENDOR_ID_CENATEK,PCI_DEVICE_ID_CENATEK_IDE), },
-	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO), },
+#if !defined(CONFIG_PATA_TOSHIBA) && !defined(CONFIG_PATA_TOSHIBA_MODULE)
 	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_1), },
 	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_2),  },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_3),  },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_5),  },
+#endif	
 	/* Must come last. If you add entries adjust this table appropriately */
 	{ PCI_ANY_ID,		PCI_ANY_ID,			   PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_STORAGE_IDE << 8, 0xFFFFFF00UL, 1},
 	{ 0, },
diff --git a/drivers/ata/pata_piccolo.c b/drivers/ata/pata_piccolo.c
new file mode 100644
index 0000000..bfe0180
--- /dev/null
+++ b/drivers/ata/pata_piccolo.c
@@ -0,0 +1,140 @@
+/*
+ *  pata_piccolo.c - Toshiba Piccolo PATA/SATA controller driver.
+ *
+ *  This is basically an update to ata_generic.c to add Toshiba Piccolo support
+ *  then split out to keep ata_generic "clean".
+ *
+ *  Copyright 2005 Red Hat Inc, all rights reserved.
+ *
+ *  Elements from ide/pci/generic.c
+ *	    Copyright (C) 2001-2002	Andre Hedrick <andre@linux-ide.org>
+ *	    Portions (C) Copyright 2002  Red Hat Inc <alan@redhat.com>
+ *
+ *  May be copied or modified under the terms of the GNU General Public License
+ *
+ *  The timing data tables/programming info are courtesy of the NetBSD driver
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+#include <linux/blkdev.h>
+#include <linux/delay.h>
+#include <scsi/scsi_host.h>
+#include <linux/libata.h>
+
+#define DRV_NAME "pata_piccolo"
+#define DRV_VERSION "0.0.1"
+
+
+
+static void tosh_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+	static const u16 pio[6] = {	/* For reg 0x50 low word & E088 */
+		0x0566, 0x0433, 0x0311, 0x0201, 0x0200, 0x0100
+	};
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	u16 conf;
+	pci_read_config_word(pdev, 0x50, &conf);
+	conf &= 0xE088;
+	conf |= pio[adev->pio_mode - XFER_PIO_0];
+	pci_write_config_word(pdev, 0x50, conf);
+}
+
+static void tosh_set_dmamode(struct ata_port *ap, struct ata_device *adev)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+	u32 conf;
+	pci_read_config_dword(pdev, 0x5C, &conf);
+	conf &= 0x78FFE088;	/* Keep the other bits */
+	if (adev->dma_mode >= XFER_UDMA_0) {
+		int udma = adev->dma_mode - XFER_UDMA_0;
+		conf |= 0x80000000;
+		conf |= (udma + 2) << 28;
+		conf |= (2 - udma) * 0x111;	/* spread into three nibbles */
+	} else {
+		static const u32 mwdma[4] = {
+			0x0655, 0x0200, 0x0200, 0x0100
+		};
+		conf |= mwdma[adev->dma_mode - XFER_MW_DMA_0];
+	}
+	pci_write_config_dword(pdev, 0x5C, conf);
+}
+
+
+static struct scsi_host_template tosh_sht = {
+	ATA_BMDMA_SHT(DRV_NAME),
+};
+
+static struct ata_port_operations tosh_port_ops = {
+	.inherits	= &ata_bmdma_port_ops,
+	.cable_detect	= ata_cable_unknown,
+	.set_piomode	= tosh_set_piomode,
+	.set_dmamode	= tosh_set_dmamode
+};
+
+/**
+ *	ata_tosh_init		-	attach generic IDE
+ *	@dev: PCI device found
+ *	@id: match entry
+ *
+ *	Called each time a matching IDE interface is found. We check if the
+ *	interface is one we wish to claim and if so we perform any chip
+ *	specific hacks then let the ATA layer do the heavy lifting.
+ */
+
+static int ata_tosh_init_one(struct pci_dev *dev, const struct pci_device_id *id)
+{
+	static const struct ata_port_info info = {
+		.flags = ATA_FLAG_SLAVE_POSS,
+		.pio_mask = ATA_PIO5,
+		.mwdma_mask = ATA_MWDMA2,
+		.udma_mask = ATA_UDMA2,
+		.port_ops = &tosh_port_ops
+	};
+	const struct ata_port_info *ppi[] = { &info, &ata_dummy_port_info };
+	/* Just one port for the moment */
+	return ata_pci_sff_init_one(dev, ppi, &tosh_sht, NULL);
+}
+
+static struct pci_device_id ata_tosh[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_1), },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_2),  },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_3),  },
+	{ PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_5),  },
+	{ 0, },
+};
+
+static struct pci_driver ata_tosh_pci_driver = {
+	.name 		= DRV_NAME,
+	.id_table	= ata_tosh,
+	.probe 		= ata_tosh_init_one,
+	.remove		= ata_pci_remove_one,
+#ifdef CONFIG_PM
+	.suspend	= ata_pci_device_suspend,
+	.resume		= ata_pci_device_resume,
+#endif
+};
+
+static int __init ata_tosh_init(void)
+{
+	return pci_register_driver(&ata_tosh_pci_driver);
+}
+
+
+static void __exit ata_tosh_exit(void)
+{
+	pci_unregister_driver(&ata_tosh_pci_driver);
+}
+
+
+MODULE_AUTHOR("Alan Cox");
+MODULE_DESCRIPTION("Low level driver for Toshiba Piccolo ATA");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(pci, ata_tosh);
+MODULE_VERSION(DRV_VERSION);
+
+module_init(ata_tosh_init);
+module_exit(ata_tosh_exit);
+
diff --git a/drivers/ide/ide-pci-generic.c b/drivers/ide/ide-pci-generic.c
index 39d4e01..3f0bc42 100644
--- a/drivers/ide/ide-pci-generic.c
+++ b/drivers/ide/ide-pci-generic.c
@@ -162,9 +162,10 @@ static const struct pci_device_id generic_pci_tbl[] = {
 #ifdef CONFIG_BLK_DEV_IDE_SATA
 	{ PCI_VDEVICE(VIA,	PCI_DEVICE_ID_VIA_8237_SATA),		 5 },
 #endif
-	{ PCI_VDEVICE(TOSHIBA,	PCI_DEVICE_ID_TOSHIBA_PICCOLO),		 4 },
 	{ PCI_VDEVICE(TOSHIBA,	PCI_DEVICE_ID_TOSHIBA_PICCOLO_1),	 4 },
 	{ PCI_VDEVICE(TOSHIBA,	PCI_DEVICE_ID_TOSHIBA_PICCOLO_2),	 4 },
+	{ PCI_VDEVICE(TOSHIBA,	PCI_DEVICE_ID_TOSHIBA_PICCOLO_3),	 4 },
+	{ PCI_VDEVICE(TOSHIBA,	PCI_DEVICE_ID_TOSHIBA_PICCOLO_5)	 4 },
 	{ PCI_VDEVICE(NETCELL,	PCI_DEVICE_ID_REVOLUTION),		 6 },
 	/*
 	 * Must come last.  If you add entries adjust
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index b19bbfd..9afa9b8 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -1496,9 +1496,10 @@
 #define PCI_DEVICE_ID_SBE_WANXL400	0x0104
 
 #define PCI_VENDOR_ID_TOSHIBA		0x1179
-#define PCI_DEVICE_ID_TOSHIBA_PICCOLO	0x0102
-#define PCI_DEVICE_ID_TOSHIBA_PICCOLO_1	0x0103
-#define PCI_DEVICE_ID_TOSHIBA_PICCOLO_2	0x0105
+#define PCI_DEVICE_ID_TOSHIBA_PICCOLO_1	0x0101
+#define PCI_DEVICE_ID_TOSHIBA_PICCOLO_2	0x0102
+#define PCI_DEVICE_ID_TOSHIBA_PICCOLO_3	0x0103
+#define PCI_DEVICE_ID_TOSHIBA_PICCOLO_5	0x0105
 #define PCI_DEVICE_ID_TOSHIBA_TOPIC95	0x060a
 #define PCI_DEVICE_ID_TOSHIBA_TOPIC97	0x060f
 #define PCI_DEVICE_ID_TOSHIBA_TOPIC100	0x0617

Re: [PATCH 6/6] pata_piccolo: Driver for old Toshiba chipsets

[Jeff Garzik]

On 11/30/2009 08:23 AM, Alan Cox wrote:
> We were never able to get docs for this out of Toshiba for years. Dave
> Barnes produced a NetBSD driver however and from that we can fill in the
> needed tables.
>
> As we correct the PCI identifiers a bit also update the old ide generic driver
> at the same time so it stays compiling.
>
> Signed-off-by: Alan Cox<alan@linux.intel.com>
> ---
>
>   drivers/ata/Kconfig           |    9 +++
>   drivers/ata/Makefile          |    1
>   drivers/ata/ata_generic.c     |    5 +
>   drivers/ata/pata_piccolo.c    |  140 +++++++++++++++++++++++++++++++++++++++++
>   drivers/ide/ide-pci-generic.c |    3 +
>   include/linux/pci_ids.h       |    7 +-
>   6 files changed, 160 insertions(+), 5 deletions(-)
>   create mode 100644 drivers/ata/pata_piccolo.c

applied 1-6 (hi DaveM)

Re: [PATCH 6/6] pata_piccolo: Driver for old Toshiba chipsets

[David Miller]

From: Jeff Garzik <jeff@garzik.org>
Date: Thu, 03 Dec 2009 02:36:08 -0500

> applied 1-6 (hi DaveM)

Ok :-)

Re: [PATCH 2/6] pata_sis: Implement MWDMA for the UDMA 133 capable chips

[Sergei Shtylyov]

Hello.

Alan Cox wrote:

> Bartlomiej pointed out that while this got fixed in the old driver whoever
> did it didn't port it across.

> Signed-off-by: Alan Cox <alan@linux.intel.com>

[...]

> +static int mwdma_clip_to_pio(struct ata_device *adev)
> +{
> +	const int mwdma_to_pio[3] = {
> +		XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
> +	};
> +	return min(mwdma_to_pio[adev->dma_mode - XFER_MW_DMA_0],
> +				adev->pio_mode - XFER_PIO_0);
> +}

    You call min() on uncomparables, i.e. mwdma_to_pio[] contains XFER_PIO_* 
and adev->pio_mode - XFER_PIO_0 yields you a mode number. Thus the second 
argument will always "win" as a minimal one.

MBR, Sergei

Re: [PATCH 2/6] pata_sis: Implement MWDMA for the UDMA 133 capable chips

[Bartlomiej Zolnierkiewicz]

On Monday 07 December 2009 02:26:00 pm Sergei Shtylyov wrote:
> Hello.
> 
> Alan Cox wrote:
> 
> > Bartlomiej pointed out that while this got fixed in the old driver whoever
> > did it didn't port it across.
> 
> > Signed-off-by: Alan Cox <alan@linux.intel.com>
> 
> [...]
> 
> > +static int mwdma_clip_to_pio(struct ata_device *adev)
> > +{
> > +	const int mwdma_to_pio[3] = {
> > +		XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
> > +	};
> > +	return min(mwdma_to_pio[adev->dma_mode - XFER_MW_DMA_0],
> > +				adev->pio_mode - XFER_PIO_0);
> > +}
> 
>     You call min() on uncomparables, i.e. mwdma_to_pio[] contains XFER_PIO_* 
> and adev->pio_mode - XFER_PIO_0 yields you a mode number. Thus the second 
> argument will always "win" as a minimal one.

There are more issues with the patch related to mwdma_clip_to_pio().

The function can return values between 0 and 4 which obviously won't work
well for the new code below for values > 2 (i.e. resulting in out-of-bounds
array access for the common-case of dev->pio_mode == 4).

@@ -400,21 +439,25 @@ static void sis_100_set_dmamode (struct ata_port *ap, struct ata_device *adev)
        struct pci_dev *pdev    = to_pci_dev(ap->host->dev);
        int speed = adev->dma_mode - XFER_MW_DMA_0;
        int drive_pci = sis_old_port_base(adev);
-       u8 timing;
+       u16 timing;
 
-       const u8 udma_bits[]  = { 0x8B, 0x87, 0x85, 0x83, 0x82, 0x81};
+       const u16 udma_bits[]  = {
+               0x8B00, 0x8700, 0x8500, 0x8300, 0x8200, 0x8100};
+       const u8 mwdma_bits[] = { 0x08, 0x32, 0x31 };
 
-       pci_read_config_byte(pdev, drive_pci + 1, &timing);
+       pci_read_config_word(pdev, drive_pci, &timing);
 
        if (adev->dma_mode < XFER_UDMA_0) {
-               /* NOT SUPPORTED YET: NEED DATA SHEET. DITTO IN OLD DRIVER */
+               speed = mwdma_clip_to_pio(adev);
+               timing &= ~0x80FF;
+               timing |= mwdma_bits[speed];
        } else {
                /* Bit 7 is UDMA on/off, bit 0-3 are cycle time */
                speed = adev->dma_mode - XFER_UDMA_0;
-               timing &= ~0x8F;
+               timing &= ~0x8F00;
                timing |= udma_bits[speed];
        }
-       pci_write_config_byte(pdev, drive_pci + 1, timing);
+       pci_write_config_word(pdev, drive_pci, timing);
 }
 
 /**
@@ -434,21 +477,26 @@ static void sis_133_early_set_dmamode (struct ata_port *ap, struct ata_device *a
        struct pci_dev *pdev    = to_pci_dev(ap->host->dev);
        int speed = adev->dma_mode - XFER_MW_DMA_0;
        int drive_pci = sis_old_port_base(adev);
-       u8 timing;
-       /* Low 4 bits are timing */
-       static const u8 udma_bits[]  = { 0x8F, 0x8A, 0x87, 0x85, 0x83, 0x82, 0x81};
+       u16 timing;
+       /* Bits 15-12  are timing */
+       static const u16 udma_bits[]  = {
+               0x8F00, 0x8A00, 0x8700, 0x8500, 0x8300, 0x8200, 0x8100
+       };
+       static const u8 mwdma_bits[] = { 0x08, 0x32, 0x31 };
 
-       pci_read_config_byte(pdev, drive_pci + 1, &timing);
+       pci_read_config_word(pdev, drive_pci, &timing);
 
        if (adev->dma_mode < XFER_UDMA_0) {
-               /* NOT SUPPORTED YET: NEED DATA SHEET. DITTO IN OLD DRIVER */
+               speed = mwdma_clip_to_pio(adev);
+               timing &= ~0x80FF;
+               timing = mwdma_bits[speed];
        } else {
                /* Bit 7 is UDMA on/off, bit 0-3 are cycle time */
                speed = adev->dma_mode - XFER_UDMA_0;
-               timing &= ~0x8F;
+               timing &= ~0x8F00;
                timing |= udma_bits[speed];
        }
-       pci_write_config_byte(pdev, drive_pci + 1, timing);
+       pci_write_config_word(pdev, drive_pci, timing);
 }
 
 /**


--
Bartlomiej Zolnierkiewicz

Re: [PATCH 2/6] pata_sis: Implement MWDMA for the UDMA 133 capable chips

[Bartlomiej Zolnierkiewicz]

On Monday 07 December 2009 04:05:28 pm Bartlomiej Zolnierkiewicz wrote:

> array access for the common-case of dev->pio_mode == 4).

s/4/XFER_PIO_4/

[ Looking at the buggy code is bad for your health. ]

Re: [PATCH 2/6] pata_sis: Implement MWDMA for the UDMA 133 capable chips

[Bartlomiej Zolnierkiewicz]

On Monday 07 December 2009 04:05:28 pm Bartlomiej Zolnierkiewicz wrote:
> On Monday 07 December 2009 02:26:00 pm Sergei Shtylyov wrote:
> > Hello.
> > 
> > Alan Cox wrote:
> > 
> > > Bartlomiej pointed out that while this got fixed in the old driver whoever
> > > did it didn't port it across.
> > 
> > > Signed-off-by: Alan Cox <alan@linux.intel.com>
> > 
> > [...]
> > 
> > > +static int mwdma_clip_to_pio(struct ata_device *adev)
> > > +{
> > > +	const int mwdma_to_pio[3] = {
> > > +		XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
> > > +	};
> > > +	return min(mwdma_to_pio[adev->dma_mode - XFER_MW_DMA_0],
> > > +				adev->pio_mode - XFER_PIO_0);
> > > +}
> > 
> >     You call min() on uncomparables, i.e. mwdma_to_pio[] contains XFER_PIO_* 
> > and adev->pio_mode - XFER_PIO_0 yields you a mode number. Thus the second 
> > argument will always "win" as a minimal one.
> 
> There are more issues with the patch related to mwdma_clip_to_pio().
> 
> The function can return values between 0 and 4 which obviously won't work
> well for the new code below for values > 2 (i.e. resulting in out-of-bounds

Fortunately above bugs won't be triggered because the patch forgot to update
MWDMA masks..  ;)

What is even more interesting is that the driver currently also lacks support
for MWDMA modes on UDMA66 chips (though it was implemented in the driver).

static const struct ata_port_info sis_info66 = {
	.flags		= ATA_FLAG_SLAVE_POSS,
	.pio_mask	= ATA_PIO4,
	/* No MWDMA */
	.udma_mask	= ATA_UDMA4,
	.port_ops	= &sis_66_ops,
};
static const struct ata_port_info sis_info100 = {
	.flags		= ATA_FLAG_SLAVE_POSS,
	.pio_mask	= ATA_PIO4,
	/* No MWDMA */
	.udma_mask	= ATA_UDMA5,
	.port_ops	= &sis_100_ops,
};
static const struct ata_port_info sis_info100_early = {
	.flags		= ATA_FLAG_SLAVE_POSS,
	.pio_mask	= ATA_PIO4,
	/* No MWDMA */
	.udma_mask	= ATA_UDMA5,
	.port_ops	= &sis_66_ops,
};
static const struct ata_port_info sis_info133 = {
	.flags		= ATA_FLAG_SLAVE_POSS,
	.pio_mask	= ATA_PIO4,
	/* No MWDMA */
	.udma_mask	= ATA_UDMA6,
	.port_ops	= &sis_133_ops,
};
const struct ata_port_info sis_info133_for_sata = {
	.flags		= ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST,
	.pio_mask	= ATA_PIO4,
	/* No MWDMA */
	.udma_mask	= ATA_UDMA6,
	.port_ops	= &sis_133_for_sata_ops,
};
static const struct ata_port_info sis_info133_early = {
	.flags		= ATA_FLAG_SLAVE_POSS,
	.pio_mask	= ATA_PIO4,
	/* No MWDMA */
	.udma_mask	= ATA_UDMA6,
	.port_ops	= &sis_133_early_ops,
};

--
Bartlomiej Zolnierkiewicz

Re: [PATCH 2/6] pata_sis: Implement MWDMA for the UDMA 133 capable chips

[Bartlomiej Zolnierkiewicz]

On Monday 07 December 2009 04:36:31 pm Bartlomiej Zolnierkiewicz wrote:
> On Monday 07 December 2009 04:05:28 pm Bartlomiej Zolnierkiewicz wrote:
> > On Monday 07 December 2009 02:26:00 pm Sergei Shtylyov wrote:
> > > Hello.
> > > 
> > > Alan Cox wrote:
> > > 
> > > > Bartlomiej pointed out that while this got fixed in the old driver whoever
> > > > did it didn't port it across.
> > > 
> > > > Signed-off-by: Alan Cox <alan@linux.intel.com>
> > > 
> > > [...]

One more thing:

@@ -479,13 +527,12 @@ static void sis_133_set_dmamode (struct ata_port *ap, struct ata_device *adev)
 	if (reg54 & 0x40000000)
 		port = 0x70;
 	port += (8 * ap->port_no) +  (4 * adev->devno);
-
 	pci_read_config_dword(pdev, port, &t1);
 
 	if (adev->dma_mode < XFER_UDMA_0) {
-		t1 &= ~0x00000004;
-		/* FIXME: need data sheet to add MWDMA here. Also lacking on
-		   ide/pci driver */
+		speed = mwdma_clip_to_pio(adev);
+		sis_133_do_piomode(ap, adev, speed);
+		t1 &= ~4;	/* UDMA off */
 	} else {
 		speed = adev->dma_mode - XFER_UDMA_0;
 		/* if & 8 no UDMA133 - need info for ... */

This way of doing things results in under-clocked timings for MWDMA0 mode,
(they're not the same as ones for PIO0 mode), old driver gets it correctly.

--
Bartlomiej Zolnierkiewicz

Re: [PATCH 2/6] pata_sis: Implement MWDMA for the UDMA 133 capable chips

[Bartlomiej Zolnierkiewicz]

On Monday 07 December 2009 04:05:28 pm Bartlomiej Zolnierkiewicz wrote:
> On Monday 07 December 2009 02:26:00 pm Sergei Shtylyov wrote:
> > Hello.
> > 
> > Alan Cox wrote:
> > 
> > > Bartlomiej pointed out that while this got fixed in the old driver whoever
> > > did it didn't port it across.
> > 
> > > Signed-off-by: Alan Cox <alan@linux.intel.com>
> > 
> > [...]
> > 
> > > +static int mwdma_clip_to_pio(struct ata_device *adev)
> > > +{
> > > +	const int mwdma_to_pio[3] = {
> > > +		XFER_PIO_0, XFER_PIO_3, XFER_PIO_4
> > > +	};
> > > +	return min(mwdma_to_pio[adev->dma_mode - XFER_MW_DMA_0],
> > > +				adev->pio_mode - XFER_PIO_0);
> > > +}
> > 
> >     You call min() on uncomparables, i.e. mwdma_to_pio[] contains XFER_PIO_* 
> > and adev->pio_mode - XFER_PIO_0 yields you a mode number. Thus the second 
> > argument will always "win" as a minimal one.
> 
> There are more issues with the patch related to mwdma_clip_to_pio().

Hmm.. why cannot ->qc_issue be used here (like in all other host drivers with
shared PIO/MWDMA timings) instead of some new 'clipping'?

->qc_issue is the standard way to do it, takes care of more corner cases
and would also allow for faster MWDMA0 timings. 

--
Bartlomiej Zolnierkiewicz

Re: [PATCH 2/6] pata_sis: Implement MWDMA for the UDMA 133 capable chips

[Alan Cox]

> Hmm.. why cannot ->qc_issue be used here (like in all other host drivers with
> shared PIO/MWDMA timings) instead of some new 'clipping'?

Agreed it would probably work providing the 8bit timings are not affected
by any of this.

> ->qc_issue is the standard way to do it, takes care of more corner cases
> and would also allow for faster MWDMA0 timings. 

All yours.. just revert the buggy patch and fix it. I won't have time to
look at it until next year and I freecycled all my SiS hardware some time
ago.

Alan

Re: [PATCH 2/6] pata_sis: Implement MWDMA for the UDMA 133 capable chips

[Bartlomiej Zolnierkiewicz]

On Monday 07 December 2009 06:53:14 pm Alan Cox wrote:
> > Hmm.. why cannot ->qc_issue be used here (like in all other host drivers with
> > shared PIO/MWDMA timings) instead of some new 'clipping'?
> 
> Agreed it would probably work providing the 8bit timings are not affected
> by any of this.

If 8bit timings are a problem ->bmdma_{start,stop} approach can be used
instead (as done for similar reason in it821x IIRC).

> > ->qc_issue is the standard way to do it, takes care of more corner cases
> > and would also allow for faster MWDMA0 timings. 
> 
> All yours.. just revert the buggy patch and fix it. I won't have time to
> look at it until next year and I freecycled all my SiS hardware some time
> ago.

Thank you but this is quite a bit of work and I'm not interested personally.

--
Bartlomiej Zolnierkiewicz