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

From: Jan Kiszka
Date: Wed Feb 08 2017 - 02:02:17 EST


On 2017-02-08 00:23, Andy Shevchenko wrote:
> 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?

Yeah, will reorder at this chance.

>
>> +#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?

Forgotten, will remove.

>
>> - /*
>> - * 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.
>

Yeah, it lacks PCI_VENDOR_ID_COMMTECH... will send an update.

Jan

--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux