serial: exar: add Connect Tech serial cards to Exar driver
v3 cover serial: exar: add Connect Tech serial cards to Exar driver
From: Parker Newman <[email protected]>
Hello,
These patches add proper support for most of Connect Tech's (CTI) Exar based serial cards. Previously, only a subset of CTI's cards would work with the Exar driver while the rest required the CTI out-of-tree driver. These patches are intended to phase out the out-of-tree driver.
I am new to the mailing lists and contributing to the kernel so please let me know if I have made any mistakes or if you have any feedback.
Changes in v2:
- Put missing PCI IDs in 8250_exar.c instead of pci_ids.h
- Split large patch into smaller ones
Changes in v3:
- Refactored patches to be easier to follow (based on feedback of v2)
- Patch specific changes listed in corresponding patch
Thank you,
Parker Newman (8):
serial: exar: adding missing CTI and Exar PCI ids
serial: exar: remove old Connect Tech setup
serial: exar: added a exar_get_nr_ports function
serial: exar: add optional board_init function
serial: exar: moved generic_rs485 further up in 8250_exar.c
serial: exar: add CTI cards to exar_get_nr_ports
serial: exar: add CTI specific setup code
serial: exar: fix: fix crash during shutdown if setup fails
drivers/tty/serial/8250/8250_exar.c | 1079 +++++++++++++++++++++++++-- 1 file changed, 1013 insertions(+), 66 deletions(-)
base-commit: fec50db7033ea478773b159e0e2efb135270e3b7 --
2.43.2
PATCH v3 1/8 serial: exar: adding missing CTI and Exar PCI ids
From: Parker Newman <[email protected]>
- Added Connect Tech and Exar IDs not already in pci_ids.h
--- drivers/tty/serial/8250/8250_exar.c | 42 +++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
drivers/tty/serial/8250/8250_exar.c +42 -0
@@ -46,8 +46,50 @@#define PCI_DEVICE_ID_COMMTECH_4228PCIE 0x0021#define PCI_DEVICE_ID_COMMTECH_4222PCIE 0x0022 +#define PCI_VENDOR_ID_CONNECT_TECH 0x12c4 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_SP_OPTO 0x0340 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_SP_OPTO_A 0x0341 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_SP_OPTO_B 0x0342 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XPRS 0x0350 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_A 0x0351 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_B 0x0352 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS 0x0353 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_16_XPRS_A 0x0354 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_16_XPRS_B 0x0355 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XPRS_OPTO 0x0360 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_OPTO_A 0x0361 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_OPTO_B 0x0362 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP 0x0370 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP_232 0x0371 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP_485 0x0372 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_SP 0x0373 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_6_2_SP 0x0374 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_6_SP 0x0375 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP_232_NS 0x0376 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XP_OPTO_LEFT 0x0380 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XP_OPTO_RIGHT 0x0381 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XP_OPTO 0x0382 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_XPRS_OPTO 0x0392 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP 0x03A0 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP_232 0x03A1 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP_485 0x03A2 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP_232_NS 0x03A3 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCIE_XEG001 0x0602 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCIE_XR35X_BASE 0x1000 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCIE_XR35X_2 0x1002 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCIE_XR35X_4 0x1004 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCIE_XR35X_8 0x1008 +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCIE_XR35X_12 0x100C +#define PCI_SUBDEVICE_ID_CONNECT_TECH_PCIE_XR35X_16 0x1010 +#define PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG00X 0x110c +#define PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG01X 0x110d +#define PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_16 0x1110 +#define PCI_DEVICE_ID_EXAR_XR17V4358 0x4358#define PCI_DEVICE_ID_EXAR_XR17V8358 0x8358 +#define PCI_DEVICE_ID_EXAR_XR17V252 0x0252 +#define PCI_DEVICE_ID_EXAR_XR17V254 0x0254 +#define PCI_DEVICE_ID_EXAR_XR17V258 0x0258#define PCI_SUBDEVICE_ID_USR_2980 0x0128#define PCI_SUBDEVICE_ID_USR_2981 0x0129
--
2.43.2
Greg Kroah-Hartman
maintainerRe: [PATCH v3 1/8] serial: exar: adding missing CTI and Exar PCI ids
Quoted 8 lines · Parker Newman
From: Parker Newman <[email protected]>
- Added Connect Tech and Exar IDs not already in
pci_ids.hSigned-off-by Parker Newman <[email protected]>--- drivers/tty/serial/8250/8250_exar.c | 42 +++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
I took this one now as it's "obviously" correct :)
thanks,
greg k-h
PATCH v3 2/8 serial: exar: remove old Connect Tech setup
In reply to Parker NewmanFrom: Parker Newman <[email protected]>
Preparatory patch removing existing Connect Tech setup code and CONNECT_DEVICE macro.
Changes in v3:
- Split code removals to own patch
--- drivers/tty/serial/8250/8250_exar.c | 37 ----------------------------- 1 file changed, 37 deletions(-)
drivers/tty/serial/8250/8250_exar.c +0 -37
@@ -357,17 +357,6 @@ pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev,return 0 ; } -static int -pci_connect_tech_setup (struct exar8250 *priv,struct pci_dev *pcidev, -struct uart_8250_port *port,int idx) -{ -unsigned int offset = idx *0x200 ; -unsigned int baud =1843200 ; - - port->port.uartclk = baud *16 ; -return default_setup (priv, pcidev, idx, offset, port); -} -static int pci_xr17c154_setup (struct exar8250 *priv,struct pci_dev *pcidev,struct uart_8250_port *port,int idx)
@@ -848,10 +837,6 @@ static const struct exar8250_board pbn_fastcom335_8 = { .setup = pci_fastcom335_setup, }; -static const struct exar8250_board pbn_connect = { - .setup = pci_connect_tech_setup, -}; -static const struct exar8250_board pbn_exar_ibm_saturn = { .num_ports =1 , .setup = pci_xr17c154_setup,
@@ -896,15 +881,6 @@ static const struct exar8250_board pbn_exar_XR17V8358 = { .exit = pci_xr17v35x_exit, }; -#define CONNECT_DEVICE (devid, sdevid, bd) { \ - PCI_DEVICE_SUB( \ - PCI_VENDOR_ID_EXAR, \ - PCI_DEVICE_ID_EXAR_##devid, \ - PCI_SUBVENDOR_ID_CONNECT_TECH, \ - PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_##sdevid), 0, 0, \ - (kernel_ulong_t)&bd \ - } -#define EXAR_DEVICE (vend, devid, bd) { PCI_DEVICE_DATA(vend, devid, &bd) }#define IBM_DEVICE (devid, sdevid, bd) { \
@@ -934,19 +910,6 @@ static const struct pci_device_id exar_pci_tbl[] = {EXAR_DEVICE (ACCESSIO ,COM_4SM , pbn_exar_XR17C15x),EXAR_DEVICE (ACCESSIO ,COM_8SM , pbn_exar_XR17C15x), -CONNECT_DEVICE (XR17C152 ,UART_2_232 , pbn_connect), -CONNECT_DEVICE (XR17C154 ,UART_4_232 , pbn_connect), -CONNECT_DEVICE (XR17C158 ,UART_8_232 , pbn_connect), -CONNECT_DEVICE (XR17C152 ,UART_1_1 , pbn_connect), -CONNECT_DEVICE (XR17C154 ,UART_2_2 , pbn_connect), -CONNECT_DEVICE (XR17C158 ,UART_4_4 , pbn_connect), -CONNECT_DEVICE (XR17C152 ,UART_2 , pbn_connect), -CONNECT_DEVICE (XR17C154 ,UART_4 , pbn_connect), -CONNECT_DEVICE (XR17C158 ,UART_8 , pbn_connect), -CONNECT_DEVICE (XR17C152 ,UART_2_485 , pbn_connect), -CONNECT_DEVICE (XR17C154 ,UART_4_485 , pbn_connect), -CONNECT_DEVICE (XR17C158 ,UART_8_485 , pbn_connect), - IBM_DEVICE(XR17C152, SATURN_SERIAL_ONE_PORT, pbn_exar_ibm_saturn),/* USRobotics USR298x-OEM PCI Modems */
--
2.43.2
Greg Kroah-Hartman
maintainerRe: [PATCH v3 2/8] serial: exar: remove old Connect Tech setup
Quoted 4 lines · Parker Newman
From: Parker Newman <[email protected]>
Preparatory patch removing existing Connect Tech setup code and
CONNECT_DEVICEmacro.
Preparatory for what? You are removing support here, right? And will you be adding it back later?
thanks,
greg k-h
PATCH v3 3/8 serial: exar: added a exar_get_nr_ports function
In reply to Parker NewmanFrom: Parker Newman <[email protected]>
Moved code for getting number of ports from exar_pci_probe() to a separate exar_get_nr_ports() function.
Chnages in v3:
- Only moved existing code in this patch, will add CTI code in subsequent patch
--- drivers/tty/serial/8250/8250_exar.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)
drivers/tty/serial/8250/8250_exar.c +21 -6
@@ -703,6 +703,21 @@ static irqreturn_t exar_misc_handler(int irq, void *data)return IRQ_HANDLED ; } +static unsigned int exar_get_nr_ports (struct exar8250_board *board, +struct pci_dev *pcidev) +{ +unsigned int nr_ports =0 ; + +if (pcidev->vendor ==PCI_VENDOR_ID_ACCESSIO ) + nr_ports =BIT (((pcidev->device &0x38 ) >>3 ) -1 ); +else if (board->num_ports) + nr_ports = board->num_ports; +else + nr_ports = pcidev->device &0x0f ; + +return nr_ports; +} +static int exar_pci_probe (struct pci_dev *pcidev,const struct pci_device_id *ent) {
@@ -722,12 +737,12 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent) maxnr =pci_resource_len (pcidev, bar) >> (board->reg_shift +3 ); -if (pcidev->vendor ==PCI_VENDOR_ID_ACCESSIO ) - nr_ports =BIT (((pcidev->device &0x38 ) >>3 ) -1 ); -else if (board->num_ports) - nr_ports = board->num_ports; -else - nr_ports = pcidev->device &0x0f ; + nr_ports =exar_get_nr_ports (board, pcidev); +if (nr_ports ==0 ) { +dev_err_probe (&pcidev->dev, -ENODEV , +"failed to get number of ports\n" ); +return -ENODEV ; + } priv =devm_kzalloc (&pcidev->dev,struct_size (priv, line, nr_ports),GFP_KERNEL );if (!priv)
--
2.43.2
Greg Kroah-Hartman
maintainerRe: [PATCH v3 3/8] serial: exar: added a exar_get_nr_ports function
Quoted 8 lines · Parker Newman
From: Parker Newman <[email protected]>
Moved code for getting number of ports from
exar_pci_probe()to a separateexar_get_nr_ports()function.Chnages in v3:
- Only moved existing code in this patch, will add CTI code in subsequent patch
Nit, the "changes" need to go below the --- line, otherwise it shows up in the change logs.
So when you resend this, can you put these below the --- line please?
thanks,
greg k-h
Re: [PATCH v3 3/8] serial: exar: added a exar_get_nr_ports function
Quoted 6 lines · On Wed, 17 Apr 2024 13:15:41 +0200 Greg Kroah-Hartman…
Quoted 5 lines · Parker Newman
From: Parker Newman <[email protected]>
Moved code for getting number of ports from
exar_pci_probe()to a separateexar_get_nr_ports()function.
Quoted 9 lines
Quoted 3 lines
Chnages in v3:
- Only moved existing code in this patch, will add CTI code in subsequent patchNit, the "changes" need to go below the --- line, otherwise it shows up in the change logs.
So when you resend this, can you put these below the --- line please?
Yes sorry see that note online now. Will do in next version.
Thanks,
Parker
Quoted 3 lines
thanks,
greg k-h
PATCH v3 4/8 serial: exar: add optional board_init function
In reply to Parker NewmanFrom: Parker Newman <[email protected]>
- Add an optional "board_init()" function pointer to struct exar8250_board which is called once during probe prior to setting up the ports.
- Fix several "missing identifier name" warnings from checkpatch in struct exar8250_platform and struct exar8250_board:
WARNING: function definition argument <arg> should also have an identifier name
- Fix warning from checkpatch:
WARNING: please, no space before tabs
+ * 0^I^I2 ^IMode bit 0$
Changes in v3:
- Renamed board_setup to board_init.
- Changed pci_err to dev_err_probe
- Added note above about checkpatch fixes
--- drivers/tty/serial/8250/8250_exar.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
drivers/tty/serial/8250/8250_exar.c +17 -6
@@ -133,7 +133,7 @@* * MPIO Port Function * ---- ---- -------- -* 0 2 Mode bit 0 +* 0 2 Mode bit 0 * 1 2 Mode bit 1 * 2 2 Terminate bus * 3 - <reserved>
@@ -169,22 +169,24 @@ struct exar8250_platform {int (*rs485_config)(struct uart_port *port,struct ktermios *termios,struct serial_rs485 *rs485);const struct serial_rs485 *rs485_supported; -int (*register_gpio)(struct pci_dev *,struct uart_8250_port *); -void (*unregister_gpio)(struct uart_8250_port *); +int (*register_gpio)(struct pci_dev *pcidev,struct uart_8250_port *port); +void (*unregister_gpio)(struct uart_8250_port *port); };/** * struct exar8250_board - board information * @num_ports: number of serial ports * @reg_shift: describes UART register mapping in PCI memory -* @setup: quirk run at ->probe() stage +* @board_init: quirk run once at ->probe() stage before setting up ports +* @setup: quirk run at ->probe() stage for each port * @exit: quirk run at ->remove() stage */ struct exar8250_board {unsigned int num_ports;unsigned int reg_shift; -int (*setup)(struct exar8250 *,struct pci_dev *, -struct uart_8250_port *,int ); +int (*board_init)(struct exar8250 *priv); +int (*setup)(struct exar8250 *priv,struct pci_dev *pcidev, +struct uart_8250_port *port,int idx);void (*exit)(struct pci_dev *pcidev); };
@@ -772,6 +774,15 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)if (rc)return rc; +if (board->board_init) { + rc = board->board_init (priv); +if (rc) { +dev_err_probe (&pcidev->dev, rc, +"failed to init serial board\n" ); +return rc; + } + } +for (i =0 ; i < nr_ports && i < maxnr; i++) { rc = board->setup (priv, pcidev, &uart, i);if (rc) {
--
2.43.2
Greg Kroah-Hartman
maintainerRe: [PATCH v3 4/8] serial: exar: add optional board_init function
Quoted 31 lines · Parker Newman
From: Parker Newman <[email protected]>
- Add an optional "
board_init()" function pointer to structexar8250_boardwhich is called once during probe prior to setting up the ports.- Fix several "missing identifier name" warnings from checkpatch in struct
exar8250_platformand structexar8250_board:WARNING: function definition argument <arg> should also have an identifier name
- Fix warning from checkpatch:
WARNING: please, no space before tabs
+ * 0^I^I2 ^IMode bit 0$Changes in v3:
- Renamedboard_setuptoboard_init.
- Changedpci_errtodev_err_probe
- Added note above about checkpatch fixesSigned-off-by Parker Newman <[email protected]>--- drivers/tty/serial/8250/8250_exar.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)drivers/tty/serial/8250/8250_exar.c +0
-0
drivers/tty/serial/8250/8250_exar.c line 136
Quoted 5 lines
drivers/tty/serial/8250/8250_exar.c +1
-1@@ -133,7 +133,7 @@* * MPIO Port Function * ---- ---- -------- -* 0 2 Mode bit 0 +* 0 2 Mode bit 0
This change isn't related to the exar8250_board change, right?
Just keep exar8250_board change as one patch, and the coding style warning fixups as one for the very end, after you add all of your new functionality.
thanks,
greg k-h
PATCH v3 5/8 serial: exar: moved generic_rs485 further up in 8250_exar.c
In reply to Parker NewmanFrom: Parker Newman <[email protected]>
Preparatory patch moving generic_rs485_config and generic_rs485_supported higher in the file to allow for CTI setup functions to use it.
Changes in v3:
- split into separate preparatory patch
--- drivers/tty/serial/8250/8250_exar.c | 50 ++++++++++++++--------------- 1 file changed, 25 insertions(+), 25 deletions(-)
drivers/tty/serial/8250/8250_exar.c +25 -25
@@ -197,6 +197,31 @@ struct exar8250 {int line[]; }; +static int generic_rs485_config (struct uart_port *port,struct ktermios *termios, +struct serial_rs485 *rs485) +{ +bool is_rs485 = !!(rs485->flags &SER_RS485_ENABLED ); +u8 __iomem *p = port->membase; +u8 value; + + value =readb (p +UART_EXAR_FCTR ); +if (is_rs485) + value |=UART_FCTR_EXAR_485 ; +else + value &= ~UART_FCTR_EXAR_485 ; + +writeb (value, p +UART_EXAR_FCTR ); + +if (is_rs485) +writeb (UART_EXAR_RS485_DLY (4 ), p +UART_MSR ); + +return 0 ; +} + +static const struct serial_rs485 generic_rs485_supported = { + .flags =SER_RS485_ENABLED |SER_RS485_RTS_ON_SEND , +}; +static void exar_pm (struct uart_port *port,unsigned int state,unsigned int old) {/*
@@ -458,27 +483,6 @@ static void xr17v35x_unregister_gpio(struct uart_8250_port *port) port->port.private_data =NULL ; } -static int generic_rs485_config (struct uart_port *port,struct ktermios *termios, -struct serial_rs485 *rs485) -{ -bool is_rs485 = !!(rs485->flags &SER_RS485_ENABLED ); -u8 __iomem *p = port->membase; -u8 value; - - value =readb (p +UART_EXAR_FCTR ); -if (is_rs485) - value |=UART_FCTR_EXAR_485 ; -else - value &= ~UART_FCTR_EXAR_485 ; - -writeb (value, p +UART_EXAR_FCTR ); - -if (is_rs485) -writeb (UART_EXAR_RS485_DLY (4 ), p +UART_MSR ); - -return 0 ; -} -static int sealevel_rs485_config (struct uart_port *port,struct ktermios *termios,struct serial_rs485 *rs485) {
@@ -517,10 +521,6 @@ static int sealevel_rs485_config(struct uart_port *port, struct ktermios *termioreturn 0 ; } -static const struct serial_rs485 generic_rs485_supported = { - .flags =SER_RS485_ENABLED |SER_RS485_RTS_ON_SEND , -}; -static const struct exar8250_platform exar8250_default_platform = { .register_gpio = xr17v35x_register_gpio, .unregister_gpio = xr17v35x_unregister_gpio,
--
2.43.2
PATCH v3 6/8 serial: exar: add CTI cards to exar_get_nr_ports
In reply to Parker NewmanFrom: Parker Newman <[email protected]>
Add code for getting number of ports of CTI cards to exar_get_nr_ports()
Changes in v3:
- moved to separate patch
- added spaces to single line comments
--- drivers/tty/serial/8250/8250_exar.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
drivers/tty/serial/8250/8250_exar.c +19 -3
@@ -710,12 +710,28 @@ static unsigned int exar_get_nr_ports(struct exar8250_board *board, {unsigned int nr_ports =0 ; -if (pcidev->vendor ==PCI_VENDOR_ID_ACCESSIO ) +if (pcidev->vendor ==PCI_VENDOR_ID_ACCESSIO ) { nr_ports =BIT (((pcidev->device &0x38 ) >>3 ) -1 ); -else if (board->num_ports) + }else if (board->num_ports >0 ) { +// Check if board struct overrides number of ports nr_ports = board->num_ports; -else + }else if (pcidev->vendor ==PCI_VENDOR_ID_EXAR ) { +// Exar encodes # ports in last nibble of PCI Device ID ex. 0358 nr_ports = pcidev->device &0x0f ; + }else if (pcidev->vendor ==PCI_VENDOR_ID_CONNECT_TECH ) { +// Handle CTI FPGA cards +switch (pcidev->device) { +case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG00X : +case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG01X : + nr_ports =12 ; +break ; +case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_16 : + nr_ports =16 ; +break ; +default : +break ; + } + }return nr_ports; }
--
2.43.2
PATCH v3 7/8 serial: exar: add CTI specific setup code
In reply to Parker NewmanFrom: Parker Newman <[email protected]>
This is a large patch but is only additions. All changes and removals are made in previous patches in this series.
- Add CTI board_init and port setup functions for each UART type
- Add CTI_EXAR_DEVICE() and CTI_PCI_DEVICE() macros
- Add support for reading a word from the Exar EEPROM.
- Add support for configuring and setting a single MPIO
- Add various helper functions for CTI boards.
- Add osc_freq, pcidev, dev to struct exar8250
Changes in v3:
- Moved all base driver changes and refactoring to preparatory patches
- Switched any user space types to kernel types
- Switched all uses of pci_xxx print functions to dev_xxx
- Added struct device pointer in struct exar8250 to simplify above
- Switched osc_freq and port_flag parsing to use GENMASK() and FIELD_GET()/FIELD_PREP()
- Renamed board_setup function pointer to board_init
- Removed some unneeded checks for priv being NULL
- Added various convenience functions instead of relying on bools ex:
exar_mpio_set_low()/exar_mpio_set_high() instead of exar_mpio_set()
- Renamed some variables and defines for clarity
- Numerous minor formatting fixes
--- drivers/tty/serial/8250/8250_exar.c | 899 ++++++++++++++++++++++++++++ 1 file changed, 899 insertions(+)
drivers/tty/serial/8250/8250_exar.c +899 -0
@@ -20,6 +20,8 @@#include <linux/property.h> #include <linux/string.h> #include <linux/types.h> +#include <linux/string_choices.h> +#include <linux/bitfield.h> #include <linux/serial_8250.h> #include <linux/serial_core.h>
@@ -128,6 +130,19 @@#define UART_EXAR_DLD 0x02/* Divisor Fractional */ #define UART_EXAR_DLD_485_POLARITY 0x80/* RS-485 Enable Signal Polarity */ +/* EEPROM registers */ +#define UART_EXAR_REGB 0x8e +#define UART_EXAR_REGB_EECK BIT(4) +#define UART_EXAR_REGB_EECS BIT(5) +#define UART_EXAR_REGB_EEDI BIT(6) +#define UART_EXAR_REGB_EEDO BIT(7) +#define UART_EXAR_REGB_EE_ADDR_SIZE 6 +#define UART_EXAR_REGB_EE_DATA_SIZE 16 + +#define UART_EXAR_XR17C15X_PORT_OFFSET 0x200 +#define UART_EXAR_XR17V25X_PORT_OFFSET 0x200 +#define UART_EXAR_XR17V35X_PORT_OFFSET 0x400 +/* * IOT2040 MPIO wiring semantics: *
@@ -163,6 +178,52 @@#define IOT2040_UARTS_ENABLE 0x03#define IOT2040_UARTS_GPIO_HI_MODE 0xF8/* enable & LED as outputs */ +/* CTI EEPROM offsets */ +#define CTI_EE_OFF_XR17C15X_OSC_FREQ 0x04/* 2 words */ +#define CTI_EE_OFF_XR17V25X_OSC_FREQ 0x08/* 2 words */ +#define CTI_EE_OFF_XR17C15X_PART_NUM 0x0A/* 4 words */ +#define CTI_EE_OFF_XR17V25X_PART_NUM 0x0E/* 4 words */ +#define CTI_EE_OFF_XR17C15X_SERIAL_NUM 0x0E/* 1 word */ +#define CTI_EE_OFF_XR17V25X_SERIAL_NUM 0x12/* 1 word */ +#define CTI_EE_OFF_XR17V35X_SERIAL_NUM 0x11/* 2 word */ +#define CTI_EE_OFF_XR17V35X_BRD_FLAGS 0x13/* 1 word */ +#define CTI_EE_OFF_XR17V35X_PORT_FLAGS 0x14/* 1 word */ + +#define CTI_EE_MASK_PORT_FLAGS_TYPE GENMASK(7, 0) +#define CTI_EE_MASK_OSC_FREQ_LOWER GENMASK(15, 0) +#define CTI_EE_MASK_OSC_FREQ_UPPER GENMASK(31, 16) + +#define CTI_FPGA_RS485_IO_REG 0x2008 +#define CTI_FPGA_CFG_INT_EN_REG 0x48 +#define CTI_FPGA_CFG_INT_EN_EXT_BIT BIT(15)/* External int enable bit */ + +#define CTI_DEFAULT_PCI_OSC_FREQ 29491200 +#define CTI_DEFAULT_PCIE_OSC_FREQ 125000000 +#define CTI_DEFAULT_FPGA_OSC_FREQ 33333333 + +/* +* CTI Serial port line types. These match the values stored in the first +* nibble of the CTI EEPROM port_flags word. +*/ +enum cti_port_type { +CTI_PORT_TYPE_NONE =0 , +CTI_PORT_TYPE_RS232 ,// RS232 ONLY +CTI_PORT_TYPE_RS422_485 ,// RS422/RS485 ONLY +CTI_PORT_TYPE_RS232_422_485_HW ,// RS232/422/485 HW ONLY Switchable +CTI_PORT_TYPE_RS232_422_485_SW ,// RS232/422/485 SW ONLY Switchable +CTI_PORT_TYPE_RS232_422_485_4B ,// RS232/422/485 HW/SW (4bit ex. BCG004) +CTI_PORT_TYPE_RS232_422_485_2B ,// RS232/422/485 HW/SW (2bit ex. BBG008) +CTI_PORT_TYPE_MAX , +}; + +#define CTI_PORT_TYPE_VALID (_port_type) \ + (((_port_type) > CTI_PORT_TYPE_NONE) && \ + ((_port_type) < CTI_PORT_TYPE_MAX)) + +#define CTI_PORT_TYPE_RS485 (_port_type) \ + (((_port_type) > CTI_PORT_TYPE_RS232) && \ + ((_port_type) < CTI_PORT_TYPE_MAX)) +struct exar8250 ;struct exar8250_platform {
@@ -192,11 +253,214 @@ struct exar8250_board {struct exar8250 {unsigned int nr; +unsigned int osc_freq; +struct pci_dev *pcidev; +struct device *dev;struct exar8250_board *board;void __iomem *virt;int line[]; }; +static inline void exar_write_reg (struct exar8250 *priv, +unsigned int reg,u8 value) +{ +writeb (value, priv->virt + reg); +} + +static inline u8 exar_read_reg (struct exar8250 *priv,unsigned int reg) +{ +return readb (priv->virt + reg); +} + +static inline void exar_ee_select (struct exar8250 *priv) +{ +// Set chip select pin high to enable EEPROM reads/writes +exar_write_reg (priv,UART_EXAR_REGB ,UART_EXAR_REGB_EECS ); +// Min ~500ns delay needed between CS assert and EEPROM access +udelay (1 ); +} + +static inline void exar_ee_deselect (struct exar8250 *priv) +{ +exar_write_reg (priv,UART_EXAR_REGB ,0x00 ); +} + +static inline void exar_ee_write_bit (struct exar8250 *priv,int bit) +{ +u8 value =UART_EXAR_REGB_EECS ; + +if (bit) + value |=UART_EXAR_REGB_EEDI ; + +// Clock out the bit on the EEPROM interface +exar_write_reg (priv,UART_EXAR_REGB , value); +// 2us delay = ~500khz clock speed +udelay (2 ); + + value |=UART_EXAR_REGB_EECK ; + +exar_write_reg (priv,UART_EXAR_REGB , value); +udelay (2 ); +} + +static inline u8 exar_ee_read_bit (struct exar8250 *priv) +{ +u8 regb; +u8 value =UART_EXAR_REGB_EECS ; + +// Clock in the bit on the EEPROM interface +exar_write_reg (priv,UART_EXAR_REGB , value); +// 2us delay = ~500khz clock speed +udelay (2 ); + + value |=UART_EXAR_REGB_EECK ; + +exar_write_reg (priv,UART_EXAR_REGB , value); +udelay (2 ); + + regb =exar_read_reg (priv,UART_EXAR_REGB ); + +return (regb &UART_EXAR_REGB_EEDO ?1 :0 ); +} + +/** +* exar_ee_read() - Read a word from the EEPROM +* @priv: Device's private structure +* @ee_addr: Offset of EEPROM to read word from +* +* Read a single 16bit word from an Exar UART's EEPROM. +* +* Return: EEPROM word +*/ +static u16 exar_ee_read (struct exar8250 *priv,u8 ee_addr) +{ +int i; +u16 data =0 ; + +exar_ee_select (priv); + +// Send read command (opcode 110) +exar_ee_write_bit (priv,1 ); +exar_ee_write_bit (priv,1 ); +exar_ee_write_bit (priv,0 ); + +// Send address to read from +for (i =1 << (UART_EXAR_REGB_EE_ADDR_SIZE -1 ); i; i >>=1 ) +exar_ee_write_bit (priv, (ee_addr & i)); + +// Read data 1 bit at a time +for (i =0 ; i <=UART_EXAR_REGB_EE_DATA_SIZE ; i++) { + data <<=1 ; + data |=exar_ee_read_bit (priv); + } + +exar_ee_deselect (priv); + +return data; +} + +/** +* _exar_mpio_config() - Configure an Exar MPIO as input or output +* @priv: Device's private structure +* @mpio_num: MPIO number/offset to configure +* @output: Configure as output if true, inout if false +* +* Configure a single MPIO as an input or output and disable tristate. +* If configuring as output it is reccomended to set value with +* exar_mpio_set_high()/exar_mpio_set_low() prior to calling this function to +* ensure default MPIO pin state. +* +* Return: 0 on success, negative error code on failure +*/ +static int _exar_mpio_config (struct exar8250 *priv, +unsigned int mpio_num,bool output) +{ +unsigned int mpio_offset; +u8 sel_reg;// MPIO Select register (input/output) +u8 tri_reg;// MPIO Tristate register +u8 value; + +if (mpio_num <8 ) { + sel_reg =UART_EXAR_MPIOSEL_7_0 ; + tri_reg =UART_EXAR_MPIO3T_7_0 ; + mpio_offset = mpio_num; + }else if (mpio_num >=8 && mpio_num <16 ) { + sel_reg =UART_EXAR_MPIOSEL_15_8 ; + tri_reg =UART_EXAR_MPIO3T_15_8 ; + mpio_offset = mpio_num -8 ; + }else { +return -EINVAL ; + } + +// Disable MPIO pin tri-state + value =exar_read_reg (priv, tri_reg); + value &= ~BIT (mpio_offset); +exar_write_reg (priv, tri_reg, value); + + value =exar_read_reg (priv, sel_reg); +if (output) + value &= ~BIT (mpio_offset); +else + value |=BIT (mpio_offset); +exar_write_reg (priv, sel_reg, value); + +return 0 ; +} + +static int exar_mpio_config_output (struct exar8250 *priv, +unsigned int mpio_num) +{ +return _exar_mpio_config (priv, mpio_num, true); +} + +/** +* _exar_mpio_set() - Set an Exar MPIO output high or low +* @priv: Device's private structure +* @mpio_num: MPIO number/offset to set +* @high: Set MPIO high if true, low if false +* +* Set a single MPIO high or low. exar_mpio_config_output() must also be called +* to configure the pin as an output. +* +* Return: 0 on success, negative error code on failure +*/ +static int _exar_mpio_set (struct exar8250 *priv, +unsigned int mpio_num,bool high) +{ +unsigned int mpio_offset; +u8 lvl_reg; +u8 value; + +if (mpio_num <8 ) { + lvl_reg =UART_EXAR_MPIOLVL_7_0 ; + mpio_offset = mpio_num; + }else if (mpio_num >=8 && mpio_num <16 ) { + lvl_reg =UART_EXAR_MPIOLVL_15_8 ; + mpio_offset = mpio_num -8 ; + }else { +return -EINVAL ; + } + + value =exar_read_reg (priv, lvl_reg); +if (high) + value |=BIT (mpio_offset); +else + value &= ~BIT (mpio_offset); +exar_write_reg (priv, lvl_reg, value); + +return 0 ; +} + +static int exar_mpio_set_low (struct exar8250 *priv,unsigned int mpio_num) +{ +return _exar_mpio_set (priv, mpio_num, false); +} + +static int exar_mpio_set_high (struct exar8250 *priv,unsigned int mpio_num) +{ +return _exar_mpio_set (priv, mpio_num, true); +} +static int generic_rs485_config (struct uart_port *port,struct ktermios *termios,struct serial_rs485 *rs485) {
@@ -384,6 +648,582 @@ pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev,return 0 ; } +/** +* _cti_set_tristate() - Enable/Disable RS485 transciever tristate +* @priv: Device's private structure +* @port_num: Port number to set tristate on/off +* @enable: Enable tristate if true, disable if false +* +* Most RS485 capable cards have a power on tristate jumper/switch that ensures +* the RS422/RS485 transciever does not drive a multi-drop RS485 bus when it is +* not the master. When this jumper is installed the user must set the RS485 +* mode to disable tristate prior to using the port. +* +* Some Exar UARTs have an auto-tristate feature while others require setting +* an MPIO to disable the tristate. +* +* Return: 0 on success, negative error code on failure +*/ +static int _cti_set_tristate (struct exar8250 *priv, +unsigned int port_num,bool enable) +{ +int ret =0 ; + +if (port_num >= priv->nr) +return -EINVAL ; + +// Only Exar based cards use MPIO, return 0 otherwise +if (priv->pcidev->vendor !=PCI_VENDOR_ID_EXAR ) +return 0 ; + +dev_dbg (priv->dev,"%s tristate for port %u\n" , +str_enable_disable (enable), port_num); + +if (enable) + ret =exar_mpio_set_low (priv, port_num); +else + ret =exar_mpio_set_high (priv, port_num); +if (ret) +return ret; + +// Ensure MPIO is an output + ret =exar_mpio_config_output (priv, port_num); + +return ret; +} + +static int cti_tristate_disable (struct exar8250 *priv,unsigned int port_num) +{ +return _cti_set_tristate (priv, port_num, false); +} + +/** +* _cti_set_plx_int_enable() - Enable/Disable PCI interrupts +* @priv: Device's private structure +* @enable: Enable interrupts if true, disable if false +* +* Some older CTI cards require MPIO_0 to be set low to enable the PCI +* interupts from the UART to the PLX PCI->PCIe bridge. +* +* Return: 0 on success, negative error code on failure +*/ +static int _cti_set_plx_int_enable (struct exar8250 *priv,bool enable) +{ +int ret =0 ; + +// Only Exar based cards use MPIO, return 0 otherwise +if (priv->pcidev->vendor !=PCI_VENDOR_ID_EXAR ) +return 0 ; + +if (enable) + ret =exar_mpio_set_low (priv,0 ); +else + ret =exar_mpio_set_high (priv,0 ); +if (ret) +return ret; + +// Ensure MPIO is an output + ret =exar_mpio_config_output (priv,0 ); + +return ret; +} + +static int cti_plx_int_enable (struct exar8250 *priv) +{ +return _cti_set_plx_int_enable (priv, true); +} + +/** +* cti_read_osc_freq() - Read the UART oscillator frequency from EEPROM +* @priv: Device's private structure +* @eeprom_offset: Offset where the oscillator frequency is stored +* +* CTI XR17x15X and XR17V25X cards have the serial boards oscillator frequency +* stored in the EEPROM. FPGA and XR17V35X based cards use the PCI/PCIe clock. +* +* Return: frequency on success, negative error code on failure +*/ +static int cti_read_osc_freq (struct exar8250 *priv,u8 eeprom_offset) +{ +u16 lower_word; +u16 upper_word; +int osc_freq; + + lower_word =exar_ee_read (priv, eeprom_offset); +// Check if EEPROM word was blank +if (lower_word ==0xFFFF ) +return -EIO ; + + upper_word =exar_ee_read (priv, (eeprom_offset +1 )); +if (upper_word ==0xFFFF ) +return -EIO ; + + osc_freq =FIELD_PREP (CTI_EE_MASK_OSC_FREQ_LOWER , lower_word) | +FIELD_PREP (CTI_EE_MASK_OSC_FREQ_UPPER , upper_word); + +dev_dbg (priv->dev,"osc_freq from EEPROM %d\n" , osc_freq); + +return osc_freq; +} + +/** +* cti_get_port_type_xr17c15x_xr17v25x() - Get port type of xr17c15x/xr17v25x +* @priv: Device's private structure +* @port_num: Port to get type of +* +* CTI xr17c15x and xr17v25x based cards port types are based on PCI IDs. +* +* Return: port type on success, CTI_PORT_TYPE_NONE on failure +*/ +static enum cti_port_type cti_get_port_type_xr17c15x_xr17v25x (struct exar8250 *priv, +unsigned int port_num) +{ +enum cti_port_type port_type; + +switch (priv->pcidev->subsystem_device) { +// RS232 only cards +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_232 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_232 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_232 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP_232 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP_232_NS : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP_232 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP_232_NS : + port_type =CTI_PORT_TYPE_RS232 ; +break ; +// 1x RS232, 1x RS422/RS485 +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_1_1 : + port_type = (port_num ==0 ) ? +CTI_PORT_TYPE_RS232 :CTI_PORT_TYPE_RS422_485 ; +break ; +// 2x RS232, 2x RS422/RS485 +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_2 : + port_type = (port_num <2 ) ? +CTI_PORT_TYPE_RS232 :CTI_PORT_TYPE_RS422_485 ; +break ; +// 4x RS232, 4x RS422/RS485 +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_SP : + port_type = (port_num <4 ) ? +CTI_PORT_TYPE_RS232 :CTI_PORT_TYPE_RS422_485 ; +break ; +// RS232/RS422/RS485 HW (jumper) selectable +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_SP_OPTO : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_SP_OPTO_A : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_SP_OPTO_B : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XPRS : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_A : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_B : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_16_XPRS_A : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_16_XPRS_B : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XPRS_OPTO : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_OPTO_A : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_OPTO_B : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XP_OPTO_LEFT : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XP_OPTO_RIGHT : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XP_OPTO : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_XPRS_OPTO : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP : + port_type =CTI_PORT_TYPE_RS232_422_485_HW ; +break ; +// RS422/RS485 HW (jumper) selectable +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_485 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_485 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_485 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP_485 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP_485 : + port_type =CTI_PORT_TYPE_RS422_485 ; +break ; +// 6x RS232, 2x RS422/RS485 +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_6_2_SP : + port_type = (port_num <6 ) ? +CTI_PORT_TYPE_RS232 :CTI_PORT_TYPE_RS422_485 ; +break ; +// 2x RS232, 6x RS422/RS485 +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_6_SP : + port_type = (port_num <2 ) ? +CTI_PORT_TYPE_RS232 :CTI_PORT_TYPE_RS422_485 ; +break ; +default : +dev_err (priv->dev,"unknown/unsupported device\n" ); + port_type =CTI_PORT_TYPE_NONE ; + } + +return port_type; +} + +/** +* cti_get_port_type_fpga() - Get the port type of a CTI FPGA card +* @priv: Device's private structure +* @port_num: Port to get type of +* +* FPGA based cards port types are based on PCI IDs. +* +* Return: port type on success, CTI_PORT_TYPE_NONE on failure +*/ +static enum cti_port_type cti_get_port_type_fpga (struct exar8250 *priv, +unsigned int port_num) +{ +enum cti_port_type port_type; + +switch (priv->pcidev->device) { +case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG00X : +case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_12_XIG01X : +case PCI_DEVICE_ID_CONNECT_TECH_PCI_XR79X_16 : + port_type =CTI_PORT_TYPE_RS232_422_485_HW ; +break ; +default : +dev_err (priv->dev,"unknown/unsupported device\n" ); +return CTI_PORT_TYPE_NONE ; + } + +return port_type; +} + +/** +* cti_get_port_type_xr17v35x() - Read port type from the EEPROM +* @priv: Device's private structure +* @port_num: port offset +* +* CTI XR17V35X based cards have the port types stored in the EEPROM. +* This function reads the port type for a single port. +* +* Return: port type on success, CTI_PORT_TYPE_NONE on failure +*/ +static enum cti_port_type cti_get_port_type_xr17v35x (struct exar8250 *priv, +unsigned int port_num) +{ +enum cti_port_type port_type; +u16 port_flags; +u8 offset; + + offset =CTI_EE_OFF_XR17V35X_PORT_FLAGS + port_num; + port_flags =exar_ee_read (priv, offset); + + port_type =FIELD_GET (CTI_EE_MASK_PORT_FLAGS_TYPE , port_flags); +if (!CTI_PORT_TYPE_VALID (port_type)) { +/* +* If the port type is missing the card assume it is a +* RS232/RS422/RS485 card to be safe. +* +* There is one known board (BEG013) that only has +* 3 of 4 port types written to the EEPROM so this +* acts as a work around. +*/ +dev_warn (priv->dev, +"failed to get port %d type from EEPROM\n" , port_num); + port_type =CTI_PORT_TYPE_RS232_422_485_HW ; + } + +return port_type; +} + +static int cti_rs485_config_mpio_tristate (struct uart_port *port, +struct ktermios *termios, +struct serial_rs485 *rs485) +{ +struct exar8250 *priv = (struct exar8250 *)port->private_data; +int ret; + + ret =generic_rs485_config (port, termios, rs485); +if (ret) +return ret; + +// Disable power-on RS485 tri-state via MPIO +return cti_tristate_disable (priv, port->port_id); +} + +static int cti_port_setup_common (struct exar8250 *priv, +int idx,unsigned int offset, +struct uart_8250_port *port) +{ +int ret; + +if (priv->osc_freq ==0 ) +return -EINVAL ; + + port->port.port_id = idx; + port->port.uartclk = priv->osc_freq; + + ret =serial8250_pci_setup_port (priv->pcidev, port,0 , offset,0 ); +if (ret) { +dev_err (priv->dev, +"failed to setup pci for port %d err: %d\n" , idx, ret); +return ret; + } + + port->port.private_data = (void *)priv; + port->port.pm = exar_pm; + port->port.shutdown = exar_shutdown; + +return 0 ; +} + +static int cti_port_setup_fpga (struct exar8250 *priv, +struct pci_dev *pcidev, +struct uart_8250_port *port, +int idx) +{ +enum cti_port_type port_type; +unsigned int offset; + + port_type =cti_get_port_type_fpga (priv, idx); + +// FPGA shares port offests with XR17C15X + offset = idx *UART_EXAR_XR17C15X_PORT_OFFSET ; + port->port.type =PORT_XR17D15X ; + + port->port.get_divisor = xr17v35x_get_divisor; + port->port.set_divisor = xr17v35x_set_divisor; + port->port.startup = xr17v35x_startup; + +if (CTI_PORT_TYPE_RS485 (port_type)) { + port->port.rs485_config = generic_rs485_config; + port->port.rs485_supported = generic_rs485_supported; + } + +return cti_port_setup_common (priv, idx, offset, port); +} + +static int cti_port_setup_xr17v35x (struct exar8250 *priv, +struct pci_dev *pcidev, +struct uart_8250_port *port, +int idx) +{ +enum cti_port_type port_type; +unsigned int offset; +int ret; + + port_type =cti_get_port_type_xr17v35x (priv, idx); + + offset = idx *UART_EXAR_XR17V35X_PORT_OFFSET ; + port->port.type =PORT_XR17V35X ; + + port->port.get_divisor = xr17v35x_get_divisor; + port->port.set_divisor = xr17v35x_set_divisor; + port->port.startup = xr17v35x_startup; + +switch (port_type) { +case CTI_PORT_TYPE_RS422_485 : +case CTI_PORT_TYPE_RS232_422_485_HW : + port->port.rs485_config = cti_rs485_config_mpio_tristate; + port->port.rs485_supported = generic_rs485_supported; +break ; +case CTI_PORT_TYPE_RS232_422_485_SW : +case CTI_PORT_TYPE_RS232_422_485_4B : +case CTI_PORT_TYPE_RS232_422_485_2B : + port->port.rs485_config = generic_rs485_config; + port->port.rs485_supported = generic_rs485_supported; +break ; +default : +break ; + } + + ret =cti_port_setup_common (priv, idx, offset, port); +if (ret) +return ret; + +exar_write_reg (priv, (offset +UART_EXAR_8XMODE ),0x00 ); +exar_write_reg (priv, (offset +UART_EXAR_FCTR ),UART_FCTR_EXAR_TRGD ); +exar_write_reg (priv, (offset +UART_EXAR_TXTRG ),128 ); +exar_write_reg (priv, (offset +UART_EXAR_RXTRG ),128 ); + +return 0 ; +} + +static int cti_port_setup_xr17v25x (struct exar8250 *priv, +struct pci_dev *pcidev, +struct uart_8250_port *port, +int idx) +{ +enum cti_port_type port_type; +unsigned int offset; +int ret; + + port_type =cti_get_port_type_xr17c15x_xr17v25x (priv, idx); + + offset = idx *UART_EXAR_XR17V25X_PORT_OFFSET ; + port->port.type =PORT_XR17D15X ; + +// XR17V25X supports fractional baudrates + port->port.get_divisor = xr17v35x_get_divisor; + port->port.set_divisor = xr17v35x_set_divisor; + port->port.startup = xr17v35x_startup; + +if (CTI_PORT_TYPE_RS485 (port_type)) { +switch (priv->pcidev->subsystem_device) { +// These cards support power on 485 tri-state via MPIO +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP_485 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_SP : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_6_2_SP : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_6_SP : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XP_OPTO_LEFT : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XP_OPTO_RIGHT : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XP_OPTO : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_XPRS_OPTO : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP_485 : + port->port.rs485_config = cti_rs485_config_mpio_tristate; +break ; +// Otherwise auto or no power on 485 tri-state support +default : + port->port.rs485_config = generic_rs485_config; +break ; + } + + port->port.rs485_supported = generic_rs485_supported; + } + + ret =cti_port_setup_common (priv, idx, offset, port); +if (ret) +return ret; + +exar_write_reg (priv, (offset +UART_EXAR_8XMODE ),0x00 ); +exar_write_reg (priv, (offset +UART_EXAR_FCTR ),UART_FCTR_EXAR_TRGD ); +exar_write_reg (priv, (offset +UART_EXAR_TXTRG ),32 ); +exar_write_reg (priv, (offset +UART_EXAR_RXTRG ),32 ); + +return 0 ; +} + +static int cti_port_setup_xr17c15x (struct exar8250 *priv, +struct pci_dev *pcidev, +struct uart_8250_port *port, +int idx) +{ +enum cti_port_type port_type; +unsigned int offset; + + port_type =cti_get_port_type_xr17c15x_xr17v25x (priv, idx); + + offset = idx *UART_EXAR_XR17C15X_PORT_OFFSET ; + port->port.type =PORT_XR17D15X ; + +if (CTI_PORT_TYPE_RS485 (port_type)) { +switch (priv->pcidev->subsystem_device) { +// These cards support power on 485 tri-state via MPIO +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_SP_485 : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_SP : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_6_2_SP : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_6_SP : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XP_OPTO_LEFT : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XP_OPTO_RIGHT : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XP_OPTO : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4_XPRS_OPTO : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS_LP_485 : + port->port.rs485_config = cti_rs485_config_mpio_tristate; +break ; +// Otherwise auto or no power on 485 tri-state support +default : + port->port.rs485_config = generic_rs485_config; +break ; + } + + port->port.rs485_supported = generic_rs485_supported; + } + +return cti_port_setup_common (priv, idx, offset, port); +} + +static int cti_board_init_xr17v35x (struct exar8250 *priv) +{ +// XR17V35X uses the PCIe clock rather than an oscillator + priv->osc_freq =CTI_DEFAULT_PCIE_OSC_FREQ ; + +return 0 ; +} + +static int cti_board_init_xr17v25x (struct exar8250 *priv) +{ +int osc_freq; + + osc_freq =cti_read_osc_freq (priv,CTI_EE_OFF_XR17V25X_OSC_FREQ ); +if (osc_freq <0 ) { +dev_warn (priv->dev, +"failed to read osc freq from EEPROM, using default\n" ); + osc_freq =CTI_DEFAULT_PCI_OSC_FREQ ; + } + + priv->osc_freq = osc_freq; + +/* enable interupts on cards that need the "PLX fix" */ +switch (priv->pcidev->subsystem_device) { +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_XPRS : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_16_XPRS_A : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_16_XPRS_B : +cti_plx_int_enable (priv); +break ; +default : +break ; + } + +return 0 ; +} + +static int cti_board_init_xr17c15x (struct exar8250 *priv) +{ +int osc_freq; + + osc_freq =cti_read_osc_freq (priv,CTI_EE_OFF_XR17C15X_OSC_FREQ ); +if (osc_freq <=0 ) { +dev_warn (priv->dev, +"failed to read osc freq from EEPROM, using default\n" ); + osc_freq =CTI_DEFAULT_PCI_OSC_FREQ ; + } + + priv->osc_freq = osc_freq; + +/* enable interrupts on cards that need the "PLX fix" */ +switch (priv->pcidev->subsystem_device) { +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XPRS : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_A : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_B : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_XPRS_OPTO : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_OPTO_A : +case PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_XPRS_OPTO_B : +cti_plx_int_enable (priv); +break ; +default : +break ; + } + +return 0 ; +} + +static int cti_board_init_fpga (struct exar8250 *priv) +{ +int ret; +u16 cfg_val; + +// FPGA OSC is fixed to the 33MHz PCI clock + priv->osc_freq =CTI_DEFAULT_FPGA_OSC_FREQ ; + +// Enable external interrupts in special cfg space register + ret =pci_read_config_word (priv->pcidev, +CTI_FPGA_CFG_INT_EN_REG , &cfg_val); +if (ret) +return ret; + + cfg_val |=CTI_FPGA_CFG_INT_EN_EXT_BIT ; + ret =pci_write_config_word (priv->pcidev, +CTI_FPGA_CFG_INT_EN_REG , cfg_val); +if (ret) +return ret; + +// RS485 gate needs to be enabled; otherwise RTS/CTS will not work +exar_write_reg (priv,CTI_FPGA_RS485_IO_REG ,0x01 ); + +return 0 ; +} +static int pci_xr17c154_setup (struct exar8250 *priv,struct pci_dev *pcidev,struct uart_8250_port *port,int idx)
@@ -767,6 +1607,8 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)return -ENOMEM ; priv->board = board; + priv->pcidev = pcidev; + priv->dev = &pcidev->dev; priv->virt =pcim_iomap (pcidev, bar,0 );if (!priv->virt)return -ENOMEM ;
@@ -879,6 +1721,26 @@ static const struct exar8250_board pbn_fastcom335_8 = { .setup = pci_fastcom335_setup, }; +static const struct exar8250_board pbn_cti_xr17c15x = { + .board_init = cti_board_init_xr17c15x, + .setup = cti_port_setup_xr17c15x, +}; + +static const struct exar8250_board pbn_cti_xr17v25x = { + .board_init = cti_board_init_xr17v25x, + .setup = cti_port_setup_xr17v25x, +}; + +static const struct exar8250_board pbn_cti_xr17v35x = { + .board_init = cti_board_init_xr17v35x, + .setup = cti_port_setup_xr17v35x, +}; + +static const struct exar8250_board pbn_cti_fpga = { + .board_init = cti_board_init_fpga, + .setup = cti_port_setup_fpga, +}; +static const struct exar8250_board pbn_exar_ibm_saturn = { .num_ports =1 , .setup = pci_xr17c154_setup,
@@ -923,6 +1785,27 @@ static const struct exar8250_board pbn_exar_XR17V8358 = { .exit = pci_xr17v35x_exit, }; +// For Connect Tech cards with Exar vendor/device PCI IDs +#define CTI_EXAR_DEVICE (devid, bd) { \ + PCI_DEVICE_SUB( \ + PCI_VENDOR_ID_EXAR, \ + PCI_DEVICE_ID_EXAR_##devid, \ + PCI_SUBVENDOR_ID_CONNECT_TECH, \ + PCI_ANY_ID), 0, 0, \ + (kernel_ulong_t)&bd \ + } + +// For Connect Tech cards with Connect Tech vendor/device PCI IDs (FPGA based) +#define CTI_PCI_DEVICE (devid, bd) { \ + PCI_DEVICE_SUB( \ + PCI_VENDOR_ID_CONNECT_TECH, \ + PCI_DEVICE_ID_CONNECT_TECH_PCI_##devid, \ + PCI_ANY_ID, \ + PCI_ANY_ID), 0, 0, \ + (kernel_ulong_t)&bd \ + } + +#define EXAR_DEVICE (vend, devid, bd) { PCI_DEVICE_DATA(vend, devid, &bd) }#define IBM_DEVICE (devid, sdevid, bd) { \
@@ -952,6 +1835,22 @@ static const struct pci_device_id exar_pci_tbl[] = {EXAR_DEVICE (ACCESSIO ,COM_4SM , pbn_exar_XR17C15x),EXAR_DEVICE (ACCESSIO ,COM_8SM , pbn_exar_XR17C15x), +CTI_EXAR_DEVICE (XR17C152 , pbn_cti_xr17c15x), +CTI_EXAR_DEVICE (XR17C154 , pbn_cti_xr17c15x), +CTI_EXAR_DEVICE (XR17C158 , pbn_cti_xr17c15x), + +CTI_EXAR_DEVICE (XR17V252 , pbn_cti_xr17v25x), +CTI_EXAR_DEVICE (XR17V254 , pbn_cti_xr17v25x), +CTI_EXAR_DEVICE (XR17V258 , pbn_cti_xr17v25x), + +CTI_EXAR_DEVICE (XR17V352 , pbn_cti_xr17v35x), +CTI_EXAR_DEVICE (XR17V354 , pbn_cti_xr17v35x), +CTI_EXAR_DEVICE (XR17V358 , pbn_cti_xr17v35x), + +CTI_PCI_DEVICE (XR79X_12_XIG00X , pbn_cti_fpga), +CTI_PCI_DEVICE (XR79X_12_XIG01X , pbn_cti_fpga), +CTI_PCI_DEVICE (XR79X_16 , pbn_cti_fpga), +IBM_DEVICE (XR17C152 ,SATURN_SERIAL_ONE_PORT , pbn_exar_ibm_saturn), /* USRobotics USR298x-OEM PCI Modems */
--
2.43.2
Greg Kroah-Hartman
maintainerRe: [PATCH v3 7/8] serial: exar: add CTI specific setup code
drivers/tty/serial/8250/8250_exar.c struct exar8250_board { · line 258
Quoted 5 lines · Parker Newman
struct exar8250 {
drivers/tty/serial/8250/8250_exar.c +3
-0unsigned int nr; +unsigned int osc_freq; +struct pci_dev *pcidev; +struct device *dev;
Why do you need both a pci_dev and a device? Aren't they the same thing here?
Quoted 17 lines
drivers/tty/serial/8250/8250_exar.c +17
-0+/** +* _cti_set_tristate() - Enable/Disable RS485 transciever tristate +* @priv: Device's private structure +* @port_num: Port number to set tristate on/off +* @enable: Enable tristate if true, disable if false +* +* Most RS485 capable cards have a power on tristate jumper/switch that ensures +* the RS422/RS485 transciever does not drive a multi-drop RS485 bus when it is +* not the master. When this jumper is installed the user must set the RS485 +* mode to disable tristate prior to using the port. +* +* Some Exar UARTs have an auto-tristate feature while others require setting +* an MPIO to disable the tristate. +* +* Return: 0 on success, negative error code on failure +*/ +static int _cti_set_tristate(struct exar8250 *priv,
drivers/tty/serial/8250/8250_exar.c pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev, · line 677
Quoted 10 lines
drivers/tty/serial/8250/8250_exar.c +10
-0+ unsigned int port_num, bool enable) +{ + int ret = 0; + +if (port_num >= priv->nr) +return -EINVAL ; + +// Only Exar based cards use MPIO, return 0 otherwise +if (priv->pcidev->vendor !=PCI_VENDOR_ID_EXAR ) +return 0 ;
How can this ever happen? Only the exar devices will call this function, or am I missing a path here?
Quoted 11 lines
drivers/tty/serial/8250/8250_exar.c +11
-0+ +dev_dbg (priv->dev,"%s tristate for port %u\n" , +str_enable_disable (enable), port_num); + +if (enable) + ret =exar_mpio_set_low (priv, port_num); +else + ret =exar_mpio_set_high (priv, port_num); +if (ret) +return ret; +
drivers/tty/serial/8250/8250_exar.c pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev, · line 698
Quoted 10 lines
drivers/tty/serial/8250/8250_exar.c +10
-0+// Ensure MPIO is an output + ret =exar_mpio_config_output (priv, port_num); + +return ret; +} + +static int cti_tristate_disable (struct exar8250 *priv,unsigned int port_num) +{ +return _cti_set_tristate (priv, port_num, false); +}
Do you ever call _cti_set_tristate() with "true"?
drivers/tty/serial/8250/8250_exar.c pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev, · line 703
Quoted 5 lines
drivers/tty/serial/8250/8250_exar.c +5
-0+ +/** +* _cti_set_plx_int_enable() - Enable/Disable PCI interrupts +* @priv: Device's private structure +* @enable: Enable interrupts if true, disable if false
But false is never used here, so why have this at all?
drivers/tty/serial/8250/8250_exar.c pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev, · line 716
Quoted 13 lines
+ *
+ * Some older CTI cards requireMPIO_0to be set low to enable the PCI
+ * interupts from the UART to the PLXPCI->PCIebridge.
+ *
+ * Return: 0 on success, negative error code on failure
+ */drivers/tty/serial/8250/8250_exar.c +7
-0+static int _cti_set_plx_int_enable (struct exar8250 *priv,bool enable) +{ +int ret =0 ; + +// Only Exar based cards use MPIO, return 0 otherwise +if (priv->pcidev->vendor !=PCI_VENDOR_ID_EXAR ) +return 0 ;
Same question here.
Quoted 7 lines
drivers/tty/serial/8250/8250_exar.c +7
-0+ +if (enable) + ret =exar_mpio_set_low (priv,0 ); +else + ret =exar_mpio_set_high (priv,0 ); +if (ret) +return ret;
drivers/tty/serial/8250/8250_exar.c pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev, · line 733
Quoted 10 lines
drivers/tty/serial/8250/8250_exar.c +10
-0+ +// Ensure MPIO is an output + ret =exar_mpio_config_output (priv,0 ); + +return ret; +} + +static int cti_plx_int_enable (struct exar8250 *priv) +{ +return _cti_set_plx_int_enable (priv, true);
Again, no wrapper needed if you never actually call that function with "false", right? Or am I missing a path here?
thanks,
greg k-h
Re: [PATCH v3 7/8] serial: exar: add CTI specific setup code
Quoted 10 lines · On Wed, 17 Apr 2024 13:24:20 +0200 Greg Kroah-Hartman…
drivers/tty/serial/8250/8250_exar.c struct exar8250_board { · line 258
Quoted 5 lines · Parker Newman
struct exar8250 {
drivers/tty/serial/8250/8250_exar.c +3
-0unsigned int nr; +unsigned int osc_freq; +struct pci_dev *pcidev; +struct device *dev;Why do you need both a
pci_devand a device? Aren't they the same thing here?
I added dev to make the prints cleaner. I personally prefer:
dev_err(priv->dev, ...); to dev_err(&priv->pcidev->dev, ...);
or to adding a:
struct device *dev = &priv->pcidev->dev;
to every function just for printing.
However, I do understand your point. I can drop dev if you prefer.
Quoted 22 lines
Quoted 22 lines
drivers/tty/serial/8250/8250_exar.c +22
-0+/** +* _cti_set_tristate() - Enable/Disable RS485 transciever tristate +* @priv: Device's private structure +* @port_num: Port number to set tristate on/off +* @enable: Enable tristate if true, disable if false +* +* Most RS485 capable cards have a power on tristate jumper/switch that ensures +* the RS422/RS485 transciever does not drive a multi-drop RS485 bus when it is +* not the master. When this jumper is installed the user must set the RS485 +* mode to disable tristate prior to using the port. +* +* Some Exar UARTs have an auto-tristate feature while others require setting +* an MPIO to disable the tristate. +* +* Return: 0 on success, negative error code on failure +*/ +static int _cti_set_tristate (struct exar8250 *priv, +unsigned int port_num,bool enable) +{ +int ret =0 ; + +if (port_num >= priv->nr)
Quoted 9 lines
drivers/tty/serial/8250/8250_exar.c pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev, · line 677
Quoted 5 lines
drivers/tty/serial/8250/8250_exar.c +5
-0+return -EINVAL ; + +// Only Exar based cards use MPIO, return 0 otherwise +if (priv->pcidev->vendor !=PCI_VENDOR_ID_EXAR ) +return 0 ;How can this ever happen? Only the exar devices will call this function, or am I missing a path here?
Yes that can go now, it used to be needed but not now. I will remove.
Quoted 16 lines
Quoted 16 lines
drivers/tty/serial/8250/8250_exar.c +15
-0+ +dev_dbg (priv->dev,"%s tristate for port %u\n" , +str_enable_disable (enable), port_num); + +if (enable) + ret =exar_mpio_set_low (priv, port_num); +else + ret =exar_mpio_set_high (priv, port_num); +if (ret) +return ret; + +// Ensure MPIO is an output + ret =exar_mpio_config_output (priv, port_num); + +return ret;
Quoted 9 lines
drivers/tty/serial/8250/8250_exar.c pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev, · line 698
Quoted 6 lines
drivers/tty/serial/8250/8250_exar.c +6
-0+} + +static int cti_tristate_disable (struct exar8250 *priv,unsigned int port_num) +{ +return _cti_set_tristate (priv, port_num, false); +}Do you ever call
_cti_set_tristate()with "true"?
Currently no. However, re-enabling tristate via a custom ioctl was a feature in our old out-of-tree driver (which was created prior to linux RS485 support).
I am not sure how it would be activated now, but I left enabling tristate as an option in to make it easier down the line when we need it.
I can add a note to the patch or remove it if you would prefer.
Quoted 8 lines
drivers/tty/serial/8250/8250_exar.c pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev, · line 703
Quoted 5 lines
drivers/tty/serial/8250/8250_exar.c +5
-0+ +/** +* _cti_set_plx_int_enable() - Enable/Disable PCI interrupts +* @priv: Device's private structure +* @enable: Enable interrupts if true, disable if false But false is never used here, so why have this at all?
Quoted 16 lines
drivers/tty/serial/8250/8250_exar.c pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev, · line 716
Quoted 13 lines
+ *
+ * Some older CTI cards requireMPIO_0to be set low to enable the PCI
+ * interupts from the UART to the PLXPCI->PCIebridge.
+ *
+ * Return: 0 on success, negative error code on failure
+ */drivers/tty/serial/8250/8250_exar.c +7
-0+static int _cti_set_plx_int_enable (struct exar8250 *priv,bool enable) +{ +int ret =0 ; + +// Only Exar based cards use MPIO, return 0 otherwise +if (priv->pcidev->vendor !=PCI_VENDOR_ID_EXAR ) +return 0 ;Same question here.
Same as above, not needed. I will remove.
Quoted 12 lines
Quoted 12 lines
drivers/tty/serial/8250/8250_exar.c +12
-0+ +if (enable) + ret =exar_mpio_set_low (priv,0 ); +else + ret =exar_mpio_set_high (priv,0 ); +if (ret) +return ret; + +// Ensure MPIO is an output + ret =exar_mpio_config_output (priv,0 ); + +return ret;
Quoted 9 lines
drivers/tty/serial/8250/8250_exar.c pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev, · line 733
Quoted 5 lines
drivers/tty/serial/8250/8250_exar.c +5
-0+} + +static int cti_plx_int_enable (struct exar8250 *priv) +{ +return _cti_set_plx_int_enable (priv, true);Again, no wrapper needed if you never actually call that function with "false", right? Or am I missing a path here?
This one is similar to _cti_set_tristate() but is less likely to be used.
Thanks again,
Parker
Quoted 3 lines
thanks,
greg k-h
Greg Kroah-Hartman
maintainerRe: [PATCH v3 7/8] serial: exar: add CTI specific setup code
Quoted 20 lines · Parker Newman
Quoted 10 lines · On Wed, 17 Apr 2024 13:24:20 +0200 Greg Kroah-Hartman…
drivers/tty/serial/8250/8250_exar.c struct exar8250_board { · line 258
Quoted 5 lines · Parker Newman
struct exar8250 {
drivers/tty/serial/8250/8250_exar.c +3
-0unsigned int nr; +unsigned int osc_freq; +struct pci_dev *pcidev; +struct device *dev;Why do you need both a
pci_devand a device? Aren't they the same thing here?I added dev to make the prints cleaner. I personally prefer:
dev_err(priv->dev, ...); to dev_err(&priv->pcidev->dev, ...);
Quoted 6 lines
or to adding a:
struct device *dev = &priv->pcidev->dev;to every function just for printing.
However, I do understand your point. I can drop dev if you prefer.
Pick one and stick with it, no need for both. I too like your first option, and if you never need the pci device pointer, just stick with "struct dev". Note, properly reference count it in case you think it could go away from underneath you (some paths it could, so be careful.)
Quoted 8 lines
Quoted 7 lines
drivers/tty/serial/8250/8250_exar.c pci_fastcom335_setup(struct exar8250 *priv, struct pci_dev *pcidev, · line 698
Quoted 4 lines
drivers/tty/serial/8250/8250_exar.c +4
-0+static int cti_tristate_disable (struct exar8250 *priv,unsigned int port_num) +{ +return _cti_set_tristate (priv, port_num, false); +}Do you ever call
_cti_set_tristate()with "true"?
Quoted 7 lines
Currently no. However, re-enabling tristate via a custom ioctl was a feature in our old out-of-tree driver (which was created prior to linux RS485 support).
I am not sure how it would be activated now, but I left enabling tristate as an option in to make it easier down the line when we need it.
I can add a note to the patch or remove it if you would prefer.
Just remove it for now, no need to ever add code you "might need in the future", just add that then, when you need it. Code for what you need to do now, that makes things simpler and removes extra stuff that you would have to force others to review :)
thanks,
greg k-h
PATCH v3 8/8 serial: exar: fix: fix crash during shutdown if setup fails
In reply to Parker NewmanFrom: Parker Newman <[email protected]>
If a port fails to register with serial8250_register_8250_port() the kernel can crash when shutting down or module removal.
This is because "priv->line[i]" will be set to a negative error code and in the exar_pci_remove() function serial8250_unregister_port() is called without checking if the "priv->line[i]" value is valid.
--- drivers/tty/serial/8250/8250_exar.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/tty/serial/8250/8250_exar.c +2 -1
@@ -1671,7 +1671,8 @@ static void exar_pci_remove(struct pci_dev *pcidev)unsigned int i;for (i =0 ; i < priv->nr; i++) -serial8250_unregister_port (priv->line[i]); +if (priv->line[i] >=0 ) +serial8250_unregister_port (priv->line[i]);/* Ensure that every init quirk is properly torn down */ if (priv->board->exit)
--
2.43.2
Greg Kroah-Hartman
maintainerRe: [PATCH v3 8/8] serial: exar: fix: fix crash during shutdown if setup fails
Quoted 19 lines · Parker Newman
From: Parker Newman <[email protected]>
If a port fails to register with
serial8250_register_8250_port()the kernel can crash when shutting down or module removal.This is because "priv->line[i]" will be set to a negative error code and in the
exar_pci_remove()functionserial8250_unregister_port()is called without checking if the "priv->line[i]" value is valid.Signed-off-by Parker Newman <[email protected]>--- drivers/tty/serial/8250/8250_exar.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/tty/serial/8250/8250_exar.c +0
-0
drivers/tty/serial/8250/8250_exar.c static void exar_pci_remove(struct pci_dev *pcidev) · line 1675
Quoted 6 lines
drivers/tty/serial/8250/8250_exar.c +2
-1@@ -1671,7 +1671,8 @@ static void exar_pci_remove(struct pci_dev *pcidev)unsigned int i;for (i =0 ; i < priv->nr; i++) -serial8250_unregister_port (priv->line[i]); +if (priv->line[i] >=0 ) +serial8250_unregister_port (priv->line[i]);
Is this a bug in the current driver? If so, can you resend it on its own so we can get it merged now?
thanks,
greg k-h
Re: [PATCH v3 8/8] serial: exar: fix: fix crash during shutdown if setup fails
Quoted 22 lines · On Wed, 17 Apr 2024 13:19:07 +0200 Greg Kroah-Hartman…
Quoted 21 lines · Parker Newman
From: Parker Newman <[email protected]>
If a port fails to register with
serial8250_register_8250_port()the kernel can crash when shutting down or module removal.This is because "priv->line[i]" will be set to a negative error code and in the
exar_pci_remove()functionserial8250_unregister_port()is called without checking if the "priv->line[i]" value is valid.Signed-off-by Parker Newman <[email protected]>--- drivers/tty/serial/8250/8250_exar.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/tty/serial/8250/8250_exar.c +0
-0@@ -1671,7 +1671,8 @@ static void exar_pci_remove(struct pci_dev *pcidev)unsigned int i;
Quoted 8 lines
drivers/tty/serial/8250/8250_exar.c static void exar_pci_remove(struct pci_dev *pcidev) · line 1675
Quoted 4 lines
drivers/tty/serial/8250/8250_exar.c +2
-1for (i =0 ; i < priv->nr; i++) -serial8250_unregister_port (priv->line[i]); +if (priv->line[i] >=0 ) +serial8250_unregister_port (priv->line[i]);Is this a bug in the current driver? If so, can you resend it on its own so we can get it merged now?
Yes it is, I can split this one out and send it on its own. Thanks,
Parker
Quoted 3 lines
thanks,
greg k-h
Greg Kroah-Hartman
maintainerRe: [PATCH v3 8/8] serial: exar: fix: fix crash during shutdown if setup fails
Quoted 34 lines · Parker Newman
Quoted 22 lines · On Wed, 17 Apr 2024 13:19:07 +0200 Greg Kroah-Hartman…
Quoted 21 lines · Parker Newman
From: Parker Newman <[email protected]>
If a port fails to register with
serial8250_register_8250_port()the kernel can crash when shutting down or module removal.This is because "priv->line[i]" will be set to a negative error code and in the
exar_pci_remove()functionserial8250_unregister_port()is called without checking if the "priv->line[i]" value is valid.Signed-off-by Parker Newman <[email protected]>--- drivers/tty/serial/8250/8250_exar.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/tty/serial/8250/8250_exar.c +0
-0@@ -1671,7 +1671,8 @@ static void exar_pci_remove(struct pci_dev *pcidev)unsigned int i;Quoted 8 lines
drivers/tty/serial/8250/8250_exar.c static void exar_pci_remove(struct pci_dev *pcidev) · line 1675
Quoted 4 lines
drivers/tty/serial/8250/8250_exar.c +2
-1for (i =0 ; i < priv->nr; i++) -serial8250_unregister_port (priv->line[i]); +if (priv->line[i] >=0 ) +serial8250_unregister_port (priv->line[i]);Is this a bug in the current driver? If so, can you resend it on its own so we can get it merged now?
Quoted 1 line
Yes it is, I can split this one out and send it on its own.
Great! Bonus points if you can find the commit id it fixes and add a "Fixes:" tag to the signed-off-by area. If not, I can guess :)
thanks,
greg k-h
Re: [PATCH v3 8/8] serial: exar: fix: fix crash during shutdown if setup fails
Quoted 35 lines · On Wed, 17 Apr 2024 15:30:56 +0200 Greg Kroah-Hartman…
Quoted 34 lines · Parker Newman
Quoted 30 lines · On Wed, 17 Apr 2024 13:19:07 +0200 Greg Kroah-Hartman…
Quoted 19 lines · Parker Newman
From: Parker Newman <[email protected]>
If a port fails to register with
serial8250_register_8250_port()the kernel can crash when shutting down or module removal.This is because "priv->line[i]" will be set to a negative error code and in the
exar_pci_remove()functionserial8250_unregister_port()is called without checking if the "priv->line[i]" value is valid.Signed-off-by Parker Newman <[email protected]>--- drivers/tty/serial/8250/8250_exar.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/tty/serial/8250/8250_exar.c +0
-0drivers/tty/serial/8250/8250_exar.c static void exar_pci_remove(struct pci_dev *pcidev) · line 1675
Quoted 6 lines
drivers/tty/serial/8250/8250_exar.c +2
-1@@ -1671,7 +1671,8 @@ static void exar_pci_remove(struct pci_dev *pcidev)unsigned int i;for (i =0 ; i < priv->nr; i++) -serial8250_unregister_port (priv->line[i]); +if (priv->line[i] >=0 ) +serial8250_unregister_port (priv->line[i]);Is this a bug in the current driver? If so, can you resend it on its own so we can get it merged now?
Quoted 8 lines
Quoted 1 line
Yes it is, I can split this one out and send it on its own.
Great! Bonus points if you can find the commit id it fixes and add a "Fixes:" tag to the signed-off-by area. If not, I can guess :)
thanks,
greg k-h
After looking at this again and doing some testing this bug does not actually happen with the driver in its current state. During my development I had it happen but that would have been due to me messing around.
When "priv->line[i]" < 0 it breaks out of the for loop and priv->nr is set to "i". so only the successfully registered ports will be unregistered in exar_pci_remove().
...
for (i = 0; i < nr_ports && i < maxnr; i++) {
rc = board->setup(priv, pcidev, &uart, i);
if (rc) {
dev_err(&pcidev->dev, "Failed to setup port %u\n", i);
break;
}
dev_dbg(&pcidev->dev, "Setup PCI port: port %lx, irq %d, type %d\n",
uart.port.iobase, uart.port.irq, uart.port.iotype);
priv->line[i] = serial8250_register_8250_port(&uart);
if (priv->line[i] < 0) {
dev_err(&pcidev->dev,
"Couldn't register serial port %lx, irq %d, type %d, error %d\n",
uart.port.iobase, uart.port.irq,
uart.port.iotype, priv->line[i]);
break;
}
}
priv->nr = i;...
Thanks,
Parker
Greg Kroah-Hartman
maintainerQuoted 10 lines · Parker Newman
From: Parker Newman <[email protected]>
Hello,
These patches add proper support for most of Connect Tech's (CTI) Exar based serial cards. Previously, only a subset of CTI's cards would work with the Exar driver while the rest required the CTI out-of-tree driver. These patches are intended to phase out the out-of-tree driver.I am new to the mailing lists and contributing to the kernel so please let me know if I have made any mistakes or if you have any feedback.
Much better. I took the 1st patch already in my tree to make it hopefully easire for you to rebase and redo the rest.
thanks,
greg k-h
Quoted 12 lines · On Wed, 17 Apr 2024 13:24:49 +0200 Greg Kroah-Hartman…
Quoted 10 lines · Parker Newman
From: Parker Newman <[email protected]>
Hello,
These patches add proper support for most of Connect Tech's (CTI) Exar based serial cards. Previously, only a subset of CTI's cards would work with the Exar driver while the rest required the CTI out-of-tree driver. These patches are intended to phase out the out-of-tree driver.I am new to the mailing lists and contributing to the kernel so please let me know if I have made any mistakes or if you have any feedback.
Quoted 6 lines
Much better. I took the 1st patch already in my tree to make it hopefully easire for you to rebase and redo the rest.
thanks,
greg k-h
I will resend with the updates.
I have been using the "main" branch of gregkh/tty.git so far. Is that correct? Or should I be using "tty-testing"?
Thanks again for bearing with me while I figure this out :). Parker
Greg Kroah-Hartman
maintainerQuoted 18 lines · Parker Newman
Quoted 15 lines · On Wed, 17 Apr 2024 13:24:49 +0200 Greg Kroah-Hartman…
Quoted 10 lines · Parker Newman
From: Parker Newman <[email protected]>
Hello,
These patches add proper support for most of Connect Tech's (CTI) Exar based serial cards. Previously, only a subset of CTI's cards would work with the Exar driver while the rest required the CTI out-of-tree driver. These patches are intended to phase out the out-of-tree driver.I am new to the mailing lists and contributing to the kernel so please let me know if I have made any mistakes or if you have any feedback.
Much better. I took the 1st patch already in my tree to make it hopefully easire for you to rebase and redo the rest.
Quoted 7 lines
Quoted 3 lines
thanks,
greg k-h
I will resend with the updates.
I have been using the "main" branch of gregkh/tty.git so far. Is that correct? Or should I be using "tty-testing"?
tty-testing is where things go before they are considered "good enough" to get into "tty-next" which is never rebased, and is what will be sent to Linus for the "next" kernel release (not this one.)
Sometimes it is rebased if I mess things up, but usually it's pretty stable. Patches only sit in there for 24 hours usually before ending up in "tty-next"
"main" just tracks Linus's branch, don't use it, I just need a branch to diff against.
"tty-linus" is what will be sent to Linus for "this" release (i.e. bug fixes only.)
Or you can just work off of the linux-next tree, which merges in both of my tty-next and tty-linus branches together (and all other maintainer trees/branches), every day, and is usually a good base for what will be the "next" release after this one.
hope this helps,
greg k-h