[PATCH 2.6.22] Add LED trigger to libata core

[PATCH 2.6.22] Add LED trigger to libata core

[Tony Vroon]

This duplicates the IDE core LED trigger in the libata core.
I plan to use this by allowing PMU LED control on G5 towers. My test platform 
is a PowerMac 7,3 (Dual G5 2.0GHz, June 2004) with a K2 (sata_svw) controller.

Signed-off-by: Tony Vroon <chainsaw@gentoo.org>

--- linux-2.6/drivers/ata/libata-scsi.c.orig	2007-03-17 22:53:56.000000000 +0000
+++ linux-2.6/drivers/ata/libata-scsi.c	2007-03-17 22:53:15.000000000 +0000
@@ -46,6 +46,7 @@
 #include <linux/libata.h>
 #include <linux/hdreg.h>
 #include <asm/uaccess.h>
+#include <linux/leds.h>
 
 #include "libata.h"
 
@@ -2791,6 +2792,7 @@
 				      struct ata_device *dev)
 {
 	int rc = 0;
+	ledtrig_ide_activity();
 
 	if (unlikely(!scmd->cmd_len)) {
 		ata_dev_printk(dev, KERN_WARNING, "WARNING: zero len CDB\n");

Re: [PATCH 2.6.22] Add LED trigger to libata core

[Tejun Heo]

Tony Vroon wrote:
> This duplicates the IDE core LED trigger in the libata core.
> I plan to use this by allowing PMU LED control on G5 towers. My test platform 
> is a PowerMac 7,3 (Dual G5 2.0GHz, June 2004) with a K2 (sata_svw) controller.

I think this fits better in libata-core.c::ata_qc_issue().  Can you move
it to there?

-- 
tejun

Re: [PATCH 2.6.22] Add LED trigger to libata core

[Alan Cox]

On Mon, 19 Mar 2007 13:42:37 +0900
Tejun Heo <htejun@gmail.com> wrote:

> Tony Vroon wrote:
> > This duplicates the IDE core LED trigger in the libata core.
> > I plan to use this by allowing PMU LED control on G5 towers. My test platform 
> > is a PowerMac 7,3 (Dual G5 2.0GHz, June 2004) with a K2 (sata_svw) controller.
> 
> I think this fits better in libata-core.c::ata_qc_issue().  Can you move
> it to there?

Gak. I'd rather it stayed out of ata_qc_issue() which is a critical path
for performance. Our command issu is already too heavy and not all
controllers have queueing to absorb that. How many controllers actually
need this hook and can we not have ata_qc_issue_with_led() helpers for
them ?

Re: [PATCH 2.6.22] Add LED trigger to libata core

[Tony Vroon]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Alan Cox wrote:
> Gak. I'd rather it stayed out of ata_qc_issue() which is a critical path
> for performance. Our command issu is already too heavy and not all
> controllers have queueing to absorb that. How many controllers actually
> need this hook and can we not have ata_qc_issue_with_led() helpers for
> them ?
We're talking about just sata_svw (Apple K2) right now. I'll have a go
at implementing it conditionally like you suggested. Consider the
patch withdrawn for now.

Regards,
Tony V.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFF/n5Dp5vW4rUFj5oRCJbzAJ4+rkkqdA21If03WLFT6jquX+np+QCgwSGX
UV41rt4ZimQvf0kXLwkuFqo=
=Ar6p
-----END PGP SIGNATURE-----

Re: [PATCH 2.6.22] Add LED trigger to libata core

[Tejun Heo]

Alan Cox wrote:
> On Mon, 19 Mar 2007 13:42:37 +0900
> Tejun Heo <htejun@gmail.com> wrote:
> 
>> Tony Vroon wrote:
>>> This duplicates the IDE core LED trigger in the libata core.
>>> I plan to use this by allowing PMU LED control on G5 towers. My test platform 
>>> is a PowerMac 7,3 (Dual G5 2.0GHz, June 2004) with a K2 (sata_svw) controller.
>> I think this fits better in libata-core.c::ata_qc_issue().  Can you move
>> it to there?
> 
> Gak. I'd rather it stayed out of ata_qc_issue() which is a critical path
> for performance.

The original place is in the critical path too.  It's just at the outer 
function which eventually calls ata_qc_issue() (the mapping is almost 
one to one).

> Our command issu is already too heavy and not all
> controllers have queueing to absorb that. How many controllers actually
> need this hook and can we not have ata_qc_issue_with_led() helpers for
> them ?

Our issue path is somewhat expensive due to SCSI -> ATA translation but 
I don't think it really matters on any modern cpu.  It can definitely 
hurt on embedded tho.  :-(

-- 
tejun

Re: [PATCH 2.6.22] Add LED trigger to libata core

[Mikael Pettersson]

On Mon, 19 Mar 2007 12:46:16 +0000, Alan Cox wrote:
> > > This duplicates the IDE core LED trigger in the libata core.
> > > I plan to use this by allowing PMU LED control on G5 towers. My test platform 
> > > is a PowerMac 7,3 (Dual G5 2.0GHz, June 2004) with a K2 (sata_svw) controller.
> > 
> > I think this fits better in libata-core.c::ata_qc_issue().  Can you move
> > it to there?
> 
> Gak. I'd rather it stayed out of ata_qc_issue() which is a critical path
> for performance. Our command issu is already too heavy and not all
> controllers have queueing to absorb that. How many controllers actually
> need this hook and can we not have ata_qc_issue_with_led() helpers for
> them ?

Agreed, toggling a led for each request seems excessive.

At least Promise controllers tend to have a HW-driven
activity led header, so wouldn't need SW-triggered
activity leds except maybe if some stupid system failed
to wire up the led header.

/Mikael