Skip to content

Commit

Permalink
Merge branch 'for-6.8/i2c-hid' into for-linus
Browse files Browse the repository at this point in the history 
- rework of wait-for-reset in order to reduce the need
  for I2C_HID_QUIRK_NO_IRQ_AFTER_RESET qurk; the success rate is now
  50% better, but there are still further improvements to be made (Hans de Goede)
  • Loading branch information
Jiri Kosina committed Jan 8, 2024
2 parents 82a18fc + 7d7a252 commit ff18ab5
Showing 1 changed file with 70 additions and 67 deletions.
137 changes: 70 additions & 67 deletions drivers/hid/i2c-hid/i2c-hid-core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,11 @@
#include "i2c-hid.h"

/* quirks to control the device */
#define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0)
#define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
#define I2C_HID_QUIRK_BOGUS_IRQ BIT(4)
#define I2C_HID_QUIRK_RESET_ON_RESUME BIT(5)
#define I2C_HID_QUIRK_BAD_INPUT_SIZE BIT(6)
#define I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET BIT(7)
#define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(0)
#define I2C_HID_QUIRK_BOGUS_IRQ BIT(1)
#define I2C_HID_QUIRK_RESET_ON_RESUME BIT(2)
#define I2C_HID_QUIRK_BAD_INPUT_SIZE BIT(3)
#define I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET BIT(4)

/* Command opcodes */
#define I2C_HID_OPCODE_RESET 0x01
Expand Down Expand Up @@ -120,8 +119,6 @@ static const struct i2c_hid_quirks {
__u16 idProduct;
__u32 quirks;
} i2c_hid_quirks[] = {
{ USB_VENDOR_ID_WEIDA, HID_ANY_ID,
I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
{ I2C_VENDOR_ID_ITE, I2C_DEVICE_ID_ITE_VOYO_WINPAD_A15,
Expand Down Expand Up @@ -395,8 +392,7 @@ static int i2c_hid_set_power(struct i2c_hid *ihid, int power_state)
* The call will get a return value (EREMOTEIO) but device will be
* triggered and activated. After that, it goes like a normal device.
*/
if (power_state == I2C_HID_PWR_ON &&
ihid->quirks & I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV) {
if (power_state == I2C_HID_PWR_ON) {
ret = i2c_hid_set_power_command(ihid, I2C_HID_PWR_ON);

/* Device was already activated */
Expand Down Expand Up @@ -426,12 +422,23 @@ static int i2c_hid_set_power(struct i2c_hid *ihid, int power_state)
return ret;
}

static int i2c_hid_execute_reset(struct i2c_hid *ihid)
static int i2c_hid_start_hwreset(struct i2c_hid *ihid)
{
size_t length = 0;
int ret;

i2c_hid_dbg(ihid, "resetting...\n");
i2c_hid_dbg(ihid, "%s\n", __func__);

/*
* This prevents sending feature reports while the device is
* being reset. Otherwise we may lose the reset complete
* interrupt.
*/
lockdep_assert_held(&ihid->reset_lock);

ret = i2c_hid_set_power(ihid, I2C_HID_PWR_ON);
if (ret)
return ret;

/* Prepare reset command. Command register goes first. */
*(__le16 *)ihid->cmdbuf = ihid->hdesc.wCommandRegister;
Expand All @@ -444,60 +451,40 @@ static int i2c_hid_execute_reset(struct i2c_hid *ihid)

ret = i2c_hid_xfer(ihid, ihid->cmdbuf, length, NULL, 0);
if (ret) {
dev_err(&ihid->client->dev, "failed to reset device.\n");
goto out;
}

if (ihid->quirks & I2C_HID_QUIRK_NO_IRQ_AFTER_RESET) {
msleep(100);
goto out;
dev_err(&ihid->client->dev,
"failed to reset device: %d\n", ret);
goto err_clear_reset;
}

i2c_hid_dbg(ihid, "%s: waiting...\n", __func__);
if (!wait_event_timeout(ihid->wait,
!test_bit(I2C_HID_RESET_PENDING, &ihid->flags),
msecs_to_jiffies(5000))) {
ret = -ENODATA;
goto out;
}
i2c_hid_dbg(ihid, "%s: finished.\n", __func__);
return 0;

out:
err_clear_reset:
clear_bit(I2C_HID_RESET_PENDING, &ihid->flags);
i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
return ret;
}

static int i2c_hid_hwreset(struct i2c_hid *ihid)
static int i2c_hid_finish_hwreset(struct i2c_hid *ihid)
{
int ret;
int ret = 0;

i2c_hid_dbg(ihid, "%s\n", __func__);

/*
* This prevents sending feature reports while the device is
* being reset. Otherwise we may lose the reset complete
* interrupt.
*/
mutex_lock(&ihid->reset_lock);

ret = i2c_hid_set_power(ihid, I2C_HID_PWR_ON);
if (ret)
goto out_unlock;
i2c_hid_dbg(ihid, "%s: waiting...\n", __func__);

ret = i2c_hid_execute_reset(ihid);
if (ret) {
dev_err(&ihid->client->dev,
"failed to reset device: %d\n", ret);
i2c_hid_set_power(ihid, I2C_HID_PWR_SLEEP);
goto out_unlock;
if (ihid->quirks & I2C_HID_QUIRK_NO_IRQ_AFTER_RESET) {
msleep(100);
clear_bit(I2C_HID_RESET_PENDING, &ihid->flags);
} else if (!wait_event_timeout(ihid->wait,
!test_bit(I2C_HID_RESET_PENDING, &ihid->flags),
msecs_to_jiffies(1000))) {
dev_warn(&ihid->client->dev, "device did not ack reset within 1000 ms\n");
clear_bit(I2C_HID_RESET_PENDING, &ihid->flags);
}
i2c_hid_dbg(ihid, "%s: finished.\n", __func__);

/* At least some SIS devices need this after reset */
if (!(ihid->quirks & I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET))
ret = i2c_hid_set_power(ihid, I2C_HID_PWR_ON);

out_unlock:
mutex_unlock(&ihid->reset_lock);
return ret;
}

Expand Down Expand Up @@ -729,11 +716,10 @@ static int i2c_hid_parse(struct hid_device *hid)
struct i2c_client *client = hid->driver_data;
struct i2c_hid *ihid = i2c_get_clientdata(client);
struct i2c_hid_desc *hdesc = &ihid->hdesc;
char *rdesc = NULL, *use_override = NULL;
unsigned int rsize;
char *rdesc;
int ret;
int tries = 3;
char *use_override;

i2c_hid_dbg(ihid, "entering %s\n", __func__);

Expand All @@ -743,14 +729,15 @@ static int i2c_hid_parse(struct hid_device *hid)
return -EINVAL;
}

mutex_lock(&ihid->reset_lock);
do {
ret = i2c_hid_hwreset(ihid);
ret = i2c_hid_start_hwreset(ihid);
if (ret)
msleep(1000);
} while (tries-- > 0 && ret);

if (ret)
return ret;
goto abort_reset;

use_override = i2c_hid_get_dmi_hid_report_desc_override(client->name,
&rsize);
Expand All @@ -762,8 +749,8 @@ static int i2c_hid_parse(struct hid_device *hid)
rdesc = kzalloc(rsize, GFP_KERNEL);

if (!rdesc) {
dbg_hid("couldn't allocate rdesc memory\n");
return -ENOMEM;
ret = -ENOMEM;
goto abort_reset;
}

i2c_hid_dbg(ihid, "asking HID report descriptor\n");
Expand All @@ -773,23 +760,34 @@ static int i2c_hid_parse(struct hid_device *hid)
rdesc, rsize);
if (ret) {
hid_err(hid, "reading report descriptor failed\n");
kfree(rdesc);
return -EIO;
goto abort_reset;
}
}

/*
* Windows directly reads the report-descriptor after sending reset
* and then waits for resets completion afterwards. Some touchpads
* actually wait for the report-descriptor to be read before signalling
* reset completion.
*/
ret = i2c_hid_finish_hwreset(ihid);
abort_reset:
clear_bit(I2C_HID_RESET_PENDING, &ihid->flags);
mutex_unlock(&ihid->reset_lock);
if (ret)
goto out;

i2c_hid_dbg(ihid, "Report Descriptor: %*ph\n", rsize, rdesc);

ret = hid_parse_report(hid, rdesc, rsize);
if (ret)
dbg_hid("parsing report descriptor failed\n");

out:
if (!use_override)
kfree(rdesc);

if (ret) {
dbg_hid("parsing report descriptor failed\n");
return ret;
}

return 0;
return ret;
}

static int i2c_hid_start(struct hid_device *hid)
Expand Down Expand Up @@ -987,10 +985,15 @@ static int i2c_hid_core_resume(struct i2c_hid *ihid)
* However some ALPS touchpads generate IRQ storm without reset, so
* let's still reset them here.
*/
if (ihid->quirks & I2C_HID_QUIRK_RESET_ON_RESUME)
ret = i2c_hid_hwreset(ihid);
else
if (ihid->quirks & I2C_HID_QUIRK_RESET_ON_RESUME) {
mutex_lock(&ihid->reset_lock);
ret = i2c_hid_start_hwreset(ihid);
if (ret == 0)
ret = i2c_hid_finish_hwreset(ihid);
mutex_unlock(&ihid->reset_lock);
} else {
ret = i2c_hid_set_power(ihid, I2C_HID_PWR_ON);
}

if (ret)
return ret;
Expand Down

0 comments on commit ff18ab5

Please sign in to comment.