[PATCH 0/5] auxdisplay: linedisp: support attribute attachment to auxdisplay devices

[PATCH 0/5] auxdisplay: linedisp: support attribute attachment to auxdisplay devices

[Jean-François Lessard]

This series modernizes the auxdisplay line display (linedisp) library to
enable seamless integration with auxdisplay parent devices while
maintaining backward compatibility.

The key improvement is adding attach/detach APIs that allow linedisp sysfs
attributes to be bound directly to their parent auxdisplay devices avoiding
child device proliferation and enabling a uniform 7-segment userspace
interface across different driver architectures.

This series introduces attachment infrastructure for linedisp devices.
The first consumer of this API will be the TM16XX driver series.
See the related patch series:
  auxdisplay: Add TM16xx 7-segment LED matrix display controllers driver

Changes include:
1. Encapsulate container_of() usage with to_linedisp() helper function for
   cleaner context retrieval
2. Improve message display behavior with static padding when message length
   is smaller than display width
3. Add 'num_chars' read-only attribute for userspace capability discovery
4. Add attach/detach API for sysfs attributes binding to parent devices
5. Document all linedisp sysfs attributes in ABI documentation

All existing linedisp_register() users remain unaffected. The new APIs
enable drivers like TM16XX to integrate 7-segment functionality within
their LED class device hierarchy while providing a uniform 7-segment API.

Thanks to Andy Shevchenko for early feedback and guidance.

RFC changelog:
- Replace scope_guard() with guard()() for synchronized list operations.
- Replace NULL assignments with proper list_entry_is_head() pattern.
- Clearly document why introducing the attach/detach APIs.
- Split in patch series, each patch containing a specific change.
- Implement static (non-scrolling) display for short messages.
- Document exisiting and new ABI sysfs attributes.

Jean-François Lessard (5):
  auxdisplay: linedisp: encapsulate container_of usage within
    to_linedisp
  auxdisplay: linedisp: display static message when length <= display
    size
  auxdisplay: linedisp: add num_chars sysfs attribute
  auxdisplay: linedisp: support attribute attachment to auxdisplay
    devices
  docs: ABI: auxdisplay: document linedisp library sysfs attributes

 .../ABI/testing/sysfs-auxdisplay-linedisp     |  90 +++++++
 drivers/auxdisplay/line-display.c             | 219 ++++++++++++++++--
 drivers/auxdisplay/line-display.h             |   4 +
 3 files changed, 295 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-auxdisplay-linedisp

-- 
2.43.0

[PATCH 1/5] auxdisplay: linedisp: encapsulate container_of usage within to_linedisp

[Jean-François Lessard]

Replace direct container_of() calls with a to_linedisp() helper function
throughout the line-display auxdisplay library module. This abstraction
prepares for upcoming dual-mode support where linedisp context retrieval
will need to handle both dedicated child devices and attached parent
auxdisplay devices.

No functional changes in this patch.

Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 drivers/auxdisplay/line-display.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index 8590a4cd2..e44341b1e 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -31,6 +31,11 @@
 
 #define DEFAULT_SCROLL_RATE	(HZ / 2)
 
+static struct linedisp *to_linedisp(struct device *dev)
+{
+	return container_of(dev, struct linedisp, dev);
+}
+
 /**
  * linedisp_scroll() - scroll the display by a character
  * @t: really a pointer to the private data structure
@@ -133,7 +138,7 @@ static int linedisp_display(struct linedisp *linedisp, const char *msg,
 static ssize_t message_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
 {
-	struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+	struct linedisp *linedisp = to_linedisp(dev);
 
 	return sysfs_emit(buf, "%s\n", linedisp->message);
 }
@@ -152,7 +157,7 @@ static ssize_t message_show(struct device *dev, struct device_attribute *attr,
 static ssize_t message_store(struct device *dev, struct device_attribute *attr,
 			     const char *buf, size_t count)
 {
-	struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+	struct linedisp *linedisp = to_linedisp(dev);
 	int err;
 
 	err = linedisp_display(linedisp, buf, count);
@@ -164,7 +169,7 @@ static DEVICE_ATTR_RW(message);
 static ssize_t scroll_step_ms_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
-	struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+	struct linedisp *linedisp = to_linedisp(dev);
 
 	return sysfs_emit(buf, "%u\n", jiffies_to_msecs(linedisp->scroll_rate));
 }
@@ -173,7 +178,7 @@ static ssize_t scroll_step_ms_store(struct device *dev,
 				    struct device_attribute *attr,
 				    const char *buf, size_t count)
 {
-	struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+	struct linedisp *linedisp = to_linedisp(dev);
 	unsigned int ms;
 	int err;
 
@@ -195,7 +200,7 @@ static DEVICE_ATTR_RW(scroll_step_ms);
 
 static ssize_t map_seg_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
-	struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+	struct linedisp *linedisp = to_linedisp(dev);
 	struct linedisp_map *map = linedisp->map;
 
 	memcpy(buf, &map->map, map->size);
@@ -205,7 +210,7 @@ static ssize_t map_seg_show(struct device *dev, struct device_attribute *attr, c
 static ssize_t map_seg_store(struct device *dev, struct device_attribute *attr,
 			     const char *buf, size_t count)
 {
-	struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+	struct linedisp *linedisp = to_linedisp(dev);
 	struct linedisp_map *map = linedisp->map;
 
 	if (count != map->size)
@@ -232,7 +237,7 @@ static struct attribute *linedisp_attrs[] = {
 static umode_t linedisp_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
 {
 	struct device *dev = kobj_to_dev(kobj);
-	struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+	struct linedisp *linedisp = to_linedisp(dev);
 	struct linedisp_map *map = linedisp->map;
 	umode_t mode = attr->mode;
 
@@ -263,7 +268,7 @@ static DEFINE_IDA(linedisp_id);
 
 static void linedisp_release(struct device *dev)
 {
-	struct linedisp *linedisp = container_of(dev, struct linedisp, dev);
+	struct linedisp *linedisp = to_linedisp(dev);
 
 	kfree(linedisp->map);
 	kfree(linedisp->message);
-- 
2.43.0

[PATCH 2/5] auxdisplay: linedisp: display static message when length <= display size

[Jean-François Lessard]

Currently, when a message shorter than the display size is written, the
content wraps around (e.g., "123" on a 4-digit display shows "1231")
without scrolling, which is confusing and unintuitive.

Change behavior to display short messages statically with space padding
(e.g. "123 ") while only scrolling messages longer than the display width.
This provides more natural behavior that aligns with user expectations
and current linedisp_display() kernel-doc.

The scroll logic is also consolidated into a helper function for clarity.

No API changes are introduced.

Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 drivers/auxdisplay/line-display.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index e44341b1e..ea23c43bb 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -36,6 +36,11 @@ static struct linedisp *to_linedisp(struct device *dev)
 	return container_of(dev, struct linedisp, dev);
 }
 
+static inline bool should_scroll(struct linedisp *linedisp)
+{
+	return linedisp->message_len > linedisp->num_chars && linedisp->scroll_rate;
+}
+
 /**
  * linedisp_scroll() - scroll the display by a character
  * @t: really a pointer to the private data structure
@@ -67,7 +72,7 @@ static void linedisp_scroll(struct timer_list *t)
 	linedisp->scroll_pos %= linedisp->message_len;
 
 	/* rearm the timer */
-	if (linedisp->message_len > num_chars && linedisp->scroll_rate)
+	if (should_scroll(linedisp))
 		mod_timer(&linedisp->timer, jiffies + linedisp->scroll_rate);
 }
 
@@ -118,8 +123,16 @@ static int linedisp_display(struct linedisp *linedisp, const char *msg,
 	linedisp->message_len = count;
 	linedisp->scroll_pos = 0;
 
-	/* update the display */
-	linedisp_scroll(&linedisp->timer);
+	if (should_scroll(linedisp)) {
+		/* display scrolling message */
+		linedisp_scroll(&linedisp->timer);
+	} else {
+		/* display static message */
+		memset(linedisp->buf, ' ', linedisp->num_chars);
+		memcpy(linedisp->buf, linedisp->message,
+		       umin(linedisp->num_chars, linedisp->message_len));
+		linedisp->ops->update(linedisp);
+	}
 
 	return 0;
 }
@@ -186,12 +199,12 @@ static ssize_t scroll_step_ms_store(struct device *dev,
 	if (err)
 		return err;
 
+	timer_delete_sync(&linedisp->timer);
+
 	linedisp->scroll_rate = msecs_to_jiffies(ms);
-	if (linedisp->message && linedisp->message_len > linedisp->num_chars) {
-		timer_delete_sync(&linedisp->timer);
-		if (linedisp->scroll_rate)
-			linedisp_scroll(&linedisp->timer);
-	}
+
+	if (should_scroll(linedisp))
+		linedisp_scroll(&linedisp->timer);
 
 	return count;
 }
-- 
2.43.0

Re: [PATCH 2/5] auxdisplay: linedisp: display static message when length <= display size

[Andy Shevchenko]

On Sun, Aug 31, 2025 at 10:00:26PM -0400, Jean-François Lessard wrote:
> Currently, when a message shorter than the display size is written, the
> content wraps around (e.g., "123" on a 4-digit display shows "1231")
> without scrolling, which is confusing and unintuitive.
> 
> Change behavior to display short messages statically with space padding
> (e.g. "123 ") while only scrolling messages longer than the display width.
> This provides more natural behavior that aligns with user expectations
> and current linedisp_display() kernel-doc.
> 
> The scroll logic is also consolidated into a helper function for clarity.
> 
> No API changes are introduced.

...

>  /**
>   * linedisp_scroll() - scroll the display by a character
>   * @t: really a pointer to the private data structure

>  	linedisp->scroll_pos %= linedisp->message_len;
>  
>  	/* rearm the timer */
> -	if (linedisp->message_len > num_chars && linedisp->scroll_rate)
> +	if (should_scroll(linedisp))
>  		mod_timer(&linedisp->timer, jiffies + linedisp->scroll_rate);
>  }
>  

...

>  	linedisp->message_len = count;
>  	linedisp->scroll_pos = 0;
>  
> -	/* update the display */
> -	linedisp_scroll(&linedisp->timer);
> +	if (should_scroll(linedisp)) {
> +		/* display scrolling message */
> +		linedisp_scroll(&linedisp->timer);
> +	} else {
> +		/* display static message */
> +		memset(linedisp->buf, ' ', linedisp->num_chars);
> +		memcpy(linedisp->buf, linedisp->message,
> +		       umin(linedisp->num_chars, linedisp->message_len));
> +		linedisp->ops->update(linedisp);
> +	}

Hmm... But it seems the linedisp_scroll already has a check, why do we need
an additional one here? Perhaps we need to pad a message somewhere else and
guarantee it won't ever be less than num_chars?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/5] auxdisplay: linedisp: display static message when length <= display size

[Jean-François Lessard]

Le 2 septembre 2025 06 h 03 min 00 s HAE, Andy Shevchenko <andriy.shevchenko@intel.com> a écrit :
>On Sun, Aug 31, 2025 at 10:00:26PM -0400, Jean-François Lessard wrote:
>> Currently, when a message shorter than the display size is written, the
>> content wraps around (e.g., "123" on a 4-digit display shows "1231")
>> without scrolling, which is confusing and unintuitive.
>> 
>> Change behavior to display short messages statically with space padding
>> (e.g. "123 ") while only scrolling messages longer than the display width.
>> This provides more natural behavior that aligns with user expectations
>> and current linedisp_display() kernel-doc.
>> 
>> The scroll logic is also consolidated into a helper function for clarity.
>> 
>> No API changes are introduced.
>
>...
>
>>  /**
>>   * linedisp_scroll() - scroll the display by a character
>>   * @t: really a pointer to the private data structure
>
>>  	linedisp->scroll_pos %= linedisp->message_len;
>>  
>>  	/* rearm the timer */
>> -	if (linedisp->message_len > num_chars && linedisp->scroll_rate)
>> +	if (should_scroll(linedisp))
>>  		mod_timer(&linedisp->timer, jiffies + linedisp->scroll_rate);
>>  }
>>  
>
>...
>
>>  	linedisp->message_len = count;
>>  	linedisp->scroll_pos = 0;
>>  
>> -	/* update the display */
>> -	linedisp_scroll(&linedisp->timer);
>> +	if (should_scroll(linedisp)) {
>> +		/* display scrolling message */
>> +		linedisp_scroll(&linedisp->timer);
>> +	} else {
>> +		/* display static message */
>> +		memset(linedisp->buf, ' ', linedisp->num_chars);
>> +		memcpy(linedisp->buf, linedisp->message,
>> +		       umin(linedisp->num_chars, linedisp->message_len));
>> +		linedisp->ops->update(linedisp);
>> +	}
>
>Hmm... But it seems the linedisp_scroll already has a check, why do we need
>an additional one here? Perhaps we need to pad a message somewhere else and
>guarantee it won't ever be less than num_chars?
>

Semantically, linedisp_scroll should scroll. I think it's better to have two
distinct paths with their specific logic:
1. Scroll: circular display and rearm the timer
2. Static: padding and direct update

But you're absolutely right. Given the explicit should_scroll() conditional
outside, the check inside linedisp_scroll() is now redundant. I'll remove it
after testing to streamline the code paths and eliminate the duplication.

[PATCH 3/5] auxdisplay: linedisp: add num_chars sysfs attribute

[Jean-François Lessard]

Add a read-only 'num_chars' sysfs attribute to report display digit count.

The num_chars attribute provides essential capability information to
userspace applications that need to know display dimensions before writing
messages, complementing the existing message and scroll controls.

No functional changes to existing behavior.

Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 drivers/auxdisplay/line-display.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/auxdisplay/line-display.c b/drivers/auxdisplay/line-display.c
index ea23c43bb..abeed8812 100644
--- a/drivers/auxdisplay/line-display.c
+++ b/drivers/auxdisplay/line-display.c
@@ -211,6 +211,16 @@ static ssize_t scroll_step_ms_store(struct device *dev,
 
 static DEVICE_ATTR_RW(scroll_step_ms);
 
+static ssize_t num_chars_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct linedisp *linedisp = to_linedisp(dev);
+
+	return sysfs_emit(buf, "%u\n", linedisp->num_chars);
+}
+
+static DEVICE_ATTR_RO(num_chars);
+
 static ssize_t map_seg_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct linedisp *linedisp = to_linedisp(dev);
@@ -242,6 +252,7 @@ static DEVICE_ATTR(map_seg14, 0644, map_seg_show, map_seg_store);
 static struct attribute *linedisp_attrs[] = {
 	&dev_attr_message.attr,
 	&dev_attr_scroll_step_ms.attr,
+	&dev_attr_num_chars.attr,
 	&dev_attr_map_seg7.attr,
 	&dev_attr_map_seg14.attr,
 	NULL
-- 
2.43.0

Re: [PATCH 3/5] auxdisplay: linedisp: add num_chars sysfs attribute

[Andy Shevchenko]

On Sun, Aug 31, 2025 at 10:00:27PM -0400, Jean-François Lessard wrote:
> Add a read-only 'num_chars' sysfs attribute to report display digit count.
> 
> The num_chars attribute provides essential capability information to
> userspace applications that need to know display dimensions before writing
> messages, complementing the existing message and scroll controls.

...

>  	&dev_attr_message.attr,
>  	&dev_attr_scroll_step_ms.attr,
> +	&dev_attr_num_chars.attr,
>  	&dev_attr_map_seg7.attr,
>  	&dev_attr_map_seg14.attr,

It looks like we have two groups of the attributes here, can we keep the upper
one sorted? (Put the new attribute one line above?)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/5] auxdisplay: linedisp: add num_chars sysfs attribute

[Jean-François Lessard]

Le 2 septembre 2025 06 h 04 min 34 s HAE, Andy Shevchenko <andriy.shevchenko@intel.com> a écrit :
>On Sun, Aug 31, 2025 at 10:00:27PM -0400, Jean-François Lessard wrote:
>> Add a read-only 'num_chars' sysfs attribute to report display digit count.
>> 
>> The num_chars attribute provides essential capability information to
>> userspace applications that need to know display dimensions before writing
>> messages, complementing the existing message and scroll controls.
>
>...
>
>>  	&dev_attr_message.attr,
>>  	&dev_attr_scroll_step_ms.attr,
>> +	&dev_attr_num_chars.attr,
>>  	&dev_attr_map_seg7.attr,
>>  	&dev_attr_map_seg14.attr,
>
>It looks like we have two groups of the attributes here, can we keep the upper
>one sorted? (Put the new attribute one line above?)
>

Acknowledged. I will sort the attributes order accordingly.

[PATCH 4/5] auxdisplay: linedisp: support attribute attachment to auxdisplay devices

[Jean-François Lessard]

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

Re: [PATCH 4/5] auxdisplay: linedisp: support attribute attachment to auxdisplay devices

[Andy Shevchenko]

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

Re: [PATCH 4/5] auxdisplay: linedisp: support attribute attachment to auxdisplay devices

[Jean-François Lessard]

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);
>> +}
>

Re: [PATCH 4/5] auxdisplay: linedisp: support attribute attachment to auxdisplay devices

[Andy Shevchenko]

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

Re: [PATCH 4/5] auxdisplay: linedisp: support attribute attachment to auxdisplay devices

[Jean-François Lessard]

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.

[PATCH 5/5] docs: ABI: auxdisplay: document linedisp library sysfs attributes

[Jean-François Lessard]

Add ABI documentation for sysfs attributes provided by the line-display
auxdisplay library module. These attributes enable text message display and
configuration on character-based auxdisplay devices.

Documents previously undocumented attributes:
- message, scroll_step_ms (introduced in v5.16)
- map_seg7, map_seg14 (introduced in v6.9)

Documents newly added attribute:
- num_chars (targeted for v6.18)

The line-display library is used by multiple auxdisplay drivers and
can expose these attributes either on linedisp.N child devices or
directly on parent auxdisplay devices.

Signed-off-by: Jean-François Lessard <jefflessard3@gmail.com>
---
 .../ABI/testing/sysfs-auxdisplay-linedisp     | 90 +++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-auxdisplay-linedisp

diff --git a/Documentation/ABI/testing/sysfs-auxdisplay-linedisp b/Documentation/ABI/testing/sysfs-auxdisplay-linedisp
new file mode 100644
index 000000000..63c47f192
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-auxdisplay-linedisp
@@ -0,0 +1,90 @@
+What:		/sys/.../message
+Date:		October 2021
+KernelVersion:	5.16
+Description:
+		Controls the text message displayed on character line displays.
+
+		Reading returns the current message with a trailing newline.
+		Writing updates the displayed message. Messages longer than the
+		display width will automatically scroll. Trailing newlines in
+		input are automatically trimmed.
+
+		Writing an empty string clears the display.
+
+		Example:
+		  echo "Hello World" > message
+		  cat message			# Returns "Hello World\n"
+
+What:		/sys/.../scroll_step_ms
+Date:		October 2021
+KernelVersion:	5.16
+Description:
+		Controls the scrolling speed for messages longer than the display
+		width, specified in milliseconds per scroll step.
+
+		Setting to 0 disables scrolling. Default is 500ms.
+
+		Example:
+		  echo "250" > scroll_step_ms	# 4Hz scrolling
+		  cat scroll_step_ms		# Returns "250\n"
+
+What:		/sys/.../num_chars
+Date:		November 2025
+KernelVersion:	6.18
+Contact:	Jean-François Lessard <jefflessard3@gmail.com>
+Description:
+		Read-only attribute showing the character width capacity of
+		the line display device. Messages longer than this will scroll.
+
+		Example:
+		  cat num_chars		# Returns "16\n" for 16-char display
+
+What:		/sys/.../map_seg7
+Date:		January 2024
+KernelVersion:	6.9
+Description:
+		Read/write binary blob representing the ASCII-to-7-segment
+		display conversion table used by the linedisp driver, as defined
+		by struct seg7_conversion_map in <linux/map_to_7segment.h>.
+
+		Only visible on displays with 7-segment capability.
+
+		This attribute is not human-readable. Writes must match the
+		struct size exactly, else -EINVAL is returned; reads return the
+		entire mapping as a binary blob.
+
+		This interface and its implementation match existing conventions
+		used in segment-mapped display drivers since 2005.
+
+		ABI note: This style of binary sysfs attribute *is an exception*
+		to current "one value per file, text only" sysfs rules, for
+		historical compatibility and driver uniformity. New drivers are
+		discouraged from introducing additional binary sysfs ABIs.
+
+		Reference interface guidance:
+		- include/uapi/linux/map_to_7segment.h
+
+What:		/sys/.../map_seg14
+Date:		January 2024
+KernelVersion:	6.9
+Description:
+		Read/write binary blob representing the ASCII-to-14-segment
+		display conversion table used by the linedisp driver, as defined
+		by struct seg14_conversion_map in <linux/map_to_14segment.h>.
+
+		Only visible on displays with 14-segment capability.
+
+		This attribute is not human-readable. Writes must match the
+		struct size exactly, else -EINVAL is returned; reads return the
+		entire mapping as a binary blob.
+
+		This interface and its implementation match existing conventions
+		used by segment-mapped display drivers since 2005.
+
+		ABI note: This style of binary sysfs attribute *is an exception*
+		to current "one value per file, text only" sysfs rules, for
+		historical compatibility and driver uniformity. New drivers are
+		discouraged from introducing additional binary sysfs ABIs.
+
+		Reference interface guidance:
+		- include/uapi/linux/map_to_14segment.h
-- 
2.43.0

Re: [PATCH 5/5] docs: ABI: auxdisplay: document linedisp library sysfs attributes

[Andy Shevchenko]

On Sun, Aug 31, 2025 at 10:00:29PM -0400, Jean-François Lessard wrote:
> Add ABI documentation for sysfs attributes provided by the line-display
> auxdisplay library module. These attributes enable text message display and
> configuration on character-based auxdisplay devices.
> 
> Documents previously undocumented attributes:
> - message, scroll_step_ms (introduced in v5.16)
> - map_seg7, map_seg14 (introduced in v6.9)
> 
> Documents newly added attribute:
> - num_chars (targeted for v6.18)
> 
> The line-display library is used by multiple auxdisplay drivers and
> can expose these attributes either on linedisp.N child devices or
> directly on parent auxdisplay devices.

Can you split to two? Document undocumented but existing ones (as the first
patch in the series) and only add a num_chars when it's implemented?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 5/5] docs: ABI: auxdisplay: document linedisp library sysfs attributes

[Jean-François Lessard]

Le 2 septembre 2025 06 h 59 min 38 s HAE, Andy Shevchenko <andriy.shevchenko@intel.com> a écrit :
>On Sun, Aug 31, 2025 at 10:00:29PM -0400, Jean-François Lessard wrote:
>> Add ABI documentation for sysfs attributes provided by the line-display
>> auxdisplay library module. These attributes enable text message display and
>> configuration on character-based auxdisplay devices.
>> 
>> Documents previously undocumented attributes:
>> - message, scroll_step_ms (introduced in v5.16)
>> - map_seg7, map_seg14 (introduced in v6.9)
>> 
>> Documents newly added attribute:
>> - num_chars (targeted for v6.18)
>> 
>> The line-display library is used by multiple auxdisplay drivers and
>> can expose these attributes either on linedisp.N child devices or
>> directly on parent auxdisplay devices.
>
>Can you split to two? Document undocumented but existing ones (as the first
>patch in the series) and only add a num_chars when it's implemented?
>

Acknowledged. I will document the existing attributes in first patch of the
series and then document the new num_chars attribute within the num_chars patch
itself.

Re: [PATCH 0/5] auxdisplay: linedisp: support attribute attachment to auxdisplay devices

[Andy Shevchenko]

On Sun, Aug 31, 2025 at 10:00:24PM -0400, Jean-François Lessard wrote:
> This series modernizes the auxdisplay line display (linedisp) library to
> enable seamless integration with auxdisplay parent devices while
> maintaining backward compatibility.
> 
> The key improvement is adding attach/detach APIs that allow linedisp sysfs
> attributes to be bound directly to their parent auxdisplay devices avoiding
> child device proliferation and enabling a uniform 7-segment userspace
> interface across different driver architectures.
> 
> This series introduces attachment infrastructure for linedisp devices.
> The first consumer of this API will be the TM16XX driver series.
> See the related patch series:
>   auxdisplay: Add TM16xx 7-segment LED matrix display controllers driver
> 
> Changes include:
> 1. Encapsulate container_of() usage with to_linedisp() helper function for
>    cleaner context retrieval
> 2. Improve message display behavior with static padding when message length
>    is smaller than display width
> 3. Add 'num_chars' read-only attribute for userspace capability discovery
> 4. Add attach/detach API for sysfs attributes binding to parent devices
> 5. Document all linedisp sysfs attributes in ABI documentation
> 
> All existing linedisp_register() users remain unaffected. The new APIs
> enable drivers like TM16XX to integrate 7-segment functionality within
> their LED class device hierarchy while providing a uniform 7-segment API.
> 
> Thanks to Andy Shevchenko for early feedback and guidance.

Overall LGTM, only one question about spin lock vs. mutex. The rest is simple
nit-picks. I'll also wait for Geert's review / Acks.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/5] auxdisplay: linedisp: support attribute attachment to auxdisplay devices

[Jean-François Lessard]

Le 2 septembre 2025 07 h 00 min 35 s HAE, Andy Shevchenko <andriy.shevchenko@intel.com> a écrit :
>On Sun, Aug 31, 2025 at 10:00:24PM -0400, Jean-François Lessard wrote:
>> This series modernizes the auxdisplay line display (linedisp) library to
>> enable seamless integration with auxdisplay parent devices while
>> maintaining backward compatibility.
>> 
>> The key improvement is adding attach/detach APIs that allow linedisp sysfs
>> attributes to be bound directly to their parent auxdisplay devices avoiding
>> child device proliferation and enabling a uniform 7-segment userspace
>> interface across different driver architectures.
>> 
>> This series introduces attachment infrastructure for linedisp devices.
>> The first consumer of this API will be the TM16XX driver series.
>> See the related patch series:
>>   auxdisplay: Add TM16xx 7-segment LED matrix display controllers driver
>> 
>> Changes include:
>> 1. Encapsulate container_of() usage with to_linedisp() helper function for
>>    cleaner context retrieval
>> 2. Improve message display behavior with static padding when message length
>>    is smaller than display width
>> 3. Add 'num_chars' read-only attribute for userspace capability discovery
>> 4. Add attach/detach API for sysfs attributes binding to parent devices
>> 5. Document all linedisp sysfs attributes in ABI documentation
>> 
>> All existing linedisp_register() users remain unaffected. The new APIs
>> enable drivers like TM16XX to integrate 7-segment functionality within
>> their LED class device hierarchy while providing a uniform 7-segment API.
>> 
>> Thanks to Andy Shevchenko for early feedback and guidance.
>
>Overall LGTM, only one question about spin lock vs. mutex. The rest is simple
>nit-picks. I'll also wait for Geert's review / Acks.
>

Agreed. I will wait for Geert's feedback before submitting V2.