[PATCH v1 27/31] serial: 8250: Allow dynamic extension of uart_port attr_group

Crescent Hsieh posted 31 patches 1 day, 11 hours ago
[PATCH v1 27/31] serial: 8250: Allow dynamic extension of uart_port attr_group
Posted by Crescent Hsieh 1 day, 11 hours ago
Currently, uart_port->attr_group can only reference a statically
defined attribute_group, which prevents drivers from appending their
own sysfs attributes.

This enables drivers to inject custom sysfs entries without overriding
the core-provided rxtrig interface.

Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
---
 drivers/tty/serial/8250/8250_core.c |  8 ++++++++
 drivers/tty/serial/8250/8250_port.c | 26 ++++++++++++++++++++++++--
 include/linux/serial_core.h         |  1 +
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 3d8575874759..66b942e9e78a 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -812,6 +812,8 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
 			uart->port.set_rxtrig = up->port.set_rxtrig;
 		if (up->port.get_rxtrig)
 			uart->port.get_rxtrig = up->port.get_rxtrig;
+		if (up->port.attr_group)
+			uart->port.attr_group = up->port.attr_group;
 		if (up->dl_read)
 			uart->dl_read = up->dl_read;
 		if (up->dl_write)
@@ -895,6 +897,12 @@ void serial8250_unregister_port(int line)
 		uart->port.type = PORT_UNKNOWN;
 		uart->port.dev = &serial8250_isa_devs->dev;
 		uart->port.port_id = line;
+
+		if (uart->port.attr_group_allocated) {
+			kfree(uart->port.attr_group->attrs);
+			kfree(uart->port.attr_group);
+		}
+		uart->port.attr_group = NULL;
 		uart->capabilities = 0;
 		serial8250_init_port(uart);
 		serial8250_apply_quirks(uart);
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index d7baceacd4ff..e84879718a51 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -3192,9 +3192,31 @@ static struct attribute_group serial8250_dev_attr_group = {
 static void register_dev_spec_attr_grp(struct uart_8250_port *up)
 {
 	const struct serial8250_config *conf_type = &uart_config[up->port.type];
+	struct attribute **upp_attrs = NULL;
+	int upp_attr_num = 0, i;
 
-	if (conf_type->rxtrig_bytes[0])
-		up->port.attr_group = &serial8250_dev_attr_group;
+	up->port.attr_group_allocated = false;
+
+	if (up->port.attr_group) {
+		upp_attrs = up->port.attr_group->attrs;
+
+		while (upp_attrs[upp_attr_num])
+			upp_attr_num++;
+
+		up->port.attr_group = kcalloc(1, sizeof(struct attribute_group), GFP_KERNEL);
+		up->port.attr_group->attrs = kcalloc(upp_attr_num + 2, sizeof(struct attribute *), GFP_KERNEL);
+
+		for (i = 0; i < upp_attr_num; ++i)
+			up->port.attr_group->attrs[i] = upp_attrs[i];
+
+		if (conf_type->rxtrig_bytes[0])
+			up->port.attr_group->attrs[upp_attr_num] = &dev_attr_rx_trig_bytes.attr;
+
+		up->port.attr_group_allocated = true;
+	} else {
+		if (conf_type->rxtrig_bytes[0])
+			up->port.attr_group = &serial8250_dev_attr_group;
+	}
 }
 
 static void serial8250_config_port(struct uart_port *port, int flags)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 96646d3f2943..95ef7b08b6d8 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -590,6 +590,7 @@ struct uart_port {
 	unsigned char		console_reinit;
 	const char		*name;			/* port name */
 	struct attribute_group	*attr_group;		/* port specific attributes */
+	bool			attr_group_allocated;	/* whether attr_group is dynamic allocated */
 	const struct attribute_group **tty_groups;	/* all attributes (serial core use only) */
 	struct serial_rs485     rs485;
 	struct serial_rs485	rs485_supported;	/* Supported mask for serial_rs485 */
-- 
2.45.2
Re: [PATCH v1 27/31] serial: 8250: Allow dynamic extension of uart_port attr_group
Posted by Andy Shevchenko 19 hours ago
On Sun, Nov 30, 2025 at 12:45 PM Crescent Hsieh
<crescentcy.hsieh@moxa.com> wrote:
>
> Currently, uart_port->attr_group can only reference a statically
> defined attribute_group, which prevents drivers from appending their
> own sysfs attributes.
>
> This enables drivers to inject custom sysfs entries without overriding
> the core-provided rxtrig interface.


> +       if (up->port.attr_group) {
> +               upp_attrs = up->port.attr_group->attrs;
> +
> +               while (upp_attrs[upp_attr_num])
> +                       upp_attr_num++;

> +               up->port.attr_group = kcalloc(1, sizeof(struct attribute_group), GFP_KERNEL);

sizeof(*...)

No error check? And why calloc(1)? Shouldn't malloc() be enough?

> +               up->port.attr_group->attrs = kcalloc(upp_attr_num + 2, sizeof(struct attribute *), GFP_KERNEL);

sizeof(*)
Error check?

> +               for (i = 0; i < upp_attr_num; ++i)
> +                       up->port.attr_group->attrs[i] = upp_attrs[i];
> +
> +               if (conf_type->rxtrig_bytes[0])
> +                       up->port.attr_group->attrs[upp_attr_num] = &dev_attr_rx_trig_bytes.attr;
> +
> +               up->port.attr_group_allocated = true;
> +       } else {
> +               if (conf_type->rxtrig_bytes[0])
> +                       up->port.attr_group = &serial8250_dev_attr_group;
> +       }
>  }

...

> struct uart_port {

>         unsigned char           console_reinit;
>         const char              *name;                  /* port name */
>         struct attribute_group  *attr_group;            /* port specific attributes */
> +       bool                    attr_group_allocated;   /* whether attr_group is dynamic allocated */

Is this the best place? Have you run `pahole`?

>         const struct attribute_group **tty_groups;      /* all attributes (serial core use only) */
>         struct serial_rs485     rs485;
>         struct serial_rs485     rs485_supported;        /* Supported mask for serial_rs485 */

-- 
With Best Regards,
Andy Shevchenko