Re: [PATCH 4/7] serial: exar: Move Commtech adapters to 8250_exar as well

[Andy Shevchenko]

On Tue, Feb 7, 2017 at 6:10 PM, Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote:
> Those are exar-based, too.

Exar-based

> With the required refactoring of the code to fit into 8250_exar, we
> automatically fix the same issue pci_xr17v35x_setup had before: 8XMODE,
> FCTL, TXTRG and RXTRG were always only set for port 0. Now they are
> initialized for the correct target port by using port.membase.

My comments below.

> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -24,11 +24,15 @@
>
>  #include "8250.h"
>
> -#define PCI_DEVICE_ID_COMMTECH_4224PCIE        0x0020
> -#define PCI_DEVICE_ID_COMMTECH_4228PCIE        0x0021
> -#define PCI_DEVICE_ID_COMMTECH_4222PCIE        0x0022
> -#define PCI_DEVICE_ID_EXAR_XR17V4358   0x4358
> -#define PCI_DEVICE_ID_EXAR_XR17V8358   0x8358

> +#define PCI_DEVICE_ID_COMMTECH_4222PCI335      0x0004
> +#define PCI_DEVICE_ID_COMMTECH_4224PCI335      0x0002

I think you assumed ID ordered list?

> +#define PCI_DEVICE_ID_COMMTECH_2324PCI335      0x000a
> +#define PCI_DEVICE_ID_COMMTECH_2328PCI335      0x000b
> +#define PCI_DEVICE_ID_COMMTECH_4224PCIE                0x0020
> +#define PCI_DEVICE_ID_COMMTECH_4228PCIE                0x0021
> +#define PCI_DEVICE_ID_COMMTECH_4222PCIE                0x0022
> +#define PCI_DEVICE_ID_EXAR_XR17V4358           0x4358
> +#define PCI_DEVICE_ID_EXAR_XR17V8358           0x8358

> @@ -292,6 +345,21 @@ static int __maybe_unused exar_resume(struct device *dev)

> +static const struct exar8250_board pbn_fastcom335_2 = {
> +       .num_ports      = 2,
> +       .setup          = pci_fastcom335_setup,
> +};
> +
> +static const struct exar8250_board pbn_fastcom335_4 = {
> +       .num_ports      = 4,
> +       .setup          = pci_fastcom335_setup,
> +};
> +
> +static const struct exar8250_board pbn_fastcom335_8 = {
> +       .num_ports      = 8,
> +       .setup          = pci_fastcom335_setup,
> +};

> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1622,54 +1622,6 @@ static int pci_eg20t_init(struct pci_dev *dev)
>  #define UART_EXAR_MPIOINV_15_8 0x98    /* MPIOINV[15:8] */
>  #define UART_EXAR_MPIOSEL_15_8 0x99    /* MPIOSEL[15:8] */
>  #define UART_EXAR_MPIOOD_15_8  0x9a    /* MPIOOD[15:8] */

And why not to remove above constants?

> -       /*
> -        * Commtech, Inc. Fastcom adapters
> -        */
> -       {       PCI_VENDOR_ID_COMMTECH, PCI_DEVICE_ID_COMMTECH_4222PCI335,
> -               PCI_ANY_ID, PCI_ANY_ID,
> -               0,
> -               0, pbn_b0_2_1152000_200 },
> -       {       PCI_VENDOR_ID_COMMTECH, PCI_DEVICE_ID_COMMTECH_4224PCI335,
> -               PCI_ANY_ID, PCI_ANY_ID,
> -               0,
> -               0, pbn_b0_4_1152000_200 },
> -       {       PCI_VENDOR_ID_COMMTECH, PCI_DEVICE_ID_COMMTECH_2324PCI335,
> -               PCI_ANY_ID, PCI_ANY_ID,
> -               0,
> -               0, pbn_b0_4_1152000_200 },
> -       {       PCI_VENDOR_ID_COMMTECH, PCI_DEVICE_ID_COMMTECH_2328PCI335,
> -               PCI_ANY_ID, PCI_ANY_ID,
> -               0,
> -               0, pbn_b0_8_1152000_200 },
> -

Check black list as well. I suppose now there is a bug and it's left
even after this patch.

-- 
With Best Regards,
Andy Shevchenko