On Thu, Mar 31, 2022 at 08:11:42AM +0100, Maciej W. Rozycki wrote:
> The EndRun PTP/1588 dual serial port device is based on the Oxford
> Semiconductor OXPCIe952 UART device with the PCI vendor:device ID set
> for EndRun Technologies and uses the same sequence to determine the
> number of ports available. Despite that we have duplicate code
> specific to the EndRun device.
>
> Remove redundant code then and factor out OxSemi Tornado device
> detection. Also correct the baud base like with commit 6cbe45d8ac93
> ("serial: 8250: Correct the clock for OxSemi PCIe devices") for the
> value of 3906250 rather than 4000000, obtained by dividing the 62.5MHz
> clock input by the default oversampling rate of 16. Finally move the
> EndRun vendor:device ID to <linux/pci_ids.h>.
>
> Signed-off-by: Maciej W. Rozycki <macro@xxxxxxxxxxx>
> Fixes: 1bc8cde46a159 ("8250_pci: Added driver for Endrun Technologies PTP PCIe card.")
> ---
> New change in v3.
> ---
> drivers/tty/serial/8250/8250_pci.c | 79 +++++++++++--------------------------
> include/linux/pci_ids.h | 3 +
> 2 files changed, 28 insertions(+), 54 deletions(-)
>
> linux-serial-8250-oxsemi-endrun.diff
> Index: linux-macro/drivers/tty/serial/8250/8250_pci.c
> ===================================================================
> --- linux-macro.orig/drivers/tty/serial/8250/8250_pci.c
> +++ linux-macro/drivers/tty/serial/8250/8250_pci.c
> @@ -994,41 +994,26 @@ static void pci_ite887x_exit(struct pci_
> }
>
> /*
> - * EndRun Technologies.
> - * Determine the number of ports available on the device.
> + * Oxford Semiconductor Inc.
> + * Check if an OxSemi device is part of the Tornado range of devices.
> */
> -#define PCI_VENDOR_ID_ENDRUN 0x7401
> -#define PCI_DEVICE_ID_ENDRUN_1588 0xe100
> -
> -static int pci_endrun_init(struct pci_dev *dev)
> +static bool pci_oxsemi_tornado_p(struct pci_dev *dev)
> {
> - u8 __iomem *p;
> - unsigned long deviceID;
> - unsigned int number_uarts = 0;
> + /* OxSemi Tornado devices are all 0xCxxx */
> + if (dev->vendor == PCI_VENDOR_ID_OXSEMI &&
> + (dev->device & 0xf000) != 0xc000)
> + return false;
>
> - /* EndRun device is all 0xexxx */
> + /* EndRun devices are all 0xExxx */
> if (dev->vendor == PCI_VENDOR_ID_ENDRUN &&
> - (dev->device & 0xf000) != 0xe000)
> - return 0;
> -
> - p = pci_iomap(dev, 0, 5);
> - if (p == NULL)
> - return -ENOMEM;
> + (dev->device & 0xf000) != 0xe000)
> + return false;
>
> - deviceID = ioread32(p);
> - /* EndRun device */
> - if (deviceID == 0x07000200) {
> - number_uarts = ioread8(p + 4);
> - pci_dbg(dev, "%d ports detected on EndRun PCI Express device\n", number_uarts);
> - }
> - pci_iounmap(dev, p);
> - return number_uarts;
> + return true;
> }
>
> /*
> - * Oxford Semiconductor Inc.
> - * Check that device is part of the Tornado range of devices, then determine
> - * the number of ports available on the device.
> + * Determine the number of ports available on a Tornado device.
> */
> static int pci_oxsemi_tornado_init(struct pci_dev *dev)
> {
> @@ -1036,9 +1021,7 @@ static int pci_oxsemi_tornado_init(struc
> unsigned long deviceID;
> unsigned int number_uarts = 0;
>
> - /* OxSemi Tornado devices are all 0xCxxx */
> - if (dev->vendor == PCI_VENDOR_ID_OXSEMI &&
> - (dev->device & 0xF000) != 0xC000)
> + if (!pci_oxsemi_tornado_p(dev))
> return 0;
>
> p = pci_iomap(dev, 0, 5);
> @@ -1049,7 +1032,10 @@ static int pci_oxsemi_tornado_init(struc
> /* Tornado device */
> if (deviceID == 0x07000200) {
> number_uarts = ioread8(p + 4);
> - pci_dbg(dev, "%d ports detected on Oxford PCI Express device\n", number_uarts);
> + pci_dbg(dev, "%d ports detected on %s PCI Express device\n",
> + number_uarts,
> + dev->vendor == PCI_VENDOR_ID_ENDRUN ?
> + "EndRun" : "Oxford");
> }
> pci_iounmap(dev, p);
> return number_uarts;
> @@ -2244,7 +2230,7 @@ static struct pci_serial_quirk pci_seria
> .device = PCI_ANY_ID,
> .subvendor = PCI_ANY_ID,
> .subdevice = PCI_ANY_ID,
> - .init = pci_endrun_init,
> + .init = pci_oxsemi_tornado_init,
> .setup = pci_default_setup,
> },
> /*
> @@ -2667,7 +2653,6 @@ enum pci_board_num_t {
> pbn_panacom2,
> pbn_panacom4,
> pbn_plx_romulus,
> - pbn_endrun_2_4000000,
> pbn_oxsemi,
> pbn_oxsemi_1_3906250,
> pbn_oxsemi_2_3906250,
> @@ -3190,20 +3175,6 @@ static struct pciserial_board pci_boards
> },
>
> /*
> - * EndRun Technologies
> - * Uses the size of PCI Base region 0 to
> - * signal now many ports are available
> - * 2 port 952 Uart support
> - */
> - [pbn_endrun_2_4000000] = {
> - .flags = FL_BASE0,
> - .num_ports = 2,
> - .base_baud = 4000000,
> - .uart_offset = 0x200,
> - .first_offset = 0x1000,
> - },
> -
> - /*
> * This board uses the size of PCI Base region 0 to
> * signal now many ports are available
> */
> @@ -4123,13 +4094,6 @@ static const struct pci_device_id serial
> 0x10b5, 0x106a, 0, 0,
> pbn_plx_romulus },
> /*
> - * EndRun Technologies. PCI express device range.
> - * EndRun PTP/1588 has 2 Native UARTs.
> - */
> - { PCI_VENDOR_ID_ENDRUN, PCI_DEVICE_ID_ENDRUN_1588,
> - PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> - pbn_endrun_2_4000000 },
> - /*
> * Quatech cards. These actually have configurable clocks but for
> * now we just use the default.
> *
> @@ -4390,6 +4354,13 @@ static const struct pci_device_id serial
> { PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_2_OX_IBM,
> PCI_SUBVENDOR_ID_IBM, PCI_ANY_ID, 0, 0,
> pbn_oxsemi_2_3906250 },
> + /*
> + * EndRun Technologies. PCI express device range.
> + * EndRun PTP/1588 has 2 Native UARTs utilizing OxSemi 952.
> + */
> + { PCI_VENDOR_ID_ENDRUN, PCI_DEVICE_ID_ENDRUN_1588,
> + PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> + pbn_oxsemi_2_3906250 },
>
> /*
> * SBS Technologies, Inc. P-Octal and PMC-OCTPRO cards,
> Index: linux-macro/include/linux/pci_ids.h
> ===================================================================
> --- linux-macro.orig/include/linux/pci_ids.h
> +++ linux-macro/include/linux/pci_ids.h
> @@ -2622,6 +2622,9 @@
> #define PCI_DEVICE_ID_DCI_PCCOM8 0x0002
> #define PCI_DEVICE_ID_DCI_PCCOM2 0x0004
>
> +#define PCI_VENDOR_ID_ENDRUN 0x7401
> +#define PCI_DEVICE_ID_ENDRUN_1588 0xe100
As per the top of this file, this should not be needed here as you are
only using it in one file. Please leave it as-is.
thanks,
greg k-h
