Re: sata_sil 3112 activity LED patch

[Jeff Garzik <jgarzik@pobox.com>]

Aric Cyr wrote:
> After finally getting fed up with not having my activity light working
> for my SATA drives, I came up with a small patch (more like hack) to
> make it work.  It works quite well, but I'm afraid that there are many
> restriction that this patch does not check for that it probably
> should... so consider this a work-in-progress.  My information is
> based on a document from Silicon Image that appears to no longer be
> available on their website (Sil-AN-0082-E; 8112-0082.pdf).  I still
> have a copy if anyone is interested.
> 
> There are two restrictions that are not checked:
> 
> 1) Is the chip a 3112 or 3114?  I assume that this would only work on
>    a 3112, but whether it is "a bad thing" on a 3114 I do not know.
> 
> 2) BAR5 + 0x54 is apparently used for the flash memory address and
>    data lines.  However for most motherboards (i.e. not add-on cards)
>    with the chip, like my EPOX 8RDA3+, there is no flash memory, so
>    these lines are hijacked as LED GPIO.  I assume that this is a
>    common practice for motherboard makers using the sil3112 since
>    Silicon Image went out of their way to produce the above mentioned
>    document.  Anyways, the problem is that this patch does not check
>    if flash memory is installed or not before twiddling with the GPIO
>    lines.  This could be extremely bad for people running the 3112
>    from add-on cards (or any implementation with flash memory
>    installed).
> 
> Setting the low 8bits at BAR5+54h seems to enable the LED circuit.  It
> seems that this circuit is patched through into the motherboard as it
> lights the regular hard drive light on the front of my case.  Setting
> bits [8:15] at BAR5+54h clears the bits, disabling the LED.  I hooked
> this logic into the ata_bmdma_start and ata_bmdma_stop which were made
> into simple wrapper functions in sata_sil.c that just set the GPIO
> bits and calls ata_bmdma_*.

I don't think its ugly, necessarily.  I do worry about the flash memory 
stuff, though, which is why I don't want to merge this upstream for now.

For your patch specifically, it would be nice to follow the coding style 
that is found in the rest of the driver (single-tab indents, etc., read 
CodingStyle in kernel source tree).

	Jeff