Re: Seeking advice on "monkey patching" a driver

Ian Pilcher
2021-07-02T12:05:53Z

On 7/1/21 11:31 PM, Greg KH wrote:
> Why are ahci devices somehow "special" here? Just add a trigger to the
> ahci core for LEDs and all should "just work". We've done that for many
> subsystems already.

It's more complicated than that, as it would need to be a separate
trigger for each drive (ATA port).

> Are you sure we don't already have LED triggers for disk activity? Have
> you tried the ledtrig-disk.c driver? It says it works on ATA devices,
> no reason it can't also work for other device types.

I stumbled on that file myself last night. I either wasn't aware of it
before, or I had forgotten its existence, possibly because it's disabled
(CONFIG_LEDS_TRIGGER_DISK=n). As mentioned above, I would be looking to
enable "per port" LEDs, so it doesn't work in its current form. It does
at least give me hope that enhancements might be accepted upstream.

>> I've invested significant time in kernel patches in the past, only to
>> see them ultimately not be accepted, so I would need to know that
>> upstream was truly interested in such a feature before I would consider
>> making such a commitment.
>
> That's not fair, there is no way anyone can promise anyone that their
> patches will be accepted, _before_ anyone sees them. What would _you_
> do if you were in the kernel maintainer's position and read something
> like this?

You're right, but that isn't what I intended to say. Basically, I can't
afford to invest the time in implementing something if the subsystem
maintainers have no interest in the *functionality*, regardless of the
state of the code. I.e., if the ATA/LED subsystem maintainers think
that software-controlled disk activity LEDs are stupid and have no
place in the kernel, then code quality is irrelevant, and anything I do
will be a waste of time.

> good luck!

Thanks!