linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] AHCI SATA vendor update from VIA
@ 2005-12-12  4:09 Jeff Garzik
  2006-03-15 16:17 ` Sergey Vlasov
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Garzik @ 2005-12-12  4:09 UTC (permalink / raw)
  To: linux-ide@vger.kernel.org; +Cc: Linux Kernel

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

I received this ahci.c patch from VIA, and pass it on for review, 
comment, and testing.

This patch won't go in as-is, because it does too much special casing. 
But until we get around to that, people with VIA controllers probably 
want this...

	Jeff



[-- Attachment #2: ahci.c.patch --]
[-- Type: text/plain, Size: 8994 bytes --]

--- ahci.c.orig	2005-11-21 14:24:48.000000000 +0800
+++ ahci.c	2005-11-21 14:25:35.000000000 +0800
@@ -59,6 +59,7 @@
 	RX_FIS_D2H_REG		= 0x40,	/* offset of D2H Register FIS data */
 
 	board_ahci		= 0,
+	board_via_ahci		= 1,
 
 	/* global controller registers */
 	HOST_CAP		= 0x00, /* host capabilities */
@@ -126,6 +127,7 @@
 	PORT_CMD_LIST_ON	= (1 << 15), /* cmd list DMA engine running */
 	PORT_CMD_FIS_ON		= (1 << 14), /* FIS DMA engine running */
 	PORT_CMD_FIS_RX		= (1 << 4), /* Enable FIS receive DMA engine */
+	PORT_CMD_CLO		= (1 << 3), /* CLO */
 	PORT_CMD_POWER_ON	= (1 << 2), /* Power up device */
 	PORT_CMD_SPIN_UP	= (1 << 1), /* Spin up device */
 	PORT_CMD_START		= (1 << 0), /* Enable port DMA engine */
@@ -183,6 +185,14 @@
 static u8 ahci_check_err(struct ata_port *ap);
 static inline int ahci_host_intr(struct ata_port *ap, struct ata_queued_cmd *qc);
 
+static int via_ahci_qc_issue(struct ata_queued_cmd *qc);
+static void via_ahci_phy_reset(struct ata_port *ap);
+static void via_ahci_port_stop(struct ata_port *ap);
+static int via_ahci_softreset(struct ata_port *ap);
+static unsigned int via_ata_busy_sleep (struct ata_port *ap,
+				    unsigned long tmout_pat,
+			    	    unsigned long tmout);
+
 static Scsi_Host_Template ahci_sht = {
 	.module			= THIS_MODULE,
 	.name			= DRV_NAME,
@@ -231,6 +241,35 @@
 	.host_stop		= ahci_host_stop,
 };
 
+static struct ata_port_operations via_ahci_ops = {
+	.port_disable		= ata_port_disable,
+
+	.check_status		= ahci_check_status,
+	.check_altstatus	= ahci_check_status,
+	.check_err		= ahci_check_err,
+	.dev_select		= ata_noop_dev_select,
+
+	.tf_read		= ahci_tf_read,
+
+
+	.qc_prep		= ahci_qc_prep,
+
+	.eng_timeout		= ahci_eng_timeout,
+
+	.irq_handler		= ahci_interrupt,
+	.irq_clear		= ahci_irq_clear,
+
+	.scr_read		= ahci_scr_read,
+	.scr_write		= ahci_scr_write,
+
+	.port_start		= ahci_port_start,
+	.host_stop		= ahci_host_stop,
+
+	.phy_reset		= via_ahci_phy_reset,
+	.qc_issue		= via_ahci_qc_issue,
+	.port_stop		= via_ahci_port_stop,
+};
+
 static struct ata_port_info ahci_port_info[] = {
 	/* board_ahci */
 	{
@@ -242,6 +281,16 @@
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
 		.port_ops	= &ahci_ops,
 	},
+	/* board_via_ahci */
+	{
+		.sht		= &ahci_sht,
+		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_SRST | ATA_FLAG_MMIO |
+				  ATA_FLAG_PIO_DMA,
+		.pio_mask	= 0x03, /* pio3-4 */
+		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
+		.port_ops	= &via_ahci_ops,
+	},
 };
 
 static struct pci_device_id ahci_pci_tbl[] = {
@@ -263,6 +312,8 @@
 	  board_ahci }, /* ESB2 */
 	{ PCI_VENDOR_ID_INTEL, 0x2683, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 	  board_ahci }, /* ESB2 */
+	{ PCI_VENDOR_ID_VIA, 0x3349, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
+	  board_via_ahci }, /* VT8251*/
 	{ }	/* terminate list */
 };
 
@@ -1049,6 +1100,244 @@
 	return rc;
 }
 
+/* START: patch code for VIA VT8251 ahci controller */
+
+static int via_ahci_softreset(struct ata_port *ap)
+{
+	void __iomem *port_mmio = (void __iomem *) ap->ioaddr.cmd_addr;
+	struct ahci_port_priv *pp = ap->private_data;
+	u32 tmp,i;
+	u8 *fisbuf;
+
+	VPRINTK("ENTER\n");
+
+	writel(0x00000000, port_mmio + PORT_IRQ_MASK);  /*disable interrupt */
+	readl (port_mmio + PORT_IRQ_MASK);  /* flush */
+
+	/* prepare the software-reset commands */
+
+	/* prepare first command header */
+	memset(pp->cmd_slot, 0, 32);
+	pp->cmd_slot[0].opts = 0x00000505;
+	pp->cmd_slot[0].status = 0;
+	pp->cmd_slot[0].tbl_addr = cpu_to_le32(pp->cmd_tbl_dma & 0xffffffff);
+	pp->cmd_slot[0].tbl_addr_hi = cpu_to_le32((pp->cmd_tbl_dma >> 16) >> 16);
+
+	/* prepare CMDFIS struct */
+	fisbuf = pp->cmd_tbl;
+	memset(fisbuf, 0, 64);
+	fisbuf[0] = 0x27;
+	fisbuf[7] = 0xa0;
+	fisbuf[15] = 0x04;
+	
+	/* trigger the commands */
+	writel(0x1, port_mmio + PORT_CMD_ISSUE);
+	readl (port_mmio + PORT_CMD_ISSUE); /* flush */
+
+	udelay(20);
+    /* wait command complete */
+	for (i = 0; i < 2000; i++) {
+		tmp = readl(port_mmio + PORT_CMD_ISSUE);
+		if ((tmp & 1) == 0) break;
+		msleep(20);
+	}
+
+	if (i == 2000) {
+		printk(KERN_WARNING "TimeOut for Wait Command complete\n");
+		return 1;
+	}
+
+	/* prepare second command header */
+	pp->cmd_slot[0].opts = 0x00000005;
+	pp->cmd_slot[0].status = 0;
+	pp->cmd_slot[0].tbl_addr = cpu_to_le32(pp->cmd_tbl_dma & 0xffffffff);
+	pp->cmd_slot[0].tbl_addr_hi = cpu_to_le32((pp->cmd_tbl_dma >> 16) >> 16);
+
+	fisbuf = pp->cmd_tbl;
+	memset(fisbuf, 0, 64);
+	fisbuf[0] = 0x27;
+	fisbuf[7] = 0xa0;
+	fisbuf[15] = 0x00;
+
+	/* trigger the commands */
+	writel(0x1, port_mmio + PORT_CMD_ISSUE);
+	readl (port_mmio + PORT_CMD_ISSUE); /* flush */
+
+	udelay(20);
+    /* wait command complete */
+	for (i = 0; i < 2000; i++) {
+		tmp = readl(port_mmio + PORT_CMD_ISSUE);
+		if ((tmp & 1) == 0) break;
+		msleep(20);
+	}
+
+	if (i == 2000) {
+		printk(KERN_WARNING "TimeOut for Wait Command complete\n");
+		return 1;
+	}
+
+	// enable port interrupt
+	writel(0xffffffff, port_mmio + PORT_IRQ_MASK);
+	readl (port_mmio + PORT_IRQ_MASK);  /* flush */
+
+	return 0;
+}
+
+static unsigned int via_ata_busy_sleep (struct ata_port *ap,
+				    unsigned long tmout_pat,
+			    	    unsigned long tmout)
+{
+	unsigned long timer_start, timeout;
+	u8 status;
+
+	status = ata_busy_wait(ap, ATA_BUSY, 300);
+	timer_start = jiffies;
+	timeout = timer_start + tmout_pat;
+	while ((status & ATA_BUSY) && (time_before(jiffies, timeout))) {
+		msleep(50);
+		status = ata_busy_wait(ap, ATA_BUSY, 3);
+	}
+
+	if (status & ATA_BUSY)
+		printk(KERN_WARNING "ata%u is slow to respond, "
+		       "please be patient\n", ap->id);
+
+	timeout = timer_start + tmout;
+	while ((status & ATA_BUSY) && (time_before(jiffies, timeout))) {
+		msleep(50);
+		status = ata_chk_status(ap);
+	}
+
+	if (status & ATA_BUSY) {
+		printk(KERN_ERR "ata%u failed to respond (%lu secs)\n",
+		       ap->id, tmout / HZ);
+		return 1;
+	}
+
+	return 0;
+}
+
+static void via_ahci_phy_reset(struct ata_port *ap)
+{
+	void __iomem *port_mmio = (void __iomem *) ap->ioaddr.cmd_addr;
+	struct ata_taskfile tf;
+	struct ata_device *dev = &ap->device[0];
+	u32 tmp;
+
+	u32 sstatus,i;
+	unsigned long timeout = jiffies + (HZ * 5);
+	u8 tmp_status;
+	
+	if (ap->flags & ATA_FLAG_SATA_RESET) {
+		/* issue phy wake/reset */
+		scr_write_flush(ap, SCR_CONTROL, 0x301);
+		udelay(400);			/* FIXME: a guess */
+	}
+	scr_write_flush(ap, SCR_CONTROL, 0x300); /* phy wake/clear reset */
+
+	/* wait for phy to become ready, if necessary */
+	do {
+		msleep(200);
+		sstatus = scr_read(ap, SCR_STATUS);
+		if ((sstatus & 0xf) != 1)
+			break;
+	} while (time_before(jiffies, timeout));
+
+	/* TODO: phy layer with polling, timeouts, etc. */
+	if (sata_dev_present(ap))
+		ata_port_probe(ap);
+	else {
+		sstatus = scr_read(ap, SCR_STATUS);
+		printk(KERN_INFO "ata%u: no device found (phy stat %08x)\n",
+		       ap->id, sstatus);
+		ata_port_disable(ap);
+	}
+
+	if (ap->flags & ATA_FLAG_PORT_DISABLED)
+		return;
+
+	/*Fix the VIA busy bug by a software reset*/
+	for (i = 0; i < 100; i++) {
+		tmp_status = ap->ops->check_status(ap);
+		if ((tmp_status & ATA_BUSY) == 0) break;
+		msleep(10);
+	}
+
+	if ((tmp_status & ATA_BUSY)) {
+		DPRINTK("Busy after CommReset, do softreset...\n"); 
+		/*set the PxCMD.CLO bit to clear BUSY and DRQ, to make the reset command sent out*/
+		tmp = readl(port_mmio + PORT_CMD);
+		tmp |= PORT_CMD_CLO;
+		writel(tmp, port_mmio + PORT_CMD);
+		readl(port_mmio + PORT_CMD); /* flush */
+
+		if (via_ahci_softreset(ap)) {
+			printk(KERN_WARNING "softreset failed\n");
+			return;
+		}
+	}
+
+	if (via_ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
+		ata_port_disable(ap);
+		return;
+	}
+
+	ap->cbl = ATA_CBL_SATA;
+
+	if (ap->flags & ATA_FLAG_PORT_DISABLED)
+		return;
+
+	tmp = readl(port_mmio + PORT_SIG);
+	tf.lbah		= (tmp >> 24)	& 0xff;
+	tf.lbam		= (tmp >> 16)	& 0xff;
+	tf.lbal		= (tmp >> 8)	& 0xff;
+	tf.nsect	= (tmp)		& 0xff;
+
+	dev->class = ata_dev_classify(&tf);
+	if (!ata_dev_present(dev))
+		ata_port_disable(ap);
+
+}
+
+static int via_ahci_qc_issue(struct ata_queued_cmd *qc)
+{
+	struct ata_port *ap = qc->ap;
+	void *port_mmio = (void *) ap->ioaddr.cmd_addr;
+
+    if (qc &&
+        qc->tf.command == ATA_CMD_SET_FEATURES &&
+        qc->tf.feature == SETFEATURES_XFER) {
+        /* skip set xfer mode process */
+
+		ata_qc_complete(qc, 0);
+		qc = NULL;
+        return 0;
+    }
+
+	writel(1, port_mmio + PORT_CMD_ISSUE);
+	readl(port_mmio + PORT_CMD_ISSUE);	/* flush */
+
+	return 0;
+}
+
+static void via_ahci_port_stop(struct ata_port *ap)
+{
+	struct device *dev = ap->host_set->dev;
+	struct ahci_port_priv *pp = ap->private_data;
+
+	/* spec says 500 msecs for each PORT_CMD_{START,FIS_RX} bit, so
+	 * this is slightly incorrect.
+	 */
+	msleep(500);
+
+	ap->private_data = NULL;
+	dma_free_coherent(dev, AHCI_PORT_PRIV_DMA_SZ,
+			  pp->cmd_slot, pp->cmd_slot_dma);
+	kfree(pp);
+	ata_port_stop(ap);
+}
+
+/* END: patch code for VIA VT8251 ahci controller */
 
 static int __init ahci_init(void)
 {

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

* Re: [PATCH] AHCI SATA vendor update from VIA
  2005-12-12  4:09 Jeff Garzik
@ 2006-03-15 16:17 ` Sergey Vlasov
  2006-03-21  1:53   ` Jeff Garzik
  0 siblings, 1 reply; 8+ messages in thread
From: Sergey Vlasov @ 2006-03-15 16:17 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Geoff Rivell, linux-ide, linux-kernel

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

On Sun, 11 Dec 2005 23:09:54 -0500 Jeff Garzik wrote:

> I received this ahci.c patch from VIA, and pass it on for review, 
> comment, and testing.
> 
> This patch won't go in as-is, because it does too much special casing. 
> But until we get around to that, people with VIA controllers probably 
> want this...
> 
> 	Jeff

What is needed to get the VT8251 support into the kernel tree?

I have looked at the patch, and it basically does three things:

1) Apparently the VT8251 hardware does not like the standard reset
   sequence performed by __sata_phy_reset() - the phy seems to become
   ready, but the ATA_BUSY bit never goes off.  So the patch authors
   just duplicated ahci_phy_reset(), inserted the whole code of
   __sata_phy_reset() in there, and added this part before the
   ata_busy_sleep() call:

+	/*Fix the VIA busy bug by a software reset*/
+	for (i = 0; i < 100; i++) {
+		tmp_status = ap->ops->check_status(ap);
+		if ((tmp_status & ATA_BUSY) == 0) break;
+		msleep(10);
+	}
+
+	if ((tmp_status & ATA_BUSY)) {
+		DPRINTK("Busy after CommReset, do softreset...\n"); 
+		/*set the PxCMD.CLO bit to clear BUSY and DRQ, to make the reset command sent out*/
+		tmp = readl(port_mmio + PORT_CMD);
+		tmp |= PORT_CMD_CLO;
+		writel(tmp, port_mmio + PORT_CMD);
+		readl(port_mmio + PORT_CMD); /* flush */
+
+		if (via_ahci_softreset(ap)) {
+			printk(KERN_WARNING "softreset failed\n");
+			return;
+		}
+	}

   Now, if this is really a chip bug, we don't have any choice except
   adding this workaround, but obviously not in this way.  What do you
   think about splitting __sata_phy_reset() in two parts -
   __sata_phy_reset_start() (everything up to the point where
   ata_busy_sleep() is called) and __sata_phy_reset_end()
   (ata_busy_sleep() and the rest), so that the low-level driver could
   insert its own code between these parts?  Or should a hook for this
   be added to ->ops instead?

2) via_ahci_qc_issue really just filters out the SETFEATURES_XFER
   command; only VIA can tell why this is needed, and is there a better
   way to do this.  However, at least some duplicated code could be
   removed easily:

static int via_ahci_qc_issue(struct ata_queued_cmd *qc)
{
	if (qc && qc->tf.command == ATA_CMD_SET_FEATURES &&
	    qc->tf.feature == SETFEATURES_XFER) {
		/* skip set xfer mode process */
		ata_qc_complete(qc);
		return 0;
	}
	return ahci_qc_issue(qc);
}

   Would this be acceptable?

3) What via_ahci_port_stop() does, I just don't understand - it is
   basically a copy of ahci_port_stop() at that time, but with clearing
   of the PORT_CMD bits removed - so nothing is stopped actually.
   Again, only VIA can tell why is this needed, but this part of the
   patch looks like a bug.

Geoff Rivell (CC'ed) tried to update the patch for 2.6.15:

http://grivell.home.comcast.net/via_ahci.patch

However, that patch does some things in a different order from the VIA
code - it calls via_ahci_softreset() before __sata_phy_reset(), which
does not seem safe to me.  Geoff, does this really work?

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

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

* re: [PATCH] AHCI SATA vendor update from VIA
@ 2006-03-16  6:07 Ph. Marek
  2006-03-17 12:08 ` Ph. Marek
  0 siblings, 1 reply; 8+ messages in thread
From: Ph. Marek @ 2006-03-16  6:07 UTC (permalink / raw)
  To: jgarzik; +Cc: linux-ide, linux-kernel

Hello Jeff,


I'd like to confirm that the patch in your email from 2005-12-11 does indeed 
work for my vt8251 on an K8M800 motherboard.

It didn't apply cleanly to 2.6.15; I had to fix two rejects, and then comment 
out the two lines with
	.host_stop =
and
	.check_err =
to make it compile.


I'd like to ask you if that could have negative effects, and when the driver 
(part) will be included in the standard kernel.


Thank you for this patch. I'm looking forward to your answer.


Regards,

Phil

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

* Re: [PATCH] AHCI SATA vendor update from VIA
  2006-03-16  6:07 [PATCH] AHCI SATA vendor update from VIA Ph. Marek
@ 2006-03-17 12:08 ` Ph. Marek
  2006-03-17 13:03   ` Geoff Rivell
  0 siblings, 1 reply; 8+ messages in thread
From: Ph. Marek @ 2006-03-17 12:08 UTC (permalink / raw)
  To: jgarzik; +Cc: linux-ide, linux-kernel

On Thursday 16 March 2006 07:07, Ph. Marek wrote:
> I'd like to confirm that the patch in your email from 2005-12-11 does
> indeed work for my vt8251 on an K8M800 motherboard.
I should probably have given the URL for the patch, too:
	http://lkml.org/lkml/2005/12/11/204


Regards,

Phil

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

* Re: [PATCH] AHCI SATA vendor update from VIA
  2006-03-17 12:08 ` Ph. Marek
@ 2006-03-17 13:03   ` Geoff Rivell
  0 siblings, 0 replies; 8+ messages in thread
From: Geoff Rivell @ 2006-03-17 13:03 UTC (permalink / raw)
  To: Ph. Marek; +Cc: jgarzik, linux-ide, linux-kernel

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

On Friday 17 March 2006 07:08, Ph. Marek wrote:
> On Thursday 16 March 2006 07:07, Ph. Marek wrote:
> > I'd like to confirm that the patch in your email from 2005-12-11 does
> > indeed work for my vt8251 on an K8M800 motherboard.
>
> I should probably have given the URL for the patch, too:
> 	http://lkml.org/lkml/2005/12/11/204

Or go to http://grivell.home.comcast.net/ and get the 2.6.15 version.

-- 
...."Have you mooed today?"...

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

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

* Re: [PATCH] AHCI SATA vendor update from VIA
  2006-03-15 16:17 ` Sergey Vlasov
@ 2006-03-21  1:53   ` Jeff Garzik
  2006-03-21 11:19     ` Tejun Heo
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Garzik @ 2006-03-21  1:53 UTC (permalink / raw)
  To: Sergey Vlasov; +Cc: Geoff Rivell, linux-ide, linux-kernel, Tejun Heo

Sergey Vlasov wrote:
> What is needed to get the VT8251 support into the kernel tree?

1) Doing what you are doing:  asking questions like this.  :)

2) Watching Tejun Heo's reset work.  He already has an AHCI soft reset 
patch, and the VIA AHCI work really depends on this.


> I have looked at the patch, and it basically does three things:
> 
> 1) Apparently the VT8251 hardware does not like the standard reset
>    sequence performed by __sata_phy_reset() - the phy seems to become
>    ready, but the ATA_BUSY bit never goes off.  So the patch authors
>    just duplicated ahci_phy_reset(), inserted the whole code of
>    __sata_phy_reset() in there, and added this part before the
>    ata_busy_sleep() call:
> 
> +	/*Fix the VIA busy bug by a software reset*/
> +	for (i = 0; i < 100; i++) {
> +		tmp_status = ap->ops->check_status(ap);
> +		if ((tmp_status & ATA_BUSY) == 0) break;
> +		msleep(10);
> +	}
> +
> +	if ((tmp_status & ATA_BUSY)) {
> +		DPRINTK("Busy after CommReset, do softreset...\n"); 
> +		/*set the PxCMD.CLO bit to clear BUSY and DRQ, to make the reset command sent out*/
> +		tmp = readl(port_mmio + PORT_CMD);
> +		tmp |= PORT_CMD_CLO;
> +		writel(tmp, port_mmio + PORT_CMD);
> +		readl(port_mmio + PORT_CMD); /* flush */
> +
> +		if (via_ahci_softreset(ap)) {
> +			printk(KERN_WARNING "softreset failed\n");
> +			return;
> +		}
> +	}
> 
>    Now, if this is really a chip bug, we don't have any choice except
>    adding this workaround, but obviously not in this way.  What do you
>    think about splitting __sata_phy_reset() in two parts -
>    __sata_phy_reset_start() (everything up to the point where
>    ata_busy_sleep() is called) and __sata_phy_reset_end()
>    (ata_busy_sleep() and the rest), so that the low-level driver could
>    insert its own code between these parts?  Or should a hook for this
>    be added to ->ops instead?

Tejun's stuff broke up this sequence, so it should be much easier to 
utilize his new reset code (in libata-dev.git#upstream, queued for 2.6.17).


> 2) via_ahci_qc_issue really just filters out the SETFEATURES_XFER
>    command; only VIA can tell why this is needed, and is there a better
>    way to do this.  However, at least some duplicated code could be
>    removed easily:
> 
> static int via_ahci_qc_issue(struct ata_queued_cmd *qc)
> {
> 	if (qc && qc->tf.command == ATA_CMD_SET_FEATURES &&
> 	    qc->tf.feature == SETFEATURES_XFER) {
> 		/* skip set xfer mode process */
> 		ata_qc_complete(qc);
> 		return 0;
> 	}
> 	return ahci_qc_issue(qc);
> }
> 
>    Would this be acceptable?

I wonder first if this actually solves some problems.  I would prefer to 
-not- do this, and see what happens.


> 3) What via_ahci_port_stop() does, I just don't understand - it is
>    basically a copy of ahci_port_stop() at that time, but with clearing
>    of the PORT_CMD bits removed - so nothing is stopped actually.
>    Again, only VIA can tell why is this needed, but this part of the
>    patch looks like a bug.

As your instinct seems to be, I would prefer to avoid this change if 
possible.

	Jeff




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

* Re: [PATCH] AHCI SATA vendor update from VIA
  2006-03-21  1:53   ` Jeff Garzik
@ 2006-03-21 11:19     ` Tejun Heo
  2006-03-21 16:39       ` Jeff Garzik
  0 siblings, 1 reply; 8+ messages in thread
From: Tejun Heo @ 2006-03-21 11:19 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Sergey Vlasov, Geoff Rivell, linux-ide, linux-kernel

Hello,

Jeff Garzik wrote:
> Sergey Vlasov wrote:
>> What is needed to get the VT8251 support into the kernel tree?
> 
> 1) Doing what you are doing:  asking questions like this.  :)
> 
> 2) Watching Tejun Heo's reset work.  He already has an AHCI soft reset 
> patch, and the VIA AHCI work really depends on this.
> 

BTW, what happened to AHCI softreset patch. It got acked[1], but it has 
not made into the tree yet. Do you want me to regenerate it against the 
current tree? Or is there anything holding it from going into the tree?

> 
>> I have looked at the patch, and it basically does three things:
>>
>> 1) Apparently the VT8251 hardware does not like the standard reset
>>    sequence performed by __sata_phy_reset() - the phy seems to become
>>    ready, but the ATA_BUSY bit never goes off.  So the patch authors
>>    just duplicated ahci_phy_reset(), inserted the whole code of
>>    __sata_phy_reset() in there, and added this part before the
>>    ata_busy_sleep() call:
>>
>> +    /*Fix the VIA busy bug by a software reset*/
>> +    for (i = 0; i < 100; i++) {
>> +        tmp_status = ap->ops->check_status(ap);
>> +        if ((tmp_status & ATA_BUSY) == 0) break;
>> +        msleep(10);
>> +    }
>> +
>> +    if ((tmp_status & ATA_BUSY)) {
>> +        DPRINTK("Busy after CommReset, do softreset...\n"); +        
>> /*set the PxCMD.CLO bit to clear BUSY and DRQ, to make the reset 
>> command sent out*/
>> +        tmp = readl(port_mmio + PORT_CMD);
>> +        tmp |= PORT_CMD_CLO;
>> +        writel(tmp, port_mmio + PORT_CMD);
>> +        readl(port_mmio + PORT_CMD); /* flush */
>> +
>> +        if (via_ahci_softreset(ap)) {
>> +            printk(KERN_WARNING "softreset failed\n");
>> +            return;
>> +        }
>> +    }
>>
>>    Now, if this is really a chip bug, we don't have any choice except
>>    adding this workaround, but obviously not in this way.  What do you
>>    think about splitting __sata_phy_reset() in two parts -
>>    __sata_phy_reset_start() (everything up to the point where
>>    ata_busy_sleep() is called) and __sata_phy_reset_end()
>>    (ata_busy_sleep() and the rest), so that the low-level driver could
>>    insert its own code between these parts?  Or should a hook for this
>>    be added to ->ops instead?
> 
> Tejun's stuff broke up this sequence, so it should be much easier to 
> utilize his new reset code (in libata-dev.git#upstream, queued for 2.6.17).
> 

If hardreset never works on via ahci, simply omitting hardreset in 
ahci_probe_reset() routine for that controller should do the job (with 
the AHCI softreset patch applied of course).

-- 
tejun

[1] http://thread.gmane.org/gmane.linux.ide/7952

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

* Re: [PATCH] AHCI SATA vendor update from VIA
  2006-03-21 11:19     ` Tejun Heo
@ 2006-03-21 16:39       ` Jeff Garzik
  0 siblings, 0 replies; 8+ messages in thread
From: Jeff Garzik @ 2006-03-21 16:39 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Sergey Vlasov, Geoff Rivell, linux-ide, linux-kernel

Tejun Heo wrote:
> Hello,
> 
> Jeff Garzik wrote:
> 
>> Sergey Vlasov wrote:
>>
>>> What is needed to get the VT8251 support into the kernel tree?
>>
>>
>> 1) Doing what you are doing:  asking questions like this.  :)
>>
>> 2) Watching Tejun Heo's reset work.  He already has an AHCI soft reset 
>> patch, and the VIA AHCI work really depends on this.
>>
> 
> BTW, what happened to AHCI softreset patch. It got acked[1], but it has 
> not made into the tree yet. Do you want me to regenerate it against the 
> current tree? Or is there anything holding it from going into the tree?

Please resend, the only pending patch I have from you is the ATA 
transport class patch (thanks for doing that BTW), which is on hold 
waiting for SCSI updates.

	Jeff




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

end of thread, other threads:[~2006-03-21 16:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-16  6:07 [PATCH] AHCI SATA vendor update from VIA Ph. Marek
2006-03-17 12:08 ` Ph. Marek
2006-03-17 13:03   ` Geoff Rivell
  -- strict thread matches above, loose matches on Subject: below --
2005-12-12  4:09 Jeff Garzik
2006-03-15 16:17 ` Sergey Vlasov
2006-03-21  1:53   ` Jeff Garzik
2006-03-21 11:19     ` Tejun Heo
2006-03-21 16:39       ` 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).