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

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

[Markus Probst]

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?

Thanks
- Markus Probst

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
Markus Probst (4):
      leds: dt-bindings: add disk trigger led pattern
      leds: dt-bindings: add disk trigger for each ata port
      leds: add delay_on, delay_off and invert attributes to disk trigger
      leds: add disk trigger for each ata port

 Documentation/devicetree/bindings/leds/common.yaml |  10 +-
 drivers/ata/libata-core.c                          |  22 +-
 drivers/leds/trigger/ledtrig-disk.c                | 298 ++++++++++++++++++++-
 drivers/scsi/libsas/sas_ata.c                      |   3 +-
 include/linux/leds.h                               |  16 +-
 include/linux/libata.h                             |   6 +-
 6 files changed, 332 insertions(+), 23 deletions(-)
---
base-commit: 24d479d26b25bce5faea3ddd9fa8f3a6c3129ea7
change-id: 20260123-ledtrig_disk_-1a500531865a

[PATCH RFC 1/4] leds: dt-bindings: add disk trigger led pattern

[Markus Probst]

Document the disk trigger led pattern.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 Documentation/devicetree/bindings/leds/common.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index f4e44b33f56d..d0f2fee7622c 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -144,6 +144,8 @@ properties:
       Each trigger may parse this property differently:
         - one-shot : two numbers specifying delay on and delay off (in ms),
         - timer : two numbers specifying delay on and delay off (in ms),
+        - disk : three numbers specifying delay on, delay off (in ms)
+          and invert (0 or 1),
         - pattern : the pattern is given by a series of tuples, of
           brightness and duration (in ms).  The exact format is
           described in:
@@ -151,7 +153,7 @@ properties:
     $ref: /schemas/types.yaml#/definitions/uint32-matrix
     items:
       minItems: 2
-      maxItems: 2
+      maxItems: 3
 
   led-max-microamp:
     description:

-- 
2.52.0

[PATCH RFC 2/4] leds: dt-bindings: add disk trigger for each ata port

[Markus Probst]

Document disk trigger showing only disk activity for one specific ata
port.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 Documentation/devicetree/bindings/leds/common.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/common.yaml b/Documentation/devicetree/bindings/leds/common.yaml
index d0f2fee7622c..ca51eeadfad0 100644
--- a/Documentation/devicetree/bindings/leds/common.yaml
+++ b/Documentation/devicetree/bindings/leds/common.yaml
@@ -136,6 +136,12 @@ properties:
       - pattern: "^mmc[0-9]+$"
         # LED is triggered by WLAN activity
       - pattern: "^phy[0-9]+tx$"
+        # LED indicates disk activity for a specific ata port
+      - pattern: "^.*-ata[0-9]+-disk-activity$"
+        # LED indicates disk read activity for a specific ata port
+      - pattern: "^.*-ata[0-9]+-disk-read$"
+        # LED indicates disk write activity for a specific ata port
+      - pattern: "^.*-ata[0-9]+-disk-write$"
 
   led-pattern:
     description: |

-- 
2.52.0

[PATCH RFC 3/4] leds: add delay_on, delay_off and invert attributes to disk trigger

[Markus Probst]

Add delay_on, delay_off and invert device attributes to leds using the
disk trigger.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/leds/trigger/ledtrig-disk.c | 194 +++++++++++++++++++++++++++++++++---
 1 file changed, 182 insertions(+), 12 deletions(-)

diff --git a/drivers/leds/trigger/ledtrig-disk.c b/drivers/leds/trigger/ledtrig-disk.c
index e9b87ee944f2..ed5ef83a5b35 100644
--- a/drivers/leds/trigger/ledtrig-disk.c
+++ b/drivers/leds/trigger/ledtrig-disk.c
@@ -9,31 +9,201 @@
 
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/list.h>
 #include <linux/leds.h>
+#include "../leds.h"
 
-#define BLINK_DELAY 30
+#define DEFAULT_BLINK_DELAY 30
 
-DEFINE_LED_TRIGGER(ledtrig_disk);
-DEFINE_LED_TRIGGER(ledtrig_disk_read);
-DEFINE_LED_TRIGGER(ledtrig_disk_write);
+struct ledtrig_disk_data {
+	unsigned long delay_on;
+	unsigned long delay_off;
+	unsigned int invert;
+};
+
+static ssize_t led_delay_on_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = led_trigger_get_led(dev);
+	struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
+
+	return sprintf(buf, "%lu\n", disk_data->delay_on);
+}
+
+static ssize_t led_delay_on_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = led_trigger_get_led(dev);
+	struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
+	unsigned long state;
+	ssize_t ret;
+
+	ret = kstrtoul(buf, 10, &state);
+	if (ret)
+		return ret;
+
+	disk_data->delay_on = state;
+
+	return size;
+}
+
+static ssize_t led_delay_off_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = led_trigger_get_led(dev);
+	struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
+
+	return sprintf(buf, "%lu\n", disk_data->delay_off);
+}
+
+static ssize_t led_delay_off_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = led_trigger_get_led(dev);
+	struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
+	unsigned long state;
+	ssize_t ret;
+
+	ret = kstrtoul(buf, 10, &state);
+	if (ret)
+		return ret;
+
+	disk_data->delay_off = state;
+
+	return size;
+}
+
+static ssize_t led_invert_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct ledtrig_disk_data *disk_data =
+		led_trigger_get_drvdata(dev);
+
+	return sprintf(buf, "%u\n", disk_data->invert);
+}
+
+static ssize_t led_invert_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = led_trigger_get_led(dev);
+	struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
+	unsigned long state;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &state);
+	if (ret)
+		return ret;
+
+	led_set_brightness_nosleep(led_cdev, state ? LED_FULL : LED_OFF);
+	disk_data->invert = !!state;
+
+	return size;
+}
+
+static DEVICE_ATTR(delay_on, 0644, led_delay_on_show, led_delay_on_store);
+static DEVICE_ATTR(delay_off, 0644, led_delay_off_show, led_delay_off_store);
+static DEVICE_ATTR(invert, 0644, led_invert_show, led_invert_store);
+
+static struct attribute *ledtrig_disk_attrs[] = {
+	&dev_attr_delay_on.attr,
+	&dev_attr_delay_off.attr,
+	&dev_attr_invert.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(ledtrig_disk);
+
+static void pattern_init(struct led_classdev *led_cdev, struct ledtrig_disk_data *disk_data)
+{
+	unsigned int size = 0;
+
+	u32 *pattern __free(kfree) = led_get_default_pattern(led_cdev, &size);
+	if (!pattern)
+		return;
+
+	if (size != 3) {
+		dev_warn(led_cdev->dev,
+			 "Expected 3 but got %u values for delays + invert pattern\n",
+			 size);
+		return;
+	}
+
+	disk_data->delay_on = pattern[0];
+	disk_data->delay_off = pattern[1];
+	disk_data->invert = !!pattern[2];
+}
+
+static int ledtrig_disk_activate(struct led_classdev *led_cdev)
+{
+	struct ledtrig_disk_data *disk_data;
+
+	disk_data = kzalloc(sizeof(*disk_data), GFP_KERNEL);
+	if (!disk_data)
+		return -ENOMEM;
+
+	disk_data->delay_on = DEFAULT_BLINK_DELAY;
+	disk_data->delay_off = DEFAULT_BLINK_DELAY;
+
+	led_set_trigger_data(led_cdev, disk_data);
+
+	if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
+		pattern_init(led_cdev, disk_data);
+		/*
+		 * Mark as initialized even on pattern_init() error because
+		 * any consecutive call to it would produce the same error.
+		 */
+		led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
+	}
+
+	led_set_brightness_nosleep(led_cdev, disk_data->invert ? LED_FULL : LED_OFF);
+
+	return 0;
+}
+
+static struct led_trigger ledtrig_disk = {
+	.name = "disk-activity",
+	.activate = ledtrig_disk_activate,
+	.groups = ledtrig_disk_groups,
+};
+static struct led_trigger ledtrig_disk_read = {
+	.name = "disk-read",
+	.activate = ledtrig_disk_activate,
+	.groups = ledtrig_disk_groups,
+};
+static struct led_trigger ledtrig_disk_write = {
+	.name = "disk-write",
+	.activate = ledtrig_disk_activate,
+	.groups = ledtrig_disk_groups,
+};
+
+static void ledtrig_disk_blink_oneshot(struct led_trigger *trig)
+{
+	struct led_classdev *led_cdev;
+	struct ledtrig_disk_data *disk_data;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(led_cdev, &trig->led_cdevs, trig_list) {
+		disk_data = led_get_trigger_data(led_cdev);
+		led_blink_set_oneshot(led_cdev, &disk_data->delay_on, &disk_data->delay_off,
+				      disk_data->invert);
+	}
+	rcu_read_unlock();
+}
 
 void ledtrig_disk_activity(bool write)
 {
-	led_trigger_blink_oneshot(ledtrig_disk, BLINK_DELAY, BLINK_DELAY, 0);
+	ledtrig_disk_blink_oneshot(&ledtrig_disk);
 	if (write)
-		led_trigger_blink_oneshot(ledtrig_disk_write,
-					  BLINK_DELAY, BLINK_DELAY, 0);
+		ledtrig_disk_blink_oneshot(&ledtrig_disk_write);
 	else
-		led_trigger_blink_oneshot(ledtrig_disk_read,
-					  BLINK_DELAY, BLINK_DELAY, 0);
+		ledtrig_disk_blink_oneshot(&ledtrig_disk_read);
 }
 EXPORT_SYMBOL(ledtrig_disk_activity);
 
 static int __init ledtrig_disk_init(void)
 {
-	led_trigger_register_simple("disk-activity", &ledtrig_disk);
-	led_trigger_register_simple("disk-read", &ledtrig_disk_read);
-	led_trigger_register_simple("disk-write", &ledtrig_disk_write);
+	led_trigger_register(&ledtrig_disk);
+	led_trigger_register(&ledtrig_disk_read);
+	led_trigger_register(&ledtrig_disk_write);
 
 	return 0;
 }

-- 
2.52.0

Re: [PATCH RFC 3/4] leds: add delay_on, delay_off and invert attributes to disk trigger

[Markus Probst]

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

On Fri, 2026-01-23 at 19:05 +0000, Markus Probst wrote:
> Add delay_on, delay_off and invert device attributes to leds using the
> disk trigger.
> 
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  drivers/leds/trigger/ledtrig-disk.c | 194 +++++++++++++++++++++++++++++++++---
>  1 file changed, 182 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/leds/trigger/ledtrig-disk.c b/drivers/leds/trigger/ledtrig-disk.c
> index e9b87ee944f2..ed5ef83a5b35 100644
> --- a/drivers/leds/trigger/ledtrig-disk.c
> +++ b/drivers/leds/trigger/ledtrig-disk.c
> @@ -9,31 +9,201 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/init.h>
> +#include <linux/list.h>
>  #include <linux/leds.h>
> +#include "../leds.h"
>  
> -#define BLINK_DELAY 30
> +#define DEFAULT_BLINK_DELAY 30
>  
> -DEFINE_LED_TRIGGER(ledtrig_disk);
> -DEFINE_LED_TRIGGER(ledtrig_disk_read);
> -DEFINE_LED_TRIGGER(ledtrig_disk_write);
> +struct ledtrig_disk_data {
> +	unsigned long delay_on;
> +	unsigned long delay_off;
> +	unsigned int invert;
> +};
> +
> +static ssize_t led_delay_on_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = led_trigger_get_led(dev);
> +	struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
> +
> +	return sprintf(buf, "%lu\n", disk_data->delay_on);
> +}
> +
> +static ssize_t led_delay_on_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t size)
> +{
> +	struct led_classdev *led_cdev = led_trigger_get_led(dev);
> +	struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
> +	unsigned long state;
> +	ssize_t ret;
> +
> +	ret = kstrtoul(buf, 10, &state);
> +	if (ret)
> +		return ret;
> +
> +	disk_data->delay_on = state;
> +
> +	return size;
> +}
> +
> +static ssize_t led_delay_off_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = led_trigger_get_led(dev);
> +	struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
> +
> +	return sprintf(buf, "%lu\n", disk_data->delay_off);
> +}
> +
> +static ssize_t led_delay_off_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t size)
> +{
> +	struct led_classdev *led_cdev = led_trigger_get_led(dev);
> +	struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
> +	unsigned long state;
> +	ssize_t ret;
> +
> +	ret = kstrtoul(buf, 10, &state);
> +	if (ret)
> +		return ret;
> +
> +	disk_data->delay_off = state;
> +
> +	return size;
> +}
> +
> +static ssize_t led_invert_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct ledtrig_disk_data *disk_data =
> +		led_trigger_get_drvdata(dev);
> +
> +	return sprintf(buf, "%u\n", disk_data->invert);
> +}
> +
> +static ssize_t led_invert_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t size)
> +{
> +	struct led_classdev *led_cdev = led_trigger_get_led(dev);
> +	struct ledtrig_disk_data *disk_data = led_get_trigger_data(led_cdev);
> +	unsigned long state;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &state);
> +	if (ret)
> +		return ret;
> +
> +	led_set_brightness_nosleep(led_cdev, state ? LED_FULL : LED_OFF);
> +	disk_data->invert = !!state;
> +
> +	return size;
> +}
> +
> +static DEVICE_ATTR(delay_on, 0644, led_delay_on_show, led_delay_on_store);
> +static DEVICE_ATTR(delay_off, 0644, led_delay_off_show, led_delay_off_store);
> +static DEVICE_ATTR(invert, 0644, led_invert_show, led_invert_store);
> +
> +static struct attribute *ledtrig_disk_attrs[] = {
> +	&dev_attr_delay_on.attr,
> +	&dev_attr_delay_off.attr,
> +	&dev_attr_invert.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(ledtrig_disk);
> +
> +static void pattern_init(struct led_classdev *led_cdev, struct ledtrig_disk_data *disk_data)
> +{
> +	unsigned int size = 0;
> +
> +	u32 *pattern __free(kfree) = led_get_default_pattern(led_cdev, &size);
> +	if (!pattern)
> +		return;
> +
> +	if (size != 3) {
> +		dev_warn(led_cdev->dev,
> +			 "Expected 3 but got %u values for delays + invert pattern\n",
> +			 size);
> +		return;
> +	}
> +
> +	disk_data->delay_on = pattern[0];
> +	disk_data->delay_off = pattern[1];
> +	disk_data->invert = !!pattern[2];
> +}
> +
> +static int ledtrig_disk_activate(struct led_classdev *led_cdev)
> +{
> +	struct ledtrig_disk_data *disk_data;
> +
> +	disk_data = kzalloc(sizeof(*disk_data), GFP_KERNEL);
> +	if (!disk_data)
> +		return -ENOMEM;
> +
> +	disk_data->delay_on = DEFAULT_BLINK_DELAY;
> +	disk_data->delay_off = DEFAULT_BLINK_DELAY;
> +
> +	led_set_trigger_data(led_cdev, disk_data);
> +
> +	if (led_cdev->flags & LED_INIT_DEFAULT_TRIGGER) {
> +		pattern_init(led_cdev, disk_data);
> +		/*
> +		 * Mark as initialized even on pattern_init() error because
> +		 * any consecutive call to it would produce the same error.
> +		 */
> +		led_cdev->flags &= ~LED_INIT_DEFAULT_TRIGGER;
> +	}
> +
> +	led_set_brightness_nosleep(led_cdev, disk_data->invert ? LED_FULL : LED_OFF);
> +
> +	return 0;
> +}
> +
> +static struct led_trigger ledtrig_disk = {
> +	.name = "disk-activity",
> +	.activate = ledtrig_disk_activate,
> +	.groups = ledtrig_disk_groups,
> +};
> +static struct led_trigger ledtrig_disk_read = {
> +	.name = "disk-read",
> +	.activate = ledtrig_disk_activate,
> +	.groups = ledtrig_disk_groups,
> +};
> +static struct led_trigger ledtrig_disk_write = {
> +	.name = "disk-write",
> +	.activate = ledtrig_disk_activate,
> +	.groups = ledtrig_disk_groups,
> +};
> +
> +static void ledtrig_disk_blink_oneshot(struct led_trigger *trig)
> +{
> +	struct led_classdev *led_cdev;
> +	struct ledtrig_disk_data *disk_data;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(led_cdev, &trig->led_cdevs, trig_list) {
> +		disk_data = led_get_trigger_data(led_cdev);
Also there is likely a race condition here, as there is a time gap
between the led being added to trig->led_cdevs and
ledtrig_disk_activate being run.

Is there a better way to avoid the race condition, than defining an own
list for each trigger?

Thanks
- Markus Probst

> +		led_blink_set_oneshot(led_cdev, &disk_data->delay_on, &disk_data->delay_off,
> +				      disk_data->invert);
> +	}
> +	rcu_read_unlock();
> +}
>  
>  void ledtrig_disk_activity(bool write)
>  {
> -	led_trigger_blink_oneshot(ledtrig_disk, BLINK_DELAY, BLINK_DELAY, 0);
> +	ledtrig_disk_blink_oneshot(&ledtrig_disk);
>  	if (write)
> -		led_trigger_blink_oneshot(ledtrig_disk_write,
> -					  BLINK_DELAY, BLINK_DELAY, 0);
> +		ledtrig_disk_blink_oneshot(&ledtrig_disk_write);
>  	else
> -		led_trigger_blink_oneshot(ledtrig_disk_read,
> -					  BLINK_DELAY, BLINK_DELAY, 0);
> +		ledtrig_disk_blink_oneshot(&ledtrig_disk_read);
>  }
>  EXPORT_SYMBOL(ledtrig_disk_activity);
>  
>  static int __init ledtrig_disk_init(void)
>  {
> -	led_trigger_register_simple("disk-activity", &ledtrig_disk);
> -	led_trigger_register_simple("disk-read", &ledtrig_disk_read);
> -	led_trigger_register_simple("disk-write", &ledtrig_disk_write);
> +	led_trigger_register(&ledtrig_disk);
> +	led_trigger_register(&ledtrig_disk_read);
> +	led_trigger_register(&ledtrig_disk_write);
>  
>  	return 0;
>  }

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

[PATCH RFC 4/4] leds: add disk trigger for each ata port

[Markus Probst]

Register a disk trigger for each ata port. This trigger will only show
the activity for the ata port it has been registered for.

This allows individual leds to be mapped to one ata port.
This is especially useful for NAS devices, which have an own led for each
disk slot.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/ata/libata-core.c           |  22 +++++-
 drivers/leds/trigger/ledtrig-disk.c | 144 ++++++++++++++++++++++++++++++------
 drivers/scsi/libsas/sas_ata.c       |   3 +-
 include/linux/leds.h                |  16 +++-
 include/linux/libata.h              |   6 +-
 5 files changed, 161 insertions(+), 30 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 09d8c035fcdf..796c46449298 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4921,8 +4921,10 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
 	struct ata_device *dev = qc->dev;
 	struct ata_eh_info *ehi = &dev->link->eh_info;
 
+#ifdef CONFIG_LEDS_TRIGGER_DISK
 	/* Trigger the LED (if available) */
-	ledtrig_disk_activity(!!(qc->tf.flags & ATA_TFLAG_WRITE));
+	ledtrig_disk_activity(ap->led_trigger, !!(qc->tf.flags & ATA_TFLAG_WRITE));
+#endif
 
 	/*
 	 * In order to synchronize EH with the regular execution path, a qc that
@@ -5538,10 +5540,13 @@ int sata_link_init_spd(struct ata_link *link)
  *	LOCKING:
  *	Inherited from calling layer (may sleep).
  */
-struct ata_port *ata_port_alloc(struct ata_host *host)
+struct ata_port *ata_port_alloc(struct ata_host *host, int port_no)
 {
 	struct ata_port *ap;
 	int id;
+#ifdef CONFIG_LEDS_TRIGGER_DISK
+	char name[32];
+#endif
 
 	ap = kzalloc(sizeof(*ap), GFP_KERNEL);
 	if (!ap)
@@ -5557,6 +5562,7 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
 	ap->print_id = id;
 	ap->host = host;
 	ap->dev = host->dev;
+	ap->port_no = port_no;
 
 	mutex_init(&ap->scsi_scan_mutex);
 	INIT_DELAYED_WORK(&ap->hotplug_task, ata_scsi_hotplug);
@@ -5579,6 +5585,11 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
 
 	ata_force_pflags(ap);
 
+#ifdef CONFIG_LEDS_TRIGGER_DISK
+	if (snprintf(name, sizeof(name), "%s-ata%d", dev_name(host->dev), port_no) < sizeof(name))
+		ap->led_trigger = ledtrig_disk_trigger_register(name);
+#endif
+
 	return ap;
 }
 EXPORT_SYMBOL_GPL(ata_port_alloc);
@@ -5588,6 +5599,10 @@ void ata_port_free(struct ata_port *ap)
 	if (!ap)
 		return;
 
+#ifdef CONFIG_LEDS_TRIGGER_DISK
+	ledtrig_disk_trigger_unregister(ap->led_trigger);
+#endif
+
 	kfree(ap->pmp_link);
 	kfree(ap->slave_link);
 	ida_free(&ata_ida, ap->print_id);
@@ -5690,11 +5705,10 @@ struct ata_host *ata_host_alloc(struct device *dev, int n_ports)
 	for (i = 0; i < n_ports; i++) {
 		struct ata_port *ap;
 
-		ap = ata_port_alloc(host);
+		ap = ata_port_alloc(host, i);
 		if (!ap)
 			goto err_out;
 
-		ap->port_no = i;
 		host->ports[i] = ap;
 	}
 
diff --git a/drivers/leds/trigger/ledtrig-disk.c b/drivers/leds/trigger/ledtrig-disk.c
index ed5ef83a5b35..fd25b4e45fb4 100644
--- a/drivers/leds/trigger/ledtrig-disk.c
+++ b/drivers/leds/trigger/ledtrig-disk.c
@@ -159,20 +159,22 @@ static int ledtrig_disk_activate(struct led_classdev *led_cdev)
 	return 0;
 }
 
-static struct led_trigger ledtrig_disk = {
-	.name = "disk-activity",
-	.activate = ledtrig_disk_activate,
-	.groups = ledtrig_disk_groups,
-};
-static struct led_trigger ledtrig_disk_read = {
-	.name = "disk-read",
-	.activate = ledtrig_disk_activate,
-	.groups = ledtrig_disk_groups,
-};
-static struct led_trigger ledtrig_disk_write = {
-	.name = "disk-write",
-	.activate = ledtrig_disk_activate,
-	.groups = ledtrig_disk_groups,
+static struct ledtrig_disk_trigger ledtrig_disk = {
+	.all = {
+		.name = "disk-activity",
+		.activate = ledtrig_disk_activate,
+		.groups = ledtrig_disk_groups,
+	},
+	.read = {
+		.name = "disk-read",
+		.activate = ledtrig_disk_activate,
+		.groups = ledtrig_disk_groups,
+	},
+	.write = {
+		.name = "disk-write",
+		.activate = ledtrig_disk_activate,
+		.groups = ledtrig_disk_groups,
+	},
 };
 
 static void ledtrig_disk_blink_oneshot(struct led_trigger *trig)
@@ -189,21 +191,121 @@ static void ledtrig_disk_blink_oneshot(struct led_trigger *trig)
 	rcu_read_unlock();
 }
 
-void ledtrig_disk_activity(bool write)
+static void ledtrig_disk_trigger_activity(struct ledtrig_disk_trigger *trig, bool write)
 {
-	ledtrig_disk_blink_oneshot(&ledtrig_disk);
+	if (IS_ERR_OR_NULL(trig))
+		return;
+	ledtrig_disk_blink_oneshot(&trig->all);
 	if (write)
-		ledtrig_disk_blink_oneshot(&ledtrig_disk_write);
+		ledtrig_disk_blink_oneshot(&trig->write);
 	else
-		ledtrig_disk_blink_oneshot(&ledtrig_disk_read);
+		ledtrig_disk_blink_oneshot(&trig->read);
+}
+
+void ledtrig_disk_activity(struct ledtrig_disk_trigger *port, bool write)
+{
+	ledtrig_disk_trigger_activity(&ledtrig_disk, write);
+	ledtrig_disk_trigger_activity(port, write);
 }
 EXPORT_SYMBOL(ledtrig_disk_activity);
 
+struct ledtrig_disk_trigger *ledtrig_disk_trigger_register(const char *name)
+{
+	struct ledtrig_disk_trigger *trigger = kzalloc(sizeof(*trigger), GFP_KERNEL);
+	int ret, n;
+
+	if (!trigger)
+		return ERR_PTR(-ENOMEM);
+
+	trigger->all.name = kzalloc(TRIG_NAME_MAX, GFP_KERNEL);
+	if (!trigger->all.name) {
+		ret = -ENOMEM;
+		goto err1;
+	}
+
+	n = snprintf((char *)trigger->all.name, TRIG_NAME_MAX, "%s-disk-activity", name);
+	if (n >= TRIG_NAME_MAX) {
+		ret = -E2BIG;
+		goto err1;
+	}
+
+	trigger->all.activate = ledtrig_disk_activate;
+	trigger->all.groups = ledtrig_disk_groups;
+
+	ret = led_trigger_register(&trigger->all);
+	if (ret)
+		goto err1;
+
+	trigger->read.name = kzalloc(TRIG_NAME_MAX, GFP_KERNEL);
+	if (!trigger->read.name) {
+		ret = -ENOMEM;
+		goto err2;
+	}
+
+	n = snprintf((char *)trigger->read.name, TRIG_NAME_MAX, "%s-disk-read", name);
+	if (n >= TRIG_NAME_MAX) {
+		ret = -E2BIG;
+		goto err2;
+	}
+
+	trigger->read.activate = ledtrig_disk_activate;
+	trigger->read.groups = ledtrig_disk_groups;
+
+	ret = led_trigger_register(&trigger->read);
+	if (ret)
+		goto err2;
+
+	trigger->write.name = kzalloc(TRIG_NAME_MAX, GFP_KERNEL);
+	if (!trigger->write.name) {
+		ret = -ENOMEM;
+		goto err3;
+	}
+
+	n = snprintf((char *)trigger->write.name, TRIG_NAME_MAX, "%s-disk-write", name);
+	if (n >= TRIG_NAME_MAX) {
+		ret = -E2BIG;
+		goto err3;
+	}
+
+	trigger->write.activate = ledtrig_disk_activate;
+	trigger->write.groups = ledtrig_disk_groups;
+
+	ret = led_trigger_register(&trigger->write);
+	if (ret)
+		goto err3;
+
+	return trigger;
+
+err3:
+	led_trigger_unregister(&trigger->read);
+err2:
+	led_trigger_unregister(&trigger->all);
+err1:
+	kfree(trigger->all.name);
+	kfree(trigger->read.name);
+	kfree(trigger->write.name);
+	kfree(trigger);
+
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL(ledtrig_disk_trigger_register);
+
+void ledtrig_disk_trigger_unregister(struct ledtrig_disk_trigger *trig)
+{
+	if (IS_ERR_OR_NULL(trig))
+		return;
+
+	led_trigger_unregister(&trig->all);
+	led_trigger_unregister(&trig->read);
+	led_trigger_unregister(&trig->write);
+}
+EXPORT_SYMBOL(ledtrig_disk_trigger_unregister);
+
 static int __init ledtrig_disk_init(void)
 {
-	led_trigger_register(&ledtrig_disk);
-	led_trigger_register(&ledtrig_disk_read);
-	led_trigger_register(&ledtrig_disk_write);
+	led_trigger_register(&ledtrig_disk.all);
+	led_trigger_register(&ledtrig_disk.read);
+	led_trigger_register(&ledtrig_disk.write);
 
 	return 0;
 }
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index bcecb4911da9..8841850684f7 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -587,14 +587,13 @@ int sas_ata_init(struct domain_device *found_dev)
 
 	ata_host_init(ata_host, ha->dev, &sas_sata_ops);
 
-	ap = ata_port_alloc(ata_host);
+	ap = ata_port_alloc(ata_host, 0);
 	if (!ap) {
 		pr_err("ata_port_alloc failed.\n");
 		rc = -ENODEV;
 		goto free_host;
 	}
 
-	ap->port_no = 0;
 	ap->pio_mask = ATA_PIO4;
 	ap->mwdma_mask = ATA_MWDMA2;
 	ap->udma_mask = ATA_UDMA6;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index b16b803cc1ac..3221be97e9c0 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -619,10 +619,22 @@ enum led_trigger_netdev_modes {
 };
 
 /* Trigger specific functions */
+struct ledtrig_disk_trigger {
+	struct led_trigger all;
+	struct led_trigger read;
+	struct led_trigger write;
+};
 #ifdef CONFIG_LEDS_TRIGGER_DISK
-void ledtrig_disk_activity(bool write);
+struct ledtrig_disk_trigger *ledtrig_disk_trigger_register(const char *name);
+void ledtrig_disk_trigger_unregister(struct ledtrig_disk_trigger *trig);
+void ledtrig_disk_activity(struct ledtrig_disk_trigger *port, bool write);
 #else
-static inline void ledtrig_disk_activity(bool write) {}
+static inline struct ledtrig_disk_trigger *ledtrig_disk_trigger_register(const char *name)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
+static inline void ledtrig_disk_trigger_unregister(struct ledtrig_disk_trigger *trig) {}
+static inline void ledtrig_disk_activity(struct ledtrig_disk_trigger *port, bool write) {}
 #endif
 
 #ifdef CONFIG_LEDS_TRIGGER_MTD
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 39534fafa36a..50124d170d13 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -940,6 +940,10 @@ struct ata_port {
 #ifdef CONFIG_ATA_ACPI
 	struct ata_acpi_gtm	__acpi_init_gtm; /* use ata_acpi_init_gtm() */
 #endif
+
+#ifdef CONFIG_LEDS_TRIGGER_DISK
+	struct ledtrig_disk_trigger *led_trigger;
+#endif
 };
 
 /* The following initializer overrides a method to NULL whether one of
@@ -1307,7 +1311,7 @@ extern int sata_link_scr_lpm(struct ata_link *link, enum ata_lpm_policy policy,
 			     bool spm_wakeup);
 extern int ata_slave_link_init(struct ata_port *ap);
 extern void ata_port_probe(struct ata_port *ap);
-extern struct ata_port *ata_port_alloc(struct ata_host *host);
+extern struct ata_port *ata_port_alloc(struct ata_host *host, int port_no);
 extern void ata_port_free(struct ata_port *ap);
 extern int ata_tport_add(struct device *parent, struct ata_port *ap);
 extern void ata_tport_delete(struct ata_port *ap);

-- 
2.52.0

Re: [PATCH RFC 4/4] leds: add disk trigger for each ata port

[Damien Le Moal]

On 1/24/26 4:05 AM, Markus Probst wrote:
> Register a disk trigger for each ata port. This trigger will only show
> the activity for the ata port it has been registered for.
> 
> This allows individual leds to be mapped to one ata port.
> This is especially useful for NAS devices, which have an own led for each
> disk slot.
> 
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  drivers/ata/libata-core.c           |  22 +++++-
>  drivers/leds/trigger/ledtrig-disk.c | 144 ++++++++++++++++++++++++++++++------
>  drivers/scsi/libsas/sas_ata.c       |   3 +-
>  include/linux/leds.h                |  16 +++-
>  include/linux/libata.h              |   6 +-
>  5 files changed, 161 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 09d8c035fcdf..796c46449298 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -4921,8 +4921,10 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
>  	struct ata_device *dev = qc->dev;
>  	struct ata_eh_info *ehi = &dev->link->eh_info;
>  
> +#ifdef CONFIG_LEDS_TRIGGER_DISK
>  	/* Trigger the LED (if available) */
> -	ledtrig_disk_activity(!!(qc->tf.flags & ATA_TFLAG_WRITE));
> +	ledtrig_disk_activity(ap->led_trigger, !!(qc->tf.flags & ATA_TFLAG_WRITE));
> +#endif

Please define an empty wrapper for the !CONFIG_LEDS_TRIGGER_DISK case to avoid
adding this ifdef.

>  
>  	/*
>  	 * In order to synchronize EH with the regular execution path, a qc that
> @@ -5538,10 +5540,13 @@ int sata_link_init_spd(struct ata_link *link)
>   *	LOCKING:
>   *	Inherited from calling layer (may sleep).
>   */
> -struct ata_port *ata_port_alloc(struct ata_host *host)
> +struct ata_port *ata_port_alloc(struct ata_host *host, int port_no)
>  {
>  	struct ata_port *ap;
>  	int id;
> +#ifdef CONFIG_LEDS_TRIGGER_DISK
> +	char name[32];
> +#endif
>  
>  	ap = kzalloc(sizeof(*ap), GFP_KERNEL);
>  	if (!ap)
> @@ -5557,6 +5562,7 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
>  	ap->print_id = id;
>  	ap->host = host;
>  	ap->dev = host->dev;
> +	ap->port_no = port_no;

Please extract this change in a prep patch.

>  
>  	mutex_init(&ap->scsi_scan_mutex);
>  	INIT_DELAYED_WORK(&ap->hotplug_task, ata_scsi_hotplug);
> @@ -5579,6 +5585,11 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
>  
>  	ata_force_pflags(ap);
>  
> +#ifdef CONFIG_LEDS_TRIGGER_DISK
> +	if (snprintf(name, sizeof(name), "%s-ata%d", dev_name(host->dev), port_no) < sizeof(name))
> +		ap->led_trigger = ledtrig_disk_trigger_register(name);
> +#endif

Same here: please define a helper function which is void for the
!CONFIG_LEDS_TRIGGER_DISK case. That will avoid both ifdefs here. Sprinkling
the code with such ifdef makes maintenance a nightmare. Wrap everything to
avoid that please.

> +
>  	return ap;
>  }
>  EXPORT_SYMBOL_GPL(ata_port_alloc);
> @@ -5588,6 +5599,10 @@ void ata_port_free(struct ata_port *ap)
>  	if (!ap)
>  		return;
>  
> +#ifdef CONFIG_LEDS_TRIGGER_DISK
> +	ledtrig_disk_trigger_unregister(ap->led_trigger);
> +#endif

Same comment. Use a void function or null macro to define this for the
!CONFIG_LEDS_TRIGGER_DISK case.

> +
>  	kfree(ap->pmp_link);
>  	kfree(ap->slave_link);
>  	ida_free(&ata_ida, ap->print_id);
> @@ -5690,11 +5705,10 @@ struct ata_host *ata_host_alloc(struct device *dev, int n_ports)
>  	for (i = 0; i < n_ports; i++) {
>  		struct ata_port *ap;
>  
> -		ap = ata_port_alloc(host);
> +		ap = ata_port_alloc(host, i);
>  		if (!ap)
>  			goto err_out;
>  
> -		ap->port_no = i;
>  		host->ports[i] = ap;
>  	}
>  
> diff --git a/drivers/leds/trigger/ledtrig-disk.c b/drivers/leds/trigger/ledtrig-disk.c
> index ed5ef83a5b35..fd25b4e45fb4 100644
> --- a/drivers/leds/trigger/ledtrig-disk.c
> +++ b/drivers/leds/trigger/ledtrig-disk.c

Can you try to split this part into a prep patch to avoid mixing led code and
ata code ?

> @@ -159,20 +159,22 @@ static int ledtrig_disk_activate(struct led_classdev *led_cdev)
>  	return 0;
>  }
>  
> -static struct led_trigger ledtrig_disk = {
> -	.name = "disk-activity",
> -	.activate = ledtrig_disk_activate,
> -	.groups = ledtrig_disk_groups,
> -};
> -static struct led_trigger ledtrig_disk_read = {
> -	.name = "disk-read",
> -	.activate = ledtrig_disk_activate,
> -	.groups = ledtrig_disk_groups,
> -};
> -static struct led_trigger ledtrig_disk_write = {
> -	.name = "disk-write",
> -	.activate = ledtrig_disk_activate,
> -	.groups = ledtrig_disk_groups,
> +static struct ledtrig_disk_trigger ledtrig_disk = {
> +	.all = {
> +		.name = "disk-activity",
> +		.activate = ledtrig_disk_activate,
> +		.groups = ledtrig_disk_groups,
> +	},
> +	.read = {
> +		.name = "disk-read",
> +		.activate = ledtrig_disk_activate,
> +		.groups = ledtrig_disk_groups,
> +	},
> +	.write = {
> +		.name = "disk-write",
> +		.activate = ledtrig_disk_activate,
> +		.groups = ledtrig_disk_groups,
> +	},
>  };
>  
>  static void ledtrig_disk_blink_oneshot(struct led_trigger *trig)
> @@ -189,21 +191,121 @@ static void ledtrig_disk_blink_oneshot(struct led_trigger *trig)
>  	rcu_read_unlock();
>  }
>  
> -void ledtrig_disk_activity(bool write)
> +static void ledtrig_disk_trigger_activity(struct ledtrig_disk_trigger *trig, bool write)
>  {
> -	ledtrig_disk_blink_oneshot(&ledtrig_disk);
> +	if (IS_ERR_OR_NULL(trig))
> +		return;
> +	ledtrig_disk_blink_oneshot(&trig->all);
>  	if (write)
> -		ledtrig_disk_blink_oneshot(&ledtrig_disk_write);
> +		ledtrig_disk_blink_oneshot(&trig->write);
>  	else
> -		ledtrig_disk_blink_oneshot(&ledtrig_disk_read);
> +		ledtrig_disk_blink_oneshot(&trig->read);
> +}
> +
> +void ledtrig_disk_activity(struct ledtrig_disk_trigger *port, bool write)
> +{
> +	ledtrig_disk_trigger_activity(&ledtrig_disk, write);
> +	ledtrig_disk_trigger_activity(port, write);
>  }
>  EXPORT_SYMBOL(ledtrig_disk_activity);
>  
> +struct ledtrig_disk_trigger *ledtrig_disk_trigger_register(const char *name)
> +{
> +	struct ledtrig_disk_trigger *trigger = kzalloc(sizeof(*trigger), GFP_KERNEL);
> +	int ret, n;
> +
> +	if (!trigger)
> +		return ERR_PTR(-ENOMEM);
> +
> +	trigger->all.name = kzalloc(TRIG_NAME_MAX, GFP_KERNEL);
> +	if (!trigger->all.name) {
> +		ret = -ENOMEM;
> +		goto err1;
> +	}
> +
> +	n = snprintf((char *)trigger->all.name, TRIG_NAME_MAX, "%s-disk-activity", name);
> +	if (n >= TRIG_NAME_MAX) {
> +		ret = -E2BIG;
> +		goto err1;
> +	}
> +
> +	trigger->all.activate = ledtrig_disk_activate;
> +	trigger->all.groups = ledtrig_disk_groups;
> +
> +	ret = led_trigger_register(&trigger->all);
> +	if (ret)
> +		goto err1;
> +
> +	trigger->read.name = kzalloc(TRIG_NAME_MAX, GFP_KERNEL);
> +	if (!trigger->read.name) {
> +		ret = -ENOMEM;
> +		goto err2;
> +	}
> +
> +	n = snprintf((char *)trigger->read.name, TRIG_NAME_MAX, "%s-disk-read", name);
> +	if (n >= TRIG_NAME_MAX) {
> +		ret = -E2BIG;
> +		goto err2;
> +	}
> +
> +	trigger->read.activate = ledtrig_disk_activate;
> +	trigger->read.groups = ledtrig_disk_groups;
> +
> +	ret = led_trigger_register(&trigger->read);
> +	if (ret)
> +		goto err2;
> +
> +	trigger->write.name = kzalloc(TRIG_NAME_MAX, GFP_KERNEL);
> +	if (!trigger->write.name) {
> +		ret = -ENOMEM;
> +		goto err3;
> +	}
> +
> +	n = snprintf((char *)trigger->write.name, TRIG_NAME_MAX, "%s-disk-write", name);
> +	if (n >= TRIG_NAME_MAX) {
> +		ret = -E2BIG;
> +		goto err3;
> +	}
> +
> +	trigger->write.activate = ledtrig_disk_activate;
> +	trigger->write.groups = ledtrig_disk_groups;
> +
> +	ret = led_trigger_register(&trigger->write);
> +	if (ret)
> +		goto err3;
> +
> +	return trigger;
> +
> +err3:
> +	led_trigger_unregister(&trigger->read);
> +err2:
> +	led_trigger_unregister(&trigger->all);
> +err1:
> +	kfree(trigger->all.name);
> +	kfree(trigger->read.name);
> +	kfree(trigger->write.name);
> +	kfree(trigger);
> +
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL(ledtrig_disk_trigger_register);
> +
> +void ledtrig_disk_trigger_unregister(struct ledtrig_disk_trigger *trig)
> +{
> +	if (IS_ERR_OR_NULL(trig))
> +		return;
> +
> +	led_trigger_unregister(&trig->all);
> +	led_trigger_unregister(&trig->read);
> +	led_trigger_unregister(&trig->write);
> +}
> +EXPORT_SYMBOL(ledtrig_disk_trigger_unregister);
> +
>  static int __init ledtrig_disk_init(void)
>  {
> -	led_trigger_register(&ledtrig_disk);
> -	led_trigger_register(&ledtrig_disk_read);
> -	led_trigger_register(&ledtrig_disk_write);
> +	led_trigger_register(&ledtrig_disk.all);
> +	led_trigger_register(&ledtrig_disk.read);
> +	led_trigger_register(&ledtrig_disk.write);
>  
>  	return 0;
>  }
> diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
> index bcecb4911da9..8841850684f7 100644
> --- a/drivers/scsi/libsas/sas_ata.c
> +++ b/drivers/scsi/libsas/sas_ata.c
> @@ -587,14 +587,13 @@ int sas_ata_init(struct domain_device *found_dev)
>  
>  	ata_host_init(ata_host, ha->dev, &sas_sata_ops);
>  
> -	ap = ata_port_alloc(ata_host);
> +	ap = ata_port_alloc(ata_host, 0);
>  	if (!ap) {
>  		pr_err("ata_port_alloc failed.\n");
>  		rc = -ENODEV;
>  		goto free_host;
>  	}
>  
> -	ap->port_no = 0;
>  	ap->pio_mask = ATA_PIO4;
>  	ap->mwdma_mask = ATA_MWDMA2;
>  	ap->udma_mask = ATA_UDMA6;
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index b16b803cc1ac..3221be97e9c0 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -619,10 +619,22 @@ enum led_trigger_netdev_modes {
>  };
>  
>  /* Trigger specific functions */
> +struct ledtrig_disk_trigger {
> +	struct led_trigger all;
> +	struct led_trigger read;
> +	struct led_trigger write;
> +};
>  #ifdef CONFIG_LEDS_TRIGGER_DISK
> -void ledtrig_disk_activity(bool write);
> +struct ledtrig_disk_trigger *ledtrig_disk_trigger_register(const char *name);
> +void ledtrig_disk_trigger_unregister(struct ledtrig_disk_trigger *trig);
> +void ledtrig_disk_activity(struct ledtrig_disk_trigger *port, bool write);
>  #else
> -static inline void ledtrig_disk_activity(bool write) {}
> +static inline struct ledtrig_disk_trigger *ledtrig_disk_trigger_register(const char *name)
> +{
> +	return ERR_PTR(-EOPNOTSUPP);
> +}
> +static inline void ledtrig_disk_trigger_unregister(struct ledtrig_disk_trigger *trig) {}
> +static inline void ledtrig_disk_activity(struct ledtrig_disk_trigger *port, bool write) {}
>  #endif
>  
>  #ifdef CONFIG_LEDS_TRIGGER_MTD
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 39534fafa36a..50124d170d13 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -940,6 +940,10 @@ struct ata_port {
>  #ifdef CONFIG_ATA_ACPI
>  	struct ata_acpi_gtm	__acpi_init_gtm; /* use ata_acpi_init_gtm() */
>  #endif
> +
> +#ifdef CONFIG_LEDS_TRIGGER_DISK
> +	struct ledtrig_disk_trigger *led_trigger;
> +#endif
>  };
>  
>  /* The following initializer overrides a method to NULL whether one of
> @@ -1307,7 +1311,7 @@ extern int sata_link_scr_lpm(struct ata_link *link, enum ata_lpm_policy policy,
>  			     bool spm_wakeup);
>  extern int ata_slave_link_init(struct ata_port *ap);
>  extern void ata_port_probe(struct ata_port *ap);
> -extern struct ata_port *ata_port_alloc(struct ata_host *host);
> +extern struct ata_port *ata_port_alloc(struct ata_host *host, int port_no);
>  extern void ata_port_free(struct ata_port *ap);
>  extern int ata_tport_add(struct device *parent, struct ata_port *ap);
>  extern void ata_tport_delete(struct ata_port *ap);
> 


-- 
Damien Le Moal
Western Digital Research

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

[Pavel Machek]

Hi!

> 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 have used led trigger before, and it annoyed me than "constant disk
activity" resulted in "constant LED blinking" instead of "LED
constantly on". I would not mind if that was fixed.

Thanks and best regards,
								Pavel

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

[Markus Probst]

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

On Sun, 2026-01-25 at 00:21 +0100, Pavel Machek wrote:
> Hi!
> 
> > 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 have used led trigger before, and it annoyed me than "constant disk
> activity" resulted in "constant LED blinking" instead of "LED
> constantly on". I would not mind if that was fixed.
> 
> Thanks and best regards,
> 								Pavel

This patch series adds support for changing the delay_off value in
blinking.

For having "constant disk activity" => "LED constantly on" the
delay_off value has to be set to 0 ms.

Let me know if I should adjust the default of delay_off from 30 ms to 0
ms.

Thanks
- Markus Probst

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

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

[Niklas Cassel]

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?

Sure, there is already the very simple ledtrig-disk driver.

But I'm not a fan of making the driver more complex.
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.

Basically the same argument as used in:
https://lore.kernel.org/linux-nvme/20220227234258.24619-1-ematsumiya@suse.de/T/#u


Kind regards,
Niklas

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

[Ian Pilcher]

On 1/26/26 3:00 AM, Niklas Cassel wrote:
> But I'm not a fan of making the driver more complex.
> 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.
> 
> Basically the same argument as used in:
> https://lore.kernel.org/linux-nvme/20220227234258.24619-1-ematsumiya@suse.de/T/#u

Niklas -

Can you provide some links on how this might be done in userspace?

I've been maintaining my out-of-tree block device trigger for years, to
make the LEDs on my NAS work.

  https://github.com/ipilcher/ledtrig-blkdev/blob/v6.9%2B/drivers/leds/trigger/ledtrig-blkdev.c

I'd love to be able to replace it with something in-tree.

-- 
========================================================================
If your user interface is intuitive in retrospect ... it isn't intuitive
========================================================================

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

[Niklas Cassel]

Hello Ian,

On Mon, Jan 26, 2026 at 10:19:07AM -0600, Ian Pilcher wrote:
> On 1/26/26 3:00 AM, Niklas Cassel wrote:
> > But I'm not a fan of making the driver more complex.
> > 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.
> > 
> > Basically the same argument as used in:
> > https://lore.kernel.org/linux-nvme/20220227234258.24619-1-ematsumiya@suse.de/T/#u
> 
> Niklas -
> 
> Can you provide some links on how this might be done in userspace?

See the link to the thread above.

Ming suggests "tracking iostat in a fixed period, and triggering one
led activity if any read/write IO happens during 0.5sec."

There is also a link to an nvme-led-daemon in the thread:
https://github.com/scarlion1/nvme-led-daemon

But considering that it is using:
https://www.kernel.org/doc/Documentation/block/stat.txt

I don't see why it can't be used for any block device.


Kind regards,
Niklas

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

[Markus Probst]

[-- 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 --]

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

[Niklas Cassel]

On Mon, Jan 26, 2026 at 10:06:02PM +0000, Markus Probst wrote:
> On Mon, 2026-01-26 at 10:00 +0100, Niklas Cassel wrote:
> > 
> > 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.

I understand why you want the feature. I just don't understand why we
should add this feature to the kernel, rather than implement it in
user space.

Having a user space implementation for your feature would also allow
an upstream kernel, without the need for any custom kernel patches.


> > 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.

There seems to be existing user space applications that handles this,
I think both the daemon I linked to before, which uses /sys/block/<dev>/stat
which is thus per device and not per port, and e.g. this:
https://linux.die.net/man/8/ledmon
https://github.com/md-raid-utilities/ledmon
https://github.com/md-raid-utilities/ledmon/blob/main/src/lib/ahci.c


> > 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.

I think my main concern is that I don't think we should bloat the kernel
for a complex feature that can just as well be implemented in user space.


Kind regards,
Niklas

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

[Markus Probst]

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

On Tue, 2026-01-27 at 10:32 +0100, Niklas Cassel wrote:
> On Mon, Jan 26, 2026 at 10:06:02PM +0000, Markus Probst wrote:
> > On Mon, 2026-01-26 at 10:00 +0100, Niklas Cassel wrote:
> > > 
> > > 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.
> 
> I understand why you want the feature. I just don't understand why we
> should add this feature to the kernel, rather than implement it in
> user space.
> 
> Having a user space implementation for your feature would also allow
> an upstream kernel, without the need for any custom kernel patches.
Only because it can be done in userspace, doesn't mean it should be.
> 
> > > 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.
> 
> There seems to be existing user space applications that handles this,
> I think both the daemon I linked to before, which uses /sys/block/<dev>/stat
> which is thus per device and not per port, and e.g. this:
> https://linux.die.net/man/8/ledmon
> https://github.com/md-raid-utilities/ledmon
> https://github.com/md-raid-utilities/ledmon/blob/main/src/lib/ahci.c
As far as I can tell, this daemon doesn't actually use the LED
Subsystem, but instead leds directly connected to the storage
controller.
But yes, I would be capable of coding such daemon.

> > > 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.
> 
> I think my main concern is that I don't think we should bloat the kernel
> for a complex feature that can just as well be implemented in user space.
It is still unclear to me if you worry about the complexity in
drivers/ata/libata-* or drivers/leds/trigger/ledtrig-disk.c

@Pavel,@Lee: I would like to know your opinion on this.

Thanks
- Markus Probst

> 
> 
> Kind regards,
> Niklas

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

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

[Damien Le Moal]

On 1/28/26 12:34 AM, Markus Probst wrote:
>> Having a user space implementation for your feature would also allow
>> an upstream kernel, without the need for any custom kernel patches.
> Only because it can be done in userspace, doesn't mean it should be.

Yes it should. Maintaining userspace is far easier than forcing kernel changes
onto users to get blinking LEDs. So unless you have a very strong argument for
doing it in the kernel, userspace is probably the right approach. That will
apply to any block device, and not just ATA devices. E.g. NAS with M.2 NVMe
storage can work with the same.

>> There seems to be existing user space applications that handles this,
>> I think both the daemon I linked to before, which uses /sys/block/<dev>/stat
>> which is thus per device and not per port, and e.g. this:
>> https://linux.die.net/man/8/ledmon
>> https://github.com/md-raid-utilities/ledmon
>> https://github.com/md-raid-utilities/ledmon/blob/main/src/lib/ahci.c
> As far as I can tell, this daemon doesn't actually use the LED
> Subsystem, but instead leds directly connected to the storage
> controller.
> But yes, I would be capable of coding such daemon.

Then let's try. That will allow checking if anything is missing in the kernel
interface to do that nicely.

>> I think my main concern is that I don't think we should bloat the kernel
>> for a complex feature that can just as well be implemented in user space.
> It is still unclear to me if you worry about the complexity in
> drivers/ata/libata-* or drivers/leds/trigger/ledtrig-disk.c

It is not so much about complexity but rather the fact that controlling
blinking LEDs in the hot IO path is not desirable. While SATA HDDs will be less
sensible to the delays caused by the calls to the LED control functions
compared to fast NVMe SSDs, we do also need to think about much faster SATA
SSDs. So instead of risking performance regressions, let's try to do this in
userspace first.


-- 
Damien Le Moal
Western Digital Research

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

[Markus Probst]

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

On Wed, 2026-01-28 at 15:34 +0900, Damien Le Moal wrote:
> On 1/28/26 12:34 AM, Markus Probst wrote:
> > > Having a user space implementation for your feature would also allow
> > > an upstream kernel, without the need for any custom kernel patches.
> > Only because it can be done in userspace, doesn't mean it should be.
> 
> Yes it should. Maintaining userspace is far easier than forcing kernel changes
> onto users to get blinking LEDs. So unless you have a very strong argument for
> doing it in the kernel, userspace is probably the right approach. That will
> apply to any block device, and not just ATA devices. E.g. NAS with M.2 NVMe
> storage can work with the same.
> 
> > > There seems to be existing user space applications that handles this,
> > > I think both the daemon I linked to before, which uses /sys/block/<dev>/stat
> > > which is thus per device and not per port, and e.g. this:
> > > https://linux.die.net/man/8/ledmon
> > > https://github.com/md-raid-utilities/ledmon
> > > https://github.com/md-raid-utilities/ledmon/blob/main/src/lib/ahci.c
> > As far as I can tell, this daemon doesn't actually use the LED
> > Subsystem, but instead leds directly connected to the storage
> > controller.
> > But yes, I would be capable of coding such daemon.
> 
> Then let's try.
I will give it a try.

@Ian: I will notify you, once there is a working version.

> That will allow checking if anything is missing in the kernel
> interface to do that nicely.
There is.

I noticed for leds, that the fwnode path isn't exposed in sysfs.
"/sys/class/leds/<name>/device/firmware_node/path" exists, but points
to the parent device.

Something similar with scsi and ata exists. scsi doesn't expose the
firmware_node and there is no symlink (or other connection that I am
ware of) between scsi_* and ata_* in sysfs. This means, I cannot map a
fwnode path to a block device.

If I want to distribute a pre-defined config for such led userspace
daemon alongside the ACPI Overlay for a specific NAS model, I need an
identifier that is equal across all devices with that specific NAS
model.

This is less of an issue for leds, but given that leds could be renamed
on name collisions the issue still exists.

Thanks
- Markus Probst

> 
> > > I think my main concern is that I don't think we should bloat the kernel
> > > for a complex feature that can just as well be implemented in user space.
> > It is still unclear to me if you worry about the complexity in
> > drivers/ata/libata-* or drivers/leds/trigger/ledtrig-disk.c
> 
> It is not so much about complexity but rather the fact that controlling
> blinking LEDs in the hot IO path is not desirable. While SATA HDDs will be less
> sensible to the delays caused by the calls to the LED control functions
> compared to fast NVMe SSDs, we do also need to think about much faster SATA
> SSDs. So instead of risking performance regressions, let's try to do this in
> userspace first.
> 

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

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

[Niklas Cassel]

Hello Markus,

On Wed, Jan 28, 2026 at 03:44:19PM +0000, Markus Probst wrote:
> Something similar with scsi and ata exists. scsi doesn't expose the
> firmware_node and there is no symlink (or other connection that I am
> ware of) between scsi_* and ata_* in sysfs. This means, I cannot map a
> fwnode path to a block device.

Hopefully this might help you:

$ ls -al /sys/class/scsi_device | grep ata
lrwxrwxrwx.  1 root root 0 Jan 29 05:12 4:0:0:0 -> ../../devices/pci0000:00/0000:00:17.0/ata5/host4/target4:0:0/4:0:0:0/scsi_device/4:0:0:0
lrwxrwxrwx.  1 root root 0 Jan 29 05:12 5:0:0:0 -> ../../devices/pci0000:00/0000:00:17.0/ata6/host5/target5:0:0/5:0:0:0/scsi_device/5:0:0:0
lrwxrwxrwx.  1 root root 0 Jan 29 05:12 8:0:0:0 -> ../../devices/pci0000:00/0000:00:17.0/ata9/host8/target8:0:0/8:0:0:0/scsi_device/8:0:0:0
lrwxrwxrwx.  1 root root 0 Jan 29 05:12 9:0:0:0 -> ../../devices/pci0000:00/0000:00:17.0/ata10/host9/target9:0:0/9:0:0:0/scsi_device/9:0:0:0


For a specific device, e.g. 4:0:0:0:

$ realpath /sys/class/scsi_device/4:0:0:0/
/sys/devices/pci0000:00/0000:00:17.0/ata5/host4/target4:0:0/4:0:0:0/scsi_device/4:0:0:0

To get the block device name:
$ ls /sys/class/scsi_device/4:0:0:0/device/block/
sda

or

$ ls /sys/devices/pci0000:00/0000:00:17.0/ata5/host4/target4:0:0/4:0:0:0/scsi_device/4:0:0:0/device/block/
sda


You can parse the port from the path. The above example is port 5.

If using a port multiplier (PM), there can be multiple links/devices per port.


Otherwise, for SATA there should be only one.

$ ls -al /sys/class/ata_port/ata5/device/ | grep link | wc -l
1

$ ls -al /sys/class/ata_port/ata5/device/link5/dev5.0/firmware_node

or

$ ls -al  /sys/class/ata_device/dev5.*/device/firmware_node



For PCI BDF, you can use /dev/disk/by-path/

$ ls -al /dev/disk/by-path/ | grep ata
lrwxrwxrwx.  1 root root   9 Jan 29 05:11 pci-0000:00:17.0-ata-3 -> ../../sda
lrwxrwxrwx.  1 root root   9 Jan 29 05:11 pci-0000:00:17.0-ata-3.0 -> ../../sda
lrwxrwxrwx.  1 root root   9 Jan 29 05:11 pci-0000:00:17.0-ata-4 -> ../../sdb
lrwxrwxrwx.  1 root root   9 Jan 29 05:11 pci-0000:00:17.0-ata-4.0 -> ../../sdb
lrwxrwxrwx.  1 root root   9 Jan 29 05:11 pci-0000:00:17.0-ata-7 -> ../../sdc
lrwxrwxrwx.  1 root root   9 Jan 29 05:11 pci-0000:00:17.0-ata-7.0 -> ../../sdc
lrwxrwxrwx.  1 root root   9 Jan 29 05:11 pci-0000:00:17.0-ata-8 -> ../../sdd
lrwxrwxrwx.  1 root root   9 Jan 29 05:11 pci-0000:00:17.0-ata-8.0 -> ../../sdd

Note that these suffixes do not correlate to the ata port number in /sys/class/ata_*


$ ls -al /sys/class/ata_port/
total 0
drwxr-xr-x.  2 root root 0 Jan 29 05:11 .
drwxr-xr-x. 84 root root 0 Jan 29 05:12 ..
lrwxrwxrwx.  1 root root 0 Nov 19 04:01 ata1 -> ../../devices/pci0000:00/0000:00:11.5/ata1/ata_port/ata1
lrwxrwxrwx.  1 root root 0 Jan 29 05:26 ata10 -> ../../devices/pci0000:00/0000:00:17.0/ata10/ata_port/ata10
lrwxrwxrwx.  1 root root 0 Nov 19 04:01 ata11 -> ../../devices/pci0000:50/0000:50:02.0/0000:51:00.0/ata11/ata_port/ata11
lrwxrwxrwx.  1 root root 0 Nov 19 04:01 ata12 -> ../../devices/pci0000:50/0000:50:02.0/0000:51:00.0/ata12/ata_port/ata12
lrwxrwxrwx.  1 root root 0 Nov 19 04:01 ata13 -> ../../devices/pci0000:50/0000:50:02.0/0000:51:00.0/ata13/ata_port/ata13
lrwxrwxrwx.  1 root root 0 Nov 19 04:01 ata2 -> ../../devices/pci0000:00/0000:00:11.5/ata2/ata_port/ata2
lrwxrwxrwx.  1 root root 0 Jan 29 05:26 ata3 -> ../../devices/pci0000:00/0000:00:17.0/ata3/ata_port/ata3
lrwxrwxrwx.  1 root root 0 Jan 29 05:26 ata4 -> ../../devices/pci0000:00/0000:00:17.0/ata4/ata_port/ata4
lrwxrwxrwx.  1 root root 0 Jan 29 05:26 ata5 -> ../../devices/pci0000:00/0000:00:17.0/ata5/ata_port/ata5
lrwxrwxrwx.  1 root root 0 Jan 29 05:26 ata6 -> ../../devices/pci0000:00/0000:00:17.0/ata6/ata_port/ata6
lrwxrwxrwx.  1 root root 0 Jan 29 05:26 ata7 -> ../../devices/pci0000:00/0000:00:17.0/ata7/ata_port/ata7
lrwxrwxrwx.  1 root root 0 Jan 29 05:26 ata8 -> ../../devices/pci0000:00/0000:00:17.0/ata8/ata_port/ata8
lrwxrwxrwx.  1 root root 0 Jan 29 05:26 ata9 -> ../../devices/pci0000:00/0000:00:17.0/ata9/ata_port/ata9


Kind regards,
Niklas

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

[Damien Le Moal]

On 1/29/26 00:44, Markus Probst wrote:
>> That will allow checking if anything is missing in the kernel
>> interface to do that nicely.
> There is.
> 
> I noticed for leds, that the fwnode path isn't exposed in sysfs.
> "/sys/class/leds/<name>/device/firmware_node/path" exists, but points
> to the parent device.
> 
> Something similar with scsi and ata exists. scsi doesn't expose the
> firmware_node and there is no symlink (or other connection that I am
> ware of) between scsi_* and ata_* in sysfs. This means, I cannot map a
> fwnode path to a block device.
> 
> If I want to distribute a pre-defined config for such led userspace
> daemon alongside the ACPI Overlay for a specific NAS model, I need an
> identifier that is equal across all devices with that specific NAS
> model.
> 
> This is less of an issue for leds, but given that leds could be renamed
> on name collisions the issue still exists.

All of this is not the hot path, so we can work on it.

If new sysfs device attributes for an ATA device, you can add them to
ata_ncq_sdev_attrs in libata-sata.c. These show up as part of the scsi device
attributes, so if you define this well with the scsi side, the same attribute
names can be used for pure scsi devices and ATA devices served with libata.


-- 
Damien Le Moal
Western Digital Research