[PATCH v1 31/31] serial: 8250_mxpcie: add RS485-2W auto-adjust sysfs control

Crescent Hsieh posted 31 patches 2 months, 1 week ago
[PATCH v1 31/31] serial: 8250_mxpcie: add RS485-2W auto-adjust sysfs control
Posted by Crescent Hsieh 2 months, 1 week ago
Moxa CP118E/CP138E/CP134EL/CP116E boards support a hardware "auto mode"
in RS485-2W where the CPLD can automatically configure terminator and
pull-state based on the selected baud rate and line condition. This mode
is not meant to be kept enabled continuously — instead, it is triggered
for a short period (about 2s) to let the hardware calibrate itself.

This patch adds a write-only sysfs attribute that requests a one-shot
hardware auto-adjust of terminator and pull-state. Writing a supported
baud rate value (one of 9600, 19200, 38400, 57600, 115200, 230400,
460800, or 921600) into the attribute programs the CPLD baud code and
runs a temporary AUTO mode cycle (about 2s) on RS485-2W ports of the
supported boards. Invalid interfaces or devices are rejected.

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

diff --git a/drivers/tty/serial/8250/8250_mxpcie.c b/drivers/tty/serial/8250/8250_mxpcie.c
index e9e3d03b7712..b3146cac0298 100644
--- a/drivers/tty/serial/8250/8250_mxpcie.c
+++ b/drivers/tty/serial/8250/8250_mxpcie.c
@@ -107,11 +107,15 @@
 #define MOXA_CPLD_SET_STATE_BASE	0x18
 #define MOXA_CPLD_STATE_MASK		0x0F
 
+#define MOXA_CPLD_SET_BAUD_BASE	0x08
+#define MOXA_CPLD_BAUD_MASK	0x07
+
 #define MOXA_CPLD_DATA_MASK	0x1F	/* Pin0 ~ Pin4 */
 #define MOXA_CPLD_CTRL_MASK	0xE0	/* Pin5 ~ Pin7 */
 
 #define MOXA_CPLD_PULL_STATE_FLAG	0x01
 #define MOXA_CPLD_TERMINATOR_FLAG	0x02
+#define MOXA_CPLD_AUTO_MODE_FLAG	0x08
 
 #define MOXA_CPLD_READ	0
 #define MOXA_CPLD_WRITE	1
@@ -125,6 +129,18 @@
 #define MOXA_PULL_STATE_ENABLE	1
 #define MOXA_PULL_STATE_DISABLE	0
 
+#define MOXA_AUTO_MODE_ENABLE	1
+#define MOXA_AUTO_MODE_DISABLE	0
+
+#define MOXA_CPLD_BAUD_9600	0
+#define MOXA_CPLD_BAUD_19200	1
+#define MOXA_CPLD_BAUD_38400	2
+#define MOXA_CPLD_BAUD_57600	3
+#define MOXA_CPLD_BAUD_115200	4
+#define MOXA_CPLD_BAUD_230400	5
+#define MOXA_CPLD_BAUD_460800	6
+#define MOXA_CPLD_BAUD_921600	7
+
 #define MOXA_UIR_OFFSET		0x04
 #define MOXA_UIR_RS232		0x00
 #define MOXA_UIR_RS422		0x01
@@ -566,6 +582,54 @@ static int mxpcie8250_cpld_get_pull_state(resource_size_t iobar_addr, u8 port_id
 	return MOXA_PULL_STATE_DISABLE;
 }
 
+/**
+ * mxpcie8250_cpld_set_auto_mode() - Set the auto mode of the specified port
+ * @iobar_addr:	The base address of the GPIO I/O region
+ * @port_idx:	The port index (0 to 7)
+ * @state:	Desired auto mode state (MOXA_AUTO_MODE_ENABLE or MOXA_AUTO_MODE_DISABLE)
+ *
+ * Updates the auto mode setting in the CPLD for the specified port by reading
+ * the current state, modifying the auto mode bit, and writing the updated
+ * state back to the CPLD.
+ */
+static void mxpcie8250_cpld_set_auto_mode(resource_size_t iobar_addr, u8 port_idx, u8 state)
+{
+	u8 addr, data;
+
+	addr = MOXA_CPLD_GET_STATE_BASE + port_idx;
+	mxpcie8250_cpld_read(iobar_addr, addr, &data);
+
+	data = data & MOXA_CPLD_STATE_MASK;
+
+	if (state == MOXA_AUTO_MODE_ENABLE)
+		data |= MOXA_CPLD_AUTO_MODE_FLAG;
+	else
+		data &= ~MOXA_CPLD_AUTO_MODE_FLAG;
+
+	addr = MOXA_CPLD_SET_STATE_BASE + port_idx;
+	mxpcie8250_cpld_write(iobar_addr, addr, data);
+}
+
+/**
+ * mxpcie8250_cpld_set_baud_rate() - Set the baud rate code of the specified port
+ * @iobar_addr:  The base address of the GPIO I/O region
+ * @port_idx:    The port index (0 to 7)
+ * @baud:        CPLD baud rate code (MOXA_CPLD_BAUD_9600 ... MOXA_CPLD_BAUD_921600)
+ *
+ * Writes the baud rate selection value directly into the CPLD SET_BAUD register
+ * for the specified port. The caller must ensure the value is pre-mapped to
+ * the 3-bit CPLD baud code.
+ */
+static void mxpcie8250_cpld_set_baud_rate(resource_size_t iobar_addr, u8 port_idx, u8 baud)
+{
+	u8 addr, data;
+
+	data = baud & MOXA_CPLD_BAUD_MASK;
+
+	addr = MOXA_CPLD_SET_BAUD_BASE + port_idx;
+	mxpcie8250_cpld_write(iobar_addr, addr, data);
+}
+
 static bool mxpcie8250_is_mini_pcie(unsigned short device)
 {
 	if (device == PCI_DEVICE_ID_MOXA_CP102N ||
@@ -1134,12 +1198,91 @@ static ssize_t mxpcie8250_pull_state_show(struct device *dev,
 	return -ENODEV;
 }
 
+static ssize_t mxpcie8250_auto_mode_store(struct device *dev,
+					  struct device_attribute *attr,
+					  const char *buf,
+					  size_t count)
+{
+	struct tty_port *tport = dev_get_drvdata(dev);
+	struct uart_state *ustate = container_of(tport, struct uart_state, port);
+	struct uart_port *port = ustate->uart_port;
+	struct pci_dev *pdev = to_pci_dev(port->dev);
+	struct mxpcie8250 *priv = pci_get_drvdata(pdev);
+	resource_size_t iobar_addr = pci_resource_start(pdev, 2);
+	int ret;
+	u32 baud;
+
+	if (!count)
+		return -EINVAL;
+
+	ret = kstrtou32(buf, 10, &baud);
+
+	if (ret < 0)
+		return ret;
+
+	if (priv->port[port->port_id].interface != MOXA_UIR_RS485_2W) {
+		dev_warn(dev, "The serial interface of this port should be RS485-2W\n");
+		return -EINVAL;
+	}
+	if (pdev->device == PCI_DEVICE_ID_MOXA_CP118E_A_I ||
+	    pdev->device == PCI_DEVICE_ID_MOXA_CP138E_A   ||
+	    pdev->device == PCI_DEVICE_ID_MOXA_CP134EL_A  ||
+	    pdev->device == PCI_DEVICE_ID_MOXA_CP116E_A_A ||
+	    pdev->device == PCI_DEVICE_ID_MOXA_CP116E_A_B) {
+		switch (baud) {
+		case 9600:
+			baud = MOXA_CPLD_BAUD_9600;
+			break;
+		case 19200:
+			baud = MOXA_CPLD_BAUD_19200;
+			break;
+		case 38400:
+			baud = MOXA_CPLD_BAUD_38400;
+			break;
+		case 57600:
+			baud = MOXA_CPLD_BAUD_57600;
+			break;
+		case 115200:
+			baud = MOXA_CPLD_BAUD_115200;
+			break;
+		case 230400:
+			baud = MOXA_CPLD_BAUD_230400;
+			break;
+		case 460800:
+			baud = MOXA_CPLD_BAUD_460800;
+			break;
+		case 921600:
+			baud = MOXA_CPLD_BAUD_921600;
+			break;
+		default:
+			dev_warn(dev, "The baud rate should be one of 9600, 19200, 38400, 57600, 115200, 230400, 460800, 921600\n");
+			return -EINVAL;
+		}
+		spin_lock(&priv->board_lock);
+		mxpcie8250_cpld_set_baud_rate(iobar_addr, port->port_id, baud);
+		mxpcie8250_cpld_set_auto_mode(iobar_addr, port->port_id, MOXA_AUTO_MODE_ENABLE);
+		spin_unlock(&priv->board_lock);
+
+		msleep(2000);
+
+		spin_lock(&priv->board_lock);
+		mxpcie8250_cpld_set_auto_mode(iobar_addr, port->port_id, MOXA_AUTO_MODE_DISABLE);
+		spin_unlock(&priv->board_lock);
+
+		return count;
+	}
+
+	return -ENODEV;
+}
+
 static DEVICE_ATTR_RW(mxpcie8250_terminator);
 static DEVICE_ATTR_RW(mxpcie8250_pull_state);
+static DEVICE_ATTR_WO(mxpcie8250_auto_mode);
 
 static struct attribute *mxpcie8250_dev_attrs[] = {
 	&dev_attr_mxpcie8250_terminator.attr,
 	&dev_attr_mxpcie8250_pull_state.attr,
+	&dev_attr_mxpcie8250_auto_mode.attr,
 	NULL
 };
 
-- 
2.45.2

Re: [PATCH v1 31/31] serial: 8250_mxpcie: add RS485-2W auto-adjust sysfs control
Posted by Andy Shevchenko 2 months, 1 week ago
On Sun, Nov 30, 2025 at 12:46 PM Crescent Hsieh
<crescentcy.hsieh@moxa.com> wrote:
>
> Moxa CP118E/CP138E/CP134EL/CP116E boards support a hardware "auto mode"
> in RS485-2W where the CPLD can automatically configure terminator and
> pull-state based on the selected baud rate and line condition. This mode
> is not meant to be kept enabled continuously — instead, it is triggered
> for a short period (about 2s) to let the hardware calibrate itself.
>
> This patch adds a write-only sysfs attribute that requests a one-shot
> hardware auto-adjust of terminator and pull-state. Writing a supported
> baud rate value (one of 9600, 19200, 38400, 57600, 115200, 230400,
> 460800, or 921600) into the attribute programs the CPLD baud code and
> runs a temporary AUTO mode cycle (about 2s) on RS485-2W ports of the
> supported boards. Invalid interfaces or devices are rejected.

I'm not going to review this, the patch series is already quite big. I
suggest you to start from the small things in a different series E.g.,
the first series is just converting MOXA from custom to 8250-based
(assuming all features are kept working while ABI is being broken,
which has to be explained in the commit message(s) / cover letter of
that series), the second one is about splitting 8250_pci to the other
Moxa case. When this is done, we may go forward with the additional
features.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 31/31] serial: 8250_mxpcie: add RS485-2W auto-adjust sysfs control
Posted by Crescent Hsieh 2 months, 1 week ago
On Mon, Dec 01, 2025 at 04:45:58AM +0200, Andy Shevchenko wrote:
> On Sun, Nov 30, 2025 at 12:46 PM Crescent Hsieh
> <crescentcy.hsieh@moxa.com> wrote:
> >
> > Moxa CP118E/CP138E/CP134EL/CP116E boards support a hardware "auto mode"
> > in RS485-2W where the CPLD can automatically configure terminator and
> > pull-state based on the selected baud rate and line condition. This mode
> > is not meant to be kept enabled continuously — instead, it is triggered
> > for a short period (about 2s) to let the hardware calibrate itself.
> >
> > This patch adds a write-only sysfs attribute that requests a one-shot
> > hardware auto-adjust of terminator and pull-state. Writing a supported
> > baud rate value (one of 9600, 19200, 38400, 57600, 115200, 230400,
> > 460800, or 921600) into the attribute programs the CPLD baud code and
> > runs a temporary AUTO mode cycle (about 2s) on RS485-2W ports of the
> > supported boards. Invalid interfaces or devices are rejected.
> 
> I'm not going to review this, the patch series is already quite big. I
> suggest you to start from the small things in a different series E.g.,
> the first series is just converting MOXA from custom to 8250-based
> (assuming all features are kept working while ABI is being broken,
> which has to be explained in the commit message(s) / cover letter of
> that series), the second one is about splitting 8250_pci to the other
> Moxa case. When this is done, we may go forward with the additional
> features.

Hi Andy,

Thanks a lot for spending so much time reviewing this large patch set.
I appreciate the detailed feedback.

I will split this big patch series into smaller and more focused series
to make it easier to review.

Thanks again for your guidance.

Sincerely,
Crescent Hsieh
Re: [PATCH v1 31/31] serial: 8250_mxpcie: add RS485-2W auto-adjust sysfs control
Posted by Andy Shevchenko 2 months, 1 week ago
On Wed, Dec 3, 2025 at 4:56 AM Crescent Hsieh <crescentcy.hsieh@moxa.com> wrote:
> On Mon, Dec 01, 2025 at 04:45:58AM +0200, Andy Shevchenko wrote:
> > On Sun, Nov 30, 2025 at 12:46 PM Crescent Hsieh
> > <crescentcy.hsieh@moxa.com> wrote:

...

> > I'm not going to review this, the patch series is already quite big. I
> > suggest you to start from the small things in a different series E.g.,
> > the first series is just converting MOXA from custom to 8250-based
> > (assuming all features are kept working while ABI is being broken,

In case you are wondering what I was talking about in the above, I
meant move from /dev/ttyMIxx to /dev/ttySxx. This will break all the
current kernel command lines and hence setups with the explicit
mention of the /dev/ttyMIxx, such as console=.  There might be other
breakages, but I leave it up to you to research and come up with a
solution.

> > which has to be explained in the commit message(s) / cover letter of
> > that series), the second one is about splitting 8250_pci to the other
> > Moxa case. When this is done, we may go forward with the additional
> > features.

> Thanks a lot for spending so much time reviewing this large patch set.
> I appreciate the detailed feedback.

You're welcome!

> I will split this big patch series into smaller and more focused series
> to make it easier to review.
>
> Thanks again for your guidance.

Any time!

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 31/31] serial: 8250_mxpcie: add RS485-2W auto-adjust sysfs control
Posted by Crescent Hsieh 2 months, 1 week ago
On Wed, Dec 03, 2025 at 11:24:58AM +0200, Andy Shevchenko wrote:
> On Wed, Dec 3, 2025 at 4:56 AM Crescent Hsieh <crescentcy.hsieh@moxa.com> wrote:
> > On Mon, Dec 01, 2025 at 04:45:58AM +0200, Andy Shevchenko wrote:
> > > On Sun, Nov 30, 2025 at 12:46 PM Crescent Hsieh
> > > <crescentcy.hsieh@moxa.com> wrote:
> 
> ...
> 
> > > I'm not going to review this, the patch series is already quite big. I
> > > suggest you to start from the small things in a different series E.g.,
> > > the first series is just converting MOXA from custom to 8250-based
> > > (assuming all features are kept working while ABI is being broken,
> 
> In case you are wondering what I was talking about in the above, I
> meant move from /dev/ttyMIxx to /dev/ttySxx. This will break all the
> current kernel command lines and hence setups with the explicit
> mention of the /dev/ttyMIxx, such as console=.  There might be other
> breakages, but I leave it up to you to research and come up with a
> solution.

Just to clarify my intention: the in-tree UPCI serial driver (mxser) has
been unmaintained for years, and my goal is to replace it with a clean
8250-based implementation that preserves reasonable user expectations
while following the upstream serial framework. This will require some
analysis to reconcile the legacy behavior with what upstream expects.

I’d also like to ask about patch ordering. Since the PCIe serial driver
is much simpler to migrate and has minimal user impact, would it be
acceptable to upstream the PCIe conversion first, before the more
complex UPCI transition? I’m happy to follow whichever order makes
review easiest for you.

---
Sincerely,
Crescent Hsieh
Re: [PATCH v1 31/31] serial: 8250_mxpcie: add RS485-2W auto-adjust sysfs control
Posted by Andy Shevchenko 2 months, 1 week ago
On Fri, Dec 5, 2025 at 7:26 AM Crescent Hsieh <crescentcy.hsieh@moxa.com> wrote:
> On Wed, Dec 03, 2025 at 11:24:58AM +0200, Andy Shevchenko wrote:
> > On Wed, Dec 3, 2025 at 4:56 AM Crescent Hsieh <crescentcy.hsieh@moxa.com> wrote:
> > > On Mon, Dec 01, 2025 at 04:45:58AM +0200, Andy Shevchenko wrote:
> > > > On Sun, Nov 30, 2025 at 12:46 PM Crescent Hsieh
> > > > <crescentcy.hsieh@moxa.com> wrote:

...

> > > > I'm not going to review this, the patch series is already quite big. I
> > > > suggest you to start from the small things in a different series E.g.,
> > > > the first series is just converting MOXA from custom to 8250-based
> > > > (assuming all features are kept working while ABI is being broken,
> >
> > In case you are wondering what I was talking about in the above, I
> > meant move from /dev/ttyMIxx to /dev/ttySxx. This will break all the
> > current kernel command lines and hence setups with the explicit
> > mention of the /dev/ttyMIxx, such as console=.  There might be other
> > breakages, but I leave it up to you to research and come up with a
> > solution.
>
> Just to clarify my intention: the in-tree UPCI serial driver (mxser) has
> been unmaintained for years, and my goal is to replace it with a clean
> 8250-based implementation that preserves reasonable user expectations
> while following the upstream serial framework. This will require some
> analysis to reconcile the legacy behavior with what upstream expects.

Right, so it's a good justification to break the above on the
expectations that the users most likely don't use the in-kernel driver
as it's unmaintained and has received no bug fixes, etc.

> I’d also like to ask about patch ordering. Since the PCIe serial driver
> is much simpler to migrate and has minimal user impact, would it be
> acceptable to upstream the PCIe conversion first, before the more
> complex UPCI transition? I’m happy to follow whichever order makes
> review easiest for you.

I don't see the downsides of this order. I believe the order of these
big parts is not so important per se.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 31/31] serial: 8250_mxpcie: add RS485-2W auto-adjust sysfs control
Posted by Andy Shevchenko 2 months, 1 week ago
On Wed, Dec 3, 2025 at 11:24 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Dec 3, 2025 at 4:56 AM Crescent Hsieh <crescentcy.hsieh@moxa.com> wrote:
> > On Mon, Dec 01, 2025 at 04:45:58AM +0200, Andy Shevchenko wrote:
> > > On Sun, Nov 30, 2025 at 12:46 PM Crescent Hsieh
> > > <crescentcy.hsieh@moxa.com> wrote:

...

> > >  while ABI is being broken,
>
> In case you are wondering what I was talking about in the above, I
> meant move from /dev/ttyMIxx to /dev/ttySxx. This will break all the
> current kernel command lines and hence setups with the explicit
> mention of the /dev/ttyMIxx, such as console=.

FWIW, we used to have in the past more than one case like this, so if
it's well justified and customers are aware of the change, it may go
in.

-- 
With Best Regards,
Andy Shevchenko