Re: libata PM question

[Robert Hancock <hancockrwd@gmail.com>]

On 11/29/2009 09:42 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2009-11-30 at 13:25 +1100, Benjamin Herrenschmidt wrote:
>> On Mon, 2009-11-30 at 13:22 +1100, Benjamin Herrenschmidt wrote:
>>> Hi there !
>>>
>>> So I (finally !) resumed work on pata_macio.c to replace ide/pmac.c :-)
>>>
>>> I've started looking at the suspend/resume stuff. It looks like most of
>>> it just comes for free thanks to libata, yay ! :-)
>>
>>   .../...
>>
>> Oh, another question... where do we spin the disk down&  park it on
>> suspend ? I'm missed at least that part :-)
>
> Oh and finally, one last question for today ...
>
> In drivers/ide, we have a call to ledtrig_ide_activity() inside
> ide_do_rw_disk() which allows me to flash the front light on
> the powerbook.
>
> libata has no such thing.
>
> It's as simple as adding that call, if we want to keep the trigger named
> "ide-disk" but where to put it then ? In ata_scsi_rw_xlat() ? Yuck :-)
>
> Another option is to stick it in drivers/scsi/sd.c but the trigger
> should not be named ide-disk anymore ... Also it will start tiggering
> for any "pseudo scsi" device in the machine which is note quite the
> purpose... It would be nice if it really only triggered on libata and as
> such blink the led only for the internal HD on those powerbooks (unless
> you have some libata based PCMCIA stuff but that doesn't matter).
>
> Anyway, nothing fancy, just a quick hack to get the activity led
> back :-) I could always stick it somewhere in pata_macio.c but that
> would suck probably even more.

Looks like the only libata driver that currently does LED blinking is 
ahci (for external enclosures) and it's just handled there using 
qc_issue plus a timer. Most drivers don't need this and given that you 
need to do it only for specific devices and only your driver has that 
knowledge, I don't see much problem with just sticking the LED trigger 
call into your qc_issue or something similar. No sense in trying to make 
something generic if it's not generic..