Enable linedisp library integration into existing kernel devices (like LED
class) to provide a uniform 7-segment userspace API without creating
separate child devices, meeting the consistent interface while maintaining
coherent device hierarchies.

This allows uniform 7-segment API across all drivers while solving device
proliferation and fragmented userspace interfaces.

The provided attributes appear in two locations depending on usage:
  1. On linedisp.N child devices (legacy linedisp_register())
  2. On the parent auxdisplay device (new linedisp_attach())
Functionality is identical in both modes.

Existing consumers of linedisp_register() are unaffected. The new API
enables drivers like TM16XX to integrate 7-segment display functionality
seamlessly within their LED class device hierarchy.

Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 drivers/auxdisplay/line-display.c | 160 +++++++++++++++++++++++++++++-
 drivers/auxdisplay/line-display.h |   4 +
 2 files changed, 161 insertions(+), 3 deletions(-)

diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index abeed8812..1176d46f0 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -6,20 +6,23 @@
  * Author: Paul Burton <paul.burton@mips.com>
  *
  * Copyright (C) 2021 Glider bv
+ * Copyright (C) 2025 Jean-François Lessard
  */
 
 #ifndef CONFIG_PANEL_BOOT_MESSAGE
 #include <generated/utsrelease.h>
 #endif
 
-#include <linux/container_of.h>
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/export.h>
 #include <linux/idr.h>
 #include <linux/jiffies.h>
 #include <linux/kstrtox.h>
+#include <linux/list.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/spinlock.h>
 #include <linux/string.h>
 #include <linux/sysfs.h>
 #include <linux/timer.h>
@@ -31,9 +34,72 @@
 
 #define DEFAULT_SCROLL_RATE	(HZ / 2)
 
+struct linedisp_attachment {
+	struct list_head list;
+	struct device *device;
+	struct linedisp *linedisp;
+	bool owns_device;  /* true for child device mode, false for attached mode */
+};
+
+static LIST_HEAD(linedisp_attachments);
+static DEFINE_SPINLOCK(linedisp_attachments_lock);
+
+static int create_attachment(struct device *dev, struct linedisp *linedisp, bool owns_device)
+{
+	struct linedisp_attachment *attachment;
+
+	attachment = kzalloc(sizeof(*attachment), GFP_KERNEL);
+	if (!attachment)
+		return -ENOMEM;
+
+	attachment->device = dev;
+	attachment->linedisp = linedisp;
+	attachment->owns_device = owns_device;
+
+	guard(spinlock)(&linedisp_attachments_lock);
+	list_add(&attachment->list, &linedisp_attachments);
+
+	return 0;
+}
+
+static struct linedisp *delete_attachment(struct device *dev, bool owns_device)
+{
+	struct linedisp_attachment *attachment;
+	struct linedisp *linedisp;
+
+	guard(spinlock)(&linedisp_attachments_lock);
+
+	list_for_each_entry(attachment, &linedisp_attachments, list) {
+		if (attachment->device == dev &&
+		    attachment->owns_device == owns_device)
+			break;
+	}
+
+	if (list_entry_is_head(attachment, &linedisp_attachments, list))
+		return NULL;
+
+	linedisp = attachment->linedisp;
+	list_del(&attachment->list);
+	kfree(attachment);
+
+	return linedisp;
+}
+
 static struct linedisp *to_linedisp(struct device *dev)
 {
-	return container_of(dev, struct linedisp, dev);
+	struct linedisp_attachment *attachment;
+
+	guard(spinlock)(&linedisp_attachments_lock);
+
+	list_for_each_entry(attachment, &linedisp_attachments, list) {
+		if (attachment->device == dev)
+			break;
+	}
+
+	if (list_entry_is_head(attachment, &linedisp_attachments, list))
+		return NULL;
+
+	return attachment->linedisp;
 }
 
 static inline bool should_scroll(struct linedisp *linedisp)
@@ -349,6 +415,87 @@ static int linedisp_init_map(struct linedisp *linedisp)
 #define LINEDISP_INIT_TEXT "Linux " UTS_RELEASE "       "
 #endif
 
+/**
+ * linedisp_attach - attach a character line display
+ * @linedisp: pointer to character line display structure
+ * @dev: pointer of the device to attach to
+ * @num_chars: the number of characters that can be displayed
+ * @ops: character line display operations
+ *
+ * Return: zero on success, else a negative error code.
+ */
+int linedisp_attach(struct linedisp *linedisp, struct device *dev,
+		    unsigned int num_chars, const struct linedisp_ops *ops)
+{
+	int err;
+
+	memset(linedisp, 0, sizeof(*linedisp));
+	linedisp->ops = ops;
+	linedisp->num_chars = num_chars;
+	linedisp->scroll_rate = DEFAULT_SCROLL_RATE;
+
+	linedisp->buf = kzalloc(linedisp->num_chars, GFP_KERNEL);
+	if (!linedisp->buf)
+		return -ENOMEM;
+
+	/* initialise a character mapping, if required */
+	err = linedisp_init_map(linedisp);
+	if (err)
+		goto out_free_buf;
+
+	/* initialise a timer for scrolling the message */
+	timer_setup(&linedisp->timer, linedisp_scroll, 0);
+
+	err = create_attachment(dev, linedisp, false);
+	if (err)
+		goto out_del_timer;
+
+	/* add attribute groups to target device */
+	err = device_add_groups(dev, linedisp_groups);
+	if (err)
+		goto out_del_attach;
+
+	/* display a default message */
+	err = linedisp_display(linedisp, LINEDISP_INIT_TEXT, -1);
+	if (err)
+		goto out_rem_groups;
+
+	return 0;
+
+out_rem_groups:
+	device_remove_groups(dev, linedisp_groups);
+out_del_attach:
+	delete_attachment(dev, false);
+out_del_timer:
+	timer_delete_sync(&linedisp->timer);
+out_free_buf:
+	kfree(linedisp->buf);
+	return err;
+}
+EXPORT_SYMBOL_NS_GPL(linedisp_attach, "LINEDISP");
+
+/**
+ * linedisp_detach - detach a character line display
+ * @dev: pointer of the device to detach from, that was previously
+ *	 attached with linedisp_attach()
+ */
+void linedisp_detach(struct device *dev)
+{
+	struct linedisp *linedisp = delete_attachment(dev, false);
+
+	if (!linedisp)
+		return;
+
+	timer_delete_sync(&linedisp->timer);
+
+	device_remove_groups(dev, linedisp_groups);
+
+	kfree(linedisp->map);
+	kfree(linedisp->message);
+	kfree(linedisp->buf);
+}
+EXPORT_SYMBOL_NS_GPL(linedisp_detach, "LINEDISP");
+
 /**
  * linedisp_register - register a character line display
  * @linedisp: pointer to character line display structure
@@ -391,10 +538,14 @@ int linedisp_register(struct linedisp *linedisp, struct device *parent,
 	/* initialise a timer for scrolling the message */
 	timer_setup(&linedisp->timer, linedisp_scroll, 0);
 
-	err = device_add(&linedisp->dev);
+	err = create_attachment(&linedisp->dev, linedisp, true);
 	if (err)
 		goto out_del_timer;
 
+	err = device_add(&linedisp->dev);
+	if (err)
+		goto out_del_attach;
+
 	/* display a default message */
 	err = linedisp_display(linedisp, LINEDISP_INIT_TEXT, -1);
 	if (err)
@@ -404,6 +555,8 @@ int linedisp_register(struct linedisp *linedisp, struct device *parent,
 
 out_del_dev:
 	device_del(&linedisp->dev);
+out_del_attach:
+	delete_attachment(&linedisp->dev, true);
 out_del_timer:
 	timer_delete_sync(&linedisp->timer);
 out_put_device:
@@ -420,6 +573,7 @@ EXPORT_SYMBOL_NS_GPL(linedisp_register, "LINEDISP");
 void linedisp_unregister(struct linedisp *linedisp)
 {
 	device_del(&linedisp->dev);
+	delete_attachment(&linedisp->dev, true);
 	timer_delete_sync(&linedisp->timer);
 	put_device(&linedisp->dev);
 }
diff --git a/drivers/auxdisplay/line-display.h b/drivers/auxdisplay/line-display.h
index 4348d7a2f..36853b639 100644
--- a/drivers/auxdisplay/line-display.h
+++ b/drivers/auxdisplay/line-display.h
@@ -6,6 +6,7 @@
  * Author: Paul Burton <paul.burton@mips.com>
  *
  * Copyright (C) 2021 Glider bv
+ * Copyright (C) 2025 Jean-François Lessard
  */
 
 #ifndef _LINEDISP_H
@@ -81,6 +82,9 @@ struct linedisp {
 	unsigned int id;
 };
 
+int linedisp_attach(struct linedisp *linedisp, struct device *dev,
+		    unsigned int num_chars, const struct linedisp_ops *ops);
+void linedisp_detach(struct device *dev);
 int linedisp_register(struct linedisp *linedisp, struct device *parent,
 		      unsigned int num_chars, const struct linedisp_ops *ops);
 void linedisp_unregister(struct linedisp *linedisp);
-- 
2.43.0

On Sun, Aug 31, 2025 at 10:00:28PM -0400, Jean-François Lessard wrote:
> Enable linedisp library integration into existing kernel devices (like LED
> class) to provide a uniform 7-segment userspace API without creating
> separate child devices, meeting the consistent interface while maintaining
> coherent device hierarchies.
> 
> This allows uniform 7-segment API across all drivers while solving device
> proliferation and fragmented userspace interfaces.
> 
> The provided attributes appear in two locations depending on usage:

You wanted to say "...in one of the two possible..."?
Otherwise it looks like undesired side effect of the change.

>   1. On linedisp.N child devices (legacy linedisp_register())
>   2. On the parent auxdisplay device (new linedisp_attach())
> Functionality is identical in both modes.
> 
> Existing consumers of linedisp_register() are unaffected. The new API
> enables drivers like TM16XX to integrate 7-segment display functionality
> seamlessly within their LED class device hierarchy.

...

> +struct linedisp_attachment {
> +	struct list_head list;
> +	struct device *device;
> +	struct linedisp *linedisp;

> +	bool owns_device;  /* true for child device mode, false for attached mode */

I would rename this to 

	bool attached; // with inverted logic

or
	bool mode; // with "default" (false) mode to be actually legacy one

(so in both cases I think we want false for the legacy mode).

> +};

...

> +static DEFINE_SPINLOCK(linedisp_attachments_lock);

Why spin lock and not mutex?

...

> +/**
> + * linedisp_attach - attach a character line display
> + * @linedisp: pointer to character line display structure
> + * @dev: pointer of the device to attach to
> + * @num_chars: the number of characters that can be displayed
> + * @ops: character line display operations
> + *

Can you add a description here, please? Important note that it should be freed
by the respective API afterwards.

> + * Return: zero on success, else a negative error code.
> + */

...

> +	/* add attribute groups to target device */
> +	err = device_add_groups(dev, linedisp_groups);
> +	if (err)
> +		goto out_del_attach;
> +
> +	/* display a default message */
> +	err = linedisp_display(linedisp, LINEDISP_INIT_TEXT, -1);
> +	if (err)
> +		goto out_rem_groups;

Can this be racy with user space? Once we publish attributes, the new
message can be already issued and here it puts another message.
OTOH this is done before device_add(), so the attributes are not visible
until the device is added.

...

> +void linedisp_detach(struct device *dev)
> +{

> +	struct linedisp *linedisp = delete_attachment(dev, false);
> +
> +	if (!linedisp)
> +		return;

Please, rewrite as

	struct linedisp *linedisp;

	linedisp = delete_attachment(dev, false);
	if (!linedisp)
		return;

> +	timer_delete_sync(&linedisp->timer);
> +
> +	device_remove_groups(dev, linedisp_groups);
> +
> +	kfree(linedisp->map);
> +	kfree(linedisp->message);
> +	kfree(linedisp->buf);
> +}

-- 
With Best Regards,
Andy Shevchenko

Le 2 septembre 2025 06 h 18 min 24 s HAE, Andy Shevchenko <andriy.shevchenko@intel.com> a écrit :
>On Sun, Aug 31, 2025 at 10:00:28PM -0400, Jean-François Lessard wrote:
>> Enable linedisp library integration into existing kernel devices (like LED
>> class) to provide a uniform 7-segment userspace API without creating
>> separate child devices, meeting the consistent interface while maintaining
>> coherent device hierarchies.
>> 
>> This allows uniform 7-segment API across all drivers while solving device
>> proliferation and fragmented userspace interfaces.
>> 
>> The provided attributes appear in two locations depending on usage:
>
>You wanted to say "...in one of the two possible..."?
>Otherwise it looks like undesired side effect of the change.
>

Yes, your wording is much clearer. I will rephrase:

The provided attributes appear in one of the two locations depending on usage:

>>   1. On linedisp.N child devices (legacy linedisp_register())
>>   2. On the parent auxdisplay device (new linedisp_attach())
>> Functionality is identical in both modes.
>> 
>> Existing consumers of linedisp_register() are unaffected. The new API
>> enables drivers like TM16XX to integrate 7-segment display functionality
>> seamlessly within their LED class device hierarchy.
>
>...
>
>> +struct linedisp_attachment {
>> +	struct list_head list;
>> +	struct device *device;
>> +	struct linedisp *linedisp;
>
>> +	bool owns_device;  /* true for child device mode, false for attached mode */
>
>I would rename this to 
>
>	bool attached; // with inverted logic
>
>or
>	bool mode; // with "default" (false) mode to be actually legacy one
>
>(so in both cases I think we want false for the legacy mode).
>

Understood. I will rename, invert logic and also document as kernel-doc instead
of inline comment.

>> +};
>
>...
>
>> +static DEFINE_SPINLOCK(linedisp_attachments_lock);
>
>Why spin lock and not mutex?
>

The attachment list operations are extremely lightweight (just adding/removing
list entries), making spinlock the optimal choice because:
- Very short critical sections: Only list traversal and pointer assignments;
  avoids context switching overhead for brief operations
- No sleeping operations: No memory allocation or I/O within locked sections
- Future-proof atomic context safety: Can be safely called from interrupt
  handlers if needed

>...
>
>> +/**
>> + * linedisp_attach - attach a character line display
>> + * @linedisp: pointer to character line display structure
>> + * @dev: pointer of the device to attach to
>> + * @num_chars: the number of characters that can be displayed
>> + * @ops: character line display operations
>> + *
>
>Can you add a description here, please? Important note that it should be freed
>by the respective API afterwards.
>

Yes, I will clarify that attach/register operations must be freed using
their corresponding detach/unregister operations.

>> + * Return: zero on success, else a negative error code.
>> + */
>
>...
>
>> +	/* add attribute groups to target device */
>> +	err = device_add_groups(dev, linedisp_groups);
>> +	if (err)
>> +		goto out_del_attach;
>> +
>> +	/* display a default message */
>> +	err = linedisp_display(linedisp, LINEDISP_INIT_TEXT, -1);
>> +	if (err)
>> +		goto out_rem_groups;
>
>Can this be racy with user space? Once we publish attributes, the new
>message can be already issued and here it puts another message.
>OTOH this is done before device_add(), so the attributes are not visible
>until the device is added.
>

This concern is perfectly valid. linedisp_attach() can and will be called after
device_add() which can be racy. In fact, current linedisp_attach() replicates
the linedisp_register() logic which is also racy since it displays initial
message after device_add(). It needs to be fixed in both attach/register cases
by first initializing the display message before calling
device_add_groups()/device_add().

>...
>
>> +void linedisp_detach(struct device *dev)
>> +{
>
>> +	struct linedisp *linedisp = delete_attachment(dev, false);
>> +
>> +	if (!linedisp)
>> +		return;
>
>Please, rewrite as
>
>	struct linedisp *linedisp;
>
>	linedisp = delete_attachment(dev, false);
>	if (!linedisp)
>		return;
>

Acknowledged. I will separate assignment from declaration.

>> +	timer_delete_sync(&linedisp->timer);
>> +
>> +	device_remove_groups(dev, linedisp_groups);
>> +
>> +	kfree(linedisp->map);
>> +	kfree(linedisp->message);
>> +	kfree(linedisp->buf);
>> +}
>

On Tue, Sep 02, 2025 at 01:37:52PM -0400, Jean-François Lessard wrote:
> Le 2 septembre 2025 06 h 18 min 24 s HAE, Andy Shevchenko <andriy.shevchenko@intel.com> a écrit :
> >On Sun, Aug 31, 2025 at 10:00:28PM -0400, Jean-François Lessard wrote:

...

> >> +static DEFINE_SPINLOCK(linedisp_attachments_lock);
> >
> >Why spin lock and not mutex?
> 
> The attachment list operations are extremely lightweight (just adding/removing
> list entries), making spinlock the optimal choice because:
> - Very short critical sections: Only list traversal and pointer assignments;
>   avoids context switching overhead for brief operations
> - No sleeping operations: No memory allocation or I/O within locked sections
> - Future-proof atomic context safety: Can be safely called from interrupt
>   handlers if needed

To me it sounds like solving non-existing problem. I am sure we will see no
driver that tries to call this API in an atomic context.

-- 
With Best Regards,
Andy Shevchenko

Le 3 septembre 2025 06 h 18 min 18 s HAE, Andy Shevchenko <andriy.shevchenko@intel.com> a écrit :
>On Tue, Sep 02, 2025 at 01:37:52PM -0400, Jean-François Lessard wrote:
>> Le 2 septembre 2025 06 h 18 min 24 s HAE, Andy Shevchenko <andriy.shevchenko@intel.com> a écrit :
>> >On Sun, Aug 31, 2025 at 10:00:28PM -0400, Jean-François Lessard wrote:
>
>...
>
>> >> +static DEFINE_SPINLOCK(linedisp_attachments_lock);
>> >
>> >Why spin lock and not mutex?
>> 
>> The attachment list operations are extremely lightweight (just adding/removing
>> list entries), making spinlock the optimal choice because:
>> - Very short critical sections: Only list traversal and pointer assignments;
>>   avoids context switching overhead for brief operations
>> - No sleeping operations: No memory allocation or I/O within locked sections
>> - Future-proof atomic context safety: Can be safely called from interrupt
>>   handlers if needed
>
>To me it sounds like solving non-existing problem. I am sure we will see no
>driver that tries to call this API in an atomic context.
>

Yeah, I should have skipped "Future-proof atomic context safety".
I don't see how attach/detach would be called from atomic context.
Maybe to_linedisp(), but not in the current implementation anyway.