2015-06-15 14:18:52

by Ricardo Ribalda Delgado

[permalink] [raw]
Subject: [PATCH 0/5] serial/8250_fintek Support any configuration.

The original driver only supported the default configuration of the chip.

This patchset add supports for all the possible configurations:
-Different io address
-Multiple chips
-Different chip_ids

Reported-by: Peter Hong <[email protected]>

Ricardo Ribalda Delgado (5):
serial/8250_fintek: Use private data structure
serial/8250_fintek: Support for multiple base_ports
serial/8250_fintek: Support for chip_ip 0x0501
serial/8250_fintek: Support keys different than default
serial/8250_fintek: Support for any io address.

drivers/tty/serial/8250/8250_fintek.c | 170 +++++++++++++++++++++-------------
1 file changed, 106 insertions(+), 64 deletions(-)

--
2.1.4


2015-06-15 14:20:25

by Ricardo Ribalda Delgado

[permalink] [raw]
Subject: [PATCH 1/5] serial/8250_fintek: Use private data structure

Save the port index and the line id in a private structure.

Reported-by: Peter Hong <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serial/8250/8250_fintek.c | 48 ++++++++++++++++++++++-------------
1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index 5815e81b5fc6..c7f1485e6a54 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -39,6 +39,11 @@

#define DRIVER_NAME "8250_fintek"

+struct fintek_8250 {
+ u8 index;
+ long line;
+};
+
static int fintek_8250_enter_key(void){

if (!request_muxed_region(ADDR_PORT, 2, DRIVER_NAME))
@@ -93,9 +98,9 @@ static int fintek_8250_rs485_config(struct uart_port *port,
struct serial_rs485 *rs485)
{
uint8_t config = 0;
- int index = fintek_8250_get_index(port->iobase);
+ struct fintek_8250 *pdata = port->private_data;

- if (index < 0)
+ if (!pdata)
return -EINVAL;

if (rs485->flags & SER_RS485_ENABLED)
@@ -129,7 +134,7 @@ static int fintek_8250_rs485_config(struct uart_port *port,
return -EBUSY;

outb(LDN, ADDR_PORT);
- outb(index, DATA_PORT);
+ outb(pdata->index, DATA_PORT);
outb(RS485, ADDR_PORT);
outb(config, DATA_PORT);
fintek_8250_exit_key();
@@ -142,14 +147,16 @@ static int fintek_8250_rs485_config(struct uart_port *port,
static int
fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
{
- int line;
struct uart_8250_port uart;
+ struct fintek_8250 *pdata;
int ret;
+ int index;

if (!pnp_port_valid(dev, 0))
return -ENODEV;

- if (fintek_8250_get_index(pnp_port_start(dev, 0)) < 0)
+ index = fintek_8250_get_index(pnp_port_start(dev, 0));
+ if (index < 0)
return -ENODEV;

/* Enable configuration registers*/
@@ -162,6 +169,12 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
if (ret)
return ret;

+ pdata = devm_kzalloc(&dev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ uart.port.private_data = pdata;
+
memset(&uart, 0, sizeof(uart));
if (!pnp_irq_valid(dev, 0))
return -ENODEV;
@@ -176,40 +189,41 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
uart.port.uartclk = 1843200;
uart.port.dev = &dev->dev;

- line = serial8250_register_8250_port(&uart);
- if (line < 0)
+ pdata->index = index;
+ pdata->line = serial8250_register_8250_port(&uart);
+ if (pdata->line < 0)
return -ENODEV;

- pnp_set_drvdata(dev, (void *)((long)line + 1));
+ pnp_set_drvdata(dev, pdata);
return 0;
}

static void fintek_8250_remove(struct pnp_dev *dev)
{
- long line = (long)pnp_get_drvdata(dev);
+ struct fintek_8250 *pdata = pnp_get_drvdata(dev);

- if (line)
- serial8250_unregister_port(line - 1);
+ if (pdata)
+ serial8250_unregister_port(pdata->line);
}

#ifdef CONFIG_PM
static int fintek_8250_suspend(struct pnp_dev *dev, pm_message_t state)
{
- long line = (long)pnp_get_drvdata(dev);
+ struct fintek_8250 *pdata = pnp_get_drvdata(dev);

- if (!line)
+ if (!pdata)
return -ENODEV;
- serial8250_suspend_port(line - 1);
+ serial8250_suspend_port(pdata->line);
return 0;
}

static int fintek_8250_resume(struct pnp_dev *dev)
{
- long line = (long)pnp_get_drvdata(dev);
+ struct fintek_8250 *pdata = pnp_get_drvdata(dev);

- if (!line)
+ if (!pdata)
return -ENODEV;
- serial8250_resume_port(line - 1);
+ serial8250_resume_port(pdata->line);
return 0;
}
#else
--
2.1.4

2015-06-15 14:18:58

by Ricardo Ribalda Delgado

[permalink] [raw]
Subject: [PATCH 2/5] serial/8250_fintek: Support for multiple base_ports

Fintek chip can be connected at address 0x4e and also 0x2e.
Add some logic to find out the address of the chip.

Reported-by: Peter Hong <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serial/8250/8250_fintek.c | 87 +++++++++++++++++++++--------------
1 file changed, 52 insertions(+), 35 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index c7f1485e6a54..ec64ae5f3bb3 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -17,8 +17,8 @@
#include <linux/serial_core.h>
#include "8250.h"

-#define ADDR_PORT 0x4E
-#define DATA_PORT 0x4F
+#define ADDR_PORT 0
+#define DATA_PORT 1
#define ENTRY_KEY 0x77
#define EXIT_KEY 0xAA
#define CHIP_ID1 0x20
@@ -40,24 +40,27 @@
#define DRIVER_NAME "8250_fintek"

struct fintek_8250 {
+ u16 base_port;
u8 index;
long line;
};

-static int fintek_8250_enter_key(void){
+static int fintek_8250_enter_key(u16 base_port)
+{

- if (!request_muxed_region(ADDR_PORT, 2, DRIVER_NAME))
+ if (!request_muxed_region(base_port, 2, DRIVER_NAME))
return -EBUSY;

- outb(ENTRY_KEY, ADDR_PORT);
- outb(ENTRY_KEY, ADDR_PORT);
+ outb(ENTRY_KEY, base_port + ADDR_PORT);
+ outb(ENTRY_KEY, base_port + ADDR_PORT);
return 0;
}

-static void fintek_8250_exit_key(void){
+static void fintek_8250_exit_key(u16 base_port)
+{

- outb(EXIT_KEY, ADDR_PORT);
- release_region(ADDR_PORT, 2);
+ outb(EXIT_KEY, base_port + ADDR_PORT);
+ release_region(base_port + ADDR_PORT, 2);
}

static int fintek_8250_get_index(resource_size_t base_addr)
@@ -72,23 +75,23 @@ static int fintek_8250_get_index(resource_size_t base_addr)
return -ENODEV;
}

-static int fintek_8250_check_id(void)
+static int fintek_8250_check_id(u16 base_port)
{

- outb(CHIP_ID1, ADDR_PORT);
- if (inb(DATA_PORT) != CHIP_ID1_VAL)
+ outb(CHIP_ID1, base_port + ADDR_PORT);
+ if (inb(base_port + DATA_PORT) != CHIP_ID1_VAL)
return -ENODEV;

- outb(CHIP_ID2, ADDR_PORT);
- if (inb(DATA_PORT) != CHIP_ID2_VAL)
+ outb(CHIP_ID2, base_port + ADDR_PORT);
+ if (inb(base_port + DATA_PORT) != CHIP_ID2_VAL)
return -ENODEV;

- outb(VENDOR_ID1, ADDR_PORT);
- if (inb(DATA_PORT) != VENDOR_ID1_VAL)
+ outb(VENDOR_ID1, base_port + ADDR_PORT);
+ if (inb(base_port + DATA_PORT) != VENDOR_ID1_VAL)
return -ENODEV;

- outb(VENDOR_ID2, ADDR_PORT);
- if (inb(DATA_PORT) != VENDOR_ID2_VAL)
+ outb(VENDOR_ID2, base_port + ADDR_PORT);
+ if (inb(base_port + DATA_PORT) != VENDOR_ID2_VAL)
return -ENODEV;

return 0;
@@ -130,45 +133,58 @@ static int fintek_8250_rs485_config(struct uart_port *port,
if (rs485->flags & SER_RS485_RTS_ON_SEND)
config |= RTS_INVERT;

- if (fintek_8250_enter_key())
+ if (fintek_8250_enter_key(pdata->base_port))
return -EBUSY;

- outb(LDN, ADDR_PORT);
- outb(pdata->index, DATA_PORT);
- outb(RS485, ADDR_PORT);
- outb(config, DATA_PORT);
- fintek_8250_exit_key();
+ outb(LDN, pdata->base_port + ADDR_PORT);
+ outb(pdata->index, pdata->base_port + DATA_PORT);
+ outb(RS485, pdata->base_port + ADDR_PORT);
+ outb(config, pdata->base_port + DATA_PORT);
+ fintek_8250_exit_key(pdata->base_port);

port->rs485 = *rs485;

return 0;
}

+static int fintek_8250_base_port(void)
+{
+ u16 addr[] = {0x4e, 0x2e};
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(addr); i++) {
+ int ret;
+
+ if (fintek_8250_enter_key(addr[i]))
+ continue;
+ ret = fintek_8250_check_id(addr[i]);
+ fintek_8250_exit_key(addr[i]);
+ if (!ret)
+ return addr[i];
+ }
+
+ return -ENODEV;
+}
+
static int
fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
{
struct uart_8250_port uart;
struct fintek_8250 *pdata;
- int ret;
int index;
+ int base_port;

if (!pnp_port_valid(dev, 0))
return -ENODEV;

+ base_port = fintek_8250_base_port();
+ if (base_port < 0)
+ return -ENODEV;
+
index = fintek_8250_get_index(pnp_port_start(dev, 0));
if (index < 0)
return -ENODEV;

- /* Enable configuration registers*/
- if (fintek_8250_enter_key())
- return -EBUSY;
-
- /*Check ID*/
- ret = fintek_8250_check_id();
- fintek_8250_exit_key();
- if (ret)
- return ret;
-
pdata = devm_kzalloc(&dev->dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
return -ENOMEM;
@@ -189,6 +205,7 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
uart.port.uartclk = 1843200;
uart.port.dev = &dev->dev;

+ pdata->base_port = base_port;
pdata->index = index;
pdata->line = serial8250_register_8250_port(&uart);
if (pdata->line < 0)
--
2.1.4

2015-06-15 14:18:55

by Ricardo Ribalda Delgado

[permalink] [raw]
Subject: [PATCH 3/5] serial/8250_fintek: Support for chip_ip 0x0501

There are some chips with the same interface but different chip ip.

Reported-by: Peter Hong <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serial/8250/8250_fintek.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index ec64ae5f3bb3..09b505e0c4d1 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -22,9 +22,9 @@
#define ENTRY_KEY 0x77
#define EXIT_KEY 0xAA
#define CHIP_ID1 0x20
-#define CHIP_ID1_VAL 0x02
#define CHIP_ID2 0x21
-#define CHIP_ID2_VAL 0x16
+#define CHIP_ID_0 0x1602
+#define CHIP_ID_1 0x0501
#define VENDOR_ID1 0x23
#define VENDOR_ID1_VAL 0x19
#define VENDOR_ID2 0x24
@@ -77,14 +77,7 @@ static int fintek_8250_get_index(resource_size_t base_addr)

static int fintek_8250_check_id(u16 base_port)
{
-
- outb(CHIP_ID1, base_port + ADDR_PORT);
- if (inb(base_port + DATA_PORT) != CHIP_ID1_VAL)
- return -ENODEV;
-
- outb(CHIP_ID2, base_port + ADDR_PORT);
- if (inb(base_port + DATA_PORT) != CHIP_ID2_VAL)
- return -ENODEV;
+ u16 chip;

outb(VENDOR_ID1, base_port + ADDR_PORT);
if (inb(base_port + DATA_PORT) != VENDOR_ID1_VAL)
@@ -94,6 +87,14 @@ static int fintek_8250_check_id(u16 base_port)
if (inb(base_port + DATA_PORT) != VENDOR_ID2_VAL)
return -ENODEV;

+ outb(CHIP_ID1, base_port + ADDR_PORT);
+ chip = inb(base_port + DATA_PORT);
+ outb(CHIP_ID2, base_port + ADDR_PORT);
+ chip |= inb(base_port + DATA_PORT) << 8;
+
+ if (chip != CHIP_ID_0 && chip != CHIP_ID_1)
+ return -ENODEV;
+
return 0;
}

--
2.1.4

2015-06-15 14:19:00

by Ricardo Ribalda Delgado

[permalink] [raw]
Subject: [PATCH 4/5] serial/8250_fintek: Support keys different than default

Chip can be configured to use entry key different than 0x77. Try all the
valid keys until one gives out the right chip id.

Reported-by: Peter Hong <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serial/8250/8250_fintek.c | 39 +++++++++++++++++++++--------------
1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index 09b505e0c4d1..c7e7ccf3f198 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -19,7 +19,6 @@

#define ADDR_PORT 0
#define DATA_PORT 1
-#define ENTRY_KEY 0x77
#define EXIT_KEY 0xAA
#define CHIP_ID1 0x20
#define CHIP_ID2 0x21
@@ -42,17 +41,18 @@
struct fintek_8250 {
u16 base_port;
u8 index;
+ u8 key;
long line;
};

-static int fintek_8250_enter_key(u16 base_port)
+static int fintek_8250_enter_key(u16 base_port, u8 key)
{

if (!request_muxed_region(base_port, 2, DRIVER_NAME))
return -EBUSY;

- outb(ENTRY_KEY, base_port + ADDR_PORT);
- outb(ENTRY_KEY, base_port + ADDR_PORT);
+ outb(key, base_port + ADDR_PORT);
+ outb(key, base_port + ADDR_PORT);
return 0;
}

@@ -134,7 +134,7 @@ static int fintek_8250_rs485_config(struct uart_port *port,
if (rs485->flags & SER_RS485_RTS_ON_SEND)
config |= RTS_INVERT;

- if (fintek_8250_enter_key(pdata->base_port))
+ if (fintek_8250_enter_key(pdata->base_port, pdata->key))
return -EBUSY;

outb(LDN, pdata->base_port + ADDR_PORT);
@@ -148,20 +148,25 @@ static int fintek_8250_rs485_config(struct uart_port *port,
return 0;
}

-static int fintek_8250_base_port(void)
+static int fintek_8250_base_port(u8 *key)
{
u16 addr[] = {0x4e, 0x2e};
- int i;
+ u16 keys[] = {0x77, 0xa0, 0x87, 0x67};
+ int i, j;

for (i = 0; i < ARRAY_SIZE(addr); i++) {
- int ret;
-
- if (fintek_8250_enter_key(addr[i]))
- continue;
- ret = fintek_8250_check_id(addr[i]);
- fintek_8250_exit_key(addr[i]);
- if (!ret)
- return addr[i];
+ for (j = 0; j < ARRAY_SIZE(keys); j++) {
+ int ret;
+
+ if (fintek_8250_enter_key(addr[i], keys[j]))
+ continue;
+ ret = fintek_8250_check_id(addr[i]);
+ fintek_8250_exit_key(addr[i]);
+ if (!ret) {
+ *key = keys[j];
+ return addr[i];
+ }
+ }
}

return -ENODEV;
@@ -174,11 +179,12 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
struct fintek_8250 *pdata;
int index;
int base_port;
+ u8 key;

if (!pnp_port_valid(dev, 0))
return -ENODEV;

- base_port = fintek_8250_base_port();
+ base_port = fintek_8250_base_port(&key);
if (base_port < 0)
return -ENODEV;

@@ -206,6 +212,7 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
uart.port.uartclk = 1843200;
uart.port.dev = &dev->dev;

+ pdata->key = key;
pdata->base_port = base_port;
pdata->index = index;
pdata->line = serial8250_register_8250_port(&uart);
--
2.1.4

2015-06-15 14:19:56

by Ricardo Ribalda Delgado

[permalink] [raw]
Subject: [PATCH 5/5] serial/8250_fintek: Support for any io address.

Fintek chip can be configured for io addresses different than the standard.

Query the chip for the configured addresses and try to match it with the
pnp address.

Reported-by: Peter Hong <[email protected]>
Signed-off-by: Ricardo Ribalda Delgado <[email protected]>
---
drivers/tty/serial/8250/8250_fintek.c | 51 ++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
index c7e7ccf3f198..07b98b5bb930 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -28,6 +28,8 @@
#define VENDOR_ID1_VAL 0x19
#define VENDOR_ID2 0x24
#define VENDOR_ID2_VAL 0x34
+#define IO_ADDR1 0x61
+#define IO_ADDR2 0x60
#define LDN 0x7

#define RS485 0xF0
@@ -63,18 +65,6 @@ static void fintek_8250_exit_key(u16 base_port)
release_region(base_port + ADDR_PORT, 2);
}

-static int fintek_8250_get_index(resource_size_t base_addr)
-{
- resource_size_t base[] = {0x3f8, 0x2f8, 0x3e8, 0x2e8};
- int i;
-
- for (i = 0; i < ARRAY_SIZE(base); i++)
- if (base_addr == base[i])
- return i;
-
- return -ENODEV;
-}
-
static int fintek_8250_check_id(u16 base_port)
{
u16 chip;
@@ -148,24 +138,41 @@ static int fintek_8250_rs485_config(struct uart_port *port,
return 0;
}

-static int fintek_8250_base_port(u8 *key)
+static int fintek_8250_base_port(u16 io_address, u8 *key, u8 *index)
{
u16 addr[] = {0x4e, 0x2e};
u16 keys[] = {0x77, 0xa0, 0x87, 0x67};
- int i, j;
+ int i, j, k;

for (i = 0; i < ARRAY_SIZE(addr); i++) {
for (j = 0; j < ARRAY_SIZE(keys); j++) {
- int ret;

if (fintek_8250_enter_key(addr[i], keys[j]))
continue;
- ret = fintek_8250_check_id(addr[i]);
- fintek_8250_exit_key(addr[i]);
- if (!ret) {
+ if (fintek_8250_check_id(addr[i])) {
+ fintek_8250_exit_key(addr[i]);
+ continue;
+ }
+
+ for (k = 0; k < 4; k++) {
+ u16 aux;
+
+ outb(LDN, addr[i] + ADDR_PORT);
+ outb(k, addr[i] + DATA_PORT);
+
+ outb(IO_ADDR1, addr[i] + ADDR_PORT);
+ aux = inb(addr[i] + DATA_PORT);
+ outb(IO_ADDR2, addr[i] + ADDR_PORT);
+ aux |= inb(addr[i] + DATA_PORT) << 8;
+ if (aux != io_address)
+ continue;
+
+ fintek_8250_exit_key(addr[i]);
*key = keys[j];
+ *index = k;
return addr[i];
}
+ fintek_8250_exit_key(addr[i]);
}
}

@@ -177,21 +184,17 @@ fintek_8250_probe(struct pnp_dev *dev, const struct pnp_device_id *dev_id)
{
struct uart_8250_port uart;
struct fintek_8250 *pdata;
- int index;
int base_port;
u8 key;
+ u8 index;

if (!pnp_port_valid(dev, 0))
return -ENODEV;

- base_port = fintek_8250_base_port(&key);
+ base_port = fintek_8250_base_port(pnp_port_start(dev, 0), &key, &index);
if (base_port < 0)
return -ENODEV;

- index = fintek_8250_get_index(pnp_port_start(dev, 0));
- if (index < 0)
- return -ENODEV;
-
pdata = devm_kzalloc(&dev->dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
return -ENOMEM;
--
2.1.4

2015-06-15 14:26:15

by Alan

[permalink] [raw]
Subject: Re: [PATCH 5/5] serial/8250_fintek: Support for any io address.

On Mon, 2015-06-15 at 16:18 +0200, Ricardo Ribalda Delgado wrote:
> Fintek chip can be configured for io addresses different than the standard.
>
> Query the chip for the configured addresses and try to match it with the
> pnp address.

Looks ok to me. I am a little concerned about false positives but it
does everything right in terms of resource management so I guess it
should be ok - and if not we are only going to find the corner case that
blows up by shipping it 8)

Please however fix the dynamic on stack arrays you've got. Doing

blah foo[] = { 1,2,3,4 };

might well be valid gcc but it generates horrible code when in fact your
arrays should be static/const, not dynamically constructed on stack from
a static copy each call.

Reviewed-by: Alan Cox <[email protected]>

2015-06-15 15:22:35

by Ricardo Ribalda Delgado

[permalink] [raw]
Subject: Re: [PATCH 5/5] serial/8250_fintek: Support for any io address.

Hello Alan

On Mon, Jun 15, 2015 at 4:25 PM, Alan Cox <[email protected]> wrote:

>
> Please however fix the dynamic on stack arrays you've got. Doing
>
> blah foo[] = { 1,2,3,4 };
>
> might well be valid gcc but it generates horrible code when in fact your
> arrays should be static/const, not dynamically constructed on stack from
> a static copy each call.

Thanks for the comments, I have resubmitted the patchset.

>
> Reviewed-by: Alan Cox <[email protected]>
>



--
Ricardo Ribalda