Received-SPF: pass (google.com: domain of linux-kernel+bounces-60565-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Michael Zaidman <michael.zaidman@gmail.com>
To: chrysh@christina-quast.de,
	daniel.beer@igorinstitute.com,
	jikos@kernel.org
Cc: linux-kernel@vger.kernel.org,
	linux-input@vger.kernel.org,
	linux-serial@vger.kernel.org,
	ilpo.jarvinen@linux.intel.com,
	johan@kernel.org,
	gregkh@linuxfoundation.org,
	equinox@diac24.net,
	michael.zaidman@gmail.com
Subject: [PATCH v1 18/19] hid-ft260: uart: fix rx data loss after device reopening
Date: Sat, 10 Feb 2024 23:51:46 +0200
Message-Id: <20240210215147.77629-19-michael.zaidman@gmail.com>
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>
References: <20240210215147.77629-1-michael.zaidman@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

The port setting may remain intact after session termination. Then, when
reopening the port without configuring the port setting, the wakeup
mechanism is not activated, and UART Rx loses the data at speeds greater
than 4800 bauds. To fix it, retrieve the baud rate from the device in the
ft260_uart_port_activate to reactivate the wakeup workaround if needed.

Example:

1. Configure the baud rate to 115200:

   $ sudo picocom -f n -p n -d 8 -b 115200 /dev/ttyFT0

2. Quit the picocom without resetting port setting by entering:
   C-a and C-q

It deactivates the wakeup workaround:

[31677.005464] ft260_uart_close:
[31677.005466] ft260_uart_chars_in_buffer:
[31677.005467] ft260_uart_port_shutdown:
[31677.005468] ft260_uart_wakeup_workaraund_enable: deactivate wakeup workaround
[31677.005476] ft260_uart_cleanup:
[31677.005477] ft260_uart_port_put:

3. Reopen the ttyFT0 port, but do not configure it. The fix retrieves the port
   baud rate from the device, compares it against the threshold, and activates
   the wakeup mechanism when needed.

$ sudo bash -c "cat < /dev/ttyFT0"

[31693.304991] ft260_uart_port_get:
[31693.304995] ft260_uart_install:
[31693.305000] ft260_uart_open:
[31693.305001] ft260_uart_port_activate:
[31693.309842] ft260_uart_wakeup_workaraund_enable: activate wakeup workaround
[31693.309847] ft260_uart_port_activate: configurd baudrate = 115200
[31698.132650] ft260_uart_start_wakeup:
[31698.132809] ft260_uart_do_wakeup:
[31698.132817] ft260_uart_wakeup:
[31702.996905] ft260_uart_start_wakeup:
[31702.996916] ft260_uart_do_wakeup:

Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 drivers/hid/hid-ft260.c | 56 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 47 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 7f3ef4f20075..6b172bfa4f98 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -90,7 +90,7 @@ enum {
 	FT260_I2C_REPORT_MAX		= 0xDE,
 	FT260_GPIO			= 0xB0,
 	FT260_UART_INTERRUPT_STATUS	= 0xB1,
-	FT260_UART_STATUS		= 0xE0,
+	FT260_UART_SETTINGS		= 0xE0,
 	FT260_UART_RI_DCD_STATUS	= 0xE1,
 	FT260_UART_REPORT_MIN		= 0xF0,
 	FT260_UART_REPORT_MAX		= 0xFE,
@@ -190,6 +190,18 @@ struct ft260_get_i2c_status_report {
 	u8 reserved;
 } __packed;
 
+struct ft260_get_uart_settings_report {
+	u8 report;		/* FT260_UART_SETTINGS */
+	u8 flow_ctrl;		/* 0 - OFF; 1 - RTS_CTS, 2 - DTR_DSR, */
+				/* 3 - XON_XOFF, 4 - No flow control */
+	/* The baudrate field is unaligned */
+	__le32 baudrate;	/* little endian, 9600 = 0x2580, 19200 = 0x4B00 */
+	u8 data_bit;		/* 7 or 8 */
+	u8 parity;		/* 0: no parity, 1: odd, 2: even, 3: high, 4: low */
+	u8 stop_bit;		/* 0: one stop bit, 2: 2 stop bits */
+	u8 breaking;		/* 0: no break */
+} __packed;
+
 /* Feature Out reports */
 
 struct ft260_set_system_clock_report {
@@ -1050,6 +1062,21 @@ static LIST_HEAD(ft260_uart_device_list);
 
 static void ft260_uart_wakeup(struct ft260_device *dev);
 
+static int ft260_get_uart_settings(struct hid_device *hdev,
+				   struct ft260_get_uart_settings_report *cfg)
+{
+	int ret;
+	int len = sizeof(struct ft260_get_uart_settings_report);
+
+	ret = ft260_hid_feature_report_get(hdev, FT260_UART_SETTINGS,
+					   (u8 *)cfg, len);
+	if (ret < 0) {
+		hid_err(hdev, "failed to retrieve uart settings\n");
+		return ret;
+	}
+	return 0;
+}
+
 static void ft260_uart_wakeup_workaraund_enable(struct ft260_device *port,
 						bool enable)
 {
@@ -1492,13 +1519,11 @@ static void ft260_uart_port_shutdown(struct tty_port *tport)
 
 static int ft260_uart_port_activate(struct tty_port *tport, struct tty_struct *tty)
 {
-	struct ft260_device *port =
-		container_of(tport, struct ft260_device, port);
+	int ret;
+	int baudrate;
+	struct ft260_get_uart_settings_report cfg;
+	struct ft260_device *port = container_of(tport, struct ft260_device, port);
 
-	/*
-	 * Set the TTY IO error marker - we will only clear this
-	 * once we have successfully opened the port.
-	 */
 	set_bit(TTY_IO_ERROR, &tty->flags);
 
 	spin_lock(&port->xmit_fifo_lock);
@@ -1507,8 +1532,21 @@ static int ft260_uart_port_activate(struct tty_port *tport, struct tty_struct *t
 
 	clear_bit(TTY_IO_ERROR, &tty->flags);
 
-	/* Wake up the chip as early as possible to not miss incoming data */
-	ft260_uart_wakeup(port);
+	/*
+	 * The port setting may remain intact after session termination.
+	 * Then, when reopening the port without configuring the port
+	 * setting, we need to retrieve the baud rate from the device to
+	 * reactivate the wakeup workaround if needed.
+	 */
+	ret = ft260_get_uart_settings(port->hdev, &cfg);
+	if (ret)
+		return ret;
+
+	baudrate = get_unaligned_le32(&cfg.baudrate);
+	if (baudrate > FT260_UART_EN_PW_SAVE_BAUD)
+		ft260_uart_wakeup_workaraund_enable(port, true);
+
+	ft260_dbg("configurd baudrate = %d", baudrate);
 
 	mod_timer(&port->wakeup_timer, jiffies +
 		  msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
-- 
2.40.1