Re: [ 019/150] serial_core: Fix type definition for PORT_BRCM_TRUMANAGE.

Re: [ 019/150] serial_core: Fix type definition for PORT_BRCM_TRUMANAGE.

[Ben Hutchings]

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

I've queued up the following for 3.2.y.  Let me know if you see any
issue with this or want to ack it.

Ben.

---
From: Ben Hutchings <ben@decadent.org.uk>
Subject: 8250: use correct value for PORT_BRCM_TRUMANAGE
Date: Sun, 03 Mar 2013 03:24:34 +0000

When backporting commit ebebd49a8eab ('8250/16?50: Add support for
Broadcom TruManage redirected serial port') I took the next
available port type number for PORT_BRCM_TRUMANAGE (22).

However, the 8250 port type numbers are exposed to userland through
the TIOC{G,S}SERIAL ioctls and so must remain stable.  Redefine
PORT_BRCM_TRUMANAGE as 25, matching mainline as of commit
85f024401bf807.

This leaves port types 22-24 within the valid range for 8250 but not
implemented there.  Change serial8250_verify_port() to specifically
reject these and change serial8250_type() to return "unknown" for them
(though I'm not sure why it would ever see them).

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
--- a/drivers/tty/serial/8250.c
+++ b/drivers/tty/serial/8250.c
@@ -2695,7 +2695,7 @@ serial8250_verify_port(struct uart_port
 	if (ser->irq >= nr_irqs || ser->irq < 0 ||
 	    ser->baud_base < 9600 || ser->type < PORT_UNKNOWN ||
 	    ser->type >= ARRAY_SIZE(uart_config) || ser->type == PORT_CIRRUS ||
-	    ser->type == PORT_STARTECH)
+	    ser->type == PORT_STARTECH || uart_config[ser->type].name == NULL)
 		return -EINVAL;
 	return 0;
 }
@@ -2705,7 +2705,7 @@ serial8250_type(struct uart_port *port)
 {
 	int type = port->type;
 
-	if (type >= ARRAY_SIZE(uart_config))
+	if (type >= ARRAY_SIZE(uart_config) || uart_config[type].name == NULL)
 		type = 0;
 	return uart_config[type].name;
 }
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -47,8 +47,8 @@
 #define PORT_U6_16550A	19	/* ST-Ericsson U6xxx internal UART */
 #define PORT_TEGRA	20	/* NVIDIA Tegra internal UART */
 #define PORT_XR17D15X	21	/* Exar XR17D15x UART */
-#define PORT_BRCM_TRUMANAGE	22
-#define PORT_MAX_8250	22	/* max port ID */
+#define PORT_BRCM_TRUMANAGE	25
+#define PORT_MAX_8250	25	/* max port ID */
 
 /*
  * ARM specific type numbers.  These are not currently guaranteed

-- 
Ben Hutchings
Time is nature's way of making sure that everything doesn't happen at once.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [ 019/150] serial_core: Fix type definition for PORT_BRCM_TRUMANAGE.

[Greg Kroah-Hartman]

On Sun, Mar 03, 2013 at 03:38:41AM +0000, Ben Hutchings wrote:
> I've queued up the following for 3.2.y.  Let me know if you see any
> issue with this or want to ack it.
> 
> Ben.
> 
> ---
> From: Ben Hutchings <ben@decadent.org.uk>
> Subject: 8250: use correct value for PORT_BRCM_TRUMANAGE
> Date: Sun, 03 Mar 2013 03:24:34 +0000
> 
> When backporting commit ebebd49a8eab ('8250/16?50: Add support for
> Broadcom TruManage redirected serial port') I took the next
> available port type number for PORT_BRCM_TRUMANAGE (22).
> 
> However, the 8250 port type numbers are exposed to userland through
> the TIOC{G,S}SERIAL ioctls and so must remain stable.  Redefine
> PORT_BRCM_TRUMANAGE as 25, matching mainline as of commit
> 85f024401bf807.
> 
> This leaves port types 22-24 within the valid range for 8250 but not
> implemented there.  Change serial8250_verify_port() to specifically
> reject these and change serial8250_type() to return "unknown" for them
> (though I'm not sure why it would ever see them).
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> --- a/drivers/tty/serial/8250.c
> +++ b/drivers/tty/serial/8250.c
> @@ -2695,7 +2695,7 @@ serial8250_verify_port(struct uart_port
>  	if (ser->irq >= nr_irqs || ser->irq < 0 ||
>  	    ser->baud_base < 9600 || ser->type < PORT_UNKNOWN ||
>  	    ser->type >= ARRAY_SIZE(uart_config) || ser->type == PORT_CIRRUS ||
> -	    ser->type == PORT_STARTECH)
> +	    ser->type == PORT_STARTECH || uart_config[ser->type].name == NULL)
>  		return -EINVAL;
>  	return 0;
>  }
> @@ -2705,7 +2705,7 @@ serial8250_type(struct uart_port *port)
>  {
>  	int type = port->type;
>  
> -	if (type >= ARRAY_SIZE(uart_config))
> +	if (type >= ARRAY_SIZE(uart_config) || uart_config[type].name == NULL)

I don't think these two changes are really needed, as you point out
above, no one should really see, or care, about this.  But if you do,
these changes should go upstream first, right?

>  		type = 0;
>  	return uart_config[type].name;
>  }
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -47,8 +47,8 @@
>  #define PORT_U6_16550A	19	/* ST-Ericsson U6xxx internal UART */
>  #define PORT_TEGRA	20	/* NVIDIA Tegra internal UART */
>  #define PORT_XR17D15X	21	/* Exar XR17D15x UART */
> -#define PORT_BRCM_TRUMANAGE	22
> -#define PORT_MAX_8250	22	/* max port ID */
> +#define PORT_BRCM_TRUMANAGE	25
> +#define PORT_MAX_8250	25	/* max port ID */

This one is good to have though, I have no objection to this hunk going
into 3.2, for it, feel free to add:
	Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [ 019/150] serial_core: Fix type definition for PORT_BRCM_TRUMANAGE.

[Ben Hutchings]

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

On Mon, 2013-03-04 at 10:41 +0800, Greg Kroah-Hartman wrote:
> On Sun, Mar 03, 2013 at 03:38:41AM +0000, Ben Hutchings wrote:
> > I've queued up the following for 3.2.y.  Let me know if you see any
> > issue with this or want to ack it.
> > 
> > Ben.
> > 
> > ---
> > From: Ben Hutchings <ben@decadent.org.uk>
> > Subject: 8250: use correct value for PORT_BRCM_TRUMANAGE
> > Date: Sun, 03 Mar 2013 03:24:34 +0000
> > 
> > When backporting commit ebebd49a8eab ('8250/16?50: Add support for
> > Broadcom TruManage redirected serial port') I took the next
> > available port type number for PORT_BRCM_TRUMANAGE (22).
> > 
> > However, the 8250 port type numbers are exposed to userland through
> > the TIOC{G,S}SERIAL ioctls and so must remain stable.  Redefine
> > PORT_BRCM_TRUMANAGE as 25, matching mainline as of commit
> > 85f024401bf807.
> > 
> > This leaves port types 22-24 within the valid range for 8250 but not
> > implemented there.  Change serial8250_verify_port() to specifically
> > reject these and change serial8250_type() to return "unknown" for them
> > (though I'm not sure why it would ever see them).
> > 
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > ---
> > --- a/drivers/tty/serial/8250.c
> > +++ b/drivers/tty/serial/8250.c
> > @@ -2695,7 +2695,7 @@ serial8250_verify_port(struct uart_port
> >  	if (ser->irq >= nr_irqs || ser->irq < 0 ||
> >  	    ser->baud_base < 9600 || ser->type < PORT_UNKNOWN ||
> >  	    ser->type >= ARRAY_SIZE(uart_config) || ser->type == PORT_CIRRUS ||
> > -	    ser->type == PORT_STARTECH)
> > +	    ser->type == PORT_STARTECH || uart_config[ser->type].name == NULL)
> >  		return -EINVAL;
> >  	return 0;
> >  }
> > @@ -2705,7 +2705,7 @@ serial8250_type(struct uart_port *port)
> >  {
> >  	int type = port->type;
> >  
> > -	if (type >= ARRAY_SIZE(uart_config))
> > +	if (type >= ARRAY_SIZE(uart_config) || uart_config[type].name == NULL)
> 
> I don't think these two changes are really needed, as you point out
> above, no one should really see, or care, about this.  But if you do,
> these changes should go upstream first, right?

The change in serial8250_verify_port() is needed in 3.2 but not mainline
because types 22-24 won't be supported in 3.2.  I could alternately
define the PORT_* macros for these types and explicitly reject each of
those here.

The change in serial8250_type() is done for the same reason.  It already
has a check for out-of-range type which suggests there's some case where
the type might not have been validated by serial8250_verify_port().
Again, not needed upstream as all elements in uart_config have a
non-null name.

Ben.

> >  		type = 0;
> >  	return uart_config[type].name;
> >  }
> > --- a/include/linux/serial_core.h
> > +++ b/include/linux/serial_core.h
> > @@ -47,8 +47,8 @@
> >  #define PORT_U6_16550A	19	/* ST-Ericsson U6xxx internal UART */
> >  #define PORT_TEGRA	20	/* NVIDIA Tegra internal UART */
> >  #define PORT_XR17D15X	21	/* Exar XR17D15x UART */
> > -#define PORT_BRCM_TRUMANAGE	22
> > -#define PORT_MAX_8250	22	/* max port ID */
> > +#define PORT_BRCM_TRUMANAGE	25
> > +#define PORT_MAX_8250	25	/* max port ID */
> 
> This one is good to have though, I have no objection to this hunk going
> into 3.2, for it, feel free to add:
> 	Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> 

-- 
Ben Hutchings
Always try to do things in chronological order;
it's less confusing that way.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]