All of lore.kernel.org
 help / color / mirror / Atom feed
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 07/19] hid-ft260: depend wakeup workaround activation on uart baud rate
Date: Sat, 10 Feb 2024 23:51:35 +0200	[thread overview]
Message-ID: <20240210215147.77629-8-michael.zaidman@gmail.com> (raw)
In-Reply-To: <20240210215147.77629-1-michael.zaidman@gmail.com>

The ft260 chip enters power save mode after 5 seconds of being in idle
state. It causes the data loss on the Rx line. The implemented workaround
sends a dummy report every 4.8 seconds, preventing the chip from entering
the power save mode. But the baud rates below 4800 work fine without a
wakeup workaround.

This commit makes the wakeup workaround activation dependent on the UART
rate, allowing entering the power saving mode for the UART rates below
4800 bauds.

[11238.542841] ft260_uart_wakeup_workaraund_enable: Activate wakeup workaround
[11238.542843] ft260_uart_change_speed: Configured termios: flow control: 0, baudrate: 9600,
[11238.542845] ft260_uart_change_speed: data_bit: 8, parity: 0, stop_bit: 0, breaking: 0

[11238.543671] ft260_uart_wakeup_workaraund_enable: Deactivate wakeup workaround
[11238.543683] ft260_uart_change_speed: Configured termios: flow control: 0, baudrate: 4800,
[11238.543684] ft260_uart_change_speed: data_bit: 8, parity: 0, stop_bit: 0, breaking: 0

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

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 19599e64c6be..b24998092d22 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -296,6 +296,8 @@ enum {
 	FT260_CFG_BAUD_MAX		= 12000000,
 };
 
+#define FT260_UART_EN_PW_SAVE_BAUD	4800
+
 static const struct hid_device_id ft260_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
 			 USB_DEVICE_ID_FT260) },
@@ -1056,6 +1058,15 @@ static const struct attribute_group ft260_attr_group = {
 static DEFINE_MUTEX(ft260_uart_list_lock);
 static LIST_HEAD(ft260_uart_device_list);
 
+static void ft260_uart_wakeup(struct ft260_device *dev);
+
+static void ft260_uart_wakeup_workaraund_enable(struct ft260_device *port,
+						bool enable)
+{
+	port->reschedule_work = enable;
+	ft260_dbg("%s wakeup workaround", enable ? "Activate" : "Deactivate");
+}
+
 static struct ft260_device *ft260_dev_by_index(int index)
 {
 	struct ft260_device *port;
@@ -1108,7 +1119,7 @@ static void ft260_uart_port_remove(struct ft260_device *port)
 	spin_unlock(&port->write_lock);
 
 	mutex_lock(&port->port.mutex);
-	port->reschedule_work = false;
+	ft260_uart_wakeup_workaraund_enable(port, false);
 	tty_port_tty_hangup(&port->port, false);
 	mutex_unlock(&port->port.mutex);
 
@@ -1266,8 +1277,11 @@ static int ft260_uart_change_speed(struct ft260_device *port,
 	struct hid_device *hdev = port->hdev;
 	unsigned int baud;
 	struct ft260_configure_uart_request req;
+	bool wakeup_workaraund = false;
 	int ret;
 
+	ft260_uart_wakeup(port);
+
 	memset(&req, 0, sizeof(req));
 
 	req.report = FT260_SYSTEM_SETTINGS;
@@ -1309,6 +1323,12 @@ static int ft260_uart_change_speed(struct ft260_device *port,
 		tty_encode_baud_rate(tty, baud, baud);
 		tty_kref_put(tty);
 	}
+
+	if (baud > FT260_UART_EN_PW_SAVE_BAUD)
+		wakeup_workaraund = true;
+
+	ft260_uart_wakeup_workaraund_enable(port, wakeup_workaraund);
+
 	put_unaligned_le32(cpu_to_le32(baud), &req.baudrate);
 
 	if (termios->c_cflag & CRTSCTS)
@@ -1435,13 +1455,13 @@ static const struct tty_operations ft260_uart_ops = {
 
 /* The FT260 has a "power saving mode" that causes the device to switch
  * to a 30 kHz oscillator if there's no activity for 5 seconds.
- * Unfortunately this mode can only be disabled by reprogramming
+ * Unfortunately, this mode can only be disabled by reprogramming
  * internal fuses, which requires an additional programming voltage.
  *
- * One effect of this mode is to cause data loss on a fast UART that
- * transmits after being idle for longer than 5 seconds. We work around
- * this by sending a dummy report at least once per 4 seconds if the
- * UART is in use.
+ * One effect of this mode is to cause data loss on an Rx line at baud
+ * rates higher than 4800 after being idle for longer than 5 seconds.
+ * We work around this by sending a dummy report at least once per 4.8
+ * seconds if the UART is in use.
  */
 static void ft260_uart_start_wakeup(struct timer_list *t)
 {
@@ -1455,10 +1475,8 @@ static void ft260_uart_start_wakeup(struct timer_list *t)
 	}
 }
 
-static void ft260_uart_do_wakeup(struct work_struct *work)
+static void ft260_uart_wakeup(struct ft260_device *dev)
 {
-	struct ft260_device *dev =
-		container_of(work, struct ft260_device, wakeup_work);
 	struct ft260_get_chip_version_report version;
 	int ret;
 
@@ -1472,12 +1490,20 @@ static void ft260_uart_do_wakeup(struct work_struct *work)
 	}
 }
 
+static void ft260_uart_do_wakeup(struct work_struct *work)
+{
+	struct ft260_device *dev =
+		container_of(work, struct ft260_device, wakeup_work);
+
+	ft260_uart_wakeup(dev);
+}
+
 static void ft260_uart_shutdown(struct tty_port *tport)
 {
 	struct ft260_device *port =
 		container_of(tport, struct ft260_device, port);
 
-	port->reschedule_work = false;
+	ft260_uart_wakeup_workaraund_enable(port, false);
 }
 
 static int ft260_uart_activate(struct tty_port *tport, struct tty_struct *tty)
@@ -1575,7 +1601,7 @@ static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
 	INIT_WORK(&dev->wakeup_work, ft260_uart_do_wakeup);
 	// FIXME: Do I need that if I have cancel_work_sync?
 	// FIXME: are all kfifo access secured by lock? with irq or not?
-	dev->reschedule_work = true;
+	ft260_uart_wakeup_workaraund_enable(dev, true);
 	/* Work not started at this point */
 	timer_setup(&dev->wakeup_timer, ft260_uart_start_wakeup, 0);
 
-- 
2.40.1


  parent reply	other threads:[~2024-02-10 21:53 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-10 21:51 [PATCH v1 00/19] hid-ft260: Fixes for serial driver patch v4 Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 01/19] hid-ft260: fix incompatible-pointer-types error Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 02/19] hid-ft260: fix Wformat warning Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 03/19] hid-ft260: fix i2c driver regression in ft260_raw_event Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 04/19] hid-ft260: remove dead code in ft260_uart_receive_chars Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 05/19] hid-ft260: fix unprotected write_buf concurrent access Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 06/19] hid-ft260: uart: enable wakeup workaround Michael Zaidman
2024-02-10 21:51 ` Michael Zaidman [this message]
2024-02-10 21:51 ` [PATCH v1 08/19] hid-ft260: depend wakeup workaround activation on eeprom config Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 09/19] hid-ft260: uart: wakeup device early to not lose rx data Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 10/19] hid-ft260: uart: do not configure baud rate twice Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 11/19] hid-ft260: uart: do not disable wakeup workaround twice Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 12/19] hid-ft260: uart: use kfifo_avail for fifo write room check Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 13/19] hid-ft260: improve usb interface type detection logic Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 14/19] hid-ft260: uart: cleanup and refactoring Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 15/19] hid-ft260: uart: remove FIXME for wake-up workaround Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 16/19] hid-ft260: uart: suppress unhandled report 0xb1 dmesg Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 17/19] hid-ft260: uart: arm wake-up timer unconditionally on tty session start Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 18/19] hid-ft260: uart: fix rx data loss after device reopening Michael Zaidman
2024-02-10 21:51 ` [PATCH v1 19/19] hid-ft260: uart: improve write performance Michael Zaidman
2024-02-23 21:22   ` Michael Zaidman
2024-02-13 10:20 ` [PATCH v1 00/19] hid-ft260: Fixes for serial driver patch v4 Jiri Kosina
2024-02-23 13:07   ` Christina Quast

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240210215147.77629-8-michael.zaidman@gmail.com \
    --to=michael.zaidman@gmail.com \
    --cc=chrysh@christina-quast.de \
    --cc=daniel.beer@igorinstitute.com \
    --cc=equinox@diac24.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=jikos@kernel.org \
    --cc=johan@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.