Re: Seeking advice on "monkey patching" a driver

From: Ian Pilcher <arequipeno@gmail.com>
To: Greg KH <greg@kroah.com>
Cc: linux-modules@vger.kernel.org, kernelnewbies@kernelnewbies.org
Subject: Re: Seeking advice on "monkey patching" a driver
Date: Fri, 2 Jul 2021 07:05:26 -0500	[thread overview]
Message-ID: <5c34419f-ef7d-a9bd-0c90-c77e2526b739@gmail.com> (raw)
In-Reply-To: <YN6Wml2QT2ZfppA2@kroah.com>

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!

-- 
========================================================================
                  In Soviet Russia, Google searches you!
========================================================================