libata-dev: pdc2027x fixes

PATCH NAK Albert Lee 5 participants 16 messages Aug 4, 2005 – Aug 11, 2005

Subsystems (2)

SCSI libata

lore

Share & tools

lore

Albert Lee

author

2005-08-04 17:20 UTC #0 lore raw

cover libata-dev: pdc2027x fixes

Jeff,

pdc2027x driver fix:

1/3 pdc1.diff:
- Convert pdc2027x from PIO to MMIO.

2/3 pdc2.diff:
- fix the mdelay() problem for IBM power5 micro-partitioning

3/3 pdc3.diff:
- fix the ATAPI DMA irq lost problem (again)

(diff'ed against libata-dev tree, pdc2027x branch
b59bb64eca815cab0b21c9761d42c4c78f7ff467)
Tested ok on x86 and ppc64.

For your review, thanks.

Albert

Albert Lee

author

2005-08-04 17:35 UTC #1 lore raw

PATCH 1/3 libata-dev: Convert pdc2027x from PIO to MMIO

Jeff,

Patch 1/3: Convert pdc2027x from PIO to MMIO

Description:
Indexed registers need two PIO accesses: one access writes to the index register and the other access
reads/writes the indexed register. Using MMIO can access the register directly and simplify the code.

Changes:
- Use MMIO instead of PIO and indexed registers.
- Minor fix such as add ata_host_stop hook and indent beautification, etc.

For your review, thanks.

Albert

Signed-off-by Albert Lee <[email protected]>

drivers/scsi/pata_pdc2027x.c +155 ~~-124~~

@@ -29,7 +29,7 @@
 #include <asm/io.h>
 
 #define DRV_NAME	"pata_pdc2027x"
-#define DRV_VERSION	"0.60"
+#define DRV_VERSION	"0.70"
 #undef PDC_DEBUG
 
 #ifdef PDC_DEBUG

@@ -39,11 +39,19 @@
 #endif
 
 enum {
-	PDC_UDMA_100        = 0,
-	PDC_UDMA_133        = 1,
+	PDC_UDMA_100		= 0,
+	PDC_UDMA_133		= 1,
 	
-	PDC_100_MHZ         = 100000000,
-	PDC_133_MHZ         = 133333333,
+	PDC_100_MHZ		= 100000000,
+	PDC_133_MHZ		= 133333333,
+
+	PDC_SYS_CTL		= 0x1100,
+	PDC_ATA_CTL		= 0x1104,
+	PDC_GLOBAL_CTL		= 0x1108,
+	PDC_CTCR0		= 0x110C,
+	PDC_CTCR1		= 0x1110,
+	PDC_BYTE_COUNT		= 0x1120,
+	PDC_PLL_CTL		= 0x1202,
 };
 
 static int pdc2027x_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);

@@ -187,6 +195,7 @@
 
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
+	.host_stop		= ata_host_stop,
 };
 
 static struct ata_port_info pdc2027x_port_info[] = {

@@ -194,7 +203,7 @@
 	{
 		.sht		= &pdc2027x_sht,
 		.host_flags	= ATA_FLAG_NO_LEGACY | ATA_FLAG_SLAVE_POSS | 
-		                  ATA_FLAG_SRST,
+		                  ATA_FLAG_SRST | ATA_FLAG_MMIO,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA5, /* udma0-5 */

@@ -204,7 +213,7 @@
 	{
 		.sht		= &pdc2027x_sht,
 		.host_flags	= ATA_FLAG_NO_LEGACY | ATA_FLAG_SLAVE_POSS | 
-                        	  ATA_FLAG_SRST,
+                        	  ATA_FLAG_SRST | ATA_FLAG_MMIO,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6, /* udma0-6 */

@@ -218,31 +227,27 @@
 MODULE_DEVICE_TABLE(pci, pdc2027x_pci_tbl);
 
 /**
- * pdc_get_indexed_reg - Get pdc202xx extended register
- * @ap: Port to get the extended register
- * @index: index of the extended register
+ *	port_mmio - Get the MMIO address of PDC2027x extended registers
+ *	@ap: Port
+ *	@offset: offset from mmio base
  */
-static u8 pdc_get_indexed_reg(struct ata_port *ap, u8 index)
+static void* port_mmio(struct ata_port *ap, unsigned int offset)
 {
-	u8 tmp8;
-
-	outb(index, ap->ioaddr.bmdma_addr + 1); 
-	tmp8 = inb(ap->ioaddr.bmdma_addr + 3);
-
-	PDPRINTK("Get index reg%X[%X] \n", index, tmp8);
-	return tmp8;
+	return ap->host_set->mmio_base + ap->port_no * 0x100 + offset;
 }
+
 /**
- * pdc_set_indexed_reg - Set pdc202xx extended register
- * @ap: Port to set the extended register
- * @index: index of the extended register
+ *	dev_mmio - Get the MMIO address of PDC2027x extended registers
+ *	@ap: Port
+ *	@adev: device
+ *	@offset: offset from mmio base
  */
-static void pdc_set_indexed_reg(struct ata_port *ap, u8 index, u8 value)
+static void* dev_mmio(struct ata_port *ap, struct ata_device *adev, unsigned int offset)
 {
-	outb(index, ap->ioaddr.bmdma_addr + 1); 
-	outb(value, ap->ioaddr.bmdma_addr + 3);
-	PDPRINTK("Set index reg%X[%X] \n", index, value);
+	u8 adj = (adev->devno) ? 0x08 : 0x00;
+	return port_mmio(ap, offset) + adj;
 }
+
 /**
  *	pdc2027x_pata_cbl_detect - Probe host controller cable detect info
  *	@ap: Port for which cable detect info is desired

@@ -255,12 +260,11 @@
  */
 static void pdc2027x_cbl_detect(struct ata_port *ap)
 {
-	u8 cbl40c;
+	u32 cgcr;
 
 	/* check cable detect results */
-	cbl40c = pdc_get_indexed_reg(ap, 0x0b) & 0x04;
-
-	if (cbl40c)
+	cgcr = readl(port_mmio(ap, PDC_GLOBAL_CTL));
+	if (cgcr & (1 << 26))
 		goto cbl40;
 
 	PDPRINTK("No cable or 80-conductor cable on port %d\n", ap->port_no);

@@ -273,14 +277,16 @@
 	ap->cbl = ATA_CBL_PATA40;
 	ap->udma_mask &= ATA_UDMA_MASK_40C;
 }
+
 /**
- * pdc2027x_port_enabled - Check extended register at 0x04 to see whether the port is enabled.
+ * pdc2027x_port_enabled - Check PDC ATA control register to see whether the port is enabled.
  * @ap: Port to check
  */
 static inline int pdc2027x_port_enabled(struct ata_port *ap)
 {
-	return pdc_get_indexed_reg(ap, 0x04) & 0x02;
+	return readb(port_mmio(ap, PDC_ATA_CTL)) & 0x02;
 }
+
 /**
  *	pdc2027x_phy_reset - Probe specified port on PATA host controller
  *	@ap: Port to probe

@@ -303,6 +309,7 @@
 	ata_port_probe(ap);
 	ata_bus_reset(ap);
 }
+
 /**
  *	pdc2027x_set_piomode - Initialize host controller PATA PIO timings
  *	@ap: Port to configure

@@ -316,9 +323,8 @@
  */
 static void pdc2027x_set_piomode(struct ata_port *ap, struct ata_device *adev)
 {
-	unsigned int pio      = adev->pio_mode - XFER_PIO_0;
-	unsigned int drive_dn = (ap->port_no ? 2 : 0) + adev->devno;
-	u8 adj		      = (drive_dn%2) ? 0x08 : 0x00;
+	unsigned int pio = adev->pio_mode - XFER_PIO_0;
+	u32 ctcr0, ctcr1;
 
 	PDPRINTK("adev->pio_mode[%X]\n", adev->pio_mode);

@@ -332,14 +338,22 @@
 	/* Set the PIO timing registers using value table for 133MHz */
 	PDPRINTK("Set pio regs... \n");
 
-	pdc_set_indexed_reg(ap, 0x0c + adj, pdc2027x_pio_timing_tbl[pio].value0);
-	pdc_set_indexed_reg(ap, 0x0d + adj, pdc2027x_pio_timing_tbl[pio].value1);
-	pdc_set_indexed_reg(ap, 0x13 + adj, pdc2027x_pio_timing_tbl[pio].value2);
+	ctcr0 = readl(dev_mmio(ap, adev, PDC_CTCR0));
+	ctcr0 &= 0xffff0000;
+	ctcr0 |= pdc2027x_pio_timing_tbl[pio].value0 |
+		(pdc2027x_pio_timing_tbl[pio].value1 << 8);
+	writel(ctcr0, dev_mmio(ap, adev, PDC_CTCR0));
+
+	ctcr1 = readl(dev_mmio(ap, adev, PDC_CTCR1));
+	ctcr1 &= 0x00ffffff;
+	ctcr1 |= (pdc2027x_pio_timing_tbl[pio].value2 << 24);
+	writel(ctcr1, dev_mmio(ap, adev, PDC_CTCR1));
 
 	PDPRINTK("Set pio regs done\n");
 
 	PDPRINTK("Set to pio mode[%u] \n", pio);
 }
+
 /**
  *	pdc2027x_set_dmamode - Initialize host controller PATA UDMA timings
  *	@ap: Port to configure

@@ -353,12 +367,10 @@
  */
 static void pdc2027x_set_dmamode(struct ata_port *ap, struct ata_device *adev)
 {
-	unsigned int dma_mode	= adev->dma_mode;
-	unsigned int drive_dn	= (ap->port_no ? 2 : 0) + adev->devno;
-	u8 adj			= (drive_dn%2) ? 0x08 : 0x00;
-	u8 tmp8;
+	unsigned int dma_mode = adev->dma_mode;
+	u32 ctcr0, ctcr1;
 
-	if ((dma_mode >= XFER_UDMA_0) && 
+	if ((dma_mode >= XFER_UDMA_0) &&
 	   (dma_mode <= XFER_UDMA_6)) {
 		/* Set the UDMA timing registers with value table for 133MHz */
 		unsigned int udma_mode = dma_mode & 0x07;

@@ -369,14 +381,19 @@
 			 * If tHOLD is '1', the hardware will add half clock for data hold time.
 			 * This code segment seems to be no effect. tHOLD will be overwritten below.
 			 */
-			tmp8 = pdc_get_indexed_reg(ap, 0x10 + adj);
-			pdc_set_indexed_reg(ap, 0x10 + adj, tmp8 & 0x7f);
+			ctcr1 = readl(dev_mmio(ap, adev, PDC_CTCR1));
+			writel(ctcr1 & ~(1 << 7), dev_mmio(ap, adev, PDC_CTCR1));
 		} 
 
 		PDPRINTK("Set udma regs... \n");
-		pdc_set_indexed_reg(ap, 0x10 + adj, pdc2027x_udma_timing_tbl[udma_mode].value0);
-		pdc_set_indexed_reg(ap, 0x11 + adj, pdc2027x_udma_timing_tbl[udma_mode].value1);
-		pdc_set_indexed_reg(ap, 0x12 + adj, pdc2027x_udma_timing_tbl[udma_mode].value2);
+
+		ctcr1 = readl(dev_mmio(ap, adev, PDC_CTCR1));
+		ctcr1 &= 0xff000000;
+		ctcr1 |= pdc2027x_udma_timing_tbl[udma_mode].value0 |
+			(pdc2027x_udma_timing_tbl[udma_mode].value1 << 8) |
+			(pdc2027x_udma_timing_tbl[udma_mode].value2 << 16);
+		writel(ctcr1, dev_mmio(ap, adev, PDC_CTCR1));
+		
 		PDPRINTK("Set udma regs done\n");
 		
 		PDPRINTK("Set to udma mode[%u] \n", udma_mode);

@@ -387,8 +404,13 @@
 		unsigned int mdma_mode = dma_mode & 0x07;
 
 		PDPRINTK("Set mdma regs... \n");
-		pdc_set_indexed_reg(ap, 0x0e + adj, pdc2027x_mdma_timing_tbl[mdma_mode].value0);
-		pdc_set_indexed_reg(ap, 0x0f + adj, pdc2027x_mdma_timing_tbl[mdma_mode].value1);
+		ctcr0 = readl(dev_mmio(ap, adev, PDC_CTCR0));
+
+		ctcr0 &= 0x0000ffff;
+		ctcr0 |= (pdc2027x_mdma_timing_tbl[mdma_mode].value0 << 16) |
+			(pdc2027x_mdma_timing_tbl[mdma_mode].value1 << 24);
+		
+		writel(ctcr0, dev_mmio(ap, adev, PDC_CTCR0));
 		PDPRINTK("Set mdma regs done\n");
 		
 		PDPRINTK("Set to mdma mode[%u] \n", mdma_mode);

@@ -413,18 +435,17 @@
 		struct ata_device *dev = &ap->device[i];
 
 		if (ata_dev_present(dev)) {
-			u8 adj = (i % 2) ? 0x08 : 0x00;
-			u8 tmp8;
 			
 			pdc2027x_set_piomode(ap, dev);
 
 			/*
 			 * Enable prefetch if the device support PIO only.
 			 */
-			if (dev->xfer_shift == ATA_SHIFT_PIO) { 
-				tmp8 = pdc_get_indexed_reg(ap, 0x13 + adj);
-				pdc_set_indexed_reg(ap, 0x13 + adj, tmp8 | 0x02);
-				
+			if (dev->xfer_shift == ATA_SHIFT_PIO) {
+				u32 ctcr1 = readl(dev_mmio(ap, dev, PDC_CTCR1));
+				ctcr1 |= (1 << 25);
+				writel(ctcr1, dev_mmio(ap, dev, PDC_CTCR1));
+
 				PDPRINTK("Turn on prefetch\n");
 			} else {
 				pdc2027x_set_dmamode(ap, dev);

@@ -462,43 +483,31 @@
 
 static unsigned long pdc_read_counter(struct ata_probe_ent *probe_ent)
 {
-	u8 ctr0, ctr1, ctr2, ctr3;
 	unsigned long counter;
-	u8 ctr0v, ctr1v, ctr2v, ctr3v;
 	int retry = 1;
+	u32 bccrl, bccrh, bccrlv, bccrhv;
 
 retry:
-	outb(0x20, probe_ent->port[0].bmdma_addr + 0x01);
-	ctr0 = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	outb(0x21, probe_ent->port[0].bmdma_addr + 0x01);
-	ctr1 = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	outb(0x20, probe_ent->port[1].bmdma_addr + 0x01);
-	ctr2 = inb(probe_ent->port[1].bmdma_addr + 0x03);
-	outb(0x21, probe_ent->port[1].bmdma_addr + 0x01);
-	ctr3 = inb(probe_ent->port[1].bmdma_addr + 0x03);
+	bccrl = readl(probe_ent->mmio_base + PDC_BYTE_COUNT) & 0xffff;
+	bccrh = readl(probe_ent->mmio_base + PDC_BYTE_COUNT + 0x100) & 0xffff;
+	rmb();
 
 	/* Read the counter values again for verification */
-	outb(0x20,  probe_ent->port[0].bmdma_addr + 0x01);
-	ctr0v = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	outb(0x21,  probe_ent->port[0].bmdma_addr + 0x01);
-	ctr1v = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	outb(0x20,  probe_ent->port[1].bmdma_addr + 0x01);
-	ctr2v = inb(probe_ent->port[1].bmdma_addr + 0x03);
-	outb(0x21,  probe_ent->port[1].bmdma_addr + 0x01);
-	ctr3v = inb(probe_ent->port[1].bmdma_addr + 0x03);
-
-	counter = (ctr3 << 23) | (ctr2 << 15) | (ctr1 << 8) | ctr0;
-
-	PDPRINTK("ctr0[%X] ctr1[%X] ctr2[%X] ctr3[%X]\n", 
-		 ctr0, ctr1, ctr2, ctr3);
-	PDPRINTK("ctr0[%X] ctr1[%X] ctr2[%X] ctr3[%X]\n", 
-		 ctr0v, ctr1v, ctr2v, ctr3v);
+	bccrlv = readl(probe_ent->mmio_base + PDC_BYTE_COUNT) & 0xffff;
+	bccrhv = readl(probe_ent->mmio_base + PDC_BYTE_COUNT + 0x100) & 0xffff;
+	rmb();
+
+	counter = (bccrh << 15) | bccrl;
+
+	PDPRINTK("bccrh [%X] bccrl [%X]\n", bccrh,  bccrl);
+	PDPRINTK("bccrhv[%X] bccrlv[%X]\n", bccrhv, bccrlv);
 
 	/*
-	 * The 30-bit decreasing counter are read by 4 pieces.
-	 * Incorrect value may be read when ctr2 and ctr3 are changing.
+	 * The 30-bit decreasing counter are read by 2 pieces.
+	 * Incorrect value may be read when both bccrh and bccrl are changing.
+	 * Ex. When 7900 decrease to 78FF, wrong value 7800 might be read.
 	 */
-	if (retry && !(ctr3 == ctr3v && ctr2 == ctr2v && ctr1 >= ctr1v)) {
+	if (retry && !(bccrh == bccrhv && bccrl >= bccrlv)) {
 		retry--;
 		PDPRINTK("rereading counter\n");
 		goto retry;

@@ -517,12 +526,11 @@
 static void pdc_adjust_pll(struct ata_probe_ent *probe_ent, long pll_clock, unsigned int board_idx) 
 {
 
-	u8 pll_ctl0, pll_ctl1;
+	u16 pll_ctl;
 	long pll_clock_khz = pll_clock / 1000;
 	long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
 	long ratio = pout_required / pll_clock_khz;
 	int F, R;
-	
 
 	/* Sanity check */
 	if (unlikely(pll_clock_khz < 5000L || pll_clock_khz > 70000L)) {

@@ -536,12 +544,9 @@
 	/* Show the current clock value of PLL control register
 	 * (maybe already configured by the firmware)
 	 */
-	outb(0x02, probe_ent->port[1].bmdma_addr + 0x01);
-	pll_ctl0 = inb(probe_ent->port[1].bmdma_addr + 0x03);
-	outb(0x03, probe_ent->port[1].bmdma_addr + 0x01);
-	pll_ctl1 = inb(probe_ent->port[1].bmdma_addr + 0x03);
+	pll_ctl = readw(probe_ent->mmio_base + PDC_PLL_CTL);
 
-	PDPRINTK("pll_ctl[%X][%X]\n", pll_ctl0, pll_ctl1);
+	PDPRINTK("pll_ctl[%X]\n", pll_ctl);
 #endif
 
 	/*

@@ -575,15 +580,12 @@
 
 	PDPRINTK("F[%d] R[%d] ratio*1000[%ld]\n", F, R, ratio);
 
-	pll_ctl0 = (u8) F;
-	pll_ctl1 = (u8) R;
+	pll_ctl = (R << 8) | F;
 
-	PDPRINTK("Writing pll_ctl[%X][%X]\n", pll_ctl0, pll_ctl1);
+	PDPRINTK("Writing pll_ctl[%X]\n", pll_ctl);
 
-	outb(0x02,     probe_ent->port[1].bmdma_addr + 0x01);
-	outb(pll_ctl0, probe_ent->port[1].bmdma_addr + 0x03);
-	outb(0x03,     probe_ent->port[1].bmdma_addr + 0x01);
-	outb(pll_ctl1, probe_ent->port[1].bmdma_addr + 0x03);
+	writew(pll_ctl, probe_ent->mmio_base + PDC_PLL_CTL);
+	wmb();
 
 	/* Wait the PLL circuit to be stable */
 	mdelay(30);

@@ -593,12 +595,9 @@
 	 *  Show the current clock value of PLL control register
 	 * (maybe configured by the firmware)
 	 */
-	outb(0x02, probe_ent->port[1].bmdma_addr + 0x01);
-	pll_ctl0 = inb(probe_ent->port[1].bmdma_addr + 0x03);
-	outb(0x03, probe_ent->port[1].bmdma_addr + 0x01);
-	pll_ctl1 = inb(probe_ent->port[1].bmdma_addr + 0x03);
+	pll_ctl = readw(probe_ent->mmio_base + PDC_PLL_CTL);
 
-	PDPRINTK("pll_ctl[%X][%X]\n", pll_ctl0, pll_ctl1);
+	PDPRINTK("pll_ctl[%X]\n", pll_ctl);
 #endif
 
 	return;

@@ -612,7 +611,7 @@
  */
 static long pdc_detect_pll_input_clock(struct ata_probe_ent *probe_ent) 
 {
-	u8 scr1;
+	u32 scr;
 	unsigned long start_count, end_count;
 	long pll_clock;

@@ -620,10 +619,10 @@
 	start_count = pdc_read_counter(probe_ent);
 
 	/* Start the test mode */
-	outb(0x01, probe_ent->port[0].bmdma_addr + 0x01);
-	scr1 = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	PDPRINTK("scr1[%X]\n", scr1);
-	outb(scr1 | 0x40, probe_ent->port[0].bmdma_addr + 0x03);
+	scr = readl(probe_ent->mmio_base + PDC_SYS_CTL);
+	PDPRINTK("scr[%X]\n", scr);
+	writel(scr | (0x01 << 14), probe_ent->mmio_base + PDC_SYS_CTL);
+	wmb();
 
 	/* Let the counter run for 100 ms. */
 	mdelay(100);

@@ -632,10 +631,10 @@
 	end_count = pdc_read_counter(probe_ent);
 
 	/* Stop the test mode */
-	outb(0x01, probe_ent->port[0].bmdma_addr + 0x01);
-	scr1 = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	PDPRINTK("scr1[%X]\n", scr1);
-	outb(scr1 & 0xBF, probe_ent->port[0].bmdma_addr + 0x03);
+	scr = readl(probe_ent->mmio_base + PDC_SYS_CTL);
+	PDPRINTK("scr[%X]\n", scr);
+	writel(scr & ~(0x01 << 14), probe_ent->mmio_base + PDC_SYS_CTL);
+	wmb();
 
 	/* calculate the input clock in Hz */
 	pll_clock = (long) ((start_count - end_count) * 10);

@@ -645,6 +644,7 @@
 
 	return pll_clock;
 }
+
 /**
  * pdc_hardware_init - Initialize the hardware.
  * @pdev: instance of pci_dev found

@@ -673,6 +673,29 @@
 
 	return 0;
 }
+
+/**
+ * pdc_ata_setup_port - setup the mmio address
+ * @port: ata ioports to setup
+ * @base: base address
+ */
+static void pdc_ata_setup_port(struct ata_ioports *port, unsigned long base)
+{
+	port->cmd_addr		=
+	port->data_addr		= base;
+	port->feature_addr	=
+	port->error_addr	= base + 0x05;
+	port->nsect_addr	= base + 0x0a;
+	port->lbal_addr		= base + 0x0f;
+	port->lbam_addr		= base + 0x10;
+	port->lbah_addr		= base + 0x15;
+	port->device_addr	= base + 0x1a;
+	port->command_addr	=
+	port->status_addr	= base + 0x1f;
+	port->altstatus_addr	=
+	port->ctl_addr		= base + 0x81a;
+}
+
 /**
  * pdc2027x_init_one - PCI probe function
  * Called when an instance of PCI adapter is inserted.

@@ -690,6 +713,8 @@
 	unsigned int board_idx = (unsigned int) ent->driver_data;
 
 	struct ata_probe_ent *probe_ent = NULL;
+	unsigned long base;
+	void *mmio_base;
 	int rc;
 
 	if (!printed_version++)

@@ -722,28 +747,31 @@
 	probe_ent->dev = pci_dev_to_dev(pdev);
 	INIT_LIST_HEAD(&probe_ent->node);
 
+	mmio_base = ioremap(pci_resource_start(pdev, 5),
+		            pci_resource_len(pdev, 5));
+
+	if (mmio_base == NULL) {
+		rc = -ENOMEM;
+		goto err_out_free_ent;
+	}
+
+	base = (unsigned long) mmio_base;
+
 	probe_ent->sht		= pdc2027x_port_info[board_idx].sht;
 	probe_ent->host_flags	= pdc2027x_port_info[board_idx].host_flags;
 	probe_ent->pio_mask	= pdc2027x_port_info[board_idx].pio_mask;
+	probe_ent->mwdma_mask	= pdc2027x_port_info[board_idx].mwdma_mask;
 	probe_ent->udma_mask	= pdc2027x_port_info[board_idx].udma_mask;
 	probe_ent->port_ops	= pdc2027x_port_info[board_idx].port_ops;
 
        	probe_ent->irq = pdev->irq;
        	probe_ent->irq_flags = SA_SHIRQ;
+	probe_ent->mmio_base = mmio_base;
 
-	probe_ent->port[0].cmd_addr = pci_resource_start(pdev, 0);
-	ata_std_ports(&probe_ent->port[0]);
-	probe_ent->port[0].altstatus_addr =
-		probe_ent->port[0].ctl_addr =
-		pci_resource_start(pdev, 1) | ATA_PCI_CTL_OFS;
-	probe_ent->port[0].bmdma_addr = pci_resource_start(pdev, 4);
-
-	probe_ent->port[1].cmd_addr = pci_resource_start(pdev, 2);
-	ata_std_ports(&probe_ent->port[1]);
-	probe_ent->port[1].altstatus_addr =
-		probe_ent->port[1].ctl_addr =
-		pci_resource_start(pdev, 3) | ATA_PCI_CTL_OFS;
-	probe_ent->port[1].bmdma_addr = pci_resource_start(pdev, 4) + 8;
+	pdc_ata_setup_port(&probe_ent->port[0], base + 0x17c0);
+	probe_ent->port[0].bmdma_addr = base + 0x1000;
+	pdc_ata_setup_port(&probe_ent->port[1], base + 0x15c0);
+	probe_ent->port[1].bmdma_addr = base + 0x1008;
 
 	probe_ent->n_ports = 2;

@@ -767,6 +795,7 @@
 	pci_disable_device(pdev);
 	return rc;
 }
+
 /**
  * pdc2027x_remove_one - Called to remove a single instance of the
  * adapter.

@@ -778,6 +807,7 @@
 {
 	ata_pci_remove_one(pdev);
 }
+
 /**
  * pdc2027x_init - Called after this module is loaded into the kernel.
  */

@@ -785,6 +815,7 @@
 {
 	return pci_module_init(&pdc2027x_pci_driver);
 }
+
 /**
  * pdc2027x_exit - Called before this module unloaded from the kernel
  */

Brett Russ

2005-08-04 19:39 UTC #2 lore raw

Re: [PATCH 1/3] libata-dev: Convert pdc2027x from PIO to MMIO

drivers/scsi/pata_pdc2027x.c line 266

Quoted 1 line · Albert Lee

drivers/scsi/pata_pdc2027x.c +1 -0
+	cgcr = readl(port_mmio(ap, PDC_GLOBAL_CTL));

I've read that read[bwl]() and write[bwl]() are deprecated and discouraged in new code and instead ioread{8,16,32}() and iowrite{8,16,32}() should be used. Is this the case?

Albert Lee

author

2005-08-09 11:05 UTC #3 lore raw

Re: [PATCH 1/3] libata-dev: Convert pdc2027x from PIO to MMIO

Hi,

Quoted 8 lines

Quoted 3 lines
+ cgcr = readl(port_mmio(ap, PDC_GLOBAL_CTL));
I've read that read[bwl]() and write[bwl]() are deprecated and discouraged in new code and instead ioread{8,16,32}() and iowrite{8,16,32}() should be used. Is this the case?
BR

I've read Linux Device Drivers 3rd Edition, it does say readx() and writex()
are deprecated. However, other libata drivers use readx() and writex(); and io_read_x() and io_write_x() are not available on older kernel, say, 2.6.5.

So, I'm not sure. Maybe later Jeff will need all libata drivers to use new io_read_x() and io_write_x() API?

Albert

Jeff Garzik

2005-08-11 02:37 UTC #4 lore raw

Re: [PATCH 1/3] libata-dev: Convert pdc2027x from PIO to MMIO

In reply to Brett Russ

Quoted 6 lines · Brett Russ

Quoted 1 line · Albert Lee
+ cgcr = readl(port_mmio(ap, PDC_GLOBAL_CTL));
I've read that read[bwl]() and write[bwl]() are deprecated and discouraged in new code and instead ioread{8,16,32}() and iowrite{8,16,32}() should be used. Is this the case?

ioread/write should only be used when the code in question may have to use either readl() or inl().

When a driver is written for hardware that is -always- used in MMIO mode, then readl() is preferred, since its faster.

Jeff

Jeff Garzik

2005-08-11 03:03 UTC #5 lore raw

Re: [PATCH 1/3] libata-dev: Convert pdc2027x from PIO to MMIO

In reply to Albert Lee

Quoted 9 lines · Albert Lee

Jeff,
Patch 1/3: Convert pdc2027x from PIO to MMIO
Description:
Indexed registers need two PIO accesses: one access writes to the
index register and the other access
reads/writes the indexed register. Using MMIO can access the register directly and simplify the code.

Quoted 4 lines

Changes:
- Use MMIO instead of PIO and indexed registers.
- Minor fix such as add ata_host_stop hook and indent beautification,
etc.

Comments:

1) port_mmio() and dev_mmio() should be static inline

2) The -only- valid way to flush a write to the device across the PCI bus is with a corresponding read[bwl](). wmb() is not sufficient.

Jeff

Albert Lee

author

2005-08-11 09:50 UTC #6 lore raw

Re: [PATCH 1/3] libata-dev: Convert pdc2027x from PIO to MMIO

Jeff,

Quoted 17 lines

Quoted 9 lines · Albert Lee
Jeff,
Patch 1/3: Convert pdc2027x from PIO to MMIO
Description:
Indexed registers need two PIO accesses: one access writes to the
index register and the other access
reads/writes the indexed register. Using MMIO can access the register directly and simplify the code.
Quoted 4 lines
Changes:
- Use MMIO instead of PIO and indexed registers.
- Minor fix such as add ata_host_stop hook and indent
beautification, etc.

Quoted 3 lines

Comments:
1) port_mmio() and dev_mmio() should be static inline

Fixed.

Quoted 3 lines

2) The -only- valid way to flush a write to the device across the PCI bus is with a corresponding read[bwl](). wmb() is not sufficient.

Fixed to use readl() instead of wmb(). Thanks for the advice. Attached please find the fixed patch for your review.

Albert

Signed-off-by Albert Lee <[email protected]>

drivers/scsi/pata_pdc2027x.c +155 ~~-124~~

@@ -29,7 +29,7 @@
 #include <asm/io.h>
 
 #define DRV_NAME	"pata_pdc2027x"
-#define DRV_VERSION	"0.60"
+#define DRV_VERSION	"0.70"
 #undef PDC_DEBUG
 
 #ifdef PDC_DEBUG

@@ -39,11 +39,19 @@
 #endif
 
 enum {
-	PDC_UDMA_100        = 0,
-	PDC_UDMA_133        = 1,
+	PDC_UDMA_100		= 0,
+	PDC_UDMA_133		= 1,
 	
-	PDC_100_MHZ         = 100000000,
-	PDC_133_MHZ         = 133333333,
+	PDC_100_MHZ		= 100000000,
+	PDC_133_MHZ		= 133333333,
+
+	PDC_SYS_CTL		= 0x1100,
+	PDC_ATA_CTL		= 0x1104,
+	PDC_GLOBAL_CTL		= 0x1108,
+	PDC_CTCR0		= 0x110C,
+	PDC_CTCR1		= 0x1110,
+	PDC_BYTE_COUNT		= 0x1120,
+	PDC_PLL_CTL		= 0x1202,
 };
 
 static int pdc2027x_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);

@@ -187,6 +195,7 @@
 
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
+	.host_stop		= ata_host_stop,
 };
 
 static struct ata_port_info pdc2027x_port_info[] = {

@@ -194,7 +203,7 @@
 	{
 		.sht		= &pdc2027x_sht,
 		.host_flags	= ATA_FLAG_NO_LEGACY | ATA_FLAG_SLAVE_POSS | 
-		                  ATA_FLAG_SRST,
+		                  ATA_FLAG_SRST | ATA_FLAG_MMIO,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA5, /* udma0-5 */

@@ -204,7 +213,7 @@
 	{
 		.sht		= &pdc2027x_sht,
 		.host_flags	= ATA_FLAG_NO_LEGACY | ATA_FLAG_SLAVE_POSS | 
-                        	  ATA_FLAG_SRST,
+                        	  ATA_FLAG_SRST | ATA_FLAG_MMIO,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6, /* udma0-6 */

@@ -218,31 +227,27 @@
 MODULE_DEVICE_TABLE(pci, pdc2027x_pci_tbl);
 
 /**
- * pdc_get_indexed_reg - Get pdc202xx extended register
- * @ap: Port to get the extended register
- * @index: index of the extended register
+ *	port_mmio - Get the MMIO address of PDC2027x extended registers
+ *	@ap: Port
+ *	@offset: offset from mmio base
  */
-static u8 pdc_get_indexed_reg(struct ata_port *ap, u8 index)
+static inline void* port_mmio(struct ata_port *ap, unsigned int offset)
 {
-	u8 tmp8;
-
-	outb(index, ap->ioaddr.bmdma_addr + 1); 
-	tmp8 = inb(ap->ioaddr.bmdma_addr + 3);
-
-	PDPRINTK("Get index reg%X[%X] \n", index, tmp8);
-	return tmp8;
+	return ap->host_set->mmio_base + ap->port_no * 0x100 + offset;
 }
+
 /**
- * pdc_set_indexed_reg - Set pdc202xx extended register
- * @ap: Port to set the extended register
- * @index: index of the extended register
+ *	dev_mmio - Get the MMIO address of PDC2027x extended registers
+ *	@ap: Port
+ *	@adev: device
+ *	@offset: offset from mmio base
  */
-static void pdc_set_indexed_reg(struct ata_port *ap, u8 index, u8 value)
+static inline void* dev_mmio(struct ata_port *ap, struct ata_device *adev, unsigned int offset)
 {
-	outb(index, ap->ioaddr.bmdma_addr + 1); 
-	outb(value, ap->ioaddr.bmdma_addr + 3);
-	PDPRINTK("Set index reg%X[%X] \n", index, value);
+	u8 adj = (adev->devno) ? 0x08 : 0x00;
+	return port_mmio(ap, offset) + adj;
 }
+
 /**
  *	pdc2027x_pata_cbl_detect - Probe host controller cable detect info
  *	@ap: Port for which cable detect info is desired

@@ -255,12 +260,11 @@
  */
 static void pdc2027x_cbl_detect(struct ata_port *ap)
 {
-	u8 cbl40c;
+	u32 cgcr;
 
 	/* check cable detect results */
-	cbl40c = pdc_get_indexed_reg(ap, 0x0b) & 0x04;
-
-	if (cbl40c)
+	cgcr = readl(port_mmio(ap, PDC_GLOBAL_CTL));
+	if (cgcr & (1 << 26))
 		goto cbl40;
 
 	PDPRINTK("No cable or 80-conductor cable on port %d\n", ap->port_no);

@@ -273,14 +277,16 @@
 	ap->cbl = ATA_CBL_PATA40;
 	ap->udma_mask &= ATA_UDMA_MASK_40C;
 }
+
 /**
- * pdc2027x_port_enabled - Check extended register at 0x04 to see whether the port is enabled.
+ * pdc2027x_port_enabled - Check PDC ATA control register to see whether the port is enabled.
  * @ap: Port to check
  */
 static inline int pdc2027x_port_enabled(struct ata_port *ap)
 {
-	return pdc_get_indexed_reg(ap, 0x04) & 0x02;
+	return readb(port_mmio(ap, PDC_ATA_CTL)) & 0x02;
 }
+
 /**
  *	pdc2027x_phy_reset - Probe specified port on PATA host controller
  *	@ap: Port to probe

@@ -303,6 +309,7 @@
 	ata_port_probe(ap);
 	ata_bus_reset(ap);
 }
+
 /**
  *	pdc2027x_set_piomode - Initialize host controller PATA PIO timings
  *	@ap: Port to configure

@@ -316,9 +323,8 @@
  */
 static void pdc2027x_set_piomode(struct ata_port *ap, struct ata_device *adev)
 {
-	unsigned int pio      = adev->pio_mode - XFER_PIO_0;
-	unsigned int drive_dn = (ap->port_no ? 2 : 0) + adev->devno;
-	u8 adj		      = (drive_dn%2) ? 0x08 : 0x00;
+	unsigned int pio = adev->pio_mode - XFER_PIO_0;
+	u32 ctcr0, ctcr1;
 
 	PDPRINTK("adev->pio_mode[%X]\n", adev->pio_mode);

@@ -332,14 +338,22 @@
 	/* Set the PIO timing registers using value table for 133MHz */
 	PDPRINTK("Set pio regs... \n");
 
-	pdc_set_indexed_reg(ap, 0x0c + adj, pdc2027x_pio_timing_tbl[pio].value0);
-	pdc_set_indexed_reg(ap, 0x0d + adj, pdc2027x_pio_timing_tbl[pio].value1);
-	pdc_set_indexed_reg(ap, 0x13 + adj, pdc2027x_pio_timing_tbl[pio].value2);
+	ctcr0 = readl(dev_mmio(ap, adev, PDC_CTCR0));
+	ctcr0 &= 0xffff0000;
+	ctcr0 |= pdc2027x_pio_timing_tbl[pio].value0 |
+		(pdc2027x_pio_timing_tbl[pio].value1 << 8);
+	writel(ctcr0, dev_mmio(ap, adev, PDC_CTCR0));
+
+	ctcr1 = readl(dev_mmio(ap, adev, PDC_CTCR1));
+	ctcr1 &= 0x00ffffff;
+	ctcr1 |= (pdc2027x_pio_timing_tbl[pio].value2 << 24);
+	writel(ctcr1, dev_mmio(ap, adev, PDC_CTCR1));
 
 	PDPRINTK("Set pio regs done\n");
 
 	PDPRINTK("Set to pio mode[%u] \n", pio);
 }
+
 /**
  *	pdc2027x_set_dmamode - Initialize host controller PATA UDMA timings
  *	@ap: Port to configure

@@ -353,12 +367,10 @@
  */
 static void pdc2027x_set_dmamode(struct ata_port *ap, struct ata_device *adev)
 {
-	unsigned int dma_mode	= adev->dma_mode;
-	unsigned int drive_dn	= (ap->port_no ? 2 : 0) + adev->devno;
-	u8 adj			= (drive_dn%2) ? 0x08 : 0x00;
-	u8 tmp8;
+	unsigned int dma_mode = adev->dma_mode;
+	u32 ctcr0, ctcr1;
 
-	if ((dma_mode >= XFER_UDMA_0) && 
+	if ((dma_mode >= XFER_UDMA_0) &&
 	   (dma_mode <= XFER_UDMA_6)) {
 		/* Set the UDMA timing registers with value table for 133MHz */
 		unsigned int udma_mode = dma_mode & 0x07;

@@ -369,14 +381,19 @@
 			 * If tHOLD is '1', the hardware will add half clock for data hold time.
 			 * This code segment seems to be no effect. tHOLD will be overwritten below.
 			 */
-			tmp8 = pdc_get_indexed_reg(ap, 0x10 + adj);
-			pdc_set_indexed_reg(ap, 0x10 + adj, tmp8 & 0x7f);
+			ctcr1 = readl(dev_mmio(ap, adev, PDC_CTCR1));
+			writel(ctcr1 & ~(1 << 7), dev_mmio(ap, adev, PDC_CTCR1));
 		} 
 
 		PDPRINTK("Set udma regs... \n");
-		pdc_set_indexed_reg(ap, 0x10 + adj, pdc2027x_udma_timing_tbl[udma_mode].value0);
-		pdc_set_indexed_reg(ap, 0x11 + adj, pdc2027x_udma_timing_tbl[udma_mode].value1);
-		pdc_set_indexed_reg(ap, 0x12 + adj, pdc2027x_udma_timing_tbl[udma_mode].value2);
+
+		ctcr1 = readl(dev_mmio(ap, adev, PDC_CTCR1));
+		ctcr1 &= 0xff000000;
+		ctcr1 |= pdc2027x_udma_timing_tbl[udma_mode].value0 |
+			(pdc2027x_udma_timing_tbl[udma_mode].value1 << 8) |
+			(pdc2027x_udma_timing_tbl[udma_mode].value2 << 16);
+		writel(ctcr1, dev_mmio(ap, adev, PDC_CTCR1));
+		
 		PDPRINTK("Set udma regs done\n");
 		
 		PDPRINTK("Set to udma mode[%u] \n", udma_mode);

@@ -387,8 +404,13 @@
 		unsigned int mdma_mode = dma_mode & 0x07;
 
 		PDPRINTK("Set mdma regs... \n");
-		pdc_set_indexed_reg(ap, 0x0e + adj, pdc2027x_mdma_timing_tbl[mdma_mode].value0);
-		pdc_set_indexed_reg(ap, 0x0f + adj, pdc2027x_mdma_timing_tbl[mdma_mode].value1);
+		ctcr0 = readl(dev_mmio(ap, adev, PDC_CTCR0));
+
+		ctcr0 &= 0x0000ffff;
+		ctcr0 |= (pdc2027x_mdma_timing_tbl[mdma_mode].value0 << 16) |
+			(pdc2027x_mdma_timing_tbl[mdma_mode].value1 << 24);
+		
+		writel(ctcr0, dev_mmio(ap, adev, PDC_CTCR0));
 		PDPRINTK("Set mdma regs done\n");
 		
 		PDPRINTK("Set to mdma mode[%u] \n", mdma_mode);

@@ -413,18 +435,17 @@
 		struct ata_device *dev = &ap->device[i];
 
 		if (ata_dev_present(dev)) {
-			u8 adj = (i % 2) ? 0x08 : 0x00;
-			u8 tmp8;
 			
 			pdc2027x_set_piomode(ap, dev);
 
 			/*
 			 * Enable prefetch if the device support PIO only.
 			 */
-			if (dev->xfer_shift == ATA_SHIFT_PIO) { 
-				tmp8 = pdc_get_indexed_reg(ap, 0x13 + adj);
-				pdc_set_indexed_reg(ap, 0x13 + adj, tmp8 | 0x02);
-				
+			if (dev->xfer_shift == ATA_SHIFT_PIO) {
+				u32 ctcr1 = readl(dev_mmio(ap, dev, PDC_CTCR1));
+				ctcr1 |= (1 << 25);
+				writel(ctcr1, dev_mmio(ap, dev, PDC_CTCR1));
+
 				PDPRINTK("Turn on prefetch\n");
 			} else {
 				pdc2027x_set_dmamode(ap, dev);

@@ -462,43 +483,31 @@
 
 static unsigned long pdc_read_counter(struct ata_probe_ent *probe_ent)
 {
-	u8 ctr0, ctr1, ctr2, ctr3;
 	unsigned long counter;
-	u8 ctr0v, ctr1v, ctr2v, ctr3v;
 	int retry = 1;
+	u32 bccrl, bccrh, bccrlv, bccrhv;
 
 retry:
-	outb(0x20, probe_ent->port[0].bmdma_addr + 0x01);
-	ctr0 = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	outb(0x21, probe_ent->port[0].bmdma_addr + 0x01);
-	ctr1 = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	outb(0x20, probe_ent->port[1].bmdma_addr + 0x01);
-	ctr2 = inb(probe_ent->port[1].bmdma_addr + 0x03);
-	outb(0x21, probe_ent->port[1].bmdma_addr + 0x01);
-	ctr3 = inb(probe_ent->port[1].bmdma_addr + 0x03);
+	bccrl = readl(probe_ent->mmio_base + PDC_BYTE_COUNT) & 0xffff;
+	bccrh = readl(probe_ent->mmio_base + PDC_BYTE_COUNT + 0x100) & 0xffff;
+	rmb();
 
 	/* Read the counter values again for verification */
-	outb(0x20,  probe_ent->port[0].bmdma_addr + 0x01);
-	ctr0v = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	outb(0x21,  probe_ent->port[0].bmdma_addr + 0x01);
-	ctr1v = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	outb(0x20,  probe_ent->port[1].bmdma_addr + 0x01);
-	ctr2v = inb(probe_ent->port[1].bmdma_addr + 0x03);
-	outb(0x21,  probe_ent->port[1].bmdma_addr + 0x01);
-	ctr3v = inb(probe_ent->port[1].bmdma_addr + 0x03);
-
-	counter = (ctr3 << 23) | (ctr2 << 15) | (ctr1 << 8) | ctr0;
-
-	PDPRINTK("ctr0[%X] ctr1[%X] ctr2[%X] ctr3[%X]\n", 
-		 ctr0, ctr1, ctr2, ctr3);
-	PDPRINTK("ctr0[%X] ctr1[%X] ctr2[%X] ctr3[%X]\n", 
-		 ctr0v, ctr1v, ctr2v, ctr3v);
+	bccrlv = readl(probe_ent->mmio_base + PDC_BYTE_COUNT) & 0xffff;
+	bccrhv = readl(probe_ent->mmio_base + PDC_BYTE_COUNT + 0x100) & 0xffff;
+	rmb();
+
+	counter = (bccrh << 15) | bccrl;
+
+	PDPRINTK("bccrh [%X] bccrl [%X]\n", bccrh,  bccrl);
+	PDPRINTK("bccrhv[%X] bccrlv[%X]\n", bccrhv, bccrlv);
 
 	/*
-	 * The 30-bit decreasing counter are read by 4 pieces.
-	 * Incorrect value may be read when ctr2 and ctr3 are changing.
+	 * The 30-bit decreasing counter are read by 2 pieces.
+	 * Incorrect value may be read when both bccrh and bccrl are changing.
+	 * Ex. When 7900 decrease to 78FF, wrong value 7800 might be read.
 	 */
-	if (retry && !(ctr3 == ctr3v && ctr2 == ctr2v && ctr1 >= ctr1v)) {
+	if (retry && !(bccrh == bccrhv && bccrl >= bccrlv)) {
 		retry--;
 		PDPRINTK("rereading counter\n");
 		goto retry;

@@ -517,12 +526,11 @@
 static void pdc_adjust_pll(struct ata_probe_ent *probe_ent, long pll_clock, unsigned int board_idx) 
 {
 
-	u8 pll_ctl0, pll_ctl1;
+	u16 pll_ctl;
 	long pll_clock_khz = pll_clock / 1000;
 	long pout_required = board_idx? PDC_133_MHZ:PDC_100_MHZ;
 	long ratio = pout_required / pll_clock_khz;
 	int F, R;
-	
 
 	/* Sanity check */
 	if (unlikely(pll_clock_khz < 5000L || pll_clock_khz > 70000L)) {

@@ -536,12 +544,9 @@
 	/* Show the current clock value of PLL control register
 	 * (maybe already configured by the firmware)
 	 */
-	outb(0x02, probe_ent->port[1].bmdma_addr + 0x01);
-	pll_ctl0 = inb(probe_ent->port[1].bmdma_addr + 0x03);
-	outb(0x03, probe_ent->port[1].bmdma_addr + 0x01);
-	pll_ctl1 = inb(probe_ent->port[1].bmdma_addr + 0x03);
+	pll_ctl = readw(probe_ent->mmio_base + PDC_PLL_CTL);
 
-	PDPRINTK("pll_ctl[%X][%X]\n", pll_ctl0, pll_ctl1);
+	PDPRINTK("pll_ctl[%X]\n", pll_ctl);
 #endif
 
 	/*

@@ -575,15 +580,12 @@
 
 	PDPRINTK("F[%d] R[%d] ratio*1000[%ld]\n", F, R, ratio);
 
-	pll_ctl0 = (u8) F;
-	pll_ctl1 = (u8) R;
+	pll_ctl = (R << 8) | F;
 
-	PDPRINTK("Writing pll_ctl[%X][%X]\n", pll_ctl0, pll_ctl1);
+	PDPRINTK("Writing pll_ctl[%X]\n", pll_ctl);
 
-	outb(0x02,     probe_ent->port[1].bmdma_addr + 0x01);
-	outb(pll_ctl0, probe_ent->port[1].bmdma_addr + 0x03);
-	outb(0x03,     probe_ent->port[1].bmdma_addr + 0x01);
-	outb(pll_ctl1, probe_ent->port[1].bmdma_addr + 0x03);
+	writew(pll_ctl, probe_ent->mmio_base + PDC_PLL_CTL);
+	readw(probe_ent->mmio_base + PDC_PLL_CTL); /* flush */
 
 	/* Wait the PLL circuit to be stable */
 	mdelay(30);

@@ -593,12 +595,9 @@
 	 *  Show the current clock value of PLL control register
 	 * (maybe configured by the firmware)
 	 */
-	outb(0x02, probe_ent->port[1].bmdma_addr + 0x01);
-	pll_ctl0 = inb(probe_ent->port[1].bmdma_addr + 0x03);
-	outb(0x03, probe_ent->port[1].bmdma_addr + 0x01);
-	pll_ctl1 = inb(probe_ent->port[1].bmdma_addr + 0x03);
+	pll_ctl = readw(probe_ent->mmio_base + PDC_PLL_CTL);
 
-	PDPRINTK("pll_ctl[%X][%X]\n", pll_ctl0, pll_ctl1);
+	PDPRINTK("pll_ctl[%X]\n", pll_ctl);
 #endif
 
 	return;

@@ -612,7 +611,7 @@
  */
 static long pdc_detect_pll_input_clock(struct ata_probe_ent *probe_ent) 
 {
-	u8 scr1;
+	u32 scr;
 	unsigned long start_count, end_count;
 	long pll_clock;

@@ -620,10 +619,10 @@
 	start_count = pdc_read_counter(probe_ent);
 
 	/* Start the test mode */
-	outb(0x01, probe_ent->port[0].bmdma_addr + 0x01);
-	scr1 = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	PDPRINTK("scr1[%X]\n", scr1);
-	outb(scr1 | 0x40, probe_ent->port[0].bmdma_addr + 0x03);
+	scr = readl(probe_ent->mmio_base + PDC_SYS_CTL);
+	PDPRINTK("scr[%X]\n", scr);
+	writel(scr | (0x01 << 14), probe_ent->mmio_base + PDC_SYS_CTL);
+	readl(probe_ent->mmio_base + PDC_SYS_CTL); /* flush */
 
 	/* Let the counter run for 100 ms. */
 	mdelay(100);

@@ -632,10 +631,10 @@
 	end_count = pdc_read_counter(probe_ent);
 
 	/* Stop the test mode */
-	outb(0x01, probe_ent->port[0].bmdma_addr + 0x01);
-	scr1 = inb(probe_ent->port[0].bmdma_addr + 0x03);
-	PDPRINTK("scr1[%X]\n", scr1);
-	outb(scr1 & 0xBF, probe_ent->port[0].bmdma_addr + 0x03);
+	scr = readl(probe_ent->mmio_base + PDC_SYS_CTL);
+	PDPRINTK("scr[%X]\n", scr);
+	writel(scr & ~(0x01 << 14), probe_ent->mmio_base + PDC_SYS_CTL);
+	readl(probe_ent->mmio_base + PDC_SYS_CTL); /* flush */
 
 	/* calculate the input clock in Hz */
 	pll_clock = (long) ((start_count - end_count) * 10);

@@ -645,6 +644,7 @@
 
 	return pll_clock;
 }
+
 /**
  * pdc_hardware_init - Initialize the hardware.
  * @pdev: instance of pci_dev found

@@ -673,6 +673,29 @@
 
 	return 0;
 }
+
+/**
+ * pdc_ata_setup_port - setup the mmio address
+ * @port: ata ioports to setup
+ * @base: base address
+ */
+static void pdc_ata_setup_port(struct ata_ioports *port, unsigned long base)
+{
+	port->cmd_addr		=
+	port->data_addr		= base;
+	port->feature_addr	=
+	port->error_addr	= base + 0x05;
+	port->nsect_addr	= base + 0x0a;
+	port->lbal_addr		= base + 0x0f;
+	port->lbam_addr		= base + 0x10;
+	port->lbah_addr		= base + 0x15;
+	port->device_addr	= base + 0x1a;
+	port->command_addr	=
+	port->status_addr	= base + 0x1f;
+	port->altstatus_addr	=
+	port->ctl_addr		= base + 0x81a;
+}
+
 /**
  * pdc2027x_init_one - PCI probe function
  * Called when an instance of PCI adapter is inserted.

@@ -690,6 +713,8 @@
 	unsigned int board_idx = (unsigned int) ent->driver_data;
 
 	struct ata_probe_ent *probe_ent = NULL;
+	unsigned long base;
+	void *mmio_base;
 	int rc;
 
 	if (!printed_version++)

@@ -722,28 +747,31 @@
 	probe_ent->dev = pci_dev_to_dev(pdev);
 	INIT_LIST_HEAD(&probe_ent->node);
 
+	mmio_base = ioremap(pci_resource_start(pdev, 5),
+		            pci_resource_len(pdev, 5));
+
+	if (mmio_base == NULL) {
+		rc = -ENOMEM;
+		goto err_out_free_ent;
+	}
+
+	base = (unsigned long) mmio_base;
+
 	probe_ent->sht		= pdc2027x_port_info[board_idx].sht;
 	probe_ent->host_flags	= pdc2027x_port_info[board_idx].host_flags;
 	probe_ent->pio_mask	= pdc2027x_port_info[board_idx].pio_mask;
+	probe_ent->mwdma_mask	= pdc2027x_port_info[board_idx].mwdma_mask;
 	probe_ent->udma_mask	= pdc2027x_port_info[board_idx].udma_mask;
 	probe_ent->port_ops	= pdc2027x_port_info[board_idx].port_ops;
 
        	probe_ent->irq = pdev->irq;
        	probe_ent->irq_flags = SA_SHIRQ;
+	probe_ent->mmio_base = mmio_base;
 
-	probe_ent->port[0].cmd_addr = pci_resource_start(pdev, 0);
-	ata_std_ports(&probe_ent->port[0]);
-	probe_ent->port[0].altstatus_addr =
-		probe_ent->port[0].ctl_addr =
-		pci_resource_start(pdev, 1) | ATA_PCI_CTL_OFS;
-	probe_ent->port[0].bmdma_addr = pci_resource_start(pdev, 4);
-
-	probe_ent->port[1].cmd_addr = pci_resource_start(pdev, 2);
-	ata_std_ports(&probe_ent->port[1]);
-	probe_ent->port[1].altstatus_addr =
-		probe_ent->port[1].ctl_addr =
-		pci_resource_start(pdev, 3) | ATA_PCI_CTL_OFS;
-	probe_ent->port[1].bmdma_addr = pci_resource_start(pdev, 4) + 8;
+	pdc_ata_setup_port(&probe_ent->port[0], base + 0x17c0);
+	probe_ent->port[0].bmdma_addr = base + 0x1000;
+	pdc_ata_setup_port(&probe_ent->port[1], base + 0x15c0);
+	probe_ent->port[1].bmdma_addr = base + 0x1008;
 
 	probe_ent->n_ports = 2;

@@ -767,6 +795,7 @@
 	pci_disable_device(pdev);
 	return rc;
 }
+
 /**
  * pdc2027x_remove_one - Called to remove a single instance of the
  * adapter.

@@ -778,6 +807,7 @@
 {
 	ata_pci_remove_one(pdev);
 }
+
 /**
  * pdc2027x_init - Called after this module is loaded into the kernel.
  */

@@ -785,6 +815,7 @@
 {
 	return pci_module_init(&pdc2027x_pci_driver);
 }
+
 /**
  * pdc2027x_exit - Called before this module unloaded from the kernel
  */

Jeff Garzik

2005-08-11 17:49 UTC #7 lore raw

Re: [PATCH 1/3] libata-dev: Convert pdc2027x from PIO to MMIO

I'm getting confused with all the patches from you :/

It's my fault, but, would you be kind enough to resend the latest versions of -all- libata and pdc2027x patches?

Jeff

Albert Lee

author

2005-08-04 17:36 UTC #8 lore raw

PATCH 2/3 libata-dev: pdc2027x mdelay() problem fix for power5 micro-partitioning

In reply to Albert Lee

Jeff,

Patch 2/3: pdc2027x mdelay() problem fix for micro-partitioning

Description:
The mdelay(100) does not delay exactly 100 ms on IBM power5 machines when the power5 CPU micro-partitioning feature is turned on.
Ex. If micro-partitioning is set to use 0.1 of the CPU time, mdelay(100) will delay about 1 sec instead of 100ms.

Changes:
- Use do_gettimeofday() to measure the actual elapsed time. According to the documentation, do_gettimeofday() works on most platforms
except some old M68K machines. There are no PCI slots on M68K machines, so using do_gettimeofday() seems to be safe here.

- Change the counter data type from "unsigned long" to "long". Since the counter value is only 30-bit, "long" should be big enough. This change is not related to the mdelay() problem, just for beautification.

Patch tested OK on x86, power4 and power5 machines.

For your review, thanks.

Albert

Signed-off-by Albert Lee <[email protected]>

drivers/scsi/pata_pdc2027x.c +10 -5

@@ -29,7 +29,7 @@
 #include <asm/io.h>
 
 #define DRV_NAME	"pata_pdc2027x"
-#define DRV_VERSION	"0.70"
+#define DRV_VERSION	"0.71"
 #undef PDC_DEBUG
 
 #ifdef PDC_DEBUG

@@ -481,9 +481,9 @@
  * @probe_ent: for the port address
  */
 
-static unsigned long pdc_read_counter(struct ata_probe_ent *probe_ent)
+static long pdc_read_counter(struct ata_probe_ent *probe_ent)
 {
-	unsigned long counter;
+	long counter;
 	int retry = 1;
 	u32 bccrl, bccrh, bccrlv, bccrhv;

@@ -612,11 +612,13 @@
 static long pdc_detect_pll_input_clock(struct ata_probe_ent *probe_ent) 
 {
 	u32 scr;
-	unsigned long start_count, end_count;
+	long start_count, end_count, usec_elapsed;
+	struct timeval start_time, end_time;
 	long pll_clock;
 
 	/* Read current counter value */
 	start_count = pdc_read_counter(probe_ent);
+	do_gettimeofday(&start_time);
 
 	/* Start the test mode */
 	scr = readl(probe_ent->mmio_base + PDC_SYS_CTL);

@@ -629,6 +631,7 @@
 
 	/* Read the counter values again */
 	end_count = pdc_read_counter(probe_ent);
+	do_gettimeofday(&end_time);
 
 	/* Stop the test mode */
 	scr = readl(probe_ent->mmio_base + PDC_SYS_CTL);

@@ -637,7 +640,9 @@
 	wmb();
 
 	/* calculate the input clock in Hz */
-	pll_clock = (long) ((start_count - end_count) * 10);
+	usec_elapsed = (end_time.tv_sec - start_time.tv_sec) * 1000000 +
+		(end_time.tv_usec - start_time.tv_usec);
+	pll_clock = (start_count - end_count) / 100 * (100000000 / usec_elapsed);
 
 	PDPRINTK("start[%lu] end[%lu] \n", start_count, end_count);
 	PDPRINTK("PLL input clock[%ld]Hz\n", pll_clock);

Benjamin Herrenschmidt

2005-08-07 09:25 UTC #9 lore raw

Re: [PATCH 2/3] libata-dev: pdc2027x mdelay() problem fix for power5 micro-partitioning

Quoted 7 lines · Albert Lee

Jeff,
Patch 2/3: pdc2027x mdelay() problem fix for micro-partitioning
Description:
The mdelay(100) does not delay exactly 100 ms on IBM power5 machines when the power5 CPU micro-partitioning feature is turned on.
Ex. If micro-partitioning is set to use 0.1 of the CPU time, mdelay(100) will delay about 1 sec instead of 100ms.

How so ? mdelay() implementation on ppc64 uses the timebase register, it should tick at a constant speed and be calibrated at boot.

Ben.

Jeff Garzik

NAK

2005-08-11 03:04 UTC #10 lore raw

Re: [PATCH 2/3] libata-dev: pdc2027x mdelay() problem fix for power5 micro-partitioning

In reply to Albert Lee

Quoted 9 lines · Albert Lee

Jeff,
Patch 2/3: pdc2027x mdelay() problem fix for micro-partitioning
Description:
The mdelay(100) does not delay exactly 100 ms on IBM power5 machines
when the power5 CPU micro-partitioning feature is turned on.
Ex. If micro-partitioning is set to use 0.1 of the CPU time, mdelay(100) will delay about 1 sec instead of 100ms.

Quoted 5 lines

Changes:
- Use do_gettimeofday() to measure the actual elapsed time. According to the documentation, do_gettimeofday() works on most platforms
except some old M68K machines. There are no PCI slots on M68K machines, so using do_gettimeofday() seems to be safe here.

NAK.

mdelay() should be investigated as to why its not working on your platform. do_gettimeofday() is -less- granular and accurate than mdelay(), usually.

Jeff

Benjamin Herrenschmidt

2005-08-11 08:37 UTC #11 lore raw

Re: [PATCH 2/3] libata-dev: pdc2027x mdelay() problem fix for power5 micro-partitioning

Quoted 3 lines · Jeff Garzik

mdelay() should be investigated as to why its not working on your platform. do_gettimeofday() is -less- granular and accurate than mdelay(), usually.

Agreed. mdelay() should have worked.

Ben.

Albert Lee

author

2005-08-11 09:53 UTC #12 lore raw

Re: [PATCH 2/3] libata-dev: pdc2027x mdelay() problem fix for power5 micro-partitioning

Quoted 8 lines · Benjamin Herrenschmidt

Quoted 1 line · Jeff Garzik
mdelay() should be investigated as to why its not working on your platform. do_gettimeofday() is -less- granular and accurate than mdelay(), usually.
Agreed. mdelay() should have worked.

Understood. Will verify it again on newer kernel.

Thanks,

Albert

Albert Lee

author

2005-08-04 17:40 UTC #13 lore raw

PATCH 3/3 libata-dev: Fix pdc2027x ATAPI DMA lost irq problem

In reply to Albert Lee

Jeff,

Patch 3/3: Fix pdc2027x ATAPI DMA lost irq problem

Description:
Sometimes pdc2027x will lost irq after ATAPI DMA data transfer. With the previous rule (cmd->request_bufflen % 256), the ATAPI DMA irq lost
problem still occurs during the test.

Root cause for the irq lost is unknown. I've tried your ATAPI DMA alignment
patch, but the problem still occurs, even the buffer is aligned. I guess it is pdc2027x hardware problem.

The following workarounds are adapted from the Promise pdc618 GPL driver.
They seems know about the problem and has the workaround.

Changes:
- Only turn on ATAPI DMA for READ, WRITE, READ_CD and READ_DVD_STRUCTURE commands.
- For WRITE_10, if LBA -45150 (FFFF4FA2h) to -1 (FFFFFFFFh) then use PIO mode.

For your review, thanks.

Albert

Signed-off-by Albert Lee <[email protected]>

drivers/scsi/pata_pdc2027x.c +34 -5

@@ -29,7 +29,7 @@
 #include <asm/io.h>
 
 #define DRV_NAME	"pata_pdc2027x"
-#define DRV_VERSION	"0.71"
+#define DRV_VERSION	"0.72"
 #undef PDC_DEBUG
 
 #ifdef PDC_DEBUG

@@ -467,11 +467,40 @@
 static int pdc2027x_check_atapi_dma(struct ata_queued_cmd *qc)
 {
 	struct scsi_cmnd *cmd = qc->scsicmd;
-	int rc = 0;
+	u8 *scsicmd = cmd->cmnd;
+	int rc = 1; /* atapi dma off by default */
+	u32 lba;
 
-	/* pdc2027x can only do ATAPI DMA for specific buffer size */
-	if (cmd->request_bufflen % 256)
-		rc = 1;
+	/*
+	 * pdc2027x might lost irq if ATAPI DMA is used
+	 * for commands not in the white list.
+	 */
+	switch (scsicmd[0]) {
+	case READ_10:
+	case READ_12:
+	case READ_6:
+	case WRITE_12:
+	case WRITE_6:
+	case 0xad: /* READ_DVD_STRUCTURE */
+	case 0xbe: /* READ_CD */
+		/* ATAPI DMA is ok */
+		rc = 0;
+		break;
+	case WRITE_10:
+		/* LBA -45150 (FFFF4FA2h) to 
+		 * -1 (FFFFFFFFh) shall use PIO mode
+		 */
+		lba = (scsicmd[2] << 24) | (scsicmd[3] << 16) |
+			(scsicmd[4] << 8) | scsicmd[5]; 
+
+		if (lba < 0xffff4fa2)
+			/* ATAPI DMA is ok */
+			rc = 0;
+
+		break;
+	default:
+		;
+	}
 
 	return rc;
 }

Doug Maxey

2005-08-04 18:47 UTC #14 lore raw

Re: [PATCH 3/3] libata-dev: Fix pdc2027x ATAPI DMA lost irq problem

I suspect that Albert will not be able to reply for a few days, there seems to be an issue with the weather in Taiwan, as in, they may be hunkered down for a typhoon.

Quoted 8 lines · Albert Lee

Jeff,
Patch 3/3: Fix pdc2027x ATAPI DMA lost irq problem
Description:
Sometimes pdc2027x will lost irq after ATAPI DMA data transfer. With the previous rule (cmd->request_bufflen % 256), the ATAPI DMA irq lost
problem still occurs during the test.

Do you want to lash up the analyzer here in Austin? I suspect this may be a manifestation of a READ CAPACITY issue. It is legal for the device fw to report the lead-in, and lead-out blocks in the total, but you cannot actually access the blocks from the device. The blocks reported by READ CAPACITY can be off by 3 to the high side.

Quoted 11 lines

Root cause for the irq lost is unknown. I've tried your ATAPI DMA alignment
patch, but the problem still occurs, even the buffer is aligned. I guess it is pdc2027x hardware problem.
The following workarounds are adapted from the Promise pdc618 GPL driver.
They seems know about the problem and has the workaround.
Changes:
- Only turn on ATAPI DMA for READ, WRITE, READ_CD and READ_DVD_STRUCTURE commands.
- For WRITE_10, if LBA -45150 (FFFF4FA2h) to -1 (FFFFFFFFh) then use PIO mode.

See above.

++doug

Jeff Garzik

2005-08-11 03:05 UTC #15 lore raw

Re: [PATCH 3/3] libata-dev: Fix pdc2027x ATAPI DMA lost irq problem

In reply to Albert Lee

Quoted 10 lines · Albert Lee

Jeff,
Patch 3/3: Fix pdc2027x ATAPI DMA lost irq problem
Description:
Sometimes pdc2027x will lost irq after ATAPI DMA data transfer. With the previous rule (cmd->request_bufflen % 256), the ATAPI DMA irq
lost
problem still occurs during the test.

Quoted 6 lines

Root cause for the irq lost is unknown. I've tried your ATAPI DMA
alignment
patch, but the problem still occurs, even the buffer is aligned. I guess it is pdc2027x hardware problem.
The following workarounds are adapted from the Promise pdc618 GPL driver.
They seems know about the problem and has the workaround.

I think this needs to be investigated a bit more, but the pdc618 driver does it, we may need to do this eventually.

Jeff