[PATCH 00/13] tty: random fixes and cleanups

[PATCH 00/13] tty: random fixes and cleanups

[Jiri Slaby (SUSE)]

Hi,

this is a series of locally accumulated patches over past months.

The series:
* makes mctp and 6pack use u8s,
* cleans up 6pack a bit,
* fixes two coverity reports,
* uses guard() to make some of the tty function easier to follow.

Cc: Andreas Koensgen <ajk@comnets.uni-bremen.de>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Jeremy Kerr <jk@codeconstruct.com.au>
Cc: linux-hams@vger.kernel.org
Cc: Matt Johnston <matt@codeconstruct.com.au>
Cc: netdev@vger.kernel.org
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Peter Hurley <peter@hurleysoftware.com>


Jiri Slaby (SUSE) (13):
  tty: simplify tty_dev_name_to_number() using guard(mutex)
  serial: protect uart_port_dtr_rts() in uart_shutdown() too
  serial: don't use uninitialized value in uart_poll_init()
  serial: remove quot_frac from serial8250_do_set_divisor()
  serial: use guards for simple mutex locks
  mxser: remove stale comment
  mxser: remove doubled sets of close times
  mctp: serial: propagage new tty types
  6pack: remove sixpack::rbuff
  6pack: drop sixpack::mtu
  6pack: drop sixpack::buffsize
  6pack: remove global strings
  6pack: propagage new tty types

 drivers/net/hamradio/6pack.c         |  60 ++++--------
 drivers/net/mctp/mctp-serial.c       |  23 ++---
 drivers/tty/mxser.c                  |   5 -
 drivers/tty/serial/8250/8250_dwlib.c |   2 +-
 drivers/tty/serial/8250/8250_exar.c  |   2 +-
 drivers/tty/serial/8250/8250_pci.c   |   2 +-
 drivers/tty/serial/8250/8250_port.c  |   4 +-
 drivers/tty/serial/serial_core.c     | 140 ++++++++++++---------------
 drivers/tty/tty_io.c                 |  11 +--
 include/linux/serial_8250.h          |   2 +-
 10 files changed, 103 insertions(+), 148 deletions(-)

-- 
2.46.0

[PATCH 01/13] tty: simplify tty_dev_name_to_number() using guard(mutex)

[Jiri Slaby (SUSE)]

In tty_dev_name_to_number(), a guard can help to make the code easier to
follow. Especially how 0 is returned in the successful case. So use a
guard there.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/tty_io.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index bc9aebcb873f..267682bcfea0 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -350,22 +350,19 @@ int tty_dev_name_to_number(const char *name, dev_t *number)
 		return ret;
 
 	prefix_length = str - name;
-	mutex_lock(&tty_mutex);
+
+	guard(mutex)(&tty_mutex);
 
 	list_for_each_entry(p, &tty_drivers, tty_drivers)
 		if (prefix_length == strlen(p->name) && strncmp(name,
 					p->name, prefix_length) == 0) {
 			if (index < p->num) {
 				*number = MKDEV(p->major, p->minor_start + index);
-				goto out;
+				return 0;
 			}
 		}
 
-	/* if here then driver wasn't found */
-	ret = -ENODEV;
-out:
-	mutex_unlock(&tty_mutex);
-	return ret;
+	return -ENODEV;
 }
 EXPORT_SYMBOL_GPL(tty_dev_name_to_number);
 
-- 
2.46.0

Re: [PATCH 01/13] tty: simplify tty_dev_name_to_number() using guard(mutex)

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

On Mon, 5 Aug 2024, Jiri Slaby (SUSE) wrote:

> In tty_dev_name_to_number(), a guard can help to make the code easier to
> follow. Especially how 0 is returned in the successful case. So use a
> guard there.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/tty_io.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index bc9aebcb873f..267682bcfea0 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -350,22 +350,19 @@ int tty_dev_name_to_number(const char *name, dev_t *number)
>  		return ret;
>  
>  	prefix_length = str - name;
> -	mutex_lock(&tty_mutex);
> +
> +	guard(mutex)(&tty_mutex);
>  
>  	list_for_each_entry(p, &tty_drivers, tty_drivers)
>  		if (prefix_length == strlen(p->name) && strncmp(name,
>  					p->name, prefix_length) == 0) {
>  			if (index < p->num) {
>  				*number = MKDEV(p->major, p->minor_start + index);
> -				goto out;
> +				return 0;
>  			}
>  		}
>  
> -	/* if here then driver wasn't found */
> -	ret = -ENODEV;
> -out:
> -	mutex_unlock(&tty_mutex);
> -	return ret;
> +	return -ENODEV;
>  }
>  EXPORT_SYMBOL_GPL(tty_dev_name_to_number);

Should add also #include <linux/cleanup.h>. With that fixed:

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

[PATCH 02/13] serial: protect uart_port_dtr_rts() in uart_shutdown() too

[Jiri Slaby (SUSE)]

Commit af224ca2df29 (serial: core: Prevent unsafe uart port access, part
3) added few uport == NULL checks. It added one to uart_shutdown(), so
the commit assumes, uport can be NULL in there. But right after that
protection, there is an unprotected "uart_port_dtr_rts(uport, false);"
call. That is invoked only if HUPCL is set, so I assume that is the
reason why we do not see lots of these reports.

Or it cannot be NULL at this point at all for some reason :P.

Until the above is investigated, stay on the safe side and move this
dereference to the if too.

I got this inconsistency from Coverity under CID 1585130. Thanks.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/serial_core.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 61c7e1268957..3afe77f05abf 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -407,14 +407,16 @@ static void uart_shutdown(struct tty_struct *tty, struct uart_state *state)
 		/*
 		 * Turn off DTR and RTS early.
 		 */
-		if (uport && uart_console(uport) && tty) {
-			uport->cons->cflag = tty->termios.c_cflag;
-			uport->cons->ispeed = tty->termios.c_ispeed;
-			uport->cons->ospeed = tty->termios.c_ospeed;
-		}
+		if (uport) {
+			if (uart_console(uport) && tty) {
+				uport->cons->cflag = tty->termios.c_cflag;
+				uport->cons->ispeed = tty->termios.c_ispeed;
+				uport->cons->ospeed = tty->termios.c_ospeed;
+			}
 
-		if (!tty || C_HUPCL(tty))
-			uart_port_dtr_rts(uport, false);
+			if (!tty || C_HUPCL(tty))
+				uart_port_dtr_rts(uport, false);
+		}
 
 		uart_port_shutdown(port);
 	}
-- 
2.46.0

[PATCH 03/13] serial: don't use uninitialized value in uart_poll_init()

[Jiri Slaby (SUSE)]

Coverity reports (as CID 1536978) that uart_poll_init() passes
uninitialized pm_state to uart_change_pm(). It is in case the first 'if'
takes the true branch (does "goto out;").

Fix this and simplify the function by simple guard(mutex). The code
needs no labels after this at all. And it is pretty clear that the code
has not fiddled with pm_state at that point.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Fixes: 5e227ef2aa38 (serial: uart_poll_init() should power on the UART)
Cc: stable@vger.kernel.org
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/serial_core.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 3afe77f05abf..d63e9b636e02 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -2690,14 +2690,13 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
 	int ret = 0;
 
 	tport = &state->port;
-	mutex_lock(&tport->mutex);
+
+	guard(mutex)(&tport->mutex);
 
 	port = uart_port_check(state);
 	if (!port || port->type == PORT_UNKNOWN ||
-	    !(port->ops->poll_get_char && port->ops->poll_put_char)) {
-		ret = -1;
-		goto out;
-	}
+	    !(port->ops->poll_get_char && port->ops->poll_put_char))
+		return -1;
 
 	pm_state = state->pm_state;
 	uart_change_pm(state, UART_PM_STATE_ON);
@@ -2717,10 +2716,10 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
 		ret = uart_set_options(port, NULL, baud, parity, bits, flow);
 		console_list_unlock();
 	}
-out:
+
 	if (ret)
 		uart_change_pm(state, pm_state);
-	mutex_unlock(&tport->mutex);
+
 	return ret;
 }
 
-- 
2.46.0

Re: [PATCH 03/13] serial: don't use uninitialized value in uart_poll_init()

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2063 bytes --]

On Mon, 5 Aug 2024, Jiri Slaby (SUSE) wrote:

> Coverity reports (as CID 1536978) that uart_poll_init() passes
> uninitialized pm_state to uart_change_pm(). It is in case the first 'if'
> takes the true branch (does "goto out;").
> 
> Fix this and simplify the function by simple guard(mutex). The code
> needs no labels after this at all. And it is pretty clear that the code
> has not fiddled with pm_state at that point.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Fixes: 5e227ef2aa38 (serial: uart_poll_init() should power on the UART)
> Cc: stable@vger.kernel.org
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/serial_core.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 3afe77f05abf..d63e9b636e02 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -2690,14 +2690,13 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
>  	int ret = 0;
>  
>  	tport = &state->port;
> -	mutex_lock(&tport->mutex);
> +
> +	guard(mutex)(&tport->mutex);
>  
>  	port = uart_port_check(state);
>  	if (!port || port->type == PORT_UNKNOWN ||
> -	    !(port->ops->poll_get_char && port->ops->poll_put_char)) {
> -		ret = -1;
> -		goto out;
> -	}
> +	    !(port->ops->poll_get_char && port->ops->poll_put_char))
> +		return -1;
>  
>  	pm_state = state->pm_state;
>  	uart_change_pm(state, UART_PM_STATE_ON);
> @@ -2717,10 +2716,10 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
>  		ret = uart_set_options(port, NULL, baud, parity, bits, flow);
>  		console_list_unlock();
>  	}
> -out:
> +
>  	if (ret)
>  		uart_change_pm(state, pm_state);
> -	mutex_unlock(&tport->mutex);
> +
>  	return ret;
>  }

This too needs #include.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH 03/13] serial: don't use uninitialized value in uart_poll_init()

[Doug Anderson]

Hi,

On Mon, Aug 5, 2024 at 7:28 AM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Mon, 5 Aug 2024, Jiri Slaby (SUSE) wrote:
>
> > Coverity reports (as CID 1536978) that uart_poll_init() passes
> > uninitialized pm_state to uart_change_pm(). It is in case the first 'if'
> > takes the true branch (does "goto out;").
> >
> > Fix this and simplify the function by simple guard(mutex). The code
> > needs no labels after this at all. And it is pretty clear that the code
> > has not fiddled with pm_state at that point.
> >
> > Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> > Fixes: 5e227ef2aa38 (serial: uart_poll_init() should power on the UART)
> > Cc: stable@vger.kernel.org
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  drivers/tty/serial/serial_core.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > index 3afe77f05abf..d63e9b636e02 100644
> > --- a/drivers/tty/serial/serial_core.c
> > +++ b/drivers/tty/serial/serial_core.c
> > @@ -2690,14 +2690,13 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
> >       int ret = 0;
> >
> >       tport = &state->port;
> > -     mutex_lock(&tport->mutex);
> > +
> > +     guard(mutex)(&tport->mutex);
> >
> >       port = uart_port_check(state);
> >       if (!port || port->type == PORT_UNKNOWN ||
> > -         !(port->ops->poll_get_char && port->ops->poll_put_char)) {
> > -             ret = -1;
> > -             goto out;
> > -     }
> > +         !(port->ops->poll_get_char && port->ops->poll_put_char))
> > +             return -1;
> >
> >       pm_state = state->pm_state;
> >       uart_change_pm(state, UART_PM_STATE_ON);
> > @@ -2717,10 +2716,10 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
> >               ret = uart_set_options(port, NULL, baud, parity, bits, flow);
> >               console_list_unlock();
> >       }
> > -out:
> > +
> >       if (ret)
> >               uart_change_pm(state, pm_state);
> > -     mutex_unlock(&tport->mutex);
> > +
> >       return ret;
> >  }
>
> This too needs #include.

Why? I see in "mutex.h" (which is already included by serial_core.c):

DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))

...so we're using the mutex guard and including the header file that
defines the mutex guard. Seems like it's all legit to me.

Re: [PATCH 03/13] serial: don't use uninitialized value in uart_poll_init()

[Jiri Slaby]

On 05. 08. 24, 17:46, Doug Anderson wrote:
>>> @@ -2717,10 +2716,10 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
>>>                ret = uart_set_options(port, NULL, baud, parity, bits, flow);
>>>                console_list_unlock();
>>>        }
>>> -out:
>>> +
>>>        if (ret)
>>>                uart_change_pm(state, pm_state);
>>> -     mutex_unlock(&tport->mutex);
>>> +
>>>        return ret;
>>>   }
>>
>> This too needs #include.
> 
> Why? I see in "mutex.h" (which is already included by serial_core.c):
> 
> DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))
> 
> ...so we're using the mutex guard and including the header file that
> defines the mutex guard. Seems like it's all legit to me.

The patches got merged. But I can post a fix on top, of course. But, 
what is the consensus here -- include or not to include? I assume 
mutex.h includes cleanup.h already due to the above guard definition.

thanks,
-- 
js
suse labs

Re: [PATCH 03/13] serial: don't use uninitialized value in uart_poll_init()

[Greg Kroah-Hartman]

On Thu, Aug 08, 2024 at 09:34:33AM +0200, Jiri Slaby wrote:
> On 05. 08. 24, 17:46, Doug Anderson wrote:
> > > > @@ -2717,10 +2716,10 @@ static int uart_poll_init(struct tty_driver *driver, int line, char *options)
> > > >                ret = uart_set_options(port, NULL, baud, parity, bits, flow);
> > > >                console_list_unlock();
> > > >        }
> > > > -out:
> > > > +
> > > >        if (ret)
> > > >                uart_change_pm(state, pm_state);
> > > > -     mutex_unlock(&tport->mutex);
> > > > +
> > > >        return ret;
> > > >   }
> > > 
> > > This too needs #include.
> > 
> > Why? I see in "mutex.h" (which is already included by serial_core.c):
> > 
> > DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))
> > 
> > ...so we're using the mutex guard and including the header file that
> > defines the mutex guard. Seems like it's all legit to me.
> 
> The patches got merged. But I can post a fix on top, of course. But, what is
> the consensus here -- include or not to include? I assume mutex.h includes
> cleanup.h already due to the above guard definition.

Leave it as-is, it's not breaking the build anywhere, so not a problem.

thanks,

greg k-h

Re: [PATCH 03/13] serial: don't use uninitialized value in uart_poll_init()

[Ilpo Järvinen]

On Thu, 8 Aug 2024, Jiri Slaby wrote:

> On 05. 08. 24, 17:46, Doug Anderson wrote:
> > > > @@ -2717,10 +2716,10 @@ static int uart_poll_init(struct tty_driver
> > > > *driver, int line, char *options)
> > > >                ret = uart_set_options(port, NULL, baud, parity, bits,
> > > > flow);
> > > >                console_list_unlock();
> > > >        }
> > > > -out:
> > > > +
> > > >        if (ret)
> > > >                uart_change_pm(state, pm_state);
> > > > -     mutex_unlock(&tport->mutex);
> > > > +
> > > >        return ret;
> > > >   }
> > > 
> > > This too needs #include.
> > 
> > Why? I see in "mutex.h" (which is already included by serial_core.c):
> > 
> > DEFINE_GUARD(mutex, struct mutex *, mutex_lock(_T), mutex_unlock(_T))
> > 
> > ...so we're using the mutex guard and including the header file that
> > defines the mutex guard. Seems like it's all legit to me.
> 
> The patches got merged. But I can post a fix on top, of course. But, what is
> the consensus here -- include or not to include? I assume mutex.h includes
> cleanup.h already due to the above guard definition.

Yeah, while guard() itself is in cleanup.h, Doug has a point that 
DEFINE_GUARD() creates a guaranteed implicit include route for cleanup.h. 
Thus you can disregard my comment as it seems unnecessary to include 
cleanup.h.

-- 
 i.

Re: [PATCH 03/13] serial: don't use uninitialized value in uart_poll_init()

[Doug Anderson]

Hi,

On Mon, Aug 5, 2024 at 3:21 AM Jiri Slaby (SUSE) <jirislaby@kernel.org> wrote:
>
> Coverity reports (as CID 1536978) that uart_poll_init() passes
> uninitialized pm_state to uart_change_pm(). It is in case the first 'if'
> takes the true branch (does "goto out;").
>
> Fix this and simplify the function by simple guard(mutex). The code
> needs no labels after this at all. And it is pretty clear that the code
> has not fiddled with pm_state at that point.
>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Fixes: 5e227ef2aa38 (serial: uart_poll_init() should power on the UART)
> Cc: stable@vger.kernel.org
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/serial_core.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

Thanks for the fix! Looks good.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

NOTE: I'm happy to defer to others, but personally I'd consider
breaking this into two changes: one that fixes the problem without
using guard() (which should be pretty simple) and one that switches to
guard(). The issue is that at the time the bug was introduced the
guard() syntax didn't exist and that means backporting will be a bit
of a pain.

Oh, though I guess maybe it doesn't matter since the bug was
introduced in 6.4 and that's not an LTS kernel so nobody cares? ...and
guard() is in 6.6, so maybe things are fine the way you have it.

[PATCH 04/13] serial: remove quot_frac from serial8250_do_set_divisor()

[Jiri Slaby (SUSE)]

quot_frac is unused in serial8250_do_set_divisor() since commit
b2b4b8ed3c06 (serial: 8250_exar: Move custom divisor support out from
8250_port). So no point to pass it.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_dwlib.c | 2 +-
 drivers/tty/serial/8250/8250_exar.c  | 2 +-
 drivers/tty/serial/8250/8250_pci.c   | 2 +-
 drivers/tty/serial/8250/8250_port.c  | 4 ++--
 include/linux/serial_8250.h          | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index 5a2520943dfd..b055d89cfb39 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -89,7 +89,7 @@ static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
 			       unsigned int quot, unsigned int quot_frac)
 {
 	dw8250_writel_ext(p, DW_UART_DLF, quot_frac);
-	serial8250_do_set_divisor(p, baud, quot, quot_frac);
+	serial8250_do_set_divisor(p, baud, quot);
 }
 
 void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios,
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 616128254bbd..b7a75db15249 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -500,7 +500,7 @@ static unsigned int xr17v35x_get_divisor(struct uart_port *p, unsigned int baud,
 static void xr17v35x_set_divisor(struct uart_port *p, unsigned int baud,
 				 unsigned int quot, unsigned int quot_frac)
 {
-	serial8250_do_set_divisor(p, baud, quot, quot_frac);
+	serial8250_do_set_divisor(p, baud, quot);
 
 	/* Preserve bits not related to baudrate; DLD[7:4]. */
 	quot_frac |= serial_port_in(p, 0x2) & 0xf0;
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index e1d7aa2fa347..6709b6a5f301 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1277,7 +1277,7 @@ static void pci_oxsemi_tornado_set_divisor(struct uart_port *port,
 	serial_icr_write(up, UART_TCR, tcr);
 	serial_icr_write(up, UART_CPR, cpr);
 	serial_icr_write(up, UART_CKS, cpr2);
-	serial8250_do_set_divisor(port, baud, quot, 0);
+	serial8250_do_set_divisor(port, baud, quot);
 }
 
 /*
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 2786918aea98..3509af7dc52b 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2609,7 +2609,7 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
 }
 
 void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
-			       unsigned int quot, unsigned int quot_frac)
+			       unsigned int quot)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
 
@@ -2641,7 +2641,7 @@ static void serial8250_set_divisor(struct uart_port *port, unsigned int baud,
 	if (port->set_divisor)
 		port->set_divisor(port, baud, quot, quot_frac);
 	else
-		serial8250_do_set_divisor(port, baud, quot, quot_frac);
+		serial8250_do_set_divisor(port, baud, quot);
 }
 
 static unsigned int serial8250_get_baud_rate(struct uart_port *port,
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index fd59ed2cca53..e0717c8393d7 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -193,7 +193,7 @@ void serial8250_do_pm(struct uart_port *port, unsigned int state,
 		      unsigned int oldstate);
 void serial8250_do_set_mctrl(struct uart_port *port, unsigned int mctrl);
 void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
-			       unsigned int quot, unsigned int quot_frac);
+			       unsigned int quot);
 int fsl8250_handle_irq(struct uart_port *port);
 int serial8250_handle_irq(struct uart_port *port, unsigned int iir);
 u16 serial8250_rx_chars(struct uart_8250_port *up, u16 lsr);
-- 
2.46.0

[PATCH 05/13] serial: use guards for simple mutex locks

[Jiri Slaby (SUSE)]

Guards can help to make the code more readable. So use it wherever they
do so.

On many places labels and 'ret' locals are eliminated completely.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---
 drivers/tty/serial/serial_core.c | 111 +++++++++++++------------------
 1 file changed, 46 insertions(+), 65 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index d63e9b636e02..0e6c1e51da99 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1096,21 +1096,19 @@ static int uart_tiocmget(struct tty_struct *tty)
 	struct uart_state *state = tty->driver_data;
 	struct tty_port *port = &state->port;
 	struct uart_port *uport;
-	int result = -EIO;
+	int result;
+
+	guard(mutex)(&port->mutex);
 
-	mutex_lock(&port->mutex);
 	uport = uart_port_check(state);
-	if (!uport)
-		goto out;
+	if (!uport || tty_io_error(tty))
+		return -EIO;
+
+	uart_port_lock_irq(uport);
+	result = uport->mctrl;
+	result |= uport->ops->get_mctrl(uport);
+	uart_port_unlock_irq(uport);
 
-	if (!tty_io_error(tty)) {
-		uart_port_lock_irq(uport);
-		result = uport->mctrl;
-		result |= uport->ops->get_mctrl(uport);
-		uart_port_unlock_irq(uport);
-	}
-out:
-	mutex_unlock(&port->mutex);
 	return result;
 }
 
@@ -1120,20 +1118,16 @@ uart_tiocmset(struct tty_struct *tty, unsigned int set, unsigned int clear)
 	struct uart_state *state = tty->driver_data;
 	struct tty_port *port = &state->port;
 	struct uart_port *uport;
-	int ret = -EIO;
 
-	mutex_lock(&port->mutex);
+	guard(mutex)(&port->mutex);
+
 	uport = uart_port_check(state);
-	if (!uport)
-		goto out;
+	if (!uport || tty_io_error(tty))
+		return -EIO;
 
-	if (!tty_io_error(tty)) {
-		uart_update_mctrl(uport, set, clear);
-		ret = 0;
-	}
-out:
-	mutex_unlock(&port->mutex);
-	return ret;
+	uart_update_mctrl(uport, set, clear);
+
+	return 0;
 }
 
 static int uart_break_ctl(struct tty_struct *tty, int break_state)
@@ -1141,19 +1135,17 @@ static int uart_break_ctl(struct tty_struct *tty, int break_state)
 	struct uart_state *state = tty->driver_data;
 	struct tty_port *port = &state->port;
 	struct uart_port *uport;
-	int ret = -EIO;
 
-	mutex_lock(&port->mutex);
+	guard(mutex)(&port->mutex);
+
 	uport = uart_port_check(state);
 	if (!uport)
-		goto out;
+		return -EIO;
 
 	if (uport->type != PORT_UNKNOWN && uport->ops->break_ctl)
 		uport->ops->break_ctl(uport, break_state);
-	ret = 0;
-out:
-	mutex_unlock(&port->mutex);
-	return ret;
+
+	return 0;
 }
 
 static int uart_do_autoconfig(struct tty_struct *tty, struct uart_state *state)
@@ -1170,17 +1162,14 @@ static int uart_do_autoconfig(struct tty_struct *tty, struct uart_state *state)
 	 * changing, and hence any extra opens of the port while
 	 * we're auto-configuring.
 	 */
-	if (mutex_lock_interruptible(&port->mutex))
-		return -ERESTARTSYS;
+	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &port->mutex) {
+		uport = uart_port_check(state);
+		if (!uport)
+			return -EIO;
 
-	uport = uart_port_check(state);
-	if (!uport) {
-		ret = -EIO;
-		goto out;
-	}
+		if (tty_port_users(port) != 1)
+			return -EBUSY;
 
-	ret = -EBUSY;
-	if (tty_port_users(port) == 1) {
 		uart_shutdown(tty, state);
 
 		/*
@@ -1201,14 +1190,15 @@ static int uart_do_autoconfig(struct tty_struct *tty, struct uart_state *state)
 		uport->ops->config_port(uport, flags);
 
 		ret = uart_startup(tty, state, true);
-		if (ret == 0)
-			tty_port_set_initialized(port, true);
+		if (ret < 0)
+			return ret;
 		if (ret > 0)
-			ret = 0;
+			return 0;
+
+		tty_port_set_initialized(port, true);
 	}
-out:
-	mutex_unlock(&port->mutex);
-	return ret;
+
+	return 0;
 }
 
 static void uart_enable_ms(struct uart_port *uport)
@@ -1703,10 +1693,11 @@ static void uart_set_termios(struct tty_struct *tty,
 	unsigned int iflag_mask = IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK;
 	bool sw_changed = false;
 
-	mutex_lock(&state->port.mutex);
+	guard(mutex)(&state->port.mutex);
+
 	uport = uart_port_check(state);
 	if (!uport)
-		goto out;
+		return;
 
 	/*
 	 * Drivers doing software flow control also need to know
@@ -1729,9 +1720,8 @@ static void uart_set_termios(struct tty_struct *tty,
 	    tty->termios.c_ospeed == old_termios->c_ospeed &&
 	    tty->termios.c_ispeed == old_termios->c_ispeed &&
 	    ((tty->termios.c_iflag ^ old_termios->c_iflag) & iflag_mask) == 0 &&
-	    !sw_changed) {
-		goto out;
-	}
+	    !sw_changed)
+		return;
 
 	uart_change_line_settings(tty, state, old_termios);
 	/* reload cflag from termios; port driver may have overridden flags */
@@ -1748,8 +1738,6 @@ static void uart_set_termios(struct tty_struct *tty,
 			mask |= TIOCM_RTS;
 		uart_set_mctrl(uport, mask);
 	}
-out:
-	mutex_unlock(&state->port.mutex);
 }
 
 /*
@@ -2043,10 +2031,11 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
 	unsigned int status;
 	int mmio;
 
-	mutex_lock(&port->mutex);
+	guard(mutex)(&port->mutex);
+
 	uport = uart_port_check(state);
 	if (!uport)
-		goto out;
+		return;
 
 	mmio = uport->iotype >= UPIO_MEM;
 	seq_printf(m, "%d: uart:%s %s%08llX irq:%d",
@@ -2058,7 +2047,7 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
 
 	if (uport->type == PORT_UNKNOWN) {
 		seq_putc(m, '\n');
-		goto out;
+		return;
 	}
 
 	if (capable(CAP_SYS_ADMIN)) {
@@ -2109,8 +2098,6 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
 	seq_putc(m, '\n');
 #undef STATBIT
 #undef INFOBIT
-out:
-	mutex_unlock(&port->mutex);
 }
 
 static int uart_proc_show(struct seq_file *m, void *v)
@@ -2387,13 +2374,12 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 	struct device *tty_dev;
 	struct uart_match match = {uport, drv};
 
-	mutex_lock(&port->mutex);
+	guard(mutex)(&port->mutex);
 
 	tty_dev = device_find_child(&uport->port_dev->dev, &match, serial_match_port);
 	if (tty_dev && device_may_wakeup(tty_dev)) {
 		enable_irq_wake(uport->irq);
 		put_device(tty_dev);
-		mutex_unlock(&port->mutex);
 		return 0;
 	}
 	put_device(tty_dev);
@@ -2454,8 +2440,6 @@ int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
 		console_stop(uport->cons);
 
 	uart_change_pm(state, UART_PM_STATE_OFF);
-unlock:
-	mutex_unlock(&port->mutex);
 
 	return 0;
 }
@@ -2469,14 +2453,13 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
 	struct uart_match match = {uport, drv};
 	struct ktermios termios;
 
-	mutex_lock(&port->mutex);
+	guard(mutex)(&port->mutex);
 
 	tty_dev = device_find_child(&uport->port_dev->dev, &match, serial_match_port);
 	if (!uport->suspended && device_may_wakeup(tty_dev)) {
 		if (irqd_is_wakeup_set(irq_get_irq_data((uport->irq))))
 			disable_irq_wake(uport->irq);
 		put_device(tty_dev);
-		mutex_unlock(&port->mutex);
 		return 0;
 	}
 	put_device(tty_dev);
@@ -2549,8 +2532,6 @@ int uart_resume_port(struct uart_driver *drv, struct uart_port *uport)
 		tty_port_set_suspended(port, false);
 	}
 
-	mutex_unlock(&port->mutex);
-
 	return 0;
 }
 EXPORT_SYMBOL(uart_resume_port);
-- 
2.46.0

Re: [PATCH 05/13] serial: use guards for simple mutex locks

[kernel test robot]

Hi Jiri,

kernel test robot noticed the following build errors:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.11-rc2 next-20240805]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jiri-Slaby-SUSE/tty-simplify-tty_dev_name_to_number-using-guard-mutex/20240805-184227
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20240805102046.307511-6-jirislaby%40kernel.org
patch subject: [PATCH 05/13] serial: use guards for simple mutex locks
config: i386-buildonly-randconfig-002-20240805 (https://download.01.org/0day-ci/archive/20240806/202408060106.20nUZZ2i-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240806/202408060106.20nUZZ2i-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408060106.20nUZZ2i-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/tty/serial/serial_core.c:2400:8: error: use of undeclared label 'unlock'
    2400 |                 goto unlock;
         |                      ^
   1 error generated.


vim +/unlock +2400 drivers/tty/serial/serial_core.c

b3b708fa2780cd2 drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2369  
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2370  int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2371  {
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2372  	struct uart_state *state = drv->state + uport->line;
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2373  	struct tty_port *port = &state->port;
b3b708fa2780cd2 drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2374  	struct device *tty_dev;
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2375  	struct uart_match match = {uport, drv};
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2376  
d2d8bbc5cc74b7e drivers/tty/serial/serial_core.c Jiri Slaby (SUSE        2024-08-05  2377) 	guard(mutex)(&port->mutex);
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2378  
b286f4e87e325b7 drivers/tty/serial/serial_core.c Tony Lindgren           2023-11-13  2379  	tty_dev = device_find_child(&uport->port_dev->dev, &match, serial_match_port);
88e2582e90bb89f drivers/tty/serial/serial_core.c Lucas Stach             2017-05-11  2380  	if (tty_dev && device_may_wakeup(tty_dev)) {
aef3ad103a686f2 drivers/tty/serial/serial_core.c Andy Shevchenko         2017-08-13  2381  		enable_irq_wake(uport->irq);
b3b708fa2780cd2 drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2382  		put_device(tty_dev);
b3b708fa2780cd2 drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2383  		return 0;
b3b708fa2780cd2 drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2384  	}
5a65dcc04cda41f drivers/tty/serial/serial_core.c Federico Vaga           2013-04-15  2385  	put_device(tty_dev);
5a65dcc04cda41f drivers/tty/serial/serial_core.c Federico Vaga           2013-04-15  2386  
c9d2325cdb92fd4 drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2387  	/*
c9d2325cdb92fd4 drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2388  	 * Nothing to do if the console is not suspending
c9d2325cdb92fd4 drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2389  	 * except stop_rx to prevent any asynchronous data
cfab87c2c271576 drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-06-08  2390  	 * over RX line. However ensure that we will be
cfab87c2c271576 drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-06-08  2391  	 * able to Re-start_rx later.
c9d2325cdb92fd4 drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2392  	 */
c9d2325cdb92fd4 drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2393  	if (!console_suspend_enabled && uart_console(uport)) {
abcb0cf1f5b2d99 drivers/tty/serial/serial_core.c John Ogness             2023-05-25  2394  		if (uport->ops->start_rx) {
559c7ff4e324558 drivers/tty/serial/serial_core.c Thomas Gleixner         2023-09-14  2395  			uart_port_lock_irq(uport);
c9d2325cdb92fd4 drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2396  			uport->ops->stop_rx(uport);
559c7ff4e324558 drivers/tty/serial/serial_core.c Thomas Gleixner         2023-09-14  2397  			uart_port_unlock_irq(uport);
abcb0cf1f5b2d99 drivers/tty/serial/serial_core.c John Ogness             2023-05-25  2398  		}
a47cf07f60dcb02 drivers/tty/serial/serial_core.c Claudiu Beznea          2024-04-30  2399  		device_set_awake_path(uport->dev);
b164c9721e3ea4c drivers/tty/serial/serial_core.c Peter Hurley            2015-01-22 @2400  		goto unlock;
c9d2325cdb92fd4 drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2401  	}
b164c9721e3ea4c drivers/tty/serial/serial_core.c Peter Hurley            2015-01-22  2402  
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2403  	uport->suspended = 1;
b3b708fa2780cd2 drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2404  
d41861ca19c9e96 drivers/tty/serial/serial_core.c Peter Hurley            2016-04-09  2405  	if (tty_port_initialized(port)) {
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2406  		const struct uart_ops *ops = uport->ops;
c8c6bfa39d6bd73 drivers/serial/serial_core.c     Russell King            2008-02-04  2407  		int tries;
18c9d4a3c249e9d drivers/tty/serial/serial_core.c Al Cooper               2022-03-24  2408  		unsigned int mctrl;
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2409  
75b20a2ac425b94 drivers/tty/serial/serial_core.c Ilpo Järvinen           2023-01-17  2410  		tty_port_set_suspended(port, true);
515be7baeddb04d drivers/tty/serial/serial_core.c Ilpo Järvinen           2023-01-17  2411  		tty_port_set_initialized(port, false);
a6b93a908508810 drivers/serial/serial_core.c     Russell King            2006-10-01  2412  
559c7ff4e324558 drivers/tty/serial/serial_core.c Thomas Gleixner         2023-09-14  2413  		uart_port_lock_irq(uport);
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2414  		ops->stop_tx(uport);
7c7f9bc986e6988 drivers/tty/serial/serial_core.c Lukas Wunner            2022-09-22  2415  		if (!(uport->rs485.flags & SER_RS485_ENABLED))
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2416  			ops->set_mctrl(uport, 0);
18c9d4a3c249e9d drivers/tty/serial/serial_core.c Al Cooper               2022-03-24  2417  		/* save mctrl so it can be restored on resume */
18c9d4a3c249e9d drivers/tty/serial/serial_core.c Al Cooper               2022-03-24  2418  		mctrl = uport->mctrl;
18c9d4a3c249e9d drivers/tty/serial/serial_core.c Al Cooper               2022-03-24  2419  		uport->mctrl = 0;
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2420  		ops->stop_rx(uport);
559c7ff4e324558 drivers/tty/serial/serial_core.c Thomas Gleixner         2023-09-14  2421  		uart_port_unlock_irq(uport);
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2422  
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2423  		/*
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2424  		 * Wait for the transmitter to empty.
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2425  		 */
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2426  		for (tries = 3; !ops->tx_empty(uport) && tries; tries--)
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2427  			msleep(10);
c8c6bfa39d6bd73 drivers/serial/serial_core.c     Russell King            2008-02-04  2428  		if (!tries)
cade3580f79aeba drivers/tty/serial/serial_core.c Andy Shevchenko         2017-03-31  2429  			dev_err(uport->dev, "%s: Unable to drain transmitter\n",
cade3580f79aeba drivers/tty/serial/serial_core.c Andy Shevchenko         2017-03-31  2430  				uport->name);
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2431  
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2432  		ops->shutdown(uport);
18c9d4a3c249e9d drivers/tty/serial/serial_core.c Al Cooper               2022-03-24  2433  		uport->mctrl = mctrl;
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2434  	}
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2435  
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2436  	/*
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2437  	 * Disable the console device before suspending.
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2438  	 */
b164c9721e3ea4c drivers/tty/serial/serial_core.c Peter Hurley            2015-01-22  2439  	if (uart_console(uport))
ccce6debb62d949 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2440  		console_stop(uport->cons);
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2441  
6f538fe31c1d453 drivers/tty/serial/serial_core.c Linus Walleij           2012-12-07  2442  	uart_change_pm(state, UART_PM_STATE_OFF);
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2443  
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2444  	return 0;
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2445  }
15dc475bcc1739c drivers/tty/serial/serial_core.c Jiri Slaby              2022-01-24  2446  EXPORT_SYMBOL(uart_suspend_port);
^1da177e4c3f415 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2447  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 05/13] serial: use guards for simple mutex locks

[kernel test robot]

Hi Jiri,

kernel test robot noticed the following build errors:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.11-rc2 next-20240805]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jiri-Slaby-SUSE/tty-simplify-tty_dev_name_to_number-using-guard-mutex/20240805-184227
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20240805102046.307511-6-jirislaby%40kernel.org
patch subject: [PATCH 05/13] serial: use guards for simple mutex locks
config: openrisc-defconfig (https://download.01.org/0day-ci/archive/20240806/202408060140.glPvoH1S-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240806/202408060140.glPvoH1S-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408060140.glPvoH1S-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/tty/serial/serial_core.c: In function 'uart_suspend_port':
>> drivers/tty/serial/serial_core.c:2400:17: error: label 'unlock' used but not defined
    2400 |                 goto unlock;
         |                 ^~~~


vim +/unlock +2400 drivers/tty/serial/serial_core.c

b3b708fa2780cd drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2369  
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2370  int uart_suspend_port(struct uart_driver *drv, struct uart_port *uport)
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2371  {
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2372  	struct uart_state *state = drv->state + uport->line;
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2373  	struct tty_port *port = &state->port;
b3b708fa2780cd drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2374  	struct device *tty_dev;
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2375  	struct uart_match match = {uport, drv};
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2376  
d2d8bbc5cc74b7 drivers/tty/serial/serial_core.c Jiri Slaby (SUSE        2024-08-05  2377) 	guard(mutex)(&port->mutex);
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2378  
b286f4e87e325b drivers/tty/serial/serial_core.c Tony Lindgren           2023-11-13  2379  	tty_dev = device_find_child(&uport->port_dev->dev, &match, serial_match_port);
88e2582e90bb89 drivers/tty/serial/serial_core.c Lucas Stach             2017-05-11  2380  	if (tty_dev && device_may_wakeup(tty_dev)) {
aef3ad103a686f drivers/tty/serial/serial_core.c Andy Shevchenko         2017-08-13  2381  		enable_irq_wake(uport->irq);
b3b708fa2780cd drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2382  		put_device(tty_dev);
b3b708fa2780cd drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2383  		return 0;
b3b708fa2780cd drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2384  	}
5a65dcc04cda41 drivers/tty/serial/serial_core.c Federico Vaga           2013-04-15  2385  	put_device(tty_dev);
5a65dcc04cda41 drivers/tty/serial/serial_core.c Federico Vaga           2013-04-15  2386  
c9d2325cdb92fd drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2387  	/*
c9d2325cdb92fd drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2388  	 * Nothing to do if the console is not suspending
c9d2325cdb92fd drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2389  	 * except stop_rx to prevent any asynchronous data
cfab87c2c27157 drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-06-08  2390  	 * over RX line. However ensure that we will be
cfab87c2c27157 drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-06-08  2391  	 * able to Re-start_rx later.
c9d2325cdb92fd drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2392  	 */
c9d2325cdb92fd drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2393  	if (!console_suspend_enabled && uart_console(uport)) {
abcb0cf1f5b2d9 drivers/tty/serial/serial_core.c John Ogness             2023-05-25  2394  		if (uport->ops->start_rx) {
559c7ff4e32455 drivers/tty/serial/serial_core.c Thomas Gleixner         2023-09-14  2395  			uart_port_lock_irq(uport);
c9d2325cdb92fd drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2396  			uport->ops->stop_rx(uport);
559c7ff4e32455 drivers/tty/serial/serial_core.c Thomas Gleixner         2023-09-14  2397  			uart_port_unlock_irq(uport);
abcb0cf1f5b2d9 drivers/tty/serial/serial_core.c John Ogness             2023-05-25  2398  		}
a47cf07f60dcb0 drivers/tty/serial/serial_core.c Claudiu Beznea          2024-04-30  2399  		device_set_awake_path(uport->dev);
b164c9721e3ea4 drivers/tty/serial/serial_core.c Peter Hurley            2015-01-22 @2400  		goto unlock;
c9d2325cdb92fd drivers/tty/serial/serial_core.c Vijaya Krishna Nivarthi 2022-05-16  2401  	}
b164c9721e3ea4 drivers/tty/serial/serial_core.c Peter Hurley            2015-01-22  2402  
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2403  	uport->suspended = 1;
b3b708fa2780cd drivers/serial/serial_core.c     Guennadi Liakhovetski   2007-10-16  2404  
d41861ca19c9e9 drivers/tty/serial/serial_core.c Peter Hurley            2016-04-09  2405  	if (tty_port_initialized(port)) {
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2406  		const struct uart_ops *ops = uport->ops;
c8c6bfa39d6bd7 drivers/serial/serial_core.c     Russell King            2008-02-04  2407  		int tries;
18c9d4a3c249e9 drivers/tty/serial/serial_core.c Al Cooper               2022-03-24  2408  		unsigned int mctrl;
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2409  
75b20a2ac425b9 drivers/tty/serial/serial_core.c Ilpo Järvinen           2023-01-17  2410  		tty_port_set_suspended(port, true);
515be7baeddb04 drivers/tty/serial/serial_core.c Ilpo Järvinen           2023-01-17  2411  		tty_port_set_initialized(port, false);
a6b93a90850881 drivers/serial/serial_core.c     Russell King            2006-10-01  2412  
559c7ff4e32455 drivers/tty/serial/serial_core.c Thomas Gleixner         2023-09-14  2413  		uart_port_lock_irq(uport);
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2414  		ops->stop_tx(uport);
7c7f9bc986e698 drivers/tty/serial/serial_core.c Lukas Wunner            2022-09-22  2415  		if (!(uport->rs485.flags & SER_RS485_ENABLED))
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2416  			ops->set_mctrl(uport, 0);
18c9d4a3c249e9 drivers/tty/serial/serial_core.c Al Cooper               2022-03-24  2417  		/* save mctrl so it can be restored on resume */
18c9d4a3c249e9 drivers/tty/serial/serial_core.c Al Cooper               2022-03-24  2418  		mctrl = uport->mctrl;
18c9d4a3c249e9 drivers/tty/serial/serial_core.c Al Cooper               2022-03-24  2419  		uport->mctrl = 0;
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2420  		ops->stop_rx(uport);
559c7ff4e32455 drivers/tty/serial/serial_core.c Thomas Gleixner         2023-09-14  2421  		uart_port_unlock_irq(uport);
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2422  
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2423  		/*
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2424  		 * Wait for the transmitter to empty.
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2425  		 */
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2426  		for (tries = 3; !ops->tx_empty(uport) && tries; tries--)
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2427  			msleep(10);
c8c6bfa39d6bd7 drivers/serial/serial_core.c     Russell King            2008-02-04  2428  		if (!tries)
cade3580f79aeb drivers/tty/serial/serial_core.c Andy Shevchenko         2017-03-31  2429  			dev_err(uport->dev, "%s: Unable to drain transmitter\n",
cade3580f79aeb drivers/tty/serial/serial_core.c Andy Shevchenko         2017-03-31  2430  				uport->name);
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2431  
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2432  		ops->shutdown(uport);
18c9d4a3c249e9 drivers/tty/serial/serial_core.c Al Cooper               2022-03-24  2433  		uport->mctrl = mctrl;
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2434  	}
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2435  
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2436  	/*
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2437  	 * Disable the console device before suspending.
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2438  	 */
b164c9721e3ea4 drivers/tty/serial/serial_core.c Peter Hurley            2015-01-22  2439  	if (uart_console(uport))
ccce6debb62d94 drivers/serial/serial_core.c     Alan Cox                2009-09-19  2440  		console_stop(uport->cons);
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2441  
6f538fe31c1d45 drivers/tty/serial/serial_core.c Linus Walleij           2012-12-07  2442  	uart_change_pm(state, UART_PM_STATE_OFF);
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2443  
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2444  	return 0;
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2445  }
15dc475bcc1739 drivers/tty/serial/serial_core.c Jiri Slaby              2022-01-24  2446  EXPORT_SYMBOL(uart_suspend_port);
^1da177e4c3f41 drivers/serial/serial_core.c     Linus Torvalds          2005-04-16  2447  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 05/13] serial: use guards for simple mutex locks

[Greg KH]

On Mon, Aug 05, 2024 at 12:20:38PM +0200, Jiri Slaby (SUSE) wrote:
> Guards can help to make the code more readable. So use it wherever they
> do so.
> 
> On many places labels and 'ret' locals are eliminated completely.
> 
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> ---
>  drivers/tty/serial/serial_core.c | 111 +++++++++++++------------------
>  1 file changed, 46 insertions(+), 65 deletions(-)

Ah, this was the breaking patch.  I applied the first 4 now, will let
you respin this one and resend the remaining.

thanks,

greg k-h

[PATCH 06/13] mxser: remove stale comment

[Jiri Slaby (SUSE)]

The comment mentions ISA removed long time ago. It also comments on
.driver_data pointing to above structures. That is not true either.
Remove that.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/mxser.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 5b97e420a95f..9a9a67b5afa0 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -208,9 +208,6 @@ static const struct {
 };
 #define UART_INFO_NUM	ARRAY_SIZE(Gpci_uart_info)
 
-
-/* driver_data correspond to the lines in the structure above
-   see also ISA probe function before you change something */
 static const struct pci_device_id mxser_pcibrds[] = {
 	{ PCI_DEVICE_DATA(MOXA, C168,		8) },
 	{ PCI_DEVICE_DATA(MOXA, C104,		4) },
-- 
2.46.0

[PATCH 07/13] mxser: remove doubled sets of close times

[Jiri Slaby (SUSE)]

tty_port::close_delay and ::closing_wait are set in tty_port_init() few
lines above already, no need to reset them (to the same values).

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/mxser.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 9a9a67b5afa0..6cfef88a18e3 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -1770,8 +1770,6 @@ static void mxser_initbrd(struct mxser_board *brd, bool high_baud)
 
 		mxser_process_txrx_fifo(info);
 
-		info->port.close_delay = 5 * HZ / 10;
-		info->port.closing_wait = 30 * HZ;
 		spin_lock_init(&info->slock);
 
 		/* before set INT ISR, disable all int */
-- 
2.46.0

[PATCH 08/13] mctp: serial: propagage new tty types

[Jiri Slaby (SUSE)]

In tty, u8 is now used for data, ssize_t for sizes (with possible
negative error codes). Propagate these types (and use unsigned in
next_chunk_len()) to mctp.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jeremy Kerr <jk@codeconstruct.com.au>
Cc: Matt Johnston <matt@codeconstruct.com.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org
---
 drivers/net/mctp/mctp-serial.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mctp/mctp-serial.c b/drivers/net/mctp/mctp-serial.c
index 5bf6fdff701c..78bd59b0930d 100644
--- a/drivers/net/mctp/mctp-serial.c
+++ b/drivers/net/mctp/mctp-serial.c
@@ -64,18 +64,18 @@ struct mctp_serial {
 	u16			txfcs, rxfcs, rxfcs_rcvd;
 	unsigned int		txlen, rxlen;
 	unsigned int		txpos, rxpos;
-	unsigned char		txbuf[BUFSIZE],
+	u8			txbuf[BUFSIZE],
 				rxbuf[BUFSIZE];
 };
 
-static bool needs_escape(unsigned char c)
+static bool needs_escape(u8 c)
 {
 	return c == BYTE_ESC || c == BYTE_FRAME;
 }
 
-static int next_chunk_len(struct mctp_serial *dev)
+static unsigned int next_chunk_len(struct mctp_serial *dev)
 {
-	int i;
+	unsigned int i;
 
 	/* either we have no bytes to send ... */
 	if (dev->txpos == dev->txlen)
@@ -99,7 +99,7 @@ static int next_chunk_len(struct mctp_serial *dev)
 	return i;
 }
 
-static int write_chunk(struct mctp_serial *dev, unsigned char *buf, int len)
+static ssize_t write_chunk(struct mctp_serial *dev, u8 *buf, size_t len)
 {
 	return dev->tty->ops->write(dev->tty, buf, len);
 }
@@ -108,9 +108,10 @@ static void mctp_serial_tx_work(struct work_struct *work)
 {
 	struct mctp_serial *dev = container_of(work, struct mctp_serial,
 					       tx_work);
-	unsigned char c, buf[3];
 	unsigned long flags;
-	int len, txlen;
+	ssize_t txlen;
+	unsigned int len;
+	u8 c, buf[3];
 
 	spin_lock_irqsave(&dev->lock, flags);
 
@@ -293,7 +294,7 @@ static void mctp_serial_rx(struct mctp_serial *dev)
 	dev->netdev->stats.rx_bytes += dev->rxlen;
 }
 
-static void mctp_serial_push_header(struct mctp_serial *dev, unsigned char c)
+static void mctp_serial_push_header(struct mctp_serial *dev, u8 c)
 {
 	switch (dev->rxpos) {
 	case 0:
@@ -323,7 +324,7 @@ static void mctp_serial_push_header(struct mctp_serial *dev, unsigned char c)
 	}
 }
 
-static void mctp_serial_push_trailer(struct mctp_serial *dev, unsigned char c)
+static void mctp_serial_push_trailer(struct mctp_serial *dev, u8 c)
 {
 	switch (dev->rxpos) {
 	case 0:
@@ -347,7 +348,7 @@ static void mctp_serial_push_trailer(struct mctp_serial *dev, unsigned char c)
 	}
 }
 
-static void mctp_serial_push(struct mctp_serial *dev, unsigned char c)
+static void mctp_serial_push(struct mctp_serial *dev, u8 c)
 {
 	switch (dev->rxstate) {
 	case STATE_IDLE:
@@ -394,7 +395,7 @@ static void mctp_serial_tty_receive_buf(struct tty_struct *tty, const u8 *c,
 					const u8 *f, size_t len)
 {
 	struct mctp_serial *dev = tty->disc_data;
-	int i;
+	size_t i;
 
 	if (!netif_running(dev->netdev))
 		return;
-- 
2.46.0

Re: [PATCH 08/13] mctp: serial: propagage new tty types

[Jeremy Kerr]

Hi Jiri,

> In tty, u8 is now used for data, ssize_t for sizes (with possible
> negative error codes). Propagate these types (and use unsigned in
> next_chunk_len()) to mctp.

All good on my side, thanks!

Reviewed-by: Jeremy Kerr <jk@codeconstruct.com.au>

I assume you're looking to merge as a series through tty, is that
right?

Cheers,


Jeremy

Re: [PATCH 08/13] mctp: serial: propagage new tty types

[Jiri Slaby]

On 06. 08. 24, 6:51, Jeremy Kerr wrote:
> Hi Jiri,
> 
>> In tty, u8 is now used for data, ssize_t for sizes (with possible
>> negative error codes). Propagate these types (and use unsigned in
>> next_chunk_len()) to mctp.
> 
> All good on my side, thanks!
> 
> Reviewed-by: Jeremy Kerr <jk@codeconstruct.com.au>

Thanks.

> I assume you're looking to merge as a series through tty, is that
> right?

Yes.

-- 
js
suse labs

[PATCH 09/13] 6pack: remove sixpack::rbuff

[Jiri Slaby (SUSE)]

It's unused (except allocation and free).

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andreas Koensgen <ajk@comnets.uni-bremen.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-hams@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 drivers/net/hamradio/6pack.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index 6ed38a3cdd73..29906901a734 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -88,7 +88,6 @@ struct sixpack {
 	struct net_device	*dev;		/* easy for intr handling  */
 
 	/* These are pointers to the malloc()ed frame buffers. */
-	unsigned char		*rbuff;		/* receiver buffer	*/
 	int			rcount;         /* received chars counter  */
 	unsigned char		*xbuff;		/* transmitter buffer	*/
 	unsigned char		*xhead;         /* next byte to XMIT */
@@ -544,7 +543,7 @@ static inline int tnc_init(struct sixpack *sp)
  */
 static int sixpack_open(struct tty_struct *tty)
 {
-	char *rbuff = NULL, *xbuff = NULL;
+	char *xbuff = NULL;
 	struct net_device *dev;
 	struct sixpack *sp;
 	unsigned long len;
@@ -574,10 +573,8 @@ static int sixpack_open(struct tty_struct *tty)
 
 	len = dev->mtu * 2;
 
-	rbuff = kmalloc(len + 4, GFP_KERNEL);
 	xbuff = kmalloc(len + 4, GFP_KERNEL);
-
-	if (rbuff == NULL || xbuff == NULL) {
+	if (xbuff == NULL) {
 		err = -ENOBUFS;
 		goto out_free;
 	}
@@ -586,7 +583,6 @@ static int sixpack_open(struct tty_struct *tty)
 
 	sp->tty = tty;
 
-	sp->rbuff	= rbuff;
 	sp->xbuff	= xbuff;
 
 	sp->mtu		= AX25_MTU + 73;
@@ -631,7 +627,6 @@ static int sixpack_open(struct tty_struct *tty)
 
 out_free:
 	kfree(xbuff);
-	kfree(rbuff);
 
 	free_netdev(dev);
 
@@ -676,7 +671,6 @@ static void sixpack_close(struct tty_struct *tty)
 	del_timer_sync(&sp->resync_t);
 
 	/* Free all 6pack frame buffers after unreg. */
-	kfree(sp->rbuff);
 	kfree(sp->xbuff);
 
 	free_netdev(sp->dev);
-- 
2.46.0

[PATCH 10/13] 6pack: drop sixpack::mtu

[Jiri Slaby (SUSE)]

It holds a constant (AX25_MTU + 73), so use that constant in place of
the single use directly.

And remove the stale comment.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andreas Koensgen <ajk@comnets.uni-bremen.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-hams@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 drivers/net/hamradio/6pack.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index 29906901a734..f8235b1b60e9 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -100,7 +100,6 @@ struct sixpack {
 	unsigned int		rx_count_cooked;
 	spinlock_t		rxlock;
 
-	int			mtu;		/* Our mtu (to spot changes!) */
 	int			buffsize;       /* Max buffers sizes */
 
 	unsigned long		flags;		/* Flag values/ mode etc */
@@ -166,7 +165,7 @@ static void sp_encaps(struct sixpack *sp, unsigned char *icp, int len)
 	unsigned char *msg, *p = icp;
 	int actual, count;
 
-	if (len > sp->mtu) {	/* sp->mtu = AX25_MTU = max. PACLEN = 256 */
+	if (len > AX25_MTU + 73) {
 		msg = "oversized transmit packet!";
 		goto out_drop;
 	}
@@ -585,7 +584,6 @@ static int sixpack_open(struct tty_struct *tty)
 
 	sp->xbuff	= xbuff;
 
-	sp->mtu		= AX25_MTU + 73;
 	sp->buffsize	= len;
 	sp->rcount	= 0;
 	sp->rx_count	= 0;
-- 
2.46.0

[PATCH 11/13] 6pack: drop sixpack::buffsize

[Jiri Slaby (SUSE)]

It's never read.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andreas Koensgen <ajk@comnets.uni-bremen.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-hams@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 drivers/net/hamradio/6pack.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index f8235b1b60e9..25d6d2308130 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -100,8 +100,6 @@ struct sixpack {
 	unsigned int		rx_count_cooked;
 	spinlock_t		rxlock;
 
-	int			buffsize;       /* Max buffers sizes */
-
 	unsigned long		flags;		/* Flag values/ mode etc */
 	unsigned char		mode;		/* 6pack mode */
 
@@ -584,7 +582,6 @@ static int sixpack_open(struct tty_struct *tty)
 
 	sp->xbuff	= xbuff;
 
-	sp->buffsize	= len;
 	sp->rcount	= 0;
 	sp->rx_count	= 0;
 	sp->rx_count_cooked = 0;
-- 
2.46.0

[PATCH 12/13] 6pack: remove global strings

[Jiri Slaby (SUSE)]

They are __init, so they are freed after init is done. But this
obfuscates the code.

Provided these days, we usually don't print anything if everything has
gone fine, drop the info print completely (along with now unused and
always artificial SIXPACK_VERSION).

And move the other string into the printk proper (while converting from
KERN_ERR to pr_err()).

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andreas Koensgen <ajk@comnets.uni-bremen.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-hams@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 drivers/net/hamradio/6pack.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index 25d6d2308130..5c47730f5d58 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -37,8 +37,6 @@
 #include <linux/semaphore.h>
 #include <linux/refcount.h>
 
-#define SIXPACK_VERSION    "Revision: 0.3.0"
-
 /* sixpack priority commands */
 #define SIXP_SEOF		0x40	/* start and end of a 6pack frame */
 #define SIXP_TX_URUN		0x48	/* transmit overrun */
@@ -745,21 +743,14 @@ static struct tty_ldisc_ops sp_ldisc = {
 
 /* Initialize 6pack control device -- register 6pack line discipline */
 
-static const char msg_banner[]  __initconst = KERN_INFO \
-	"AX.25: 6pack driver, " SIXPACK_VERSION "\n";
-static const char msg_regfail[] __initconst = KERN_ERR  \
-	"6pack: can't register line discipline (err = %d)\n";
-
 static int __init sixpack_init_driver(void)
 {
 	int status;
 
-	printk(msg_banner);
-
 	/* Register the provided line protocol discipline */
 	status = tty_register_ldisc(&sp_ldisc);
 	if (status)
-		printk(msg_regfail, status);
+		pr_err("6pack: can't register line discipline (err = %d)\n", status);
 
 	return status;
 }
-- 
2.46.0

[PATCH 13/13] 6pack: propagage new tty types

[Jiri Slaby (SUSE)]

In tty, u8 is now used for data, ssize_t for sizes (with possible
negative error codes). Propagate these types to 6pack.

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andreas Koensgen <ajk@comnets.uni-bremen.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-hams@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 drivers/net/hamradio/6pack.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index 5c47730f5d58..3bf6785f9057 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -91,8 +91,8 @@ struct sixpack {
 	unsigned char		*xhead;         /* next byte to XMIT */
 	int			xleft;          /* bytes left in XMIT queue  */
 
-	unsigned char		raw_buf[4];
-	unsigned char		cooked_buf[400];
+	u8			raw_buf[4];
+	u8			cooked_buf[400];
 
 	unsigned int		rx_count;
 	unsigned int		rx_count_cooked;
@@ -107,8 +107,8 @@ struct sixpack {
 	unsigned char		slottime;
 	unsigned char		duplex;
 	unsigned char		led_state;
-	unsigned char		status;
-	unsigned char		status1;
+	u8			status;
+	u8			status1;
 	unsigned char		status2;
 	unsigned char		tx_enable;
 	unsigned char		tnc_state;
@@ -122,7 +122,7 @@ struct sixpack {
 
 #define AX25_6PACK_HEADER_LEN 0
 
-static void sixpack_decode(struct sixpack *, const unsigned char[], int);
+static void sixpack_decode(struct sixpack *, const u8 *, size_t);
 static int encode_sixpack(unsigned char *, unsigned char *, int, unsigned char);
 
 /*
@@ -327,7 +327,7 @@ static void sp_bump(struct sixpack *sp, char cmd)
 {
 	struct sk_buff *skb;
 	int count;
-	unsigned char *ptr;
+	u8 *ptr;
 
 	count = sp->rcount + 1;
 
@@ -425,7 +425,7 @@ static void sixpack_receive_buf(struct tty_struct *tty, const u8 *cp,
 				const u8 *fp, size_t count)
 {
 	struct sixpack *sp;
-	int count1;
+	size_t count1;
 
 	if (!count)
 		return;
@@ -800,9 +800,9 @@ static int encode_sixpack(unsigned char *tx_buf, unsigned char *tx_buf_raw,
 
 /* decode 4 sixpack-encoded bytes into 3 data bytes */
 
-static void decode_data(struct sixpack *sp, unsigned char inbyte)
+static void decode_data(struct sixpack *sp, u8 inbyte)
 {
-	unsigned char *buf;
+	u8 *buf;
 
 	if (sp->rx_count != 3) {
 		sp->raw_buf[sp->rx_count++] = inbyte;
@@ -828,9 +828,9 @@ static void decode_data(struct sixpack *sp, unsigned char inbyte)
 
 /* identify and execute a 6pack priority command byte */
 
-static void decode_prio_command(struct sixpack *sp, unsigned char cmd)
+static void decode_prio_command(struct sixpack *sp, u8 cmd)
 {
-	int actual;
+	ssize_t actual;
 
 	if ((cmd & SIXP_PRIO_DATA_MASK) != 0) {     /* idle ? */
 
@@ -878,9 +878,9 @@ static void decode_prio_command(struct sixpack *sp, unsigned char cmd)
 
 /* identify and execute a standard 6pack command byte */
 
-static void decode_std_command(struct sixpack *sp, unsigned char cmd)
+static void decode_std_command(struct sixpack *sp, u8 cmd)
 {
-	unsigned char checksum = 0, rest = 0;
+	u8 checksum = 0, rest = 0;
 	short i;
 
 	switch (cmd & SIXP_CMD_MASK) {     /* normal command */
@@ -928,10 +928,10 @@ static void decode_std_command(struct sixpack *sp, unsigned char cmd)
 /* decode a 6pack packet */
 
 static void
-sixpack_decode(struct sixpack *sp, const unsigned char *pre_rbuff, int count)
+sixpack_decode(struct sixpack *sp, const u8 *pre_rbuff, size_t count)
 {
-	unsigned char inbyte;
-	int count1;
+	size_t count1;
+	u8 inbyte;
 
 	for (count1 = 0; count1 < count; count1++) {
 		inbyte = pre_rbuff[count1];
-- 
2.46.0

Re: [PATCH 00/13] tty: random fixes and cleanups

[Greg KH]

On Mon, Aug 05, 2024 at 12:20:33PM +0200, Jiri Slaby (SUSE) wrote:
> Hi,
> 
> this is a series of locally accumulated patches over past months.
> 
> The series:
> * makes mctp and 6pack use u8s,
> * cleans up 6pack a bit,
> * fixes two coverity reports,
> * uses guard() to make some of the tty function easier to follow.

This series breaks the build for me:

drivers/tty/serial/serial_core.c: In function ‘uart_suspend_port’:
drivers/tty/serial/serial_core.c:2400:17: error: label ‘unlock’ used but not defined
 2400 |                 goto unlock;
      |                 ^~~~
make[5]: *** [scripts/Makefile.build:244: drivers/tty/serial/serial_core.o] Error 1
make[5]: *** Waiting for unfinished jobs....