lkml.org 
[lkml]   [2006]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] AHCI SATA vendor update from VIA
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



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

\
 
 \ /
  Last update: 2006-03-21 02:55    [W:0.115 / U:0.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site