diff mbox series

[v3,10/12] HID: ft260: wake up device from power saving mode

Message ID 20221030203403.4637-11-michael.zaidman@gmail.com
State New
Headers show
Series HID: ft260: fixes and performance improvements | expand

Commit Message

Michael Zaidman Oct. 30, 2022, 8:34 p.m. UTC 
The FT260 can enter a power saving mode after being idle for longer than
5 seconds.

When being woken up from power saving mode by an I2C write request, it
looks like a possible NACK will not be correctly reported back. As a
workaround, the driver will request an I2C status report before starting
the next I2C transfer if the FT260 has been idle for longer than 4800ms.

Signed-off-by: Enrik Berkhan <Enrik.Berkhan@inka.de>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 drivers/hid/hid-ft260.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Enrik Berkhan Oct. 31, 2022, 5:12 p.m. UTC | #1 
Hi Michael,

On Sun, 2022-10-30 at 22:34 +0200, Michael Zaidman wrote:
> The FT260 can enter a power saving mode after being idle for longer than
> 5 seconds.
> 
> When being woken up from power saving mode by an I2C write request, it
> looks like a possible NACK will not be correctly reported back. As a
> workaround, the driver will request an I2C status report before starting
> the next I2C transfer if the FT260 has been idle for longer than 4800ms.

Unfortunately, I have found issues with this patch as it is. When I
tested from an installation running in KVM, I saw missed NACKs again.

A possible fix seems to be to send the extra status request after the
output report triggering the write. See
https://github.com/MichaelZaidman/hid-ft260/pull/7.

Cheers,
Enrik
diff mbox series

Patch

diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 40fae81386e3..00cbe7693ba0 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -31,6 +31,8 @@  MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
 #define FT260_REPORT_MAX_LENGTH (64)
 #define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4)
 
+#define FT260_WAKEUP_NEEDED_AFTER_MS (4800) /* 5s minus 200ms margin */
+
 /*
  * The ft260 input report format defines 62 bytes for the data payload, but
  * when requested 62 bytes, the controller returns 60 and 2 in separate input
@@ -237,6 +239,7 @@  struct ft260_device {
 	struct completion wait;
 	struct mutex lock;
 	u8 write_buf[FT260_REPORT_MAX_LENGTH];
+	unsigned long need_wakeup_at;
 	u8 *read_buf;
 	u16 read_idx;
 	u16 read_len;
@@ -392,6 +395,12 @@  static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
 	struct ft260_i2c_write_request_report *rep =
 		(struct ft260_i2c_write_request_report *)dev->write_buf;
 
+
+	if (time_is_before_jiffies(dev->need_wakeup_at)) {
+		(void)ft260_xfer_status(dev);
+		ft260_dbg("device wakeup");
+	}
+
 	rep->flag = FT260_FLAG_START;
 
 	do {
@@ -441,6 +450,11 @@  static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
 	if (data_len >= sizeof(rep->data))
 		return -EINVAL;
 
+	if (time_is_before_jiffies(dev->need_wakeup_at)) {
+		(void)ft260_xfer_status(dev);
+		ft260_dbg("device wakeup");
+	}
+
 	rep->address = addr;
 	rep->data[0] = cmd;
 	rep->length = data_len + 1;
@@ -607,6 +621,8 @@  static int ft260_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
 
 	ret = num;
 i2c_exit:
+	dev->need_wakeup_at =
+		jiffies + msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS);
 	hid_hw_power(hdev, PM_HINT_NORMAL);
 	mutex_unlock(&dev->lock);
 	return ret;
@@ -707,6 +723,8 @@  static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags,
 	}
 
 smbus_exit:
+	dev->need_wakeup_at =
+		jiffies + msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS);
 	hid_hw_power(hdev, PM_HINT_NORMAL);
 	mutex_unlock(&dev->lock);
 	return ret;