Fix real serial port count for F81504/508/512 with multi-function mode.
Fintek F81504/508/512 are multi-function boards. It could be configurated
via PCI configuration space register F0h/F3h with external EEPROM that
programmed by customer.
F0h bit0~5: Enable GPIO0~5
bit6~7: Reserve
F3h bit0~5: Multi-Functional Flag (0:GPIO/1:UART)
bit0: UART2 pin out for UART2 / GPIO0
bit1: UART3 pin out for UART3 / GPIO1
bit2: UART8 pin out for UART8 / GPIO2
bit3: UART9 pin out for UART9 / GPIO3
bit4: UART10 pin out for UART10 / GPIO4
bit5: UART11 pin out for UART11 / GPIO5
bit6~7: Reserve
It'll use (F0h & ~F3h) & 0x3f union set to find max set of GPIOs.
If a port is indicated as GPIO, it'll not report as serial port and reserve
for userspace to manipulate.
F81504: Max for 4 serial ports.
UART2/3 is multi-function.
F81508: Max for 8 serial ports.
UART2/3 is multi-function.
8/9/10/11 is GPIO only
F81512: Max for 12 serial ports.
UART2/3/8/9/10/11 is multi-function.
Signed-off-by: Peter Hung <[email protected]>
---
drivers/tty/serial/8250/8250_pci.c | 114 +++++++++++++++++++++++++++++++++++--
1 file changed, 108 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 4097f3f..8a639cb 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1532,6 +1532,9 @@ pci_brcm_trumanage_setup(struct serial_private *priv,
/* only worked with FINTEK_RTS_CONTROL_BY_HW on */
#define FINTEK_RTS_INVERT BIT(5)
+/* The device is multi-function with UART & GPIO */
+static u8 fintek_gpio_mapping[] = {2, 3, 8, 9, 10, 11};
+
/* We should do proper H/W transceiver setting before change to RS485 mode */
static int pci_fintek_rs485_config(struct uart_port *port,
struct serial_rs485 *rs485)
@@ -1586,10 +1589,41 @@ static int pci_fintek_setup(struct serial_private *priv,
{
struct pci_dev *pdev = priv->dev;
u8 *data;
- u8 config_base;
- u16 iobase;
+ u8 tmp;
+ u8 config_base = 0;
+ u16 iobase, i, max_port, count = 0;
- config_base = 0x40 + 0x08 * idx;
+ switch (pdev->device) {
+ case 0x1104: /* 4 ports */
+ case 0x1108: /* 8 ports */
+ max_port = pdev->device & 0xff;
+ break;
+ case 0x1112: /* 12 ports */
+ max_port = 12;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* find real serial port index from logic idx */
+ for (i = 0; i < max_port; ++i) {
+ config_base = 0x40 + 0x08 * i;
+
+ pci_read_config_byte(pdev, config_base, &tmp);
+ if (!tmp)
+ continue;
+
+ if (count == idx)
+ break;
+
+ ++count;
+ }
+
+ if (i >= max_port) {
+ dev_err(&pdev->dev, "%s: mapping error! i=%d, idx=%d\n",
+ __func__, i, idx);
+ return -ENODEV;
+ }
/* Get the io address from configuration space */
pci_read_config_word(pdev, config_base + 4, &iobase);
@@ -1604,8 +1638,8 @@ static int pci_fintek_setup(struct serial_private *priv,
if (!data)
return -ENOMEM;
- /* preserve index in PCI configuration space */
- *data = idx;
+ /* preserve real index in PCI configuration space */
+ *data = i;
port->port.private_data = data;
return 0;
@@ -1614,12 +1648,40 @@ static int pci_fintek_setup(struct serial_private *priv,
static int pci_fintek_init(struct pci_dev *dev)
{
unsigned long iobase;
- u32 max_port, i;
+ u32 max_port, i, j;
u32 bar_data[3];
u8 config_base;
+ u8 tmp, f0h_data, f3h_data;
+ bool skip_init;
struct serial_private *priv = pci_get_drvdata(dev);
struct uart_8250_port *port;
+ /*
+ * The PCI board is multi-function, some serial port can converts to
+ * GPIO function. Customers could changes the F0/F3h values in EEPROM
+ *
+ * F0h bit0~5: Enable GPIO0~5
+ * bit6~7: Reserve
+ *
+ * F3h bit0~5: Multi-Functional Flag (0:GPIO/1:UART)
+ * bit0: UART2 pin out for UART2 / GPIO0
+ * bit1: UART3 pin out for UART3 / GPIO1
+ * bit2: UART8 pin out for UART8 / GPIO2
+ * bit3: UART9 pin out for UART9 / GPIO3
+ * bit4: UART10 pin out for UART10 / GPIO4
+ * bit5: UART11 pin out for UART11 / GPIO5
+ * bit6~7: Reserve
+ */
+ pci_read_config_byte(dev, 0xf0, &f0h_data);
+ pci_read_config_byte(dev, 0xf3, &f3h_data);
+
+ /* find the max set of GPIOs */
+ tmp = f0h_data | ~f3h_data;
+
+ /* rewrite GPIO setting */
+ pci_write_config_byte(dev, 0xf0, tmp & 0x3f);
+ pci_write_config_byte(dev, 0xf3, ~tmp & 0x3f);
+
switch (dev->device) {
case 0x1104: /* 4 ports */
case 0x1108: /* 8 ports */
@@ -1641,6 +1703,32 @@ static int pci_fintek_init(struct pci_dev *dev)
/* UART0 configuration offset start from 0x40 */
config_base = 0x40 + 0x08 * i;
+ skip_init = false;
+
+ /* find every port to check is multi-function port? */
+ for (j = 0; j < ARRAY_SIZE(fintek_gpio_mapping); ++j) {
+ if (fintek_gpio_mapping[j] != i || !(tmp & BIT(j)))
+ continue;
+
+ /*
+ * This port is multi-function and enabled as gpio
+ * mode. So we'll not configure it as serial port.
+ */
+ skip_init = true;
+ break;
+ }
+
+ /*
+ * If the serial port is setting to gpio mode, don't init it.
+ * Disable the serial port for user-space application to
+ * control.
+ */
+ if (skip_init) {
+ /* Disable current serial port */
+ pci_write_config_byte(dev, config_base + 0x00, 0x00);
+ continue;
+ }
+
/* Calculate Real IO Port */
iobase = (bar_data[i / 4] & 0xffffffe0) + (i % 4) * 8;
@@ -1674,6 +1762,20 @@ static int pci_fintek_init(struct pci_dev *dev)
}
}
+ /* Calculate real serial port number */
+ for (i = 0; i < ARRAY_SIZE(fintek_gpio_mapping); ++i) {
+ switch (dev->device) {
+ case 0x1104: /* 4 ports */
+ case 0x1108: /* 8 ports */
+ if (i >= 2) /* Ignore all bits higher than 0 & 1 */
+ break;
+ case 0x1112: /* 12 ports */
+ if (tmp & BIT(i))
+ --max_port;
+ break;
+ }
+ }
+
return max_port;
}
--
1.9.1
On Tue, Dec 1, 2015 at 8:54 AM, Peter Hung <[email protected]> wrote:
> Fix real serial port count for F81504/508/512 with multi-function mode.
>
> Fintek F81504/508/512 are multi-function boards. It could be configurated
> via PCI configuration space register F0h/F3h with external EEPROM that
> programmed by customer.
>
> F0h bit0~5: Enable GPIO0~5
> bit6~7: Reserve
>
> F3h bit0~5: Multi-Functional Flag (0:GPIO/1:UART)
> bit0: UART2 pin out for UART2 / GPIO0
> bit1: UART3 pin out for UART3 / GPIO1
> bit2: UART8 pin out for UART8 / GPIO2
> bit3: UART9 pin out for UART9 / GPIO3
> bit4: UART10 pin out for UART10 / GPIO4
> bit5: UART11 pin out for UART11 / GPIO5
> bit6~7: Reserve
>
> It'll use (F0h & ~F3h) & 0x3f union set to find max set of GPIOs.
> If a port is indicated as GPIO, it'll not report as serial port and reserve
> for userspace to manipulate.
>
> F81504: Max for 4 serial ports.
> UART2/3 is multi-function.
>
> F81508: Max for 8 serial ports.
> UART2/3 is multi-function.
> 8/9/10/11 is GPIO only
>
> F81512: Max for 12 serial ports.
> UART2/3/8/9/10/11 is multi-function.
First of all, maybe you can consider to split this part of the driver
to separate one? (Like we did for 8250_mid.c). It seems 8250_pci is
too bloated. But it's just an idea, maybe for future.
>
> Signed-off-by: Peter Hung <[email protected]>
> ---
> drivers/tty/serial/8250/8250_pci.c | 114 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 108 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 4097f3f..8a639cb 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1532,6 +1532,9 @@ pci_brcm_trumanage_setup(struct serial_private *priv,
> /* only worked with FINTEK_RTS_CONTROL_BY_HW on */
> #define FINTEK_RTS_INVERT BIT(5)
>
> +/* The device is multi-function with UART & GPIO */
> +static u8 fintek_gpio_mapping[] = {2, 3, 8, 9, 10, 11};
Clearly you have bit combination here
Bit 1: 1
Bit 3: 1
So, mask as 0x0a shall cover this IIAC.
> +
> /* We should do proper H/W transceiver setting before change to RS485 mode */
> static int pci_fintek_rs485_config(struct uart_port *port,
> struct serial_rs485 *rs485)
> @@ -1586,10 +1589,41 @@ static int pci_fintek_setup(struct serial_private *priv,
> {
> struct pci_dev *pdev = priv->dev;
> u8 *data;
> - u8 config_base;
> - u16 iobase;
> + u8 tmp;
> + u8 config_base = 0;
> + u16 iobase, i, max_port, count = 0;
>
> - config_base = 0x40 + 0x08 * idx;
> + switch (pdev->device) {
> + case 0x1104: /* 4 ports */
Maybe you can introduce constants for IDs.
> + case 0x1108: /* 8 ports */
> + max_port = pdev->device & 0xff;
> + break;
> + case 0x1112: /* 12 ports */
> + max_port = 12;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + /* find real serial port index from logic idx */
> + for (i = 0; i < max_port; ++i) {
> + config_base = 0x40 + 0x08 * i;
> +
> + pci_read_config_byte(pdev, config_base, &tmp);
> + if (!tmp)
> + continue;
> +
> + if (count == idx)
> + break;
> +
> + ++count;
> + }
> +
> + if (i >= max_port) {
> + dev_err(&pdev->dev, "%s: mapping error! i=%d, idx=%d\n",
> + __func__, i, idx);
> + return -ENODEV;
> + }
>
> /* Get the io address from configuration space */
> pci_read_config_word(pdev, config_base + 4, &iobase);
> @@ -1604,8 +1638,8 @@ static int pci_fintek_setup(struct serial_private *priv,
> if (!data)
> return -ENOMEM;
>
> - /* preserve index in PCI configuration space */
> - *data = idx;
> + /* preserve real index in PCI configuration space */
> + *data = i;
> port->port.private_data = data;
>
> return 0;
> @@ -1614,12 +1648,40 @@ static int pci_fintek_setup(struct serial_private *priv,
> static int pci_fintek_init(struct pci_dev *dev)
> {
> unsigned long iobase;
> - u32 max_port, i;
> + u32 max_port, i, j;
> u32 bar_data[3];
> u8 config_base;
> + u8 tmp, f0h_data, f3h_data;
> + bool skip_init;
> struct serial_private *priv = pci_get_drvdata(dev);
> struct uart_8250_port *port;
>
> + /*
> + * The PCI board is multi-function, some serial port can converts to
> + * GPIO function. Customers could changes the F0/F3h values in EEPROM
> + *
> + * F0h bit0~5: Enable GPIO0~5
> + * bit6~7: Reserve
> + *
> + * F3h bit0~5: Multi-Functional Flag (0:GPIO/1:UART)
> + * bit0: UART2 pin out for UART2 / GPIO0
> + * bit1: UART3 pin out for UART3 / GPIO1
> + * bit2: UART8 pin out for UART8 / GPIO2
> + * bit3: UART9 pin out for UART9 / GPIO3
> + * bit4: UART10 pin out for UART10 / GPIO4
> + * bit5: UART11 pin out for UART11 / GPIO5
> + * bit6~7: Reserve
> + */
> + pci_read_config_byte(dev, 0xf0, &f0h_data);
> + pci_read_config_byte(dev, 0xf3, &f3h_data);
> +
> + /* find the max set of GPIOs */
> + tmp = f0h_data | ~f3h_data;
> +
> + /* rewrite GPIO setting */
> + pci_write_config_byte(dev, 0xf0, tmp & 0x3f);
> + pci_write_config_byte(dev, 0xf3, ~tmp & 0x3f);
Do we have definition for magic f0, f3 ?
> +
> switch (dev->device) {
> case 0x1104: /* 4 ports */
> case 0x1108: /* 8 ports */
> @@ -1641,6 +1703,32 @@ static int pci_fintek_init(struct pci_dev *dev)
> /* UART0 configuration offset start from 0x40 */
> config_base = 0x40 + 0x08 * i;
>
> + skip_init = false;
> +
> + /* find every port to check is multi-function port? */
> + for (j = 0; j < ARRAY_SIZE(fintek_gpio_mapping); ++j) {
> + if (fintek_gpio_mapping[j] != i || !(tmp & BIT(j)))
> + continue;
> +
> + /*
> + * This port is multi-function and enabled as gpio
> + * mode. So we'll not configure it as serial port.
> + */
> + skip_init = true;
> + break;
> + }
> +
> + /*
> + * If the serial port is setting to gpio mode, don't init it.
> + * Disable the serial port for user-space application to
> + * control.
> + */
> + if (skip_init) {
> + /* Disable current serial port */
> + pci_write_config_byte(dev, config_base + 0x00, 0x00);
> + continue;
> + }
> +
> /* Calculate Real IO Port */
> iobase = (bar_data[i / 4] & 0xffffffe0) + (i % 4) * 8;
>
> @@ -1674,6 +1762,20 @@ static int pci_fintek_init(struct pci_dev *dev)
> }
> }
>
> + /* Calculate real serial port number */
> + for (i = 0; i < ARRAY_SIZE(fintek_gpio_mapping); ++i) {
> + switch (dev->device) {
> + case 0x1104: /* 4 ports */
> + case 0x1108: /* 8 ports */
> + if (i >= 2) /* Ignore all bits higher than 0 & 1 */
> + break;
Better to have comment that you are going to pass through.
> + case 0x1112: /* 12 ports */
> + if (tmp & BIT(i))
> + --max_port;
> + break;
> + }
> + }
> +
> return max_port;
> }
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
With Best Regards,
Andy Shevchenko
Hello,
Andy Shevchenko 於 2015/12/13 上午 09:08 寫道:
> On Tue, Dec 1, 2015 at 8:54 AM, Peter Hung <[email protected]> wrote:
> First of all, maybe you can consider to split this part of the driver
> to separate one? (Like we did for 8250_mid.c). It seems 8250_pci is
> too bloated. But it's just an idea, maybe for future.
It's a good idea. Our PCI-to-UART device is a multifunctional
(GPIO/UART) board, I need try to split it from 8250_pci.c to
implements all functions.
>>
>> +/* The device is multi-function with UART & GPIO */
>> +static u8 fintek_gpio_mapping[] = {2, 3, 8, 9, 10, 11};
>
> Clearly you have bit combination here
> Bit 1: 1
> Bit 3: 1
>
> So, mask as 0x0a shall cover this IIAC.
IMO, It maybe wrong. If we checked only with 0x0a mask,
the 0x06 & 0x07 will be passed.
I had try with k-map to reduce from 0~11 (12~15 for don't care).
The final boolean value is a + c b(bar) for a is MSB.
>> - config_base = 0x40 + 0x08 * idx;
>> + switch (pdev->device) {
>> + case 0x1104: /* 4 ports */
>
> Maybe you can introduce constants for IDs.
I'll make the magic numbers with #define marco.
Thanks for your advices.
--
With Best Regards,
Peter Hung
On Tue, Dec 15, 2015 at 4:56 AM, Peter Hung <[email protected]> wrote:
> Andy Shevchenko 於 2015/12/13 上午 09:08 寫道:
>>
>> On Tue, Dec 1, 2015 at 8:54 AM, Peter Hung <[email protected]> wrote:
>>> +/* The device is multi-function with UART & GPIO */
>>> +static u8 fintek_gpio_mapping[] = {2, 3, 8, 9, 10, 11};
>>
>>
>> Clearly you have bit combination here
>> Bit 1: 1
>> Bit 3: 1
>>
>> So, mask as 0x0a shall cover this IIAC.
>
>
> IMO, It maybe wrong. If we checked only with 0x0a mask,
> the 0x06 & 0x07 will be passed.
Ah, yes, it will be a bit more than just one check, something like:
(x & 0x0a) && (x & 0x0e != 6)
--
With Best Regards,
Andy Shevchenko
Hi Greg,
Andy Shevchenko 於 2015/12/13 上午 09:08 寫道:
> First of all, maybe you can consider to split this part of the driver
> to separate one? (Like we did for 8250_mid.c). It seems 8250_pci is
> too bloated. But it's just an idea, maybe for future.
>
Did you have reviewed this patch? Please skip this patch if not
reviewed.
The F81504/508/512 is multi-function card. It contains
Serial/GPIO functions. It maybe enlarge the scale of 8250_pci.c and
make it less maintainable if we add GPIOLIB support into 8250_pci.c
Could I split the F81504/508/512 driver from 8250_pci.c into a
new module file and implements GPIOLIB?
Thanks
--
With Best Regards,
Peter Hung