From e26a78057c25dd56f112d536319c38735ed92ba4 Mon Sep 17 00:00:00 2001 From: Alex Henrie Date: Sun, 16 Jan 2022 16:01:58 -0700 Subject: [PATCH 01/11] HID: apple: Set the tilde quirk flag on the Wellspring 5 and later Markus reports that his 2011 MacBook with a German ISO keyboard (USB product code 05ac:0246, HID country code 13) has the tilde key quirk. Seeing as all of the standalone Apple ISO keyboards since about 2008 have the quirk, it seems reasonable to assume that once the integrated laptop keyboards started having the quirk, they likewise never stopped having it. Reported-by: Markus Wageringel Signed-off-by: Alex Henrie Signed-off-by: Jiri Kosina --- drivers/hid/hid-apple.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c index 24802a4a636ee..7dc89dc6b0f0e 100644 --- a/drivers/hid/hid-apple.c +++ b/drivers/hid/hid-apple.c @@ -691,49 +691,49 @@ static const struct hid_device_id apple_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5_ANSI), .driver_data = APPLE_HAS_FN }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5_ISO), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5_JIS), .driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6_ANSI), .driver_data = APPLE_HAS_FN }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6_ISO), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6_JIS), .driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6A_ANSI), .driver_data = APPLE_HAS_FN }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6A_ISO), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING6A_JIS), .driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5A_ANSI), .driver_data = APPLE_HAS_FN }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5A_ISO), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING5A_JIS), .driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7_ANSI), .driver_data = APPLE_HAS_FN }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7_ISO), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7_JIS), .driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7A_ANSI), .driver_data = APPLE_HAS_FN }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7A_ISO), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING7A_JIS), .driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING8_ANSI), .driver_data = APPLE_HAS_FN }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING8_ISO), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING8_JIS), .driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING9_ANSI), .driver_data = APPLE_HAS_FN }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING9_ISO), - .driver_data = APPLE_HAS_FN }, + .driver_data = APPLE_HAS_FN | APPLE_ISO_TILDE_QUIRK }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_WELLSPRING9_JIS), .driver_data = APPLE_HAS_FN | APPLE_RDESC_JIS }, { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_WIRELESS_2009_ANSI), From 817b8b9c5396d2b2d92311b46719aad5d3339dbe Mon Sep 17 00:00:00 2001 From: Dongliang Mu Date: Sat, 22 Jan 2022 17:48:26 +0800 Subject: [PATCH 02/11] HID: elo: fix memory leak in elo_probe When hid_parse() in elo_probe() fails, it forgets to call usb_put_dev to decrease the refcount. Fix this by adding usb_put_dev() in the error handling code of elo_probe(). Fixes: fbf42729d0e9 ("HID: elo: update the reference count of the usb device structure") Reported-by: syzkaller Signed-off-by: Dongliang Mu Signed-off-by: Jiri Kosina --- drivers/hid/hid-elo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/hid/hid-elo.c b/drivers/hid/hid-elo.c index 8e960d7b233b3..9b42b0cdeef06 100644 --- a/drivers/hid/hid-elo.c +++ b/drivers/hid/hid-elo.c @@ -262,6 +262,7 @@ static int elo_probe(struct hid_device *hdev, const struct hid_device_id *id) return 0; err_free: + usb_put_dev(udev); kfree(priv); return ret; } From 2787710f73fcce4a9bdab540aaf1aef778a27462 Mon Sep 17 00:00:00 2001 From: Daniel Thompson Date: Fri, 28 Jan 2022 17:46:25 +0000 Subject: [PATCH 03/11] HID: i2c-hid: goodix: Fix a lockdep splat I'm was on the receiving end of a lockdep splat from this driver and after scratching my head I couldn't be entirely sure it was a false positive given we would also have to think about whether the regulator locking is safe (since the notifier is called whilst holding regulator locks which are also needed for regulator_is_enabled() ). Regardless of whether it is a real bug or not, the mutex isn't needed. We can use reference counting tricks instead to avoid races with the notifier calls. The observed splat follows: ------------------------------------------------------ kworker/u16:3/127 is trying to acquire lock: ffff00008021fb20 (&ihid_goodix->regulator_mutex){+.+.}-{4:4}, at: ihid_goodix_vdd_notify+0x30/0x94 but task is already holding lock: ffff0000835c60c0 (&(&rdev->notifier)->rwsem){++++}-{4:4}, at: blocking_notifier_call_chain+0x30/0x70 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&(&rdev->notifier)->rwsem){++++}-{4:4}: down_write+0x68/0x8c blocking_notifier_chain_register+0x54/0x70 regulator_register_notifier+0x1c/0x24 devm_regulator_register_notifier+0x58/0x98 i2c_hid_of_goodix_probe+0xdc/0x158 i2c_device_probe+0x25d/0x270 really_probe+0x174/0x2cc __driver_probe_device+0xc0/0xd8 driver_probe_device+0x50/0xe4 __device_attach_driver+0xa8/0xc0 bus_for_each_drv+0x9c/0xc0 __device_attach_async_helper+0x6c/0xbc async_run_entry_fn+0x38/0x100 process_one_work+0x294/0x438 worker_thread+0x180/0x258 kthread+0x120/0x130 ret_from_fork+0x10/0x20 -> #0 (&ihid_goodix->regulator_mutex){+.+.}-{4:4}: __lock_acquire+0xd24/0xfe8 lock_acquire+0x288/0x2f4 __mutex_lock+0xa0/0x338 mutex_lock_nested+0x3c/0x5c ihid_goodix_vdd_notify+0x30/0x94 notifier_call_chain+0x6c/0x8c blocking_notifier_call_chain+0x48/0x70 _notifier_call_chain.isra.0+0x18/0x20 _regulator_enable+0xc0/0x178 regulator_enable+0x40/0x7c goodix_i2c_hid_power_up+0x18/0x20 i2c_hid_core_power_up.isra.0+0x1c/0x2c i2c_hid_core_probe+0xd8/0x3d4 i2c_hid_of_goodix_probe+0x14c/0x158 i2c_device_probe+0x25c/0x270 really_probe+0x174/0x2cc __driver_probe_device+0xc0/0xd8 driver_probe_device+0x50/0xe4 __device_attach_driver+0xa8/0xc0 bus_for_each_drv+0x9c/0xc0 __device_attach_async_helper+0x6c/0xbc async_run_entry_fn+0x38/0x100 process_one_work+0x294/0x438 worker_thread+0x180/0x258 kthread+0x120/0x130 ret_from_fork+0x10/0x20 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&(&rdev->notifier)->rwsem); lock(&ihid_goodix->regulator_mutex); lock(&(&rdev->notifier)->rwsem); lock(&ihid_goodix->regulator_mutex); *** DEADLOCK *** Signed-off-by: Daniel Thompson Fixes: 18eeef46d359 ("HID: i2c-hid: goodix: Tie the reset line to true state of the regulator") Reviewed-by: Douglas Anderson Signed-off-by: Jiri Kosina --- drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 28 +++++++++++-------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c index b4dad66fa954d..ec6c73f75ffe0 100644 --- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c +++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c @@ -27,7 +27,6 @@ struct i2c_hid_of_goodix { struct regulator *vdd; struct notifier_block nb; - struct mutex regulator_mutex; struct gpio_desc *reset_gpio; const struct goodix_i2c_hid_timing_data *timings; }; @@ -67,8 +66,6 @@ static int ihid_goodix_vdd_notify(struct notifier_block *nb, container_of(nb, struct i2c_hid_of_goodix, nb); int ret = NOTIFY_OK; - mutex_lock(&ihid_goodix->regulator_mutex); - switch (event) { case REGULATOR_EVENT_PRE_DISABLE: gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1); @@ -87,8 +84,6 @@ static int ihid_goodix_vdd_notify(struct notifier_block *nb, break; } - mutex_unlock(&ihid_goodix->regulator_mutex); - return ret; } @@ -102,8 +97,6 @@ static int i2c_hid_of_goodix_probe(struct i2c_client *client, if (!ihid_goodix) return -ENOMEM; - mutex_init(&ihid_goodix->regulator_mutex); - ihid_goodix->ops.power_up = goodix_i2c_hid_power_up; ihid_goodix->ops.power_down = goodix_i2c_hid_power_down; @@ -130,25 +123,28 @@ static int i2c_hid_of_goodix_probe(struct i2c_client *client, * long. Holding the controller in reset apparently draws extra * power. */ - mutex_lock(&ihid_goodix->regulator_mutex); ihid_goodix->nb.notifier_call = ihid_goodix_vdd_notify; ret = devm_regulator_register_notifier(ihid_goodix->vdd, &ihid_goodix->nb); - if (ret) { - mutex_unlock(&ihid_goodix->regulator_mutex); + if (ret) return dev_err_probe(&client->dev, ret, "regulator notifier request failed\n"); - } /* * If someone else is holding the regulator on (or the regulator is * an always-on one) we might never be told to deassert reset. Do it - * now. Here we'll assume that someone else might have _just - * barely_ turned the regulator on so we'll do the full - * "post_power_delay" just in case. + * now... and temporarily bump the regulator reference count just to + * make sure it is impossible for this to race with our own notifier! + * We also assume that someone else might have _just barely_ turned + * the regulator on so we'll do the full "post_power_delay" just in + * case. */ - if (ihid_goodix->reset_gpio && regulator_is_enabled(ihid_goodix->vdd)) + if (ihid_goodix->reset_gpio && regulator_is_enabled(ihid_goodix->vdd)) { + ret = regulator_enable(ihid_goodix->vdd); + if (ret) + return ret; goodix_i2c_hid_deassert_reset(ihid_goodix, true); - mutex_unlock(&ihid_goodix->regulator_mutex); + regulator_disable(ihid_goodix->vdd); + } return i2c_hid_core_probe(client, &ihid_goodix->ops, 0x0001, 0); } From a7072c01c3ac3ae6ecd08fa7b43431cfc8ed331f Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Mon, 31 Jan 2022 22:48:32 +0530 Subject: [PATCH 04/11] HID: amd_sfh: Increase sensor command timeout HPD sensors take more time to initialize. Hence increasing sensor command timeout to get response with status within a max timeout. Fixes: 173709f50e98 ("HID: amd_sfh: Add command response to check command status") Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c index 2503be0253d3e..d3f32ffe299a8 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c @@ -37,11 +37,11 @@ static int amd_sfh_wait_response_v2(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_ { union cmd_response cmd_resp; - /* Get response with status within a max of 800 ms timeout */ + /* Get response with status within a max of 1600 ms timeout */ if (!readl_poll_timeout(mp2->mmio + AMD_P2C_MSG(0), cmd_resp.resp, (cmd_resp.response_v2.response == sensor_sts && cmd_resp.response_v2.status == 0 && (sid == 0xff || - cmd_resp.response_v2.sensor_id == sid)), 500, 800000)) + cmd_resp.response_v2.sensor_id == sid)), 500, 1600000)) return cmd_resp.response_v2.response; return SENSOR_DISABLED; From 91aaea527bc3b707c5d3208cde035421ed54f79c Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Mon, 31 Jan 2022 22:48:33 +0530 Subject: [PATCH 05/11] HID: amd_sfh: Add illuminance mask to limit ALS max value ALS illuminance value present only in first 15 bits from SFH firmware for V2 platforms. Hence added a mask of 15 bit to limit ALS max illuminance values to get correct illuminance value. Fixes: 0aad9c95eb9a ("HID: amd_sfh: Extend ALS support for newer AMD platform") Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c index be41f83b0289c..76095bd53c655 100644 --- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c +++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c @@ -27,6 +27,7 @@ #define HID_USAGE_SENSOR_STATE_READY_ENUM 0x02 #define HID_USAGE_SENSOR_STATE_INITIALIZING_ENUM 0x05 #define HID_USAGE_SENSOR_EVENT_DATA_UPDATED_ENUM 0x04 +#define ILLUMINANCE_MASK GENMASK(14, 0) int get_report_descriptor(int sensor_idx, u8 *rep_desc) { @@ -246,7 +247,8 @@ u8 get_input_report(u8 current_index, int sensor_idx, int report_id, struct amd_ get_common_inputs(&als_input.common_property, report_id); /* For ALS ,V2 Platforms uses C2P_MSG5 register instead of DRAM access method */ if (supported_input == V2_STATUS) - als_input.illuminance_value = (int)readl(privdata->mmio + AMD_C2P_MSG(5)); + als_input.illuminance_value = + readl(privdata->mmio + AMD_C2P_MSG(5)) & ILLUMINANCE_MASK; else als_input.illuminance_value = (int)sensor_virt_addr[0] / AMD_SFH_FW_MULTIPLIER; From fd5dd6acd8f823ea804f76d3af64fa1be9d5fb78 Mon Sep 17 00:00:00 2001 From: Sergio Costas Date: Fri, 4 Feb 2022 10:01:17 +0100 Subject: [PATCH 06/11] HID:Add support for UGTABLET WP5540 This patch adds support for the UGTABLET WP5540 digitizer tablet devices. Without it, the pen moves the cursor, but neither the buttons nor the tap sensor in the tip do work. Signed-off-by: Sergio Costas Link: https://lore.kernel.org/r/63dece1d-91ca-1b1b-d90d-335be66896be@gmail.com Cc: stable@vger.kernel.org Signed-off-by: Benjamin Tissoires --- 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 85975031389b3..78bd3ddda4426 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -1370,6 +1370,7 @@ #define USB_VENDOR_ID_UGTIZER 0x2179 #define USB_DEVICE_ID_UGTIZER_TABLET_GP0610 0x0053 #define USB_DEVICE_ID_UGTIZER_TABLET_GT5040 0x0077 +#define USB_DEVICE_ID_UGTIZER_TABLET_WP5540 0x0004 #define USB_VENDOR_ID_VIEWSONIC 0x0543 #define USB_DEVICE_ID_VIEWSONIC_PD1011 0xe621 diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c index 9af1dc8ae3a2f..c066ba901867b 100644 --- a/drivers/hid/hid-quirks.c +++ b/drivers/hid/hid-quirks.c @@ -187,6 +187,7 @@ static const struct hid_device_id hid_quirks[] = { { HID_USB_DEVICE(USB_VENDOR_ID_TURBOX, USB_DEVICE_ID_TURBOX_KEYBOARD), HID_QUIRK_NOGET }, { HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_KNA5), HID_QUIRK_MULTI_INPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_TWA60), HID_QUIRK_MULTI_INPUT }, + { HID_USB_DEVICE(USB_VENDOR_ID_UGTIZER, USB_DEVICE_ID_UGTIZER_TABLET_WP5540), HID_QUIRK_MULTI_INPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_MEDIA_TABLET_10_6_INCH), HID_QUIRK_MULTI_INPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_MEDIA_TABLET_14_1_INCH), HID_QUIRK_MULTI_INPUT }, { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_SIRIUS_BATTERY_FREE_TABLET), HID_QUIRK_MULTI_INPUT }, From 0cf74235f4403b760a37f77271d2ca3424001ff9 Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Tue, 8 Feb 2022 17:51:08 +0530 Subject: [PATCH 07/11] HID: amd_sfh: Handle amd_sfh work buffer in PM ops Since in the current amd_sfh design the sensor data is periodically obtained in the form of poll data, during the suspend/resume cycle, scheduling a delayed work adds no value. So, cancel the work and restart back during the suspend/resume cycle respectively. Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c index d3f32ffe299a8..dacac30a6b27a 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c @@ -290,6 +290,8 @@ static int __maybe_unused amd_mp2_pci_resume(struct device *dev) } } + schedule_delayed_work(&cl_data->work_buffer, msecs_to_jiffies(AMD_SFH_IDLE_LOOP)); + return 0; } @@ -312,6 +314,8 @@ static int __maybe_unused amd_mp2_pci_suspend(struct device *dev) } } + cancel_delayed_work_sync(&cl_data->work_buffer); + return 0; } From aa0b724a2bf041036e56cbb3b4b3afde7c5e7c9e Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Tue, 8 Feb 2022 17:51:09 +0530 Subject: [PATCH 08/11] HID: amd_sfh: Correct the structure field name Misinterpreted intr_enable field name. Hence correct the structure field name accordingly to reflect the functionality. Fixes: f264481ad614 ("HID: amd_sfh: Extend driver capabilities for multi-generation support") Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/amd_sfh_pcie.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h index ae30e059f8475..8a9c544c27aef 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h @@ -49,7 +49,7 @@ union sfh_cmd_base { } s; struct { u32 cmd_id : 4; - u32 intr_enable : 1; + u32 intr_disable : 1; u32 rsvd1 : 3; u32 length : 7; u32 mem_type : 1; From b300667b33b2b5a2c8e5f8f22826befb3d7f4f2b Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Tue, 8 Feb 2022 17:51:10 +0530 Subject: [PATCH 09/11] HID: amd_sfh: Disable the interrupt for all command Sensor data is processed in polling mode. Hence disable the interrupt for all sensor command. Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c index dacac30a6b27a..16670e910014b 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c @@ -53,6 +53,7 @@ static void amd_start_sensor_v2(struct amd_mp2_dev *privdata, struct amd_mp2_sen cmd_base.ul = 0; cmd_base.cmd_v2.cmd_id = ENABLE_SENSOR; + cmd_base.cmd_v2.intr_disable = 1; cmd_base.cmd_v2.period = info.period; cmd_base.cmd_v2.sensor_id = info.sensor_idx; cmd_base.cmd_v2.length = 16; @@ -70,6 +71,7 @@ static void amd_stop_sensor_v2(struct amd_mp2_dev *privdata, u16 sensor_idx) cmd_base.ul = 0; cmd_base.cmd_v2.cmd_id = DISABLE_SENSOR; + cmd_base.cmd_v2.intr_disable = 1; cmd_base.cmd_v2.period = 0; cmd_base.cmd_v2.sensor_id = sensor_idx; cmd_base.cmd_v2.length = 16; @@ -83,6 +85,7 @@ static void amd_stop_all_sensor_v2(struct amd_mp2_dev *privdata) union sfh_cmd_base cmd_base; cmd_base.cmd_v2.cmd_id = STOP_ALL_SENSORS; + cmd_base.cmd_v2.intr_disable = 1; cmd_base.cmd_v2.period = 0; cmd_base.cmd_v2.sensor_id = 0; From fb75a3791a8032848c987db29b622878d8fe2b1c Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Tue, 8 Feb 2022 17:51:11 +0530 Subject: [PATCH 10/11] HID: amd_sfh: Add functionality to clear interrupts Newer AMD platforms with SFH may generate interrupts on some events which are unwarranted. Until this is cleared the actual MP2 data processing maybe stalled in some cases. Add a mechanism to clear the pending interrupts (if any) during the driver initialization and sensor command operations. Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 25 ++++++++++++++++++++++++- drivers/hid/amd-sfh-hid/amd_sfh_pcie.h | 1 + 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c index 16670e910014b..7236b223fb68e 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c @@ -92,6 +92,20 @@ static void amd_stop_all_sensor_v2(struct amd_mp2_dev *privdata) writel(cmd_base.ul, privdata->mmio + AMD_C2P_MSG0); } +static void amd_sfh_clear_intr_v2(struct amd_mp2_dev *privdata) +{ + if (readl(privdata->mmio + AMD_P2C_MSG(4))) { + writel(0, privdata->mmio + AMD_P2C_MSG(4)); + writel(0xf, privdata->mmio + AMD_P2C_MSG(5)); + } +} + +static void amd_sfh_clear_intr(struct amd_mp2_dev *privdata) +{ + if (privdata->mp2_ops->clear_intr) + privdata->mp2_ops->clear_intr(privdata); +} + void amd_start_sensor(struct amd_mp2_dev *privdata, struct amd_mp2_sensor_info info) { union sfh_cmd_param cmd_param; @@ -196,6 +210,7 @@ static void amd_mp2_pci_remove(void *privdata) struct amd_mp2_dev *mp2 = privdata; amd_sfh_hid_client_deinit(privdata); mp2->mp2_ops->stop_all(mp2); + amd_sfh_clear_intr(mp2); } static const struct amd_mp2_ops amd_sfh_ops_v2 = { @@ -203,6 +218,7 @@ static const struct amd_mp2_ops amd_sfh_ops_v2 = { .stop = amd_stop_sensor_v2, .stop_all = amd_stop_all_sensor_v2, .response = amd_sfh_wait_response_v2, + .clear_intr = amd_sfh_clear_intr_v2, }; static const struct amd_mp2_ops amd_sfh_ops = { @@ -265,8 +281,13 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i mp2_select_ops(privdata); rc = amd_sfh_hid_client_init(privdata); - if (rc) + if (rc) { + amd_sfh_clear_intr(privdata); + dev_err(&pdev->dev, "amd_sfh_hid_client_init failed\n"); return rc; + } + + amd_sfh_clear_intr(privdata); return devm_add_action_or_reset(&pdev->dev, amd_mp2_pci_remove, privdata); } @@ -294,6 +315,7 @@ static int __maybe_unused amd_mp2_pci_resume(struct device *dev) } schedule_delayed_work(&cl_data->work_buffer, msecs_to_jiffies(AMD_SFH_IDLE_LOOP)); + amd_sfh_clear_intr(mp2); return 0; } @@ -318,6 +340,7 @@ static int __maybe_unused amd_mp2_pci_suspend(struct device *dev) } cancel_delayed_work_sync(&cl_data->work_buffer); + amd_sfh_clear_intr(mp2); return 0; } diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h index 8a9c544c27aef..61de09ba51242 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h @@ -141,5 +141,6 @@ struct amd_mp2_ops { void (*stop)(struct amd_mp2_dev *privdata, u16 sensor_idx); void (*stop_all)(struct amd_mp2_dev *privdata); int (*response)(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts); + void (*clear_intr)(struct amd_mp2_dev *privdata); }; #endif From 7f016b35ca7623c71b31facdde080e8ce171a697 Mon Sep 17 00:00:00 2001 From: Basavaraj Natikar Date: Tue, 8 Feb 2022 17:51:12 +0530 Subject: [PATCH 11/11] HID: amd_sfh: Add interrupt handler to process interrupts On newer AMD platforms with SFH, it is observed that random interrupts get generated on the SFH hardware and until this is cleared the firmware sensor processing is stalled, resulting in no data been received to driver side. Add routines to handle these interrupts, so that firmware operations are not stalled. Signed-off-by: Basavaraj Natikar Signed-off-by: Jiri Kosina --- drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 40 ++++++++++++++++++++++++++ drivers/hid/amd-sfh-hid/amd_sfh_pcie.h | 1 + 2 files changed, 41 insertions(+) diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c index 7236b223fb68e..19fa734a9a793 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c @@ -106,6 +106,30 @@ static void amd_sfh_clear_intr(struct amd_mp2_dev *privdata) privdata->mp2_ops->clear_intr(privdata); } +static irqreturn_t amd_sfh_irq_handler(int irq, void *data) +{ + amd_sfh_clear_intr(data); + + return IRQ_HANDLED; +} + +static int amd_sfh_irq_init_v2(struct amd_mp2_dev *privdata) +{ + int rc; + + pci_intx(privdata->pdev, true); + + rc = devm_request_irq(&privdata->pdev->dev, privdata->pdev->irq, + amd_sfh_irq_handler, 0, DRIVER_NAME, privdata); + if (rc) { + dev_err(&privdata->pdev->dev, "failed to request irq %d err=%d\n", + privdata->pdev->irq, rc); + return rc; + } + + return 0; +} + void amd_start_sensor(struct amd_mp2_dev *privdata, struct amd_mp2_sensor_info info) { union sfh_cmd_param cmd_param; @@ -210,6 +234,7 @@ static void amd_mp2_pci_remove(void *privdata) struct amd_mp2_dev *mp2 = privdata; amd_sfh_hid_client_deinit(privdata); mp2->mp2_ops->stop_all(mp2); + pci_intx(mp2->pdev, false); amd_sfh_clear_intr(mp2); } @@ -219,6 +244,7 @@ static const struct amd_mp2_ops amd_sfh_ops_v2 = { .stop_all = amd_stop_all_sensor_v2, .response = amd_sfh_wait_response_v2, .clear_intr = amd_sfh_clear_intr_v2, + .init_intr = amd_sfh_irq_init_v2, }; static const struct amd_mp2_ops amd_sfh_ops = { @@ -244,6 +270,14 @@ static void mp2_select_ops(struct amd_mp2_dev *privdata) } } +static int amd_sfh_irq_init(struct amd_mp2_dev *privdata) +{ + if (privdata->mp2_ops->init_intr) + return privdata->mp2_ops->init_intr(privdata); + + return 0; +} + static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { struct amd_mp2_dev *privdata; @@ -280,6 +314,12 @@ static int amd_mp2_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i mp2_select_ops(privdata); + rc = amd_sfh_irq_init(privdata); + if (rc) { + dev_err(&pdev->dev, "amd_sfh_irq_init failed\n"); + return rc; + } + rc = amd_sfh_hid_client_init(privdata); if (rc) { amd_sfh_clear_intr(privdata); diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h index 61de09ba51242..97b99861fae25 100644 --- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h +++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h @@ -142,5 +142,6 @@ struct amd_mp2_ops { void (*stop_all)(struct amd_mp2_dev *privdata); int (*response)(struct amd_mp2_dev *mp2, u8 sid, u32 sensor_sts); void (*clear_intr)(struct amd_mp2_dev *privdata); + int (*init_intr)(struct amd_mp2_dev *privdata); }; #endif