2022-10-30 20:37:42

by Michael Zaidman

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

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 <[email protected]>
Signed-off-by: Michael Zaidman <[email protected]>
---
drivers/hid/hid-ft260.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

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;
--
2.34.1

2022-10-31 17:39:05

by Enrik Berkhan

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

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