diff mbox

[SRU,Zesty] drivers/tty: 8250: only call fintek_8250_probe when doing port I/O

Message ID 20170530163540.2gavxupcgdt7rwhr@xps13.dannf
State New
Headers show

Commit Message

dann frazier May 30, 2017, 4:35 p.m. UTC
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

BugLink: https://bugs.launchpad.net/bugs/1692548

Commit fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base")
modified the probing logic for PNP0501 devices, to remove a collision
between the generic 16550A driver and the Fintek driver, which reused
the same ACPI _HID.

The Fintek device probe is now incorporated into the common 8250 probe
path, and gets called for all discovered 16550A compatible devices,
including ones that are MMIO mapped rather than IO mapped. However,
the Fintek driver assumes the port base is a I/O address, and proceeds
to probe some arbitrary offsets above it.

This is generally a wrong thing to do, but on ARM systems (having no
native port I/O), this may result in faulting accesses of completely
unrelated MMIO regions in the PCI I/O space. Given that this is at
serial probe time, this results in hard to diagnose crashes at boot.

So let's restrict the Fintek probe to devices that we know are using
port I/O in the first place.

Fixes: fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base")
Suggested-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Ricardo Ribalda <ricardo.ribalda@gmail.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 4c4fc90964b1cf205a67df566cc82ea1731bcb00)
Signed-off-by: dann frazier <dann.frazier@canonical.com>
---
 drivers/tty/serial/8250/8250_port.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Colin Ian King May 31, 2017, 8:16 a.m. UTC | #1
On 30/05/17 17:35, dann frazier wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> BugLink: https://bugs.launchpad.net/bugs/1692548
> 
> Commit fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base")
> modified the probing logic for PNP0501 devices, to remove a collision
> between the generic 16550A driver and the Fintek driver, which reused
> the same ACPI _HID.
> 
> The Fintek device probe is now incorporated into the common 8250 probe
> path, and gets called for all discovered 16550A compatible devices,
> including ones that are MMIO mapped rather than IO mapped. However,
> the Fintek driver assumes the port base is a I/O address, and proceeds
> to probe some arbitrary offsets above it.
> 
> This is generally a wrong thing to do, but on ARM systems (having no
> native port I/O), this may result in faulting accesses of completely
> unrelated MMIO regions in the PCI I/O space. Given that this is at
> serial probe time, this results in hard to diagnose crashes at boot.
> 
> So let's restrict the Fintek probe to devices that we know are using
> port I/O in the first place.
> 
> Fixes: fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base")
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Ricardo Ribalda <ricardo.ribalda@gmail.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> (cherry picked from commit 4c4fc90964b1cf205a67df566cc82ea1731bcb00)
> Signed-off-by: dann frazier <dann.frazier@canonical.com>
> ---
>  drivers/tty/serial/8250/8250_port.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index c13fec451d03..331bbe38e34b 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1322,7 +1322,7 @@ static void autoconfig(struct uart_8250_port *up)
>  	/*
>  	 * Check if the device is a Fintek F81216A
>  	 */
> -	if (port->type == PORT_16550A)
> +	if (port->type == PORT_16550A && port->iotype == UPIO_PORT)
>  		fintek_8250_probe(up);
>  
>  	if (up->capabilities != old_capabilities) {
> 

Upstream cherry pick, looks sensible to me.

Thanks Dann

Acked-by: Colin Ian King <colin.king@canonical.com>
Seth Forshee June 1, 2017, 8:11 p.m. UTC | #2
On Tue, May 30, 2017 at 10:35:40AM -0600, dann frazier wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> BugLink: https://bugs.launchpad.net/bugs/1692548
> 
> Commit fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base")
> modified the probing logic for PNP0501 devices, to remove a collision
> between the generic 16550A driver and the Fintek driver, which reused
> the same ACPI _HID.
> 
> The Fintek device probe is now incorporated into the common 8250 probe
> path, and gets called for all discovered 16550A compatible devices,
> including ones that are MMIO mapped rather than IO mapped. However,
> the Fintek driver assumes the port base is a I/O address, and proceeds
> to probe some arbitrary offsets above it.
> 
> This is generally a wrong thing to do, but on ARM systems (having no
> native port I/O), this may result in faulting accesses of completely
> unrelated MMIO regions in the PCI I/O space. Given that this is at
> serial probe time, this results in hard to diagnose crashes at boot.
> 
> So let's restrict the Fintek probe to devices that we know are using
> port I/O in the first place.
> 
> Fixes: fa01e2ca9f53 ("serial: 8250: Integrate Fintek into 8250_base")
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Ricardo Ribalda <ricardo.ribalda@gmail.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: stable <stable@vger.kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> (cherry picked from commit 4c4fc90964b1cf205a67df566cc82ea1731bcb00)
> Signed-off-by: dann frazier <dann.frazier@canonical.com>

Clean cherry pick, straightforward bug fix.

Acked-by: Seth Forshee <seth.forshee@canonical.com>

Also applied to artful/master-next.
Kleber Sacilotto de Souza June 7, 2017, 9:43 a.m. UTC | #3
Applied to zesty/master-next branch. Thank you.
diff mbox

Patch

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index c13fec451d03..331bbe38e34b 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1322,7 +1322,7 @@  static void autoconfig(struct uart_8250_port *up)
 	/*
 	 * Check if the device is a Fintek F81216A
 	 */
-	if (port->type == PORT_16550A)
+	if (port->type == PORT_16550A && port->iotype == UPIO_PORT)
 		fintek_8250_probe(up);
 
 	if (up->capabilities != old_capabilities) {