Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] HID: corsair: Add support for the GLAIVE RGB gaming mouse
From: Guus Sliepen @ 2018-02-10 17:12 UTC (permalink / raw)
  To: linux-input, linux-kernel, jikos; +Cc: Guus Sliepen

This mouse sold by Corsair as the GLAIVE RGB gaming mouse has the same
problem with its HID reports as the Scimitar PRO RGB, so reuse the
same fix for the GLAIVE RGB.

Signed-off-by: Guus Sliepen <guus@sliepen.org>
---
 drivers/hid/hid-corsair.c | 16 ++++++++++------
 drivers/hid/hid-ids.h     |  1 +
 drivers/hid/hid-quirks.c  |  2 ++
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
index 9ba5d98a1..6c9c8fe1b 100644
--- a/drivers/hid/hid-corsair.c
+++ b/drivers/hid/hid-corsair.c
@@ -673,7 +673,7 @@ static int corsair_input_mapping(struct hid_device *dev,
 }
 
 /*
- * The report descriptor of Corsair Scimitar RGB Pro gaming mouse is
+ * The report descriptor of some of the Corsair gaming mice is
  * non parseable as they define two consecutive Logical Minimum for
  * the Usage Page (Consumer) in rdescs bytes 75 and 77 being 77 0x16
  * that should be obviousy 0x26 for Logical Magimum of 16 bits. This
@@ -681,7 +681,8 @@ static int corsair_input_mapping(struct hid_device *dev,
  * Minimum being larger than Logical Maximum.
  *
  * This driver fixes the report descriptor for:
- * - USB ID b1c:1b3e, sold as Scimitar RGB Pro Gaming mouse
+ * - USB ID 1b1c:1b34, sold as GLAIVE RGB Gaming mouse
+ * - USB ID 1b1c:1b3e, sold as Scimitar RGB Pro Gaming mouse
  */
 
 static __u8 *corsair_mouse_report_fixup(struct hid_device *hdev, __u8 *rdesc,
@@ -691,13 +692,14 @@ static __u8 *corsair_mouse_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 
 	if (intf->cur_altsetting->desc.bInterfaceNumber == 1) {
 		/*
-		 * Corsair Scimitar RGB Pro report descriptor is broken and
-		 * defines two different Logical Minimum for the Consumer
-		 * Application. The byte 77 should be a 0x26 defining a 16
-		 * bits integer for the Logical Maximum but it is a 0x16
+		 * Corsair GLAIVE RGB and Scimitar RGB Pro report descriptor is
+		 * broken and defines two different Logical Minimum for the
+		 * Consumer Application. The byte 77 should be a 0x26 defining
+		 * a 16 bits integer for the Logical Maximum but it is a 0x16
 		 * instead (Logical Minimum)
 		 */
 		switch (hdev->product) {
+		case USB_DEVICE_ID_CORSAIR_GLAIVE_RGB:
 		case USB_DEVICE_ID_CORSAIR_SCIMITAR_PRO_RGB:
 			if (*rsize >= 172 && rdesc[75] == 0x15 && rdesc[77] == 0x16
 			&& rdesc[78] == 0xff && rdesc[79] == 0x0f) {
@@ -715,6 +717,8 @@ static const struct hid_device_id corsair_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K90),
 		.driver_data = CORSAIR_USE_K90_MACRO |
 			       CORSAIR_USE_K90_BACKLIGHT },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR,
+            USB_DEVICE_ID_CORSAIR_GLAIVE_RGB) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR,
             USB_DEVICE_ID_CORSAIR_SCIMITAR_PRO_RGB) },
 	{}
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 43ddcdfbd..a099e5194 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -291,6 +291,7 @@
 #define USB_DEVICE_ID_CORSAIR_K70RGB    0x1b13
 #define USB_DEVICE_ID_CORSAIR_STRAFE    0x1b15
 #define USB_DEVICE_ID_CORSAIR_K65RGB    0x1b17
+#define USB_DEVICE_ID_CORSAIR_GLAIVE_RGB        0x1b34
 #define USB_DEVICE_ID_CORSAIR_K70RGB_RAPIDFIRE  0x1b38
 #define USB_DEVICE_ID_CORSAIR_K65RGB_RAPIDFIRE  0x1b39
 #define USB_DEVICE_ID_CORSAIR_SCIMITAR_PRO_RGB  0x1b3e
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 5f6035a5c..7203eba3c 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -62,6 +62,7 @@ static const struct hid_device_id hid_quirks[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K70R), HID_QUIRK_NO_INIT_REPORTS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K95RGB), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_M65RGB), HID_QUIRK_NO_INIT_REPORTS },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_GLAIVE_RGB), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_SCIMITAR_PRO_RGB), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_STRAFE), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51), HID_QUIRK_NOGET },
@@ -317,6 +318,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 #endif
 #if IS_ENABLED(CONFIG_HID_CORSAIR)
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K90) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_GLAIVE_RGB) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_SCIMITAR_PRO_RGB) },
 #endif
 #if IS_ENABLED(CONFIG_HID_CP2112)
-- 
2.16.1


^ permalink raw reply related

* Re: Software evolution around “checkpatch.pl”?
From: Jonathan Cameron @ 2018-02-10 15:57 UTC (permalink / raw)
  To: Joe Perches
  Cc: SF Markus Elfring, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
	Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1518274783.6579.2.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>

On Sat, 10 Feb 2018 06:59:43 -0800
Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote:

> On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> > While it would be great to improve checkpatches false
> > positive rate, it's very nature as a string matcher makes
> > this hard.  
> 
> true.
> 
> what are the false positives you see?
> 
This particular case is only 'sort of' a false positive
in the warning that a message printed on a memory allocation
failure 'may' not add any information over the generic case.

Very hard to judge on whether it is useful to know more than
an allocation failed somewhere or not.

Message makes this clear:
>“WARNING: Possible unnecessary 'out of memory' message”
>(from the script “checkpatch.pl”)  

We also have the balance between any changes to existing code
adding 'some' maintenance overhead vs changing this stuff
in a new driver - which is what checkpatch is really intended
for.

So I think checkpatch is striking the right balance here in
how it warns.  Obviously if it could assess the text
and come to an informed decision that would be great but
we are some way from that ;)

Jonathan

^ permalink raw reply

* Re: Software evolution around “checkpatch.pl”?
From: Joe Perches @ 2018-02-10 14:59 UTC (permalink / raw)
  To: Jonathan Cameron, SF Markus Elfring
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
	Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20180210145336.233c721d@archlinux>

On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> While it would be great to improve checkpatches false
> positive rate, it's very nature as a string matcher makes
> this hard.

true.

what are the false positives you see?

^ permalink raw reply

* Re: Software evolution around “checkpatch.pl”?
From: Jonathan Cameron @ 2018-02-10 14:53 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: linux-iio, linux-input, Jonathan Cameron, Hartmut Knaack,
	Jiri Kosina, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Srinivas Pandruvada, LKML, kernel-janitors
In-Reply-To: <f008911b-c595-6e0f-d85f-076e994ae7db@users.sourceforge.net>

On Tue, 6 Feb 2018 09:45:48 +0100
SF Markus Elfring <elfring@users.sourceforge.net> wrote:

> > Check patch is dumb and often gives false positives.  
> 
> Would you like to improve this software situation anyhow?

While it would be great to improve checkpatches false
positive rate, it's very nature as a string matcher makes
this hard.

Jonathan

> 
> Regards,
> Markus


^ permalink raw reply

* [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action
From: Jeffy Chen @ 2018-02-10 11:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: briannorris, dtor, dianders, Jeffy Chen, Enric Balletbo i Serra,
	Thomas Gleixner, Joseph Lo, stephen lu, Dmitry Torokhov,
	Kate Stewart, linux-input, Greg Kroah-Hartman,
	Philippe Ombredanne, Arvind Yadav
In-Reply-To: <20180210110907.5504-1-jeffy.chen@rock-chips.com>

Add support for specifying event actions to trigger wakeup when using
the gpio-keys input device as a wakeup source.

This would allow the device to configure when to wakeup the system. For
example a gpio-keys input device for pen insert, may only want to wakeup
the system when ejecting the pen.

Suggested-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v2:
Specify wakeup event action instead of irq trigger type as Brian
suggested.

 drivers/input/keyboard/gpio_keys.c     | 27 +++++++++++++++++++++++++++
 include/linux/gpio_keys.h              |  2 ++
 include/uapi/linux/input-event-codes.h |  9 +++++++++
 3 files changed, 38 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 87e613dc33b8..5c57339d3999 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -45,6 +45,8 @@ struct gpio_button_data {
 	unsigned int software_debounce;	/* in msecs, for GPIO-driven buttons */
 
 	unsigned int irq;
+	unsigned int irq_trigger_type;
+	unsigned int wakeup_trigger_type;
 	spinlock_t lock;
 	bool disabled;
 	bool key_pressed;
@@ -540,6 +542,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 	}
 
 	if (bdata->gpiod) {
+		int active_low = gpiod_is_active_low(bdata->gpiod);
+
 		if (button->debounce_interval) {
 			error = gpiod_set_debounce(bdata->gpiod,
 					button->debounce_interval * 1000);
@@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 		isr = gpio_keys_gpio_isr;
 		irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
 
+		switch (button->wakeup_event_action) {
+		case EV_ACT_ASSERTED:
+			bdata->wakeup_trigger_type = active_low ?
+				IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
+			break;
+		case EV_ACT_DEASSERTED:
+			bdata->wakeup_trigger_type = active_low ?
+				IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
+			break;
+		}
 	} else {
 		if (!button->irq) {
 			dev_err(dev, "Found button without gpio or irq\n");
@@ -618,6 +632,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 		return error;
 	}
 
+	bdata->irq_trigger_type = irq_get_trigger_type(bdata->irq);
+
 	return 0;
 }
 
@@ -718,6 +734,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 			/* legacy name */
 			fwnode_property_read_bool(child, "gpio-key,wakeup");
 
+		fwnode_property_read_u32(child, "wakeup-event-action",
+					 &button->wakeup_event_action);
+
 		button->can_disable =
 			fwnode_property_read_bool(child, "linux,can-disable");
 
@@ -854,6 +873,10 @@ static int __maybe_unused gpio_keys_suspend(struct device *dev)
 	if (device_may_wakeup(dev)) {
 		for (i = 0; i < ddata->pdata->nbuttons; i++) {
 			struct gpio_button_data *bdata = &ddata->data[i];
+
+			if (bdata->button->wakeup && bdata->wakeup_trigger_type)
+				irq_set_irq_type(bdata->irq,
+						 bdata->wakeup_trigger_type);
 			if (bdata->button->wakeup)
 				enable_irq_wake(bdata->irq);
 			bdata->suspended = true;
@@ -878,6 +901,10 @@ static int __maybe_unused gpio_keys_resume(struct device *dev)
 	if (device_may_wakeup(dev)) {
 		for (i = 0; i < ddata->pdata->nbuttons; i++) {
 			struct gpio_button_data *bdata = &ddata->data[i];
+
+			if (bdata->button->wakeup && bdata->wakeup_trigger_type)
+				irq_set_irq_type(bdata->irq,
+						 bdata->irq_trigger_type);
 			if (bdata->button->wakeup)
 				disable_irq_wake(bdata->irq);
 			bdata->suspended = false;
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index d06bf77400f1..7160df54a6fe 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -13,6 +13,7 @@ struct device;
  * @desc:		label that will be attached to button's gpio
  * @type:		input event type (%EV_KEY, %EV_SW, %EV_ABS)
  * @wakeup:		configure the button as a wake-up source
+ * @wakeup_event_action:	event action to trigger wakeup
  * @debounce_interval:	debounce ticks interval in msecs
  * @can_disable:	%true indicates that userspace is allowed to
  *			disable button via sysfs
@@ -26,6 +27,7 @@ struct gpio_keys_button {
 	const char *desc;
 	unsigned int type;
 	int wakeup;
+	int wakeup_event_action;
 	int debounce_interval;
 	bool can_disable;
 	int value;
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 53fbae27b280..d7917b0bd438 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -32,6 +32,15 @@
 #define INPUT_PROP_CNT			(INPUT_PROP_MAX + 1)
 
 /*
+ * Event action types
+ */
+#define EV_ACT_ANY			0x00	/* asserted or deasserted */
+#define EV_ACT_ASSERTED			0x01	/* asserted */
+#define EV_ACT_DEASSERTED		0x02	/* deasserted */
+#define EV_ACT_MAX			0x02
+#define EV_ACT_CNT			(EV_ACT_MAX+1)
+
+/*
  * Event types
  */
 
-- 
2.11.0



^ permalink raw reply related

* [PATCH v2 2/3] Input: gpio-keys - allow setting wakeup event action in DT
From: Jeffy Chen @ 2018-02-10 11:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: briannorris, dtor, dianders, Jeffy Chen, devicetree, Rob Herring,
	Dmitry Torokhov, linux-input, Mark Rutland
In-Reply-To: <20180210110907.5504-1-jeffy.chen@rock-chips.com>

Allow specifying event actions to trigger wakeup when using the
gpio-keys input device as a wakeup source.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v2:
Specify wakeup event action instead of irq trigger type as Brian
suggested.

 Documentation/devicetree/bindings/input/gpio-keys.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt
index a94940481e55..996ce84352cb 100644
--- a/Documentation/devicetree/bindings/input/gpio-keys.txt
+++ b/Documentation/devicetree/bindings/input/gpio-keys.txt
@@ -26,6 +26,14 @@ Optional subnode-properties:
 	  If not specified defaults to 5.
 	- wakeup-source: Boolean, button can wake-up the system.
 			 (Legacy property supported: "gpio-key,wakeup")
+	- wakeup-event-action: Specifies whether the key should wake the
+	  system when asserted, when deasserted, or both. This property is
+	  only valid for keys that wake up the system (e.g., when the
+	  "wakeup-source" property is also provided).
+	  Supported values are defined in linux-event-codes.h:
+		EV_ACT_ASSERTED		- asserted
+		EV_ACT_DEASSERTED	- deasserted
+		EV_ACT_ANY		- both asserted and deasserted
 	- linux,can-disable: Boolean, indicates that button is connected
 	  to dedicated (not shared) interrupt which can be disabled to
 	  suppress events from the button.
-- 
2.11.0

^ permalink raw reply related

* [PATCH v2 0/3] gpio-keys: Add support for specifying wakeup event action
From: Jeffy Chen @ 2018-02-10 11:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: briannorris, dtor, dianders, Jeffy Chen, Arnd Bergmann, Joseph Lo,
	Rob Herring, Catalin Marinas, Emil Renner Berthing,
	Enric Balletbo i Serra, Heiko Stuebner, Brian Norris,
	Thomas Gleixner, Philippe Ombredanne, linux-rockchip,
	Kate Stewart, linux-input, Will Deacon, Matthias Kaehlcke,
	devicetree, stephen lu, Greg Kroah-Hartman


On chromebook kevin, we are using gpio-keys for pen insert event. But
we only want it to wakeup the system when ejecting the pen.

So we may need to change the interrupt trigger type during suspending.

Changes in v2:
Specify wakeup event action instead of irq trigger type as Brian
suggested.

Jeffy Chen (3):
  Input: gpio-keys - add support for wakeup event action
  Input: gpio-keys - allow setting wakeup event action in DT
  arm64: dts: rockchip: Avoid wakeup when inserting the pen

 .../devicetree/bindings/input/gpio-keys.txt        |  8 +++++++
 arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts  |  2 ++
 drivers/input/keyboard/gpio_keys.c                 | 27 ++++++++++++++++++++++
 include/linux/gpio_keys.h                          |  2 ++
 include/uapi/linux/input-event-codes.h             |  9 ++++++++
 5 files changed, 48 insertions(+)

-- 
2.11.0

^ permalink raw reply

* UHID Device Reports Do Not Generate OS Activity
From: R0b0t1 @ 2018-02-10  9:33 UTC (permalink / raw)
  To: linux-input, linux-usb

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

Hello,

I am trying to simulate a keyboard using the uhid interface. The fake
device shows up in dmesg, but attempting to press the "a" key does
nothing I can see. No input appears in the active terminal, nor in an
xev window.

The program receives UHID_OPEN, UHID_START, and UHID_CLOSE.

Many different report descriptors were tried. The one currently
uncommented is the one distributed with the USB implementor forum's
HID report descriptor tool. The other is one found in a description of
a boot device compatible keyboard.

Thanks in advance,
     R0b0t1

[-- Attachment #2: kbd.c --]
[-- Type: text/x-csrc, Size: 4931 bytes --]

#define _GNU_SOURCE
#include <stdlib.h>
#include <stdio.h>
#include <stdint.h>
#include <unistd.h>
#include <string.h>

#include <errno.h>
#include <fcntl.h>
#include <poll.h>

#include <sys/ioctl.h>
#include <linux/uhid.h>

#define handle_error(m) \
	do { perror(m); exit(EXIT_FAILURE); } while (0)

static unsigned char rdesc[] = {
	0x05, 0x01,
	0x09, 0x06,
	0xa1, 0x01,

	0x05, 0x07,
	0x19, 0xe0,
	0x29, 0xe7,
	0x15, 0x00,
	0x25, 0x01,
	0x75, 0x01,
	0x95, 0x08,
	0x81, 0x02,

	0x95, 0x01,
	0x75, 0x08,
	0x81, 0x03,

	0x95, 0x05,
	0x75, 0x01,
	0x05, 0x08,
	0x19, 0x01,
	0x29, 0x05,
	0x91, 0x02,

	0x95, 0x01,
	0x75, 0x03,
	0x91, 0x03,

	0x95, 0x06,
	0x75, 0x08,
	0x15, 0x00,
	0x25, 0x65,
	0x05, 0x07,
	0x19, 0x00,
	0x29, 0x65,
	0x81, 0x00,

	0xc0
};

/*
static unsigned char rdesc[] = {
	0x05, 0x01, // Usage Page (Generic Desktop)
	0x09, 0x06, // Usage (Keyboard)

	0xa1, 0x01, // Collection (Application)
	
	// Modifier report.
	0x75, 0x01, // Report Size (1)
	0x95, 0x08, // Report Count (8)
	0x05, 0x07, // Usage Page (Key Codes)
	0x19, 0xe0, // Usage Minimum (224)
	0x29, 0xe7, // Usage Maximum (231)
	0x15, 0x00, // Logical Minimum (0) - Shorten (0x14)?
	0x25, 0x01, // Logical Maximum (1)
	0x81, 0x02, // Input (Data, Variable, Absolute)

	// Reserved byte.
	0x95, 0x01, // Report Count (1)
	0x75, 0x08, // Report Size (8)
	0x81, 0x01, // Input (Constant)

	// LED report.
	0x95, 0x05, // Report Count (5)
	0x75, 0x01, // Report Size (1)
	0x05, 0x08, // Usage Page (LEDs)
	0x19, 0x01, // Usage Minimum (1)
	0x29, 0x05, // Usage Maximum (5)
	0x91, 0x02, // Output (Data, Variable, Absolute)

	// LED report padding.
	0x95, 0x01, // Report Count (1)
	0x75, 0x03, // Report Size (3)
	0x91, 0x01, // Output (Constant)

	// Key code report.
	0x95, 0x06, // Report Count (6)
	0x75, 0x08, // Report Size (8)
	0x15, 0x00, // Logical Minimum (0)
	0x26, 0xff, 0x00, // Logical Maximum (255)
	0x05, 0x07, // Usage Page (Key Codes)
	0x19, 0x00, // Usage Minimum (0)
	0x29, 0xff, // Usage Maximum (255)
	0x81, 0x00, // Input (Data, Array)

	0xc0		// End Collection
};
*/


static int
uhid_write(int fd, const struct uhid_event *he)
{
	ssize_t r = write(fd, he, sizeof(*he));
	if (r < 0) {
		fprintf(stderr, "write: cannot write to uhid: %m\n");
		return -errno;
	} else if (r != sizeof(*he)) {
		fprintf(stderr, "write: wrong size written to uhid: %ld != %lu\n",
				r, sizeof(*he));
		return -EFAULT;
	} else return 0;
}

static int
uhid_event(int fd)
{
	struct uhid_event he;
	ssize_t sz;
	if ((sz = read(fd, &he, sizeof(he))) == -1)
		handle_error("read");

	switch (he.type) {
	case UHID_START:
		fprintf(stderr, "UHID_START from uhid-dev.\n");
		break;
	case UHID_STOP:
		fprintf(stderr, "UHID_STOP from uhid-dev.\n");
		break;
	case UHID_OPEN:
		fprintf(stderr, "UHID_OPEN from uhid-dev.\n");
		return 0;
	case UHID_CLOSE:
		fprintf(stderr, "UHID_CLOSE from uhid-dev.\n");
		break;
	case UHID_OUTPUT:
		fprintf(stderr, "UHID_OUTPUT from uhid-dev.\n");
		// TODO: Handle output.
		break;
	case UHID_OUTPUT_EV:
		fprintf(stderr, "UHID_OUTPUT_EV from uhid-dev.\n");
		break;
	default:
		fprintf(stderr, "Invalid event from uhid-dev: %u.\n", he.type);
	}

	return 1;
}

int
main(int argc, char *argv[])
{
	int fd;
	ssize_t sz;
	struct uhid_event he;

	if ((fd = open("/dev/uhid", O_RDWR | O_CLOEXEC)) == -1)
		handle_error("open");

	struct pollfd fds[1] = {
		{.fd = fd, .events = POLLIN}
	};

	memset(&he, 0, sizeof(he));
	he.type = UHID_CREATE2;
	strcpy((char *)he.u.create.name, "kb-daemon-device");
	memcpy(he.u.create2.rd_data, rdesc, sizeof(rdesc));
	he.u.create2.rd_size = sizeof(rdesc);
	he.u.create2.bus = BUS_USB;
	he.u.create2.vendor = 0x15d9;
	he.u.create2.product = 0x0a37;
	he.u.create2.version = 0;
	he.u.create2.country = 0;
	if (uhid_write(fd, &he))
		handle_error("uhid_write");

	char cont = 1;
	while (cont) {
		int n = ppoll(fds, sizeof(fds), NULL, NULL);
		if (n == -1)
			handle_error("ppoll");

		for (int i = 0; i < n; i++) {
			if (fds[i].fd == fd &&
				fds[i].revents & POLLIN)
				cont = uhid_event(fds[i].fd);
		}
	}

	memset(&he, 0, sizeof(he));
	he.type = UHID_INPUT2;
	he.u.input2.size = 8;
	he.u.input2.data[0] = 0x00;
	he.u.input2.data[1] = 0x00;
	he.u.input2.data[2] = 0x04;
	he.u.input2.data[3] = 0x00;
	he.u.input2.data[4] = 0x00;
	he.u.input2.data[5] = 0x00;
	he.u.input2.data[6] = 0x00;
	he.u.input2.data[7] = 0x00;
	if (uhid_write(fd, &he))
		handle_error("uhid_write");

	memset(&he, 0, sizeof(he));
	he.type = UHID_INPUT2;
	he.u.input2.size = 8;
	he.u.input2.data[0] = 0x00;
	he.u.input2.data[1] = 0x00;
	he.u.input2.data[2] = 0x00;
	he.u.input2.data[3] = 0x00;
	he.u.input2.data[4] = 0x00;
	he.u.input2.data[5] = 0x00;
	he.u.input2.data[6] = 0x00;
	he.u.input2.data[7] = 0x00;
	if (uhid_write(fd, &he))
		handle_error("uhid_write");
	
	memset(&he, 0, sizeof(he));
	he.type = UHID_DESTROY;
	if (uhid_write(fd, &he))
		handle_error("uhid_write");
	uhid_event(fd);


	return EXIT_SUCCESS;
}

^ permalink raw reply

* Re: [PATCH 2/3] Input: gpio-keys - allow setting wakeup interrupt trigger type in DT
From: JeffyChen @ 2018-02-10  0:06 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	briannorris-hpIqsD4AKlfQT0dZR+AlfA, dtor-hpIqsD4AKlfQT0dZR+AlfA,
	dianders-hpIqsD4AKlfQT0dZR+AlfA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Dmitry Torokhov,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Mark Rutland
In-Reply-To: <20180209234246.GA252335-1WoqFLEneaORBCj4nEdE8WJtCfot02Oa@public.gmane.org>

Hi Brian,

Thanks for your reply.

On 02/10/2018 07:42 AM, Brian Norris wrote:
> Hi Jeffy,
>
> On Fri, Feb 09, 2018 at 07:55:09PM +0800, Jeffy Chen wrote:
>> Allow specifying a different interrupt trigger type for wakeup when
>> using the gpio-keys input device as a wakeup source.
>>
>> Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> ---
>>
>>   Documentation/devicetree/bindings/input/gpio-keys.txt | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt
>> index a94940481e55..61926cef708f 100644
>> --- a/Documentation/devicetree/bindings/input/gpio-keys.txt
>> +++ b/Documentation/devicetree/bindings/input/gpio-keys.txt
>> @@ -26,6 +26,15 @@ Optional subnode-properties:
>>   	  If not specified defaults to 5.
>>   	- wakeup-source: Boolean, button can wake-up the system.
>>   			 (Legacy property supported: "gpio-key,wakeup")
>> +	- wakeup-trigger-type: Specifies the interrupt trigger type for wakeup.
>> +		 The value is defined in <dt-bindings/interrupt-controller/irq.h>
>
> Do you really want to codify interrupt triggers here? It seems like most
> of the information about edge vs. level is already codified elsewhere,
> so this becomes a little redundant. And in fact, some bindings may be
> specifying a "gpio", not technically an interrupt (at least not
> directly), so it feels weird to apply IRQ_* flags to them right here.
> Anyway, I think he only piece you really want to describe here is, do we
> wake on "event asserted", "event deasserted", or both. (The "none" case
> would just mean you shouldn't have the "wakeup-source" property.)
>
> So maybe:
>
> 	wakeup-trigger-type: Specifies whether the key should wake the
> 	system when asserted, when deasserted, or both. This property is
> 	only valid for keys that wake up the system (e.g., when the
> 	"wakeup-source" property is also provided). Supported values
> 	are:
> 	  1: asserted
> 	  2: deasserted
> 	  3: both asserted and deasserted
>
> ? We could still make macros out of those, if we want
> (input/linux-event-codes.h?). And then leave it up to the driver to
> determine how to translate that into the appropriate edge or level
> triggers.
make sense, will do it in the next version.
>
> Brian
>
>> +		 Only the following flags are supported:
>> +			IRQ_TYPE_NONE
>> +			IRQ_TYPE_EDGE_RISING
>> +			IRQ_TYPE_EDGE_FALLING
>> +			IRQ_TYPE_EDGE_BOTH
>> +			IRQ_TYPE_LEVEL_HIGH
>> +			IRQ_TYPE_LEVEL_LOW
>>   	- linux,can-disable: Boolean, indicates that button is connected
>>   	  to dedicated (not shared) interrupt which can be disabled to
>>   	  suppress events from the button.
>> --
>> 2.11.0
>>
>>
>
>
>


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/3] Input: gpio-keys - allow setting wakeup interrupt trigger type in DT
From: Brian Norris @ 2018-02-09 23:42 UTC (permalink / raw)
  To: Jeffy Chen
  Cc: linux-kernel, briannorris, dtor, dianders, devicetree,
	Rob Herring, Dmitry Torokhov, linux-input, Mark Rutland
In-Reply-To: <20180209115510.11868-3-jeffy.chen@rock-chips.com>

Hi Jeffy,

On Fri, Feb 09, 2018 at 07:55:09PM +0800, Jeffy Chen wrote:
> Allow specifying a different interrupt trigger type for wakeup when
> using the gpio-keys input device as a wakeup source.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
> 
>  Documentation/devicetree/bindings/input/gpio-keys.txt | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt
> index a94940481e55..61926cef708f 100644
> --- a/Documentation/devicetree/bindings/input/gpio-keys.txt
> +++ b/Documentation/devicetree/bindings/input/gpio-keys.txt
> @@ -26,6 +26,15 @@ Optional subnode-properties:
>  	  If not specified defaults to 5.
>  	- wakeup-source: Boolean, button can wake-up the system.
>  			 (Legacy property supported: "gpio-key,wakeup")
> +	- wakeup-trigger-type: Specifies the interrupt trigger type for wakeup.
> +		 The value is defined in <dt-bindings/interrupt-controller/irq.h>

Do you really want to codify interrupt triggers here? It seems like most
of the information about edge vs. level is already codified elsewhere,
so this becomes a little redundant. And in fact, some bindings may be
specifying a "gpio", not technically an interrupt (at least not
directly), so it feels weird to apply IRQ_* flags to them right here.
Anyway, I think he only piece you really want to describe here is, do we
wake on "event asserted", "event deasserted", or both. (The "none" case
would just mean you shouldn't have the "wakeup-source" property.)

So maybe:

	wakeup-trigger-type: Specifies whether the key should wake the
	system when asserted, when deasserted, or both. This property is
	only valid for keys that wake up the system (e.g., when the
	"wakeup-source" property is also provided). Supported values
	are:
	  1: asserted
	  2: deasserted
	  3: both asserted and deasserted

? We could still make macros out of those, if we want
(input/linux-event-codes.h?). And then leave it up to the driver to
determine how to translate that into the appropriate edge or level
triggers.

Brian

> +		 Only the following flags are supported:
> +			IRQ_TYPE_NONE
> +			IRQ_TYPE_EDGE_RISING
> +			IRQ_TYPE_EDGE_FALLING
> +			IRQ_TYPE_EDGE_BOTH
> +			IRQ_TYPE_LEVEL_HIGH
> +			IRQ_TYPE_LEVEL_LOW
>  	- linux,can-disable: Boolean, indicates that button is connected
>  	  to dedicated (not shared) interrupt which can be disabled to
>  	  suppress events from the button.
> -- 
> 2.11.0
> 
> 

^ permalink raw reply

* Re: [PATCH] Input: gpio_keys: Add level trigger support for GPIO keys
From: Andy Shevchenko @ 2018-02-09 15:03 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
	stephen lu, Arvind Yadav, Joseph Lo, Kate Stewart,
	Philippe Ombredanne, Thomas Gleixner,
	linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, DTML, lkml,
	Mark Brown, Linus Walleij
In-Reply-To: <CAMz4kuKb8g5dAu5WQ847CODtVgJQBa_svbeQGGDsYpCjipBp_g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Feb 9, 2018 at 11:23 AM, Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 9 February 2018 at 14:39, Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Thu, Feb 8, 2018 at 10:08 PM, Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>> On some platforms (such as Spreadtrum platform), the GPIO keys can only
>>> be triggered by level type.
>>
>> How do you stop the interrupt from re-triggering as long as the key
>> stays pressed?
>
> We will set the level type irq handler as handle_level_irq(), in this
> function, it will mask and ack the irq firstly.

Wouldn't be ambiguous?

1. User presses the key ->
  a) we got edge followed by level signaling;
  b) IRQ core masks line, calls handler, ACKs, unmasks;
  c) somewhere here Press Event is sent;
  d) we still have level... We get IRQ fired again? But see 1. It
obviously not the case.
2. User releases the key ->
  ...

So, the main question if I understood Dmitry correctly is the period
in time where IRQ line should be masked on one hand, and on the other
it will guarantee that user didn't release-press cylcle.

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 2/3] Input: gpio-keys - allow setting wakeup interrupt trigger type in DT
From: Jeffy Chen @ 2018-02-09 11:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: briannorris, dtor, dianders, Jeffy Chen, devicetree, Rob Herring,
	Dmitry Torokhov, linux-input, Mark Rutland
In-Reply-To: <20180209115510.11868-1-jeffy.chen@rock-chips.com>

Allow specifying a different interrupt trigger type for wakeup when
using the gpio-keys input device as a wakeup source.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

 Documentation/devicetree/bindings/input/gpio-keys.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt
index a94940481e55..61926cef708f 100644
--- a/Documentation/devicetree/bindings/input/gpio-keys.txt
+++ b/Documentation/devicetree/bindings/input/gpio-keys.txt
@@ -26,6 +26,15 @@ Optional subnode-properties:
 	  If not specified defaults to 5.
 	- wakeup-source: Boolean, button can wake-up the system.
 			 (Legacy property supported: "gpio-key,wakeup")
+	- wakeup-trigger-type: Specifies the interrupt trigger type for wakeup.
+		 The value is defined in <dt-bindings/interrupt-controller/irq.h>
+		 Only the following flags are supported:
+			IRQ_TYPE_NONE
+			IRQ_TYPE_EDGE_RISING
+			IRQ_TYPE_EDGE_FALLING
+			IRQ_TYPE_EDGE_BOTH
+			IRQ_TYPE_LEVEL_HIGH
+			IRQ_TYPE_LEVEL_LOW
 	- linux,can-disable: Boolean, indicates that button is connected
 	  to dedicated (not shared) interrupt which can be disabled to
 	  suppress events from the button.
-- 
2.11.0

^ permalink raw reply related

* [PATCH 1/3] Input: gpio-keys - add support for wakeup interrupt trigger type
From: Jeffy Chen @ 2018-02-09 11:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: briannorris, dtor, dianders, Jeffy Chen, Guenter Roeck,
	Thomas Gleixner, Joseph Lo, stephen lu, Dmitry Torokhov,
	Kate Stewart, linux-input, Greg Kroah-Hartman,
	Philippe Ombredanne, Arvind Yadav
In-Reply-To: <20180209115510.11868-1-jeffy.chen@rock-chips.com>

Add support for specifying a different interrupt trigger type for wakeup
when using the gpio-keys input device as a wakeup source.

This would allow the device to configure when to wakeup the system. For
example a gpio-keys input device for pen insert, may only want to wakeup
the system when ejecting the pen.

Suggested-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

 drivers/input/keyboard/gpio_keys.c | 20 ++++++++++++++++++--
 include/linux/gpio_keys.h          |  2 ++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 87e613dc33b8..5e5c93b75a71 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -45,6 +45,7 @@ struct gpio_button_data {
 	unsigned int software_debounce;	/* in msecs, for GPIO-driven buttons */
 
 	unsigned int irq;
+	unsigned int irq_trigger_type;
 	spinlock_t lock;
 	bool disabled;
 	bool key_pressed;
@@ -618,6 +619,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 		return error;
 	}
 
+	bdata->irq_trigger_type = irq_get_trigger_type(bdata->irq);
+
 	return 0;
 }
 
@@ -718,6 +721,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 			/* legacy name */
 			fwnode_property_read_bool(child, "gpio-key,wakeup");
 
+		fwnode_property_read_u32(child, "wakeup-trigger-type",
+					 &button->wakeup_trigger_type);
+
 		button->can_disable =
 			fwnode_property_read_bool(child, "linux,can-disable");
 
@@ -854,7 +860,12 @@ static int __maybe_unused gpio_keys_suspend(struct device *dev)
 	if (device_may_wakeup(dev)) {
 		for (i = 0; i < ddata->pdata->nbuttons; i++) {
 			struct gpio_button_data *bdata = &ddata->data[i];
-			if (bdata->button->wakeup)
+			const struct gpio_keys_button *button = bdata->button;
+
+			if (button->wakeup && button->wakeup_trigger_type)
+				irq_set_irq_type(bdata->irq,
+						 button->wakeup_trigger_type);
+			if (button->wakeup)
 				enable_irq_wake(bdata->irq);
 			bdata->suspended = true;
 		}
@@ -878,7 +889,12 @@ static int __maybe_unused gpio_keys_resume(struct device *dev)
 	if (device_may_wakeup(dev)) {
 		for (i = 0; i < ddata->pdata->nbuttons; i++) {
 			struct gpio_button_data *bdata = &ddata->data[i];
-			if (bdata->button->wakeup)
+			const struct gpio_keys_button *button = bdata->button;
+
+			if (button->wakeup && button->wakeup_trigger_type)
+				irq_set_irq_type(bdata->irq,
+						 bdata->irq_trigger_type);
+			if (button->wakeup)
 				disable_irq_wake(bdata->irq);
 			bdata->suspended = false;
 		}
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index d06bf77400f1..9b7e7137f768 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -13,6 +13,7 @@ struct device;
  * @desc:		label that will be attached to button's gpio
  * @type:		input event type (%EV_KEY, %EV_SW, %EV_ABS)
  * @wakeup:		configure the button as a wake-up source
+ * @wakeup_trigger_type:	wakeup interrupt trigger type
  * @debounce_interval:	debounce ticks interval in msecs
  * @can_disable:	%true indicates that userspace is allowed to
  *			disable button via sysfs
@@ -26,6 +27,7 @@ struct gpio_keys_button {
 	const char *desc;
 	unsigned int type;
 	int wakeup;
+	int wakeup_trigger_type;
 	int debounce_interval;
 	bool can_disable;
 	int value;
-- 
2.11.0

^ permalink raw reply related

* [PATCH 0/3] gpio-keys: Add support for specifying wakeup interrupt trigger type
From: Jeffy Chen @ 2018-02-09 11:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: briannorris, dtor, dianders, Jeffy Chen, Guenter Roeck,
	Arnd Bergmann, Joseph Lo, Rob Herring, Catalin Marinas,
	Emil Renner Berthing, Heiko Stuebner, Brian Norris,
	Thomas Gleixner, Philippe Ombredanne, linux-rockchip,
	Kate Stewart, linux-input, Will Deacon, Matthias Kaehlcke,
	devicetree, stephen lu, Greg Kroah-Hartman


On chromebook kevin, we are using gpio-keys for pen insert event. But
we only want it to wakeup the system when ejecting the pen.

So we may need to change the interrupt trigger type during suspending.


Jeffy Chen (3):
  Input: gpio-keys - add support for wakeup interrupt trigger type
  Input: gpio-keys - allow setting wakeup interrupt trigger type in DT
  arm64: dts: rockchip: Avoid wakeup when inserting the pen

 .../devicetree/bindings/input/gpio-keys.txt          |  9 +++++++++
 arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts    |  2 ++
 drivers/input/keyboard/gpio_keys.c                   | 20 ++++++++++++++++++--
 include/linux/gpio_keys.h                            |  2 ++
 4 files changed, 31 insertions(+), 2 deletions(-)

-- 
2.11.0

^ permalink raw reply

* Re: [PATCH] Input: gpio_keys: Add level trigger support for GPIO keys
From: Baolin Wang @ 2018-02-09  9:23 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rob Herring, Mark Rutland, Greg Kroah-Hartman, stephen lu,
	Arvind Yadav, Joseph Lo, Kate Stewart, Philippe Ombredanne,
	Thomas Gleixner,
	linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, DTML, lkml,
	Mark Brown, Linus Walleij
In-Reply-To: <CAKdAkRQ6+9=GUJfoMT_gtwS8DqWKeH9FAqPG2BFzL9GPbPNB4w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 9 February 2018 at 14:39, Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Feb 8, 2018 at 10:08 PM, Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On some platforms (such as Spreadtrum platform), the GPIO keys can only
>> be triggered by level type.
>
> How do you stop the interrupt from re-triggering as long as the key
> stays pressed?

We will set the level type irq handler as handle_level_irq(), in this
function, it will mask and ack the irq firstly.

-- 
Baolin.wang
Best Regards
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] Input: gpio_keys: Add level trigger support for GPIO keys
From: Dmitry Torokhov @ 2018-02-09  6:39 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Rob Herring, Mark Rutland, Greg Kroah-Hartman, stephen lu,
	Arvind Yadav, Joseph Lo, Kate Stewart, Philippe Ombredanne,
	Thomas Gleixner,
	linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA, lkml, Mark Brown,
	Linus Walleij
In-Reply-To: <6c53b9ccfd4a56fc1b00e243bb9e7cae9e124932.1518155780.git.baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Thu, Feb 8, 2018 at 10:08 PM, Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On some platforms (such as Spreadtrum platform), the GPIO keys can only
> be triggered by level type.

How do you stop the interrupt from re-triggering as long as the key
stays pressed?

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] Input: gpio_keys: Add level trigger support for GPIO keys
From: Baolin Wang @ 2018-02-09  6:08 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, mark.rutland
  Cc: gregkh, lumotuwe, arvind.yadav.cs, josephl, kstewart, pombredanne,
	tglx, linux-input, devicetree, linux-kernel, broonie,
	linus.walleij, baolin.wang

On some platforms (such as Spreadtrum platform), the GPIO keys can only
be triggered by level type. So this patch introduces one property to
indicate if the GPIO trigger type is level trigger or edge trigger.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 .../devicetree/bindings/input/gpio-keys.txt        |    2 ++
 drivers/input/keyboard/gpio_keys.c                 |   22 +++++++++++++++++++-
 include/linux/gpio_keys.h                          |    1 +
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt
index a949404..e3104bd 100644
--- a/Documentation/devicetree/bindings/input/gpio-keys.txt
+++ b/Documentation/devicetree/bindings/input/gpio-keys.txt
@@ -29,6 +29,8 @@ Optional subnode-properties:
 	- linux,can-disable: Boolean, indicates that button is connected
 	  to dedicated (not shared) interrupt which can be disabled to
 	  suppress events from the button.
+	- gpio-key,level-trigger: Boolean, indicates that button's interrupt
+	  type is level trigger. Otherwise it is edge trigger as default.
 
 Example nodes:
 
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 87e613d..d3b4bb6 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -385,6 +385,19 @@ static void gpio_keys_gpio_work_func(struct work_struct *work)
 	struct gpio_button_data *bdata =
 		container_of(work, struct gpio_button_data, work.work);
 
+	if (bdata->button->level_trigger) {
+		unsigned int trigger =
+			irq_get_trigger_type(bdata->irq) & ~IRQF_TRIGGER_MASK;
+		int state = gpiod_get_raw_value_cansleep(bdata->gpiod);
+
+		if (state)
+			trigger |= IRQF_TRIGGER_LOW;
+		else
+			trigger |= IRQF_TRIGGER_HIGH;
+
+		irq_set_irq_type(bdata->irq, trigger);
+	}
+
 	gpio_keys_gpio_report_event(bdata);
 
 	if (bdata->button->wakeup)
@@ -566,7 +579,11 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 		INIT_DELAYED_WORK(&bdata->work, gpio_keys_gpio_work_func);
 
 		isr = gpio_keys_gpio_isr;
-		irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
+		if (button->level_trigger)
+			irqflags = gpiod_is_active_low(bdata->gpiod) ?
+				IRQF_TRIGGER_LOW : IRQF_TRIGGER_HIGH;
+		else
+			irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
 
 	} else {
 		if (!button->irq) {
@@ -721,6 +738,9 @@ static void gpio_keys_close(struct input_dev *input)
 		button->can_disable =
 			fwnode_property_read_bool(child, "linux,can-disable");
 
+		button->level_trigger =
+			fwnode_property_read_bool(child, "gpio-key,level-trigger");
+
 		if (fwnode_property_read_u32(child, "debounce-interval",
 					 &button->debounce_interval))
 			button->debounce_interval = 5;
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index d06bf77..5095645 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -28,6 +28,7 @@ struct gpio_keys_button {
 	int wakeup;
 	int debounce_interval;
 	bool can_disable;
+	bool level_trigger;
 	int value;
 	unsigned int irq;
 };
-- 
1.7.9.5


^ permalink raw reply related

* Re: [git pull] Input updates for v4.15-rc8
From: Dmitry Torokhov @ 2018-02-09  1:39 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: linux-kernel, linux-input
In-Reply-To: <b41e15f7-845a-6873-ddd1-0829b39319a4@suse.cz>

On Thu, Feb 08, 2018 at 11:04:22AM +0100, Jiri Slaby wrote:
> On 01/19/2018, 06:42 PM, Dmitry Torokhov wrote:
> > Please pull from:
> > 
> > 	git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
> > 
> > to receive updates for the input subsystem. You will get:
> > 
> ...
> > - touchpad on Lenovo T640p is switched over to SMbus/RMI
> ...
> 
> > 王振杰 (1):
> >       Input: synaptics - Lenovo Thinkpad T460p devices should use RMI
> 
> Hi,
> 
> one of openSUSE users with T460p reports that the touchpad is defunct
> after resume in 4.15. psmouse.synaptics_intertouch=0 works around the
> problem.
> 
> The problem is:
>  kernel: rmi4_smbus 8-002c: failed to get SMBus version number!
>  kernel: rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed to read
> current IRQ mask.
>  kernel: rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -16.
>  kernel: rmi4_f01 rmi4-00.fn01: Resume failed with code -16.
>  kernel: rmi4_physical rmi4-00: Failed to suspend functions: -16
>  kernel: rmi4_smbus 8-002c: Failed to resume device: -16
>  kernel: rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to F03
> TX register (-16).
> 
> The downstream bug is at:
> https://bugzilla.suse.com/show_bug.cgi?id=1079862
> 
> Any ideas?

Hmm, maybe the seuence between psmouse and rmi/smbus is not quite right
on resume on that box. Can you ask the reporter to:

echo 1 > /sys/power/pm_print_times
suspend/resume

and send dmesg?

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: ALPS Trackpoint & pressure
From: Peter Hutterer @ 2018-02-08 23:57 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Dmitry Torokhov, Masaki Ota, linux-input, linux-kernel
In-Reply-To: <20180208232141.2jxjd2xzkfua42mr@pali>

On Fri, Feb 09, 2018 at 12:21:41AM +0100, Pali Rohár wrote:
> On Tuesday 06 February 2018 10:29:47 Peter Hutterer wrote:
> > On Mon, Feb 05, 2018 at 02:49:55PM -0800, Dmitry Torokhov wrote:
> > > On Sun, Feb 04, 2018 at 04:08:39PM +0100, Pali Rohár wrote:
> > > > Hi! Now playing again with trackpoint connected to ALPS rushmore
> > > > touchpad and I'm seeking a nice feature. Via ALPS PS/2 protocol it
> > > > reports pressure of trackpoint. Parser for it is already implemented in
> > > > alps.c and value is assigned to variable "z". When I just move
> > > > trackpoint z is zero, when I push trackpoint while moving, then z is
> > > > number higher, maximally 32. Variable "z" is set, but unused.
> > > > 
> > > > Do we have some input interface which can be used to report this
> > > > pressure of trackpoint to userspace? I can use this feature e.g. as
> > > > additional button...
> > > 
> > > We could either do the conversion in kernel and emit BTN_LEFT, or
> > > report ABS_PRESSURE and see if userspace will be able to handle
> > > REL_X/REL_Y/ABS_PRESSURE device.
> > > 
> > > Adding Peter.
> > 
> > judging by trackpoint history, I'd leave the pressure->click conversion to
> > userspace because every trackpoint may need a different threshold setting.
> > "easier" to have this in userspace with dmi matches etc. plus, converting to
> > BTN_LEFT in the kernel means we cannot use it as a separate interaction
> > anymore.
> 
> Also BTN_LEFT is already reported when left button under trackpoint is
> pressed. Therefore it would not be possible to distinguish between
> trackpoint "press" and real left button press.

yep, that's what I meant with "we cannot use it as separate interaction",
we'd have no way to know how the click was generated.

> 
> > That aside, I think exporting ABS_PRESSURE is fine, that's what it's there
> > for. Nothing will use it for now though, tbh not sure yet how that would be
> > exported from libinput. but worth filing a bug for, please assign it to me.
> 
> Ok, so ABS_PRESSURE? Also then question is, how to report minimal and
> maximal (possible) value? If we are going to "standardize" API for it,
> we should also define min/max values, so userspace would be able to
> normalize this pressure event. I can imagine that some devices can
> report 8bit value, but ALPS rushmore reports only 5bit value.

tbh, I'm not putting my hopes on this being an accurate range ever. So
what's likely going to happen is that you pick a best-guess min/max for the
kernel and then we have dmi-based overrides for every single trackpoint in
userspace to tell us what a realistic threshold value for a click is and
what the actual min/max ranges is.

This is relatively easy to measure in userspace, we can pop it into
60-evdev.hwdb to override the min/max and ship the threshold values as
libinput-specific files. It's awful, but most likely the best we can do.

But hey, at least a min of 0 will be accurate ;)

Cheers,
   Peter

^ permalink raw reply

* Re: ALPS Trackpoint & pressure
From: Pali Rohár @ 2018-02-08 23:21 UTC (permalink / raw)
  To: Peter Hutterer; +Cc: Dmitry Torokhov, Masaki Ota, linux-input, linux-kernel
In-Reply-To: <20180206002947.GB27776@jelly>

[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]

On Tuesday 06 February 2018 10:29:47 Peter Hutterer wrote:
> On Mon, Feb 05, 2018 at 02:49:55PM -0800, Dmitry Torokhov wrote:
> > On Sun, Feb 04, 2018 at 04:08:39PM +0100, Pali Rohár wrote:
> > > Hi! Now playing again with trackpoint connected to ALPS rushmore
> > > touchpad and I'm seeking a nice feature. Via ALPS PS/2 protocol it
> > > reports pressure of trackpoint. Parser for it is already implemented in
> > > alps.c and value is assigned to variable "z". When I just move
> > > trackpoint z is zero, when I push trackpoint while moving, then z is
> > > number higher, maximally 32. Variable "z" is set, but unused.
> > > 
> > > Do we have some input interface which can be used to report this
> > > pressure of trackpoint to userspace? I can use this feature e.g. as
> > > additional button...
> > 
> > We could either do the conversion in kernel and emit BTN_LEFT, or
> > report ABS_PRESSURE and see if userspace will be able to handle
> > REL_X/REL_Y/ABS_PRESSURE device.
> > 
> > Adding Peter.
> 
> judging by trackpoint history, I'd leave the pressure->click conversion to
> userspace because every trackpoint may need a different threshold setting.
> "easier" to have this in userspace with dmi matches etc. plus, converting to
> BTN_LEFT in the kernel means we cannot use it as a separate interaction
> anymore.

Also BTN_LEFT is already reported when left button under trackpoint is
pressed. Therefore it would not be possible to distinguish between
trackpoint "press" and real left button press.

> That aside, I think exporting ABS_PRESSURE is fine, that's what it's there
> for. Nothing will use it for now though, tbh not sure yet how that would be
> exported from libinput. but worth filing a bug for, please assign it to me.

Ok, so ABS_PRESSURE? Also then question is, how to report minimal and
maximal (possible) value? If we are going to "standardize" API for it,
we should also define min/max values, so userspace would be able to
normalize this pressure event. I can imagine that some devices can
report 8bit value, but ALPS rushmore reports only 5bit value.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply

* Re: [git pull] Input updates for v4.15-rc8
From: Jiri Slaby @ 2018-02-08 10:04 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-kernel, linux-input
In-Reply-To: <20180119174205.iq76oggc2h7neymf@dtor-ws>

On 01/19/2018, 06:42 PM, Dmitry Torokhov wrote:
> Please pull from:
> 
> 	git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
> 
> to receive updates for the input subsystem. You will get:
> 
...
> - touchpad on Lenovo T640p is switched over to SMbus/RMI
...

> 王振杰 (1):
>       Input: synaptics - Lenovo Thinkpad T460p devices should use RMI

Hi,

one of openSUSE users with T460p reports that the touchpad is defunct
after resume in 4.15. psmouse.synaptics_intertouch=0 works around the
problem.

The problem is:
 kernel: rmi4_smbus 8-002c: failed to get SMBus version number!
 kernel: rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed to read
current IRQ mask.
 kernel: rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -16.
 kernel: rmi4_f01 rmi4-00.fn01: Resume failed with code -16.
 kernel: rmi4_physical rmi4-00: Failed to suspend functions: -16
 kernel: rmi4_smbus 8-002c: Failed to resume device: -16
 kernel: rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to F03
TX register (-16).

The downstream bug is at:
https://bugzilla.suse.com/show_bug.cgi?id=1079862

Any ideas?

thanks,
-- 
js
suse labs

^ permalink raw reply

* Re: [PATCH 3/3] hid: logitech-dj: delete unnecessary error messages
From: Christoph Böhmwalder @ 2018-02-08  9:06 UTC (permalink / raw)
  To: Marcus Folkesson; +Cc: jikos, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20180208075628.GA2860@gmail.com>

On Thu, Feb 08, 2018 at 08:56:28AM +0100, Marcus Folkesson wrote:
> 
> Thank you, but Markus Elfring already has a submitted a patch for this one.
> 
> /Marcus

Ah sorry, I must've missed that one.  Feel free to dismiss it then (it's
obviously independent of the other two anyways).

--
Regards,
Christoph

^ permalink raw reply

* [PATCH 1/4] add missing blob structure field for tag id
From: Florian Echtler @ 2018-02-08  8:43 UTC (permalink / raw)
  To: hverkuil, linux-media; +Cc: linux-input, modin, Florian Echtler
In-Reply-To: <1518079386-4647-1-git-send-email-floe@butterbrot.org>

The SUR40 can recognize specific printed patterns directly in hardware;
this information (i.e. the pattern id) is present but currently unused
in the blob structure.

Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
 drivers/input/touchscreen/sur40.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index f16f835..8375b06 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -81,7 +81,10 @@ struct sur40_blob {
 
 	__le32 area;       /* size in pixels/pressure (?) */
 
-	u8 padding[32];
+	u8 padding[24];
+
+	__le32 tag_id;     /* valid when type == 0x04 (SUR40_TAG) */
+	__le32 unknown;
 
 } __packed;
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH 2/4] add default settings and module parameters for video controls
From: Florian Echtler @ 2018-02-08  8:43 UTC (permalink / raw)
  To: hverkuil, linux-media; +Cc: linux-input, modin, Florian Echtler
In-Reply-To: <1518079386-4647-1-git-send-email-floe@butterbrot.org>

This patch adds parameter definitions and module parameters for the four
userspace controls that the SUR40 can currently provide.

Signed-off-by: Florian Echtler <floe@butterbrot.org>
---
 drivers/input/touchscreen/sur40.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 8375b06..8a5b031 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -149,6 +149,40 @@ struct sur40_image_header {
 #define SUR40_TOUCH	0x02
 #define SUR40_TAG	0x04
 
+/* video controls */
+#define SUR40_BRIGHTNESS_MAX 0xff
+#define SUR40_BRIGHTNESS_MIN 0x00
+#define SUR40_BRIGHTNESS_DEF 0xff
+
+#define SUR40_CONTRAST_MAX 0x0f
+#define SUR40_CONTRAST_MIN 0x00
+#define SUR40_CONTRAST_DEF 0x0a
+
+#define SUR40_GAIN_MAX 0x09
+#define SUR40_GAIN_MIN 0x00
+#define SUR40_GAIN_DEF 0x08
+
+#define SUR40_BACKLIGHT_MAX 0x01
+#define SUR40_BACKLIGHT_MIN 0x00
+#define SUR40_BACKLIGHT_DEF 0x01
+
+#define sur40_str(s) #s
+#define SUR40_PARAM_RANGE(lo, hi) " (range " sur40_str(lo) "-" sur40_str(hi) ")"
+
+/* module parameters */
+static uint brightness = SUR40_BRIGHTNESS_DEF;
+module_param(brightness, uint, 0644);
+MODULE_PARM_DESC(brightness, "set initial brightness"
+	SUR40_PARAM_RANGE(SUR40_BRIGHTNESS_MIN, SUR40_BRIGHTNESS_MAX));
+static uint contrast = SUR40_CONTRAST_DEF;
+module_param(contrast, uint, 0644);
+MODULE_PARM_DESC(contrast, "set initial contrast"
+	SUR40_PARAM_RANGE(SUR40_CONTRAST_MIN, SUR40_CONTRAST_MAX));
+static uint gain = SUR40_GAIN_DEF;
+module_param(gain, uint, 0644);
+MODULE_PARM_DESC(gain, "set initial gain"
+	SUR40_PARAM_RANGE(SUR40_GAIN_MIN, SUR40_GAIN_MAX));
+
 static const struct v4l2_pix_format sur40_pix_format[] = {
 	{
 		.pixelformat = V4L2_TCH_FMT_TU08,
-- 
2.7.4


^ permalink raw reply related

* [PATCH v4] add video controls for SUR40 driver
From: Florian Echtler @ 2018-02-08  8:43 UTC (permalink / raw)
  To: hverkuil, linux-media; +Cc: linux-input, modin

Iteration 4, now with min/max values for module parameters.

Best regards, Florian


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox