diff mbox series

[v4,3/3] serial: core: Fix serial core controller port name to show controller id

Message ID 20230724050709.17544-4-tony@atomide.com
State New
Headers
Series Serial core controller port device name fixes |

Commit Message

Tony Lindgren July 24, 2023, 5:07 a.m. UTC 
  We are missing the serial core controller id for the serial core port
name. Let's fix the issue for sane /sys/bus/serial-core/devices, and to
avoid issues addressing serial ports later on.

Fixes: 84a9582fd203 ("serial: core: Start managing serial controllers to enable runtime PM")
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/tty/serial/serial_base_bus.c | 34 ++++++++++++++++++----------
 1 file changed, 22 insertions(+), 12 deletions(-)

Comments

Andy Shevchenko July 24, 2023, 8:55 a.m. UTC | #1 
On Mon, Jul 24, 2023 at 08:07:05AM +0300, Tony Lindgren wrote:
> We are missing the serial core controller id for the serial core port
> name. Let's fix the issue for sane /sys/bus/serial-core/devices, and to
> avoid issues addressing serial ports later on.

...

> -	return dev_set_name(dev, "%s.%s.%d", type->name, dev_name(port->dev), id);
> -}
> +	if (type == &serial_ctrl_type)

	...

> +}

While at it, have you tried to use --patience when forming patches? Does it
make any (better) difference?
Jiri Slaby July 24, 2023, 9:06 a.m. UTC | #2 
On 24. 07. 23, 10:55, Andy Shevchenko wrote:
> On Mon, Jul 24, 2023 at 08:07:05AM +0300, Tony Lindgren wrote:
>> We are missing the serial core controller id for the serial core port
>> name. Let's fix the issue for sane /sys/bus/serial-core/devices, and to
>> avoid issues addressing serial ports later on.
> 
> ...
> 
>> -	return dev_set_name(dev, "%s.%s.%d", type->name, dev_name(port->dev), id);
>> -}
>> +	if (type == &serial_ctrl_type)
> 
> 	...
> 
>> +}
> 
> While at it, have you tried to use --patience when forming patches? Does it
> make any (better) difference?

Or moving "struct device_type"s to one place in a separate patch might 
do the desired trick too ;).
Tony Lindgren July 24, 2023, 9:08 a.m. UTC | #3 
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> [230724 08:56]:
> On Mon, Jul 24, 2023 at 08:07:05AM +0300, Tony Lindgren wrote:
> > We are missing the serial core controller id for the serial core port
> > name. Let's fix the issue for sane /sys/bus/serial-core/devices, and to
> > avoid issues addressing serial ports later on.
> 
> ...
> 
> > -	return dev_set_name(dev, "%s.%s.%d", type->name, dev_name(port->dev), id);
> > -}
> > +	if (type == &serial_ctrl_type)
> 
> 	...
> 
> > +}
> 
> While at it, have you tried to use --patience when forming patches? Does it
> make any (better) difference?

I'll check if it makes it more readable. Seems I need to also enable the
--patience option for myself for sending patches to avoid messing up things
like Reviewed-by tags :)

Tony
diff mbox series

Patch

diff --git a/drivers/tty/serial/serial_base_bus.c b/drivers/tty/serial/serial_base_bus.c
--- a/drivers/tty/serial/serial_base_bus.c
+++ b/drivers/tty/serial/serial_base_bus.c
@@ -19,6 +19,14 @@ 
 
 static bool serial_base_initialized;
 
+static const struct device_type serial_ctrl_type = {
+	.name = "ctrl",
+};
+
+static const struct device_type serial_port_type = {
+	.name = "port",
+};
+
 static int serial_base_match(struct device *dev, struct device_driver *drv)
 {
 	int len = strlen(drv->name);
@@ -48,7 +56,8 @@  static int serial_base_device_init(struct uart_port *port,
 				   struct device *parent_dev,
 				   const struct device_type *type,
 				   void (*release)(struct device *dev),
-				   int id)
+				   unsigned int ctrl_id,
+				   unsigned int port_id)
 {
 	device_initialize(dev);
 	dev->type = type;
@@ -61,12 +70,17 @@  static int serial_base_device_init(struct uart_port *port,
 		return -EPROBE_DEFER;
 	}
 
-	return dev_set_name(dev, "%s.%s.%d", type->name, dev_name(port->dev), id);
-}
+	if (type == &serial_ctrl_type)
+		return dev_set_name(dev, "%s.%s.%d", type->name,
+				    dev_name(port->dev), ctrl_id);
 
-static const struct device_type serial_ctrl_type = {
-	.name = "ctrl",
-};
+	if (type == &serial_port_type)
+		return dev_set_name(dev, "%s.%s.%d.%d", type->name,
+				    dev_name(port->dev), ctrl_id,
+				    port_id);
+
+	return -EINVAL;
+}
 
 static void serial_base_ctrl_release(struct device *dev)
 {
@@ -96,7 +110,7 @@  struct serial_ctrl_device *serial_base_ctrl_add(struct uart_port *port,
 	err = serial_base_device_init(port, &ctrl_dev->dev,
 				      parent, &serial_ctrl_type,
 				      serial_base_ctrl_release,
-				      port->ctrl_id);
+				      port->ctrl_id, 0);
 	if (err)
 		goto err_put_device;
 
@@ -112,10 +126,6 @@  struct serial_ctrl_device *serial_base_ctrl_add(struct uart_port *port,
 	return ERR_PTR(err);
 }
 
-static const struct device_type serial_port_type = {
-	.name = "port",
-};
-
 static void serial_base_port_release(struct device *dev)
 {
 	struct serial_port_device *port_dev = to_serial_base_port_device(dev);
@@ -136,7 +146,7 @@  struct serial_port_device *serial_base_port_add(struct uart_port *port,
 	err = serial_base_device_init(port, &port_dev->dev,
 				      &ctrl_dev->dev, &serial_port_type,
 				      serial_base_port_release,
-				      port->port_id);
+				      port->ctrl_id, port->port_id);
 	if (err)
 		goto err_put_device;