[PATCH v3 5/6] tty: serial: 8250: Fix MOXA RS422/RS485 PCIe boards not work by default

Crescent CY Hsieh posted 6 patches 2 years, 2 months ago
[PATCH v3 5/6] tty: serial: 8250: Fix MOXA RS422/RS485 PCIe boards not work by default
Posted by Crescent CY Hsieh 2 years, 2 months ago
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 | 52 ++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index b2be3783f..29a28e72b 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1968,6 +1968,12 @@ pci_sunix_setup(struct serial_private *priv,
 
 #define MOXA_GPIO_PIN2	BIT(2)
 
+#define MOXA_RS232	0x00
+#define MOXA_RS422	0x01
+#define MOXA_RS485_4W	0x0B
+#define MOXA_RS485_2W	0x0F
+#define MOXA_UIR_OFFSET	0x04
+
 static bool pci_moxa_is_mini_pcie(unsigned short device)
 {
 	if (device == PCI_DEVICE_ID_MOXA_CP102N	||
@@ -1981,13 +1987,59 @@ static bool pci_moxa_is_mini_pcie(unsigned short device)
 	return false;
 }
 
+/*
+ * The second digit of the MOXA PCIe device ID in hexadecimal indicates
+ * which serial interface modes this board supports:
+ *
+ *	0x*0** - RS232
+ *	0x*1** - RS232/RS422/RS485
+ *	0x*3** - RS422/RS485
+ *	0x*6** - RS232
+ */
+static bool pci_moxa_match_second_digit(unsigned short device,
+					unsigned short pattern)
+{
+	return (device & 0x0F00) == pattern;
+}
+
+static int pci_moxa_set_interface(const struct pci_dev *dev,
+				  unsigned int port_idx,
+				  u8 mode)
+{
+	resource_size_t iobar_addr = pci_resource_start(dev, 2);
+	resource_size_t UIR_addr = iobar_addr + MOXA_UIR_OFFSET + port_idx / 2;
+	u8 val;
+
+	val = inb(UIR_addr);
+
+	if (port_idx % 2) {
+		val &= 0x0F;
+		val |= mode << 4;
+	} else {
+		val &= 0xF0;
+		val |= mode;
+	}
+	outb(val, UIR_addr);
+
+	return 0;
+}
+
 static int pci_moxa_init(struct pci_dev *dev)
 {
 	unsigned short device = dev->device;
 	resource_size_t iobar_addr = pci_resource_start(dev, 2);
+	unsigned int i;
 	unsigned int num_ports = (device & 0x00F0) >> 4;
 	u8 val;
 
+	/*
+	 * 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 (pci_moxa_match_second_digit(device, 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 boots up.
 	 * This hardware buffer is only supported on Mini PCIe series.
-- 
2.34.1
Re: [PATCH v3 5/6] tty: serial: 8250: Fix MOXA RS422/RS485 PCIe boards not work by default
Posted by Ilpo Järvinen 2 years, 1 month ago
On Wed, 18 Oct 2023, 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 | 52 ++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index b2be3783f..29a28e72b 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1968,6 +1968,12 @@ pci_sunix_setup(struct serial_private *priv,
>  
>  #define MOXA_GPIO_PIN2	BIT(2)
>  
> +#define MOXA_RS232	0x00
> +#define MOXA_RS422	0x01
> +#define MOXA_RS485_4W	0x0B
> +#define MOXA_RS485_2W	0x0F
> +#define MOXA_UIR_OFFSET	0x04
> +
>  static bool pci_moxa_is_mini_pcie(unsigned short device)
>  {
>  	if (device == PCI_DEVICE_ID_MOXA_CP102N	||
> @@ -1981,13 +1987,59 @@ static bool pci_moxa_is_mini_pcie(unsigned short device)
>  	return false;
>  }
>  
> +/*
> + * The second digit of the MOXA PCIe device ID in hexadecimal indicates
> + * which serial interface modes this board supports:
> + *
> + *	0x*0** - RS232
> + *	0x*1** - RS232/RS422/RS485
> + *	0x*3** - RS422/RS485
> + *	0x*6** - RS232
> + */
> +static bool pci_moxa_match_second_digit(unsigned short device,
> +					unsigned short pattern)
> +{
> +	return (device & 0x0F00) == pattern;

Define a named field, not literal.

IMO, the function name is not good because it tells what code does, not 
why it does. I think the name should be based on .

> +}
> +
> +static int pci_moxa_set_interface(const struct pci_dev *dev,
> +				  unsigned int port_idx,
> +				  u8 mode)
> +{
> +	resource_size_t iobar_addr = pci_resource_start(dev, 2);
> +	resource_size_t UIR_addr = iobar_addr + MOXA_UIR_OFFSET + port_idx / 2;
> +	u8 val;
> +
> +	val = inb(UIR_addr);
> +
> +	if (port_idx % 2) {
> +		val &= 0x0F;
> +		val |= mode << 4;
> +	} else {
> +		val &= 0xF0;
> +		val |= mode;

These should use the typical pattern instead:

	val &= ~NAMED_DEFINE;
	val |= FIELD_PREP(NAMED_DEFINE, mode);

Also, don't forget to add bitfield.h if it's not there yet among the 
includes.

-- 
 i.



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

Put declaration of i after num_ports.

>  	u8 val;
>  
> +	/*
> +	 * 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 (pci_moxa_match_second_digit(device, 0x0300)) {

Name the literal with define, once you have better name for the function 
and no literal, the comment is no longer necessary at all because the code 
will document itself much better!

> +		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 boots up.
>  	 * This hardware buffer is only supported on Mini PCIe series.
>
Re: [PATCH v3 5/6] tty: serial: 8250: Fix MOXA RS422/RS485 PCIe boards not work by default
Posted by Jiri Slaby 2 years, 2 months ago
On 18. 10. 23, 11:17, 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 | 52 ++++++++++++++++++++++++++++++
>   1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index b2be3783f..29a28e72b 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
...
> @@ -1981,13 +1987,59 @@ static bool pci_moxa_is_mini_pcie(unsigned short device)
>   	return false;
>   }
>   
> +/*
> + * The second digit of the MOXA PCIe device ID in hexadecimal indicates
> + * which serial interface modes this board supports:
> + *
> + *	0x*0** - RS232
> + *	0x*1** - RS232/RS422/RS485
> + *	0x*3** - RS422/RS485
> + *	0x*6** - RS232
> + */
> +static bool pci_moxa_match_second_digit(unsigned short device,
> +					unsigned short pattern)
> +{
> +	return (device & 0x0F00) == pattern;
> +}

Actually, would it make sense to define the below instead:

enum {
    MOXA_SUPP_RS232 = BIT(0),
    MOXA_SUPP_RS422 = BIT(1),
    MOXA_SUPP_RS485 = BIT(2),
};

u32 pci_moxa_supported_rs(struct pci_dev *dev)
{
    switch (dev->device & 0x0f00) {
    case 0x0000:
    case 0x0600: // or default: ???
      return MOXA_SUPP_RS232;
    case 0x0100:
      return MOXA_SUPP_RS232 | MOXA_SUPP_RS422 | MOXA_SUPP_RS485;
    case 0x0300:
      return MOXA_SUPP_RS422 | MOXA_SUPP_RS485;
    }

    return 0;
}

...
>   static int pci_moxa_init(struct pci_dev *dev)
>   {
>   	unsigned short device = dev->device;
>   	resource_size_t iobar_addr = pci_resource_start(dev, 2);
> +	unsigned int i;
>   	unsigned int num_ports = (device & 0x00F0) >> 4;
>   	u8 val;
>   
> +	/*
> +	 * 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 (pci_moxa_match_second_digit(device, 0x0300)) {

And here:
if (pci_moxa_supported_rs(dev) == MOXA_SUPP_RS422 | MOXA_SUPP_RS485)

or perhaps:
if (!(pci_moxa_supported_rs(dev) & MOXA_SUPP_RS232))
?

> +		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 boots up.
>   	 * This hardware buffer is only supported on Mini PCIe series.

-- 
js
suse labs
Re: [PATCH v3 5/6] tty: serial: 8250: Fix MOXA RS422/RS485 PCIe boards not work by default
Posted by Jiri Slaby 2 years, 2 months ago
On 18. 10. 23, 11:17, 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>

Reviewed-by: Jiri Slaby <jirislaby@kernel.org>


-- 
js
suse labs