uart_get_baud_rate has input parameters 'min' and 'max' limiting the
range of acceptable baud rates from the caller's perspective. If neither
current or old termios structures have acceptable baud rate setting and
9600 is not in the min/max range either the function returns 0 and
issues a warning.
However for a UART that does not support speed of 9600 baud this is
expected behavior.
Clarify that 0 can be (and always could be) returned from the
uart_get_baud_rate. Don't issue a warning in that case.
Move the warinng to the uart_get_divisor instead, which is often called
with the uart_get_baud_rate return value.
Signed-off-by: Max Filippov <[email protected]>
---
drivers/tty/serial/serial_core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 7bdc21d5e13b..a8e2915832e8 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -431,7 +431,7 @@ EXPORT_SYMBOL(uart_update_timeout);
* baud.
*
* If the new baud rate is invalid, try the @old termios setting. If it's still
- * invalid, we try 9600 baud.
+ * invalid, we try 9600 baud. If that is also invalid 0 is returned.
*
* The @termios structure is updated to reflect the baud rate we're actually
* going to be using. Don't do this for the case where B0 is requested ("hang
@@ -515,8 +515,6 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
max - 1, max - 1);
}
}
- /* Should never happen */
- WARN_ON(1);
return 0;
}
EXPORT_SYMBOL(uart_get_baud_rate);
@@ -539,6 +537,7 @@ uart_get_divisor(struct uart_port *port, unsigned int baud)
{
unsigned int quot;
+ WARN_ON(baud == 0);
/*
* Old custom speed handling.
*/
--
2.30.2
On Tue, Sep 19, 2023 at 07:26:40PM -0700, Max Filippov wrote:
> uart_get_baud_rate has input parameters 'min' and 'max' limiting the
> range of acceptable baud rates from the caller's perspective. If neither
> current or old termios structures have acceptable baud rate setting and
> 9600 is not in the min/max range either the function returns 0 and
> issues a warning.
> However for a UART that does not support speed of 9600 baud this is
> expected behavior.
> Clarify that 0 can be (and always could be) returned from the
> uart_get_baud_rate. Don't issue a warning in that case.
> Move the warinng to the uart_get_divisor instead, which is often called
> with the uart_get_baud_rate return value.
>
> Signed-off-by: Max Filippov <[email protected]>
> ---
> drivers/tty/serial/serial_core.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 7bdc21d5e13b..a8e2915832e8 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -431,7 +431,7 @@ EXPORT_SYMBOL(uart_update_timeout);
> * baud.
> *
> * If the new baud rate is invalid, try the @old termios setting. If it's still
> - * invalid, we try 9600 baud.
> + * invalid, we try 9600 baud. If that is also invalid 0 is returned.
> *
> * The @termios structure is updated to reflect the baud rate we're actually
> * going to be using. Don't do this for the case where B0 is requested ("hang
> @@ -515,8 +515,6 @@ uart_get_baud_rate(struct uart_port *port, struct ktermios *termios,
> max - 1, max - 1);
> }
> }
> - /* Should never happen */
> - WARN_ON(1);
I'm ok with this removal, but:
> return 0;
> }
> EXPORT_SYMBOL(uart_get_baud_rate);
> @@ -539,6 +537,7 @@ uart_get_divisor(struct uart_port *port, unsigned int baud)
> {
> unsigned int quot;
>
> + WARN_ON(baud == 0);
Why is this needed? If this isn't happening today, then there's no need
to check for this here. Or if it can happen, we should return an error,
not cause a possible reboot of the system if panic-on-warn is enabled.
thanks,
greg k-h
On Thu, Sep 28, 2023 at 1:17 AM Greg Kroah-Hartman
<[email protected]> wrote:
> On Tue, Sep 19, 2023 at 07:26:40PM -0700, Max Filippov wrote:
> > @@ -539,6 +537,7 @@ uart_get_divisor(struct uart_port *port, unsigned int baud)
> > {
> > unsigned int quot;
> >
> > + WARN_ON(baud == 0);
>
> Why is this needed? If this isn't happening today, then there's no need
> to check for this here.
I'll drop it then.
--
Thanks.
-- Max