[PATCH V2 1/2] serial: exar: Revert "serial: exar: Add support for Sealevel 7xxxC serial cards"

[PATCH V2 1/2] serial: exar: Revert "serial: exar: Add support for Sealevel 7xxxC serial cards"

[Matthew Howell]

From: Matthew Howell <matthew.howell@sealevel.com>

Hardware ID of Sealevel 7xxxC cards changed prior to release, rendering 
the previous commit redundant.

Revert "serial: exar: Add support for Sealevel 7xxxC serial cards"

This reverts commit 14ee78d5932afeb710c8305196a676a715bfdea8.

Link: 
https://lore.kernel.org/linux-serial/687157c-3d18-e18b-1936-f3f5b98ae760@sealevel.com/

Fixes: 14ee78d5932a ("Add support for Sealevel 7xxxC serial cards")
Signed-off-by: Matthew Howell <matthew.howell@sealevel.com>
---
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 077c3ba3539e..3886f78ecbbf 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -47,12 +47,6 @@
 #define PCI_SUBDEVICE_ID_USR_2980		0x0128
 #define PCI_SUBDEVICE_ID_USR_2981		0x0129
 
-#define PCI_DEVICE_ID_SEALEVEL_710xC		0x1001
-#define PCI_DEVICE_ID_SEALEVEL_720xC		0x1002
-#define PCI_DEVICE_ID_SEALEVEL_740xC		0x1004
-#define PCI_DEVICE_ID_SEALEVEL_780xC		0x1008
-#define PCI_DEVICE_ID_SEALEVEL_716xC		0x1010
-
 #define UART_EXAR_INT0		0x80
 #define UART_EXAR_8XMODE	0x88	/* 8X sampling rate select */
 #define UART_EXAR_SLEEP		0x8b	/* Sleep mode */
@@ -652,8 +646,6 @@ exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
 		nr_ports = BIT(((pcidev->device & 0x38) >> 3) - 1);
 	else if (board->num_ports)
 		nr_ports = board->num_ports;
-	else if (pcidev->vendor == PCI_VENDOR_ID_SEALEVEL)
-		nr_ports = pcidev->device & 0xff;
 	else
 		nr_ports = pcidev->device & 0x0f;
 
@@ -893,12 +885,6 @@ static const struct pci_device_id exar_pci_tbl[] = {
 	EXAR_DEVICE(COMMTECH, 4224PCI335, pbn_fastcom335_4),
 	EXAR_DEVICE(COMMTECH, 2324PCI335, pbn_fastcom335_4),
 	EXAR_DEVICE(COMMTECH, 2328PCI335, pbn_fastcom335_8),
-
-	EXAR_DEVICE(SEALEVEL, 710xC, pbn_exar_XR17V35x),
-	EXAR_DEVICE(SEALEVEL, 720xC, pbn_exar_XR17V35x),
-	EXAR_DEVICE(SEALEVEL, 740xC, pbn_exar_XR17V35x),
-	EXAR_DEVICE(SEALEVEL, 780xC, pbn_exar_XR17V35x),
-	EXAR_DEVICE(SEALEVEL, 716xC, pbn_exar_XR17V35x),
 	{ 0, }
 };
 MODULE_DEVICE_TABLE(pci, exar_pci_tbl);

[PATCH V2 2/2] serial: exar: Add RS-485 support for Sealevel XR17V35X based cards

[Matthew Howell]

From: Matthew Howell <matthew.howell@sealevel.com>

Sealevel XR1735X based cards utilize DTR to control RS-485 Enable, but the 
current implementation of 8250_exar uses RTS for the auto-RS485-Enable 
mode of the XR17V35X UARTs. This patch applies a new sealevel_rs485_config 
function to configure the XR17V35X of Sealevel cards for DTR control of 
RS485 Enable.

Based on feedback from the first submission I replaced the hex values with 
defines and fixed up various format issues. I couldn't find an existing 
define for the DLD register or its RS485 Polarity bit so I created a new 
define. I tried to follow the format of the other defines in serial_reg.h.

Signed-off-by: Matthew Howell <matthew.howell@sealevel.com>
---
diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
index 3886f78ecbbf..2fd2bbf9364f 100644
--- a/drivers/tty/serial/8250/8250_exar.c
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -439,6 +439,35 @@ static int generic_rs485_config(struct uart_port *port, struct ktermios *termios
 	return 0;
 }
 
+static int sealevel_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 old_lcr;
+
+	generic_rs485_config(port, termios, rs485);
+
+	if (is_rs485) {
+		// Set EFR[4]=1 to enable enhanced feature registers
+		writeb(readb(p + UART_XR_EFR) | UART_EFR_ECB, p + UART_XR_EFR);
+
+		// Set MCR to use DTR as Auto-RS485 Enable signal
+		writeb(UART_MCR_OUT1, p + UART_MCR);
+
+		// Store original LCR and set LCR[7]=1 to enable access to DLD register
+		old_lcr = readb(p + UART_LCR);
+		writeb(old_lcr | UART_LCR_DLAB, p + UART_LCR);
+
+		// Set DLD[7]=1 for inverted RS485 Enable logic
+		writeb(readb(p + UART_DLD) | UART_DLD_485_POLARITY, p + UART_DLD);
+
+		writeb(old_lcr, p + UART_LCR);
+    }
+
+	return 0;
+ }
+
 static const struct serial_rs485 generic_rs485_supported = {
 	.flags = SER_RS485_ENABLED,
 };
@@ -744,6 +773,19 @@ static int __maybe_unused exar_resume(struct device *dev)
 	return 0;
 }
 
+static int pci_sealevel_setup(struct exar8250 *priv, struct pci_dev *pcidev,
+							  struct uart_8250_port *port, int idx)
+{
+	int ret = pci_xr17v35x_setup(priv, pcidev, port, idx);
+
+	if (ret)
+		return ret;
+
+	port->port.rs485_config = sealevel_rs485_config;
+
+	return ret;
+}
+
 static SIMPLE_DEV_PM_OPS(exar_pci_pm, exar_suspend, exar_resume);
 
 static const struct exar8250_board pbn_fastcom335_2 = {
@@ -809,6 +851,17 @@ static const struct exar8250_board pbn_exar_XR17V8358 = {
 	.exit		= pci_xr17v35x_exit,
 };
 
+static const struct exar8250_board pbn_sealevel = {
+	.setup		= pci_sealevel_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+static const struct exar8250_board pbn_sealevel_16 = {
+	.num_ports  = 16,
+    .setup		= pci_sealevel_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
 #define CONNECT_DEVICE(devid, sdevid, bd) {				\
 	PCI_DEVICE_SUB(							\
 		PCI_VENDOR_ID_EXAR,					\
@@ -838,6 +891,15 @@ static const struct exar8250_board pbn_exar_XR17V8358 = {
 		(kernel_ulong_t)&bd			\
 	}
 
+#define SEALEVEL_DEVICE(devid, bd) {			\
+	PCI_DEVICE_SUB(					\
+		PCI_VENDOR_ID_EXAR,			\
+		PCI_DEVICE_ID_EXAR_##devid,		\
+		PCI_VENDOR_ID_SEALEVEL,			\
+		PCI_ANY_ID), 0, 0,	\
+		(kernel_ulong_t)&bd			\
+	}
+
 static const struct pci_device_id exar_pci_tbl[] = {
 	EXAR_DEVICE(ACCESSIO, COM_2S, pbn_exar_XR17C15x),
 	EXAR_DEVICE(ACCESSIO, COM_4S, pbn_exar_XR17C15x),
@@ -860,6 +922,12 @@ static const struct pci_device_id exar_pci_tbl[] = {
 	CONNECT_DEVICE(XR17C154, UART_4_485, pbn_connect),
 	CONNECT_DEVICE(XR17C158, UART_8_485, pbn_connect),
 
+	SEALEVEL_DEVICE(XR17V352, pbn_sealevel),
+	SEALEVEL_DEVICE(XR17V354, pbn_sealevel),
+	SEALEVEL_DEVICE(XR17V358, pbn_sealevel),
+	SEALEVEL_DEVICE(XR17V4358, pbn_sealevel_16),
+	SEALEVEL_DEVICE(XR17V8358, pbn_sealevel_16),
+
 	IBM_DEVICE(XR17C152, SATURN_SERIAL_ONE_PORT, pbn_exar_ibm_saturn),
 
 	/* USRobotics USR298x-OEM PCI Modems */
@@ -885,6 +953,7 @@ static const struct pci_device_id exar_pci_tbl[] = {
 	EXAR_DEVICE(COMMTECH, 4224PCI335, pbn_fastcom335_4),
 	EXAR_DEVICE(COMMTECH, 2324PCI335, pbn_fastcom335_4),
 	EXAR_DEVICE(COMMTECH, 2328PCI335, pbn_fastcom335_8),
+
 	{ 0, }
 };
 MODULE_DEVICE_TABLE(pci, exar_pci_tbl);
diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
index 08b3527e1b93..188c5032f218 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -164,6 +164,8 @@
  */
 #define UART_DLL	0	/* Out: Divisor Latch Low */
 #define UART_DLM	1	/* Out: Divisor Latch High */
+#define UART_DLD	2	/* Divisor Fractional */
+#define UART_DLD_485_POLARITY 0x80 /* RS-485 Enable Signal Polarity */
 #define UART_DIV_MAX	0xFFFF	/* Max divisor value */
 
 /*

Re: [PATCH V2 2/2] serial: exar: Add RS-485 support for Sealevel XR17V35X based cards

[Andy Shevchenko]

On Wed, Aug 30, 2023 at 11:08:28AM -0400, Matthew Howell wrote:
> From: Matthew Howell <matthew.howell@sealevel.com>
> 
> Sealevel XR1735X based cards utilize DTR to control RS-485 Enable, but the 
> current implementation of 8250_exar uses RTS for the auto-RS485-Enable 
> mode of the XR17V35X UARTs. This patch applies a new sealevel_rs485_config 

s/This patch applies/Apply/

> function to configure the XR17V35X of Sealevel cards for DTR control of 
> RS485 Enable.
> 
> Based on feedback from the first submission I replaced the hex values with 
> defines and fixed up various format issues. I couldn't find an existing 
> define for the DLD register or its RS485 Polarity bit so I created a new 
> define. I tried to follow the format of the other defines in serial_reg.h.

...

> +static int sealevel_rs485_config(struct uart_port *port, struct ktermios *termios,
> +				struct serial_rs485 *rs485)
> +{
> +	bool is_rs485 = !!(rs485->flags & SER_RS485_ENABLED);

!!() is redundant.

> +	u8 __iomem *p = port->membase;
> +	u8 old_lcr;
> +
> +	generic_rs485_config(port, termios, rs485);

> +	if (is_rs485) {

	if (!is_rs485)
		return 0;

...

> +		// Set EFR[4]=1 to enable enhanced feature registers

> +		// Set MCR to use DTR as Auto-RS485 Enable signal

> +		// Store original LCR and set LCR[7]=1 to enable access to DLD register

> +		// Set DLD[7]=1 for inverted RS485 Enable logic

I believe the comment style in this file is /* */.

> +    }
> +
> +	return 0;
> + }

...

> +static int pci_sealevel_setup(struct exar8250 *priv, struct pci_dev *pcidev,
> +							  struct uart_8250_port *port, int idx)

Wrong indentation.

> +{
> +	int ret = pci_xr17v35x_setup(priv, pcidev, port, idx);
> +
> +	if (ret)
> +		return ret;

Use more robust style

	int ret;

	ret = pci_xr17v35x_setup(priv, pcidev, port, idx);
	if (ret)
		return ret;

> +	port->port.rs485_config = sealevel_rs485_config;
> +
> +	return ret;

	return 0;

> +}

...

> +static const struct exar8250_board pbn_sealevel_16 = {
> +	.num_ports  = 16,

> +    .setup		= pci_sealevel_setup,

TABs vs. spaces, please fix.

> +	.exit		= pci_xr17v35x_exit,
> +};

...

> @@ -885,6 +953,7 @@ static const struct pci_device_id exar_pci_tbl[] = {
>  	EXAR_DEVICE(COMMTECH, 4224PCI335, pbn_fastcom335_4),
>  	EXAR_DEVICE(COMMTECH, 2324PCI335, pbn_fastcom335_4),
>  	EXAR_DEVICE(COMMTECH, 2328PCI335, pbn_fastcom335_8),
> +
>  	{ 0, }
>  };
>  MODULE_DEVICE_TABLE(pci, exar_pci_tbl);

Stray change.

...

> --- a/include/uapi/linux/serial_reg.h
> +++ b/include/uapi/linux/serial_reg.h

Really?
Please, localize this to the driver.

It may be the same reason to do so as in 7e12357ed64a ("serial: exar:
Move register defines from uapi header to consumer site")

>  #define UART_DLL	0	/* Out: Divisor Latch Low */
>  #define UART_DLM	1	/* Out: Divisor Latch High */
> +#define UART_DLD	2	/* Divisor Fractional */
> +#define UART_DLD_485_POLARITY 0x80 /* RS-485 Enable Signal Polarity */
>  #define UART_DIV_MAX	0xFFFF	/* Max divisor value */

-- 
With Best Regards,
Andy Shevchenko