Re: [PATCH RFC 0/4] leds: extend disk trigger

[Markus Probst <markus.probst@posteo.de>]

[-- Attachment #1: Type: text/plain, Size: 3154 bytes --]

On Mon, 2026-01-26 at 10:00 +0100, Niklas Cassel wrote:
> Hello Markus,
> 
> On Fri, Jan 23, 2026 at 07:05:03PM +0000, Markus Probst wrote:
> > Extend the disk trigger
> > - to allow configuration of the blinking delays
> >   and whether the led should be kept on, on idle.
> > - to allow an individual led to be mapped to an ata port
> > 
> > I would also like to add another patch to this series, only leaving the led
> > on with invert 1 if also at least one disk is present on the ata port.
> > The led would then not only indicate activity, but also if a disk is
> > present.
> > That is why it is an RFC.
> > 
> > @Damien,Niclas: What would be the most straightforward way of telling
> > the led trigger if at least one disk is present on the ata port and
> > notifing it when this changes?
> 
> Why do we want to have this in kernel space?
Because there are more than enough devices that could make use of it.

Just search the term "NAS device" and you see rarely any devices for
which this wouldn't be useful.

The only reason the leds work on those devices currently, is because
they get shipped with a custom modified kernel by the manufacturer.
This shouldn't be a requirement for running Linux properly on a NAS
device with disk leds.


> Sure, there is already the very simple ledtrig-disk driver.
> 
> But I'm not a fan of making the driver more complex.
Do you mean the complexity it would introduce in libata or for the led
trigger itself?

At least with the current patches it looks fairly maintainable.
For instance the pattern led trigger is more complex in my opinion.

In the case of libata and the indication for a presence of a disk, I
would suggest that I implement it first and we can see after I have a
working version if it is acceptable or not.

I am still asking for guidance on checking if at least one disk is
present on a ata port.

> If we want something more complex than what is already there, then it
> is probably much better handled in user space, considering the amount
> of possible configuration options.
A userspace daemon by itself is possible, but I don't think it is the
best solution. Having an indicator for disk activity on a per-disk
basis seems like basic led functionality that should be present in the
kernel.

It is a very minor detail, but I would prefer to have "linux,default-
trigger" set on the led in the fwnode and having the functionality
automatically for every linux system on the hardware, instead of having
to deal with a userspace daemon.
If this is the easiest solution for nas manufacturer to do disk leds,
there is a good chance it getting adopted some day in the future by
those manufacturer and thus making it work out of the box when
switching away from their proprietary os.

> 
> Basically the same argument as used in:
> https://lore.kernel.org/linux-nvme/20220227234258.24619-1-ematsumiya@suse.de/T/#u
If I understood it corretly, the argument there is that led code
shouldn't be present in a fast path.

This does not apply to this scenario.

Thanks
- Markus Probst

> 
> 
> Kind regards,
> Niklas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]