* Re: [PATCH 7/8] Input: qt2160 - convert to use devm_* api
From: Dmitry Torokhov @ 2023-07-24 5:19 UTC (permalink / raw)
To: Yangtao Li; +Cc: linux-input, linux-kernel
In-Reply-To: <20230714080611.81302-7-frank.li@vivo.com>
Hi Yangtao,
On Fri, Jul 14, 2023 at 04:06:10PM +0800, Yangtao Li wrote:
> Use devm_* api to simplify code, this makes it unnecessary to explicitly
> release resources.
>
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
> drivers/input/keyboard/qt2160.c | 30 +++++++++++-------------------
> 1 file changed, 11 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/input/keyboard/qt2160.c b/drivers/input/keyboard/qt2160.c
> index 599ea85cfd30..218ef92b8c2b 100644
> --- a/drivers/input/keyboard/qt2160.c
> +++ b/drivers/input/keyboard/qt2160.c
> @@ -340,6 +340,7 @@ static bool qt2160_identify(struct i2c_client *client)
>
> static int qt2160_probe(struct i2c_client *client)
> {
> + struct device *dev = &client->dev;
You create a temporary, but half of the code does not use it. Please if
you introduce a temporary make sure everything is using it.
> struct qt2160_data *qt2160;
> struct input_dev *input;
> int i;
> @@ -358,12 +359,11 @@ static int qt2160_probe(struct i2c_client *client)
> return -ENODEV;
>
> /* Chip is valid and active. Allocate structure */
> - qt2160 = kzalloc(sizeof(struct qt2160_data), GFP_KERNEL);
> - input = input_allocate_device();
> + qt2160 = devm_kzalloc(dev, sizeof(struct qt2160_data), GFP_KERNEL);
> + input = devm_input_allocate_device(dev);
> if (!qt2160 || !input) {
This double check was a cheat when resources did not free automatically,
it makes no sense to carry with devm.
> dev_err(&client->dev, "insufficient memory\n");
> - error = -ENOMEM;
> - goto err_free_mem;
> + return -ENOMEM;
> }
>
> qt2160->client = client;
> @@ -389,23 +389,23 @@ static int qt2160_probe(struct i2c_client *client)
> error = qt2160_write(client, QT2160_CMD_CALIBRATE, 1);
> if (error) {
> dev_err(&client->dev, "failed to calibrate device\n");
> - goto err_free_mem;
> + return error;
> }
>
> if (client->irq) {
> - error = request_irq(client->irq, qt2160_irq,
> - IRQF_TRIGGER_FALLING, "qt2160", qt2160);
> + error = devm_request_irq(dev, client->irq, qt2160_irq,
> + IRQF_TRIGGER_FALLING, "qt2160", qt2160);
> if (error) {
> dev_err(&client->dev,
> "failed to allocate irq %d\n", client->irq);
> - goto err_free_mem;
> + return error;
> }
> }
>
> error = qt2160_register_leds(qt2160);
> if (error) {
> dev_err(&client->dev, "Failed to register leds\n");
> - goto err_free_irq;
> + return error;
> }
>
> error = input_register_device(qt2160->input);
> @@ -422,29 +422,21 @@ static int qt2160_probe(struct i2c_client *client)
>
> err_unregister_leds:
> qt2160_unregister_leds(qt2160);
LEDs can also be registered with devm.
> -err_free_irq:
> - if (client->irq)
> - free_irq(client->irq, qt2160);
> -err_free_mem:
> - input_free_device(input);
> - kfree(qt2160);
> return error;
> }
>
> static void qt2160_remove(struct i2c_client *client)
> {
> struct qt2160_data *qt2160 = i2c_get_clientdata(client);
> + struct device *dev = &client->dev;
>
> qt2160_unregister_leds(qt2160);
>
> /* Release IRQ so no queue will be scheduled */
> if (client->irq)
> - free_irq(client->irq, qt2160);
> + devm_free_irq(dev, client->irq, qt2160);
>
> cancel_delayed_work_sync(&qt2160->dwork);
It is not great that we are left with non-empty qt2160_remove(). The
driver should be converted away from using work item, and then entirety
of qt2160_remove() can be deleted.
I posted a series that cleans up the driver and incorporates an updated
version of your patch.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH 4/4] Input: qt2160 - convert to use devm_* api
From: Dmitry Torokhov @ 2023-07-24 5:13 UTC (permalink / raw)
To: linux-input; +Cc: Yangtao Li, linux-kernel
In-Reply-To: <20230724051345.335219-1-dmitry.torokhov@gmail.com>
From: Yangtao Li <frank.li@vivo.com>
Use devm_* api to simplify code, this makes it unnecessary to explicitly
release resources.
Signed-off-by: Yangtao Li <frank.li@vivo.com>
Link: https://lore.kernel.org/r/20230714080611.81302-7-frank.li@vivo.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/qt2160.c | 77 +++++++++------------------------
1 file changed, 20 insertions(+), 57 deletions(-)
diff --git a/drivers/input/keyboard/qt2160.c b/drivers/input/keyboard/qt2160.c
index 6cfaabd10482..7e3b09642ab7 100644
--- a/drivers/input/keyboard/qt2160.c
+++ b/drivers/input/keyboard/qt2160.c
@@ -239,7 +239,7 @@ static int qt2160_write(struct i2c_client *client, u8 reg, u8 data)
static int qt2160_register_leds(struct qt2160_data *qt2160)
{
struct i2c_client *client = qt2160->client;
- int ret;
+ int error;
int i;
for (i = 0; i < QT2160_NUM_LEDS_X; i++) {
@@ -252,9 +252,9 @@ static int qt2160_register_leds(struct qt2160_data *qt2160)
led->id = i;
led->qt2160 = qt2160;
- ret = led_classdev_register(&client->dev, &led->cdev);
- if (ret < 0)
- return ret;
+ error = devm_led_classdev_register(&client->dev, &led->cdev);
+ if (error)
+ return error;
}
/* Tur off LEDs */
@@ -265,14 +265,6 @@ static int qt2160_register_leds(struct qt2160_data *qt2160)
return 0;
}
-static void qt2160_unregister_leds(struct qt2160_data *qt2160)
-{
- int i;
-
- for (i = 0; i < QT2160_NUM_LEDS_X; i++)
- led_classdev_unregister(&qt2160->leds[i].cdev);
-}
-
#else
static inline int qt2160_register_leds(struct qt2160_data *qt2160)
@@ -280,10 +272,6 @@ static inline int qt2160_register_leds(struct qt2160_data *qt2160)
return 0;
}
-static inline void qt2160_unregister_leds(struct qt2160_data *qt2160)
-{
-}
-
#endif
static bool qt2160_identify(struct i2c_client *client)
@@ -334,13 +322,13 @@ static int qt2160_probe(struct i2c_client *client)
return -ENODEV;
/* Chip is valid and active. Allocate structure */
- qt2160 = kzalloc(sizeof(struct qt2160_data), GFP_KERNEL);
- input = input_allocate_device();
- if (!qt2160 || !input) {
- dev_err(&client->dev, "insufficient memory\n");
- error = -ENOMEM;
- goto err_free_mem;
- }
+ qt2160 = devm_kzalloc(&client->dev, sizeof(*qt2160), GFP_KERNEL);
+ if (!qt2160)
+ return -ENOMEM;
+
+ input = devm_input_allocate_device(&client->dev);
+ if (!input)
+ return -ENOMEM;
qt2160->client = client;
qt2160->input = input;
@@ -366,22 +354,24 @@ static int qt2160_probe(struct i2c_client *client)
error = qt2160_write(client, QT2160_CMD_CALIBRATE, 1);
if (error) {
dev_err(&client->dev, "failed to calibrate device\n");
- goto err_free_mem;
+ return error;
}
if (client->irq) {
- error = request_threaded_irq(client->irq, NULL, qt2160_irq,
- IRQF_ONESHOT, "qt2160", input);
+ error = devm_request_threaded_irq(&client->dev, client->irq,
+ NULL, qt2160_irq,
+ IRQF_ONESHOT,
+ "qt2160", input);
if (error) {
dev_err(&client->dev,
"failed to allocate irq %d\n", client->irq);
- goto err_free_mem;
+ return error;
}
} else {
error = input_setup_polling(input, qt2160_get_key_matrix);
if (error) {
dev_err(&client->dev, "Failed to setup polling\n");
- goto err_free_mem;
+ return error;
}
input_set_poll_interval(input, QT2160_CYCLE_INTERVAL);
}
@@ -389,43 +379,17 @@ static int qt2160_probe(struct i2c_client *client)
error = qt2160_register_leds(qt2160);
if (error) {
dev_err(&client->dev, "Failed to register leds\n");
- goto err_free_irq;
+ return error;
}
error = input_register_device(qt2160->input);
if (error) {
dev_err(&client->dev,
"Failed to register input device\n");
- goto err_unregister_leds;
+ return error;
}
- i2c_set_clientdata(client, qt2160);
-
return 0;
-
-err_unregister_leds:
- qt2160_unregister_leds(qt2160);
-err_free_irq:
- if (client->irq)
- free_irq(client->irq, qt2160);
-err_free_mem:
- input_free_device(input);
- kfree(qt2160);
- return error;
-}
-
-static void qt2160_remove(struct i2c_client *client)
-{
- struct qt2160_data *qt2160 = i2c_get_clientdata(client);
-
- qt2160_unregister_leds(qt2160);
-
- /* Release IRQ so no queue will be scheduled */
- if (client->irq)
- free_irq(client->irq, qt2160);
-
- input_unregister_device(qt2160->input);
- kfree(qt2160);
}
static const struct i2c_device_id qt2160_idtable[] = {
@@ -442,7 +406,6 @@ static struct i2c_driver qt2160_driver = {
.id_table = qt2160_idtable,
.probe = qt2160_probe,
- .remove = qt2160_remove,
};
module_i2c_driver(qt2160_driver);
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 3/4] Input: qt2160 - do not hard code interrupt trigger
From: Dmitry Torokhov @ 2023-07-24 5:13 UTC (permalink / raw)
To: linux-input; +Cc: Yangtao Li, linux-kernel
In-Reply-To: <20230724051345.335219-1-dmitry.torokhov@gmail.com>
Rely on the platform and ACPI/DT to set up the interrupt trigger.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/qt2160.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/input/keyboard/qt2160.c b/drivers/input/keyboard/qt2160.c
index b8c0f0ebf604..6cfaabd10482 100644
--- a/drivers/input/keyboard/qt2160.c
+++ b/drivers/input/keyboard/qt2160.c
@@ -371,8 +371,7 @@ static int qt2160_probe(struct i2c_client *client)
if (client->irq) {
error = request_threaded_irq(client->irq, NULL, qt2160_irq,
- IRQF_TRIGGER_LOW | IRQF_ONESHOT,
- "qt2160", input);
+ IRQF_ONESHOT, "qt2160", input);
if (error) {
dev_err(&client->dev,
"failed to allocate irq %d\n", client->irq);
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 2/4] Input: qt2160 - switch to using threaded interrupt handler
From: Dmitry Torokhov @ 2023-07-24 5:13 UTC (permalink / raw)
To: linux-input; +Cc: Yangtao Li, linux-kernel
In-Reply-To: <20230724051345.335219-1-dmitry.torokhov@gmail.com>
Instead of using combination of normal IRQ and work item which required
careful handling on device teardown, use standard threaded interrupt that
allows communication wityh the chip over slow (I2C) bus directly in the
interrupt handler.
To support polling mode switch to standard polling support implemented by
the input core.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/qt2160.c | 53 ++++++++++++---------------------
1 file changed, 19 insertions(+), 34 deletions(-)
diff --git a/drivers/input/keyboard/qt2160.c b/drivers/input/keyboard/qt2160.c
index b0b9d7a2691e..b8c0f0ebf604 100644
--- a/drivers/input/keyboard/qt2160.c
+++ b/drivers/input/keyboard/qt2160.c
@@ -32,7 +32,7 @@
#define QT2160_NUM_LEDS_X 8
-#define QT2160_CYCLE_INTERVAL (2*HZ)
+#define QT2160_CYCLE_INTERVAL 2000 /* msec - 2 sec */
static unsigned char qt2160_key2code[] = {
KEY_0, KEY_1, KEY_2, KEY_3,
@@ -54,7 +54,6 @@ struct qt2160_led {
struct qt2160_data {
struct i2c_client *client;
struct input_dev *input;
- struct delayed_work dwork;
unsigned short keycodes[ARRAY_SIZE(qt2160_key2code)];
u16 key_matrix;
#ifdef CONFIG_LEDS_CLASS
@@ -155,10 +154,10 @@ static int qt2160_read_block(struct i2c_client *client,
return 0;
}
-static int qt2160_get_key_matrix(struct qt2160_data *qt2160)
+static void qt2160_get_key_matrix(struct input_dev *input)
{
+ struct qt2160_data *qt2160 = input_get_drvdata(input);
struct i2c_client *client = qt2160->client;
- struct input_dev *input = qt2160->input;
u8 regs[6];
u16 old_matrix, new_matrix;
int ret, i, mask;
@@ -173,7 +172,7 @@ static int qt2160_get_key_matrix(struct qt2160_data *qt2160)
if (ret) {
dev_err(&client->dev,
"could not perform chip read.\n");
- return ret;
+ return;
}
old_matrix = qt2160->key_matrix;
@@ -191,37 +190,17 @@ static int qt2160_get_key_matrix(struct qt2160_data *qt2160)
}
input_sync(input);
-
- return 0;
}
-static irqreturn_t qt2160_irq(int irq, void *_qt2160)
+static irqreturn_t qt2160_irq(int irq, void *data)
{
- struct qt2160_data *qt2160 = _qt2160;
+ struct input_dev *input = data;
- mod_delayed_work(system_wq, &qt2160->dwork, 0);
+ qt2160_get_key_matrix(input);
return IRQ_HANDLED;
}
-static void qt2160_schedule_read(struct qt2160_data *qt2160)
-{
- schedule_delayed_work(&qt2160->dwork, QT2160_CYCLE_INTERVAL);
-}
-
-static void qt2160_worker(struct work_struct *work)
-{
- struct qt2160_data *qt2160 =
- container_of(work, struct qt2160_data, dwork.work);
-
- dev_dbg(&qt2160->client->dev, "worker\n");
-
- qt2160_get_key_matrix(qt2160);
-
- /* Avoid device lock up by checking every so often */
- qt2160_schedule_read(qt2160);
-}
-
static int qt2160_read(struct i2c_client *client, u8 reg)
{
int ret;
@@ -365,7 +344,6 @@ static int qt2160_probe(struct i2c_client *client)
qt2160->client = client;
qt2160->input = input;
- INIT_DELAYED_WORK(&qt2160->dwork, qt2160_worker);
input->name = "AT42QT2160 Touch Sense Keyboard";
input->id.bustype = BUS_I2C;
@@ -382,6 +360,8 @@ static int qt2160_probe(struct i2c_client *client)
}
__clear_bit(KEY_RESERVED, input->keybit);
+ input_set_drvdata(input, qt2160);
+
/* Calibrate device */
error = qt2160_write(client, QT2160_CMD_CALIBRATE, 1);
if (error) {
@@ -390,13 +370,21 @@ static int qt2160_probe(struct i2c_client *client)
}
if (client->irq) {
- error = request_irq(client->irq, qt2160_irq,
- IRQF_TRIGGER_FALLING, "qt2160", qt2160);
+ error = request_threaded_irq(client->irq, NULL, qt2160_irq,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ "qt2160", input);
if (error) {
dev_err(&client->dev,
"failed to allocate irq %d\n", client->irq);
goto err_free_mem;
}
+ } else {
+ error = input_setup_polling(input, qt2160_get_key_matrix);
+ if (error) {
+ dev_err(&client->dev, "Failed to setup polling\n");
+ goto err_free_mem;
+ }
+ input_set_poll_interval(input, QT2160_CYCLE_INTERVAL);
}
error = qt2160_register_leds(qt2160);
@@ -413,7 +401,6 @@ static int qt2160_probe(struct i2c_client *client)
}
i2c_set_clientdata(client, qt2160);
- qt2160_schedule_read(qt2160);
return 0;
@@ -438,8 +425,6 @@ static void qt2160_remove(struct i2c_client *client)
if (client->irq)
free_irq(client->irq, qt2160);
- cancel_delayed_work_sync(&qt2160->dwork);
-
input_unregister_device(qt2160->input);
kfree(qt2160);
}
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH 1/4] Input: qt2160 - tweak check for i2c adapter functionality
From: Dmitry Torokhov @ 2023-07-24 5:13 UTC (permalink / raw)
To: linux-input; +Cc: Yangtao Li, linux-kernel
i2c_check_functionality() returns essentially a boolean and not an error
code, so treat it as such.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/qt2160.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/input/keyboard/qt2160.c b/drivers/input/keyboard/qt2160.c
index 599ea85cfd30..b0b9d7a2691e 100644
--- a/drivers/input/keyboard/qt2160.c
+++ b/drivers/input/keyboard/qt2160.c
@@ -345,12 +345,9 @@ static int qt2160_probe(struct i2c_client *client)
int i;
int error;
- /* Check functionality */
- error = i2c_check_functionality(client->adapter,
- I2C_FUNC_SMBUS_BYTE);
- if (!error) {
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE)) {
dev_err(&client->dev, "%s adapter not supported\n",
- dev_driver_string(&client->adapter->dev));
+ dev_driver_string(&client->adapter->dev));
return -ENODEV;
}
--
2.41.0.487.g6d72f3e995-goog
^ permalink raw reply related
* [PATCH AUTOSEL 5.15 14/23] HID: add quirk for 03f0:464a HP Elite Presenter Mouse
From: Sasha Levin @ 2023-07-24 1:23 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Marco Morandini, Jiri Kosina, Sasha Levin, jikos, linux-input
In-Reply-To: <20230724012334.2317140-1-sashal@kernel.org>
From: Marco Morandini <marco.morandini@polimi.it>
[ Upstream commit 0db117359e47750d8bd310d19f13e1c4ef7fc26a ]
HP Elite Presenter Mouse HID Record Descriptor shows
two mouses (Repord ID 0x1 and 0x2), one keypad (Report ID 0x5),
two Consumer Controls (Report IDs 0x6 and 0x3).
Previous to this commit it registers one mouse, one keypad
and one Consumer Control, and it was usable only as a
digitl laser pointer (one of the two mouses). This patch defines
the 464a USB device ID and enables the HID_QUIRK_MULTI_INPUT
quirk for it, allowing to use the device both as a mouse
and a digital laser pointer.
Signed-off-by: Marco Morandini <marco.morandini@polimi.it>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5daec769df7ae..5fceefb3c707e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -593,6 +593,7 @@
#define USB_DEVICE_ID_UGCI_FIGHTING 0x0030
#define USB_VENDOR_ID_HP 0x03f0
+#define USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A 0x464a
#define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A 0x0a4a
#define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0B4A 0x0b4a
#define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE 0x134a
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index c7c06aa958c4d..96ca7d981ee20 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -96,6 +96,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_KEYBOARD_A096), HID_QUIRK_NO_INIT_REPORTS },
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_KEYBOARD_A293), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A), HID_QUIRK_ALWAYS_POLL },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0B4A), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_094A), HID_QUIRK_ALWAYS_POLL },
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 5.15 12/23] HID: logitech-hidpp: Add USB and Bluetooth IDs for the Logitech G915 TKL Keyboard
From: Sasha Levin @ 2023-07-24 1:23 UTC (permalink / raw)
To: linux-kernel, stable
Cc: stuarthayhurst, Bastien Nocera, Jiri Kosina, Sasha Levin, jikos,
linux-input
In-Reply-To: <20230724012334.2317140-1-sashal@kernel.org>
From: stuarthayhurst <stuart.a.hayhurst@gmail.com>
[ Upstream commit 48aea8b445c422a372cf15915101035a47105421 ]
Adds the USB and Bluetooth IDs for the Logitech G915 TKL keyboard, for device detection
For this device, this provides battery reporting on top of hid-generic
Reviewed-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-logitech-hidpp.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index baa68ae9b9efc..e5e6874684d2e 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4377,6 +4377,8 @@ static const struct hid_device_id hidpp_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC086) },
{ /* Logitech G903 Hero Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC091) },
+ { /* Logitech G915 TKL Keyboard over USB */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC343) },
{ /* Logitech G920 Wheel over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL),
.driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
@@ -4392,6 +4394,8 @@ static const struct hid_device_id hidpp_devices[] = {
{ /* MX5500 keyboard over Bluetooth */
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb30b),
.driver_data = HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS },
+ { /* Logitech G915 TKL keyboard over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb35f) },
{ /* M-RCQ142 V470 Cordless Laser Mouse over Bluetooth */
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb008) },
{ /* MX Master mouse over Bluetooth */
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.1 35/41] HID: intel-ish-hid: ipc: Add Arrow Lake PCI device ID
From: Sasha Levin @ 2023-07-24 1:21 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Even Xu, Srinivas Pandruvada, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20230724012118.2316073-1-sashal@kernel.org>
From: Even Xu <even.xu@intel.com>
[ Upstream commit 4982126e3029cd59fbd1da0d9cc0365a0585fe64 ]
Add device ID of Arrow Lake-H into ishtp support list.
Signed-off-by: Even Xu <even.xu@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/intel-ish-hid/ipc/hw-ish.h | 1 +
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/intel-ish-hid/ipc/hw-ish.h b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
index fc108f19a64c3..e99f3a3c65e15 100644
--- a/drivers/hid/intel-ish-hid/ipc/hw-ish.h
+++ b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
@@ -33,6 +33,7 @@
#define ADL_N_DEVICE_ID 0x54FC
#define RPL_S_DEVICE_ID 0x7A78
#define MTL_P_DEVICE_ID 0x7E45
+#define ARL_H_DEVICE_ID 0x7745
#define REVISION_ID_CHT_A0 0x6
#define REVISION_ID_CHT_Ax_SI 0x0
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 7120b30ac51d0..55cb25038e632 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -44,6 +44,7 @@ static const struct pci_device_id ish_pci_tbl[] = {
{PCI_DEVICE(PCI_VENDOR_ID_INTEL, ADL_N_DEVICE_ID)},
{PCI_DEVICE(PCI_VENDOR_ID_INTEL, RPL_S_DEVICE_ID)},
{PCI_DEVICE(PCI_VENDOR_ID_INTEL, MTL_P_DEVICE_ID)},
+ {PCI_DEVICE(PCI_VENDOR_ID_INTEL, ARL_H_DEVICE_ID)},
{0, }
};
MODULE_DEVICE_TABLE(pci, ish_pci_tbl);
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.1 29/41] HID: add quirk for 03f0:464a HP Elite Presenter Mouse
From: Sasha Levin @ 2023-07-24 1:21 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Marco Morandini, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20230724012118.2316073-1-sashal@kernel.org>
From: Marco Morandini <marco.morandini@polimi.it>
[ Upstream commit 0db117359e47750d8bd310d19f13e1c4ef7fc26a ]
HP Elite Presenter Mouse HID Record Descriptor shows
two mouses (Repord ID 0x1 and 0x2), one keypad (Report ID 0x5),
two Consumer Controls (Report IDs 0x6 and 0x3).
Previous to this commit it registers one mouse, one keypad
and one Consumer Control, and it was usable only as a
digitl laser pointer (one of the two mouses). This patch defines
the 464a USB device ID and enables the HID_QUIRK_MULTI_INPUT
quirk for it, allowing to use the device both as a mouse
and a digital laser pointer.
Signed-off-by: Marco Morandini <marco.morandini@polimi.it>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 653db6cdab579..9a17e5cc3539b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -614,6 +614,7 @@
#define USB_DEVICE_ID_UGCI_FIGHTING 0x0030
#define USB_VENDOR_ID_HP 0x03f0
+#define USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A 0x464a
#define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A 0x0a4a
#define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0B4A 0x0b4a
#define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE 0x134a
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 66e64350f1386..f8f20a7c24b17 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -96,6 +96,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_KEYBOARD_A096), HID_QUIRK_NO_INIT_REPORTS },
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_KEYBOARD_A293), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A), HID_QUIRK_ALWAYS_POLL },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0B4A), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_094A), HID_QUIRK_ALWAYS_POLL },
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.1 24/41] HID: logitech-hidpp: Add USB and Bluetooth IDs for the Logitech G915 TKL Keyboard
From: Sasha Levin @ 2023-07-24 1:20 UTC (permalink / raw)
To: linux-kernel, stable
Cc: stuarthayhurst, Bastien Nocera, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20230724012118.2316073-1-sashal@kernel.org>
From: stuarthayhurst <stuart.a.hayhurst@gmail.com>
[ Upstream commit 48aea8b445c422a372cf15915101035a47105421 ]
Adds the USB and Bluetooth IDs for the Logitech G915 TKL keyboard, for device detection
For this device, this provides battery reporting on top of hid-generic
Reviewed-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-logitech-hidpp.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index b2cd7527de195..28761272afe52 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4403,6 +4403,8 @@ static const struct hid_device_id hidpp_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC086) },
{ /* Logitech G903 Hero Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC091) },
+ { /* Logitech G915 TKL Keyboard over USB */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC343) },
{ /* Logitech G920 Wheel over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL),
.driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
@@ -4418,6 +4420,8 @@ static const struct hid_device_id hidpp_devices[] = {
{ /* MX5500 keyboard over Bluetooth */
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb30b),
.driver_data = HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS },
+ { /* Logitech G915 TKL keyboard over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb35f) },
{ /* M-RCQ142 V470 Cordless Laser Mouse over Bluetooth */
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb008) },
{ /* MX Master mouse over Bluetooth */
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.4 52/58] HID: intel-ish-hid: ipc: Add Arrow Lake PCI device ID
From: Sasha Levin @ 2023-07-24 1:13 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Even Xu, Srinivas Pandruvada, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20230724011338.2298062-1-sashal@kernel.org>
From: Even Xu <even.xu@intel.com>
[ Upstream commit 4982126e3029cd59fbd1da0d9cc0365a0585fe64 ]
Add device ID of Arrow Lake-H into ishtp support list.
Signed-off-by: Even Xu <even.xu@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/intel-ish-hid/ipc/hw-ish.h | 1 +
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/intel-ish-hid/ipc/hw-ish.h b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
index fc108f19a64c3..e99f3a3c65e15 100644
--- a/drivers/hid/intel-ish-hid/ipc/hw-ish.h
+++ b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
@@ -33,6 +33,7 @@
#define ADL_N_DEVICE_ID 0x54FC
#define RPL_S_DEVICE_ID 0x7A78
#define MTL_P_DEVICE_ID 0x7E45
+#define ARL_H_DEVICE_ID 0x7745
#define REVISION_ID_CHT_A0 0x6
#define REVISION_ID_CHT_Ax_SI 0x0
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 7120b30ac51d0..55cb25038e632 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -44,6 +44,7 @@ static const struct pci_device_id ish_pci_tbl[] = {
{PCI_DEVICE(PCI_VENDOR_ID_INTEL, ADL_N_DEVICE_ID)},
{PCI_DEVICE(PCI_VENDOR_ID_INTEL, RPL_S_DEVICE_ID)},
{PCI_DEVICE(PCI_VENDOR_ID_INTEL, MTL_P_DEVICE_ID)},
+ {PCI_DEVICE(PCI_VENDOR_ID_INTEL, ARL_H_DEVICE_ID)},
{0, }
};
MODULE_DEVICE_TABLE(pci, ish_pci_tbl);
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.4 34/58] dt-bindings: input: goodix: Add "goodix,no-reset-during-suspend" property
From: Sasha Levin @ 2023-07-24 1:13 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Fei Shao, Jeff LaBundy, Douglas Anderson, Matthias Brugger,
Rob Herring, Jiri Kosina, Sasha Levin, dmitry.torokhov, robh+dt,
krzysztof.kozlowski+dt, conor+dt, linux-input, devicetree
In-Reply-To: <20230724011338.2298062-1-sashal@kernel.org>
From: Fei Shao <fshao@chromium.org>
[ Upstream commit 359ed24a0dd3802e703ec8071dc3b6ed446de5f0 ]
We observed that on Chromebook device Steelix, if Goodix GT7375P
touchscreen is powered in suspend (because, for example, it connects to
an always-on regulator) and with the reset GPIO asserted, it will
introduce about 14mW power leakage.
To address that, we add this property to skip reset during suspend.
If it's set, the driver will stop asserting the reset GPIO during
power-down. Refer to the comments in the driver for details.
Signed-off-by: Fei Shao <fshao@chromium.org>
Suggested-by: Jeff LaBundy <jeff@labundy.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
.../devicetree/bindings/input/goodix,gt7375p.yaml | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
index ce18d7dadae23..1edad1da1196d 100644
--- a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
+++ b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
@@ -43,6 +43,15 @@ properties:
itself as long as it allows the main board to make signals compatible
with what the touchscreen is expecting for its IO rails.
+ goodix,no-reset-during-suspend:
+ description:
+ Set this to true to enforce the driver to not assert the reset GPIO
+ during suspend.
+ Due to potential touchscreen hardware flaw, back-powering could happen in
+ suspend if the power supply is on and with active-low reset GPIO asserted.
+ This property is used to avoid the back-powering issue.
+ type: boolean
+
required:
- compatible
- reg
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.4 35/58] HID: i2c-hid: goodix: Add support for "goodix,no-reset-during-suspend" property
From: Sasha Levin @ 2023-07-24 1:13 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Fei Shao, Douglas Anderson, Jeff LaBundy, Jiri Kosina,
Sasha Levin, jikos, benjamin.tissoires, dmitry.torokhov, mka,
steve, u.kleine-koenig, linux-input
In-Reply-To: <20230724011338.2298062-1-sashal@kernel.org>
From: Fei Shao <fshao@chromium.org>
[ Upstream commit 7607f12ba735f04e0ef8718dabadf3a765c9a477 ]
In the beginning, commit 18eeef46d359 ("HID: i2c-hid: goodix: Tie the
reset line to true state of the regulator") introduced a change to tie
the reset line of the Goodix touchscreen to the state of the regulator
to fix a power leakage issue in suspend.
After some time, the change was deemed unnecessary and was reverted in
commit 557e05fa9fdd ("HID: i2c-hid: goodix: Stop tying the reset line to
the regulator") due to difficulties in managing regulator notifiers for
designs like Evoker, which provides a second power rail to touchscreen.
However, the revert caused a power regression on another Chromebook
device Steelix in the field, which has a dedicated always-on regulator
for touchscreen and was covered by the workaround in the first commit.
To address both cases, this patch adds the support for the new
"goodix,no-reset-during-suspend" property in the driver:
- When set to true, the driver does not assert the reset GPIO during
power-down.
Instead, the GPIO will be asserted during power-up to ensure the
touchscreen always has a clean start and consistent behavior after
resuming.
This is for designs with a dedicated always-on regulator.
- When set to false or unset, the driver uses the original control flow
and asserts GPIO and disables regulators normally.
This is for the two-regulator and shared-regulator designs.
Signed-off-by: Fei Shao <fshao@chromium.org>
Suggested-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
index 0060e3dcd775d..db4639db98407 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
@@ -28,6 +28,7 @@ struct i2c_hid_of_goodix {
struct regulator *vdd;
struct regulator *vddio;
struct gpio_desc *reset_gpio;
+ bool no_reset_during_suspend;
const struct goodix_i2c_hid_timing_data *timings;
};
@@ -37,6 +38,14 @@ static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
container_of(ops, struct i2c_hid_of_goodix, ops);
int ret;
+ /*
+ * We assert reset GPIO here (instead of during power-down) to ensure
+ * the device will have a clean state after powering up, just like the
+ * normal scenarios will have.
+ */
+ if (ihid_goodix->no_reset_during_suspend)
+ gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1);
+
ret = regulator_enable(ihid_goodix->vdd);
if (ret)
return ret;
@@ -60,7 +69,9 @@ static void goodix_i2c_hid_power_down(struct i2chid_ops *ops)
struct i2c_hid_of_goodix *ihid_goodix =
container_of(ops, struct i2c_hid_of_goodix, ops);
- gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1);
+ if (!ihid_goodix->no_reset_during_suspend)
+ gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1);
+
regulator_disable(ihid_goodix->vddio);
regulator_disable(ihid_goodix->vdd);
}
@@ -91,6 +102,9 @@ static int i2c_hid_of_goodix_probe(struct i2c_client *client)
if (IS_ERR(ihid_goodix->vddio))
return PTR_ERR(ihid_goodix->vddio);
+ ihid_goodix->no_reset_during_suspend =
+ of_property_read_bool(client->dev.of_node, "goodix,no-reset-during-suspend");
+
ihid_goodix->timings = device_get_match_data(&client->dev);
return i2c_hid_core_probe(client, &ihid_goodix->ops, 0x0001, 0);
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.4 43/58] HID: add quirk for 03f0:464a HP Elite Presenter Mouse
From: Sasha Levin @ 2023-07-24 1:13 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Marco Morandini, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20230724011338.2298062-1-sashal@kernel.org>
From: Marco Morandini <marco.morandini@polimi.it>
[ Upstream commit 0db117359e47750d8bd310d19f13e1c4ef7fc26a ]
HP Elite Presenter Mouse HID Record Descriptor shows
two mouses (Repord ID 0x1 and 0x2), one keypad (Report ID 0x5),
two Consumer Controls (Report IDs 0x6 and 0x3).
Previous to this commit it registers one mouse, one keypad
and one Consumer Control, and it was usable only as a
digitl laser pointer (one of the two mouses). This patch defines
the 464a USB device ID and enables the HID_QUIRK_MULTI_INPUT
quirk for it, allowing to use the device both as a mouse
and a digital laser pointer.
Signed-off-by: Marco Morandini <marco.morandini@polimi.it>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5d29abac2300e..55a436a6dde98 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -620,6 +620,7 @@
#define USB_DEVICE_ID_UGCI_FIGHTING 0x0030
#define USB_VENDOR_ID_HP 0x03f0
+#define USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A 0x464a
#define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A 0x0a4a
#define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0B4A 0x0b4a
#define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE 0x134a
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 804fc03600cc9..3983b4f282f8f 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -96,6 +96,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_KEYBOARD_A096), HID_QUIRK_NO_INIT_REPORTS },
{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT, USB_DEVICE_ID_HOLTEK_ALT_KEYBOARD_A293), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A), HID_QUIRK_ALWAYS_POLL },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0B4A), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_094A), HID_QUIRK_ALWAYS_POLL },
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.4 36/58] HID: logitech-hidpp: Add USB and Bluetooth IDs for the Logitech G915 TKL Keyboard
From: Sasha Levin @ 2023-07-24 1:13 UTC (permalink / raw)
To: linux-kernel, stable
Cc: stuarthayhurst, Bastien Nocera, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20230724011338.2298062-1-sashal@kernel.org>
From: stuarthayhurst <stuart.a.hayhurst@gmail.com>
[ Upstream commit 48aea8b445c422a372cf15915101035a47105421 ]
Adds the USB and Bluetooth IDs for the Logitech G915 TKL keyboard, for device detection
For this device, this provides battery reporting on top of hid-generic
Reviewed-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-logitech-hidpp.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 5e1a412fd28fa..2ff007244d5e1 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4608,6 +4608,8 @@ static const struct hid_device_id hidpp_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC086) },
{ /* Logitech G903 Hero Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC091) },
+ { /* Logitech G915 TKL Keyboard over USB */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC343) },
{ /* Logitech G920 Wheel over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL),
.driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
@@ -4630,6 +4632,8 @@ static const struct hid_device_id hidpp_devices[] = {
{ /* MX5500 keyboard over Bluetooth */
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb30b),
.driver_data = HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS },
+ { /* Logitech G915 TKL keyboard over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb35f) },
{ /* M-RCQ142 V470 Cordless Laser Mouse over Bluetooth */
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb008) },
{ /* MX Master mouse over Bluetooth */
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
From: Dmitry Torokhov @ 2023-07-23 20:06 UTC (permalink / raw)
To: Biju Das
Cc: Mark Brown, Mike Looijmans, Andreas Helbech Kleist,
Geert Uytterhoeven, Uwe Kleine-König,
linux-input@vger.kernel.org, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org, Wolfram Sang, Andy Shevchenko,
linux-kernel@vger.kernel.org
In-Reply-To: <OS0PR01MB5922328753110C5C35101357863DA@OS0PR01MB5922.jpnprd01.prod.outlook.com>
On Sun, Jul 23, 2023 at 06:50:29AM +0000, Biju Das wrote:
>
> Can you please post a patch based on this?
It looks like you are already taking care of this so I'll let you
finish.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend
From: Silvan Jegen @ 2023-07-23 18:58 UTC (permalink / raw)
To: Martino Fontana; +Cc: linux-input
In-Reply-To: <CAKst+mAnPmLLCCdJWQdz2d=dUZmuUHNxT_+rs+R_Z7981hS7xA@mail.gmail.com>
Heyho
Martino Fontana <tinozzo123@gmail.com> wrote:
> It is my first patch on the Linux kernel, so I just did kinda what I
> would do on GitHub (amend and force push).
> What should I do here in case of trivial adjustments?
I would leave it as is and only respin this one if either there are more
comments on the old version or if the maintainer tells you to.
BTW, just as a heads-up: posting an email reply at the top of the quoted
email (aka "top-posting") is frowned upon in the Linux mailing lists. See
https://people.kernel.org/tglx/
for some pointers regarding netiquette.
Cheers,
Silvan
^ permalink raw reply
* Re: [PATCH] Input: i8042 - Add quirk for polling the KBD port
From: Mario Limonciello @ 2023-07-23 15:45 UTC (permalink / raw)
To: Friedrich Vock, Dmitry Torokhov; +Cc: linux-input
In-Reply-To: <a8e101fd-d221-7f56-77fd-d4656bc909f0@gmx.de>
On 7/22/23 10:35, Friedrich Vock wrote:
> Hi Dmitry,
>
> On 22.07.23 01:44, Dmitry Torokhov wrote:
>> Hi Friedrich,
>>
>> On Tue, May 30, 2023 at 05:36:44PM +0200, Friedrich Vock wrote:
>>> It seems like there are some devices in the ASUS TUF A16 laptops that
>>> just don't send any keyboard interrupts until you read from the KBD
>>> port.
>> I am sorry, but continuously polling keyboard port will absolutely wreck
>> battery life on these devices, so this can not be a real solution.
>>
>> I wonder if this is yet another example of incorrect IRQ 1 polarity
>> override on devices with AMD chipsets (CC-ing Mario).
> I'm pretty sure that's the case. I only found Mario's patch reverting
> these overrides sometime after sending out this one, but that patch
> indeed fixes this problem as well. It's contained in 6.5-rc2, so this
> patch is not necessary anymore. Sorry for not sending a "please
> disregard" earlier.
>
> Thanks,
> Friedrich
These are the Phoenix based laptops right? There was also a second patch
series that went out that fixed and interrupt storm caused by a
misconfigured
GPIO. Everything is in 6.5-rc2 and most of it is coming back to stable
kernels.
>>
>>> Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>
>>> ---
>>> drivers/input/serio/i8042-acpipnpio.h | 30 +++++++++++++++--
>>> drivers/input/serio/i8042.c | 47
>>> ++++++++++++++++++++++-----
>>> drivers/input/serio/i8042.h | 2 +-
>>> 3 files changed, 67 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/input/serio/i8042-acpipnpio.h
>>> b/drivers/input/serio/i8042-acpipnpio.h
>>> index 028e45bd050b..be2e72aaa658 100644
>>> --- a/drivers/input/serio/i8042-acpipnpio.h
>>> +++ b/drivers/input/serio/i8042-acpipnpio.h
>>> @@ -83,6 +83,7 @@ static inline void i8042_write_command(int val)
>>> #define SERIO_QUIRK_KBDRESET BIT(12)
>>> #define SERIO_QUIRK_DRITEK BIT(13)
>>> #define SERIO_QUIRK_NOPNP BIT(14)
>>> +#define SERIO_QUIRK_POLL_KBD BIT(15)
>>>
>>> /* Quirk table for different mainboards. Options similar or
>>> identical to i8042
>>> * module parameters.
>>> @@ -99,6 +100,26 @@ static const struct dmi_system_id
>>> i8042_dmi_quirk_table[] __initconst = {
>>> },
>>> .driver_data = (void *)(SERIO_QUIRK_NOMUX)
>>> },
>>> + /* Some laptops seem to not trigger any keyboard interrupts at
>>> all,
>>> + * even when there is data available. On these devices, manually
>>> + * polling the keyboard port is required.
>>> + */
>>> + {
>>> + /* ASUS TUF Gaming A16 with Ryzen 7 7735HS */
>>> + .matches = {
>>> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
>>> + DMI_MATCH(DMI_PRODUCT_NAME, "FA617NS"),
>>> + },
>>> + .driver_data = (void *)(SERIO_QUIRK_POLL_KBD)
>>> + },
>>> + {
>>> + /* ASUS TUF Gaming A16 with Ryzen 9 7940HS */
>>> + .matches = {
>>> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
>>> + DMI_MATCH(DMI_PRODUCT_NAME, "FA617XS"),
>>> + },
>>> + .driver_data = (void *)(SERIO_QUIRK_POLL_KBD)
>>> + },
>>> {
>>> .matches = {
>>> DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
>>> @@ -1634,6 +1655,8 @@ static void __init i8042_check_quirks(void)
>>> if (quirks & SERIO_QUIRK_NOPNP)
>>> i8042_nopnp = true;
>>> #endif
>>> + if (quirks & SERIO_QUIRK_POLL_KBD)
>>> + i8042_poll_kbd = true;
>>> }
>>> #else
>>> static inline void i8042_check_quirks(void) {}
>>> @@ -1667,7 +1690,7 @@ static int __init i8042_platform_init(void)
>>>
>>> i8042_check_quirks();
>>>
>>> - pr_debug("Active quirks (empty means
>>> none):%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
>>> + pr_debug("Active quirks (empty means
>>> none):%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
>>> i8042_nokbd ? " nokbd" : "",
>>> i8042_noaux ? " noaux" : "",
>>> i8042_nomux ? " nomux" : "",
>>> @@ -1687,10 +1710,11 @@ static int __init i8042_platform_init(void)
>>> "",
>>> #endif
>>> #ifdef CONFIG_PNP
>>> - i8042_nopnp ? " nopnp" : "");
>>> + i8042_nopnp ? " nopnp" : "",
>>> #else
>>> - "");
>>> + "",
>>> #endif
>>> + i8042_poll_kbd ? "poll_kbd" : "");
>>>
>>> retval = i8042_pnp_init();
>>> if (retval)
>>> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
>>> index 6dac7c1853a5..7212263d3a41 100644
>>> --- a/drivers/input/serio/i8042.c
>>> +++ b/drivers/input/serio/i8042.c
>>> @@ -115,6 +115,10 @@ module_param_named(nopnp, i8042_nopnp, bool, 0);
>>> MODULE_PARM_DESC(nopnp, "Do not use PNP to detect controller
>>> settings");
>>> #endif
>>>
>>> +static bool i8042_poll_kbd;
>>> +module_param_named(poll_kbd, i8042_poll_kbd, bool, 0);
>>> +MODULE_PARM_DESC(poll_kbd, "Continuously poll the KBD port instead
>>> of relying on interrupts");
>>> +
>>> #define DEBUG
>>> #ifdef DEBUG
>>> static bool i8042_debug;
>>> @@ -178,6 +182,24 @@ static irqreturn_t i8042_interrupt(int irq,
>>> void *dev_id);
>>> static bool (*i8042_platform_filter)(unsigned char data, unsigned
>>> char str,
>>> struct serio *serio);
>>>
>>> +#define POLL_TIME 1
>>> +static void i8042_poll_func(struct timer_list *timer)
>>> +{
>>> + unsigned char status;
>>> + unsigned long flags;
>>> +
>>> + do {
>>> + spin_lock_irqsave(&i8042_lock, flags);
>>> + status = i8042_read_status();
>>> + spin_unlock_irqrestore(&i8042_lock, flags);
>>> + if (status & I8042_STR_OBF)
>>> + i8042_interrupt(0, NULL);
>>> + } while (status & I8042_STR_OBF);
>>> + mod_timer(timer, jiffies + msecs_to_jiffies(POLL_TIME));
>>> +}
>>> +
>>> +DEFINE_TIMER(poll_timer, i8042_poll_func);
>>> +
>>> void i8042_lock_chip(void)
>>> {
>>> mutex_lock(&i8042_mutex);
>>> @@ -1437,13 +1459,15 @@ static void i8042_unregister_ports(void)
>>> }
>>> }
>>>
>>> +
>>> static void i8042_free_irqs(void)
>>> {
>>> if (i8042_aux_irq_registered)
>>> free_irq(I8042_AUX_IRQ, i8042_platform_device);
>>> - if (i8042_kbd_irq_registered)
>>> + if (i8042_poll_kbd)
>>> + del_timer(&poll_timer);
>>> + else if (i8042_kbd_irq_registered)
>>> free_irq(I8042_KBD_IRQ, i8042_platform_device);
>>> -
>>> i8042_aux_irq_registered = i8042_kbd_irq_registered = false;
>>> }
>>>
>>> @@ -1497,10 +1521,14 @@ static int i8042_setup_kbd(void)
>>> if (error)
>>> return error;
>>>
>>> - error = request_irq(I8042_KBD_IRQ, i8042_interrupt, IRQF_SHARED,
>>> - "i8042", i8042_platform_device);
>>> - if (error)
>>> - goto err_free_port;
>>> + if (i8042_poll_kbd)
>>> + mod_timer(&poll_timer, msecs_to_jiffies(POLL_TIME));
>>> + else {
>>> + error = request_irq(I8042_KBD_IRQ, i8042_interrupt,
>>> IRQF_SHARED,
>>> + "i8042", i8042_platform_device);
>>> + if (error)
>>> + goto err_free_port;
>>> + }
>>>
>>> error = i8042_enable_kbd_port();
>>> if (error)
>>> @@ -1510,8 +1538,11 @@ static int i8042_setup_kbd(void)
>>> return 0;
>>>
>>> err_free_irq:
>>> - free_irq(I8042_KBD_IRQ, i8042_platform_device);
>>> - err_free_port:
>>> + if (i8042_poll_kbd)
>>> + del_timer(&poll_timer);
>>> + else
>>> + free_irq(I8042_KBD_IRQ, i8042_platform_device);
>>> +err_free_port:
>>> i8042_free_kbd_port();
>>> return error;
>>> }
>>> --
>>> 2.40.1
>>>
>> Thanks.
>>
^ permalink raw reply
* 6.5-rc2: keys autorepeating when they should not
From: Pavel Machek @ 2023-07-23 12:24 UTC (permalink / raw)
To: kernel list, dmitry.torokhov, linux-input
[-- Attachment #1: Type: text/plain, Size: 312 bytes --]
Hi!
I updated to 6.5-rc2, and now I see keyboard problems (in X). ~ once
in two days I get keyboard "stuck", autorepeating. It is USB keyboard,
hitting another key cancels the autorepeat.
Any ideas?
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* RE: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
From: Biju Das @ 2023-07-23 6:50 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mark Brown, Mike Looijmans, Andreas Helbech Kleist,
Geert Uytterhoeven, Uwe Kleine-König,
linux-input@vger.kernel.org, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org, Wolfram Sang, Andy Shevchenko,
linux-kernel@vger.kernel.org
In-Reply-To: <OS0PR01MB59222E6CE0EEED3A5FE843C1863DA@OS0PR01MB5922.jpnprd01.prod.outlook.com>
> Subject: RE: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
>
> Hi Dmitry,
>
> Thanks for the feedback.
>
> > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> >
> > On Sat, Jul 22, 2023 at 05:51:17PM +0000, Biju Das wrote:
> > > Hi Dmitry Torokhov,
> > >
> > > Thanks for the feedback.
> > >
> > > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> > > >
> > > > On Wed, Jul 19, 2023 at 06:43:47AM +0000, Biju Das wrote:
> > > > > Hi Dmitry Torokhov,
> > > > >
> > > > > Thanks for the feedback.
> > > > >
> > > > > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> > > > > >
> > > > > > On Mon, Jul 17, 2023 at 06:45:27PM +0000, Biju Das wrote:
> > > > > > > Hi Dmitry,
> > > > > > >
> > > > > > > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify
> > > > > > > > probe()
> > > > > > > >
> > > > > > > > On Mon, Jul 17, 2023 at 07:15:50PM +0100, Mark Brown
> wrote:
> > > > > > > > > On Mon, Jul 17, 2023 at 04:35:02PM +0000, Biju Das
> wrote:
> > > > > > > > >
> > > > > > > > > > The .device_get_match_data callbacks are missing for
> > > > > > > > > > I2C and SPI bus
> > > > > > > > subsystems.
> > > > > > > > > > Can you please throw some lights on this?
> > > > > > > > >
> > > > > > > > > It's the first time I've ever heard of that callback, I
> > > > > > > > > don't know why whoever added it wouldn't have done those
> > > > > > > > > buses in particular or if it just didn't happen. Try
> > > > > > > > > adding it and if it works send
> > > > > > the patches?
> > > > > > > >
> > > > > > > > I think there is a disconnect. Right now
> > > > > > > > device_get_match_data callbacks are part of
> > > > > > > > fwnode_operations. I was proposing to add another optional
> > > > > > > > device_get_match_data callback to 'struct
> > > > bus_type'
> > > > > > > > to allow individual buses control how match data is
> > > > > > > > handled, before (or after) jumping into the fwnode-backed
> > > > > > > > device_get_match_data
> > > > > > callbacks.
> > > > > > >
> > > > > > > That is what implemented here [1] and [2] right?
> > > > > > >
> > > > > > >
> > > > > > > First it check for fwnode-backed device_get_match_data
> > > > > > > callbacks and Fallback is bus-type based match.
> > > > > > >
> > > > > > > Looks like you are proposing to unify [1] and [2] and you
> > > > > > > want the logic to be other way around. ie, first bus-type
> > > > > > > match, then fwnode-backed callbacks?
> > > > > > >
> > > > > >
> > > > > > I do not have a strong preference for the ordering, i.e. I
> > > > > > think it is perfectly fine to do the generic fwnode-based
> > > > > > lookup and if there is no match have bus method called as a
> > > > > > fallback,
> > > > >
> > > > > That involves a bit of work.
> > > > >
> > > > > const void *device_get_match_data(const struct device *dev);
> > > > >
> > > > > const struct i2c_device_id *i2c_match_id(const struct
> > > > > i2c_device_id
> > > > *id,
> > > > > const struct i2c_client
> > > > > *client);
> > > > >
> > > > > const struct spi_device_id *spi_get_device_id(const struct
> > > > > spi_device *sdev);
> > > > >
> > > > > Basically, the bus-client driver(such as exc3000) needs to pass
> > > > > struct device and device_get_match_data after generic
> > > > > fwnode-based lookup, needs to find the bus type based on struct
> > > > > device and call a new generic
> > > > > void* bus_get_match_data(void*) callback, so that each bus
> > > > > interface can do a match.
> > > >
> > > > Yes, something like this (which does not seem that involved to
> > me...):
> > >
> > > Looks it will work.
> > >
> > > But there is some 2 additional checks in core code, every driver
> > > which
> > is not bus type need to go through this checks.
> > >
> > > Also in Bus specific callback, there are 2 additional checks.
> > >
> > > So, performance wise [1] is better.
> >
> > I do not believe this is a concern whatsoever: majority of
> > architectures/boards have been converted to ACPI/DT, which are being
> > matched first as they are now, so the fallback to bus-specific
> > matching against bus-specific device ID tables will be very
> infrequent.
> > Additionally, device_get_match_data() is predominantly called from
> > driver probe paths, so we need not be concerned with it being used
> > with class devices or other kinds of devices not associated with a
> bus.
>
> Looks like most of the i2c client driver uses similar handling for
> ACPI/DT and ID tables. If that is the case, it is good to have this
> proposed change which will simplify most of the drivers listed in [1]
>
> [1]
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir
> .bootlin.com%2Flinux%2Flatest%2FA%2Fident%2Fi2c_match_id&data=05%7C01%7C
> biju.das.jz%40bp.renesas.com%7C2a07c353ab7649fdf29a08db8b42cca3%7C53d825
> 71da1947e49cb4625a166a4a2a%7C0%7C0%7C638256891245437404%7CUnknown%7CTWFp
> bGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%
> 3D%7C3000%7C%7C%7C&sdata=tOxuTgGKc%2FQYFx94rYUJ8TDTWmGKkETzASV3qUjP2vk%3
> D&reserved=0
>
> Eg: drivers/hwmon/pmbus/ibm-cffps.c
>
> enum versions vs = cffps_unknown;
> const void *md = of_device_get_match_data(&client->dev);
> const struct i2c_device_id *id;
>
> if (md) {
> vs = (enum versions)md;
> } else {
> id = i2c_match_id(ibm_cffps_id, client);
> if (id)
> vs = (enum versions)id->driver_data;
> }
>
> The above code can be converted to
> vs = (enum versions)device_get_match_data(&client->dev);
>
> >
> > >
> > > Moreover, we need to avoid code duplication with [1]
> > >
> > > [1]
> >
> > If and when my proposed solution gets into the kernel we can drop
> > i2c_get_match_data() altogether.
>
> Agreed. Will wait for other people's view on this topic.
Also remove spi_get_device_match_data and
Make i2c_match_id() and spi_get_device_id() as static and
Replace all these with device_get_natch_data() from all i2c/spi client drivers.
Can you please post a patch based on this?
Cheers,
Biju
^ permalink raw reply
* RE: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
From: Biju Das @ 2023-07-23 6:05 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mark Brown, Mike Looijmans, Andreas Helbech Kleist,
Geert Uytterhoeven, Uwe Kleine-König,
linux-input@vger.kernel.org, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org, Wolfram Sang, Andy Shevchenko,
linux-kernel@vger.kernel.org
In-Reply-To: <CAKdAkRT9tMnLnDLgWAevE_4HQ0wYMPehvsYaAeYrXdGGiyjXRA@mail.gmail.com>
Hi Dmitry,
Thanks for the feedback.
> Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
>
> On Sat, Jul 22, 2023 at 05:51:17PM +0000, Biju Das wrote:
> > Hi Dmitry Torokhov,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> > >
> > > On Wed, Jul 19, 2023 at 06:43:47AM +0000, Biju Das wrote:
> > > > Hi Dmitry Torokhov,
> > > >
> > > > Thanks for the feedback.
> > > >
> > > > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> > > > >
> > > > > On Mon, Jul 17, 2023 at 06:45:27PM +0000, Biju Das wrote:
> > > > > > Hi Dmitry,
> > > > > >
> > > > > > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify
> > > > > > > probe()
> > > > > > >
> > > > > > > On Mon, Jul 17, 2023 at 07:15:50PM +0100, Mark Brown wrote:
> > > > > > > > On Mon, Jul 17, 2023 at 04:35:02PM +0000, Biju Das wrote:
> > > > > > > >
> > > > > > > > > The .device_get_match_data callbacks are missing for I2C
> > > > > > > > > and SPI bus
> > > > > > > subsystems.
> > > > > > > > > Can you please throw some lights on this?
> > > > > > > >
> > > > > > > > It's the first time I've ever heard of that callback, I
> > > > > > > > don't know why whoever added it wouldn't have done those
> > > > > > > > buses in particular or if it just didn't happen. Try
> > > > > > > > adding it and if it works send
> > > > > the patches?
> > > > > > >
> > > > > > > I think there is a disconnect. Right now
> > > > > > > device_get_match_data callbacks are part of
> > > > > > > fwnode_operations. I was proposing to add another optional
> > > > > > > device_get_match_data callback to 'struct
> > > bus_type'
> > > > > > > to allow individual buses control how match data is handled,
> > > > > > > before (or after) jumping into the fwnode-backed
> > > > > > > device_get_match_data
> > > > > callbacks.
> > > > > >
> > > > > > That is what implemented here [1] and [2] right?
> > > > > >
> > > > > >
> > > > > > First it check for fwnode-backed device_get_match_data
> > > > > > callbacks and Fallback is bus-type based match.
> > > > > >
> > > > > > Looks like you are proposing to unify [1] and [2] and you want
> > > > > > the logic to be other way around. ie, first bus-type match,
> > > > > > then fwnode-backed callbacks?
> > > > > >
> > > > >
> > > > > I do not have a strong preference for the ordering, i.e. I think
> > > > > it is perfectly fine to do the generic fwnode-based lookup and
> > > > > if there is no match have bus method called as a fallback,
> > > >
> > > > That involves a bit of work.
> > > >
> > > > const void *device_get_match_data(const struct device *dev);
> > > >
> > > > const struct i2c_device_id *i2c_match_id(const struct
> > > > i2c_device_id
> > > *id,
> > > > const struct i2c_client
> > > > *client);
> > > >
> > > > const struct spi_device_id *spi_get_device_id(const struct
> > > > spi_device *sdev);
> > > >
> > > > Basically, the bus-client driver(such as exc3000) needs to pass
> > > > struct device and device_get_match_data after generic fwnode-based
> > > > lookup, needs to find the bus type based on struct device and call
> > > > a new generic
> > > > void* bus_get_match_data(void*) callback, so that each bus
> > > > interface can do a match.
> > >
> > > Yes, something like this (which does not seem that involved to
> me...):
> >
> > Looks it will work.
> >
> > But there is some 2 additional checks in core code, every driver which
> is not bus type need to go through this checks.
> >
> > Also in Bus specific callback, there are 2 additional checks.
> >
> > So, performance wise [1] is better.
>
> I do not believe this is a concern whatsoever: majority of
> architectures/boards have been converted to ACPI/DT, which are being
> matched first as they are now, so the fallback to bus-specific matching
> against bus-specific device ID tables will be very infrequent.
> Additionally, device_get_match_data() is predominantly called from
> driver probe paths, so we need not be concerned with it being used with
> class devices or other kinds of devices not associated with a bus.
Looks like most of the i2c client driver uses similar handling for
ACPI/DT and ID tables. If that is the case, it is good to have this
proposed change which will simplify most of the drivers listed in [1]
[1] https://elixir.bootlin.com/linux/latest/A/ident/i2c_match_id
Eg: drivers/hwmon/pmbus/ibm-cffps.c
enum versions vs = cffps_unknown;
const void *md = of_device_get_match_data(&client->dev);
const struct i2c_device_id *id;
if (md) {
vs = (enum versions)md;
} else {
id = i2c_match_id(ibm_cffps_id, client);
if (id)
vs = (enum versions)id->driver_data;
}
The above code can be converted to
vs = (enum versions)device_get_match_data(&client->dev);
>
> >
> > Moreover, we need to avoid code duplication with [1]
> >
> > [1]
>
> If and when my proposed solution gets into the kernel we can drop
> i2c_get_match_data() altogether.
Agreed. Will wait for other people's view on this topic.
Cheers,
Biju
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
From: Dmitry Torokhov @ 2023-07-23 1:17 UTC (permalink / raw)
To: Biju Das
Cc: Mark Brown, Mike Looijmans, Andreas Helbech Kleist,
Geert Uytterhoeven, Uwe Kleine-König,
linux-input@vger.kernel.org, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org, Wolfram Sang, Andy Shevchenko,
linux-kernel@vger.kernel.org
In-Reply-To: <TYCPR01MB5933D4252360AAE57D90FE1C863CA@TYCPR01MB5933.jpnprd01.prod.outlook.com>
On Sat, Jul 22, 2023 at 05:51:17PM +0000, Biju Das wrote:
> Hi Dmitry Torokhov,
>
> Thanks for the feedback.
>
> > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> >
> > On Wed, Jul 19, 2023 at 06:43:47AM +0000, Biju Das wrote:
> > > Hi Dmitry Torokhov,
> > >
> > > Thanks for the feedback.
> > >
> > > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> > > >
> > > > On Mon, Jul 17, 2023 at 06:45:27PM +0000, Biju Das wrote:
> > > > > Hi Dmitry,
> > > > >
> > > > > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> > > > > >
> > > > > > On Mon, Jul 17, 2023 at 07:15:50PM +0100, Mark Brown wrote:
> > > > > > > On Mon, Jul 17, 2023 at 04:35:02PM +0000, Biju Das wrote:
> > > > > > >
> > > > > > > > The .device_get_match_data callbacks are missing for I2C and
> > > > > > > > SPI bus
> > > > > > subsystems.
> > > > > > > > Can you please throw some lights on this?
> > > > > > >
> > > > > > > It's the first time I've ever heard of that callback, I don't
> > > > > > > know why whoever added it wouldn't have done those buses in
> > > > > > > particular or if it just didn't happen. Try adding it and if
> > > > > > > it works send
> > > > the patches?
> > > > > >
> > > > > > I think there is a disconnect. Right now device_get_match_data
> > > > > > callbacks are part of fwnode_operations. I was proposing to add
> > > > > > another optional device_get_match_data callback to 'struct
> > bus_type'
> > > > > > to allow individual buses control how match data is handled,
> > > > > > before (or after) jumping into the fwnode-backed
> > > > > > device_get_match_data
> > > > callbacks.
> > > > >
> > > > > That is what implemented here [1] and [2] right?
> > > > >
> > > > >
> > > > > First it check for fwnode-backed device_get_match_data callbacks
> > > > > and Fallback is bus-type based match.
> > > > >
> > > > > Looks like you are proposing to unify [1] and [2] and you want the
> > > > > logic to be other way around. ie, first bus-type match, then
> > > > > fwnode-backed callbacks?
> > > > >
> > > >
> > > > I do not have a strong preference for the ordering, i.e. I think it
> > > > is perfectly fine to do the generic fwnode-based lookup and if there
> > > > is no match have bus method called as a fallback,
> > >
> > > That involves a bit of work.
> > >
> > > const void *device_get_match_data(const struct device *dev);
> > >
> > > const struct i2c_device_id *i2c_match_id(const struct i2c_device_id
> > *id,
> > > const struct i2c_client *client);
> > >
> > > const struct spi_device_id *spi_get_device_id(const struct spi_device
> > > *sdev);
> > >
> > > Basically, the bus-client driver(such as exc3000) needs to pass struct
> > > device and device_get_match_data after generic fwnode-based lookup,
> > > needs to find the bus type based on struct device and call a new
> > > generic
> > > void* bus_get_match_data(void*) callback, so that each bus interface
> > > can do a match.
> >
> > Yes, something like this (which does not seem that involved to me...):
>
> Looks it will work.
>
> But there is some 2 additional checks in core code, every driver which is not bus type need to go through this checks.
>
> Also in Bus specific callback, there are 2 additional checks.
>
> So, performance wise [1] is better.
I do not believe this is a concern whatsoever: majority of
architectures/boards have been converted to ACPI/DT, which are being
matched first as they are now, so the fallback to bus-specific matching
against bus-specific device ID tables will be very infrequent.
Additionally, device_get_match_data() is predominantly called from
driver probe paths, so we need not be concerned with it being used with
class devices or other kinds of devices not associated with a bus.
>
> Moreover, we need to avoid code duplication with [1]
>
> [1] https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/i2c/i2c-core-base.c#L125
If and when my proposed solution gets into the kernel we can drop
i2c_get_match_data() altogether.
Thanks.
--
Dmitry
^ permalink raw reply
* RE: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
From: Biju Das @ 2023-07-22 17:51 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mark Brown, Mike Looijmans, Andreas Helbech Kleist,
Geert Uytterhoeven, Uwe Kleine-König,
linux-input@vger.kernel.org, Prabhakar Mahadev Lad,
linux-renesas-soc@vger.kernel.org, Wolfram Sang, Andy Shevchenko,
linux-kernel@vger.kernel.org
In-Reply-To: <ZLsCOj1t4JvG3SEp@google.com>
Hi Dmitry Torokhov,
Thanks for the feedback.
> Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
>
> On Wed, Jul 19, 2023 at 06:43:47AM +0000, Biju Das wrote:
> > Hi Dmitry Torokhov,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> > >
> > > On Mon, Jul 17, 2023 at 06:45:27PM +0000, Biju Das wrote:
> > > > Hi Dmitry,
> > > >
> > > > > Subject: Re: [PATCH v2 1/2] Input: exc3000 - Simplify probe()
> > > > >
> > > > > On Mon, Jul 17, 2023 at 07:15:50PM +0100, Mark Brown wrote:
> > > > > > On Mon, Jul 17, 2023 at 04:35:02PM +0000, Biju Das wrote:
> > > > > >
> > > > > > > The .device_get_match_data callbacks are missing for I2C and
> > > > > > > SPI bus
> > > > > subsystems.
> > > > > > > Can you please throw some lights on this?
> > > > > >
> > > > > > It's the first time I've ever heard of that callback, I don't
> > > > > > know why whoever added it wouldn't have done those buses in
> > > > > > particular or if it just didn't happen. Try adding it and if
> > > > > > it works send
> > > the patches?
> > > > >
> > > > > I think there is a disconnect. Right now device_get_match_data
> > > > > callbacks are part of fwnode_operations. I was proposing to add
> > > > > another optional device_get_match_data callback to 'struct
> bus_type'
> > > > > to allow individual buses control how match data is handled,
> > > > > before (or after) jumping into the fwnode-backed
> > > > > device_get_match_data
> > > callbacks.
> > > >
> > > > That is what implemented here [1] and [2] right?
> > > >
> > > >
> > > > First it check for fwnode-backed device_get_match_data callbacks
> > > > and Fallback is bus-type based match.
> > > >
> > > > Looks like you are proposing to unify [1] and [2] and you want the
> > > > logic to be other way around. ie, first bus-type match, then
> > > > fwnode-backed callbacks?
> > > >
> > >
> > > I do not have a strong preference for the ordering, i.e. I think it
> > > is perfectly fine to do the generic fwnode-based lookup and if there
> > > is no match have bus method called as a fallback,
> >
> > That involves a bit of work.
> >
> > const void *device_get_match_data(const struct device *dev);
> >
> > const struct i2c_device_id *i2c_match_id(const struct i2c_device_id
> *id,
> > const struct i2c_client *client);
> >
> > const struct spi_device_id *spi_get_device_id(const struct spi_device
> > *sdev);
> >
> > Basically, the bus-client driver(such as exc3000) needs to pass struct
> > device and device_get_match_data after generic fwnode-based lookup,
> > needs to find the bus type based on struct device and call a new
> > generic
> > void* bus_get_match_data(void*) callback, so that each bus interface
> > can do a match.
>
> Yes, something like this (which does not seem that involved to me...):
Looks it will work.
But there is some 2 additional checks in core code, every driver which is not bus type need to go through this checks.
Also in Bus specific callback, there are 2 additional checks.
So, performance wise [1] is better.
Moreover, we need to avoid code duplication with [1]
[1] https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/i2c/i2c-core-base.c#L125
What core people thinking about Dmitry's proposal?
Cheers,
Biju
>
> diff --git a/drivers/base/property.c b/drivers/base/property.c index
> 8c40abed7852..cc0bf7bb6f3a 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -1277,7 +1277,13 @@ EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
>
> const void *device_get_match_data(const struct device *dev) {
> - return fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data,
> dev);
> + const void *data;
> +
> + data = fwnode_call_ptr_op(dev_fwnode(dev), device_get_match_data,
> dev);
> + if (!data && dev->bus && dev->bus->get_match_data)
> + data = dev->bus->get_match_data(dev);
> +
> + return data;
> }
> EXPORT_SYMBOL_GPL(device_get_match_data);
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 60746652fd52..5fe47bc491a6 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -114,6 +114,26 @@ const struct i2c_device_id *i2c_match_id(const
> struct i2c_device_id *id, } EXPORT_SYMBOL_GPL(i2c_match_id);
>
> +static const void *i2c_device_get_match_data(const struct device *dev)
> +{
> + const struct i2c_client *client = to_i2c_client(dev);
> + const struct i2c_driver *driver;
> + const struct i2c_device_id *match;
> +
> + if (!dev->driver)
> + return NULL;
> +
> + driver = to_i2c_driver(dev->driver);
> + if (!driver)
> + return NULL;
> +
> + match = i2c_match_id(driver->id_table, client);
> + if (!match)
> + return NULL;
> +
> + return (const void *)match->driver_data; }
> +
> const void *i2c_get_match_data(const struct i2c_client *client) {
> struct i2c_driver *driver = to_i2c_driver(client->dev.driver);
> @@ -695,6 +715,7 @@ struct bus_type i2c_bus_type = {
> .probe = i2c_device_probe,
> .remove = i2c_device_remove,
> .shutdown = i2c_device_shutdown,
> + .get_match_data = i2c_device_get_match_data,
> };
> EXPORT_SYMBOL_GPL(i2c_bus_type);
>
> diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
> index ae10c4322754..3f2cba28a1af 100644
> --- a/include/linux/device/bus.h
> +++ b/include/linux/device/bus.h
> @@ -102,6 +102,8 @@ struct bus_type {
> int (*dma_configure)(struct device *dev);
> void (*dma_cleanup)(struct device *dev);
>
> + const void *(*get_match_data)(const struct device *dev);
> +
> const struct dev_pm_ops *pm;
>
> const struct iommu_ops *iommu_ops;
>
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH] Input: i8042 - Add quirk for polling the KBD port
From: Friedrich Vock @ 2023-07-22 15:35 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Mario Limonciello
In-Reply-To: <ZLsYUlSMIK0Gtr21@google.com>
Hi Dmitry,
On 22.07.23 01:44, Dmitry Torokhov wrote:
> Hi Friedrich,
>
> On Tue, May 30, 2023 at 05:36:44PM +0200, Friedrich Vock wrote:
>> It seems like there are some devices in the ASUS TUF A16 laptops that
>> just don't send any keyboard interrupts until you read from the KBD port.
> I am sorry, but continuously polling keyboard port will absolutely wreck
> battery life on these devices, so this can not be a real solution.
>
> I wonder if this is yet another example of incorrect IRQ 1 polarity
> override on devices with AMD chipsets (CC-ing Mario).
I'm pretty sure that's the case. I only found Mario's patch reverting
these overrides sometime after sending out this one, but that patch
indeed fixes this problem as well. It's contained in 6.5-rc2, so this
patch is not necessary anymore. Sorry for not sending a "please
disregard" earlier.
Thanks,
Friedrich
>
>> Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>
>> ---
>> drivers/input/serio/i8042-acpipnpio.h | 30 +++++++++++++++--
>> drivers/input/serio/i8042.c | 47 ++++++++++++++++++++++-----
>> drivers/input/serio/i8042.h | 2 +-
>> 3 files changed, 67 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
>> index 028e45bd050b..be2e72aaa658 100644
>> --- a/drivers/input/serio/i8042-acpipnpio.h
>> +++ b/drivers/input/serio/i8042-acpipnpio.h
>> @@ -83,6 +83,7 @@ static inline void i8042_write_command(int val)
>> #define SERIO_QUIRK_KBDRESET BIT(12)
>> #define SERIO_QUIRK_DRITEK BIT(13)
>> #define SERIO_QUIRK_NOPNP BIT(14)
>> +#define SERIO_QUIRK_POLL_KBD BIT(15)
>>
>> /* Quirk table for different mainboards. Options similar or identical to i8042
>> * module parameters.
>> @@ -99,6 +100,26 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
>> },
>> .driver_data = (void *)(SERIO_QUIRK_NOMUX)
>> },
>> + /* Some laptops seem to not trigger any keyboard interrupts at all,
>> + * even when there is data available. On these devices, manually
>> + * polling the keyboard port is required.
>> + */
>> + {
>> + /* ASUS TUF Gaming A16 with Ryzen 7 7735HS */
>> + .matches = {
>> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "FA617NS"),
>> + },
>> + .driver_data = (void *)(SERIO_QUIRK_POLL_KBD)
>> + },
>> + {
>> + /* ASUS TUF Gaming A16 with Ryzen 9 7940HS */
>> + .matches = {
>> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "FA617XS"),
>> + },
>> + .driver_data = (void *)(SERIO_QUIRK_POLL_KBD)
>> + },
>> {
>> .matches = {
>> DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
>> @@ -1634,6 +1655,8 @@ static void __init i8042_check_quirks(void)
>> if (quirks & SERIO_QUIRK_NOPNP)
>> i8042_nopnp = true;
>> #endif
>> + if (quirks & SERIO_QUIRK_POLL_KBD)
>> + i8042_poll_kbd = true;
>> }
>> #else
>> static inline void i8042_check_quirks(void) {}
>> @@ -1667,7 +1690,7 @@ static int __init i8042_platform_init(void)
>>
>> i8042_check_quirks();
>>
>> - pr_debug("Active quirks (empty means none):%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
>> + pr_debug("Active quirks (empty means none):%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n",
>> i8042_nokbd ? " nokbd" : "",
>> i8042_noaux ? " noaux" : "",
>> i8042_nomux ? " nomux" : "",
>> @@ -1687,10 +1710,11 @@ static int __init i8042_platform_init(void)
>> "",
>> #endif
>> #ifdef CONFIG_PNP
>> - i8042_nopnp ? " nopnp" : "");
>> + i8042_nopnp ? " nopnp" : "",
>> #else
>> - "");
>> + "",
>> #endif
>> + i8042_poll_kbd ? "poll_kbd" : "");
>>
>> retval = i8042_pnp_init();
>> if (retval)
>> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
>> index 6dac7c1853a5..7212263d3a41 100644
>> --- a/drivers/input/serio/i8042.c
>> +++ b/drivers/input/serio/i8042.c
>> @@ -115,6 +115,10 @@ module_param_named(nopnp, i8042_nopnp, bool, 0);
>> MODULE_PARM_DESC(nopnp, "Do not use PNP to detect controller settings");
>> #endif
>>
>> +static bool i8042_poll_kbd;
>> +module_param_named(poll_kbd, i8042_poll_kbd, bool, 0);
>> +MODULE_PARM_DESC(poll_kbd, "Continuously poll the KBD port instead of relying on interrupts");
>> +
>> #define DEBUG
>> #ifdef DEBUG
>> static bool i8042_debug;
>> @@ -178,6 +182,24 @@ static irqreturn_t i8042_interrupt(int irq, void *dev_id);
>> static bool (*i8042_platform_filter)(unsigned char data, unsigned char str,
>> struct serio *serio);
>>
>> +#define POLL_TIME 1
>> +static void i8042_poll_func(struct timer_list *timer)
>> +{
>> + unsigned char status;
>> + unsigned long flags;
>> +
>> + do {
>> + spin_lock_irqsave(&i8042_lock, flags);
>> + status = i8042_read_status();
>> + spin_unlock_irqrestore(&i8042_lock, flags);
>> + if (status & I8042_STR_OBF)
>> + i8042_interrupt(0, NULL);
>> + } while (status & I8042_STR_OBF);
>> + mod_timer(timer, jiffies + msecs_to_jiffies(POLL_TIME));
>> +}
>> +
>> +DEFINE_TIMER(poll_timer, i8042_poll_func);
>> +
>> void i8042_lock_chip(void)
>> {
>> mutex_lock(&i8042_mutex);
>> @@ -1437,13 +1459,15 @@ static void i8042_unregister_ports(void)
>> }
>> }
>>
>> +
>> static void i8042_free_irqs(void)
>> {
>> if (i8042_aux_irq_registered)
>> free_irq(I8042_AUX_IRQ, i8042_platform_device);
>> - if (i8042_kbd_irq_registered)
>> + if (i8042_poll_kbd)
>> + del_timer(&poll_timer);
>> + else if (i8042_kbd_irq_registered)
>> free_irq(I8042_KBD_IRQ, i8042_platform_device);
>> -
>> i8042_aux_irq_registered = i8042_kbd_irq_registered = false;
>> }
>>
>> @@ -1497,10 +1521,14 @@ static int i8042_setup_kbd(void)
>> if (error)
>> return error;
>>
>> - error = request_irq(I8042_KBD_IRQ, i8042_interrupt, IRQF_SHARED,
>> - "i8042", i8042_platform_device);
>> - if (error)
>> - goto err_free_port;
>> + if (i8042_poll_kbd)
>> + mod_timer(&poll_timer, msecs_to_jiffies(POLL_TIME));
>> + else {
>> + error = request_irq(I8042_KBD_IRQ, i8042_interrupt, IRQF_SHARED,
>> + "i8042", i8042_platform_device);
>> + if (error)
>> + goto err_free_port;
>> + }
>>
>> error = i8042_enable_kbd_port();
>> if (error)
>> @@ -1510,8 +1538,11 @@ static int i8042_setup_kbd(void)
>> return 0;
>>
>> err_free_irq:
>> - free_irq(I8042_KBD_IRQ, i8042_platform_device);
>> - err_free_port:
>> + if (i8042_poll_kbd)
>> + del_timer(&poll_timer);
>> + else
>> + free_irq(I8042_KBD_IRQ, i8042_platform_device);
>> +err_free_port:
>> i8042_free_kbd_port();
>> return error;
>> }
>> --
>> 2.40.1
>>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH v2] HID: nintendo: reinitialize USB Pro Controller after resuming from suspend
From: Martino Fontana @ 2023-07-22 10:29 UTC (permalink / raw)
To: Silvan Jegen; +Cc: linux-input
In-Reply-To: <38NABT0Q0GDC0.32EJOTLAGT0T2@homearch.localdomain>
It is my first patch on the Linux kernel, so I just did kinda what I
would do on GitHub (amend and force push).
What should I do here in case of trivial adjustments?
On Fri, Jul 21, 2023 at 9:02 PM Silvan Jegen <s.jegen@gmail.com> wrote:
>
> Martino Fontana <tinozzo123@gmail.com> wrote:
> > When suspending the computer, a Switch Pro Controller connected via USB will
> > lose its internal status. However, because the USB connection was technically
> > never lost, when resuming the computer, the driver will attempt to communicate
> > with the controller as if nothing happened (and fail).
> > Because of this, the user was forced to manually disconnect the controller
> > (or to press the sync button on the controller to power it off), so that it
> > can be re-initialized.
> >
> > With this patch, the controller will be automatically re-initialized after
> > resuming from suspend.
> >
> > Fixes https://bugzilla.kernel.org/show_bug.cgi?id=216233
> >
> > Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
>
> It would be good to add a small section about what changed between v1
> and v2 of the patch. Here is an example for how this can be done.
>
> https://lore.kernel.org/lkml/cover.b24362332ec6099bc8db4e8e06a67545c653291d.1689842332.git-series.apopple@nvidia.com/T/#m73dd8d44f40742e67cbb0d4f030a90b6264a88d3
>
> It's probably not worth resending this patch just for this though. Just
> something to keep in mind if there will be another patch version needed.
>
> Cheers,
> Silvan
>
> > ---
> > drivers/hid/hid-nintendo.c | 178 ++++++++++++++++++++++---------------
> > 1 file changed, 106 insertions(+), 72 deletions(-)
> >
> > diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> > index 250f5d2f8..a5ebe857a 100644
> > --- a/drivers/hid/hid-nintendo.c
> > +++ b/drivers/hid/hid-nintendo.c
> > @@ -2088,7 +2088,9 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
> > struct joycon_input_report *report;
> >
> > req.subcmd_id = JC_SUBCMD_REQ_DEV_INFO;
> > + mutex_lock(&ctlr->output_mutex);
> > ret = joycon_send_subcmd(ctlr, &req, 0, HZ);
> > + mutex_unlock(&ctlr->output_mutex);
> > if (ret) {
> > hid_err(ctlr->hdev, "Failed to get joycon info; ret=%d\n", ret);
> > return ret;
> > @@ -2117,6 +2119,88 @@ static int joycon_read_info(struct joycon_ctlr *ctlr)
> > return 0;
> > }
> >
> > +static int joycon_init(struct hid_device *hdev)
> > +{
> > + struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
> > + int ret = 0;
> > +
> > + mutex_lock(&ctlr->output_mutex);
> > + /* if handshake command fails, assume ble pro controller */
> > + if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
> > + !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> > + hid_dbg(hdev, "detected USB controller\n");
> > + /* set baudrate for improved latency */
> > + ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
> > + if (ret) {
> > + hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
> > + goto err_mutex;
> > + }
> > + /* handshake */
> > + ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
> > + if (ret) {
> > + hid_err(hdev, "Failed handshake; ret=%d\n", ret);
> > + goto err_mutex;
> > + }
> > + /*
> > + * Set no timeout (to keep controller in USB mode).
> > + * This doesn't send a response, so ignore the timeout.
> > + */
> > + joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
> > + } else if (jc_type_is_chrggrip(ctlr)) {
> > + hid_err(hdev, "Failed charging grip handshake\n");
> > + ret = -ETIMEDOUT;
> > + goto err_mutex;
> > + }
> > +
> > + /* get controller calibration data, and parse it */
> > + ret = joycon_request_calibration(ctlr);
> > + if (ret) {
> > + /*
> > + * We can function with default calibration, but it may be
> > + * inaccurate. Provide a warning, and continue on.
> > + */
> > + hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> > + }
> > +
> > + /* get IMU calibration data, and parse it */
> > + ret = joycon_request_imu_calibration(ctlr);
> > + if (ret) {
> > + /*
> > + * We can function with default calibration, but it may be
> > + * inaccurate. Provide a warning, and continue on.
> > + */
> > + hid_warn(hdev, "Unable to read IMU calibration data\n");
> > + }
> > +
> > + /* Set the reporting mode to 0x30, which is the full report mode */
> > + ret = joycon_set_report_mode(ctlr);
> > + if (ret) {
> > + hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> > + goto err_mutex;
> > + }
> > +
> > + /* Enable rumble */
> > + ret = joycon_enable_rumble(ctlr);
> > + if (ret) {
> > + hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> > + goto err_mutex;
> > + }
> > +
> > + /* Enable the IMU */
> > + ret = joycon_enable_imu(ctlr);
> > + if (ret) {
> > + hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> > + goto err_mutex;
> > + }
> > +
> > + mutex_unlock(&ctlr->output_mutex);
> > + return 0;
> > +
> > +err_mutex:
> > + mutex_unlock(&ctlr->output_mutex);
> > + return ret;
> > +}
> > +
> > /* Common handler for parsing inputs */
> > static int joycon_ctlr_read_handler(struct joycon_ctlr *ctlr, u8 *data,
> > int size)
> > @@ -2248,85 +2332,19 @@ static int nintendo_hid_probe(struct hid_device *hdev,
> >
> > hid_device_io_start(hdev);
> >
> > - /* Initialize the controller */
> > - mutex_lock(&ctlr->output_mutex);
> > - /* if handshake command fails, assume ble pro controller */
> > - if ((jc_type_is_procon(ctlr) || jc_type_is_chrggrip(ctlr)) &&
> > - !joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ)) {
> > - hid_dbg(hdev, "detected USB controller\n");
> > - /* set baudrate for improved latency */
> > - ret = joycon_send_usb(ctlr, JC_USB_CMD_BAUDRATE_3M, HZ);
> > - if (ret) {
> > - hid_err(hdev, "Failed to set baudrate; ret=%d\n", ret);
> > - goto err_mutex;
> > - }
> > - /* handshake */
> > - ret = joycon_send_usb(ctlr, JC_USB_CMD_HANDSHAKE, HZ);
> > - if (ret) {
> > - hid_err(hdev, "Failed handshake; ret=%d\n", ret);
> > - goto err_mutex;
> > - }
> > - /*
> > - * Set no timeout (to keep controller in USB mode).
> > - * This doesn't send a response, so ignore the timeout.
> > - */
> > - joycon_send_usb(ctlr, JC_USB_CMD_NO_TIMEOUT, HZ/10);
> > - } else if (jc_type_is_chrggrip(ctlr)) {
> > - hid_err(hdev, "Failed charging grip handshake\n");
> > - ret = -ETIMEDOUT;
> > - goto err_mutex;
> > - }
> > -
> > - /* get controller calibration data, and parse it */
> > - ret = joycon_request_calibration(ctlr);
> > - if (ret) {
> > - /*
> > - * We can function with default calibration, but it may be
> > - * inaccurate. Provide a warning, and continue on.
> > - */
> > - hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> > - }
> > -
> > - /* get IMU calibration data, and parse it */
> > - ret = joycon_request_imu_calibration(ctlr);
> > - if (ret) {
> > - /*
> > - * We can function with default calibration, but it may be
> > - * inaccurate. Provide a warning, and continue on.
> > - */
> > - hid_warn(hdev, "Unable to read IMU calibration data\n");
> > - }
> > -
> > - /* Set the reporting mode to 0x30, which is the full report mode */
> > - ret = joycon_set_report_mode(ctlr);
> > - if (ret) {
> > - hid_err(hdev, "Failed to set report mode; ret=%d\n", ret);
> > - goto err_mutex;
> > - }
> > -
> > - /* Enable rumble */
> > - ret = joycon_enable_rumble(ctlr);
> > - if (ret) {
> > - hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> > - goto err_mutex;
> > - }
> > -
> > - /* Enable the IMU */
> > - ret = joycon_enable_imu(ctlr);
> > + ret = joycon_init(hdev);
> > if (ret) {
> > - hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> > - goto err_mutex;
> > + hid_err(hdev, "Failed to initialize controller; ret=%d\n", ret);
> > + goto err_close;
> > }
> >
> > ret = joycon_read_info(ctlr);
> > if (ret) {
> > hid_err(hdev, "Failed to retrieve controller info; ret=%d\n",
> > ret);
> > - goto err_mutex;
> > + goto err_close;
> > }
> >
> > - mutex_unlock(&ctlr->output_mutex);
> > -
> > /* Initialize the leds */
> > ret = joycon_leds_create(ctlr);
> > if (ret) {
> > @@ -2352,8 +2370,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
> > hid_dbg(hdev, "probe - success\n");
> > return 0;
> >
> > -err_mutex:
> > - mutex_unlock(&ctlr->output_mutex);
> > err_close:
> > hid_hw_close(hdev);
> > err_stop:
> > @@ -2383,6 +2399,20 @@ static void nintendo_hid_remove(struct hid_device *hdev)
> > hid_hw_stop(hdev);
> > }
> >
> > +#ifdef CONFIG_PM
> > +
> > +static int nintendo_hid_resume(struct hid_device *hdev)
> > +{
> > + int ret = joycon_init(hdev);
> > +
> > + if (ret)
> > + hid_err(hdev, "Failed to restore controller after resume");
> > +
> > + return ret;
> > +}
> > +
> > +#endif
> > +
> > static const struct hid_device_id nintendo_hid_devices[] = {
> > { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> > USB_DEVICE_ID_NINTENDO_PROCON) },
> > @@ -2404,6 +2434,10 @@ static struct hid_driver nintendo_hid_driver = {
> > .probe = nintendo_hid_probe,
> > .remove = nintendo_hid_remove,
> > .raw_event = nintendo_hid_event,
> > +
> > +#ifdef CONFIG_PM
> > + .resume = nintendo_hid_resume,
> > +#endif
> > };
> > module_hid_driver(nintendo_hid_driver);
> >
>
>
^ permalink raw reply
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