[PATCH] tty: serial: 8250: remove U6715 16550A auto-detection

[PATCH] tty: serial: 8250: remove U6715 16550A auto-detection

[Philippe Langlais]

The auto-detection based on 64 bytes fifo size causes troubles
for the identification of the MultiTechZPX Modems (pci 11c1:0480).
They loose the ability to do hardware flow control
(specifically CTS is never set).

Signed-off-by: Philippe Langlais <philippe.langlais@st.com>
---
 drivers/tty/serial/8250/8250.c |    9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index cf6a538..3255014 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -1026,15 +1026,6 @@ static void autoconfig_16550a(struct uart_8250_port *up)
 
 		return;
 	}
-
-	/*
-	 * We distinguish between 16550A and U6 16550A by counting
-	 * how many bytes are in the FIFO.
-	 */
-	if (up->port.type == PORT_16550A && size_fifo(up) == 64) {
-		up->port.type = PORT_U6_16550A;
-		up->capabilities |= UART_CAP_AFE;
-	}
 }
 
 /*
-- 
1.7.9.5

Re: [PATCH] tty: serial: 8250: remove U6715 16550A auto-detection

[Greg Kroah-Hartman]

On Tue, Mar 19, 2013 at 08:50:09PM +0100, Philippe Langlais wrote:
> The auto-detection based on 64 bytes fifo size causes troubles
> for the identification of the MultiTechZPX Modems (pci 11c1:0480).
> They loose the ability to do hardware flow control
> (specifically CTS is never set).
> 
> Signed-off-by: Philippe Langlais <philippe.langlais@st.com>
> ---
>  drivers/tty/serial/8250/8250.c |    9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
> index cf6a538..3255014 100644
> --- a/drivers/tty/serial/8250/8250.c
> +++ b/drivers/tty/serial/8250/8250.c
> @@ -1026,15 +1026,6 @@ static void autoconfig_16550a(struct uart_8250_port *up)
>  
>  		return;
>  	}
> -
> -	/*
> -	 * We distinguish between 16550A and U6 16550A by counting
> -	 * how many bytes are in the FIFO.
> -	 */
> -	if (up->port.type == PORT_16550A && size_fifo(up) == 64) {
> -		up->port.type = PORT_U6_16550A;
> -		up->capabilities |= UART_CAP_AFE;
> -	}

So you just broke this functionality for all of the other devices it
does work properly for?  That's not nice, and not acceptable.

Please fix this properly.

greg k-h

Re: [PATCH] tty: serial: 8250: remove U6715 16550A auto-detection

[Heikki Krogerus]

Hi guys,

On Wed, Mar 20, 2013 at 10:21:25AM -0700, Greg Kroah-Hartman wrote:
> On Tue, Mar 19, 2013 at 08:50:09PM +0100, Philippe Langlais wrote:
> > -	if (up->port.type == PORT_16550A && size_fifo(up) == 64) {
> > -		up->port.type = PORT_U6_16550A;
> > -		up->capabilities |= UART_CAP_AFE;
> > -	}
> 
> So you just broke this functionality for all of the other devices it
> does work properly for?  That's not nice, and not acceptable.
> 
> Please fix this properly.

Is there actually anything depending on this quirk? It looks like
U6715 support newer made it to upstream, and I believe the type
PORT_U6_16550A is designed for U6715 only.

In any case, replacing the type PORT_16550A with a type that has
custom capabilities based on fifosize alone feels dangerous to me. 64B
fifo for 16550 compatible UARTs is very common these days. 

Since we have this conditions in autoconfig_16550a(), then would it be
OK to replace it with something like:

-       /*
-        * We distinguish between 16550A and U6 16550A by counting
-        * how many bytes are in the FIFO.
-        */
-       if (up->port.type == PORT_16550A && size_fifo(up) == 64) {
-               up->port.type = PORT_U6_16550A;
-               up->capabilities |= UART_CAP_AFE;
-       }
+       if (up->port.type == PORT_16550A && size_fifo(up) == 64)
+               up->port.fifosize = 64;
+
+       if (up->port.type == PORT_16550A && size_fifo(up) == 128)
+               up->port.fifosize = 128;
 }


We set the port->up.fifosize for a few common fifo sizes, like 64 and
128, instead of replacing the type and capabilities?

Thanks,

-- 
heikki

Re: [PATCH] tty: serial: 8250: remove U6715 16550A auto-detection

[Philippe LANGLAIS]

On 03/20/2013 06:21 PM, Greg Kroah-Hartman wrote:
> On Tue, Mar 19, 2013 at 08:50:09PM +0100, Philippe Langlais wrote:
>> The auto-detection based on 64 bytes fifo size causes troubles
>> for the identification of the MultiTechZPX Modems (pci 11c1:0480).
>> They loose the ability to do hardware flow control
>> (specifically CTS is never set).
>>
>> Signed-off-by: Philippe Langlais <philippe.langlais@st.com>
>> ---
>>   drivers/tty/serial/8250/8250.c |    9 ---------
>>   1 file changed, 9 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
>> index cf6a538..3255014 100644
>> --- a/drivers/tty/serial/8250/8250.c
>> +++ b/drivers/tty/serial/8250/8250.c
>> @@ -1026,15 +1026,6 @@ static void autoconfig_16550a(struct uart_8250_port *up)
>>   
>>   		return;
>>   	}
>> -
>> -	/*
>> -	 * We distinguish between 16550A and U6 16550A by counting
>> -	 * how many bytes are in the FIFO.
>> -	 */
>> -	if (up->port.type == PORT_16550A && size_fifo(up) == 64) {
>> -		up->port.type = PORT_U6_16550A;
>> -		up->capabilities |= UART_CAP_AFE;
>> -	}
> So you just broke this functionality for all of the other devices it
> does work properly for?  That's not nice, and not acceptable.
Yes, I know, but as U6715 & U6XXX kernel maintainer there is no risk to 
do that,
kernel for such platforms are not managed in mainstream, there are more 
risks
to keep these lines for other platforms.
>
> Please fix this properly.
I don't know how to distinguish MultiTechZPX Modems, so I rather to 
remove these lines.
>
> greg k-h
Philippe