[PATCH 2.6.14-rc2 0/2] libata: Marvell SATA support (v0.23-0.24)

[PATCH 2.6.14-rc2 0/2] libata: Marvell SATA support (v0.23-0.24)

[Brett Russ]

This patch series should fix up lockups that people were seeing due to
improper spinlock placement (nested==bad).  Additionally, there are
other changes to simplify things (complex=bad).  The second patch adds
comment headers to functions that need it.

Thanks,
BR

[PATCH 2.6.14-rc2 1/2] libata: Marvell spinlock fixes and simplification

[Brett Russ]

This patch should fix up lockups that people were seeing due to
improper spinlock placement.  Also, the start/stop DMA routines put
guarded trust in the cached state of DMA.

Thanks,
BR

Signed-off-by: Brett Russ <russb@emc.com>


Index: linux-2.6.14-rc2/drivers/scsi/sata_mv.c
===================================================================
--- linux-2.6.14-rc2.orig/drivers/scsi/sata_mv.c
+++ linux-2.6.14-rc2/drivers/scsi/sata_mv.c
@@ -35,7 +35,7 @@
 #include <asm/io.h>
 
 #define DRV_NAME	"sata_mv"
-#define DRV_VERSION	"0.22"
+#define DRV_VERSION	"0.23"
 
 enum {
 	/* BAR's are enumerated in terms of pci_resource_start() terms */
@@ -406,40 +406,30 @@ static void mv_irq_clear(struct ata_port
 {
 }
 
-static void mv_start_dma(void __iomem *base, struct mv_port_priv *pp,
-			 struct ata_port *ap)
+static void mv_start_dma(void __iomem *base, struct mv_port_priv *pp)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&ap->host_set->lock, flags);
-	
-	writelfl(EDMA_EN, base + EDMA_CMD_OFS);
-	pp->pp_flags |= MV_PP_FLAG_EDMA_EN;
-
-	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+	if (!(MV_PP_FLAG_EDMA_EN & pp->pp_flags)) {
+		writelfl(EDMA_EN, base + EDMA_CMD_OFS);
+		pp->pp_flags |= MV_PP_FLAG_EDMA_EN;
+	}
+	assert(EDMA_EN & readl(base + EDMA_CMD_OFS));
 }
 
 static void mv_stop_dma(struct ata_port *ap)
 {
 	void __iomem *port_mmio = mv_ap_base(ap);
 	struct mv_port_priv *pp	= ap->private_data;
-	unsigned long flags;
 	u32 reg;
 	int i;
 
-	spin_lock_irqsave(&ap->host_set->lock, flags);
-	
-	if (!(MV_PP_FLAG_EDMA_DS_ACT & pp->pp_flags) &&
-	    ((MV_PP_FLAG_EDMA_EN & pp->pp_flags) ||
-	     (EDMA_EN & readl(port_mmio + EDMA_CMD_OFS)))) {
-		/* Disable EDMA if we're not already trying to disable it
-		 * and it is currently active.   The disable bit auto clears.
+	if (MV_PP_FLAG_EDMA_EN & pp->pp_flags) {
+		/* Disable EDMA if active.   The disable bit auto clears.
 		 */
-		pp->pp_flags |= MV_PP_FLAG_EDMA_DS_ACT;
 		writelfl(EDMA_DS, port_mmio + EDMA_CMD_OFS);
 		pp->pp_flags &= ~MV_PP_FLAG_EDMA_EN;
-	}
-	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+	} else {
+		assert(!(EDMA_EN & readl(port_mmio + EDMA_CMD_OFS)));
+  	}
 	
 	/* now properly wait for the eDMA to stop */
 	for (i = 1000; i > 0; i--) {
@@ -450,12 +440,9 @@ static void mv_stop_dma(struct ata_port 
 		udelay(100);
 	}
 
-	spin_lock_irqsave(&ap->host_set->lock, flags);
-	pp->pp_flags &= ~MV_PP_FLAG_EDMA_DS_ACT;
-	spin_unlock_irqrestore(&ap->host_set->lock, flags);
-
 	if (EDMA_EN & reg) {
 		printk(KERN_ERR "ata%u: Unable to stop eDMA\n", ap->id);
+		/* FIXME: Consider doing a reset here to recover */
 	}
 }
 
@@ -716,8 +703,11 @@ static void mv_port_stop(struct ata_port
 {
 	struct device *dev = ap->host_set->dev;
 	struct mv_port_priv *pp = ap->private_data;
+	unsigned long flags;
 
+	spin_lock_irqsave(&ap->host_set->lock, flags);
 	mv_stop_dma(ap);
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
 
 	ap->private_data = NULL;
 	dma_free_coherent(dev, MV_PORT_PRIV_DMA_SZ, pp->crpb, pp->crpb_dma);
@@ -867,11 +857,7 @@ static int mv_qc_issue(struct ata_queued
 
 	mv_inc_q_index(&pp->req_producer);	/* now incr producer index */
 
-	if (!(MV_PP_FLAG_EDMA_EN & pp->pp_flags)) {
-		/* turn on EDMA if not already on */
-		mv_start_dma(port_mmio, pp, qc->ap);
-	}
-	assert(EDMA_EN & readl(port_mmio + EDMA_CMD_OFS));
+	mv_start_dma(port_mmio, pp);
 
 	/* and write the request in pointer to kick the EDMA to life */
 	in_ptr &= EDMA_REQ_Q_BASE_LO_MASK;
@@ -921,8 +907,12 @@ static void mv_err_intr(struct ata_port 
 		serr = scr_read(ap, SCR_ERROR);
 		scr_write_flush(ap, SCR_ERROR, serr);
 	}
-	DPRINTK("port %u error; EDMA err cause: 0x%08x SERR: 0x%08x\n", 
-		ap->port_no, edma_err_cause, serr);
+	if (EDMA_ERR_SELF_DIS & edma_err_cause) {
+		struct mv_port_priv *pp	= ap->private_data;
+		pp->pp_flags &= ~MV_PP_FLAG_EDMA_EN;
+	}
+	DPRINTK(KERN_ERR "ata%u: port error; EDMA err cause: 0x%08x "
+		"SERR: 0x%08x\n", ap->id, edma_err_cause, serr);
 
 	/* Clear EDMA now that SERR cleanup done */
 	writelfl(0, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
@@ -1034,7 +1024,7 @@ static irqreturn_t mv_interrupt(int irq,
 		printk(KERN_ERR DRV_NAME ": PCI ERROR; PCI IRQ cause=0x%08x\n",
 		       readl(mmio + PCI_IRQ_CAUSE_OFS));
 
-		VPRINTK("All regs @ PCI error\n");
+		DPRINTK("All regs @ PCI error\n");
 		mv_dump_all_regs(mmio, -1, to_pci_dev(host_set->dev));
 
 		writelfl(0, mmio + PCI_IRQ_CAUSE_OFS);

Re: [PATCH 2.6.14-rc2 1/2] libata: Marvell spinlock fixes and simplification

[Jeff Garzik]

applied 1-2 to 'upstream' branch of libata-dev.git

Re: [PATCH 2.6.14-rc2 1/2] libata: Marvell spinlock fixes and simplification

[Jeff Garzik]

[-- Attachment #1: Type: text/plain, Size: 143 bytes --]

Also, I just applied the attached patch, to kill a warning:

drivers/scsi/sata_mv.c:471: warning: 'mv_dump_mem' defined but not used

	Jeff




[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 558 bytes --]

diff --git a/drivers/scsi/sata_mv.c b/drivers/scsi/sata_mv.c
--- a/drivers/scsi/sata_mv.c
+++ b/drivers/scsi/sata_mv.c
@@ -467,9 +467,9 @@ static void mv_stop_dma(struct ata_port 
 	}
 }
 
+#ifdef ATA_DEBUG
 static void mv_dump_mem(void __iomem *start, unsigned bytes)
 {
-#ifdef ATA_DEBUG
 	int b, w;
 	for (b = 0; b < bytes; ) {
 		DPRINTK("%p: ", start + b);
@@ -479,8 +479,9 @@ static void mv_dump_mem(void __iomem *st
 		}
 		printk("\n");
 	}
-#endif
 }
+#endif
+
 static void mv_dump_pci_cfg(struct pci_dev *pdev, unsigned bytes)
 {
 #ifdef ATA_DEBUG

Re: [PATCH 2.6.14-rc2 1/2] libata: Marvell spinlock fixes and simplification

[Michael Madore]

On Wed, 2005-10-05 at 17:08 -0400, Brett Russ wrote:
> This patch should fix up lockups that people were seeing due to
> improper spinlock placement.  Also, the start/stop DMA routines put
> guarded trust in the cached state of DMA.

Hi Brett,

I assume this patch doesn't address the 'abnormal status 0x80' issue on
the 6081.  On the 5081, I still get two machine checks followed by a
hard lockup when I load the driver.

Mike

Re: [PATCH 2.6.14-rc2 1/2] libata: Marvell spinlock fixes and simplification

[Brett Russ]

Michael Madore wrote:
> I assume this patch doesn't address the 'abnormal status 0x80' issue on
> the 6081.

Correct assumption.  This message seems benign as I've seen it and am 
working fine despite it.  I'll take a look soon.

Is the driver working for you on the 6081?

> On the 5081, I still get two machine checks followed by a
> hard lockup when I load the driver.

Would you turn on ATA_DEBUG and ATA_VERBOSE_DEBUG in 
include/linux/libata.h and send along the console output when loading 
the driver?

Thanks,
BR

Re: [PATCH 2.6.14-rc2 1/2] libata: Marvell spinlock fixes and simplification

[Michael Madore]

Hi Brett,

On Thu, 2005-10-06 at 07:52 -0400, Brett Russ wrote:
> Michael Madore wrote:
> > I assume this patch doesn't address the 'abnormal status 0x80' issue on
> > the 6081.
> 
> Correct assumption.  This message seems benign as I've seen it and am 
> working fine despite it.  I'll take a look soon.
> 
> Is the driver working for you on the 6081?

No.  The driver loads, but the disk is not accessible.

> > On the 5081, I still get two machine checks followed by a
> > hard lockup when I load the driver.
> 
> Would you turn on ATA_DEBUG and ATA_VERBOSE_DEBUG in 
> include/linux/libata.h and send along the console output when loading 
> the driver?

Output below, this time with a single Maxtor Maxline Plus II.  I have
included the debug output from both the 5081 and 6081.  In the case of
the 6081, modprobe never returns.

Mike

5081:

SCSI subsystem initialized
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err
cause/mask=0x00000000/0x00001f7f
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err
cause/mask=0x00000000/0x00001f7f
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err
cause/mask=0x00000000/0x00001f7f
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err
cause/mask=0x00000000/0x00001f7f
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err
cause/mask=0x00000000/0x00001f7f
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err
cause/mask=0x00000000/0x00001f7f
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err
cause/mask=0x00000000/0x00001f7f
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err
cause/mask=0x00000000/0x00001f7f
mv_host_init: HC0: HC config=0x417feb15 HC IRQ cause (before
clear)=0x00000111
mv_host_init: HC1: HC config=0x00000001 HC IRQ cause (before
clear)=0x00000000
mv_host_init: HC MAIN IRQ cause/mask=0x00000000/0x0007ffff PCI int
cause/mask=0x00000000/0x00577fe6
mv_dump_pci_cfg: 00: 508111ab 02b00317 01040000 00004008
mv_dump_pci_cfg: 10: fc300004 00000000 00000000 00000000
mv_dump_pci_cfg: 20: 00000000 00000000 00000000 508115d9
mv_dump_pci_cfg: 30: 00000000 00000040 00000000 0000010b
mv_dump_pci_cfg: 40: 000a5001 00000000 00000000 00000000
mv_dump_pci_cfg: 50: 00816005 fee01004 00000000 000040e1
mv_dump_pci_cfg: 60: 00300007 01830310
ata_device_add: ENTER
ata_host_add: ENTER
ata_host_add: ENTER
ata_host_add: ENTER
ata_host_add: ENTER
ata_host_add: ENTER
ata_host_add: ENTER
ata_host_add: ENTER
ata_host_add: ENTER
ata_device_add: probe begin
ata_device_add: ata1: probe begin
mv_phy_reset: ENTER, port 0, mmio 0xf8b22000
CPU 2: Machine Check Exception: 0000000000000004
CPU 3: Machine Check Exception: 0000000000000004

6081:

Oct 10 11:36:33 asl48 kernel: SCSI subsystem initialized
Oct 10 11:36:33 asl48 kernel: sata_mv version 0.24
Oct 10 11:36:33 asl48 kernel: ACPI: PCI Interrupt 0000:03:03.0[A] -> GSI
28 (level, low) -> IRQ 209
Oct 10 11:36:33 asl48 kernel: mv_port_init: EDMA cfg=0x0000291f EDMA IRQ
err cause/mask=0x00000000/0xffffffff
Oct 10 11:36:33 asl48 last message repeated 7 times
Oct 10 11:36:33 asl48 kernel: mv_host_init: HC0: HC config=0x000100ff HC
IRQ cause (before clear)=0x00000000
Oct 10 11:36:33 asl48 kernel: mv_host_init: HC1: HC config=0x000100ff HC
IRQ cause (before clear)=0x00000000
Oct 10 11:36:33 asl48 kernel: mv_host_init: HC MAIN IRQ
cause/mask=0x00000000/0x0087ffff PCI int
cause/mask=0x00000000/0x00557fee
Oct 10 11:36:33 asl48 kernel: mv_dump_pci_cfg: 00: 608111ab 02b00317
01000009 00004008
Oct 10 11:36:33 asl48 kernel: mv_dump_pci_cfg: 10: fc300004 00000000
00004001 00000004
Oct 10 11:36:33 asl48 kernel: mv_dump_pci_cfg: 20: 00000000 00000000
00000000 11ab11ab
Oct 10 11:36:33 asl48 kernel: mv_dump_pci_cfg: 30: 00000000 00000040
00000000 0000010b
Oct 10 11:36:33 asl48 kernel: mv_dump_pci_cfg: 40: 00025001 00000000
00000000 00000000
Oct 10 11:36:33 asl48 kernel: mv_dump_pci_cfg: 50: 00816005 fee01004
00000000 000040e1
Oct 10 11:36:33 asl48 kernel: mv_dump_pci_cfg: 60: 00300007 01830318
Oct 10 11:36:33 asl48 kernel: sata_mv(0000:03:03.0) 32 slots 8 ports
SCSI mode IRQ via MSI
Oct 10 11:36:33 asl48 kernel: ata_device_add: ENTER
Oct 10 11:36:33 asl48 kernel: ata_host_add: ENTER
Oct 10 11:36:33 asl48 kernel: ata1: SATA max UDMA/133 cmd 0x0 ctl
0xF8B22120 bmdma 0x0 irq 209
Oct 10 11:36:33 asl48 kernel: ata_host_add: ENTER
Oct 10 11:36:33 asl48 kernel: ata2: SATA max UDMA/133 cmd 0x0 ctl
0xF8B24120 bmdma 0x0 irq 209
Oct 10 11:36:33 asl48 kernel: ata_host_add: ENTER
Oct 10 11:36:33 asl48 kernel: ata3: SATA max UDMA/133 cmd 0x0 ctl
0xF8B26120 bmdma 0x0 irq 209
Oct 10 11:36:33 asl48 kernel: ata_host_add: ENTER
Oct 10 11:36:33 asl48 kernel: ata4: SATA max UDMA/133 cmd 0x0 ctl
0xF8B28120 bmdma 0x0 irq 209
Oct 10 11:36:33 asl48 kernel: ata_host_add: ENTER
Oct 10 11:36:33 asl48 kernel: ata5: SATA max UDMA/133 cmd 0x0 ctl
0xF8B32120 bmdma 0x0 irq 209
Oct 10 11:36:33 asl48 kernel: ata_host_add: ENTER
Oct 10 11:36:33 asl48 kernel: ata6: SATA max UDMA/133 cmd 0x0 ctl
0xF8B34120 bmdma 0x0 irq 209
Oct 10 11:36:33 asl48 kernel: ata_host_add: ENTER
Oct 10 11:36:33 asl48 kernel: ata7: SATA max UDMA/133 cmd 0x0 ctl
0xF8B36120 bmdma 0x0 irq 209
Oct 10 11:36:33 asl48 kernel: ata_host_add: ENTER
Oct 10 11:36:33 asl48 kernel: ata8: SATA max UDMA/133 cmd 0x0 ctl
0xF8B38120 bmdma 0x0 irq 209
Oct 10 11:36:33 asl48 kernel: ata_device_add: probe begin
Oct 10 11:36:33 asl48 kernel: ata_device_add: ata1: probe begin
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: ENTER, port 0, mmio
0xf8b22000
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after ATA_RST: SStat
0x00000004 SErr 0x00000000 SCtrl 0x00000004
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after PHY wake: SStat
0x00000000 SErr 0x00000000 SCtrl 0x00000300
Oct 10 11:36:33 asl48 kernel: ata1: no device found (phy stat 00000000)
Oct 10 11:36:33 asl48 kernel: ata_device_add: ata1: probe end
Oct 10 11:36:33 asl48 kernel: scsi0 : sata_mv
Oct 10 11:36:33 asl48 kernel: ata_device_add: ata2: probe begin
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: ENTER, port 1, mmio
0xf8b24000
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after ATA_RST: SStat
0x00000004 SErr 0x00000000 SCtrl 0x00000004
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after PHY wake: SStat
0x00000000 SErr 0x00000000 SCtrl 0x00000300
Oct 10 11:36:33 asl48 kernel: ata2: no device found (phy stat 00000000)
Oct 10 11:36:33 asl48 kernel: ata_device_add: ata2: probe end
Oct 10 11:36:33 asl48 kernel: scsi1 : sata_mv
Oct 10 11:36:33 asl48 kernel: ata_device_add: ata3: probe begin
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: ENTER, port 2, mmio
0xf8b26000
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after ATA_RST: SStat
0x00000004 SErr 0x00000000 SCtrl 0x00000004
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after PHY wake: SStat
0x00000000 SErr 0x00000000 SCtrl 0x00000300
Oct 10 11:36:33 asl48 kernel: ata3: no device found (phy stat 00000000)
Oct 10 11:36:33 asl48 kernel: ata_device_add: ata3: probe end
Oct 10 11:36:33 asl48 kernel: scsi2 : sata_mv
Oct 10 11:36:33 asl48 kernel: ata_device_add: ata4: probe begin
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: ENTER, port 3, mmio
0xf8b28000
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after ATA_RST: SStat
0x00000004 SErr 0x00000000 SCtrl 0x00000004
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after PHY wake: SStat
0x00000000 SErr 0x00000000 SCtrl 0x00000300
Oct 10 11:36:33 asl48 kernel: ata4: no device found (phy stat 00000000)
Oct 10 11:36:33 asl48 kernel: ata_device_add: ata4: probe end
Oct 10 11:36:33 asl48 kernel: scsi3 : sata_mv
Oct 10 11:36:33 asl48 kernel: ata_device_add: ata5: probe begin
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: ENTER, port 4, mmio
0xf8b32000
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after ATA_RST: SStat
0x00000004 SErr 0x00000000 SCtrl 0x00000004
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after PHY wake: SStat
0x00000000 SErr 0x00000000 SCtrl 0x00000300
Oct 10 11:36:33 asl48 kernel: ata5: no device found (phy stat 00000000)
Oct 10 11:36:33 asl48 kernel: ata_device_add: ata5: probe end
Oct 10 11:36:33 asl48 kernel: scsi4 : sata_mv
Oct 10 11:36:33 asl48 kernel: ata_device_add: ata6: probe begin
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: ENTER, port 5, mmio
0xf8b34000
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after ATA_RST: SStat
0x00000004 SErr 0x00000000 SCtrl 0x00000004
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: S-regs after PHY wake: SStat
0x00000113 SErr 0x04010000 SCtrl 0x00000300
Oct 10 11:36:33 asl48 kernel: ata_dev_classify: found ATA device by sig
Oct 10 11:36:33 asl48 kernel: mv_phy_reset: EXIT
Oct 10 11:36:33 asl48 kernel: ata_dev_identify: ENTER, host 6, dev 0
Oct 10 11:36:33 asl48 kernel: ata_dev_select: ENTER, ata6: device 0,
wait 1
Oct 10 11:36:33 asl48 kernel: ata_dev_identify: do ATA identify
Oct 10 11:36:33 asl48 kernel: ata_dev_select: ENTER, ata6: device 0,
wait 1
Oct 10 11:36:33 asl48 kernel: ata_exec_command_mmio: ata6: cmd 0xEC
Oct 10 11:36:33 asl48 kernel: ata_pio_sector: data read
Oct 10 11:36:33 asl48 kernel: ata_qc_complete: EXIT
Oct 10 11:36:33 asl48 kernel: ata_dump_id: 49==0x2f00  53==0x0007
63==0x0407  64==0x0003  75==0x0000
Oct 10 11:36:33 asl48 kernel: ata_dump_id: 80==0x00fe  81==0x001e
82==0x7c6b  83==0x7f09  84==0x4003
Oct 10 11:36:33 asl48 kernel: ata_dump_id: 88==0x007f  93==0x0000
Oct 10 11:36:33 asl48 kernel: ata6: dev 0 ATA, max UDMA/133, 490234752
sectors: lba48
Oct 10 11:36:33 asl48 kernel: ata_dev_identify: EXIT, drv_stat = 0x50
Oct 10 11:36:33 asl48 kernel: ata_dev_identify: ENTER/EXIT (host 6, dev
1) -- nodev
Oct 10 11:36:33 asl48 kernel: ata_host_set_pio: base 0x8 xfer_mode 0xc
mask 0x1f x 4
Oct 10 11:36:33 asl48 kernel: ata_dev_set_xfermode: set features - xfer
mode
Oct 10 11:36:33 asl48 kernel: ata_dev_select: ENTER, ata6: device 0,
wait 1
Oct 10 11:36:33 asl48 kernel: ata_tf_load_mmio: hob: feat 0x0 nsect 0x0,
lba 0x0 0x0 0x0
Oct 10 11:36:33 asl48 kernel: ata_tf_load_mmio: feat 0x3 nsect 0x46 lba
0x0 0x0 0x0
Oct 10 11:36:33 asl48 kernel: ata_tf_load_mmio: device 0xA0
Oct 10 11:36:33 asl48 kernel: ata_exec_command_mmio: ata6: cmd 0xEF

[PATCH 2.6.14-rc2 2/2] libata: Marvell function headers

[Brett Russ]

This patch adds helpful function header comments.

Thanks,
BR

Signed-off-by: Brett Russ <russb@emc.com>


Index: linux-2.6.14-rc2/drivers/scsi/sata_mv.c
===================================================================
--- linux-2.6.14-rc2.orig/drivers/scsi/sata_mv.c
+++ linux-2.6.14-rc2/drivers/scsi/sata_mv.c
@@ -35,7 +35,7 @@
 #include <asm/io.h>
 
 #define DRV_NAME	"sata_mv"
-#define DRV_VERSION	"0.23"
+#define DRV_VERSION	"0.24"
 
 enum {
 	/* BAR's are enumerated in terms of pci_resource_start() terms */
@@ -406,6 +406,17 @@ static void mv_irq_clear(struct ata_port
 {
 }
 
+/**
+ *      mv_start_dma - Enable eDMA engine
+ *      @base: port base address
+ *      @pp: port private data
+ *
+ *      Verify the local cache of the eDMA state is accurate with an
+ *      assert.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static void mv_start_dma(void __iomem *base, struct mv_port_priv *pp)
 {
 	if (!(MV_PP_FLAG_EDMA_EN & pp->pp_flags)) {
@@ -415,6 +426,16 @@ static void mv_start_dma(void __iomem *b
 	assert(EDMA_EN & readl(base + EDMA_CMD_OFS));
 }
 
+/**
+ *      mv_stop_dma - Disable eDMA engine
+ *      @ap: ATA channel to manipulate
+ *
+ *      Verify the local cache of the eDMA state is accurate with an
+ *      assert.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static void mv_stop_dma(struct ata_port *ap)
 {
 	void __iomem *port_mmio = mv_ap_base(ap);
@@ -561,7 +582,15 @@ static void mv_scr_write(struct ata_port
 	}
 }
 
-/* This routine only applies to 6xxx parts */
+/**
+ *      mv_global_soft_reset - Perform the 6xxx global soft reset
+ *      @mmio_base: base address of the HBA
+ *
+ *      This routine only applies to 6xxx parts.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static int mv_global_soft_reset(void __iomem *mmio_base)
 {
 	void __iomem *reg = mmio_base + PCI_MAIN_CMD_STS_OFS;
@@ -617,6 +646,16 @@ done:
 	return rc;
 }
 
+/**
+ *      mv_host_stop - Host specific cleanup/stop routine.
+ *      @host_set: host data structure
+ *
+ *      Disable ints, cleanup host memory, call general purpose
+ *      host_stop.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static void mv_host_stop(struct ata_host_set *host_set)
 {
 	struct mv_host_priv *hpriv = host_set->private_data;
@@ -631,6 +670,16 @@ static void mv_host_stop(struct ata_host
 	ata_host_stop(host_set);
 }
 
+/**
+ *      mv_port_start - Port specific init/start routine.
+ *      @ap: ATA channel to manipulate
+ *
+ *      Allocate and point to DMA memory, init port private memory,
+ *      zero indices.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static int mv_port_start(struct ata_port *ap)
 {
 	struct device *dev = ap->host_set->dev;
@@ -699,6 +748,15 @@ static int mv_port_start(struct ata_port
 	return 0;
 }
 
+/**
+ *      mv_port_stop - Port specific cleanup/stop routine.
+ *      @ap: ATA channel to manipulate
+ *
+ *      Stop DMA, cleanup port memory.
+ *
+ *      LOCKING:
+ *      This routine uses the host_set lock to protect the DMA stop.
+ */
 static void mv_port_stop(struct ata_port *ap)
 {
 	struct device *dev = ap->host_set->dev;
@@ -714,6 +772,15 @@ static void mv_port_stop(struct ata_port
 	kfree(pp);
 }
 
+/**
+ *      mv_fill_sg - Fill out the Marvell ePRD (scatter gather) entries
+ *      @qc: queued command whose SG list to source from
+ *
+ *      Populate the SG list and mark the last entry.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static void mv_fill_sg(struct ata_queued_cmd *qc)
 {
 	struct mv_port_priv *pp = qc->ap->private_data;
@@ -748,6 +815,18 @@ static inline void mv_crqb_pack_cmd(u16 
 		(last ? CRQB_CMD_LAST : 0);
 }
 
+/**
+ *      mv_qc_prep - Host specific command preparation.
+ *      @qc: queued command to prepare
+ *
+ *      This routine simply redirects to the general purpose routine
+ *      if command is not DMA.  Else, it handles prep of the CRQB
+ *      (command request block), does some sanity checking, and calls
+ *      the SG load routine.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static void mv_qc_prep(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
@@ -830,6 +909,18 @@ static void mv_qc_prep(struct ata_queued
 	mv_fill_sg(qc);
 }
 
+/**
+ *      mv_qc_issue - Initiate a command to the host
+ *      @qc: queued command to start
+ *
+ *      This routine simply redirects to the general purpose routine
+ *      if command is not DMA.  Else, it sanity checks our local
+ *      caches of the request producer/consumer indices then enables
+ *      DMA and bumps the request producer index.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static int mv_qc_issue(struct ata_queued_cmd *qc)
 {
 	void __iomem *port_mmio = mv_ap_base(qc->ap);
@@ -867,6 +958,19 @@ static int mv_qc_issue(struct ata_queued
 	return 0;
 }
 
+/**
+ *      mv_get_crpb_status - get status from most recently completed cmd
+ *      @ap: ATA channel to manipulate
+ *
+ *      This routine is for use when the port is in DMA mode, when it
+ *      will be using the CRPB (command response block) method of
+ *      returning command completion information.  We assert indices
+ *      are good, grab status, and bump the response consumer index to
+ *      prove that we're up to date.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static u8 mv_get_crpb_status(struct ata_port *ap)
 {
 	void __iomem *port_mmio = mv_ap_base(ap);
@@ -896,6 +1000,19 @@ static u8 mv_get_crpb_status(struct ata_
 	return (pp->crpb[pp->rsp_consumer].flags >> CRPB_FLAG_STATUS_SHIFT);
 }
 
+/**
+ *      mv_err_intr - Handle error interrupts on the port
+ *      @ap: ATA channel to manipulate
+ *
+ *      In most cases, just clear the interrupt and move on.  However,
+ *      some cases require an eDMA reset, which is done right before
+ *      the COMRESET in mv_phy_reset().  The SERR case requires a
+ *      clear of pending errors in the SATA SERROR register.  Finally,
+ *      if the port disabled DMA, update our cached copy to match.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static void mv_err_intr(struct ata_port *ap)
 {
 	void __iomem *port_mmio = mv_ap_base(ap);
@@ -923,7 +1040,22 @@ static void mv_err_intr(struct ata_port 
 	}
 }
 
-/* Handle any outstanding interrupts in a single SATAHC */
+/**
+ *      mv_host_intr - Handle all interrupts on the given host controller
+ *      @host_set: host specific structure
+ *      @relevant: port error bits relevant to this host controller
+ *      @hc: which host controller we're to look at
+ *
+ *      Read then write clear the HC interrupt status then walk each
+ *      port connected to the HC and see if it needs servicing.  Port
+ *      success ints are reported in the HC interrupt status reg, the
+ *      port error ints are reported in the higher level main
+ *      interrupt status register and thus are passed in via the
+ *      'relevant' argument.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static void mv_host_intr(struct ata_host_set *host_set, u32 relevant,
 			 unsigned int hc)
 {
@@ -993,6 +1125,21 @@ static void mv_host_intr(struct ata_host
 	VPRINTK("EXIT\n");
 }
 
+/**
+ *      mv_interrupt - 
+ *      @irq: unused
+ *      @dev_instance: private data; in this case the host structure
+ *      @regs: unused
+ *
+ *      Read the read only register to determine if any host
+ *      controllers have pending interrupts.  If so, call lower level
+ *      routine to handle.  Also check for PCI errors which are only
+ *      reported here.
+ *
+ *      LOCKING: 
+ *      This routine holds the host_set lock while processing pending
+ *      interrupts.
+ */
 static irqreturn_t mv_interrupt(int irq, void *dev_instance,
 				struct pt_regs *regs)
 {
@@ -1035,14 +1182,32 @@ static irqreturn_t mv_interrupt(int irq,
 	return IRQ_RETVAL(handled);
 }
 
+/**
+ *      mv_check_err - Return the error shadow register to caller.
+ *      @ap: ATA channel to manipulate
+ *
+ *      Marvell requires DMA to be stopped before accessing shadow
+ *      registers.  So we do that, then return the needed register.
+ *
+ *      LOCKING:
+ *      Inherited from caller.  FIXME: protect mv_stop_dma with lock?
+ */
 static u8 mv_check_err(struct ata_port *ap)
 {
 	mv_stop_dma(ap);		/* can't read shadow regs if DMA on */
 	return readb((void __iomem *) ap->ioaddr.error_addr);
 }
 
-/* Part of this is taken from __sata_phy_reset and modified to not sleep
- * since this routine gets called from interrupt level.
+/**
+ *      mv_phy_reset - Perform eDMA reset followed by COMRESET
+ *      @ap: ATA channel to manipulate
+ *
+ *      Part of this is taken from __sata_phy_reset and modified to
+ *      not sleep since this routine gets called from interrupt level.
+ *
+ *      LOCKING:
+ *      Inherited from caller.  This is coded to safe to call at
+ *      interrupt level, i.e. it does not sleep.
  */
 static void mv_phy_reset(struct ata_port *ap)
 {
@@ -1105,6 +1270,16 @@ static void mv_phy_reset(struct ata_port
 	VPRINTK("EXIT\n");
 }
 
+/**
+ *      mv_eng_timeout - Routine called by libata when SCSI times out I/O
+ *      @ap: ATA channel to manipulate
+ *
+ *      Intent is to clear all pending error conditions, reset the
+ *      chip/bus, fail the command, and move on.
+ *
+ *      LOCKING:
+ *      This routine holds the host_set lock while failing the command.
+ */
 static void mv_eng_timeout(struct ata_port *ap)
 {
 	struct ata_queued_cmd *qc;
@@ -1140,6 +1315,18 @@ static void mv_eng_timeout(struct ata_po
 	}
 }
 
+/**
+ *      mv_port_init - Perform some early initialization on a single port.
+ *      @port: libata data structure storing shadow register addresses
+ *      @port_mmio: base address of the port
+ *
+ *      Initialize shadow register mmio addresses, clear outstanding
+ *      interrupts on the port, and unmask interrupts for the future
+ *      start of the port.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static void mv_port_init(struct ata_ioports *port,  void __iomem *port_mmio)
 {
 	unsigned long shd_base = (unsigned long) port_mmio + SHD_BLK_OFS;
@@ -1177,6 +1364,16 @@ static void mv_port_init(struct ata_iopo
 		readl(port_mmio + EDMA_ERR_IRQ_MASK_OFS));
 }
 
+/**
+ *      mv_host_init - Perform some early initialization of the host.
+ *      @probe_ent: early data struct representing the host
+ *
+ *      If possible, do an early global reset of the host.  Then do
+ *      our port init and clear/unmask all/relevant host interrupts.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static int mv_host_init(struct ata_probe_ent *probe_ent)
 {
 	int rc = 0, n_hc, port, hc;
@@ -1226,7 +1423,15 @@ done:
 	return rc;
 }
 
-/* FIXME: complete this */
+/**
+ *      mv_print_info - Dump key info to kernel log for perusal.
+ *      @probe_ent: early data struct representing the host
+ *
+ *      FIXME: complete this.
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static void mv_print_info(struct ata_probe_ent *probe_ent)
 {
 	struct pci_dev *pdev = to_pci_dev(probe_ent->dev);
@@ -1253,6 +1458,14 @@ static void mv_print_info(struct ata_pro
 	       scc_s, (MV_HP_FLAG_MSI & hpriv->hp_flags) ? "MSI" : "INTx");
 }
 
+/**
+ *      mv_init_one - handle a positive probe of a Marvell host
+ *      @pdev: PCI device found
+ *      @ent: PCI device ID entry for the matched host
+ *
+ *      LOCKING:
+ *      Inherited from caller.
+ */
 static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	static int printed_version = 0;

Re: [PATCH 2.6.14-rc2 0/2] libata: Marvell SATA support (v0.23-0.24)

[Bogdan Costescu]

On Wed, 5 Oct 2005, Brett Russ wrote:

> This patch series should fix up lockups that people were seeing due to
> improper spinlock placement (nested==bad).  Additionally, there are
> other changes to simplify things (complex=bad).  The second patch adds
> comment headers to functions that need it.

I guess that 0.24 was not supposed to fix any 50x1 problems, but I 
just wanted to provide more debug output. The disk (this time another 
one, but still connected to the first port) is still not detected:

sata_mv version 0.24
ACPI: PCI Interrupt 0000:02:08.0[A] -> GSI 26 (level, low) -> IRQ 201
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err cause/mask=0x00000000/0x00001f7f
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err cause/mask=0x00000000/0x00001f7f
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err cause/mask=0x00000000/0x00001f7f
mv_port_init: EDMA cfg=0x0000011f EDMA IRQ err cause/mask=0x00000000/0x00001f7f
mv_host_init: HC0: HC config=0x11dcf013 HC IRQ cause (before clear)=0x00000111
mv_host_init: HC MAIN IRQ cause/mask=0x00000000/0x0007ffff PCI int cause/mask=0x00000000/0x00577fe6
mv_dump_pci_cfg: 00: 504111ab 02b00007 01000003 00002008 
mv_dump_pci_cfg: 10: f5000004 00000000 00000000 00000000 
mv_dump_pci_cfg: 20: 00000000 00000000 00000000 81241043 
mv_dump_pci_cfg: 30: 00000000 00000040 00000000 00000109 
mv_dump_pci_cfg: 40: 000a5001 00000000 00000000 00000000 
mv_dump_pci_cfg: 50: 00816005 fee01004 00000000 000040d9 
mv_dump_pci_cfg: 60: 00300007 01830240 
sata_mv(0000:02:08.0) 32 slots 4 ports SCSI mode IRQ via MSI
ata3: SATA max PIO4 cmd 0x0 ctl 0xF8A22120 bmdma 0x0 irq 201
ata4: SATA max PIO4 cmd 0x0 ctl 0xF8A24120 bmdma 0x0 irq 201
ata5: SATA max PIO4 cmd 0x0 ctl 0xF8A26120 bmdma 0x0 irq 201
ata6: SATA max PIO4 cmd 0x0 ctl 0xF8A28120 bmdma 0x0 irq 201
mv_phy_reset: ENTER, port 0, mmio 0xf8a22000
mv_phy_reset: S-regs after ATA_RST: SStat 0x00000000 SErr 0x00000000 SCtrl 0x00000000
mv_phy_reset: S-regs after PHY wake: SStat 0x00000000 SErr 0x00000000 SCtrl 0x00000000
ata3: no device found (phy stat 00000000)
scsi2 : sata_mv
mv_phy_reset: ENTER, port 1, mmio 0xf8a24000
mv_phy_reset: S-regs after ATA_RST: SStat 0x00000000 SErr 0x00000000 SCtrl 0x00000000
mv_phy_reset: S-regs after PHY wake: SStat 0x00000000 SErr 0x00000000 SCtrl 0x00000000
ata4: no device found (phy stat 00000000)
scsi3 : sata_mv
mv_phy_reset: ENTER, port 2, mmio 0xf8a26000
mv_phy_reset: S-regs after ATA_RST: SStat 0x00000000 SErr 0x00000000 SCtrl 0x00000000
mv_phy_reset: S-regs after PHY wake: SStat 0x00000000 SErr 0x00000000 SCtrl 0x00000000
ata5: no device found (phy stat 00000000)
scsi4 : sata_mv
mv_phy_reset: ENTER, port 3, mmio 0xf8a28000
mv_phy_reset: S-regs after ATA_RST: SStat 0x00000000 SErr 0x00000000 SCtrl 0x00000000
mv_phy_reset: S-regs after PHY wake: SStat 0x00000000 SErr 0x00000000 SCtrl 0x00000000
ata6: no device found (phy stat 00000000)
scsi5 : sata_mv

Thanks for working on this driver !

-- 
Bogdan Costescu

IWR - Interdisziplinaeres Zentrum fuer Wissenschaftliches Rechnen
Universitaet Heidelberg, INF 368, D-69120 Heidelberg, GERMANY
Telephone: +49 6221 54 8869, Telefax: +49 6221 54 8868
E-mail: Bogdan.Costescu@IWR.Uni-Heidelberg.De

Re: [PATCH 2.6.14-rc2 0/2] libata: Marvell SATA support (v0.23-0.24)

[Jeff Garzik]

Bogdan Costescu wrote:
> mv_phy_reset: ENTER, port 0, mmio 0xf8a22000
> mv_phy_reset: S-regs after ATA_RST: SStat 0x00000000 SErr 0x00000000 
> SCtrl 0x00000000
> mv_phy_reset: S-regs after PHY wake: SStat 0x00000000 SErr 0x00000000 
> SCtrl 0x00000000
> ata3: no device found (phy stat 00000000)
> scsi2 : sata_mv
> mv_phy_reset: ENTER, port 1, mmio 0xf8a24000
> mv_phy_reset: S-regs after ATA_RST: SStat 0x00000000 SErr 0x00000000 
> SCtrl 0x00000000
> mv_phy_reset: S-regs after PHY wake: SStat 0x00000000 SErr 0x00000000 
> SCtrl 0x00000000
> ata4: no device found (phy stat 00000000)
> scsi3 : sata_mv
> mv_phy_reset: ENTER, port 2, mmio 0xf8a26000
> mv_phy_reset: S-regs after ATA_RST: SStat 0x00000000 SErr 0x00000000 
> SCtrl 0x00000000
> mv_phy_reset: S-regs after PHY wake: SStat 0x00000000 SErr 0x00000000 
> SCtrl 0x00000000
> ata5: no device found (phy stat 00000000)
> scsi4 : sata_mv
> mv_phy_reset: ENTER, port 3, mmio 0xf8a28000
> mv_phy_reset: S-regs after ATA_RST: SStat 0x00000000 SErr 0x00000000 
> SCtrl 0x00000000
> mv_phy_reset: S-regs after PHY wake: SStat 0x00000000 SErr 0x00000000 
> SCtrl 0x00000000
> ata6: no device found (phy stat 00000000)
> scsi5 : sata_mv

Staring at the docs a bit, I notice that the 50xx and 60xx have SATA 
S{status,control,error} registers at different locations.

50xx:

	SStatus:
	SATAHC0 0x20100 0x20200 0x20300 0x20400
	SATAHC1 0x30100 0x30200 0x30300 0x30400

	SError:
	SStatus addr + 4

	SControl:
	SStatus addr + 8

60xx:

	SStatus:
	SATAHC0 0x22300 0x24300 0x26300 0x28300
	SATAHC1 0x32300 0x34300 0x36300 0x38300

	SError:
	SStatus addr + 4

	SControl:
	SStatus addr + 8

Re: [PATCH 2.6.14-rc2 0/2] libata: Marvell SATA support (v0.23-0.24)

[Brett Russ]

Jeff Garzik wrote:
> Staring at the docs a bit, I notice that the 50xx and 60xx have SATA 
> S{status,control,error} registers at different locations.


Yes and also even some registers that are at the same location have 
changed bit definitions.  Aye caramba.

Best solution will probably be to create separate enums for each chip 
generation, in addition to a common enum, and point to the relevant one 
based on the chip identifier.

No surprise we're seeing so many problems.  I have just not spent any 
time at all on 5xxx.  Probably should yank it from the pci device table 
for now.

BR

Re: [PATCH 2.6.14-rc2 0/2] libata: Marvell SATA support (v0.23-0.24)

[Jeff Garzik]

Brett Russ wrote:
> Jeff Garzik wrote:
> 
>> Staring at the docs a bit, I notice that the 50xx and 60xx have SATA 
>> S{status,control,error} registers at different locations.
> 
> 
> 
> Yes and also even some registers that are at the same location have 
> changed bit definitions.  Aye caramba.
> 
> Best solution will probably be to create separate enums for each chip 
> generation, in addition to a common enum, and point to the relevant one 
> based on the chip identifier.
> 
> No surprise we're seeing so many problems.  I have just not spent any 
> time at all on 5xxx.  Probably should yank it from the pci device table 
> for now.

I would suggest submitting a patch to put #if 0 around the PCI table 
entries, so that testers can easily turn it back on...

	Jeff

Re: [PATCH 2.6.14-rc2 0/2] libata: Marvell SATA support (v0.23-0.24)

[Bogdan Costescu]

On Thu, 6 Oct 2005, Brett Russ wrote:

> No surprise we're seeing so many problems.  I have just not spent 
> any time at all on 5xxx.  Probably should yank it from the pci 
> device table for now.

Fair enough. Given that the register addresses that Jeff mentioned are 
not enough if the register content is different, I'll just wait for 
somebody with documentation to come up with proper code.

... which means that I'll stop testing now, as I only have 5xxx 
controllers. Please specifically mention 5xxx if a new patch starts to 
support them. Thanks again!

-- 
Bogdan Costescu

IWR - Interdisziplinaeres Zentrum fuer Wissenschaftliches Rechnen
Universitaet Heidelberg, INF 368, D-69120 Heidelberg, GERMANY
Telephone: +49 6221 54 8869, Telefax: +49 6221 54 8868
E-mail: Bogdan.Costescu@IWR.Uni-Heidelberg.De