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

[Ben Hutchings <ben@decadent.org.uk>]

[-- 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 --]