[PATCH 0/4] tty: serial: 8250: Changes of MOXA PCIe boards in 8250_pci.c

[PATCH 0/4] tty: serial: 8250: Changes of MOXA PCIe boards in 8250_pci.c

[Crescent CY Hsieh]

These patch series do some changes to MOXA PCIe boards in 8250_pci.c,
including:

- Cleanup MOXA configurations for future maintainability
- Add support for MOXA Mini PCIe boards
- Fix MOXA RS422/RS485 boards not function by default
- Add support for MOXA PCIe boards to switch between different serial
  interfaces

Each patch depends on previous one.

Crescent CY Hsieh (4):
  tty: serial: 8250: Cleanup MOXA configurations in 8250_pci.c
  tty: serial: 8250: Add support for MOXA Mini PCIe boards
  tty: serial: 8250: Fix MOXA RS422/RS485 PCIe boards not work by
    default
  tty: serial: 8250: Add support for MOXA PCIe boards to switch
    interface between RS422/RS485

 drivers/tty/serial/8250/8250_pci.c | 205 ++++++++++++++++++++++-------
 drivers/tty/serial/serial_core.c   |  20 ++-
 include/uapi/linux/serial.h        |   4 +
 3 files changed, 178 insertions(+), 51 deletions(-)

-- 
2.34.1

[PATCH 1/4] tty: serial: 8250: Cleanup MOXA configurations in 8250_pci.c

[Crescent CY Hsieh]

To enhance the maintainability of MOXA configurations in 8250_pci.c,
clean up the code to achieve simplicity, clarity and consistency.

Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com>
---
 drivers/tty/serial/8250/8250_pci.c | 73 +++++++++++-------------------
 1 file changed, 26 insertions(+), 47 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 62a9bd30b4db..a010790ccfcd 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1887,10 +1887,10 @@ pci_sunix_setup(struct serial_private *priv,
 	return setup_port(priv, port, bar, offset, 0);
 }
 
-static int
-pci_moxa_setup(struct serial_private *priv,
-		const struct pciserial_board *board,
-		struct uart_8250_port *port, int idx)
+static int pci_moxa_setup(struct serial_private *priv,
+			  const struct pciserial_board *board,
+			  struct uart_8250_port *port,
+			  int idx)
 {
 	unsigned int bar = FL_GET_BASE(board->flags);
 	int offset;
@@ -1958,6 +1958,9 @@ pci_moxa_setup(struct serial_private *priv,
 #define PCIE_DEVICE_ID_WCH_CH384_8S	0x3853
 #define PCIE_DEVICE_ID_WCH_CH382_2S	0x3253
 
+/* MOXA */
+#define PCI_VENDOR_ID_MOXA	0x1393
+/* MOXA PCIe */
 #define	PCI_DEVICE_ID_MOXA_CP102E	0x1024
 #define	PCI_DEVICE_ID_MOXA_CP102EL	0x1025
 #define	PCI_DEVICE_ID_MOXA_CP104EL_A	0x1045
@@ -2854,9 +2857,9 @@ enum pci_board_num_t {
 	pbn_titan_2_4000000,
 	pbn_titan_4_4000000,
 	pbn_titan_8_4000000,
-	pbn_moxa8250_2p,
-	pbn_moxa8250_4p,
-	pbn_moxa8250_8p,
+	pbn_moxa_2,
+	pbn_moxa_4,
+	pbn_moxa_8,
 };
 
 /*
@@ -3628,19 +3631,19 @@ static struct pciserial_board pci_boards[] = {
 		.uart_offset	= 0x200,
 		.first_offset	= 0x1000,
 	},
-	[pbn_moxa8250_2p] = {
+	[pbn_moxa_2] = {
 		.flags		= FL_BASE1,
 		.num_ports      = 2,
 		.base_baud      = 921600,
 		.uart_offset	= 0x200,
 	},
-	[pbn_moxa8250_4p] = {
+	[pbn_moxa_4] = {
 		.flags		= FL_BASE1,
 		.num_ports      = 4,
 		.base_baud      = 921600,
 		.uart_offset	= 0x200,
 	},
-	[pbn_moxa8250_8p] = {
+	[pbn_moxa_8] = {
 		.flags		= FL_BASE1,
 		.num_ports      = 8,
 		.base_baud      = 921600,
@@ -5347,44 +5350,20 @@ static const struct pci_device_id serial_pci_tbl[] = {
 		pbn_ni8430_4 },
 
 	/*
-	 * MOXA
+	 * MOXA PCIe
 	 */
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102E,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_2p },
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102EL,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_2p },
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP104EL_A,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_4p },
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP114EL,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_4p },
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP116E_A_A,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_8p },
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP116E_A_B,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_8p },
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP118EL_A,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_8p },
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP118E_A_I,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_8p },
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP132EL,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_2p },
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP134EL_A,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_4p },
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP138E_A,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_8p },
-	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP168EL_A,
-		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
-		pbn_moxa8250_8p },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102E),	 0, 0, pbn_moxa_2 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102EL),	 0, 0, pbn_moxa_2 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP104EL_A),	 0, 0, pbn_moxa_4 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP114EL),	 0, 0, pbn_moxa_4 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP116E_A_A), 0, 0, pbn_moxa_8 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP116E_A_B), 0, 0, pbn_moxa_8 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP118EL_A),	 0, 0, pbn_moxa_8 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP118E_A_I), 0, 0, pbn_moxa_8 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP132EL),	 0, 0, pbn_moxa_2 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP134EL_A),	 0, 0, pbn_moxa_4 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP138E_A),	 0, 0, pbn_moxa_8 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP168EL_A),	 0, 0, pbn_moxa_8 },
 
 	/*
 	* ADDI-DATA GmbH communication cards <info@addi-data.com>
-- 
2.34.1

Re: [PATCH 1/4] tty: serial: 8250: Cleanup MOXA configurations in 8250_pci.c

[Jiri Slaby]

Hi,

On 02. 10. 23, 3:56, Crescent CY Hsieh wrote:
> To enhance the maintainability of MOXA configurations in 8250_pci.c,
> clean up the code to achieve simplicity, clarity and consistency.
> 
> Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com>
> ---
>   drivers/tty/serial/8250/8250_pci.c | 73 +++++++++++-------------------
>   1 file changed, 26 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 62a9bd30b4db..a010790ccfcd 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1887,10 +1887,10 @@ pci_sunix_setup(struct serial_private *priv,
>   	return setup_port(priv, port, bar, offset, 0);
>   }
>   
> -static int
> -pci_moxa_setup(struct serial_private *priv,
> -		const struct pciserial_board *board,
> -		struct uart_8250_port *port, int idx)
> +static int pci_moxa_setup(struct serial_private *priv,
> +			  const struct pciserial_board *board,
> +			  struct uart_8250_port *port,
> +			  int idx)

You should either change all or none.

>   {
>   	unsigned int bar = FL_GET_BASE(board->flags);
>   	int offset;
> @@ -1958,6 +1958,9 @@ pci_moxa_setup(struct serial_private *priv,
>   #define PCIE_DEVICE_ID_WCH_CH384_8S	0x3853
>   #define PCIE_DEVICE_ID_WCH_CH382_2S	0x3253
>   
> +/* MOXA */
> +#define PCI_VENDOR_ID_MOXA	0x1393

Isn't this a redefinition of the pci-ids.h one?

> +/* MOXA PCIe */
>   #define	PCI_DEVICE_ID_MOXA_CP102E	0x1024
>   #define	PCI_DEVICE_ID_MOXA_CP102EL	0x1025
>   #define	PCI_DEVICE_ID_MOXA_CP104EL_A	0x1045
> @@ -2854,9 +2857,9 @@ enum pci_board_num_t {
>   	pbn_titan_2_4000000,
>   	pbn_titan_4_4000000,
>   	pbn_titan_8_4000000,
> -	pbn_moxa8250_2p,
> -	pbn_moxa8250_4p,
> -	pbn_moxa8250_8p,
> +	pbn_moxa_2,
> +	pbn_moxa_4,
> +	pbn_moxa_8,

This should be in a separate patch.

>   };
>   
>   /*
> @@ -3628,19 +3631,19 @@ static struct pciserial_board pci_boards[] = {
>   		.uart_offset	= 0x200,
>   		.first_offset	= 0x1000,
>   	},
> -	[pbn_moxa8250_2p] = {
> +	[pbn_moxa_2] = {
>   		.flags		= FL_BASE1,
>   		.num_ports      = 2,
>   		.base_baud      = 921600,
>   		.uart_offset	= 0x200,
>   	},
> -	[pbn_moxa8250_4p] = {
> +	[pbn_moxa_4] = {
>   		.flags		= FL_BASE1,
>   		.num_ports      = 4,
>   		.base_baud      = 921600,
>   		.uart_offset	= 0x200,
>   	},
> -	[pbn_moxa8250_8p] = {
> +	[pbn_moxa_8] = {
>   		.flags		= FL_BASE1,
>   		.num_ports      = 8,
>   		.base_baud      = 921600,
> @@ -5347,44 +5350,20 @@ static const struct pci_device_id serial_pci_tbl[] = {
>   		pbn_ni8430_4 },
>   
>   	/*
> -	 * MOXA
> +	 * MOXA PCIe
>   	 */
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102E,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_2p },
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102EL,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_2p },
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP104EL_A,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_4p },
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP114EL,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_4p },
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP116E_A_A,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_8p },
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP116E_A_B,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_8p },
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP118EL_A,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_8p },
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP118E_A_I,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_8p },
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP132EL,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_2p },
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP134EL_A,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_4p },
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP138E_A,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_8p },
> -	{	PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP168EL_A,
> -		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -		pbn_moxa8250_8p },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102E),	 0, 0, pbn_moxa_2 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102EL),	 0, 0, pbn_moxa_2 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP104EL_A),	 0, 0, pbn_moxa_4 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP114EL),	 0, 0, pbn_moxa_4 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP116E_A_A), 0, 0, pbn_moxa_8 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP116E_A_B), 0, 0, pbn_moxa_8 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP118EL_A),	 0, 0, pbn_moxa_8 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP118E_A_I), 0, 0, pbn_moxa_8 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP132EL),	 0, 0, pbn_moxa_2 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP134EL_A),	 0, 0, pbn_moxa_4 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP138E_A),	 0, 0, pbn_moxa_8 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP168EL_A),	 0, 0, pbn_moxa_8 },

Use PCI_VDEVICE()?


thanks,
-- 
js
suse labs

Re: [PATCH 1/4] tty: serial: 8250: Cleanup MOXA configurations in 8250_pci.c

[Crescent CY Hsieh]

On Mon, Oct 02, 2023 at 08:51:03AM +0200, Jiri Slaby wrote:
> On 02. 10. 23, 3:56, Crescent CY Hsieh wrote:
> > @@ -1958,6 +1958,9 @@ pci_moxa_setup(struct serial_private *priv,
> >   #define PCIE_DEVICE_ID_WCH_CH384_8S	0x3853
> >   #define PCIE_DEVICE_ID_WCH_CH382_2S	0x3253
> > +/* MOXA */
> > +#define PCI_VENDOR_ID_MOXA	0x1393
> 
> Isn't this a redefinition of the pci-ids.h one?

At first, I attempt to place DEVICE_ID macros into pci_ids.h to enable
their usage throughout 8250_pci.c and to establish centralized
management. However, I notice the comment in pci_ids.h which is:

'Do not add new entries to this file unless the definitions are shared
between multiple drivers'

So I add this VENDOR_ID just for the clarity, even though it results in
duplication.

Should I put these macros into pci_ids.h? If so, I am willing to help
relocate all other macros.

---
Sincerely,
Crescent CY Hsieh

Re: [PATCH 1/4] tty: serial: 8250: Cleanup MOXA configurations in 8250_pci.c

[Greg KH]

On Tue, Oct 03, 2023 at 03:17:47PM +0800, Crescent CY Hsieh wrote:
> On Mon, Oct 02, 2023 at 08:51:03AM +0200, Jiri Slaby wrote:
> > On 02. 10. 23, 3:56, Crescent CY Hsieh wrote:
> > > @@ -1958,6 +1958,9 @@ pci_moxa_setup(struct serial_private *priv,
> > >   #define PCIE_DEVICE_ID_WCH_CH384_8S	0x3853
> > >   #define PCIE_DEVICE_ID_WCH_CH382_2S	0x3253
> > > +/* MOXA */
> > > +#define PCI_VENDOR_ID_MOXA	0x1393
> > 
> > Isn't this a redefinition of the pci-ids.h one?
> 
> At first, I attempt to place DEVICE_ID macros into pci_ids.h to enable
> their usage throughout 8250_pci.c and to establish centralized
> management. However, I notice the comment in pci_ids.h which is:
> 
> 'Do not add new entries to this file unless the definitions are shared
> between multiple drivers'
> 
> So I add this VENDOR_ID just for the clarity, even though it results in
> duplication.
> 
> Should I put these macros into pci_ids.h? If so, I am willing to help
> relocate all other macros.

Please do not add new defines to pci_ids.h unless they are needed in
multiple files.

thanks,

greg k-h

Re: [PATCH 1/4] tty: serial: 8250: Cleanup MOXA configurations in 8250_pci.c

[Jiri Slaby]

On 03. 10. 23, 9:30, Greg KH wrote:
>> Should I put these macros into pci_ids.h? If so, I am willing to help
>> relocate all other macros.
> 
> Please do not add new defines to pci_ids.h unless they are needed in
> multiple files.

(Which VENDOR_MOXA is, but DEVICE ids are not.)


-- 
js
suse labs

[PATCH 2/4] tty: serial: 8250: Add support for MOXA Mini PCIe boards

[Crescent CY Hsieh]

Add support for MOXA Mini PCIe serial boards:

- CP102N: 2 ports | RS232
- CP104N: 4 ports | RS232
- CP112N: 2 ports | RS232/RS422/RS485
- CP114N: 4 ports | RS232/RS422/RS485
- CP132N: 2 ports | RS422/RS485
- CP134N: 4 ports | RS422/RS485

Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com>
---
 drivers/tty/serial/8250/8250_pci.c | 54 ++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index a010790ccfcd..a70546ac361e 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1887,6 +1887,42 @@ pci_sunix_setup(struct serial_private *priv,
 	return setup_port(priv, port, bar, offset, 0);
 }
 
+#define MOXA_PUART_GPIO_EN	0x09
+#define MOXA_PUART_GPIO_OUT	0x0A
+
+#define MOXA_GPIO_SET_ALL_OUTPUT	0x0F
+
+static int pci_moxa_init(struct pci_dev *dev)
+{
+	unsigned short device = dev->device;
+	unsigned long iobar_addr = pci_resource_start(dev, 2);
+	int num_ports = (device & 0x00F0) >> 4;
+	unsigned char val;
+
+	outb(MOXA_GPIO_SET_ALL_OUTPUT, iobar_addr + MOXA_PUART_GPIO_EN);
+
+	/*
+	 * Enable hardware buffer to prevent break signal output when system boot up.
+	 * This hardware buffer is only supported on Mini PCIe series.
+	 */
+	if (device == 0x1027 ||	/* MOXA_CP102N */
+	    device == 0x1046 ||	/* MOXA_CP104N */
+	    device == 0x1121 ||	/* MOXA_CP112N */
+	    device == 0x1145 ||	/* MOXA_CP114N */
+	    device == 0x1323 ||	/* MOXA_CP132N */
+	    device == 0x1343) {	/* MOXA_CP134N */
+		/* Set GPIO direction */
+		val = inb(iobar_addr + MOXA_PUART_GPIO_EN);
+		val |= (1 << 2);
+		outb(val, iobar_addr + MOXA_PUART_GPIO_EN);
+		/* Enable low GPIO */
+		val = inb(iobar_addr + MOXA_PUART_GPIO_OUT);
+		val &= ~(1 << 2);
+		outb(val, iobar_addr + MOXA_PUART_GPIO_OUT);
+	}
+	return num_ports;
+}
+
 static int pci_moxa_setup(struct serial_private *priv,
 			  const struct pciserial_board *board,
 			  struct uart_8250_port *port,
@@ -1973,6 +2009,13 @@ static int pci_moxa_setup(struct serial_private *priv,
 #define	PCI_DEVICE_ID_MOXA_CP134EL_A	0x1342
 #define	PCI_DEVICE_ID_MOXA_CP138E_A	0x1381
 #define	PCI_DEVICE_ID_MOXA_CP168EL_A	0x1683
+/* MOXA Mini PCIe */
+#define PCI_DEVICE_ID_MOXA_CP102N	0x1027
+#define PCI_DEVICE_ID_MOXA_CP104N	0x1046
+#define PCI_DEVICE_ID_MOXA_CP112N	0x1121
+#define PCI_DEVICE_ID_MOXA_CP114N	0x1145
+#define PCI_DEVICE_ID_MOXA_CP132N	0x1323
+#define PCI_DEVICE_ID_MOXA_CP134N	0x1343
 
 /* Unknown vendors/cards - this should not be in linux/pci_ids.h */
 #define PCI_SUBDEVICE_ID_UNKNOWN_0x1584	0x1584
@@ -2638,6 +2681,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
 		.device		= PCI_ANY_ID,
 		.subvendor	= PCI_ANY_ID,
 		.subdevice	= PCI_ANY_ID,
+		.init		= pci_moxa_init,
 		.setup		= pci_moxa_setup,
 	},
 	{
@@ -5365,6 +5409,16 @@ static const struct pci_device_id serial_pci_tbl[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP138E_A),	 0, 0, pbn_moxa_8 },
 	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP168EL_A),	 0, 0, pbn_moxa_8 },
 
+	/*
+	 * MOXA Mini PCIe
+	 */
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102N), 0, 0, pbn_moxa_2 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP104N), 0, 0, pbn_moxa_4 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP112N), 0, 0, pbn_moxa_2 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP114N), 0, 0, pbn_moxa_4 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP132N), 0, 0, pbn_moxa_2 },
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP134N), 0, 0, pbn_moxa_4 },
+
 	/*
 	* ADDI-DATA GmbH communication cards <info@addi-data.com>
 	*/
-- 
2.34.1

Re: [PATCH 2/4] tty: serial: 8250: Add support for MOXA Mini PCIe boards

[Jiri Slaby]

On 02. 10. 23, 3:57, Crescent CY Hsieh wrote:
> Add support for MOXA Mini PCIe serial boards:
> 
> - CP102N: 2 ports | RS232
> - CP104N: 4 ports | RS232
> - CP112N: 2 ports | RS232/RS422/RS485
> - CP114N: 4 ports | RS232/RS422/RS485
> - CP132N: 2 ports | RS422/RS485
> - CP134N: 4 ports | RS422/RS485
> 
> Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com>
> ---
>   drivers/tty/serial/8250/8250_pci.c | 54 ++++++++++++++++++++++++++++++
>   1 file changed, 54 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index a010790ccfcd..a70546ac361e 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1887,6 +1887,42 @@ pci_sunix_setup(struct serial_private *priv,
>   	return setup_port(priv, port, bar, offset, 0);
>   }
>   
> +#define MOXA_PUART_GPIO_EN	0x09
> +#define MOXA_PUART_GPIO_OUT	0x0A
> +
> +#define MOXA_GPIO_SET_ALL_OUTPUT	0x0F
> +
> +static int pci_moxa_init(struct pci_dev *dev)
> +{
> +	unsigned short device = dev->device;
> +	unsigned long iobar_addr = pci_resource_start(dev, 2);

resource_size_t

> +	int num_ports = (device & 0x00F0) >> 4;
> +	unsigned char val;

u8

> +
> +	outb(MOXA_GPIO_SET_ALL_OUTPUT, iobar_addr + MOXA_PUART_GPIO_EN);

You need to comment in the commit log why this doesn't matter for other 
moxa cards.

> +	/*
> +	 * Enable hardware buffer to prevent break signal output when system boot up.

boots up.

> +	 * This hardware buffer is only supported on Mini PCIe series.
> +	 */
> +	if (device == 0x1027 ||	/* MOXA_CP102N */
> +	    device == 0x1046 ||	/* MOXA_CP104N */
> +	    device == 0x1121 ||	/* MOXA_CP112N */
> +	    device == 0x1145 ||	/* MOXA_CP114N */
> +	    device == 0x1323 ||	/* MOXA_CP132N */
> +	    device == 0x1343) {	/* MOXA_CP134N */

Why not use the definitions below? You should define a function for this 
anyway.

> +		/* Set GPIO direction */
> +		val = inb(iobar_addr + MOXA_PUART_GPIO_EN);
> +		val |= (1 << 2);

Too magic constant. Use BIT() and define that 2 as some constant.

> +		outb(val, iobar_addr + MOXA_PUART_GPIO_EN);
> +		/* Enable low GPIO */
> +		val = inb(iobar_addr + MOXA_PUART_GPIO_OUT);
> +		val &= ~(1 << 2);
> +		outb(val, iobar_addr + MOXA_PUART_GPIO_OUT);
> +	}

One more \n here.

> +	return num_ports;
> +}
> +
>   static int pci_moxa_setup(struct serial_private *priv,
>   			  const struct pciserial_board *board,
>   			  struct uart_8250_port *port,
> @@ -1973,6 +2009,13 @@ static int pci_moxa_setup(struct serial_private *priv,
>   #define	PCI_DEVICE_ID_MOXA_CP134EL_A	0x1342
>   #define	PCI_DEVICE_ID_MOXA_CP138E_A	0x1381
>   #define	PCI_DEVICE_ID_MOXA_CP168EL_A	0x1683
> +/* MOXA Mini PCIe */
> +#define PCI_DEVICE_ID_MOXA_CP102N	0x1027
> +#define PCI_DEVICE_ID_MOXA_CP104N	0x1046
> +#define PCI_DEVICE_ID_MOXA_CP112N	0x1121
> +#define PCI_DEVICE_ID_MOXA_CP114N	0x1145
> +#define PCI_DEVICE_ID_MOXA_CP132N	0x1323
> +#define PCI_DEVICE_ID_MOXA_CP134N	0x1343

I am not sure it matters they are mini PCIe. I would sort them into the 
above preexisting list instead.

>   
>   /* Unknown vendors/cards - this should not be in linux/pci_ids.h */
>   #define PCI_SUBDEVICE_ID_UNKNOWN_0x1584	0x1584
> @@ -2638,6 +2681,7 @@ static struct pci_serial_quirk pci_serial_quirks[] = {
>   		.device		= PCI_ANY_ID,
>   		.subvendor	= PCI_ANY_ID,
>   		.subdevice	= PCI_ANY_ID,
> +		.init		= pci_moxa_init,
>   		.setup		= pci_moxa_setup,
>   	},
>   	{
> @@ -5365,6 +5409,16 @@ static const struct pci_device_id serial_pci_tbl[] = {
>   	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP138E_A),	 0, 0, pbn_moxa_8 },
>   	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP168EL_A),	 0, 0, pbn_moxa_8 },
>   
> +	/*
> +	 * MOXA Mini PCIe
> +	 */
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102N), 0, 0, pbn_moxa_2 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP104N), 0, 0, pbn_moxa_4 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP112N), 0, 0, pbn_moxa_2 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP114N), 0, 0, pbn_moxa_4 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP132N), 0, 0, pbn_moxa_2 },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP134N), 0, 0, pbn_moxa_4 },

PCI_VDEVICE() again?

thanks,
-- 
js
suse labs

Re: [PATCH 2/4] tty: serial: 8250: Add support for MOXA Mini PCIe boards

[Crescent CY Hsieh]

On Mon, Oct 02, 2023 at 09:05:31AM +0200, Jiri Slaby wrote:
> On 02. 10. 23, 3:57, Crescent CY Hsieh wrote:
> > +	 * This hardware buffer is only supported on Mini PCIe series.
> > +	 */
> > +	if (device == 0x1027 ||	/* MOXA_CP102N */
> > +	    device == 0x1046 ||	/* MOXA_CP104N */
> > +	    device == 0x1121 ||	/* MOXA_CP112N */
> > +	    device == 0x1145 ||	/* MOXA_CP114N */
> > +	    device == 0x1323 ||	/* MOXA_CP132N */
> > +	    device == 0x1343) {	/* MOXA_CP134N */
> 
> Why not use the definitions below? You should define a function for this
> anyway.

I cannot use macros here, unless I put the definitions before this
function or into pci_ids.h.

> > @@ -1973,6 +2009,13 @@ static int pci_moxa_setup(struct serial_private *priv,
> >   #define	PCI_DEVICE_ID_MOXA_CP134EL_A	0x1342
> >   #define	PCI_DEVICE_ID_MOXA_CP138E_A	0x1381
> >   #define	PCI_DEVICE_ID_MOXA_CP168EL_A	0x1683
> > +/* MOXA Mini PCIe */
> > +#define PCI_DEVICE_ID_MOXA_CP102N	0x1027
> > +#define PCI_DEVICE_ID_MOXA_CP104N	0x1046
> > +#define PCI_DEVICE_ID_MOXA_CP112N	0x1121
> > +#define PCI_DEVICE_ID_MOXA_CP114N	0x1145
> > +#define PCI_DEVICE_ID_MOXA_CP132N	0x1323
> > +#define PCI_DEVICE_ID_MOXA_CP134N	0x1343
> 
> I am not sure it matters they are mini PCIe. I would sort them into the
> above preexisting list instead.

For these lines, classify the type of boards is just for clarity, but
sort them into preexisting list is also feasible.

---
Sincerely,
Crescent CY Hsieh

[PATCH 3/4] tty: serial: 8250: Fix MOXA RS422/RS485 PCIe boards not work by default

[Crescent CY Hsieh]

MOXA PCIe RS422/RS485 boards will not function by default because of the
initial default serial interface of all MOXA PCIe boards is set to RS232.

This patch fixes the problem above by setting the initial default serial
interface to RS422 for those MOXA RS422/RS485 PCIe boards.

Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com>
---
 drivers/tty/serial/8250/8250_pci.c | 39 +++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index a70546ac361e..31bfe5381000 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1890,17 +1890,54 @@ pci_sunix_setup(struct serial_private *priv,
 #define MOXA_PUART_GPIO_EN	0x09
 #define MOXA_PUART_GPIO_OUT	0x0A
 
+#define MOXA_RS232	0x00
+#define MOXA_RS422	0x01
+#define MOXA_RS485_4W	0x0B
+#define MOXA_RS485_2W	0x0F
+#define MOXA_UIR_OFFSET	0x04
+
 #define MOXA_GPIO_SET_ALL_OUTPUT	0x0F
 
+static int pci_moxa_set_interface(struct pci_dev *dev,
+				  unsigned int port_idx,
+				  unsigned char mode)
+{
+	unsigned long iobar_addr = pci_resource_start(dev, 2);
+	unsigned long UIR_addr = iobar_addr + MOXA_UIR_OFFSET + (port_idx / 2);
+	unsigned char val, intf;
+
+	val = inb(UIR_addr);
+
+	if (port_idx % 2) {
+		intf = mode << 4;
+		val &= 0x0F;
+	} else {
+		intf = mode;
+		val &= 0xF0;
+	}
+	val |= intf;
+	outb(val, UIR_addr);
+
+	return 0;
+}
+
 static int pci_moxa_init(struct pci_dev *dev)
 {
 	unsigned short device = dev->device;
 	unsigned long iobar_addr = pci_resource_start(dev, 2);
+	int i;
 	int num_ports = (device & 0x00F0) >> 4;
 	unsigned char val;
 
 	outb(MOXA_GPIO_SET_ALL_OUTPUT, iobar_addr + MOXA_PUART_GPIO_EN);
-
+	/*
+	 * For the device IDs of MOXA PCIe boards match the pattern 0x*3**,
+	 * the initial default serial interface mode should be set to RS422.
+	 */
+	if ((device & 0x0F00) == 0x0300) {
+		for (i = 0; i < num_ports; ++i)
+			pci_moxa_set_interface(dev, i, MOXA_RS422);
+	}
 	/*
 	 * Enable hardware buffer to prevent break signal output when system boot up.
 	 * This hardware buffer is only supported on Mini PCIe series.
-- 
2.34.1

Re: [PATCH 3/4] tty: serial: 8250: Fix MOXA RS422/RS485 PCIe boards not work by default

[Jiri Slaby]

On 02. 10. 23, 3:57, Crescent CY Hsieh wrote:
> MOXA PCIe RS422/RS485 boards will not function by default because of the
> initial default serial interface of all MOXA PCIe boards is set to RS232.
> 
> This patch fixes the problem above by setting the initial default serial
> interface to RS422 for those MOXA RS422/RS485 PCIe boards.
> 
> Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com>
> ---
>   drivers/tty/serial/8250/8250_pci.c | 39 +++++++++++++++++++++++++++++-
>   1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index a70546ac361e..31bfe5381000 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1890,17 +1890,54 @@ pci_sunix_setup(struct serial_private *priv,
>   #define MOXA_PUART_GPIO_EN	0x09
>   #define MOXA_PUART_GPIO_OUT	0x0A
>   
> +#define MOXA_RS232	0x00
> +#define MOXA_RS422	0x01
> +#define MOXA_RS485_4W	0x0B
> +#define MOXA_RS485_2W	0x0F
> +#define MOXA_UIR_OFFSET	0x04
> +
>   #define MOXA_GPIO_SET_ALL_OUTPUT	0x0F
>   
> +static int pci_moxa_set_interface(struct pci_dev *dev,

can it be const?

> +				  unsigned int port_idx,
> +				  unsigned char mode)

u8

> +{
> +	unsigned long iobar_addr = pci_resource_start(dev, 2);

resource_size_t

> +	unsigned long UIR_addr = iobar_addr + MOXA_UIR_OFFSET + (port_idx / 2);

resource_size_t. Why the parentheses?

> +	unsigned char val, intf;

u8

> +	val = inb(UIR_addr);
> +
> +	if (port_idx % 2) {
> +		intf = mode << 4;
> +		val &= 0x0F;
> +	} else {
> +		intf = mode;
> +		val &= 0xF0;
> +	}
> +	val |= intf;

May be better:
   if (port_idx % 2) {
     val &= 0x0F;
     val |= mode << 4;
   } else {
     val &= 0xF0;
     val |= mode;
   }
(no need for intf)?


> +	outb(val, UIR_addr);
> +
> +	return 0;
> +}
> +
>   static int pci_moxa_init(struct pci_dev *dev)
>   {
>   	unsigned short device = dev->device;
>   	unsigned long iobar_addr = pci_resource_start(dev, 2);
> +	int i;

unsigned

>   	int num_ports = (device & 0x00F0) >> 4;

And this one too (in the previous patch).

>   	unsigned char val;
>   
>   	outb(MOXA_GPIO_SET_ALL_OUTPUT, iobar_addr + MOXA_PUART_GPIO_EN);
> -
> +	/*
> +	 * For the device IDs of MOXA PCIe boards match the pattern 0x*3**,
> +	 * the initial default serial interface mode should be set to RS422.
> +	 */
> +	if ((device & 0x0F00) == 0x0300) {
> +		for (i = 0; i < num_ports; ++i)
> +			pci_moxa_set_interface(dev, i, MOXA_RS422);
> +	}
>   	/*
>   	 * Enable hardware buffer to prevent break signal output when system boot up.
>   	 * This hardware buffer is only supported on Mini PCIe series.

-- 
js
suse labs

Re: [PATCH 3/4] tty: serial: 8250: Fix MOXA RS422/RS485 PCIe boards not work by default

[Crescent CY Hsieh]

On Mon, Oct 02, 2023 at 09:14:32AM +0200, Jiri Slaby wrote:
> On 02. 10. 23, 3:57, Crescent CY Hsieh wrote:
> >   static int pci_moxa_init(struct pci_dev *dev)
> >   {
> >   	unsigned short device = dev->device;
> >   	unsigned long iobar_addr = pci_resource_start(dev, 2);
> > +	int i;
> 
> unsigned
> 
> >   	int num_ports = (device & 0x00F0) >> 4;
> 
> And this one too (in the previous patch).

Sorry, I am not sure I get your meaning, could you explain to me?

---
Sincerely,
Crescent CY Hsieh

[PATCH 4/4] tty: serial: 8250: Add support for MOXA PCIe boards to switch interface between RS422/RS485

[Crescent CY Hsieh]

MOXA PCIe boards have 4 serial interfaces and don't require additional
stuff to switch between interfaces:

- RS232
- RS422
- RS485_2W (half-duplex)
- RS485_4W (full-duplex)

By using ioctl command "TIOCRS485", it can switch between default
interface and RS485 if supported.

That means, for RS422/RS485 board, it can switch between RS422 and
RS485 by setting the flags within struct serial_rs485.

However, for the RS232/RS422/RS485 board, it can only switch between
RS232 and RS485, there's no flag for switching interface into RS422.

This patch adds a flag call "SER_RS422_ENALBED" in serial.h and modifies
serial_core.c to make it possible to switch interface between RS232,
RS422 and RS485.

Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com>
---
 drivers/tty/serial/8250/8250_pci.c | 41 ++++++++++++++++++++++++++++++
 drivers/tty/serial/serial_core.c   | 20 ++++++++++++---
 include/uapi/linux/serial.h        |  4 +++
 3 files changed, 61 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 31bfe5381000..86dce37f08c0 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1898,6 +1898,10 @@ pci_sunix_setup(struct serial_private *priv,
 
 #define MOXA_GPIO_SET_ALL_OUTPUT	0x0F
 
+static const struct serial_rs485 pci_moxa_rs485_supported = {
+	.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX | SER_RS422_ENABLED,
+};
+
 static int pci_moxa_set_interface(struct pci_dev *dev,
 				  unsigned int port_idx,
 				  unsigned char mode)
@@ -1921,6 +1925,30 @@ static int pci_moxa_set_interface(struct pci_dev *dev,
 	return 0;
 }
 
+static int pci_moxa_rs485_config(struct uart_port *port,
+				 struct ktermios *termios,
+				 struct serial_rs485 *rs485)
+{
+	struct pci_dev *dev = to_pci_dev(port->dev);
+	unsigned short device = dev->device;
+	unsigned char mode = MOXA_RS232;
+
+	if (rs485->flags & SER_RS485_ENABLED) {
+		if (rs485->flags & SER_RS485_RX_DURING_TX)
+			mode = MOXA_RS485_4W;
+		else
+			mode = MOXA_RS485_2W;
+	} else if (rs485->flags & SER_RS422_ENABLED) {
+		mode = MOXA_RS422;
+	} else {
+		if ((device & 0x0F00) == 0x0300) {
+			pci_warn(dev, "RS232 interface is not supported.");
+			return -EINVAL;
+		}
+	}
+	return pci_moxa_set_interface(dev, port->line, mode);
+}
+
 static int pci_moxa_init(struct pci_dev *dev)
 {
 	unsigned short device = dev->device;
@@ -1965,9 +1993,22 @@ static int pci_moxa_setup(struct serial_private *priv,
 			  struct uart_8250_port *port,
 			  int idx)
 {
+	struct pci_dev *dev = priv->dev;
+	unsigned short device = dev->device;
 	unsigned int bar = FL_GET_BASE(board->flags);
 	int offset;
 
+	/*
+	 * For the device IDs of MOXA PCIe boards match the pattern 0x*3** and 0x*1**,
+	 * these boards support switching interface between RS422/RS485 using TIOCSRS485.
+	 */
+	if ((device & 0x0F00) == 0x0100 || (device & 0x0F00) == 0x0300) {
+		port->port.rs485_config = pci_moxa_rs485_config;
+		port->port.rs485_supported = pci_moxa_rs485_supported;
+
+		if ((device & 0x0F00) == 0x0300)
+			port->port.rs485.flags = SER_RE422_ENABLED;
+	}
 	if (board->num_ports == 4 && idx == 3)
 		offset = 7 * board->uart_offset;
 	else
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 7bdc21d5e13b..f316d9705809 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1299,7 +1299,7 @@ static int uart_get_icount(struct tty_struct *tty,
 
 #define SER_RS485_LEGACY_FLAGS	(SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | \
 				 SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX | \
-				 SER_RS485_TERMINATE_BUS)
+				 SER_RS485_TERMINATE_BUS | SER_RS422_ENALBED)
 
 static int uart_check_rs485_flags(struct uart_port *port, struct serial_rs485 *rs485)
 {
@@ -1365,11 +1365,23 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
 {
 	u32 supported_flags = port->rs485_supported.flags;
 
-	if (!(rs485->flags & SER_RS485_ENABLED)) {
+	if (!(rs485->flags & SER_RS485_ENABLED) && !(rs485->flags & SER_RS422_ENABLED)) {
 		memset(rs485, 0, sizeof(*rs485));
 		return;
 	}
-
+	/* Pick sane setting if the user enables both interfaces */
+	if (rs485->flags & SER_RS485_ENABLED && rs485->flags & SER_RS422_ENABLED) {
+		dev_warn_ratelimited(port->dev,
+			"%s (%d): Invalid serial interface setting, using RS485 instead\n",
+			port->name, port->line);
+		rs485->flags &= ~(SER_RS422_ENABLED);
+	}
+	/* Clear other bits and return if enalbe RS422 */
+	if (rs485->flags & SER_RS422_ENABLED) {
+		memset(rs485, 0, sizeof(*rs485));
+		rs485->flags |= SER_RS422_ENABLED;
+		return;
+	}
 	/* Pick sane settings if the user hasn't */
 	if ((supported_flags & (SER_RS485_RTS_ON_SEND|SER_RS485_RTS_AFTER_SEND)) &&
 	    !(rs485->flags & SER_RS485_RTS_ON_SEND) ==
@@ -1394,7 +1406,7 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
 static void uart_set_rs485_termination(struct uart_port *port,
 				       const struct serial_rs485 *rs485)
 {
-	if (!(rs485->flags & SER_RS485_ENABLED))
+	if (!(rs485->flags & SER_RS485_ENABLED) && !(rs485->flags & SER_RS422_ENABLED))
 		return;
 
 	gpiod_set_value_cansleep(port->rs485_term_gpio,
diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
index 53bc1af67a41..427609fd52cb 100644
--- a/include/uapi/linux/serial.h
+++ b/include/uapi/linux/serial.h
@@ -137,6 +137,8 @@ struct serial_icounter_struct {
  * * %SER_RS485_ADDRB		- Enable RS485 addressing mode.
  * * %SER_RS485_ADDR_RECV - Receive address filter (enables @addr_recv). Requires %SER_RS485_ADDRB.
  * * %SER_RS485_ADDR_DEST - Destination address (enables @addr_dest). Requires %SER_RS485_ADDRB.
+ *
+ * * %SER_RS422_ENABLED		- RS422 enabled.
  */
 struct serial_rs485 {
 	__u32	flags;
@@ -149,6 +151,8 @@ struct serial_rs485 {
 #define SER_RS485_ADDR_RECV		(1 << 7)
 #define SER_RS485_ADDR_DEST		(1 << 8)
 
+#define SER_RS422_ENABLED		(1 << 9)
+
 	__u32	delay_rts_before_send;
 	__u32	delay_rts_after_send;
 
-- 
2.34.1

Re: [PATCH 4/4] tty: serial: 8250: Add support for MOXA PCIe boards to switch interface between RS422/RS485

[kernel test robot]

Hi Crescent,

kernel test robot noticed the following build errors:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.6-rc4 next-20230929]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Crescent-CY-Hsieh/tty-serial-8250-Cleanup-MOXA-configurations-in-8250_pci-c/20231002-095945
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20231002015702.30509-5-crescentcy.hsieh%40moxa.com
patch subject: [PATCH 4/4] tty: serial: 8250: Add support for MOXA PCIe boards to switch interface between RS422/RS485
config: sparc-allnoconfig (https://download.01.org/0day-ci/archive/20231002/202310021202.xDwDImQv-lkp@intel.com/config)
compiler: sparc-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231002/202310021202.xDwDImQv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310021202.xDwDImQv-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/tty/serial/serial_core.c: In function 'uart_check_rs485_flags':
>> drivers/tty/serial/serial_core.c:1302:60: error: 'SER_RS422_ENALBED' undeclared (first use in this function); did you mean 'SER_RS422_ENABLED'?
    1302 |                                  SER_RS485_TERMINATE_BUS | SER_RS422_ENALBED)
         |                                                            ^~~~~~~~~~~~~~~~~
   drivers/tty/serial/serial_core.c:1309:19: note: in expansion of macro 'SER_RS485_LEGACY_FLAGS'
    1309 |         flags &= ~SER_RS485_LEGACY_FLAGS;
         |                   ^~~~~~~~~~~~~~~~~~~~~~
   drivers/tty/serial/serial_core.c:1302:60: note: each undeclared identifier is reported only once for each function it appears in
    1302 |                                  SER_RS485_TERMINATE_BUS | SER_RS422_ENALBED)
         |                                                            ^~~~~~~~~~~~~~~~~
   drivers/tty/serial/serial_core.c:1309:19: note: in expansion of macro 'SER_RS485_LEGACY_FLAGS'
    1309 |         flags &= ~SER_RS485_LEGACY_FLAGS;
         |                   ^~~~~~~~~~~~~~~~~~~~~~


vim +1302 drivers/tty/serial/serial_core.c

  1299	
  1300	#define SER_RS485_LEGACY_FLAGS	(SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | \
  1301					 SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX | \
> 1302					 SER_RS485_TERMINATE_BUS | SER_RS422_ENALBED)
  1303	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 4/4] tty: serial: 8250: Add support for MOXA PCIe boards to switch interface between RS422/RS485

[kernel test robot]

Hi Crescent,

kernel test robot noticed the following build errors:

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on tty/tty-next tty/tty-linus usb/usb-testing usb/usb-next usb/usb-linus linus/master v6.6-rc4 next-20230929]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Crescent-CY-Hsieh/tty-serial-8250-Cleanup-MOXA-configurations-in-8250_pci-c/20231002-095945
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
patch link:    https://lore.kernel.org/r/20231002015702.30509-5-crescentcy.hsieh%40moxa.com
patch subject: [PATCH 4/4] tty: serial: 8250: Add support for MOXA PCIe boards to switch interface between RS422/RS485
config: alpha-defconfig (https://download.01.org/0day-ci/archive/20231002/202310021306.ewtlC1QY-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231002/202310021306.ewtlC1QY-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310021306.ewtlC1QY-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/tty/serial/8250/8250_pci.c: In function 'pci_moxa_setup':
>> drivers/tty/serial/8250/8250_pci.c:2010:50: error: 'SER_RE422_ENABLED' undeclared (first use in this function); did you mean 'SER_RS422_ENABLED'?
    2010 |                         port->port.rs485.flags = SER_RE422_ENABLED;
         |                                                  ^~~~~~~~~~~~~~~~~
         |                                                  SER_RS422_ENABLED
   drivers/tty/serial/8250/8250_pci.c:2010:50: note: each undeclared identifier is reported only once for each function it appears in


vim +2010 drivers/tty/serial/8250/8250_pci.c

  1990	
  1991	static int pci_moxa_setup(struct serial_private *priv,
  1992				  const struct pciserial_board *board,
  1993				  struct uart_8250_port *port,
  1994				  int idx)
  1995	{
  1996		struct pci_dev *dev = priv->dev;
  1997		unsigned short device = dev->device;
  1998		unsigned int bar = FL_GET_BASE(board->flags);
  1999		int offset;
  2000	
  2001		/*
  2002		 * For the device IDs of MOXA PCIe boards match the pattern 0x*3** and 0x*1**,
  2003		 * these boards support switching interface between RS422/RS485 using TIOCSRS485.
  2004		 */
  2005		if ((device & 0x0F00) == 0x0100 || (device & 0x0F00) == 0x0300) {
  2006			port->port.rs485_config = pci_moxa_rs485_config;
  2007			port->port.rs485_supported = pci_moxa_rs485_supported;
  2008	
  2009			if ((device & 0x0F00) == 0x0300)
> 2010				port->port.rs485.flags = SER_RE422_ENABLED;
  2011		}
  2012		if (board->num_ports == 4 && idx == 3)
  2013			offset = 7 * board->uart_offset;
  2014		else
  2015			offset = idx * board->uart_offset;
  2016	
  2017		return setup_port(priv, port, bar, offset, 0);
  2018	}
  2019	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 4/4] tty: serial: 8250: Add support for MOXA PCIe boards to switch interface between RS422/RS485

[Jiri Slaby]

On 02. 10. 23, 3:57, Crescent CY Hsieh wrote:
> MOXA PCIe boards have 4 serial interfaces and don't require additional
> stuff to switch between interfaces:
> 
> - RS232
> - RS422
> - RS485_2W (half-duplex)
> - RS485_4W (full-duplex)
> 
> By using ioctl command "TIOCRS485", it can switch between default
> interface and RS485 if supported.
> 
> That means, for RS422/RS485 board, it can switch between RS422 and
> RS485 by setting the flags within struct serial_rs485.
> 
> However, for the RS232/RS422/RS485 board, it can only switch between
> RS232 and RS485, there's no flag for switching interface into RS422.
> 
> This patch adds a flag call "SER_RS422_ENALBED" in serial.h and modifies

Hopefully not SER_RS422_ENALBED.

> serial_core.c to make it possible to switch interface between RS232,
> RS422 and RS485.
> 
> Signed-off-by: Crescent CY Hsieh <crescentcy.hsieh@moxa.com>
> ---
>   drivers/tty/serial/8250/8250_pci.c | 41 ++++++++++++++++++++++++++++++
>   drivers/tty/serial/serial_core.c   | 20 ++++++++++++---
>   include/uapi/linux/serial.h        |  4 +++
>   3 files changed, 61 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 31bfe5381000..86dce37f08c0 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1898,6 +1898,10 @@ pci_sunix_setup(struct serial_private *priv,
>   
>   #define MOXA_GPIO_SET_ALL_OUTPUT	0x0F
>   
> +static const struct serial_rs485 pci_moxa_rs485_supported = {
> +	.flags = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX | SER_RS422_ENABLED,
> +};
> +
>   static int pci_moxa_set_interface(struct pci_dev *dev,
>   				  unsigned int port_idx,
>   				  unsigned char mode)
> @@ -1921,6 +1925,30 @@ static int pci_moxa_set_interface(struct pci_dev *dev,
>   	return 0;
>   }
>   
> +static int pci_moxa_rs485_config(struct uart_port *port,
> +				 struct ktermios *termios,
> +				 struct serial_rs485 *rs485)
> +{
> +	struct pci_dev *dev = to_pci_dev(port->dev);
> +	unsigned short device = dev->device;
> +	unsigned char mode = MOXA_RS232;

u8

> +
> +	if (rs485->flags & SER_RS485_ENABLED) {
> +		if (rs485->flags & SER_RS485_RX_DURING_TX)
> +			mode = MOXA_RS485_4W;
> +		else
> +			mode = MOXA_RS485_2W;
> +	} else if (rs485->flags & SER_RS422_ENABLED) {
> +		mode = MOXA_RS422;
> +	} else {
> +		if ((device & 0x0F00) == 0x0300) {

This is the second time you test this. Introduce a function.

> +			pci_warn(dev, "RS232 interface is not supported.");

Don't spam the logs.

> +			return -EINVAL;
> +		}
> +	}

add \n here

> +	return pci_moxa_set_interface(dev, port->line, mode);
> +}
> +
>   static int pci_moxa_init(struct pci_dev *dev)
>   {
>   	unsigned short device = dev->device;
> @@ -1965,9 +1993,22 @@ static int pci_moxa_setup(struct serial_private *priv,
>   			  struct uart_8250_port *port,
>   			  int idx)
>   {
> +	struct pci_dev *dev = priv->dev;
> +	unsigned short device = dev->device;
>   	unsigned int bar = FL_GET_BASE(board->flags);
>   	int offset;
>   
> +	/*
> +	 * For the device IDs of MOXA PCIe boards match the pattern 0x*3** and 0x*1**,
> +	 * these boards support switching interface between RS422/RS485 using TIOCSRS485.
> +	 */
> +	if ((device & 0x0F00) == 0x0100 || (device & 0x0F00) == 0x0300) {

Use helpers.

> +		port->port.rs485_config = pci_moxa_rs485_config;
> +		port->port.rs485_supported = pci_moxa_rs485_supported;
> +
> +		if ((device & 0x0F00) == 0x0300)
> +			port->port.rs485.flags = SER_RE422_ENABLED;
> +	}
>   	if (board->num_ports == 4 && idx == 3)
>   		offset = 7 * board->uart_offset;
>   	else
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 7bdc21d5e13b..f316d9705809 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1299,7 +1299,7 @@ static int uart_get_icount(struct tty_struct *tty,
>   
>   #define SER_RS485_LEGACY_FLAGS	(SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | \
>   				 SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX | \
> -				 SER_RS485_TERMINATE_BUS)
> +				 SER_RS485_TERMINATE_BUS | SER_RS422_ENALBED)

So have you tested this at all?

> @@ -1365,11 +1365,23 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
>   {
>   	u32 supported_flags = port->rs485_supported.flags;
>   
> -	if (!(rs485->flags & SER_RS485_ENABLED)) {
> +	if (!(rs485->flags & SER_RS485_ENABLED) && !(rs485->flags & SER_RS422_ENABLED)) {
>   		memset(rs485, 0, sizeof(*rs485));
>   		return;
>   	}
> -
> +	/* Pick sane setting if the user enables both interfaces */
> +	if (rs485->flags & SER_RS485_ENABLED && rs485->flags & SER_RS422_ENABLED) {
> +		dev_warn_ratelimited(port->dev,
> +			"%s (%d): Invalid serial interface setting, using RS485 instead\n",
> +			port->name, port->line);
> +		rs485->flags &= ~(SER_RS422_ENABLED);

No need for parens.

> +	}
> +	/* Clear other bits and return if enalbe RS422 */

enable

> +	if (rs485->flags & SER_RS422_ENABLED) {
> +		memset(rs485, 0, sizeof(*rs485));
> +		rs485->flags |= SER_RS422_ENABLED;
> +		return;
> +	}
>   	/* Pick sane settings if the user hasn't */
>   	if ((supported_flags & (SER_RS485_RTS_ON_SEND|SER_RS485_RTS_AFTER_SEND)) &&
>   	    !(rs485->flags & SER_RS485_RTS_ON_SEND) ==
> @@ -1394,7 +1406,7 @@ static void uart_sanitize_serial_rs485(struct uart_port *port, struct serial_rs4
>   static void uart_set_rs485_termination(struct uart_port *port,
>   				       const struct serial_rs485 *rs485)
>   {
> -	if (!(rs485->flags & SER_RS485_ENABLED))
> +	if (!(rs485->flags & SER_RS485_ENABLED) && !(rs485->flags & SER_RS422_ENABLED))
>   		return;
>   
>   	gpiod_set_value_cansleep(port->rs485_term_gpio,
> diff --git a/include/uapi/linux/serial.h b/include/uapi/linux/serial.h
> index 53bc1af67a41..427609fd52cb 100644
> --- a/include/uapi/linux/serial.h
> +++ b/include/uapi/linux/serial.h
> @@ -137,6 +137,8 @@ struct serial_icounter_struct {
>    * * %SER_RS485_ADDRB		- Enable RS485 addressing mode.
>    * * %SER_RS485_ADDR_RECV - Receive address filter (enables @addr_recv). Requires %SER_RS485_ADDRB.
>    * * %SER_RS485_ADDR_DEST - Destination address (enables @addr_dest). Requires %SER_RS485_ADDRB.
> + *
> + * * %SER_RS422_ENABLED		- RS422 enabled.
>    */
>   struct serial_rs485 {
>   	__u32	flags;
> @@ -149,6 +151,8 @@ struct serial_rs485 {
>   #define SER_RS485_ADDR_RECV		(1 << 7)
>   #define SER_RS485_ADDR_DEST		(1 << 8)
>   
> +#define SER_RS422_ENABLED		(1 << 9)
> +
>   	__u32	delay_rts_before_send;
>   	__u32	delay_rts_after_send;
>   

thanks,
-- 
js
suse labs

Re: [PATCH 4/4] tty: serial: 8250: Add support for MOXA PCIe boards to switch interface between RS422/RS485

[Crescent CY Hsieh]

On Mon, Oct 02, 2023 at 09:19:26AM +0200, Jiri Slaby wrote:
> On 02. 10. 23, 3:57, Crescent CY Hsieh wrote:
> > MOXA PCIe boards have 4 serial interfaces and don't require additional
> > stuff to switch between interfaces:
> > 
> > - RS232
> > - RS422
> > - RS485_2W (half-duplex)
> > - RS485_4W (full-duplex)
> > 
> > By using ioctl command "TIOCRS485", it can switch between default
> > interface and RS485 if supported.
> > 
> > That means, for RS422/RS485 board, it can switch between RS422 and
> > RS485 by setting the flags within struct serial_rs485.
> > 
> > However, for the RS232/RS422/RS485 board, it can only switch between
> > RS232 and RS485, there's no flag for switching interface into RS422.
> > 
> > This patch adds a flag call "SER_RS422_ENALBED" in serial.h and modifies
> 
> Hopefully not SER_RS422_ENALBED.

What if I use an existing flag within struct serial_rs485 to represent
RS422 as a workaround solution?

> > @@ -1299,7 +1299,7 @@ static int uart_get_icount(struct tty_struct *tty,
> >   #define SER_RS485_LEGACY_FLAGS	(SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | \
> >   				 SER_RS485_RTS_AFTER_SEND | SER_RS485_RX_DURING_TX | \
> > -				 SER_RS485_TERMINATE_BUS)
> > +				 SER_RS485_TERMINATE_BUS | SER_RS422_ENALBED)
> 
> So have you tested this at all?

I have tested it by switching the serial interface using TIOCSRS485
ioctl command, but it seems that I send the wrong patch which mistyping
SER_RS422_ENABLED.

---
Sincerely,
Crescent CY Hsieh