diff mbox

[v3] HID:i2c-hid: add a simple quirk to fix device defects

Message ID 1478749633-28185-1-git-send-email-hn.chen@weidahitech.com (mailing list archive)
State New, archived
Headers show

Commit Message

HungNien Chen Nov. 10, 2016, 3:47 a.m. UTC
From: HungNien Chen <hn.chen@weidahitech.com>

Rename i2c_hid_blacklist as i2c_hid_quirks.
Make the syntax changes of macro & for_loop slightly.

Signed-off-by: HungNien Chen <hn.chen@weidahitech.com>
---
 drivers/hid/hid-ids.h         |  5 ++++
 drivers/hid/i2c-hid/i2c-hid.c | 57 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

Comments

Benjamin Tissoires Nov. 10, 2016, 8:39 a.m. UTC | #1
On Nov 10 2016 or thereabouts, hn.chen@weidahitech.com wrote:
> From: HungNien Chen <hn.chen@weidahitech.com>
> 
> Rename i2c_hid_blacklist as i2c_hid_quirks.
> Make the syntax changes of macro & for_loop slightly.
> 
> Signed-off-by: HungNien Chen <hn.chen@weidahitech.com>

Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Kosina Nov. 10, 2016, 8:40 a.m. UTC | #2
On Thu, 10 Nov 2016, hn.chen@weidahitech.com wrote:

> From: HungNien Chen <hn.chen@weidahitech.com>
> 
> Rename i2c_hid_blacklist as i2c_hid_quirks.
> Make the syntax changes of macro & for_loop slightly.
> 
> Signed-off-by: HungNien Chen <hn.chen@weidahitech.com>
> ---
>  drivers/hid/hid-ids.h         |  5 ++++
>  drivers/hid/i2c-hid/i2c-hid.c | 57 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 6cfb5ca..787afdf 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -1033,6 +1033,11 @@
>  #define USB_DEVICE_ID_WALTOP_MEDIA_TABLET_14_1_INCH	0x0500
>  #define USB_DEVICE_ID_WALTOP_SIRIUS_BATTERY_FREE_TABLET	0x0502
>  
> +#define	USB_VENDOR_ID_WEIDA		0x2575
> +#define	USB_DEVICE_ID_WEIDA_8756	0x8756

The patch looks good to me now, thanks for incorporating all the feedback. 
My only remaining question -- why is 0x8756 being added into hid-ids.h, 
but then not actually used for defining a quirk?
HungNien Chen Nov. 10, 2016, 8:59 a.m. UTC | #3
Hi Jiri,

We had a proprietary driver for WDT8756 in /input/touchscreen.
Just thought the product_id might be useful in the future.
Should I remove it out ?

Hn.chen.

-----Original Message-----
From: Jiri Kosina [mailto:jikos@kernel.org] 
Sent: Thursday, November 10, 2016 4:41 PM
To: Hn Chen
Cc: benjamin.tissoires@redhat.com; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org
Subject: Re: [PATCH v3] HID:i2c-hid: add a simple quirk to fix device defects

On Thu, 10 Nov 2016, hn.chen@weidahitech.com wrote:

> From: HungNien Chen <hn.chen@weidahitech.com>
> 
> Rename i2c_hid_blacklist as i2c_hid_quirks.
> Make the syntax changes of macro & for_loop slightly.
> 
> Signed-off-by: HungNien Chen <hn.chen@weidahitech.com>
> ---
>  drivers/hid/hid-ids.h         |  5 ++++
>  drivers/hid/i2c-hid/i2c-hid.c | 57 
> +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 
> 6cfb5ca..787afdf 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -1033,6 +1033,11 @@
>  #define USB_DEVICE_ID_WALTOP_MEDIA_TABLET_14_1_INCH	0x0500
>  #define USB_DEVICE_ID_WALTOP_SIRIUS_BATTERY_FREE_TABLET	0x0502
>  
> +#define	USB_VENDOR_ID_WEIDA		0x2575
> +#define	USB_DEVICE_ID_WEIDA_8756	0x8756

The patch looks good to me now, thanks for incorporating all the feedback. 
My only remaining question -- why is 0x8756 being added into hid-ids.h, but then not actually used for defining a quirk?

--
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Kosina Nov. 10, 2016, 9:15 a.m. UTC | #4
On Thu, 10 Nov 2016, Hn Chen wrote:

> Hi Jiri,
> 
> We had a proprietary driver for WDT8756 in /input/touchscreen.
> Just thought the product_id might be useful in the future.
> Should I remove it out ?

Yeah, please bring it in only if it's actually used in code. I will remove 
it while applying the patch, no need for you to resubmit just because of 
this.
HungNien Chen Nov. 10, 2016, 9:19 a.m. UTC | #5
Hi Jiri,

Thanks !

Hn.chen.

-----Original Message-----
From: Jiri Kosina [mailto:jikos@kernel.org] 
Sent: Thursday, November 10, 2016 5:15 PM
To: Hn Chen
Cc: benjamin.tissoires@redhat.com; dmitry.torokhov@gmail.com; linux-input@vger.kernel.org
Subject: RE: [PATCH v3] HID:i2c-hid: add a simple quirk to fix device defects

On Thu, 10 Nov 2016, Hn Chen wrote:

> Hi Jiri,
> 
> We had a proprietary driver for WDT8756 in /input/touchscreen.
> Just thought the product_id might be useful in the future.
> Should I remove it out ?

Yeah, please bring it in only if it's actually used in code. I will remove it while applying the patch, no need for you to resubmit just because of this.

--
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Kosina Nov. 10, 2016, 9:33 a.m. UTC | #6
Applied to hid.git#for-4.10/i2c-hid

Thanks,
diff mbox

Patch

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 6cfb5ca..787afdf 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1033,6 +1033,11 @@ 
 #define USB_DEVICE_ID_WALTOP_MEDIA_TABLET_14_1_INCH	0x0500
 #define USB_DEVICE_ID_WALTOP_SIRIUS_BATTERY_FREE_TABLET	0x0502
 
+#define	USB_VENDOR_ID_WEIDA		0x2575
+#define	USB_DEVICE_ID_WEIDA_8756	0x8756
+#define	USB_DEVICE_ID_WEIDA_8752	0xC300
+#define	USB_DEVICE_ID_WEIDA_8755	0xC301
+
 #define USB_VENDOR_ID_WISEGROUP		0x0925
 #define USB_DEVICE_ID_SMARTJOY_PLUS	0x0005
 #define USB_DEVICE_ID_SUPER_JOY_BOX_3	0x8888
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index b3ec4f2..b1bce80 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -41,6 +41,11 @@ 
 
 #include <linux/i2c/i2c-hid.h>
 
+#include "../hid-ids.h"
+
+/* quirks to control the device */
+#define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV	BIT(0)
+
 /* flags */
 #define I2C_HID_STARTED		0
 #define I2C_HID_RESET_PENDING	1
@@ -143,6 +148,7 @@  struct i2c_hid {
 	char			*argsbuf;	/* Command arguments buffer */
 
 	unsigned long		flags;		/* device flags */
+	unsigned long		quirks;		/* Various quirks */
 
 	wait_queue_head_t	wait;		/* For waiting the interrupt */
 	struct gpio_desc	*desc;
@@ -154,6 +160,39 @@  struct i2c_hid {
 	struct mutex		reset_lock;
 };
 
+static const struct i2c_hid_quirks {
+	__u16 idVendor;
+	__u16 idProduct;
+	__u32 quirks;
+} i2c_hid_quirks[] = {
+	{ USB_VENDOR_ID_WEIDA, USB_DEVICE_ID_WEIDA_8752,
+		I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
+	{ USB_VENDOR_ID_WEIDA, USB_DEVICE_ID_WEIDA_8755,
+		I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
+	{ 0, 0 }
+};
+
+/*
+ * i2c_hid_lookup_quirk: return any quirks associated with a I2C HID device
+ * @idVendor: the 16-bit vendor ID
+ * @idProduct: the 16-bit product ID
+ *
+ * Returns: a u32 quirks value.
+ */
+static u32 i2c_hid_lookup_quirk(const u16 idVendor, const u16 idProduct)
+{
+	u32 quirks = 0;
+	int n;
+
+	for (n = 0; i2c_hid_quirks[n].idVendor; n++)
+		if (i2c_hid_quirks[n].idVendor == idVendor &&
+		    (i2c_hid_quirks[n].idProduct == (__u16)HID_ANY_ID ||
+		     i2c_hid_quirks[n].idProduct == idProduct))
+			quirks = i2c_hid_quirks[n].quirks;
+
+	return quirks;
+}
+
 static int __i2c_hid_command(struct i2c_client *client,
 		const struct i2c_hid_cmd *command, u8 reportID,
 		u8 reportType, u8 *args, int args_len,
@@ -346,11 +385,27 @@  static int i2c_hid_set_power(struct i2c_client *client, int power_state)
 
 	i2c_hid_dbg(ihid, "%s\n", __func__);
 
+	/*
+	 * Some devices require to send a command to wakeup before power on.
+	 * 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) {
+		ret = i2c_hid_command(client, &hid_set_power_cmd, NULL, 0);
+
+		/* Device was already activated */
+		if (!ret)
+			goto set_pwr_exit;
+	}
+
 	ret = __i2c_hid_command(client, &hid_set_power_cmd, power_state,
 		0, NULL, 0, NULL, 0);
+
 	if (ret)
 		dev_err(&client->dev, "failed to change power setting.\n");
 
+set_pwr_exit:
 	return ret;
 }
 
@@ -1050,6 +1105,8 @@  static int i2c_hid_probe(struct i2c_client *client,
 		 client->name, hid->vendor, hid->product);
 	strlcpy(hid->phys, dev_name(&client->dev), sizeof(hid->phys));
 
+	ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
+
 	ret = hid_add_device(hid);
 	if (ret) {
 		if (ret != -ENODEV)