[PATCH] AHCI SATA vendor update from VIA

[PATCH] AHCI SATA vendor update from VIA

Post by Jeff Garzi » Tue, 13 Dec 2005 13:20:14


This is a multi-part message in MIME format.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
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
 
 
 

[PATCH] AHCI SATA vendor update from VIA

Post by Sergey Vla » Fri, 17 Mar 2006 01:20:15


To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
On Sun, 11 Dec 2005 23:09:54 -0500 Jeff Garzik wrote:


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?

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFEGD4jW82GfkQfsqIRAo3PAJ0euP/7MgHDbBRQcAYR+Pm9kuLyCACfYV3e
ImpJLywsjZRq+Nl645oO/lE=
=CL7c
-----END PGP SIGNATURE-----


 
 
 

[PATCH] AHCI SATA vendor update from VIA

Post by Ph. Mare » Fri, 17 Mar 2006 15:10:05

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[PATCH] AHCI SATA vendor update from VIA

Post by Ph. Mare » Sat, 18 Mar 2006 21:10:28


I should probably have given the URL for the patch, too:
http://www.yqcomputer.com/


Regards,

Phil
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[PATCH] AHCI SATA vendor update from VIA

Post by Geoff Rive » Sat, 18 Mar 2006 22:30:18

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/



Or go to http://www.yqcomputer.com/

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

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)

iD8DBQBEGrObSP1WOObttzQRAmZhAJ0dQ4KMDxpVXcvY1JOpNoWKfQXEsACdECyB
eXGSF9cByfVJqMLNJALc024=
=4539
-----END PGP SIGNATURE-----
 
 
 

[PATCH] AHCI SATA vendor update from VIA

Post by Jeff Garzi » Wed, 22 Mar 2006 11:00:13

ergey Vlasov wrote:

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.



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).



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



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

Jeff



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 
 
 

[PATCH] AHCI SATA vendor update from VIA

Post by Tejun He » Wed, 22 Mar 2006 20:30:05

Hello,




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?


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://www.yqcomputer.com/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/
 
 
 

[PATCH] AHCI SATA vendor update from VIA

Post by Jeff Garzi » Thu, 23 Mar 2006 01:50:08


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



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to XXXX@XXXXX.COM
More majordomo info at http://www.yqcomputer.com/
Please read the FAQ at http://www.yqcomputer.com/