ahci, SActive flag, and the HD activity LED

Martin Wilck
2005-08-02T15:44:11Z

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

Hello Jeff, hello Jens, hello everybody,

I am referring to the debate about whether or not setting the SActive
bit for non-NCQ ATA commands (e.g. http://lkml.org/lkml/2005/5/26/142).

In our machines, this behavior of the Linux AHCI driver causes the HD
activity LED to stay on all the time. If I apply the attached trivial
patch (this is for the RedHat EL4.0-U1 kernel), the LED behaves nicely.

Jeff has stated in the above thread that "SActive is intentionally used
for non-NCQ devices". However I find clear indication in the specs that
the SActive flag should be set if and only if tagged queuing is being
used, and only for a specified subset of commands that support queuing
(http://www.t13.org/docs2005/D1699r1e-ATA8-ACS.pdf, secs. 4.19 and
4.20). The current mainline driver doesn't use queuing.

If I am reading the specs correctly, that'd mean the ahci driver is
wrong in setting the SActive bit. Could you please comment? Jeff, in
particular, could you please give more detail why you say this flag is
"intentionally used"?

Regards
Martin

--
Martin Wilck Phone: +49 5251 8 15113
Fujitsu Siemens Computers Fax: +49 5251 8 20409
Heinz-Nixdorf-Ring 1 mailto:Martin.Wilck@Fujitsu-Siemens.com
D-33106 Paderborn http://www.fujitsu-siemens.com/primergy

[-- Attachment #2: ahci_sactive.diff --]
[-- Type: text/x-patch, Size: 383 bytes --]

--- ahci.c.orig 2005-06-13 11:39:26.000000000 +0200
+++ ahci.c 2005-08-02 10:48:47.000000000 +0200
@@ -691,9 +703,6 @@
struct ata_port *ap = qc->ap;
void *port_mmio = (void *) ap->ioaddr.cmd_addr;

- writel(1, port_mmio + PORT_SCR_ACT);
- readl(port_mmio + PORT_SCR_ACT); /* flush */
-
writel(1, port_mmio + PORT_CMD_ISSUE);
readl(port_mmio + PORT_CMD_ISSUE); /* flush */