[PATCH v2] serial: serial_core.c: printk replacement

[PATCH v2] serial: serial_core.c: printk replacement

[Sudip Mukherjee]

printk replaced with corresponding pr_err, dev_alert, dev_notice and pr_info.
fixed two broken user-visible strings used by the corresponding printk

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

In the first patch i sent Greg suggested to use dev_err instead of pr_err , but as later Jiri pointed out in my later patches that uport->dev is having a null check so can not be used , tty_dev is also having a put_device , so back to pr_err. 

 drivers/tty/serial/serial_core.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 29a7be4..0e03c31 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -892,10 +892,11 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
 			 */
 			if (uport->flags & UPF_SPD_MASK) {
 				char buf[64];
-				printk(KERN_NOTICE
-				       "%s sets custom speed on %s. This "
-				       "is deprecated.\n", current->comm,
-				       tty_name(port->tty, buf));
+
+				dev_notice(uport->dev,
+				       "%s sets custom speed on %s. This is deprecated.\n",
+				      current->comm,
+				      tty_name(port->tty, buf));
 			}
 			uart_change_speed(tty, state, NULL);
 		}
@@ -1975,12 +1976,11 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 		for (tries = 3; !ops->tx_empty(uport) && tries; tries--)
 			msleep(10);
 		if (!tries)
-			printk(KERN_ERR "%s%s%s%d: Unable to drain "
-					"transmitter\n",
-			       uport->dev ? dev_name(uport->dev) : "",
-			       uport->dev ? ": " : "",
-			       drv->dev_name,
-			       drv->tty_driver->name_base + uport->line);
+			pr_err("%s%s%s%d: Unable to drain transmitter\n",
+				uport->dev ? dev_name(uport->dev) : "",
+				uport->dev ? ": " : "",
+				drv->dev_name,
+				drv->tty_driver->name_base + uport->line);
 
 		if (console_suspend_enabled || !uart_console(uport))
 			ops->shutdown(uport);
@@ -2109,7 +2109,7 @@ uart_report_port(struct uart_driver *drv, struct uart_port *port)
 		break;
 	}
 
-	printk(KERN_INFO "%s%s%s%d at %s (irq = %d, base_baud = %d) is a %s\n",
+	pr_info("%s%s%s%d at %s (irq = %d, base_baud = %d) is a %s\n",
 	       port->dev ? dev_name(port->dev) : "",
 	       port->dev ? ": " : "",
 	       drv->dev_name,
@@ -2640,7 +2640,7 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport)
 	if (likely(!IS_ERR(tty_dev))) {
 		device_set_wakeup_capable(tty_dev, 1);
 	} else {
-		printk(KERN_ERR "Cannot register tty device on line %d\n",
+		dev_err(uport->dev, "Cannot register tty device on line %d\n",
 		       uport->line);
 	}
 
@@ -2675,7 +2675,7 @@ int uart_remove_one_port(struct uart_driver *drv, struct uart_port *uport)
 	BUG_ON(in_interrupt());
 
 	if (state->uart_port != uport)
-		printk(KERN_ALERT "Removing wrong port: %p != %p\n",
+		dev_alert(uport->dev, "Removing wrong port: %p != %p\n",
 			state->uart_port, uport);
 
 	mutex_lock(&port_mutex);
-- 
1.8.1.2

Re: [PATCH v2] serial: serial_core.c: printk replacement

[Ryan Mallon]

On 29/08/14 00:15, Sudip Mukherjee wrote:
> printk replaced with corresponding pr_err, dev_alert, dev_notice and pr_info.
> fixed two broken user-visible strings used by the corresponding printk
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> 
> In the first patch i sent Greg suggested to use dev_err instead of pr_err , but as later Jiri pointed out in my later patches that uport->dev is having a null check so can not be used , tty_dev is also having a put_device , so back to pr_err. 
> 
>  drivers/tty/serial/serial_core.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 

> @@ -1975,12 +1976,11 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
>  		for (tries = 3; !ops->tx_empty(uport) && tries; tries--)
>  			msleep(10);
>  		if (!tries)
> -			printk(KERN_ERR "%s%s%s%d: Unable to drain "
> -					"transmitter\n",
> -			       uport->dev ? dev_name(uport->dev) : "",
> -			       uport->dev ? ": " : "",
> -			       drv->dev_name,
> -			       drv->tty_driver->name_base + uport->line);
> +			pr_err("%s%s%s%d: Unable to drain transmitter\n",
> +				uport->dev ? dev_name(uport->dev) : "",
> +				uport->dev ? ": " : "",
> +				drv->dev_name,
> +				drv->tty_driver->name_base + uport->line);

dev_printk() prints the value of dev_name() and handles the NULL case,
so this could be changed to:

	dev_err(uport->dev, "%s%d: Unable to drain transmitter\n",
		drv->dev_name,
		drv->tty_driver->name_base + uport->line);

It might also be possible to remove drv->dev_name since dev_printk()
also prints the driver string, but I don't know if they are equivalent
in this case.

>  
>  		if (console_suspend_enabled || !uart_console(uport))
>  			ops->shutdown(uport);
> @@ -2109,7 +2109,7 @@ uart_report_port(struct uart_driver *drv, struct uart_port *port)
>  		break;
>  	}
>  
> -	printk(KERN_INFO "%s%s%s%d at %s (irq = %d, base_baud = %d) is a %s\n",
> +	pr_info("%s%s%s%d at %s (irq = %d, base_baud = %d) is a %s\n",
>  	       port->dev ? dev_name(port->dev) : "",
>  	       port->dev ? ": " : "",
>  	       drv->dev_name,

Same here.

~Ryan

Re: [PATCH v2] serial: serial_core.c: printk replacement

[Sudip Mukherjee]

On Mon, Sep 01, 2014 at 10:11:29AM +1000, Ryan Mallon wrote:
> 
> 
> On 29/08/14 00:15, Sudip Mukherjee wrote:
> > printk replaced with corresponding pr_err, dev_alert, dev_notice and pr_info.
> > fixed two broken user-visible strings used by the corresponding printk
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > ---
> > 
> > In the first patch i sent Greg suggested to use dev_err instead of pr_err , but as later Jiri pointed out in my later patches that uport->dev is having a null check so can not be used , tty_dev is also having a put_device , so back to pr_err. 
> > 
> >  drivers/tty/serial/serial_core.c | 26 +++++++++++++-------------
> >  1 file changed, 13 insertions(+), 13 deletions(-)
> > 
> 
> > @@ -1975,12 +1976,11 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
> >  		for (tries = 3; !ops->tx_empty(uport) && tries; tries--)
> >  			msleep(10);
> >  		if (!tries)
> > -			printk(KERN_ERR "%s%s%s%d: Unable to drain "
> > -					"transmitter\n",
> > -			       uport->dev ? dev_name(uport->dev) : "",
> > -			       uport->dev ? ": " : "",
> > -			       drv->dev_name,
> > -			       drv->tty_driver->name_base + uport->line);
> > +			pr_err("%s%s%s%d: Unable to drain transmitter\n",
> > +				uport->dev ? dev_name(uport->dev) : "",
> > +				uport->dev ? ": " : "",
> > +				drv->dev_name,
> > +				drv->tty_driver->name_base + uport->line);
> 
> dev_printk() prints the value of dev_name() and handles the NULL case,
> so this could be changed to:
> 
> 	dev_err(uport->dev, "%s%d: Unable to drain transmitter\n",
> 		drv->dev_name,
> 		drv->tty_driver->name_base + uport->line);
> 
> It might also be possible to remove drv->dev_name since dev_printk()
> also prints the driver string, but I don't know if they are equivalent
> in this case.
> 

they are not equivalent. drv->dev_name is giving "ttyS" , and the driver string as printed by dev_printk() is "serial".
So, I guess I will make a v3 of the patch and mention all these in the commit message.

thanks
sudip

> >  
> >  		if (console_suspend_enabled || !uart_console(uport))
> >  			ops->shutdown(uport);
> > @@ -2109,7 +2109,7 @@ uart_report_port(struct uart_driver *drv, struct uart_port *port)
> >  		break;
> >  	}
> >  
> > -	printk(KERN_INFO "%s%s%s%d at %s (irq = %d, base_baud = %d) is a %s\n",
> > +	pr_info("%s%s%s%d at %s (irq = %d, base_baud = %d) is a %s\n",
> >  	       port->dev ? dev_name(port->dev) : "",
> >  	       port->dev ? ": " : "",
> >  	       drv->dev_name,
> 
> Same here.
> 
> ~Ryan