Re: SATA DVD-Drive

Nathan Butcher wrote:
> I think somebody determined that there has been some regression here,
> because the JMicron controller works on FreeBSD 6.2 apparently.
> That counts as two SATA chipsets I've noticed regression in with
> 7.0-CURRENT (Jmicron and Promise SATA 150/300 TX4), which I really hope
> get fixed before 7.0-RELEASE.

Hi, All.

Seems only an AHCI controllers affected by this problem. I think the
regression was introduced in the rev. 1.187 of ata-chipset.c:
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/ata/ata-chipset.c.diff?r1=1.186;r2=1.187
But the cause of problem is not here. I think kern/111699 related to
this problem too.

The problem in the port signature reading. Before this commit we have
the following code:

if ((ATA_IDX_INB(ch, ATA_CYL_LSB) == ATAPI_MAGIC_LSB) &&
     (ATA_IDX_INB(ch, ATA_CYL_MSB) == ATAPI_MAGIC_MSB))
     ch->devices = ATA_ATAPI_MASTER;
else
     ch->devices = ATA_ATA_MASTER;

It works in the FreeBSD-6.x. Now we have:

switch (ATA_INL(ctlr->r_res2, ATA_AHCI_P_SIG + offset)) {
case 0xeb140101:
     ch->devices = ATA_ATAPI_MASTER;
     device_printf(ch->dev, "SATA ATAPI devices not supported yet\n");
     ch->devices = 0;
     break;
case 0x96690101:
     ch->devices = ATA_PORTMULTIPLIER;
     device_printf(ch->dev, "Portmultipliers not supported yet\n");
     ch->devices = 0;
     break;
case 0x00000101:
     ch->devices = ATA_ATA_MASTER;
     break;
}

And it don't work. The returned signature is 0xFFFFFFFF.
As a workaround we can add the
default:
     ch->devices = ATA_ATA_MASTER;

This code will be similar to 6.x code. But i think this is not right.
May be someone can test the attached patch?

Patch try to read port signature after starting AHCI engine,
like linux do. Maybe this will help..

PS. Sorry, i can't this patch, because i don't have working AHCI
controller.

-- 
WBR, Andrey V. Elsukov

--- src/sys/dev/ata/ata-chipset.c.orig	2007-09-10 23:16:39.000000000 +0400
+++ src/sys/dev/ata/ata-chipset.c	2007-09-25 09:25:47.000000000 +0400
_at__at_ -678,7 +678,7 @@ ata_ahci_reset(device_t dev)
     struct ata_channel *ch = device_get_softc(dev);
     u_int32_t cmd;
     int offset = ch->unit << 7;
-    int timeout;
+    int timeout, ret;
 
     if (!(ATA_INL(ctlr->r_res2, ATA_AHCI_PI) & (1 << ch->unit))) {
 	device_printf(dev, "port not implemented\n");
_at__at_ -720,8 +720,23 _at__at_ ata_ahci_reset(device_t dev)
     ATA_OUTL(ctlr->r_res2, ATA_AHCI_P_CMD + offset, ATA_AHCI_P_CMD_SUD);
 
     /* enable interface */
-    if (ata_sata_phy_reset(dev)) {
-	switch (ATA_INL(ctlr->r_res2, ATA_AHCI_P_SIG + offset)) {
+    ret = ata_sata_phy_reset(dev);
+
+    /* clear any interrupts pending on this channel */
+    ATA_OUTL(ctlr->r_res2, ATA_AHCI_P_IS + offset,
+	     ATA_INL(ctlr->r_res2, ATA_AHCI_P_IS + offset));
+
+    /* start operations on this channel */
+    ATA_OUTL(ctlr->r_res2, ATA_AHCI_P_CMD + offset,
+	     (ATA_AHCI_P_CMD_ACTIVE | ATA_AHCI_P_CMD_FRE |
+	      ATA_AHCI_P_CMD_POD | ATA_AHCI_P_CMD_SUD | ATA_AHCI_P_CMD_ST));
+
+    /* Read port signature */
+    if (ret) {
+	cmd = ATA_INL(ctlr->r_res2, ATA_AHCI_P_SIG + offset);
+	if (bootverbose)
+	    device_printf(ch->dev, "Port signature: 0x%08x\n", cmd);
+	switch (cmd) {
 	case 0xeb140101:
 	    ch->devices = ATA_ATAPI_MASTER;
 	    device_printf(ch->dev, "SATA ATAPI devices not supported yet\n");
_at__at_ -737,15 +752,6 _at__at_ ata_ahci_reset(device_t dev)
 	    break;
 	}
     }
-
-    /* clear any interrupts pending on this channel */
-    ATA_OUTL(ctlr->r_res2, ATA_AHCI_P_IS + offset,
-	     ATA_INL(ctlr->r_res2, ATA_AHCI_P_IS + offset));
-
-    /* start operations on this channel */
-    ATA_OUTL(ctlr->r_res2, ATA_AHCI_P_CMD + offset,
-	     (ATA_AHCI_P_CMD_ACTIVE | ATA_AHCI_P_CMD_FRE |
-	      ATA_AHCI_P_CMD_POD | ATA_AHCI_P_CMD_SUD | ATA_AHCI_P_CMD_ST));
 }
 
 static void