Re: [PATCH V2 4/4] serial: 8250_pci: Remove Fintek F81504/508/512 UART driver

2016-01-31 Thread Peter Hung 

Hi Andy,

Andy Shevchenko 於 2016/1/29 下午 08:40 寫道:

On Fri, 2016-01-29 at 16:20 +0800, Peter Hung wrote:

Hi Andy,

Andy Shevchenko 於 2016/1/28 下午 08:04 寫道:

On Thu, 2016-01-28 at 17:20 +0800, Peter Hung wrote:

-   /* Fintek PCI serial cards */
-   { PCI_DEVICE(0x1c29, 0x1104), .driver_data =
pbn_fintek_4 },
-   { PCI_DEVICE(0x1c29, 0x1108), .driver_data =
pbn_fintek_8 },
-   { PCI_DEVICE(0x1c29, 0x1112), .driver_data =
pbn_fintek_12
},


Shouldn't you blacklist them in 8250_pci?



You are referring to add blacklist instead of remove F81504/508/512
code?


No.


  or add blacklist and remove code?


This one.


ok


Check what lspci tells you about your device. I'm pretty sure that it
has Serial Class, which would trigger enumeration in 8250_pci.c if it
comes first.



I had add log with 8250_pci.c. It really trigger once by 8250_pci.c,
but will failed with serial_pci_guess_board(). So It can be handled by
f81504-core.c. I should add pid/vid to blacklist and comments it'll
be handled by f81504-core.c

Thanks
--
With Best Regards,
Peter Hung

Re: [PATCH V2 4/4] serial: 8250_pci: Remove Fintek F81504/508/512 UART driver

2016-01-31 Thread Peter Hung 

Hi Andy,

Andy Shevchenko 於 2016/1/29 下午 08:40 寫道:

On Fri, 2016-01-29 at 16:20 +0800, Peter Hung wrote:

Hi Andy,

Andy Shevchenko 於 2016/1/28 下午 08:04 寫道:

On Thu, 2016-01-28 at 17:20 +0800, Peter Hung wrote:

-   /* Fintek PCI serial cards */
-   { PCI_DEVICE(0x1c29, 0x1104), .driver_data =
pbn_fintek_4 },
-   { PCI_DEVICE(0x1c29, 0x1108), .driver_data =
pbn_fintek_8 },
-   { PCI_DEVICE(0x1c29, 0x1112), .driver_data =
pbn_fintek_12
},


Shouldn't you blacklist them in 8250_pci?



You are referring to add blacklist instead of remove F81504/508/512
code?


No.


  or add blacklist and remove code?


This one.


ok


Check what lspci tells you about your device. I'm pretty sure that it
has Serial Class, which would trigger enumeration in 8250_pci.c if it
comes first.



I had add log with 8250_pci.c. It really trigger once by 8250_pci.c,
but will failed with serial_pci_guess_board(). So It can be handled by
f81504-core.c. I should add pid/vid to blacklist and comments it'll
be handled by f81504-core.c

Thanks
--
With Best Regards,
Peter Hung

Re: [PATCH V2 4/4] serial: 8250_pci: Remove Fintek F81504/508/512 UART driver

2016-01-29 Thread Andy Shevchenko 
On Fri, 2016-01-29 at 16:20 +0800, Peter Hung wrote:
> Hi Andy,
> 
> Andy Shevchenko 於 2016/1/28 下午 08:04 寫道:
> > On Thu, 2016-01-28 at 17:20 +0800, Peter Hung wrote:
> > > - /* Fintek PCI serial cards */
> > > - { PCI_DEVICE(0x1c29, 0x1104), .driver_data =
> > > pbn_fintek_4 },
> > > - { PCI_DEVICE(0x1c29, 0x1108), .driver_data =
> > > pbn_fintek_8 },
> > > - { PCI_DEVICE(0x1c29, 0x1112), .driver_data =
> > > pbn_fintek_12
> > > },
> > 
> > Shouldn't you blacklist them in 8250_pci?
> > 
> 
> You are referring to add blacklist instead of remove F81504/508/512
> code?

No.

>  or add blacklist and remove code?

This one.

Check what lspci tells you about your device. I'm pretty sure that it
has Serial Class, which would trigger enumeration in 8250_pci.c if it
comes first.

-- 
Andy Shevchenko 
Intel Finland Oy

Re: [PATCH V2 4/4] serial: 8250_pci: Remove Fintek F81504/508/512 UART driver

2016-01-29 Thread Peter Hung 

Hi Andy,

Andy Shevchenko 於 2016/1/28 下午 08:04 寫道:

On Thu, 2016-01-28 at 17:20 +0800, Peter Hung wrote:

-   /* Fintek PCI serial cards */
-   { PCI_DEVICE(0x1c29, 0x1104), .driver_data = pbn_fintek_4 },
-   { PCI_DEVICE(0x1c29, 0x1108), .driver_data = pbn_fintek_8 },
-   { PCI_DEVICE(0x1c29, 0x1112), .driver_data = pbn_fintek_12
},


Shouldn't you blacklist them in 8250_pci?



You are referring to add blacklist instead of remove F81504/508/512
code? or add blacklist and remove code?

--
With Best Regards,
Peter Hung

Re: [PATCH V2 4/4] serial: 8250_pci: Remove Fintek F81504/508/512 UART driver

2016-01-29 Thread Andy Shevchenko 
On Fri, 2016-01-29 at 16:20 +0800, Peter Hung wrote:
> Hi Andy,
> 
> Andy Shevchenko 於 2016/1/28 下午 08:04 寫道:
> > On Thu, 2016-01-28 at 17:20 +0800, Peter Hung wrote:
> > > - /* Fintek PCI serial cards */
> > > - { PCI_DEVICE(0x1c29, 0x1104), .driver_data =
> > > pbn_fintek_4 },
> > > - { PCI_DEVICE(0x1c29, 0x1108), .driver_data =
> > > pbn_fintek_8 },
> > > - { PCI_DEVICE(0x1c29, 0x1112), .driver_data =
> > > pbn_fintek_12
> > > },
> > 
> > Shouldn't you blacklist them in 8250_pci?
> > 
> 
> You are referring to add blacklist instead of remove F81504/508/512
> code?

No.

>  or add blacklist and remove code?

This one.

Check what lspci tells you about your device. I'm pretty sure that it
has Serial Class, which would trigger enumeration in 8250_pci.c if it
comes first.

-- 
Andy Shevchenko 
Intel Finland Oy

Re: [PATCH V2 4/4] serial: 8250_pci: Remove Fintek F81504/508/512 UART driver

2016-01-29 Thread Peter Hung 

Hi Andy,

Andy Shevchenko 於 2016/1/28 下午 08:04 寫道:

On Thu, 2016-01-28 at 17:20 +0800, Peter Hung wrote:

-   /* Fintek PCI serial cards */
-   { PCI_DEVICE(0x1c29, 0x1104), .driver_data = pbn_fintek_4 },
-   { PCI_DEVICE(0x1c29, 0x1108), .driver_data = pbn_fintek_8 },
-   { PCI_DEVICE(0x1c29, 0x1112), .driver_data = pbn_fintek_12
},


Shouldn't you blacklist them in 8250_pci?



You are referring to add blacklist instead of remove F81504/508/512
code? or add blacklist and remove code?

--
With Best Regards,
Peter Hung

Re: [PATCH V2 4/4] serial: 8250_pci: Remove Fintek F81504/508/512 UART driver

2016-01-28 Thread Andy Shevchenko 
On Thu, 2016-01-28 at 17:20 +0800, Peter Hung wrote:
> Remove Fintek F81504/508/512 PCIE-to-UART device driver from
> 8250_pci.c
> 
> Paul recommed us do less code deletion to avoid confusing problem
> when
> bisect.
> https://lkml.org/lkml/2016/1/18/646
> 
> 


> - /* Fintek PCI serial cards */
> - { PCI_DEVICE(0x1c29, 0x1104), .driver_data = pbn_fintek_4 },
> - { PCI_DEVICE(0x1c29, 0x1108), .driver_data = pbn_fintek_8 },
> - { PCI_DEVICE(0x1c29, 0x1112), .driver_data = pbn_fintek_12
> },

Shouldn't you blacklist them in 8250_pci?

-- 
Andy Shevchenko 
Intel Finland Oy

[PATCH V2 4/4] serial: 8250_pci: Remove Fintek F81504/508/512 UART driver

2016-01-28 Thread Peter Hung 
Remove Fintek F81504/508/512 PCIE-to-UART device driver from 8250_pci.c

Paul recommed us do less code deletion to avoid confusing problem when
bisect.
https://lkml.org/lkml/2016/1/18/646

But this patch is sent after with following patch.
8250: 8250_f81504: Add Fintek F81504/508/512 PCIE-to-UART/GPIO UART support
mfd: f81504-core: Add Fintek F81504/508/512 PCIE-to-UART/GPIO core support

We must remove F81504/508/512 support in 8250_pci.c and migrate to
f81504-core/8250_f81504 to enable MFD support.

Suggested-by: Paul Gortmaker 
Signed-off-by: Peter Hung 
---
 drivers/tty/serial/8250/8250_pci.c | 201 -
 1 file changed, 201 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c 
b/drivers/tty/serial/8250/8250_pci.c
index 4097f3f..0eeb4a3 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1527,156 +1527,6 @@ pci_brcm_trumanage_setup(struct serial_private *priv,
return ret;
 }
 
-/* RTS will control by MCR if this bit is 0 */
-#define FINTEK_RTS_CONTROL_BY_HW   BIT(4)
-/* only worked with FINTEK_RTS_CONTROL_BY_HW on */
-#define FINTEK_RTS_INVERT  BIT(5)
-
-/* 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)
-{
-   u8 setting;
-   u8 *index = (u8 *) port->private_data;
-   struct pci_dev *pci_dev = container_of(port->dev, struct pci_dev,
-   dev);
-
-   pci_read_config_byte(pci_dev, 0x40 + 8 * *index + 7, );
-
-   if (!rs485)
-   rs485 = >rs485;
-   else if (rs485->flags & SER_RS485_ENABLED)
-   memset(rs485->padding, 0, sizeof(rs485->padding));
-   else
-   memset(rs485, 0, sizeof(*rs485));
-
-   /* F81504/508/512 not support RTS delay before or after send */
-   rs485->flags &= SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND;
-
-   if (rs485->flags & SER_RS485_ENABLED) {
-   /* Enable RTS H/W control mode */
-   setting |= FINTEK_RTS_CONTROL_BY_HW;
-
-   if (rs485->flags & SER_RS485_RTS_ON_SEND) {
-   /* RTS driving high on TX */
-   setting &= ~FINTEK_RTS_INVERT;
-   } else {
-   /* RTS driving low on TX */
-   setting |= FINTEK_RTS_INVERT;
-   }
-
-   rs485->delay_rts_after_send = 0;
-   rs485->delay_rts_before_send = 0;
-   } else {
-   /* Disable RTS H/W control mode */
-   setting &= ~(FINTEK_RTS_CONTROL_BY_HW | FINTEK_RTS_INVERT);
-   }
-
-   pci_write_config_byte(pci_dev, 0x40 + 8 * *index + 7, setting);
-
-   if (rs485 != >rs485)
-   port->rs485 = *rs485;
-
-   return 0;
-}
-
-static int pci_fintek_setup(struct serial_private *priv,
-   const struct pciserial_board *board,
-   struct uart_8250_port *port, int idx)
-{
-   struct pci_dev *pdev = priv->dev;
-   u8 *data;
-   u8 config_base;
-   u16 iobase;
-
-   config_base = 0x40 + 0x08 * idx;
-
-   /* Get the io address from configuration space */
-   pci_read_config_word(pdev, config_base + 4, );
-
-   dev_dbg(>dev, "%s: idx=%d iobase=0x%x", __func__, idx, iobase);
-
-   port->port.iotype = UPIO_PORT;
-   port->port.iobase = iobase;
-   port->port.rs485_config = pci_fintek_rs485_config;
-
-   data = devm_kzalloc(>dev, sizeof(u8), GFP_KERNEL);
-   if (!data)
-   return -ENOMEM;
-
-   /* preserve index in PCI configuration space */
-   *data = idx;
-   port->port.private_data = data;
-
-   return 0;
-}
-
-static int pci_fintek_init(struct pci_dev *dev)
-{
-   unsigned long iobase;
-   u32 max_port, i;
-   u32 bar_data[3];
-   u8 config_base;
-   struct serial_private *priv = pci_get_drvdata(dev);
-   struct uart_8250_port *port;
-
-   switch (dev->device) {
-   case 0x1104: /* 4 ports */
-   case 0x1108: /* 8 ports */
-   max_port = dev->device & 0xff;
-   break;
-   case 0x1112: /* 12 ports */
-   max_port = 12;
-   break;
-   default:
-   return -EINVAL;
-   }
-
-   /* Get the io address dispatch from the BIOS */
-   pci_read_config_dword(dev, 0x24, _data[0]);
-   pci_read_config_dword(dev, 0x20, _data[1]);
-   pci_read_config_dword(dev, 0x1c, _data[2]);
-
-   for (i = 0; i < max_port; ++i) {
-   /* UART0 configuration offset start from 0x40 */
-   config_base = 0x40 + 0x08 * i;
-
-   /* Calculate Real IO Port */
-   iobase = (bar_data[i / 4] & 0xffe0) + (i % 4) * 8;
-
-   /* Enable UART I/O port */
-   pci_write_config_byte(dev, config_base +

[PATCH V2 4/4] serial: 8250_pci: Remove Fintek F81504/508/512 UART driver

2016-01-28 Thread Peter Hung 
Remove Fintek F81504/508/512 PCIE-to-UART device driver from 8250_pci.c

Paul recommed us do less code deletion to avoid confusing problem when
bisect.
https://lkml.org/lkml/2016/1/18/646

But this patch is sent after with following patch.
8250: 8250_f81504: Add Fintek F81504/508/512 PCIE-to-UART/GPIO UART support
mfd: f81504-core: Add Fintek F81504/508/512 PCIE-to-UART/GPIO core support

We must remove F81504/508/512 support in 8250_pci.c and migrate to
f81504-core/8250_f81504 to enable MFD support.

Suggested-by: Paul Gortmaker 
Signed-off-by: Peter Hung 
---
 drivers/tty/serial/8250/8250_pci.c | 201 -
 1 file changed, 201 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c 
b/drivers/tty/serial/8250/8250_pci.c
index 4097f3f..0eeb4a3 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1527,156 +1527,6 @@ pci_brcm_trumanage_setup(struct serial_private *priv,
return ret;
 }
 
-/* RTS will control by MCR if this bit is 0 */
-#define FINTEK_RTS_CONTROL_BY_HW   BIT(4)
-/* only worked with FINTEK_RTS_CONTROL_BY_HW on */
-#define FINTEK_RTS_INVERT  BIT(5)
-
-/* 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)
-{
-   u8 setting;
-   u8 *index = (u8 *) port->private_data;
-   struct pci_dev *pci_dev = container_of(port->dev, struct pci_dev,
-   dev);
-
-   pci_read_config_byte(pci_dev, 0x40 + 8 * *index + 7, );
-
-   if (!rs485)
-   rs485 = >rs485;
-   else if (rs485->flags & SER_RS485_ENABLED)
-   memset(rs485->padding, 0, sizeof(rs485->padding));
-   else
-   memset(rs485, 0, sizeof(*rs485));
-
-   /* F81504/508/512 not support RTS delay before or after send */
-   rs485->flags &= SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND;
-
-   if (rs485->flags & SER_RS485_ENABLED) {
-   /* Enable RTS H/W control mode */
-   setting |= FINTEK_RTS_CONTROL_BY_HW;
-
-   if (rs485->flags & SER_RS485_RTS_ON_SEND) {
-   /* RTS driving high on TX */
-   setting &= ~FINTEK_RTS_INVERT;
-   } else {
-   /* RTS driving low on TX */
-   setting |= FINTEK_RTS_INVERT;
-   }
-
-   rs485->delay_rts_after_send = 0;
-   rs485->delay_rts_before_send = 0;
-   } else {
-   /* Disable RTS H/W control mode */
-   setting &= ~(FINTEK_RTS_CONTROL_BY_HW | FINTEK_RTS_INVERT);
-   }
-
-   pci_write_config_byte(pci_dev, 0x40 + 8 * *index + 7, setting);
-
-   if (rs485 != >rs485)
-   port->rs485 = *rs485;
-
-   return 0;
-}
-
-static int pci_fintek_setup(struct serial_private *priv,
-   const struct pciserial_board *board,
-   struct uart_8250_port *port, int idx)
-{
-   struct pci_dev *pdev = priv->dev;
-   u8 *data;
-   u8 config_base;
-   u16 iobase;
-
-   config_base = 0x40 + 0x08 * idx;
-
-   /* Get the io address from configuration space */
-   pci_read_config_word(pdev, config_base + 4, );
-
-   dev_dbg(>dev, "%s: idx=%d iobase=0x%x", __func__, idx, iobase);
-
-   port->port.iotype = UPIO_PORT;
-   port->port.iobase = iobase;
-   port->port.rs485_config = pci_fintek_rs485_config;
-
-   data = devm_kzalloc(>dev, sizeof(u8), GFP_KERNEL);
-   if (!data)
-   return -ENOMEM;
-
-   /* preserve index in PCI configuration space */
-   *data = idx;
-   port->port.private_data = data;
-
-   return 0;
-}
-
-static int pci_fintek_init(struct pci_dev *dev)
-{
-   unsigned long iobase;
-   u32 max_port, i;
-   u32 bar_data[3];
-   u8 config_base;
-   struct serial_private *priv = pci_get_drvdata(dev);
-   struct uart_8250_port *port;
-
-   switch (dev->device) {
-   case 0x1104: /* 4 ports */
-   case 0x1108: /* 8 ports */
-   max_port = dev->device & 0xff;
-   break;
-   case 0x1112: /* 12 ports */
-   max_port = 12;
-   break;
-   default:
-   return -EINVAL;
-   }
-
-   /* Get the io address dispatch from the BIOS */
-   pci_read_config_dword(dev, 0x24, _data[0]);
-   pci_read_config_dword(dev, 0x20, _data[1]);
-   pci_read_config_dword(dev, 0x1c, _data[2]);
-
-   for (i = 0; i < max_port; ++i) {
-   /* UART0 configuration offset start from 0x40 */
-   config_base = 0x40 + 0x08 * i;
-
-   /* Calculate Real IO Port */
-   iobase = (bar_data[i / 4] & 0xffe0) + (i % 4) * 8;
-
-   /* Enable UART I/O port

Re: [PATCH V2 4/4] serial: 8250_pci: Remove Fintek F81504/508/512 UART driver

2016-01-28 Thread Andy Shevchenko 
On Thu, 2016-01-28 at 17:20 +0800, Peter Hung wrote:
> Remove Fintek F81504/508/512 PCIE-to-UART device driver from
> 8250_pci.c
> 
> Paul recommed us do less code deletion to avoid confusing problem
> when
> bisect.
> https://lkml.org/lkml/2016/1/18/646
> 
> 


> - /* Fintek PCI serial cards */
> - { PCI_DEVICE(0x1c29, 0x1104), .driver_data = pbn_fintek_4 },
> - { PCI_DEVICE(0x1c29, 0x1108), .driver_data = pbn_fintek_8 },
> - { PCI_DEVICE(0x1c29, 0x1112), .driver_data = pbn_fintek_12
> },

Shouldn't you blacklist them in 8250_pci?

-- 
Andy Shevchenko 
Intel Finland Oy