* RE: [PATCH 1/2] HID: wacom: Lazy-init batteries
From: Limonciello, Mario @ 2023-04-13 19:03 UTC (permalink / raw)
To: Jason Gerecke, linux-input@vger.kernel.org, Benjamin Tissoires,
Jiri Kosina
Cc: Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens, Jason Gerecke,
Bastien Nocera
In-Reply-To: <20230413181743.7849-1-jason.gerecke@wacom.com>
[Public]
> From: Jason Gerecke <killertofu@gmail.com>
>
> Rather than creating batteries as part of the initial device probe, let's
> make the process lazy. This gives us the opportunity to prevent batteries
> from being created in situations where they are unnecessary.
>
> There are two cases in particular where batteries are being unnecessarily
> created at initialization. These are AES sensors (for which we don't know
> any battery status information until a battery-powered pen actually comes
> into prox) peripheral tablets which share HID descriptors between the
> wired-only and wireless-capable SKUs of a family of devices.
>
> This patch will delay battery initialization of the former until a pen
> actually comes into prox. It will delay battery initialization of the
> latter until either a pen comes into prox or a "heartbeat" packet is
> processed.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
Some other tags to add:
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217062
Link: https://gitlab.gnome.org/GNOME/gnome-control-center/-/issues/2354
> ---
> drivers/hid/wacom_sys.c | 10 ----------
> drivers/hid/wacom_wac.c | 13 ++++++-------
> 2 files changed, 6 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index fb538a6c4add8..8214896adadad 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2372,13 +2372,6 @@ static int wacom_parse_and_register(struct
> wacom *wacom, bool wireless)
> if (error)
> goto fail;
>
> - if (!(features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
> &&
> - (features->quirks & WACOM_QUIRK_BATTERY)) {
> - error = wacom_initialize_battery(wacom);
> - if (error)
> - goto fail;
> - }
> -
> error = wacom_register_inputs(wacom);
> if (error)
> goto fail;
> @@ -2509,9 +2502,6 @@ static void wacom_wireless_work(struct
> work_struct *work)
>
> strscpy(wacom_wac->name, wacom_wac1->name,
> sizeof(wacom_wac->name));
> - error = wacom_initialize_battery(wacom);
> - if (error)
> - goto fail;
> }
>
> return;
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 4cfa51416dbcb..391fde5bf6024 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -113,6 +113,11 @@ static void wacom_notify_battery(struct
> wacom_wac *wacom_wac,
> bool bat_connected, bool ps_connected)
> {
> struct wacom *wacom = container_of(wacom_wac, struct wacom,
> wacom_wac);
> + bool bat_initialized = wacom->battery.battery;
> + bool has_quirk = wacom_wac->features.quirks &
> WACOM_QUIRK_BATTERY;
> +
> + if (bat_initialized != has_quirk)
> + wacom_schedule_work(wacom_wac,
> WACOM_WORKER_BATTERY);
>
> __wacom_notify_battery(&wacom->battery, bat_status,
> bat_capacity,
> bat_charging, bat_connected, ps_connected);
> @@ -3391,19 +3396,13 @@ static int wacom_status_irq(struct wacom_wac
> *wacom_wac, size_t len)
> int battery = (data[8] & 0x3f) * 100 / 31;
> bool charging = !!(data[8] & 0x80);
>
> + features->quirks |= WACOM_QUIRK_BATTERY;
> wacom_notify_battery(wacom_wac,
> WACOM_POWER_SUPPLY_STATUS_AUTO,
> battery, charging, battery || charging, 1);
> -
> - if (!wacom->battery.battery &&
> - !(features->quirks & WACOM_QUIRK_BATTERY)) {
> - features->quirks |= WACOM_QUIRK_BATTERY;
> - wacom_schedule_work(wacom_wac,
> WACOM_WORKER_BATTERY);
> - }
> }
> else if ((features->quirks & WACOM_QUIRK_BATTERY) &&
> wacom->battery.battery) {
> features->quirks &= ~WACOM_QUIRK_BATTERY;
> - wacom_schedule_work(wacom_wac,
> WACOM_WORKER_BATTERY);
> wacom_notify_battery(wacom_wac,
> POWER_SUPPLY_STATUS_UNKNOWN, 0, 0, 0, 0);
> }
> return 0;
> --
> 2.40.0
^ permalink raw reply
* [PATCH 2/2] HID: wacom: generic: Set battery quirk only when we see battery data
From: Jason Gerecke @ 2023-04-13 18:17 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina
Cc: Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens, Jason Gerecke,
Jason Gerecke, Mario Limonciello
In-Reply-To: <20230413181743.7849-1-jason.gerecke@wacom.com>
From: Jason Gerecke <killertofu@gmail.com>
Some devices will include battery status usages in the HID descriptor
but we won't see that battery data for one reason or another. For example,
AES sensors won't send battery data unless an AES pen is in proximity.
If a user does not have an AES pen but instead only interacts with the
AES touchscreen with their fingers then there is no need for us to create
a battery object. Similarly, if a family of peripherals shares the same
HID descriptor between wired-only and wireless-capable SKUs, users of the
former may never see a battery event and will not want a power_supply
object created.
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Tested-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/hid/wacom_wac.c | 33 +++++++++++----------------------
1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 391fde5bf6024..5db7de7a6b171 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1960,18 +1960,7 @@ static void wacom_map_usage(struct input_dev *input, struct hid_usage *usage,
static void wacom_wac_battery_usage_mapping(struct hid_device *hdev,
struct hid_field *field, struct hid_usage *usage)
{
- struct wacom *wacom = hid_get_drvdata(hdev);
- struct wacom_wac *wacom_wac = &wacom->wacom_wac;
- struct wacom_features *features = &wacom_wac->features;
- unsigned equivalent_usage = wacom_equivalent_usage(usage->hid);
-
- switch (equivalent_usage) {
- case HID_DG_BATTERYSTRENGTH:
- case WACOM_HID_WD_BATTERY_LEVEL:
- case WACOM_HID_WD_BATTERY_CHARGING:
- features->quirks |= WACOM_QUIRK_BATTERY;
- break;
- }
+ return;
}
static void wacom_wac_battery_event(struct hid_device *hdev, struct hid_field *field,
@@ -1992,18 +1981,21 @@ static void wacom_wac_battery_event(struct hid_device *hdev, struct hid_field *f
wacom_wac->hid_data.bat_connected = 1;
wacom_wac->hid_data.bat_status = WACOM_POWER_SUPPLY_STATUS_AUTO;
}
+ wacom_wac->features.quirks |= WACOM_QUIRK_BATTERY;
break;
case WACOM_HID_WD_BATTERY_LEVEL:
value = value * 100 / (field->logical_maximum - field->logical_minimum);
wacom_wac->hid_data.battery_capacity = value;
wacom_wac->hid_data.bat_connected = 1;
wacom_wac->hid_data.bat_status = WACOM_POWER_SUPPLY_STATUS_AUTO;
+ wacom_wac->features.quirks |= WACOM_QUIRK_BATTERY;
break;
case WACOM_HID_WD_BATTERY_CHARGING:
wacom_wac->hid_data.bat_charging = value;
wacom_wac->hid_data.ps_connected = value;
wacom_wac->hid_data.bat_connected = 1;
wacom_wac->hid_data.bat_status = WACOM_POWER_SUPPLY_STATUS_AUTO;
+ wacom_wac->features.quirks |= WACOM_QUIRK_BATTERY;
break;
}
}
@@ -2019,18 +2011,15 @@ static void wacom_wac_battery_report(struct hid_device *hdev,
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_wac *wacom_wac = &wacom->wacom_wac;
- struct wacom_features *features = &wacom_wac->features;
- if (features->quirks & WACOM_QUIRK_BATTERY) {
- int status = wacom_wac->hid_data.bat_status;
- int capacity = wacom_wac->hid_data.battery_capacity;
- bool charging = wacom_wac->hid_data.bat_charging;
- bool connected = wacom_wac->hid_data.bat_connected;
- bool powered = wacom_wac->hid_data.ps_connected;
+ int status = wacom_wac->hid_data.bat_status;
+ int capacity = wacom_wac->hid_data.battery_capacity;
+ bool charging = wacom_wac->hid_data.bat_charging;
+ bool connected = wacom_wac->hid_data.bat_connected;
+ bool powered = wacom_wac->hid_data.ps_connected;
- wacom_notify_battery(wacom_wac, status, capacity, charging,
- connected, powered);
- }
+ wacom_notify_battery(wacom_wac, status, capacity, charging,
+ connected, powered);
}
static void wacom_wac_pad_usage_mapping(struct hid_device *hdev,
--
2.40.0
^ permalink raw reply related
* [PATCH 1/2] HID: wacom: Lazy-init batteries
From: Jason Gerecke @ 2023-04-13 18:17 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina
Cc: Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens, Jason Gerecke,
Jason Gerecke, Mario Limonciello
From: Jason Gerecke <killertofu@gmail.com>
Rather than creating batteries as part of the initial device probe, let's
make the process lazy. This gives us the opportunity to prevent batteries
from being created in situations where they are unnecessary.
There are two cases in particular where batteries are being unnecessarily
created at initialization. These are AES sensors (for which we don't know
any battery status information until a battery-powered pen actually comes
into prox) peripheral tablets which share HID descriptors between the
wired-only and wireless-capable SKUs of a family of devices.
This patch will delay battery initialization of the former until a pen
actually comes into prox. It will delay battery initialization of the
latter until either a pen comes into prox or a "heartbeat" packet is
processed.
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Tested-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/hid/wacom_sys.c | 10 ----------
drivers/hid/wacom_wac.c | 13 ++++++-------
2 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index fb538a6c4add8..8214896adadad 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2372,13 +2372,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
if (error)
goto fail;
- if (!(features->device_type & WACOM_DEVICETYPE_WL_MONITOR) &&
- (features->quirks & WACOM_QUIRK_BATTERY)) {
- error = wacom_initialize_battery(wacom);
- if (error)
- goto fail;
- }
-
error = wacom_register_inputs(wacom);
if (error)
goto fail;
@@ -2509,9 +2502,6 @@ static void wacom_wireless_work(struct work_struct *work)
strscpy(wacom_wac->name, wacom_wac1->name,
sizeof(wacom_wac->name));
- error = wacom_initialize_battery(wacom);
- if (error)
- goto fail;
}
return;
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 4cfa51416dbcb..391fde5bf6024 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -113,6 +113,11 @@ static void wacom_notify_battery(struct wacom_wac *wacom_wac,
bool bat_connected, bool ps_connected)
{
struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+ bool bat_initialized = wacom->battery.battery;
+ bool has_quirk = wacom_wac->features.quirks & WACOM_QUIRK_BATTERY;
+
+ if (bat_initialized != has_quirk)
+ wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY);
__wacom_notify_battery(&wacom->battery, bat_status, bat_capacity,
bat_charging, bat_connected, ps_connected);
@@ -3391,19 +3396,13 @@ static int wacom_status_irq(struct wacom_wac *wacom_wac, size_t len)
int battery = (data[8] & 0x3f) * 100 / 31;
bool charging = !!(data[8] & 0x80);
+ features->quirks |= WACOM_QUIRK_BATTERY;
wacom_notify_battery(wacom_wac, WACOM_POWER_SUPPLY_STATUS_AUTO,
battery, charging, battery || charging, 1);
-
- if (!wacom->battery.battery &&
- !(features->quirks & WACOM_QUIRK_BATTERY)) {
- features->quirks |= WACOM_QUIRK_BATTERY;
- wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY);
- }
}
else if ((features->quirks & WACOM_QUIRK_BATTERY) &&
wacom->battery.battery) {
features->quirks &= ~WACOM_QUIRK_BATTERY;
- wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY);
wacom_notify_battery(wacom_wac, POWER_SUPPLY_STATUS_UNKNOWN, 0, 0, 0, 0);
}
return 0;
--
2.40.0
^ permalink raw reply related
* Re: [PATCH 0/2] Fixes for the mcp2221 gpiochip get_* calls
From: Louis Morhet @ 2023-04-13 16:46 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: gupt21, jikos, linux-i2c, linux-input, linux-kernel
In-Reply-To: <20230413144913.53a22nat4s6i2mib@mail.corp.redhat.com>
On Thu, Apr 13, 2023 at 04:49:13PM +0200, Benjamin Tissoires wrote:
> As you should have seen in the automatic replied, I took that series in
> because it seems to fix a rather worrying bug.
Thanks!
> Also, just a side note, this driver is very limited in term of scope, as
> it only touches one dedicated device. Which means that whatever solution
> feels the more elegant to you has a good chance of being accepted :)
Well, using a struct to describe the layout of a message seems more
elegant; but if I'm not mistaken, taking offsets of fields in a packed
struct is UB... so I was surprised to even see that in Linux, that's why
I was wondering if I should pursue in that direction.
--
Louis
^ permalink raw reply
* Re: [PATCH v3] staging: HID: Add ShanWan USB WirelessGamepad driver
From: Benjamin Tissoires @ 2023-04-13 16:13 UTC (permalink / raw)
To: Mubashshir; +Cc: Jiri Kosina, Huseyin BIYIK, linux-kernel, linux-input
In-Reply-To: <fd1961a773558930b6ae0aa2f6fc7f84833ee634.1681131540.git.ahmubashshir@gmail.com>
On Apr 10 2023, Mubashshir wrote:
> This device has a quirky initialization process.
> Depending on how it was initialized, behaves completely differently.
> In default mode, it behaves as expected, but in fallback it disables
> force-feedback, analog stick configurations and L3/R3.
>
> Signed-off-by: Huseyin BIYIK <huseyinbiyik@hotmail.com>
> Signed-off-by: Mubashshir <ahmubashshir@gmail.com>
> ---
> v3: Another missed semicolon
>
> drivers/hid/Kconfig | 19 +++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 3 +
> drivers/hid/hid-shanwan.c | 256 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 279 insertions(+)
> create mode 100644 drivers/hid/hid-shanwan.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 82f64fb31fda..a17db9c9694c 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -990,6 +990,25 @@ config HID_SEMITEK
> - Woo-dy
> - X-Bows Nature/Knight
>
> +config HID_SHANWAN
> + tristate "ShanWan USB WirelessGamepad"
> + depends on USB_HID
> + help
> + Support for Shanwan USB WirelessGamepad (and clones).
> +
> + This device has a quirky initialization process.
> + Depending on how it was initialized, it behaves completely differently.
> + In default mode, it behaves as expected, but in fallback it disables
> + force-feedback, analog stick configurations and L3/R3.
> +
> +config SHANWAN_FF
> + bool "ShanWan USB WirelessGamepad force feedback support"
> + depends on HID_SHANWAN
> + select INPUT_FF_MEMLESS
> + help
> + Say Y here if you have a ShanWan USB WirelessGamepad and want to enable
> + force feedback support for it.
> +
> config HID_SIGMAMICRO
> tristate "SiGma Micro-based keyboards"
> depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 5d37cacbde33..52878455fc10 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -116,6 +116,7 @@ obj-$(CONFIG_HID_RMI) += hid-rmi.o
> obj-$(CONFIG_HID_SAITEK) += hid-saitek.o
> obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
> obj-$(CONFIG_HID_SEMITEK) += hid-semitek.o
> +obj-$(CONFIG_HID_SHANWAN) += hid-shanwan.o
> obj-$(CONFIG_HID_SIGMAMICRO) += hid-sigmamicro.o
> obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
> obj-$(CONFIG_HID_SONY) += hid-sony.o
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 63545cd307e5..278914e37eb7 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -623,6 +623,9 @@
> #define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_0641 0x0641
> #define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_1f4a 0x1f4a
>
> +#define USB_VENDOR_ID_SHANWAN 0x2563
> +#define USB_PRODUCT_ID_SHANWAN_USB_WIRELESSGAMEPAD 0x0575
> +
> #define USB_VENDOR_ID_HUION 0x256c
> #define USB_DEVICE_ID_HUION_TABLET 0x006e
> #define USB_DEVICE_ID_HUION_TABLET2 0x006d
> diff --git a/drivers/hid/hid-shanwan.c b/drivers/hid/hid-shanwan.c
> new file mode 100644
> index 000000000000..39c1bb6f40c6
> --- /dev/null
> +++ b/drivers/hid/hid-shanwan.c
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Force feedback support for Shanwan USB WirelessGamepad
> + *
> + * Copyright (c) 2022-2023 Huseyin BIYIK <huseyinbiyik@hotmail.com>
> + * Copyright (c) 2023 Ahmad Hasan Mubashshir <ahmubashshir@gmail.com>
> + *
> + * mapping according to Gamepad Protocol
> + *
> + * Button 01: BTN_SOUTH (CROSS)
> + * Button 02: BTN_EAST(CIRCLE)
> + * Button 03: BTN_NORTH (TRIANGLE)
> + * Button 04: BTN_WEST (SQUARE)
> + * Button 05: BTL_TL (L1)
> + * Button 06: BTM_TR (R1)
> + * Button 07: BTN_TL2 (L2)
> + * Button 08: BTN_TR2 (R2)
> + * Button 09: BTN_SELECT
> + * Button 10: BTN_START
> + * Button 11: BTN_MODE
> + * Button 12: BTN_THUMBL (LS1)
> + * Button 13: BTN_THUMBR (LS1)
> + * LS1: X/Y AXIS
> + * LS2: Rx/Ry AXIS
> + * R2/L2 Touch Sensors: R/Rz
> + */
> +
> +#include <linux/input.h>
> +#include <linux/slab.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/moduleparam.h>
> +
> +#include "hid-ids.h"
> +
> +#define PID0575_RDESC_ORIG_SIZE 137
> +
> +static bool swap_motors;
> +module_param_named(swap, swap_motors, bool, 0);
> +MODULE_PARM_DESC(swap, "Swap Weak/Strong Feedback motors");
> +
> +static __u8 pid0575_rdesc_fixed[] = {
> + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls)
> + 0x09, 0x05, // Usage (Game Pad)
> + 0xA1, 0x01, // Collection (Application)
> + 0x15, 0x00, // Logical Minimum (0)
> + 0x25, 0x01, // Logical Maximum (1)
> + 0x35, 0x00, // Physical Minimum (0)
> + 0x45, 0x01, // Physical Maximum (1)
> + 0x75, 0x01, // Report Size (1)
> + 0x95, 0x0D, // Report Count (13)
> + 0x05, 0x09, // Usage Page (Button)
> + 0x09, 0x03, // Usage (BTN_NORTH)
> + 0x09, 0x02, // Usage (BTN_EAST)
> + 0x09, 0x01, // Usage (BTN_SOUTH)
> + 0x09, 0x04, // Usage (BTN_WEST)
> + 0x09, 0x05, // Usage (BTN_TL)
> + 0x09, 0x06, // Usage (BTN_TR)
> + 0x09, 0x07, // Usage (BTN_TL2)
> + 0x09, 0x08, // Usage (BTN_TR2)
> + 0x09, 0x09, // Usage (BTN_SELECT)
> + 0x09, 0x10, // Usage (BTN_START)
> + 0x09, 0x12, // Usage (BTN_THUMBL)
> + 0x09, 0x13, // Usage (BTN_THUMBR)
> + 0x09, 0x11, // Usage (BTN_MODE)
> + 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,PreferredState,NoNullPosition)
> + 0x75, 0x01, // Report Size (1)
> + 0x95, 0x03, // Report Count (3)
> + 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,PreferredState,NoNullPosition)
> + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls)
> + 0x25, 0x07, // Logical Maximum (7)
> + 0x46, 0x3B, 0x01, // Physical Maximum (315)
> + 0x75, 0x04, // Report Size (4)
> + 0x95, 0x01, // Report Count (1)
> + 0x65, 0x14, // Unit (System: English Rotation, Length: Centimeter)
> + 0x09, 0x39, // Usage (Hat switch)
> + 0x81, 0x42, // Input (Data,Var,Abs,No Wrap,Linear,PreferredState,NullState)
> + 0x65, 0x00, // Unit (None)
> + 0x95, 0x01, // Report Count (1)
> + 0x81, 0x01, // Input(Const,Array,Abs,NoWrap,Linear,PreferredState,NoNullPosition)
> + 0x26, 0xFF, 0x00, // Logical Maximum (255)
> + 0x46, 0xFF, 0x00, // Physical Maximum (255)
> + 0x09, 0x30, // Usage (X)
> + 0x09, 0x31, // Usage (Y)
> + 0x09, 0x33, // Usage (Rx)
> + 0x09, 0x34, // Usage (Ry)
> + 0x75, 0x08, // Report Size (8)
> + 0x95, 0x04, // Report Count (4)
> + 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,PreferredState,NoNullPosition)
> + 0x95, 0x0A, // Report Count (10)
> + 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,PreferredState,NoNullPosition)
> + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls)
> + 0x26, 0xFF, 0x00, // Logical Maximum (255)
> + 0x46, 0xFF, 0x00, // Physical Maximum (255)
> + 0x09, 0x32, // Usage (Z)
> + 0x09, 0x35, // Usage (Rz)
> + 0x95, 0x02, // Report Count (2)
> + 0x81, 0x02, // Input(Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
> + 0x95, 0x08, // Report Count (8)
> + 0x81, 0x01, // Input(Const,Array,Abs,NoWrap,Linear,PreferredState,NoNullPosition)
> + 0x06, 0x00, 0xFF, // Usage Page (Vendor Defined 0xFF00)
> + 0xB1, 0x02, // Feature(Data,Var,Abs,No Wrap,Linear,PreferredState,NoNullPosition,!volatile)
> + 0x0A, 0x21, 0x26, // Usage (0x2621)
> + 0x95, 0x08, // Report Count (8)
> + 0x91, 0x02, // Output(Data,Var,Abs,No Wrap,Linear,PreferredState,NoNullPosition,!volatile)
> + 0x0A, 0x21, 0x26, // Usage (0x2621)
> + 0x95, 0x08, // Report Count (8)
> + 0x81, 0x02, // Input(Data,Var,Abs,No Wrap,Linear,Preferred State,NoNullPosition)
> + 0xC0, // End Collection
> +};
> +
> +struct shanwan_device {
> + struct hid_report *report;
> +};
> +
> +#ifdef CONFIG_SHANWAN_FF
> +static int shanwan_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
> +{
> + struct hid_device *hid = input_get_drvdata(dev);
> + struct shanwan_device *shanwan = data;
> + struct hid_report *report = shanwan->report;
> +
> + if (effect->type != FF_RUMBLE)
> + return 0;
> +
> + report->field[0]->value[0] = 0x02; /* 2 = rumble effect message */
> + report->field[0]->value[1] = 0x08; /* reserved value, always 8 */
> + if (swap_motors) {
> + /* weak rumble / strong rumble */
> + report->field[0]->value[2] = effect->u.rumble.strong_magnitude / 256;
> + report->field[0]->value[3] = effect->u.rumble.weak_magnitude / 256;
> + } else {
> + /* strong rumble / weak rumble */
> + report->field[0]->value[2] = effect->u.rumble.weak_magnitude / 256;
> + report->field[0]->value[3] = effect->u.rumble.strong_magnitude / 256;
> + }
> + report->field[0]->value[4] = 0xff; /* duration 0-254 (255 = nonstop) */
> + report->field[0]->value[5] = 0x00; /* padding */
> + report->field[0]->value[6] = 0x00; /* padding */
> + report->field[0]->value[7] = 0x00; /* padding */
> + hid_hw_request(hid, report, HID_REQ_SET_REPORT);
I see some other drivers using a workqueue here because ->play() can be
called in atomic context. Are you sure you can sleep here?
> +
> + return 0;
> +}
> +
> +static int shanwan_init_ff(struct hid_device *hid)
> +{
> + struct shanwan_device *shanwan;
> + struct hid_report *report;
> + struct hid_input *hidinput;
> + struct list_head *report_list = &hid->report_enum[HID_OUTPUT_REPORT].report_list;
> + struct input_dev *dev;
> +
> + if (list_empty(&hid->inputs)) {
> + hid_err(hid, "no inputs found\n");
> + return -ENODEV;
> + }
> + hidinput = list_first_entry(&hid->inputs, struct hid_input, list);
> + dev = hidinput->input;
> +
> + if (list_empty(report_list)) {
> + hid_err(hid, "no output reports found\n");
> + return -ENODEV;
> + }
> +
> + report = list_first_entry(report_list, struct hid_report, list);
> +
> + shanwan = kzalloc(sizeof(*shanwan), GFP_KERNEL);
> + if (!shanwan)
> + return -ENOMEM;
> +
> + set_bit(FF_RUMBLE, dev->ffbit);
> +
> + if (input_ff_create_memless(dev, shanwan, shanwan_play_effect)) {
> + kfree(shanwan);
> + return -ENODEV;
> + }
> +
> + shanwan->report = report;
> +
> + return 0;
> +}
> +#else
> +static int shanwan_init_ff(struct hid_device *hid)
> +{
> + return 0;
> +}
> +#endif
> +
> +static int shanwan_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> + int error;
> +
> + error = hid_parse(hdev);
> + if (error) {
> + hid_err(hdev, "parse failed\n");
> + return error;
> + }
> +
> + error = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
> + if (error) {
> + hid_err(hdev, "hw start failed\n");
> + return error;
> + }
> +
> + error = shanwan_init_ff(hdev);
> + if (error)
> + hid_warn(hdev, "Failed to enable force feedback support, error: %d\n", error);
> +
> + error = hid_hw_open(hdev);
What's the point of keeping it opened for the lifetime of the device? Do
you really need this?
> + if (error) {
> + dev_err(&hdev->dev, "hw open failed\n");
> + hid_hw_stop(hdev);
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static void shanwan_remove(struct hid_device *hdev)
> +{
> + hid_hw_close(hdev);
If you can drop the last hid_hw_open/close, then you can entirely skip
the ->remove().
Cheers,
Benjamin
> + hid_hw_stop(hdev);
> +}
> +
> +static __u8 *shanwan_report_fixup(struct hid_device *hid, __u8 *rdesc, unsigned int *rsize)
> +{
> + if (*rsize == PID0575_RDESC_ORIG_SIZE) {
> + rdesc = pid0575_rdesc_fixed;
> + *rsize = sizeof(pid0575_rdesc_fixed);
> + } else {
> + hid_warn(hid, "unexpected rdesc, please submit for review\n");
> + }
> + return rdesc;
> +}
> +
> +static const struct hid_device_id shanwan_devices[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_SHANWAN, USB_PRODUCT_ID_SHANWAN_USB_WIRELESSGAMEPAD) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, shanwan_devices);
> +
> +static struct hid_driver shanwan_driver = {
> + .name = "shanwan",
> + .id_table = shanwan_devices,
> + .probe = shanwan_probe,
> + .report_fixup = shanwan_report_fixup,
> + .remove = shanwan_remove,
> +};
> +module_hid_driver(shanwan_driver);
> +
> +MODULE_AUTHOR("Huseyin BIYIK <huseyinbiyik@hotmail.com>");
> +MODULE_AUTHOR("Ahmad Hasan Mubashshir <ahmubashshir@gmail.com>");
> +MODULE_DESCRIPTION("Force feedback support for Shanwan USB WirelessGamepad");
> +MODULE_LICENSE("GPL");
> --
> 2.40.0
>
^ permalink raw reply
* Re: [PATCH] HID: hid-stadiaff: add support for Stadia force feedback
From: Benjamin Tissoires @ 2023-04-13 16:00 UTC (permalink / raw)
To: Fabio Baltieri; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20230403103324.1746758-1-fabiobaltieri@chromium.org>
Hi,
On Apr 03 2023, Fabio Baltieri wrote:
> Add a hid-stadiaff module to support rumble based force feedback on the
> Google Stadia controller. This works using the HID output endpoint
> exposed on both the USB and BLE interface.
>
> Signed-off-by: Fabio Baltieri <fabiobaltieri@chromium.org>
> ---
>
> Hi, this adds rumble support to the stadia controller using the input
> interface. Up to now this has only been implemented at application level
> using hidraw:
>
> https://source.chromium.org/chromium/chromium/src/+/main:device/gamepad/hid_haptic_gamepad.cc
>
> Tested with fftest, works both over USB and BLE.
>
> drivers/hid/Kconfig | 7 ++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-stadiaff.c | 132 +++++++++++++++++++++++++++++++++++++
Mind renaming this hid-google-stadiaff.c?
It's hard to know that stadia is from Google otherwise.
> 4 files changed, 141 insertions(+)
> create mode 100644 drivers/hid/hid-stadiaff.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 82f64fb31fda..934f73e9b800 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -1031,6 +1031,13 @@ config HID_SPEEDLINK
> help
> Support for Speedlink Vicious and Divine Cezanne mouse.
>
> +config HID_STADIA_FF
> + tristate "Google Stadia force feedback"
> + select INPUT_FF_MEMLESS
> + help
> + Say Y here if you want to enable force feedback support for the Google
> + Stadia controller.
> +
> config HID_STEAM
> tristate "Steam Controller/Deck support"
> select POWER_SUPPLY
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 5d37cacbde33..1d900fa55890 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -120,6 +120,7 @@ obj-$(CONFIG_HID_SIGMAMICRO) += hid-sigmamicro.o
> obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
> obj-$(CONFIG_HID_SONY) += hid-sony.o
> obj-$(CONFIG_HID_SPEEDLINK) += hid-speedlink.o
> +obj-$(CONFIG_HID_STADIA_FF) += hid-stadiaff.o
> obj-$(CONFIG_HID_STEAM) += hid-steam.o
> obj-$(CONFIG_HID_STEELSERIES) += hid-steelseries.o
> obj-$(CONFIG_HID_SUNPLUS) += hid-sunplus.o
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 63545cd307e5..cffd4ac609a0 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -525,6 +525,7 @@
> #define USB_DEVICE_ID_GOOGLE_MOONBALL 0x5044
> #define USB_DEVICE_ID_GOOGLE_DON 0x5050
> #define USB_DEVICE_ID_GOOGLE_EEL 0x5057
> +#define USB_DEVICE_ID_GOOGLE_STADIA 0x9400
>
> #define USB_VENDOR_ID_GOTOP 0x08f2
> #define USB_DEVICE_ID_SUPER_Q2 0x007f
> diff --git a/drivers/hid/hid-stadiaff.c b/drivers/hid/hid-stadiaff.c
> new file mode 100644
> index 000000000000..f974b9e24d46
> --- /dev/null
> +++ b/drivers/hid/hid-stadiaff.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Stadia controller rumble support.
> + *
> + * Copyright 2023 Google LLC
> + */
> +
> +#include <linux/hid.h>
> +#include <linux/input.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +
> +#include "hid-ids.h"
> +
> +#define STADIA_FF_REPORT_ID 5
> +
> +struct stadiaff_device {
> + struct hid_device *hid;
> + struct hid_report *report;
> + struct work_struct work;
> +};
> +
> +static void stadiaff_work(struct work_struct *work)
> +{
> + struct stadiaff_device *stadiaff =
> + container_of(work, struct stadiaff_device, work);
> +
> + hid_hw_request(stadiaff->hid, stadiaff->report, HID_REQ_SET_REPORT);
> +}
> +
> +static int stadiaff_play(struct input_dev *dev, void *data,
> + struct ff_effect *effect)
> +{
> + struct hid_device *hid = input_get_drvdata(dev);
> + struct stadiaff_device *stadiaff = hid_get_drvdata(hid);
> + struct hid_field *rumble_field = stadiaff->report->field[0];
> +
> + rumble_field->value[0] = effect->u.rumble.strong_magnitude;
> + rumble_field->value[1] = effect->u.rumble.weak_magnitude;
> +
> + schedule_work(&stadiaff->work);
It seems weird that you don't have any locking in place here.
What if you are sending a report (in stadiaff_work) while having your
_play() function called at the same time?
> +
> + return 0;
> +}
> +
> +static int stadiaff_init(struct hid_device *hid)
> +{
> + struct stadiaff_device *stadiaff;
> + struct hid_report *report;
> + struct hid_input *hidinput;
> + struct input_dev *dev;
> + int error;
> +
> + if (list_empty(&hid->inputs)) {
> + hid_err(hid, "no inputs found\n");
> + return -ENODEV;
> + }
> + hidinput = list_entry(hid->inputs.next, struct hid_input, list);
> + dev = hidinput->input;
> +
> + report = hid_validate_values(hid, HID_OUTPUT_REPORT,
> + STADIA_FF_REPORT_ID, 0, 2);
> + if (!report)
> + return -ENODEV;
> +
> + stadiaff = devm_kzalloc(&hid->dev, sizeof(struct stadiaff_device),
> + GFP_KERNEL);
> + if (!stadiaff)
> + return -ENOMEM;
> +
> + hid_set_drvdata(hid, stadiaff);
> +
> + input_set_capability(dev, EV_FF, FF_RUMBLE);
> +
> + error = input_ff_create_memless(dev, NULL, stadiaff_play);
> + if (error)
> + return error;
> +
> + stadiaff->hid = hid;
> + stadiaff->report = report;
> + INIT_WORK(&stadiaff->work, stadiaff_work);
> +
> + hid_info(hid, "Force Feedback for Google Stadia controller\n");
> +
> + return 0;
> +}
> +
> +static int stadia_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> + int ret;
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "parse failed\n");
> + return ret;
> + }
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
> + if (ret) {
> + hid_err(hdev, "hw start failed\n");
> + return ret;
> + }
> +
> + stadiaff_init(hdev);
> +
> + return 0;
> +}
> +
> +static void stadia_remove(struct hid_device *hid)
> +{
> + struct stadiaff_device *stadiaff = hid_get_drvdata(hid);
> +
> + cancel_work_sync(&stadiaff->work);
What if you have a ff play event scheduled right here? Don't you need
some way of forcing the work to not be scheduled once again?
> + hid_hw_stop(hid);
> +}
> +
> +static const struct hid_device_id stadia_devices[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_STADIA) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_STADIA) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, stadia_devices);
> +
> +static struct hid_driver stadia_driver = {
> + .name = "stadia",
> + .id_table = stadia_devices,
> + .probe = stadia_probe,
> + .remove = stadia_remove,
> +};
> +module_hid_driver(stadia_driver);
> +
> +MODULE_LICENSE("GPL");
> --
> 2.40.0.348.gf938b09366-goog
>
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH v1] HID: Ignore battery for ELAN touchscreen on ROG Flow X13 GV301RA
From: Benjamin Tissoires @ 2023-04-13 15:26 UTC (permalink / raw)
To: jikos, weiliang1503; +Cc: rydberg, linux-input, linux-kernel
In-Reply-To: <20230330115638.16146-1-weiliang1503@gmail.com>
On Thu, 30 Mar 2023 19:56:38 +0800, weiliang1503 wrote:
> Ignore the reported battery level of the built-in touchscreen to suppress
> battery warnings when a stylus is used. The device ID was added and the
> battery ignore quirk was enabled.
>
>
Applied to hid/hid.git (for-6.4/core), thanks!
[1/1] HID: Ignore battery for ELAN touchscreen on ROG Flow X13 GV301RA
https://git.kernel.org/hid/hid/c/35903009dbde
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* Re: [PATCH v2 1/1] HID: shield: Initial driver implementation with Thunderstrike support
From: Benjamin Tissoires @ 2023-04-13 15:16 UTC (permalink / raw)
To: Rahul Rameshbabu; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20230410170840.16119-2-rrameshbabu@nvidia.com>
Hi,
On Apr 10 2023, Rahul Rameshbabu wrote:
> Supports the Thunderstrike (SHIELD 2017) controller. Implements support for
> the Thunderstrike HOSTCMD firmware interface. Adds sysfs attributes about a
> SHIELD device and introduces haptics support for controllers.
>
> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> ---
> MAINTAINERS | 6 +
> drivers/hid/Kconfig | 18 ++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 3 +
> drivers/hid/hid-shield.c | 587 +++++++++++++++++++++++++++++++++++++++
In addition to what Jiri said, would you mind changing the name to
hid-nvidia-shield.c or just hid-nvidia.c?
The "normal" naming scheme in the hid tree is to group devices by
vendors, and TBH, knowing that the "shield" is from Nvidia is not
necessarily obvious.
Cheers,
Benjamin
> 5 files changed, 615 insertions(+)
> create mode 100644 drivers/hid/hid-shield.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2b073facf399..4fa673401bc7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9122,6 +9122,12 @@ F: drivers/hid/hid-sensor-*
> F: drivers/iio/*/hid-*
> F: include/linux/hid-sensor-*
>
> +HID SHIELD DRIVER
> +M: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> +L: linux-input@vger.kernel.org
> +S: Maintained
> +F: drivers/hid/hid-shield.c
> +
> HID VRC-2 CAR CONTROLLER DRIVER
> M: Marcus Folkesson <marcus.folkesson@gmail.com>
> L: linux-input@vger.kernel.org
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 82f64fb31fda..eb19debaa1f5 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -990,6 +990,24 @@ config HID_SEMITEK
> - Woo-dy
> - X-Bows Nature/Knight
>
> +config HID_SHIELD
> + tristate "SHIELD HID Driver"
> + depends on USB_HID
> + depends on BT_HIDP
> + help
> + Support for NVIDIA SHIELD accessories.
> +
> + Supported devices:
> + - Thunderstrike (NVIDIA SHIELD Controller 2017)
> +
> +config SHIELD_FF
> + bool "SHIELD force feedback support"
> + depends on HID_SHIELD
> + select INPUT_FF_MEMLESS
> + help
> + Say Y here if you would like to enable force feedback support for
> + NVIDIA SHIELD accessories with haptics capabilities.
> +
> config HID_SIGMAMICRO
> tristate "SiGma Micro-based keyboards"
> depends on USB_HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 5d37cacbde33..a2ed1db9ed9d 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -116,6 +116,7 @@ obj-$(CONFIG_HID_RMI) += hid-rmi.o
> obj-$(CONFIG_HID_SAITEK) += hid-saitek.o
> obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
> obj-$(CONFIG_HID_SEMITEK) += hid-semitek.o
> +obj-$(CONFIG_HID_SHIELD) += hid-shield.o
> obj-$(CONFIG_HID_SIGMAMICRO) += hid-sigmamicro.o
> obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
> obj-$(CONFIG_HID_SONY) += hid-sony.o
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 63545cd307e5..4000d53b1bac 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -993,6 +993,9 @@
> #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_18 0x0014
> #define USB_DEVICE_ID_NTRIG_DUOSENSE 0x1500
>
> +#define USB_VENDOR_ID_NVIDIA 0x0955
> +#define USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER 0x7214
> +
> #define USB_VENDOR_ID_ONTRAK 0x0a07
> #define USB_DEVICE_ID_ONTRAK_ADU100 0x0064
>
> diff --git a/drivers/hid/hid-shield.c b/drivers/hid/hid-shield.c
> new file mode 100644
> index 000000000000..3befa8f133c7
> --- /dev/null
> +++ b/drivers/hid/hid-shield.c
> @@ -0,0 +1,587 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> + *
> + * HID driver for NVIDIA SHIELD peripherals.
> + */
> +
> +#include <linux/hid.h>
> +#include <linux/input-event-codes.h>
> +#include <linux/input.h>
> +#include <linux/module.h>
> +#include <linux/spinlock.h>
> +#include <linux/workqueue.h>
> +
> +#include "hid-ids.h"
> +
> +#define NOT_INIT_STR "NOT INITIALIZED"
> +
> +enum {
> + SHIELD_FW_VERSION_INITIALIZED = 0,
> + SHIELD_BOARD_INFO_INITIALIZED,
> +};
> +
> +enum {
> + THUNDERSTRIKE_FW_VERSION_UPDATE = 0,
> + THUNDERSTRIKE_BOARD_INFO_UPDATE,
> + THUNDERSTRIKE_HAPTICS_UPDATE,
> +};
> +
> +enum {
> + THUNDERSTRIKE_HOSTCMD_REPORT_SIZE = 33,
> + THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID = 0x4,
> + THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID = 0x3,
> +};
> +
> +enum {
> + THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION = 1,
> + THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO = 16,
> + THUNDERSTRIKE_HOSTCMD_ID_USB_INIT = 53,
> + THUNDERSTRIKE_HOSTCMD_ID_HAPTICS = 57,
> + THUNDERSTRIKE_HOSTCMD_ID_BLUETOOTH_INIT = 58,
> +};
> +
> +struct thunderstrike_hostcmd_board_info {
> + __le16 revision;
> + __le16 serial[7];
> +};
> +
> +struct thunderstrike_hostcmd_haptics {
> + u8 motor_left;
> + u8 motor_right;
> +};
> +
> +struct thunderstrike_hostcmd_resp_report {
> + u8 report_id; /* THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID */
> + u8 cmd_id;
> + u8 reserved_at_10;
> +
> + union {
> + struct thunderstrike_hostcmd_board_info board_info;
> + struct thunderstrike_hostcmd_haptics motors;
> + __le16 fw_version;
> + u8 payload[30];
> + };
> +} __packed;
> +static_assert(sizeof(struct thunderstrike_hostcmd_resp_report) ==
> + THUNDERSTRIKE_HOSTCMD_REPORT_SIZE);
> +
> +struct thunderstrike_hostcmd_req_report {
> + u8 report_id; /* THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID */
> + u8 cmd_id;
> + u8 reserved_at_10;
> +
> + struct {
> + u8 update;
> + struct thunderstrike_hostcmd_haptics motors;
> + } haptics;
> + u8 reserved_at_30[27];
> +} __packed;
> +static_assert(sizeof(struct thunderstrike_hostcmd_req_report) ==
> + THUNDERSTRIKE_HOSTCMD_REPORT_SIZE);
> +
> +/* Common struct for shield accessories. */
> +struct shield_device {
> + struct hid_device *hdev;
> +
> + unsigned long initialized_flags;
> + const char *codename;
> + u16 fw_version;
> + struct {
> + u16 revision;
> + char serial_number[15];
> + } board_info;
> +};
> +
> +struct thunderstrike {
> + struct shield_device base;
> +
> + /* Sub-devices */
> + struct input_dev *haptics_dev;
> +
> + /* Resources */
> + void *req_report_dmabuf;
> + unsigned long update_flags;
> + struct thunderstrike_hostcmd_haptics haptics_val;
> + spinlock_t haptics_update_lock;
> + struct work_struct hostcmd_req_work;
> +};
> +
> +static inline void thunderstrike_hostcmd_req_report_init(
> + struct thunderstrike_hostcmd_req_report *report, u8 cmd_id)
> +{
> + memset(report, 0, sizeof(*report));
> + report->report_id = THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID;
> + report->cmd_id = cmd_id;
> +}
> +
> +static inline void shield_strrev(char *dest, size_t len, u16 rev)
> +{
> + dest[0] = ('A' - 1) + (rev >> 8);
> + snprintf(&dest[1], len - 1, "%02X", 0xff & rev);
> +}
> +
> +static struct input_dev *shield_allocate_input_dev(struct hid_device *hdev,
> + const char *name_suffix)
> +{
> + struct input_dev *idev;
> +
> + idev = input_allocate_device();
> + if (!idev)
> + goto err_device;
> +
> + idev->id.bustype = hdev->bus;
> + idev->id.vendor = hdev->vendor;
> + idev->id.product = hdev->product;
> + idev->id.version = hdev->version;
> + idev->uniq = hdev->uniq;
> + idev->name = devm_kasprintf(&idev->dev, GFP_KERNEL, "%s %s", hdev->name,
> + name_suffix);
> + if (!idev->name)
> + goto err_name;
> +
> + input_set_drvdata(idev, hdev);
> +
> + return idev;
> +
> +err_name:
> + input_free_device(idev);
> +err_device:
> + return ERR_PTR(-ENOMEM);
> +}
> +
> +static struct input_dev *shield_haptics_create(
> + struct shield_device *dev,
> + int (*play_effect)(struct input_dev *, void *, struct ff_effect *))
> +{
> + struct input_dev *haptics;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_SHIELD_FF))
> + return NULL;
> +
> + haptics = shield_allocate_input_dev(dev->hdev, "Haptics");
> + if (IS_ERR(haptics))
> + return haptics;
> +
> + input_set_capability(haptics, EV_FF, FF_RUMBLE);
> + input_ff_create_memless(haptics, NULL, play_effect);
> +
> + ret = input_register_device(haptics);
> + if (ret)
> + goto err;
> +
> + return haptics;
> +
> + err:
> + input_free_device(haptics);
> + return ERR_PTR(ret);
> +}
> +
> +static inline void thunderstrike_send_hostcmd_request(struct thunderstrike *ts)
> +{
> + struct thunderstrike_hostcmd_req_report *report = ts->req_report_dmabuf;
> + struct shield_device *shield_dev = &ts->base;
> + int ret;
> +
> + ret = hid_hw_raw_request(shield_dev->hdev, report->report_id,
> + ts->req_report_dmabuf,
> + THUNDERSTRIKE_HOSTCMD_REPORT_SIZE,
> + HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
> +
> + if (ret < 0) {
> + hid_err(shield_dev->hdev,
> + "Failed to output Thunderstrike HOSTCMD request HID report due to %pe\n",
> + ERR_PTR(ret));
> + }
> +}
> +
> +static void thunderstrike_hostcmd_req_work_handler(struct work_struct *work)
> +{
> + struct thunderstrike *ts =
> + container_of(work, struct thunderstrike, hostcmd_req_work);
> + struct thunderstrike_hostcmd_req_report *report;
> + unsigned long flags;
> +
> + report = ts->req_report_dmabuf;
> +
> + if (test_and_clear_bit(THUNDERSTRIKE_FW_VERSION_UPDATE, &ts->update_flags)) {
> + thunderstrike_hostcmd_req_report_init(
> + report, THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION);
> + thunderstrike_send_hostcmd_request(ts);
> + }
> +
> + if (test_and_clear_bit(THUNDERSTRIKE_BOARD_INFO_UPDATE, &ts->update_flags)) {
> + thunderstrike_hostcmd_req_report_init(
> + report, THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO);
> + thunderstrike_send_hostcmd_request(ts);
> + }
> +
> + if (test_and_clear_bit(THUNDERSTRIKE_HAPTICS_UPDATE, &ts->update_flags)) {
> + thunderstrike_hostcmd_req_report_init(
> + report, THUNDERSTRIKE_HOSTCMD_ID_HAPTICS);
> +
> + report->haptics.update = 1;
> + spin_lock_irqsave(&ts->haptics_update_lock, flags);
> + report->haptics.motors = ts->haptics_val;
> + spin_unlock_irqrestore(&ts->haptics_update_lock, flags);
> +
> + thunderstrike_send_hostcmd_request(ts);
> + }
> +}
> +
> +static inline void thunderstrike_request_firmware_version(struct thunderstrike *ts)
> +{
> + set_bit(THUNDERSTRIKE_FW_VERSION_UPDATE, &ts->update_flags);
> + schedule_work(&ts->hostcmd_req_work);
> +}
> +
> +static inline void thunderstrike_request_board_info(struct thunderstrike *ts)
> +{
> + set_bit(THUNDERSTRIKE_BOARD_INFO_UPDATE, &ts->update_flags);
> + schedule_work(&ts->hostcmd_req_work);
> +}
> +
> +static inline int
> +thunderstrike_update_haptics(struct thunderstrike *ts,
> + struct thunderstrike_hostcmd_haptics *motors)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ts->haptics_update_lock, flags);
> + ts->haptics_val = *motors;
> + spin_unlock_irqrestore(&ts->haptics_update_lock, flags);
> +
> + set_bit(THUNDERSTRIKE_HAPTICS_UPDATE, &ts->update_flags);
> + schedule_work(&ts->hostcmd_req_work);
> +
> + return 0;
> +}
> +
> +static int thunderstrike_play_effect(struct input_dev *idev, void *data,
> + struct ff_effect *effect)
> +{
> + struct hid_device *hdev = input_get_drvdata(idev);
> + struct thunderstrike_hostcmd_haptics motors;
> + struct shield_device *shield_dev;
> + struct thunderstrike *ts;
> +
> + if (effect->type != FF_RUMBLE)
> + return 0;
> +
> + shield_dev = hid_get_drvdata(hdev);
> + ts = container_of(shield_dev, struct thunderstrike, base);
> +
> + /* Thunderstrike motor values range from 0 to 32 inclusively */
> + motors.motor_left = effect->u.rumble.strong_magnitude / 2047;
> + motors.motor_right = effect->u.rumble.weak_magnitude / 2047;
> +
> + hid_dbg(hdev, "Thunderstrike FF_RUMBLE request, left: %u right: %u\n",
> + motors.motor_left, motors.motor_right);
> +
> + return thunderstrike_update_haptics(ts, &motors);
> +}
> +
> +static void
> +thunderstrike_parse_fw_version_payload(struct shield_device *shield_dev,
> + __le16 fw_version)
> +{
> + shield_dev->fw_version = le16_to_cpu(fw_version);
> +
> + set_bit(SHIELD_FW_VERSION_INITIALIZED, &shield_dev->initialized_flags);
> +
> + hid_dbg(shield_dev->hdev, "Thunderstrike firmware version 0x%04X\n",
> + shield_dev->fw_version);
> +}
> +
> +static void
> +thunderstrike_parse_board_info_payload(struct shield_device *shield_dev,
> + struct thunderstrike_hostcmd_board_info *board_info)
> +{
> + char board_revision_str[4];
> + int i;
> +
> + shield_dev->board_info.revision = le16_to_cpu(board_info->revision);
> + for (i = 0; i < 7; ++i) {
> + u16 val = le16_to_cpu(board_info->serial[i]);
> +
> + shield_dev->board_info.serial_number[2 * i] = val & 0xFF;
> + shield_dev->board_info.serial_number[2 * i + 1] = val >> 8;
> + }
> + shield_dev->board_info.serial_number[14] = '\0';
> +
> + set_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags);
> +
> + shield_strrev(board_revision_str, 4, shield_dev->board_info.revision);
> + hid_dbg(shield_dev->hdev,
> + "Thunderstrike BOARD_REVISION_%s (0x%04X) S/N: %s\n",
> + board_revision_str, shield_dev->board_info.revision,
> + shield_dev->board_info.serial_number);
> +}
> +
> +static inline void
> +thunderstrike_parse_haptics_payload(struct shield_device *shield_dev,
> + struct thunderstrike_hostcmd_haptics *haptics)
> +{
> + hid_dbg(shield_dev->hdev,
> + "Thunderstrike haptics HOSTCMD response, left: %u right: %u\n",
> + haptics->motor_left, haptics->motor_right);
> +}
> +
> +static int thunderstrike_parse_report(struct shield_device *shield_dev,
> + struct hid_report *report, u8 *data,
> + int size)
> +{
> + struct thunderstrike_hostcmd_resp_report *hostcmd_resp_report;
> + struct thunderstrike *ts =
> + container_of(shield_dev, struct thunderstrike, base);
> + struct hid_device *hdev = shield_dev->hdev;
> +
> + switch (report->id) {
> + case THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID:
> + if (size != THUNDERSTRIKE_HOSTCMD_REPORT_SIZE) {
> + hid_err(hdev,
> + "Encountered Thunderstrike HOSTCMD HID report with unexpected size %d\n",
> + size);
> + return -EINVAL;
> + }
> +
> + hostcmd_resp_report =
> + (struct thunderstrike_hostcmd_resp_report *)data;
> +
> + switch (hostcmd_resp_report->cmd_id) {
> + case THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION:
> + thunderstrike_parse_fw_version_payload(
> + shield_dev, hostcmd_resp_report->fw_version);
> + break;
> + case THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO:
> + thunderstrike_parse_board_info_payload(
> + shield_dev, &hostcmd_resp_report->board_info);
> + break;
> + case THUNDERSTRIKE_HOSTCMD_ID_HAPTICS:
> + thunderstrike_parse_haptics_payload(
> + shield_dev, &hostcmd_resp_report->motors);
> + break;
> +
> + case THUNDERSTRIKE_HOSTCMD_ID_USB_INIT:
> + case THUNDERSTRIKE_HOSTCMD_ID_BLUETOOTH_INIT:
> + /* May block HOSTCMD requests till received initially */
> + thunderstrike_request_firmware_version(ts);
> + thunderstrike_request_board_info(ts);
> + /* Only HOSTCMD that can be triggered without a request */
> + return 0;
> + default:
> + hid_warn(hdev,
> + "Unhandled Thunderstrike HOSTCMD id %d\n",
> + hostcmd_resp_report->cmd_id);
> + return -ENOENT;
> + }
> +
> + break;
> + default:
> + return 0;
> + }
> +
> + return 0;
> +}
> +
> +static struct shield_device *thunderstrike_create(struct hid_device *hdev)
> +{
> + struct shield_device *shield_dev;
> + struct thunderstrike *ts;
> +
> + ts = devm_kzalloc(&hdev->dev, sizeof(*ts), GFP_KERNEL);
> + if (!ts)
> + return ERR_PTR(-ENOMEM);
> +
> + ts->req_report_dmabuf = devm_kzalloc(
> + &hdev->dev, THUNDERSTRIKE_HOSTCMD_REPORT_SIZE, GFP_KERNEL);
> + if (!ts->req_report_dmabuf)
> + return ERR_PTR(-ENOMEM);
> +
> + shield_dev = &ts->base;
> + shield_dev->hdev = hdev;
> + shield_dev->codename = "Thunderstrike";
> +
> + spin_lock_init(&ts->haptics_update_lock);
> + INIT_WORK(&ts->hostcmd_req_work, thunderstrike_hostcmd_req_work_handler);
> +
> + hid_set_drvdata(hdev, shield_dev);
> +
> + ts->haptics_dev = shield_haptics_create(shield_dev, thunderstrike_play_effect);
> + if (IS_ERR(ts->haptics_dev))
> + return ERR_CAST(ts->haptics_dev);
> +
> + hid_info(hdev, "Registered Thunderstrike controller\n");
> + return shield_dev;
> +}
> +
> +static ssize_t firmware_version_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct hid_device *hdev = to_hid_device(dev);
> + struct shield_device *shield_dev;
> + int ret;
> +
> + shield_dev = hid_get_drvdata(hdev);
> +
> + if (test_bit(SHIELD_FW_VERSION_INITIALIZED, &shield_dev->initialized_flags))
> + ret = sysfs_emit(buf, "0x%04X\n", shield_dev->fw_version);
> + else
> + ret = sysfs_emit(buf, NOT_INIT_STR "\n");
> +
> + return ret;
> +}
> +
> +static DEVICE_ATTR_RO(firmware_version);
> +
> +static ssize_t hardware_version_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct hid_device *hdev = to_hid_device(dev);
> + struct shield_device *shield_dev;
> + char board_revision_str[4];
> + int ret;
> +
> + shield_dev = hid_get_drvdata(hdev);
> +
> + if (test_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags)) {
> + shield_strrev(board_revision_str, 4, shield_dev->board_info.revision);
> + ret = sysfs_emit(buf, "%s BOARD_REVISION_%s (0x%04X)\n",
> + shield_dev->codename, board_revision_str,
> + shield_dev->board_info.revision);
> + } else
> + ret = sysfs_emit(buf, NOT_INIT_STR "\n");
> +
> + return ret;
> +}
> +
> +static DEVICE_ATTR_RO(hardware_version);
> +
> +static ssize_t serial_number_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct hid_device *hdev = to_hid_device(dev);
> + struct shield_device *shield_dev;
> + int ret;
> +
> + shield_dev = hid_get_drvdata(hdev);
> +
> + if (test_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags))
> + ret = sysfs_emit(buf, "%s\n", shield_dev->board_info.serial_number);
> + else
> + ret = sysfs_emit(buf, NOT_INIT_STR "\n");
> +
> + return ret;
> +}
> +
> +static DEVICE_ATTR_RO(serial_number);
> +
> +static struct attribute *shield_device_attrs[] = {
> + &dev_attr_firmware_version.attr,
> + &dev_attr_hardware_version.attr,
> + &dev_attr_serial_number.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(shield_device);
> +
> +static int shield_raw_event(struct hid_device *hdev, struct hid_report *report,
> + u8 *data, int size)
> +{
> + struct shield_device *dev = hid_get_drvdata(hdev);
> +
> + return thunderstrike_parse_report(dev, report, data, size);
> +}
> +
> +static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
> +{
> + struct shield_device *shield_dev = NULL;
> + struct thunderstrike *ts;
> + int ret;
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev, "Parse failed\n");
> + return ret;
> + }
> +
> + switch (id->product) {
> + case USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER:
> + shield_dev = thunderstrike_create(hdev);
> + break;
> + }
> +
> + if (unlikely(!shield_dev)) {
> + hid_err(hdev, "Failed to identify SHIELD device\n");
> + return -ENODEV;
> + }
> + if (IS_ERR(shield_dev)) {
> + hid_err(hdev, "Failed to create SHIELD device\n");
> + return PTR_ERR(shield_dev);
> + }
> +
> + ts = container_of(shield_dev, struct thunderstrike, base);
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_HIDINPUT);
> + if (ret) {
> + hid_err(hdev, "Failed to start HID device\n");
> + goto err_haptics;
> + }
> +
> + ret = hid_hw_open(hdev);
> + if (ret) {
> + hid_err(hdev, "Failed to open HID device\n");
> + goto err_stop;
> + }
> +
> + thunderstrike_request_firmware_version(ts);
> + thunderstrike_request_board_info(ts);
> +
> + return ret;
> +
> +err_stop:
> + hid_hw_stop(hdev);
> +err_haptics:
> + if (ts->haptics_dev)
> + input_unregister_device(ts->haptics_dev);
> + return ret;
> +}
> +
> +static void shield_remove(struct hid_device *hdev)
> +{
> + struct shield_device *dev = hid_get_drvdata(hdev);
> + struct thunderstrike *ts;
> +
> + ts = container_of(dev, struct thunderstrike, base);
> +
> + hid_hw_close(hdev);
> + if (ts->haptics_dev)
> + input_unregister_device(ts->haptics_dev);
> + cancel_work_sync(&ts->hostcmd_req_work);
> + hid_hw_stop(hdev);
> +}
> +
> +static const struct hid_device_id shield_devices[] = {
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NVIDIA,
> + USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_NVIDIA,
> + USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER) },
> + {}
> +};
> +MODULE_DEVICE_TABLE(hid, shield_devices);
> +
> +static struct hid_driver shield_driver = {
> + .name = "shield",
> + .id_table = shield_devices,
> + .probe = shield_probe,
> + .remove = shield_remove,
> + .raw_event = shield_raw_event,
> + .driver = {
> + .dev_groups = shield_device_groups,
> + },
> +};
> +module_hid_driver(shield_driver);
> +
> +MODULE_AUTHOR("Rahul Rameshbabu <rrameshbabu@nvidia.com>");
> +MODULE_DESCRIPTION("HID Driver for NVIDIA SHIELD peripherals.");
> +MODULE_LICENSE("GPL");
> --
> 2.38.4
>
^ permalink raw reply
* Re: [PATCH 0/4] HID: explicitly include linux/leds.h
From: Benjamin Tissoires @ 2023-04-13 15:11 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Jiri Kosina, Hans de Goede, linux-input, linux-kernel
In-Reply-To: <7fb9e268-189a-47d5-8737-310d9b6f0f35@t-8ch.de>
On Mar 20 2023, Thomas Weißschuh wrote:
> Friendly ping,
>
> this seems to have fallen through the cracks.
Sorry for the delay.
I have applied them now, as should have told you the automatic
notifications.
Cheers,
Benjamin
>
> Thanks,
> Thomas
>
> On Wed, Feb 15, 2023 at 01:03:33AM +0000, Thomas Weißschuh wrote:
> > Instead of relying on an accidental, transitive inclusion of linux/leds.h
> > use it directly.
> >
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> > Thomas Weißschuh (4):
> > HID: steelseries: explicitly include linux/leds.h
> > HID: lg-g15: explicitly include linux/leds.h
> > HID: asus: explicitly include linux/leds.h
> > HID: apple: explicitly include linux/leds.h
> >
> > drivers/hid/hid-apple.c | 1 +
> > drivers/hid/hid-asus.c | 1 +
> > drivers/hid/hid-lg-g15.c | 1 +
> > drivers/hid/hid-steelseries.c | 1 +
> > 4 files changed, 4 insertions(+)
> > ---
> > base-commit: e1c04510f521e853019afeca2a5991a5ef8d6a5b
> > change-id: 20230215-power_supply-leds-hid-f99a0a3fd2bf
> >
> > Best regards,
> > --
> > Thomas Weißschuh <linux@weissschuh.net>
> >
^ permalink raw reply
* Re: (subset) [PATCH 0/4] HID: explicitly include linux/leds.h
From: Benjamin Tissoires @ 2023-04-13 15:10 UTC (permalink / raw)
To: Jiri Kosina, Hans de Goede, Thomas Weißschuh
Cc: linux-input, linux-kernel
In-Reply-To: <20230215-power_supply-leds-hid-v1-0-35b6f1dcee8a@weissschuh.net>
On Wed, 15 Feb 2023 01:03:33 +0000, Thomas Weißschuh wrote:
> Instead of relying on an accidental, transitive inclusion of linux/leds.h
> use it directly.
>
>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-6.4/led-includes), thanks!
[1/4] HID: steelseries: explicitly include linux/leds.h
https://git.kernel.org/hid/hid/c/f28bb5ce4f15
[2/4] HID: lg-g15: explicitly include linux/leds.h
https://git.kernel.org/hid/hid/c/21c5bd5b4b06
[3/4] HID: asus: explicitly include linux/leds.h
https://git.kernel.org/hid/hid/c/a2654c1f640c
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* Re: [PATCH v2 1/1] HID: shield: Initial driver implementation with Thunderstrike support
From: Jiri Kosina @ 2023-04-13 15:07 UTC (permalink / raw)
To: Rahul Rameshbabu; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20230410170840.16119-2-rrameshbabu@nvidia.com>
On Mon, 10 Apr 2023, Rahul Rameshbabu wrote:
> Supports the Thunderstrike (SHIELD 2017) controller. Implements support for
> the Thunderstrike HOSTCMD firmware interface. Adds sysfs attributes about a
> SHIELD device and introduces haptics support for controllers.
>
> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Thanks a lot for submitting the driver. I have one minor question:
[ ... snip ... ]
> +thunderstrike_update_haptics(struct thunderstrike *ts,
> + struct thunderstrike_hostcmd_haptics *motors)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&ts->haptics_update_lock, flags);
> + ts->haptics_val = *motors;
> + spin_unlock_irqrestore(&ts->haptics_update_lock, flags);
Do we really have to disable interrupts when taking haptics_update_lock?
Is it ever being taken from interrupt context?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: (subset) [PATCH 0/4] HID: explicitly include linux/leds.h
From: Benjamin Tissoires @ 2023-04-13 15:04 UTC (permalink / raw)
To: Jiri Kosina, Hans de Goede, Thomas Weißschuh
Cc: linux-input, linux-kernel
In-Reply-To: <20230215-power_supply-leds-hid-v1-0-35b6f1dcee8a@weissschuh.net>
On Wed, 15 Feb 2023 01:03:33 +0000, Thomas Weißschuh wrote:
> Instead of relying on an accidental, transitive inclusion of linux/leds.h
> use it directly.
>
>
Applied to hid/hid.git (for-6.4/apple), thanks!
[4/4] HID: apple: explicitly include linux/leds.h
https://git.kernel.org/hid/hid/c/6c89c1160321
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* Re: [PATCH] HID: apple: Set the tilde quirk flag on the Geyser 3
From: Benjamin Tissoires @ 2023-04-13 15:04 UTC (permalink / raw)
To: linux-input, jkosina, Alex Henrie
In-Reply-To: <20230404024829.13982-1-alexhenrie24@gmail.com>
On Mon, 03 Apr 2023 20:48:29 -0600, Alex Henrie wrote:
> I was finally able to obtain a MacBook1,1 to test and I've now confirmed
> that it has the tilde key quirk as well:
>
> Product Model Year System CPU Shape Labels Country Quirky
> ============================================================================
> 05ac:0218 A1181 2006 MacBook1,1 T2500 ISO British 13 Yes
>
> [...]
Applied to hid/hid.git (for-6.4/apple), thanks!
[1/1] HID: apple: Set the tilde quirk flag on the Geyser 3
https://git.kernel.org/hid/hid/c/29e1ecc197d4
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* Re: [PATCH 0/2] Fixes for the mcp2221 gpiochip get_* calls
From: Benjamin Tissoires @ 2023-04-13 14:49 UTC (permalink / raw)
To: Louis Morhet; +Cc: gupt21, jikos, linux-i2c, linux-input, linux-kernel
In-Reply-To: <cover.1680602387.git.lmorhet@kalrayinc.com>
On Apr 04 2023, Louis Morhet wrote:
> The mcp2221 HID driver exposes a gpiochip device.
> While gpioset seemed to work fine, gpioget always returned 1 on all 4
> GPIOs of the component (0x01 for input in the field "direction",
> according to the documentation).
>
> This patch series addresses this issue by fixing the order of the fields
> in the driver's representation of the chip answer, and adding
> consistency to the way the callbacks prepare their command and the way
> the hid event handler gets these fields.
> The set callbacks use a similar mechanism, but work for now because
> setting a direction/value only requires gpio-specific positioning in the
> command preparation, and not in the raw_event handler.
As you should have seen in the automatic replied, I took that series in
because it seems to fix a rather worrying bug.
>
> The core of this issue being a discrepancy in the way the command and
> the answer fetch their fields of interest, I would also like to ask if
> we should uniformize a bit the way this driver handles gpio, and how.
> I thought about several possible implementations for that:
> Use mcp->gp_idx as the base offset of the gpio for set too, and modify
> the raw_event handler to fetch all relevant data based on that; or set
> the buffers in the mcp structure as unions of the various commands
> handled and use gp_idx simply as the gpio index to access relevant data
> directly from the structured representation everywhere; or go back to ye
> old defines to ensure portability...
Honestly, it's hard to make a choice here. You haven't got a replied
from the other mcp2221 folks in almost 10 days so I am not sure you'll
get feedback directly.
May I suggest that you work on any one of these idea, and then submit a
series? Generally, having the code ready makes it way easier to decide
if this is a good solution or not, while having 3 different vague
suggestions makes it hard to see the implications of them.
Also, just a side note, this driver is very limited in term of scope, as
it only touches one dedicated device. Which means that whatever solution
feels the more elegant to you has a good chance of being accepted :)
Cheers,
Benjamin
>
> Louis Morhet (2):
> HID: mcp2221: fix report layout for gpio get
> HID: mcp2221: fix get and get_direction for gpio
>
> drivers/hid/hid-mcp2221.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> --
> 2.17.1
>
>
>
>
>
^ permalink raw reply
* Re: [PATCH 0/2] Fixes for the mcp2221 gpiochip get_* calls
From: Benjamin Tissoires @ 2023-04-13 14:43 UTC (permalink / raw)
To: gupt21, jikos, Louis Morhet; +Cc: linux-i2c, linux-input, linux-kernel
In-Reply-To: <cover.1680602387.git.lmorhet@kalrayinc.com>
On Tue, 04 Apr 2023 14:14:58 +0200, Louis Morhet wrote:
> The mcp2221 HID driver exposes a gpiochip device.
> While gpioset seemed to work fine, gpioget always returned 1 on all 4
> GPIOs of the component (0x01 for input in the field "direction",
> according to the documentation).
>
> This patch series addresses this issue by fixing the order of the fields
> in the driver's representation of the chip answer, and adding
> consistency to the way the callbacks prepare their command and the way
> the hid event handler gets these fields.
> The set callbacks use a similar mechanism, but work for now because
> setting a direction/value only requires gpio-specific positioning in the
> command preparation, and not in the raw_event handler.
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-6.4/mcp2221), thanks!
[1/2] HID: mcp2221: fix report layout for gpio get
https://git.kernel.org/hid/hid/c/e36c31f8cac5
[2/2] HID: mcp2221: fix get and get_direction for gpio
https://git.kernel.org/hid/hid/c/ca6961d8a851
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* Re: [PATCH] HID: wacom: Set a default resolution for older tablets
From: Benjamin Tissoires @ 2023-04-13 14:37 UTC (permalink / raw)
To: linux-input, Ping Cheng
Cc: jkosina, jason.gerecke, aaron.skomra, joshua.dickens, Ping Cheng
In-Reply-To: <20230409164229.29777-1-ping.cheng@wacom.com>
On Sun, 09 Apr 2023 09:42:29 -0700, Ping Cheng wrote:
> Some older tablets may not report physical maximum for X/Y
> coordinates. Set a default to prevent undefined resolution.
>
>
Applied to hid/hid.git (for-6.4/wacom), thanks!
[1/1] HID: wacom: Set a default resolution for older tablets
https://git.kernel.org/hid/hid/c/08a46b4190d3
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* Re: [PATCH v2 0/3] HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms
From: Benjamin Tissoires @ 2023-04-13 14:29 UTC (permalink / raw)
To: Jiri Kosina, Hans de Goede; +Cc: Douglas Anderson, linux-input
In-Reply-To: <20230413093625.71146-1-hdegoede@redhat.com>
On Thu, 13 Apr 2023 11:36:22 +0200, Hans de Goede wrote:
> Here is a v2 of my series to allow using i2c-hid-of on non OF platforms
> to allow I2C-HID devices which are not enumerated by ACPI to work on ACPI
> platforms (by manual i2c_client instantiation using i2c_client_id matching).
>
> Changes in v2:
> - As discussed Drop the patches to consolidate all the i2c-hid-of*
> drivers into one
> - Add a comment to the "post-reset-deassert-delay-ms" property reading,
> that it is a kernel internal (non public) property used between x86
> platform code and the i2c-hid driver.
>
> [...]
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-6.4/i2c-hid), thanks!
[1/3] HID: i2c-hid-of: Consistenly use dev local variable in probe()
https://git.kernel.org/hid/hid/c/9d793e7c1f88
[2/3] HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms
https://git.kernel.org/hid/hid/c/728ec8b6eda8
[3/3] HID: i2c-hid-of: Add reset GPIO support to i2c-hid-of
https://git.kernel.org/hid/hid/c/2be404486c05
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* Re: [PATCH] HID: kye: Fix rdesc for kye tablets
From: Benjamin Tissoires @ 2023-04-13 14:25 UTC (permalink / raw)
To: linux-input, David Yang; +Cc: Jiri Kosina, linux-kernel
In-Reply-To: <20230411173308.422756-1-mmyangfl@gmail.com>
On Wed, 12 Apr 2023 01:33:08 +0800, David Yang wrote:
> I forget to add them in previous commit 2dd438cdc2e9
> ("HID: kye: Add support for all kye tablets").
>
>
Applied to hid/hid.git (for-6.4/kye), thanks!
[1/1] HID: kye: Fix rdesc for kye tablets
https://git.kernel.org/hid/hid/c/0f6fac2cfbef
Cheers,
--
Benjamin Tissoires <benjamin.tissoires@redhat.com>
^ permalink raw reply
* Re: [PATCH v2 0/3] HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms
From: Benjamin Tissoires @ 2023-04-13 14:07 UTC (permalink / raw)
To: Hans de Goede; +Cc: Jiri Kosina, Douglas Anderson, linux-input
In-Reply-To: <714731d2-de10-8db1-f271-a2a68cb55910@redhat.com>
On Thu, Apr 13, 2023 at 4:03 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 4/13/23 16:01, Benjamin Tissoires wrote:
> > On Apr 13 2023, Hans de Goede wrote:
> >> Hi Benjamin,
> >>
> >> Here is a v2 of my series to allow using i2c-hid-of on non OF platforms
> >> to allow I2C-HID devices which are not enumerated by ACPI to work on ACPI
> >> platforms (by manual i2c_client instantiation using i2c_client_id matching).
> >>
> >> Changes in v2:
> >> - As discussed Drop the patches to consolidate all the i2c-hid-of*
> >> drivers into one
> >> - Add a comment to the "post-reset-deassert-delay-ms" property reading,
> >> that it is a kernel internal (non public) property used between x86
> >> platform code and the i2c-hid driver.
> >
> > The series is:
> > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> Thank you.
>
> > I'd rather have at least Doug test this one before we merge it, but I'll
> > be out next week, so you'll have to rely on Jiri to merge it (or wait
> > for one week).
>
> Since Doug's devices use the separate specialized drivers and this series
> now only touches the generic i2c-hid-of driver I don't think Doug can test
> this, or am I missing something ?
Ooh, sorry for the confusion. I thought we were having:
i2c-hid-of-elan -> i2c-hid-of -> i2c-hid-core
When actually, i2c-hid-of-elan is only talking to i2c-hid-core.
This is why I wanted i2c-hid-of-elan/goodix to override the property
in their probe and then forward the payload to i2c-hid-of...
I guess we can take this series right away then
Cheers,
Benjamin
>
> Regards,
>
> Hans
>
>
>
>
>
> >> Hans de Goede (3):
> >> HID: i2c-hid-of: Consistenly use dev local variable in probe()
> >> HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms
> >> HID: i2c-hid-of: Add reset GPIO support to i2c-hid-of
> >>
> >> drivers/hid/i2c-hid/Kconfig | 6 +++--
> >> drivers/hid/i2c-hid/i2c-hid-of.c | 38 ++++++++++++++++++++++++--------
> >> 2 files changed, 33 insertions(+), 11 deletions(-)
> >>
> >> --
> >> 2.39.1
> >>
> >
>
^ permalink raw reply
* Re: [PATCH v2 0/3] HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms
From: Hans de Goede @ 2023-04-13 14:03 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, Douglas Anderson, linux-input
In-Reply-To: <20230413140148.3cqpylpfwpna5vj5@mail.corp.redhat.com>
Hi,
On 4/13/23 16:01, Benjamin Tissoires wrote:
> On Apr 13 2023, Hans de Goede wrote:
>> Hi Benjamin,
>>
>> Here is a v2 of my series to allow using i2c-hid-of on non OF platforms
>> to allow I2C-HID devices which are not enumerated by ACPI to work on ACPI
>> platforms (by manual i2c_client instantiation using i2c_client_id matching).
>>
>> Changes in v2:
>> - As discussed Drop the patches to consolidate all the i2c-hid-of*
>> drivers into one
>> - Add a comment to the "post-reset-deassert-delay-ms" property reading,
>> that it is a kernel internal (non public) property used between x86
>> platform code and the i2c-hid driver.
>
> The series is:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Thank you.
> I'd rather have at least Doug test this one before we merge it, but I'll
> be out next week, so you'll have to rely on Jiri to merge it (or wait
> for one week).
Since Doug's devices use the separate specialized drivers and this series
now only touches the generic i2c-hid-of driver I don't think Doug can test
this, or am I missing something ?
Regards,
Hans
>> Hans de Goede (3):
>> HID: i2c-hid-of: Consistenly use dev local variable in probe()
>> HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms
>> HID: i2c-hid-of: Add reset GPIO support to i2c-hid-of
>>
>> drivers/hid/i2c-hid/Kconfig | 6 +++--
>> drivers/hid/i2c-hid/i2c-hid-of.c | 38 ++++++++++++++++++++++++--------
>> 2 files changed, 33 insertions(+), 11 deletions(-)
>>
>> --
>> 2.39.1
>>
>
^ permalink raw reply
* Re: [PATCH v2 0/3] HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms
From: Benjamin Tissoires @ 2023-04-13 14:01 UTC (permalink / raw)
To: Hans de Goede; +Cc: Jiri Kosina, Douglas Anderson, linux-input
In-Reply-To: <20230413093625.71146-1-hdegoede@redhat.com>
On Apr 13 2023, Hans de Goede wrote:
> Hi Benjamin,
>
> Here is a v2 of my series to allow using i2c-hid-of on non OF platforms
> to allow I2C-HID devices which are not enumerated by ACPI to work on ACPI
> platforms (by manual i2c_client instantiation using i2c_client_id matching).
>
> Changes in v2:
> - As discussed Drop the patches to consolidate all the i2c-hid-of*
> drivers into one
> - Add a comment to the "post-reset-deassert-delay-ms" property reading,
> that it is a kernel internal (non public) property used between x86
> platform code and the i2c-hid driver.
The series is:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
I'd rather have at least Doug test this one before we merge it, but I'll
be out next week, so you'll have to rely on Jiri to merge it (or wait
for one week).
Cheers,
Benjamin
>
> Regards,
>
> Hans
>
>
> Hans de Goede (3):
> HID: i2c-hid-of: Consistenly use dev local variable in probe()
> HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms
> HID: i2c-hid-of: Add reset GPIO support to i2c-hid-of
>
> drivers/hid/i2c-hid/Kconfig | 6 +++--
> drivers/hid/i2c-hid/i2c-hid-of.c | 38 ++++++++++++++++++++++++--------
> 2 files changed, 33 insertions(+), 11 deletions(-)
>
> --
> 2.39.1
>
^ permalink raw reply
* Re: [PATCH] HID: amd_sfh: Support for additional light sensor
From: Jiri Kosina @ 2023-04-13 13:57 UTC (permalink / raw)
To: Basavaraj Natikar; +Cc: benjamin.tissoires, linux-input
In-Reply-To: <20230411161901.909940-1-Basavaraj.Natikar@amd.com>
On Tue, 11 Apr 2023, Basavaraj Natikar wrote:
> There is support for additional light sensors in the SFH firmware.
> As a result, add support for additional light sensors.
>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Applied.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/7] Fixes to amd_sfh
From: Jiri Kosina @ 2023-04-13 13:56 UTC (permalink / raw)
To: Basavaraj Natikar; +Cc: benjamin.tissoires, linux-input
In-Reply-To: <20230411161030.909350-1-Basavaraj.Natikar@amd.com>
On Tue, 11 Apr 2023, Basavaraj Natikar wrote:
> Changes include correcting structure fields, illuminance values,
> shutdown PM operations, stop all command, increasing sensor
> command timeout and no sensor condition.
>
> Basavaraj Natikar (7):
> HID: amd_sfh: Correct the structure fields
> HID: amd_sfh: Correct the sensor enable and disable command
> HID: amd_sfh: Fix illuminance value
> HID: amd_sfh: Add support for shutdown operation
> HID: amd_sfh: Correct the stop all command
> HID: amd_sfh: Increase sensor command timeout for SFH1.1
> HID: amd_sfh: Handle "no sensors" enabled for SFH1.1
>
> drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 9 +++++++++
> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 2 +-
> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 11 +++++++++++
> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c | 10 +++++++---
> drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h | 8 ++++----
> 5 files changed, 32 insertions(+), 8 deletions(-)
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH v2 2/3] HID: i2c-hid-of: Allow using i2c-hid-of on non OF platforms
From: Hans de Goede @ 2023-04-13 9:36 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Hans de Goede, Douglas Anderson, linux-input
In-Reply-To: <20230413093625.71146-1-hdegoede@redhat.com>
There are some x86 tablets / 2-in-1s which ship with Android as their
factory OS image. These have pretty broken ACPI tables, relying on
everything being hardcoded in the factory kernel image.
platform/x86/x86-android-tablets.c manually instantiates i2c-clients for
i2c devices on these tablets to make them work with the mainline kernel.
The Lenovo Yoga Book 1 (yb1-x90f/l) is such a 2-in-1. It has 2 I2C-HID
devices its main touchscreen and a Wacom digitizer. Its main touchscreen
can alternatively also be used in HiDeep's native protocol mode but
for the Wacom digitizer we really need I2C-HID.
This patch allows using i2c-hid-of on non OF platforms so that it can
bind to a non ACPI instantiated i2c_client on x86 for the Wacom digitizer.
Note the driver already has an "i2c-over-hid" i2c_device_id (rather then
an of_device_id).
Besides enabling building on non-OF platforms this also replaces
the only of_property_read_u32() call with device_property_read_u32() note
that other properties where already read using device_property_read_...().
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/hid/i2c-hid/Kconfig | 6 ++++--
drivers/hid/i2c-hid/i2c-hid-of.c | 4 +++-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/i2c-hid/Kconfig b/drivers/hid/i2c-hid/Kconfig
index 4439be7fa74d..3be17109301a 100644
--- a/drivers/hid/i2c-hid/Kconfig
+++ b/drivers/hid/i2c-hid/Kconfig
@@ -23,12 +23,14 @@ config I2C_HID_ACPI
config I2C_HID_OF
tristate "HID over I2C transport layer Open Firmware driver"
- depends on OF
+ # No "depends on OF" because this can also be used for manually
+ # (board-file) instantiated "hid-over-i2c" type i2c-clients.
select I2C_HID_CORE
help
Say Y here if you use a keyboard, a touchpad, a touchscreen, or any
other HID based devices which is connected to your computer via I2C.
- This driver supports Open Firmware (Device Tree)-based systems.
+ This driver supports Open Firmware (Device Tree)-based systems as
+ well as binding to manually (board-file) instantiated i2c-hid-clients.
If unsure, say N.
diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
index c82a5a54c3e6..385f7460e03c 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of.c
@@ -82,7 +82,7 @@ static int i2c_hid_of_probe(struct i2c_client *client)
ihid_of->ops.power_up = i2c_hid_of_power_up;
ihid_of->ops.power_down = i2c_hid_of_power_down;
- ret = of_property_read_u32(dev->of_node, "hid-descr-addr", &val);
+ ret = device_property_read_u32(dev, "hid-descr-addr", &val);
if (ret) {
dev_err(dev, "HID register address not provided\n");
return -ENODEV;
@@ -113,11 +113,13 @@ static int i2c_hid_of_probe(struct i2c_client *client)
hid_descriptor_address, quirks);
}
+#ifdef CONFIG_OF
static const struct of_device_id i2c_hid_of_match[] = {
{ .compatible = "hid-over-i2c" },
{},
};
MODULE_DEVICE_TABLE(of, i2c_hid_of_match);
+#endif
static const struct i2c_device_id i2c_hid_of_id_table[] = {
{ "hid", 0 },
--
2.39.1
^ permalink raw reply related
* [PATCH v2 3/3] HID: i2c-hid-of: Add reset GPIO support to i2c-hid-of
From: Hans de Goede @ 2023-04-13 9:36 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Hans de Goede, Douglas Anderson, linux-input
In-Reply-To: <20230413093625.71146-1-hdegoede@redhat.com>
Add reset GPIO support to the generic i2c-hid-of driver
This is necessary to make the Wacom digitizer on the Lenovo Yoga Book 1
(yb1-x90f/l) work and this will also allow consolidating the 2 specialized
i2c-hid-of-elan.c and i2c-hid-of-goodix.c drivers into the generic
i2c-hid-of driver.
For now the new "post-reset-deassert-delay-ms" property is only used on
x86/ACPI (non devicetree) devs. IOW it is not used in actual devicetree
files and the same goes for the reset GPIO. The devicetree-bindings
maintainers have requested properties like these to not be added to
the devicetree-bindings, so the new property + GPIO are deliberately
not added to the existing devicetree-bindings.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Chanhes in v2:
- Add a comment to the "post-reset-deassert-delay-ms" property reading,
that it is a kernel internal (non public) property used between x86
platform code and the i2c-hid driver.
---
drivers/hid/i2c-hid/i2c-hid-of.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c
index 385f7460e03c..855f53092f4e 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of.c
@@ -21,6 +21,7 @@
#include <linux/delay.h>
#include <linux/device.h>
+#include <linux/gpio/consumer.h>
#include <linux/hid.h>
#include <linux/i2c.h>
#include <linux/kernel.h>
@@ -35,8 +36,10 @@ struct i2c_hid_of {
struct i2chid_ops ops;
struct i2c_client *client;
+ struct gpio_desc *reset_gpio;
struct regulator_bulk_data supplies[2];
int post_power_delay_ms;
+ int post_reset_delay_ms;
};
static int i2c_hid_of_power_up(struct i2chid_ops *ops)
@@ -55,6 +58,10 @@ static int i2c_hid_of_power_up(struct i2chid_ops *ops)
if (ihid_of->post_power_delay_ms)
msleep(ihid_of->post_power_delay_ms);
+ gpiod_set_value_cansleep(ihid_of->reset_gpio, 0);
+ if (ihid_of->post_reset_delay_ms)
+ msleep(ihid_of->post_reset_delay_ms);
+
return 0;
}
@@ -62,6 +69,7 @@ static void i2c_hid_of_power_down(struct i2chid_ops *ops)
{
struct i2c_hid_of *ihid_of = container_of(ops, struct i2c_hid_of, ops);
+ gpiod_set_value_cansleep(ihid_of->reset_gpio, 1);
regulator_bulk_disable(ARRAY_SIZE(ihid_of->supplies),
ihid_of->supplies);
}
@@ -96,6 +104,19 @@ static int i2c_hid_of_probe(struct i2c_client *client)
if (!device_property_read_u32(dev, "post-power-on-delay-ms", &val))
ihid_of->post_power_delay_ms = val;
+ /*
+ * Note this is a kernel internal device-property set by x86 platform code,
+ * this MUST not be used in devicetree files without first adding it to
+ * the DT bindings.
+ */
+ if (!device_property_read_u32(dev, "post-reset-deassert-delay-ms", &val))
+ ihid_of->post_reset_delay_ms = val;
+
+ /* Start out with reset asserted */
+ ihid_of->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(ihid_of->reset_gpio))
+ return PTR_ERR(ihid_of->reset_gpio);
+
ihid_of->supplies[0].supply = "vdd";
ihid_of->supplies[1].supply = "vddl";
ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ihid_of->supplies),
--
2.39.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox