[PATCH libata-dev-2.6:sil24 00/08] sil24: misc patches

[PATCH libata-dev-2.6:sil24 00/08] sil24: misc patches

[Tejun Heo]

 Hello, Edward, Jeff and Carlos.

 These are misc patches against new sil24 driver acked by Jeff, and
two new ones from me and two new ones from Edward.  I'm sorry this
patchset took so long.  It just slipped my attention.  I hope this
wouldn't cause too much delay in Edward's work, so that we can include
this driver in mainline soon.

#01-#04: these are already acked by Jeff.  They can just go in.
#05-#06: two new ones from me.  It removes PORT_TF as it doesn't work.
#07-#08: constant fixes and comments from Edward Falk.

 Notes

* I haven't included the patch which adds IO flushing after masking
  irq during initialization, as it seems that it isn't necessary here.

>>>> FROM: Jeff
>>>>add a readl() to flush this write out to the PCI bus ("PCI posting")
>>>
>>> FROM: Me
>>> Sure.  And, out of curiosity, isn't sync unnecessary unless we're
>>>gonna perform some kind of timed waiting following it?  We don't have
>>>any timing requirement after above interrupt masking, do we?
>>
>> FROM: Edward
>>I think we're ok here; the code reads PORT_CTRL_STAT a few lines down;
>>that will flush the write.  I don't think there's a race condition
>>involved.
>
>FROM: Jeff
>No race condition.  Typically there is often a mistaken assumption that
>
>        writel(...)
>        udelay(...)
>
>will accomplish the desired effect.  Due to posted writes, the write may
>be posted to the PCI device after some delay, such that, the udelay()
>and the posted write execute concurrently, skewing the desired timing
>effect.

* Edward, I removed PORT_SLOT/PORT_SG stuff from your patch, as it
  currently isn't used anywhere.  If you get to use those, please
  include the change in the patch which use those.

* Stuff remaining to be done
  Show stoppers
      - reading proper signature value during probing
      - reading D2H FIS to determine proper stat/err values
      - less fragile command completion handling
  Others
      - 64bit enable
      - verify resetting and other code against hardware doc
      - (later) NCQ support
      - (later) hotplug support
      - (later) port multiplier support
      - (later) better error reporting
      - (maybe) multiple command queueing for non-NCQ drives

[ Start of patch descriptions ]

01_sil24_add-FIXME-comment.patch
	: add FIXME comment above ata_device_add

	Add FIXME comment above ata_device_add.

02_sil24_remove-irq-disable-on-spurious-intr.patch
	: remove irq disable code on spurious interrupt

	If interrupt occurs on a disabled port, the driver used to
        mask the port's interrupt, but we don't know if such action is
        necessary yet and that's not what other drives do.  So, just
        do nothing and tell IRQ subsystem that it's not our interrupt.

03_sil24_add-pci-fault-check.patch
	: add testing for PCI fault

	On entry to interrupt handler, PORT_SLOT_STAT register is read
        first.  Check if PCI fault or device removal has occurred by
        testing the value for 0xffffffff.

04_sil24_add-error_intr-function.patch
	: move error handling out of hot interrupt path

	Move error handling from sil24_host_intr into separate
        function - sil24_error_intr.

05_sil24_remove-PORT_TF.patch
	: remove PORT_TF

	Remove PORT_TF, as taskfile isn't located at PORT_TF.

06_sil24_remove-pp-port.patch
	: replace pp->port w/ ap->ioaddr.cmd_addr

	As ap->ioaddr.cmd_addr isn't used for PORT_TF anymore, replace
	pp->port w/ it as AHCI does.

07_sil24_fix-PORT_CTRL_STAT-constants.patch
	: fix PORT_CTRL_STAT constants

	PORT_CTRL_STAT constants were copied incorrectly from the
	preview driver.

	Signed-off-by: Edward Falk

08_sil24_add-comments-for-constants.patch
	: add more comments for constants

	Add more comments to constants.

	Signed-off-by: Edward Falk

[ End of patch descriptions ]

 Thanks.

--
tejun

Re: [PATCH libata-dev-2.6:sil24 01/08] sil24: add FIXME comment above ata_device_add

[Tejun Heo]

01_sil24_add-FIXME-comment.patch

	Add FIXME comment above ata_device_add.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 sata_sil24.c |    1 +
 1 files changed, 1 insertion(+)

Index: work/drivers/scsi/sata_sil24.c
===================================================================
--- work.orig/drivers/scsi/sata_sil24.c	2005-08-17 12:32:41.000000000 +0900
+++ work/drivers/scsi/sata_sil24.c	2005-08-17 13:06:06.000000000 +0900
@@ -749,6 +749,7 @@ static int sil24_init_one(struct pci_dev
 
 	pci_set_master(pdev);
 
+	/* FIXME: check ata_device_add return value */
 	ata_device_add(probe_ent);
 
 	kfree(probe_ent);

Re: [PATCH libata-dev-2.6:sil24 02/08] sil24: remove irq disable code on spurious interrupt

[Tejun Heo]

02_sil24_remove-irq-disable-on-spurious-intr.patch

	If interrupt occurs on a disabled port, the driver used to
        mask the port's interrupt, but we don't know if such action is
        necessary yet and that's not what other drives do.  So, just
        do nothing and tell IRQ subsystem that it's not our interrupt.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 sata_sil24.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

Index: work/drivers/scsi/sata_sil24.c
===================================================================
--- work.orig/drivers/scsi/sata_sil24.c	2005-08-17 13:06:06.000000000 +0900
+++ work/drivers/scsi/sata_sil24.c	2005-08-17 13:06:07.000000000 +0900
@@ -535,17 +535,12 @@ static irqreturn_t sil24_interrupt(int i
 	for (i = 0; i < host_set->n_ports; i++)
 		if (status & (1 << i)) {
 			struct ata_port *ap = host_set->ports[i];
-			if (ap && !(ap->flags & ATA_FLAG_PORT_DISABLED))
+			if (ap && !(ap->flags & ATA_FLAG_PORT_DISABLED)) {
 				sil24_host_intr(host_set->ports[i]);
-			else {
-				u32 tmp;
-				printk(KERN_WARNING DRV_NAME
-				       ": spurious interrupt from port %d\n", i);
-				tmp = readl(hpriv->host_base + HOST_CTRL);
-				tmp &= ~(1 << i);
-				writel(tmp, hpriv->host_base + HOST_CTRL);
-			}
-			handled++;
+				handled++;
+			} else
+				printk(KERN_ERR DRV_NAME
+				       ": interrupt from disabled port %d\n", i);
 		}
 
 	spin_unlock(&host_set->lock);

Re: [PATCH libata-dev-2.6:sil24 03/08] sil24: add testing for PCI fault

[Tejun Heo]

03_sil24_add-pci-fault-check.patch

	On entry to interrupt handler, PORT_SLOT_STAT register is read
        first.  Check if PCI fault or device removal has occurred by
        testing the value for 0xffffffff.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 sata_sil24.c |    6 ++++++
 1 files changed, 6 insertions(+)

Index: work/drivers/scsi/sata_sil24.c
===================================================================
--- work.orig/drivers/scsi/sata_sil24.c	2005-08-17 13:06:07.000000000 +0900
+++ work/drivers/scsi/sata_sil24.c	2005-08-17 13:06:07.000000000 +0900
@@ -527,6 +527,12 @@ static irqreturn_t sil24_interrupt(int i
 
 	status = readl(hpriv->host_base + HOST_IRQ_STAT);
 
+	if (status == 0xffffffff) {
+		printk(KERN_ERR DRV_NAME ": IRQ status == 0xffffffff, "
+		       "PCI fault or device removal?\n");
+		goto out;
+	}
+
 	if (!(status & IRQ_STAT_4PORTS))
 		goto out;
 

Re: [PATCH libata-dev-2.6:sil24 04/08] sil24: move error handling out of hot interrupt path

[Tejun Heo]

04_sil24_add-error_intr-function.patch

	Move error handling from sil24_host_intr into separate
        function - sil24_error_intr.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 sata_sil24.c |   55 +++++++++++++++++++++++++++++++------------------------
 1 files changed, 31 insertions(+), 24 deletions(-)

Index: work/drivers/scsi/sata_sil24.c
===================================================================
--- work.orig/drivers/scsi/sata_sil24.c	2005-08-17 13:06:07.000000000 +0900
+++ work/drivers/scsi/sata_sil24.c	2005-08-17 13:06:08.000000000 +0900
@@ -480,6 +480,35 @@ static void sil24_eng_timeout(struct ata
 	sil24_reset_controller(ap);
 }
 
+static void sil24_error_intr(struct ata_port *ap, u32 slot_stat)
+{
+	struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
+	struct sil24_port_priv *pp = ap->private_data;
+	void *port = pp->port;
+	u32 irq_stat, cmd_err, sstatus, serror;
+
+	irq_stat = readl(port + PORT_IRQ_STAT);
+	cmd_err = readl(port + PORT_CMD_ERR);
+	sstatus = readl(port + PORT_SSTATUS);
+	serror = readl(port + PORT_SERROR);
+
+	/* Clear IRQ/errors */
+	writel(irq_stat, port + PORT_IRQ_STAT);
+	if (cmd_err)
+		writel(cmd_err, port + PORT_CMD_ERR);
+	if (serror)
+		writel(serror, port + PORT_SERROR);
+
+	printk(KERN_ERR DRV_NAME " ata%u: error interrupt on port%d\n"
+	       "  stat=0x%x irq=0x%x cmd_err=%d sstatus=0x%x serror=0x%x\n",
+	       ap->id, ap->port_no, slot_stat, irq_stat, cmd_err, sstatus, serror);
+
+	if (qc)
+		ata_qc_complete(qc, ATA_ERR);
+
+	sil24_reset_controller(ap);
+}
+
 static inline void sil24_host_intr(struct ata_port *ap)
 {
 	struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
@@ -491,30 +520,8 @@ static inline void sil24_host_intr(struc
 	if (!(slot_stat & HOST_SSTAT_ATTN)) {
 		if (qc)
 			ata_qc_complete(qc, 0);
-	} else {
-		u32 irq_stat, cmd_err, sstatus, serror;
-
-		irq_stat = readl(port + PORT_IRQ_STAT);
-		cmd_err = readl(port + PORT_CMD_ERR);
-		sstatus = readl(port + PORT_SSTATUS);
-		serror = readl(port + PORT_SERROR);
-
-		/* Clear IRQ/errors */
-		writel(irq_stat, port + PORT_IRQ_STAT);
-		if (cmd_err)
-			writel(cmd_err, port + PORT_CMD_ERR);
-		if (serror)
-			writel(serror, port + PORT_SERROR);
-
-		printk(KERN_ERR DRV_NAME " ata%u: error interrupt on port%d\n"
-		       "  stat=0x%x irq=0x%x cmd_err=%d sstatus=0x%x serror=0x%x\n",
-		       ap->id, ap->port_no, slot_stat, irq_stat, cmd_err, sstatus, serror);
-
-		if (qc)
-			ata_qc_complete(qc, ATA_ERR);
-
-		sil24_reset_controller(ap);
-	}
+	} else
+		sil24_error_intr(ap, slot_stat);
 }
 
 static irqreturn_t sil24_interrupt(int irq, void *dev_instance, struct pt_regs *regs)

Re: [PATCH libata-dev-2.6:sil24 05/08] sil24: remove PORT_TF

[Tejun Heo]

05_sil24_remove-PORT_TF.patch

	Remove PORT_TF, as taskfile isn't located at PORT_TF.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 sata_sil24.c |    7 +------
 1 files changed, 1 insertion(+), 6 deletions(-)

Index: work/drivers/scsi/sata_sil24.c
===================================================================
--- work.orig/drivers/scsi/sata_sil24.c	2005-08-17 13:06:08.000000000 +0900
+++ work/drivers/scsi/sata_sil24.c	2005-08-17 13:06:08.000000000 +0900
@@ -104,9 +104,6 @@ enum {
 	 */
 	PORT_REGS_SIZE		= 0x2000,
 	PORT_PRB		= 0x0000, /* (32 bytes PRB + 16 bytes SGEs * 6) * 31 (3968 bytes) */
-		/* TF is overlayed w/ PRB regs in the preview driver,
-		 * but it doesn't seem to work. */
-	PORT_TF			= 0x0000,
 
 	PORT_PM			= 0x0f80, /* 8 bytes PM * 16 (128 bytes) */
 		/* 32 bit regs */
@@ -703,9 +700,7 @@ static int sil24_init_one(struct pci_dev
 		u32 tmp;
 		int cnt;
 
-		probe_ent->port[i].cmd_addr = portu + PORT_TF;
-		probe_ent->port[i].ctl_addr = portu + PORT_TF + 0xa;
-		probe_ent->port[i].altstatus_addr = portu + PORT_TF + 0xa;
+		probe_ent->port[i].cmd_addr = portu;
 		probe_ent->port[i].scr_addr = portu + PORT_SCONTROL;
 
 		ata_std_ports(&probe_ent->port[i]);

Re: [PATCH libata-dev-2.6:sil24 06/08] sil24: replace pp->port w/ ap->ioaddr.cmd_addr

[Tejun Heo]

06_sil24_remove-pp-port.patch

	As ap->ioaddr.cmd_addr isn't used for PORT_TF anymore, replace
	pp->port w/ it as AHCI does.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 sata_sil24.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)

Index: work/drivers/scsi/sata_sil24.c
===================================================================
--- work.orig/drivers/scsi/sata_sil24.c	2005-08-17 13:06:08.000000000 +0900
+++ work/drivers/scsi/sata_sil24.c	2005-08-17 13:06:08.000000000 +0900
@@ -214,7 +214,6 @@ struct sil24_cmd_block {
  * here from the previous interrupt.
  */
 struct sil24_port_priv {
-	void *port;
 	struct sil24_cmd_block *cmd_block;	/* 32 cmd blocks */
 	dma_addr_t cmd_block_dma;		/* DMA base addr for them */
 };
@@ -414,10 +413,11 @@ static void sil24_qc_prep(struct ata_que
 static int sil24_qc_issue(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
+	void *port = (void *)ap->ioaddr.cmd_addr;
 	struct sil24_port_priv *pp = ap->private_data;
 	dma_addr_t paddr = pp->cmd_block_dma + qc->tag * sizeof(*pp->cmd_block);
 
-	writel((u32)paddr, pp->port + PORT_CMD_ACTIVATE);
+	writel((u32)paddr, port + PORT_CMD_ACTIVATE);
 	return 0;
 }
 
@@ -428,8 +428,7 @@ static void sil24_irq_clear(struct ata_p
 
 static void sil24_reset_controller(struct ata_port *ap)
 {
-	struct sil24_port_priv *pp = ap->private_data;
-	void *port = pp->port;
+	void *port = (void *)ap->ioaddr.cmd_addr;
 	int cnt;
 	u32 tmp;
 
@@ -480,8 +479,7 @@ static void sil24_eng_timeout(struct ata
 static void sil24_error_intr(struct ata_port *ap, u32 slot_stat)
 {
 	struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
-	struct sil24_port_priv *pp = ap->private_data;
-	void *port = pp->port;
+	void *port = (void *)ap->ioaddr.cmd_addr;
 	u32 irq_stat, cmd_err, sstatus, serror;
 
 	irq_stat = readl(port + PORT_IRQ_STAT);
@@ -509,8 +507,7 @@ static void sil24_error_intr(struct ata_
 static inline void sil24_host_intr(struct ata_port *ap)
 {
 	struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
-	struct sil24_port_priv *pp = ap->private_data;
-	void *port = pp->port;
+	void *port = (void *)ap->ioaddr.cmd_addr;
 	u32 slot_stat;
 
 	slot_stat = readl(port + PORT_SLOT_STAT);
@@ -561,7 +558,6 @@ static irqreturn_t sil24_interrupt(int i
 static int sil24_port_start(struct ata_port *ap)
 {
 	struct device *dev = ap->host_set->dev;
-	struct sil24_host_priv *hpriv = ap->host_set->private_data;
 	struct sil24_port_priv *pp;
 	struct sil24_cmd_block *cb;
 	size_t cb_size = sizeof(*cb);
@@ -579,7 +575,6 @@ static int sil24_port_start(struct ata_p
 	}
 	memset(cb, 0, cb_size);
 
-	pp->port = hpriv->port_base + ap->port_no * PORT_REGS_SIZE;
 	pp->cmd_block = cb;
 	pp->cmd_block_dma = cb_dma;
 
@@ -700,7 +695,7 @@ static int sil24_init_one(struct pci_dev
 		u32 tmp;
 		int cnt;
 
-		probe_ent->port[i].cmd_addr = portu;
+		probe_ent->port[i].cmd_addr = portu + PORT_PRB;
 		probe_ent->port[i].scr_addr = portu + PORT_SCONTROL;
 
 		ata_std_ports(&probe_ent->port[i]);

Re: [PATCH libata-dev-2.6:sil24 07/08] sil24: fix PORT_CTRL_STAT constants

[Tejun Heo]

07_sil24_fix-PORT_CTRL_STAT-constants.patch

	PORT_CTRL_STAT constants were copied incorrectly from the
	preview driver.

Signed-off-by: Edward Falk

 sata_sil24.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

Index: work/drivers/scsi/sata_sil24.c
===================================================================
--- work.orig/drivers/scsi/sata_sil24.c	2005-08-17 13:06:08.000000000 +0900
+++ work/drivers/scsi/sata_sil24.c	2005-08-17 13:06:08.000000000 +0900
@@ -140,10 +140,10 @@ enum {
 	PORT_CS_DEV_RST		= (1 << 1), /* device reset */
 	PORT_CS_INIT		= (1 << 2), /* port initialize */
 	PORT_CS_IRQ_WOC		= (1 << 3), /* interrupt write one to clear */
-	PORT_CS_RESUME		= (1 << 4), /* port resume */
-	PORT_CS_32BIT_ACTV	= (1 << 5), /* 32-bit activation */
-	PORT_CS_PM_EN		= (1 << 6), /* port multiplier enable */
-	PORT_CS_RDY		= (1 << 7), /* port ready to accept commands */
+	PORT_CS_RESUME		= (1 << 6), /* port resume */
+	PORT_CS_32BIT_ACTV	= (1 << 10), /* 32-bit activation */
+	PORT_CS_PM_EN		= (1 << 13), /* port multiplier enable */
+	PORT_CS_RDY		= (1 << 31), /* port ready to accept commands */
 
 	/* PORT_IRQ_STAT/ENABLE_SET/CLR */
 	/* bits[11:0] are masked */

Re: [PATCH libata-dev-2.6:sil24 08/08] sil24: add more comments for constants

[Tejun Heo]

08_sil24_add-comments-for-constants.patch

	Add more comments to constants.

Signed-off-by: Edward Falk

 sata_sil24.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

Index: work/drivers/scsi/sata_sil24.c
===================================================================
--- work.orig/drivers/scsi/sata_sil24.c	2005-08-17 13:06:08.000000000 +0900
+++ work/drivers/scsi/sata_sil24.c	2005-08-17 13:06:09.000000000 +0900
@@ -107,14 +107,14 @@ enum {
 
 	PORT_PM			= 0x0f80, /* 8 bytes PM * 16 (128 bytes) */
 		/* 32 bit regs */
-	PORT_CTRL_STAT		= 0x1000, /* write:ctrl, read:stat */
-	PORT_CTRL_CLR		= 0x1004,
-	PORT_IRQ_STAT		= 0x1008,
-	PORT_IRQ_ENABLE_SET	= 0x1010,
-	PORT_IRQ_ENABLE_CLR	= 0x1014,
+	PORT_CTRL_STAT		= 0x1000, /* write: ctrl-set, read: stat */
+	PORT_CTRL_CLR		= 0x1004, /* write: ctrl-clear */
+	PORT_IRQ_STAT		= 0x1008, /* high: status, low: interrupt */
+	PORT_IRQ_ENABLE_SET	= 0x1010, /* write: enable-set */
+	PORT_IRQ_ENABLE_CLR	= 0x1014, /* write: enable-clear */
 	PORT_ACTIVATE_UPPER_ADDR= 0x101c,
-	PORT_EXEC_FIFO		= 0x1020,
-	PORT_CMD_ERR		= 0x1024,
+	PORT_EXEC_FIFO		= 0x1020, /* command execution fifo */
+	PORT_CMD_ERR		= 0x1024, /* command error number */
 	PORT_FIS_CFG		= 0x1028,
 	PORT_FIFO_THRES		= 0x102c,
 		/* 16 bit regs */
@@ -187,7 +187,7 @@ enum {
 	PORT_CERR_XFR_TGTABRT	= 33, /* PSD ecode 01 - target abort */
 	PORT_CERR_XFR_MSGABRT	= 34, /* PSD ecode 10 - master abort */
 	PORT_CERR_XFR_PCIPERR	= 35, /* PSD ecode 11 - PCI prity err during transfer */
-	PORT_CERR_SENDSERVICE	= 36, /* FIS received whiel sending service */
+	PORT_CERR_SENDSERVICE	= 36, /* FIS received while sending service */
 
 	/*
 	 * Other constants

Re: [PATCH libata-dev-2.6:sil24 01/08] sil24: add FIXME comment above ata_device_add

[Jeff Garzik]

applied patches 1-8 to 'sil24' branch of libata-dev.git