linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sata_via: convert to new EH
@ 2006-06-13 11:51 Tejun Heo
  2006-06-13 21:46 ` [PATCH] sata_via: convert to new EH, take #2 Tejun Heo
  0 siblings, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2006-06-13 11:51 UTC (permalink / raw)
  To: Jeff Garzik, linux-ide

Convert sata_via to new EH.  The driver used SRST for vt6420 and sata
hardreset for vt6421.  This patch preserves the behavior using private
port flag SVIA_FORCE_SATA_RESET and private error handler.

Once it's determined that vt6421 can do SRST, the flag and private
error handler can be replaced with stock bmdma EH.

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

---

 drivers/scsi/sata_via.c |   26 ++++++++++++++++++++------
 1 files changed, 20 insertions(+), 6 deletions(-)

90c02687c5f5b1fe18933389ac41f4cc8730fad3
diff --git a/drivers/scsi/sata_via.c b/drivers/scsi/sata_via.c
index c6975c5..8241f58 100644
--- a/drivers/scsi/sata_via.c
+++ b/drivers/scsi/sata_via.c
@@ -69,11 +69,15 @@ enum {
 
 	SATA_EXT_PHY		= (1 << 6), /* 0==use PATA, 1==ext phy */
 	SATA_2DEV		= (1 << 5), /* SATA is master/slave */
+
+	/* private host flags */
+	SVIA_FORCE_SATA_RESET	= (1 << 24),
 };
 
 static int svia_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
 static u32 svia_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void svia_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
+static void svia_error_handler (struct ata_port *ap);
 
 static const struct pci_device_id svia_pci_tbl[] = {
 	{ 0x1106, 0x3149, PCI_ANY_ID, PCI_ANY_ID, 0, 0, vt6420 },
@@ -116,8 +120,6 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
 
-	.phy_reset		= sata_phy_reset,
-
 	.bmdma_setup            = ata_bmdma_setup,
 	.bmdma_start            = ata_bmdma_start,
 	.bmdma_stop		= ata_bmdma_stop,
@@ -127,7 +129,8 @@ static const struct ata_port_operations 
 	.qc_issue		= ata_qc_issue_prot,
 	.data_xfer		= ata_pio_data_xfer,
 
-	.eng_timeout		= ata_eng_timeout,
+	.error_handler		= svia_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
 	.irq_handler		= ata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
@@ -142,7 +145,7 @@ static const struct ata_port_operations 
 
 static struct ata_port_info svia_port_info = {
 	.sht		= &svia_sht,
-	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST | ATA_FLAG_NO_LEGACY,
+	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
 	.pio_mask	= 0x1f,
 	.mwdma_mask	= 0x07,
 	.udma_mask	= 0x7f,
@@ -169,6 +172,17 @@ static void svia_scr_write (struct ata_p
 	outl(val, ap->ioaddr.scr_addr + (4 * sc_reg));
 }
 
+static void svia_error_handler (struct ata_port *ap)
+{
+	ata_reset_fn_t softreset = ata_std_softreset;
+
+	if (ap->flags & SVIA_FORCE_SATA_RESET)
+		softreset = NULL;
+
+	ata_bmdma_drive_eh(ap, ata_std_prereset, softreset, sata_std_hardreset,
+			   ata_std_postreset);
+}
+
 static const unsigned int svia_bar_sizes[] = {
 	8, 4, 8, 4, 16, 256
 };
@@ -237,8 +251,8 @@ static struct ata_probe_ent *vt6421_init
 	INIT_LIST_HEAD(&probe_ent->node);
 
 	probe_ent->sht		= &svia_sht;
-	probe_ent->host_flags	= ATA_FLAG_SATA | ATA_FLAG_SATA_RESET |
-				  ATA_FLAG_NO_LEGACY;
+	probe_ent->host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  SVIA_FORCE_SATA_RESET;
 	probe_ent->port_ops	= &svia_sata_ops;
 	probe_ent->n_ports	= N_PORTS;
 	probe_ent->irq		= pdev->irq;
-- 
1.3.2


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

* Re: [PATCH] sata_via: convert to new EH, take #2
  2006-06-13 11:51 [PATCH] sata_via: convert to new EH Tejun Heo
@ 2006-06-13 21:46 ` Tejun Heo
  2006-06-16  6:13   ` [PATCH] sata_via: convert to new EH, take #3 Tejun Heo
  0 siblings, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2006-06-13 21:46 UTC (permalink / raw)
  To: Jeff Garzik, linux-ide

Convert sata_via to new EH.  The driver used SRST for vt6420 and sata
hardreset for vt6421.  This patch preserves the behavior using private
port flag SVIA_FORCE_SATA_RESET and private error handler.

Once it's determined that vt6421 can do SRST, the flag and private
error handler can be replaced with stock bmdma EH.

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

---

freeze/thaw() added.  Again, only compile tested.

Thanks.

 drivers/scsi/sata_via.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)

e3a255c119c63f938a1481c1e2fcd1db8a406383
diff --git a/drivers/scsi/sata_via.c b/drivers/scsi/sata_via.c
index c6975c5..dc487b7 100644
--- a/drivers/scsi/sata_via.c
+++ b/drivers/scsi/sata_via.c
@@ -69,11 +69,15 @@ enum {
 
 	SATA_EXT_PHY		= (1 << 6), /* 0==use PATA, 1==ext phy */
 	SATA_2DEV		= (1 << 5), /* SATA is master/slave */
+
+	/* private host flags */
+	SVIA_FORCE_SATA_RESET	= (1 << 24),
 };
 
 static int svia_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
 static u32 svia_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void svia_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
+static void svia_error_handler (struct ata_port *ap);
 
 static const struct pci_device_id svia_pci_tbl[] = {
 	{ 0x1106, 0x3149, PCI_ANY_ID, PCI_ANY_ID, 0, 0, vt6420 },
@@ -116,8 +120,6 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
 
-	.phy_reset		= sata_phy_reset,
-
 	.bmdma_setup            = ata_bmdma_setup,
 	.bmdma_start            = ata_bmdma_start,
 	.bmdma_stop		= ata_bmdma_stop,
@@ -127,7 +129,10 @@ static const struct ata_port_operations 
 	.qc_issue		= ata_qc_issue_prot,
 	.data_xfer		= ata_pio_data_xfer,
 
-	.eng_timeout		= ata_eng_timeout,
+	.freeze			= ata_bmdma_freeze,
+	.thaw			= ata_bmdma_thaw,
+	.error_handler		= svia_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
 	.irq_handler		= ata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
@@ -142,7 +147,7 @@ static const struct ata_port_operations 
 
 static struct ata_port_info svia_port_info = {
 	.sht		= &svia_sht,
-	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST | ATA_FLAG_NO_LEGACY,
+	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
 	.pio_mask	= 0x1f,
 	.mwdma_mask	= 0x07,
 	.udma_mask	= 0x7f,
@@ -169,6 +174,17 @@ static void svia_scr_write (struct ata_p
 	outl(val, ap->ioaddr.scr_addr + (4 * sc_reg));
 }
 
+static void svia_error_handler (struct ata_port *ap)
+{
+	ata_reset_fn_t softreset = ata_std_softreset;
+
+	if (ap->flags & SVIA_FORCE_SATA_RESET)
+		softreset = NULL;
+
+	ata_bmdma_drive_eh(ap, ata_std_prereset, softreset, sata_std_hardreset,
+			   ata_std_postreset);
+}
+
 static const unsigned int svia_bar_sizes[] = {
 	8, 4, 8, 4, 16, 256
 };
@@ -237,8 +253,8 @@ static struct ata_probe_ent *vt6421_init
 	INIT_LIST_HEAD(&probe_ent->node);
 
 	probe_ent->sht		= &svia_sht;
-	probe_ent->host_flags	= ATA_FLAG_SATA | ATA_FLAG_SATA_RESET |
-				  ATA_FLAG_NO_LEGACY;
+	probe_ent->host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  SVIA_FORCE_SATA_RESET;
 	probe_ent->port_ops	= &svia_sata_ops;
 	probe_ent->n_ports	= N_PORTS;
 	probe_ent->irq		= pdev->irq;
-- 
1.3.2


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

* [PATCH] sata_via: convert to new EH, take #3
  2006-06-13 21:46 ` [PATCH] sata_via: convert to new EH, take #2 Tejun Heo
@ 2006-06-16  6:13   ` Tejun Heo
  2006-06-20  9:16     ` Jeff Garzik
  0 siblings, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2006-06-16  6:13 UTC (permalink / raw)
  To: Jeff Garzik, linux-ide

Convert sata_via to new EH.  vt6420 used ATA_FLAG_SRST while vt6421
used ATA_FLAG_SATA_RESET.  This difference seems to be an accident
rather than intended.  This patch makes both flavors use
ata_bmdma_error_handler() which makes use of both SRST and SATA
hardreset.  This behavior change is intended and if it breaks
anything, it should be very easy to spot.

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

---

 drivers/scsi/sata_via.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

8b4fa8a307bc348a0c20f8c6e6ec75292c13f052
diff --git a/drivers/scsi/sata_via.c b/drivers/scsi/sata_via.c
index c6975c5..322890b 100644
--- a/drivers/scsi/sata_via.c
+++ b/drivers/scsi/sata_via.c
@@ -116,8 +116,6 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.dev_select		= ata_std_dev_select,
 
-	.phy_reset		= sata_phy_reset,
-
 	.bmdma_setup            = ata_bmdma_setup,
 	.bmdma_start            = ata_bmdma_start,
 	.bmdma_stop		= ata_bmdma_stop,
@@ -127,7 +125,10 @@ static const struct ata_port_operations 
 	.qc_issue		= ata_qc_issue_prot,
 	.data_xfer		= ata_pio_data_xfer,
 
-	.eng_timeout		= ata_eng_timeout,
+	.freeze			= ata_bmdma_freeze,
+	.thaw			= ata_bmdma_thaw,
+	.error_handler		= ata_bmdma_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
 	.irq_handler		= ata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
@@ -142,7 +143,7 @@ static const struct ata_port_operations 
 
 static struct ata_port_info svia_port_info = {
 	.sht		= &svia_sht,
-	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_SRST | ATA_FLAG_NO_LEGACY,
+	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
 	.pio_mask	= 0x1f,
 	.mwdma_mask	= 0x07,
 	.udma_mask	= 0x7f,
@@ -237,8 +238,7 @@ static struct ata_probe_ent *vt6421_init
 	INIT_LIST_HEAD(&probe_ent->node);
 
 	probe_ent->sht		= &svia_sht;
-	probe_ent->host_flags	= ATA_FLAG_SATA | ATA_FLAG_SATA_RESET |
-				  ATA_FLAG_NO_LEGACY;
+	probe_ent->host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY;
 	probe_ent->port_ops	= &svia_sata_ops;
 	probe_ent->n_ports	= N_PORTS;
 	probe_ent->irq		= pdev->irq;
-- 
1.3.2


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

* Re: [PATCH] sata_via: convert to new EH, take #3
  2006-06-16  6:13   ` [PATCH] sata_via: convert to new EH, take #3 Tejun Heo
@ 2006-06-20  9:16     ` Jeff Garzik
  0 siblings, 0 replies; 4+ messages in thread
From: Jeff Garzik @ 2006-06-20  9:16 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide

Tejun Heo wrote:
> Convert sata_via to new EH.  vt6420 used ATA_FLAG_SRST while vt6421
> used ATA_FLAG_SATA_RESET.  This difference seems to be an accident
> rather than intended.  This patch makes both flavors use
> ata_bmdma_error_handler() which makes use of both SRST and SATA
> hardreset.  This behavior change is intended and if it breaks
> anything, it should be very easy to spot.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

Applied.  Addressing the "rather than intended" comment above, the 
history is actually this:

Throughout libata, I have always preferred SATA PHY reset to SRST. 
SATA_RESET didn't work well on one VTxxxx chip, while it did work on 
another.  Thus, the driver reflected what worked.

While we may need to disable COMRESET completely on vt6420 (look for 
sata_via bug reports here!), I would prefer to shove both chips down the 
EH path that you have presented.  If that doesn't work, we'll take 
further action from here.

	Jeff




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

end of thread, other threads:[~2006-06-20  9:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-06-13 11:51 [PATCH] sata_via: convert to new EH Tejun Heo
2006-06-13 21:46 ` [PATCH] sata_via: convert to new EH, take #2 Tejun Heo
2006-06-16  6:13   ` [PATCH] sata_via: convert to new EH, take #3 Tejun Heo
2006-06-20  9:16     ` Jeff Garzik

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