Skip to contentSkip to conversation

serial: 8250_fintek: fix the mismatched IRQ mode

PATCH In review Ji-Ze Hong (Peter Hong) 2 participants 3 messages May 27, 2016 – Jun 28, 2016 TTY layer and serial drivers

lore
Share & tools
lore

Ji-Ze Hong (Peter Hong)

author
#0 lore raw

PATCH 1/1 serial: 8250_fintek: fix the mismatched IRQ mode

Some BIOS only use _OSI("Linux") to distinguish between Linux & Windows. Apply Level/Low to UART trigger mode if Windows, Edge/High mode otherwise. But since 2.6.23 the mainline kernel no longer returns true for _OSI(“Linux”).

The default IRQ0~15 trigger mode in Linux is Edge/High mode without ACPI MADT override. It mismatches IRQ mode and makes UART malfunctional on such motherboard.

This patch will check the current IRQ mode and apply correct mode to UART.

The following link is F81216AD spec PDF:
http://html.alldatasheet.com/html-pdf/257956/FINTEK/F81216AD/5569/
25/F81216AD.html

LDN0~3
70h: IRQ channel & Mode register
Bit 6~5 :
00 : Active low level mode
01 : Active high edge mode
Bit 4 : Sharing Flag (0: not share/1: share)
Bit 3~0 : IRQ channel

Signed-off-by Ji-Ze Hong (Peter Hong) <[email protected]>
---
 drivers/tty/serial/8250/8250_fintek.c | 36 ++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)
drivers/tty/serial/8250/8250_fintek.c +35 -1
index 870981d..737b4b3 100644
@@ -13,6 +13,7 @@
 #include <linux/pnp.h>
 #include <linux/kernel.h>
 #include <linux/serial_core.h>
+#include <linux/irq.h>
 #include  "8250.h"
 
 #define ADDR_PORT 0

@@ -30,6 +31,12 @@
 #define IO_ADDR2 0x60
 #define LDN 0x7
 
+#define IRQ_MODE	0x70
+#define IRQ_SHARE	BIT(4)
+#define IRQ_MODE_MASK	(BIT(6) | BIT(5))
+#define IRQ_LEVEL_LOW	0
+#define IRQ_EDGE_HIGH	BIT(5)
+
 #define RS485  0xF0
 #define RTS_INVERT BIT(5)
 #define RS485_URA BIT(4)

@@ -176,10 +183,37 @@ static int find_base_port(struct fintek_8250 *pdata, u16 io_address)
 	return -ENODEV;
 }
 
+static int fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool level_mode)
+{
+	int status;
+	u8 tmp;
+
+	status = fintek_8250_enter_key(pdata->base_port, pdata->key);
+	if (status)
+		return status;
+
+	outb(LDN, pdata->base_port + ADDR_PORT);
+	outb(pdata->index, pdata->base_port + DATA_PORT);
+
+	outb(IRQ_MODE, pdata->base_port + ADDR_PORT);
+	tmp = inb(pdata->base_port + DATA_PORT);
+
+	tmp &= ~IRQ_MODE_MASK;
+	tmp |= IRQ_SHARE;
+	if (!level_mode)
+		tmp |= IRQ_EDGE_HIGH;
+
+	outb(tmp, pdata->base_port + DATA_PORT);
+	fintek_8250_exit_key(pdata->base_port);
+	return 0;
+}
+
 int fintek_8250_probe(struct uart_8250_port *uart)
 {
 	struct fintek_8250 *pdata;
 	struct fintek_8250 probe_data;
+	struct irq_data *irq_data = irq_get_irq_data(uart->port.irq);
+	bool level_mode = irqd_is_level_type(irq_data);
 
 	if (find_base_port(&probe_data, uart->port.iobase))
 		return -ENODEV;

@@ -192,5 +226,5 @@ int fintek_8250_probe(struct uart_8250_port *uart)
 	uart->port.rs485_config = fintek_8250_rs485_config;
 	uart->port.private_data = pdata;
 
-	return 0;
+	return fintek_8250_set_irq_mode(pdata, level_mode);
 }
Signature · 1 line
1.9.1
#1 lore raw

Hi Peter,. Hi Greg

drivers/tty/serial/8250/8250_fintek.c static int find_base_port(struct fintek_8250 *pdata, u16 io_address) · line 216

Quoted 5 lines · Ji-Ze Hong (Peter Hong)
 {
        struct fintek_8250 *pdata;
        struct fintek_8250 probe_data;
drivers/tty/serial/8250/8250_fintek.c +2 -0
+       struct irq_data *irq_data = irq_get_irq_data(uart->port.irq);
+       bool level_mode = irqd_is_level_type(irq_data);

I see a potential problem with this patch:

If irq_get_irq_data fails, it will return NULL, resulting on a segmentation fault on irqd_is_level_type:

#define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors)

I believe that we need to add error checks here

drivers/tty/serial/8250/8250_fintek.c int fintek_8250_probe(struct uart_8250_port *uart) · line 229

Quoted 9 lines
        if (find_base_port(&probe_data, uart->port.iobase))
                return -ENODEV;
drivers/tty/serial/8250/8250_fintek.c +1 -1
@@ -192,5 +226,5 @@ int fintek_8250_probe(struct uart_8250_port *uart)
        uart->port.rs485_config = fintek_8250_rs485_config;
        uart->port.private_data = pdata;
 
-       return 0;
+       return fintek_8250_set_irq_mode(pdata, level_mode);

Also why do not call irq_get_irq_data() and irqd_is_level_type() before return? There is no need to do that work if the probe determines that it is not a fintek chip.

Greg, this is already queued for 4.7.... what shall we do here?

Regards!

Signature · 1 line
Ricardo Ribalda
author
#2 lore raw

Hi Ricardo & Greg,

Ricardo Ribalda Delgado 於 2016/6/27 下午 06:25 寫道:

Quoted 7 lines

Hi Peter,. Hi Greg
On Fri, May 27, 2016 at 4:02 AM, Ji-Ze Hong (Peter Hong)

drivers/tty/serial/8250/8250_fintek.c static int find_base_port(struct fintek_8250 *pdata, u16 io_address) · line 216

Quoted 2 lines
drivers/tty/serial/8250/8250_fintek.c +2 -0
+       struct irq_data *irq_data = irq_get_irq_data(uart->port.irq);
+       bool level_mode = irqd_is_level_type(irq_data);

I see a potential problem with this patch:

Quoted 7 lines

If irq_get_irq_data fails, it will return NULL, resulting on a segmentation fault on irqd_is_level_type:

#define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors)

I believe that we need to add error checks here

I'll try to send a patch to add more checks.

Quoted 7 lines

drivers/tty/serial/8250/8250_fintek.c int fintek_8250_probe(struct uart_8250_port *uart) · line 229

Quoted 2 lines
drivers/tty/serial/8250/8250_fintek.c +1 -1
-       return 0;
+       return fintek_8250_set_irq_mode(pdata, level_mode);

Also why do not call irq_get_irq_data() and irqd_is_level_type() before return? There is no need to do that work if the probe determines that it is not a fintek chip.

Also determine the IRQ mode when found a fintek chip.

Thanks to point out the potential issue.

Signature · 2 lines
With Best Regards,
Peter Hung