Messages in this thread | | | Date | Mon, 20 Mar 2006 20:53:01 -0500 | From | Jeff Garzik <> | Subject | Re: [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/
| |