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

Ben Hutchings
2013-03-04T03:35:54Z

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