* [PATCH 00/17] Further hid-pidff improvements and fixes
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
Another batch of improvements/fixes/updates to the hid-pidff driver. A lot of
code quality improvements with probably more to come as we better understand the
driver and strive to simplify it's inner workings. I think we're currently past
75% of touchups + Oleg is working on some compatibility changes for Simagic
support in a "pass-through" mode.
Direction fix only for conditional effects fixes FFB in Forza games on Moza.
I removed Anssi's email from the "welcome message" that appears on succesful
PID init to make sure people will look for LKML to send in bug reports.
Tomasz Pakuła (17):
HID: pidff: Use direction fix only for conditional effects
HID: pidff: Remove unhelpful pidff_set_actuators helper
HID: pidff: Remove unneeded debug
HID: pidff: Use ARRAY_SIZE macro instead of sizeof
HID: pidff: Treat PID_REQUIRED_REPORTS as count, not max
HID: pidff: Better quirk assigment when searching for fields
HID: pidff: Simplify HID field/usage searching logic
HID: pidff: Add support for AXES_ENABLE field
HID: pidff: Update debug messages
HID: pidff: Rework pidff_upload_effect
HID: pidff: Separate check for infinite duration
HID: pidff: PERMISSIVE_CONTROL quirk autodetection
HID: pidff: Remove Anssi's email address from info msg
HID: pidff: Define all cardinal directions
HID: pidff: clang-format pass
HID: universal-pidff: clang-format pass
HID: pidff: Reduce PID_EFFECT_OPERATION spam
drivers/hid/hid-universal-pidff.c | 57 +--
drivers/hid/usbhid/hid-pidff.c | 711 +++++++++++++++++-------------
drivers/hid/usbhid/hid-pidff.h | 2 +-
3 files changed, 439 insertions(+), 331 deletions(-)
--
2.50.1
^ permalink raw reply
* [PATCH 01/17] HID: pidff: Use direction fix only for conditional effects
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
The already fixed bug in SDL only affected conditional effects. This
should fix FFB in Forza Horizion 4/5 on Moza Devices as Forza Horizon
flips the constant force direction instead of using negative magnitude
values.
Changing the direction in the effect directly in pidff_upload_effect()
would affect it's value in further operations like comparing to the old
effect and/or just reading the effect values in the user application.
This, in turn, would lead to constant PID_SET_EFFECT spam as the effect
direction would constantly not match the value that's set by the
application.
This way, it's still transparent to any software/API.
Only affects conditional effects now so it's better for it to explicitly
state that in the name. If any HW ever needs fixed direction for other
effects, we'll add more quirks.
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
Reviewed-by: Oleg Makarenko <oleg@makarenk.ooo>
---
drivers/hid/hid-universal-pidff.c | 20 ++++++++++----------
drivers/hid/usbhid/hid-pidff.c | 28 +++++++++++++++++++++++-----
drivers/hid/usbhid/hid-pidff.h | 2 +-
3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/hid-universal-pidff.c b/drivers/hid/hid-universal-pidff.c
index 554a6559aeb7..70fce0f88e82 100644
--- a/drivers/hid/hid-universal-pidff.c
+++ b/drivers/hid/hid-universal-pidff.c
@@ -144,25 +144,25 @@ static int universal_pidff_input_configured(struct hid_device *hdev,
static const struct hid_device_id universal_pidff_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R3),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R3_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R5),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R5_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R9),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R9_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R12),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R12_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R16_R21),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R16_R21_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_CAMMUS, USB_DEVICE_ID_CAMMUS_C5) },
{ HID_USB_DEVICE(USB_VENDOR_ID_CAMMUS, USB_DEVICE_ID_CAMMUS_C12) },
{ HID_USB_DEVICE(USB_VENDOR_ID_VRS, USB_DEVICE_ID_VRS_DFP),
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 614a20b62023..c6b4f61e535d 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -205,6 +205,14 @@ struct pidff_device {
u8 effect_count;
};
+static int pidff_is_effect_conditional(struct ff_effect *effect)
+{
+ return effect->type == FF_SPRING ||
+ effect->type == FF_DAMPER ||
+ effect->type == FF_INERTIA ||
+ effect->type == FF_FRICTION;
+}
+
/*
* Clamp value for a given field
*/
@@ -294,6 +302,20 @@ static void pidff_set_duration(struct pidff_usage *usage, u16 duration)
pidff_set_time(usage, duration);
}
+static void pidff_set_effect_direction(struct pidff_device *pidff,
+ struct ff_effect *effect)
+{
+ u16 direction = effect->direction;
+
+ /* Use fixed direction if needed */
+ if (pidff->quirks & HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION &&
+ pidff_is_effect_conditional(effect))
+ direction = PIDFF_FIXED_WHEEL_DIRECTION;
+
+ pidff->effect_direction->value[0] =
+ pidff_rescale(direction, U16_MAX, pidff->effect_direction);
+}
+
/*
* Send envelope report to the device
*/
@@ -395,11 +417,7 @@ static void pidff_set_effect_report(struct pidff_device *pidff,
pidff->set_effect[PID_GAIN].field->logical_maximum;
pidff->set_effect[PID_DIRECTION_ENABLE].value[0] = 1;
- /* Use fixed direction if needed */
- pidff->effect_direction->value[0] = pidff_rescale(
- pidff->quirks & HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION ?
- PIDFF_FIXED_WHEEL_DIRECTION : effect->direction,
- U16_MAX, pidff->effect_direction);
+ pidff_set_effect_direction(pidff, effect);
/* Omit setting delay field if it's missing */
if (!(pidff->quirks & HID_PIDFF_QUIRK_MISSING_DELAY))
diff --git a/drivers/hid/usbhid/hid-pidff.h b/drivers/hid/usbhid/hid-pidff.h
index a53a8b436baa..f321f675e131 100644
--- a/drivers/hid/usbhid/hid-pidff.h
+++ b/drivers/hid/usbhid/hid-pidff.h
@@ -16,7 +16,7 @@
#define HID_PIDFF_QUIRK_PERMISSIVE_CONTROL BIT(2)
/* Use fixed 0x4000 direction during SET_EFFECT report upload */
-#define HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION BIT(3)
+#define HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION BIT(3)
/* Force all periodic effects to be uploaded as SINE */
#define HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY BIT(4)
--
2.50.1
^ permalink raw reply related
* [PATCH 02/17] HID: pidff: Remove unhelpful pidff_set_actuators helper
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Abstracts away too little of the functionality and replaces a nice,
defined value with a magic bool. There's no actual need for it.
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
Reviewed-by: Oleg Makarenko <oleg@makarenk.ooo>
---
drivers/hid/usbhid/hid-pidff.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index c6b4f61e535d..3cf844f7ad4a 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -605,16 +605,6 @@ static void pidff_set_device_control(struct pidff_device *pidff, int field)
hid_hw_wait(pidff->hid);
}
-/*
- * Modify actuators state
- */
-static void pidff_set_actuators(struct pidff_device *pidff, bool enable)
-{
- hid_dbg(pidff->hid, "%s actuators\n", enable ? "Enable" : "Disable");
- pidff_set_device_control(pidff,
- enable ? PID_ENABLE_ACTUATORS : PID_DISABLE_ACTUATORS);
-}
-
/*
* Reset the device, stop all effects, enable actuators
*/
@@ -626,7 +616,7 @@ static void pidff_reset(struct pidff_device *pidff)
pidff->effect_count = 0;
pidff_set_device_control(pidff, PID_STOP_ALL_EFFECTS);
- pidff_set_actuators(pidff, 1);
+ pidff_set_device_control(pidff, PID_ENABLE_ACTUATORS);
}
/*
--
2.50.1
^ permalink raw reply related
* [PATCH 03/17] HID: pidff: Remove unneeded debug
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
Reviewed-by: Oleg Makarenko <oleg@makarenk.ooo>
---
drivers/hid/usbhid/hid-pidff.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 3cf844f7ad4a..75fc6dbe435c 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -339,10 +339,6 @@ static void pidff_set_envelope_report(struct pidff_device *pidff,
pidff_set_time(&pidff->set_envelope[PID_FADE_TIME],
envelope->fade_length);
- hid_dbg(pidff->hid, "attack %u => %d\n",
- envelope->attack_level,
- pidff->set_envelope[PID_ATTACK_LEVEL].value[0]);
-
hid_hw_request(pidff->hid, pidff->reports[PID_SET_ENVELOPE],
HID_REQ_SET_REPORT);
}
--
2.50.1
^ permalink raw reply related
* [PATCH 04/17] HID: pidff: Use ARRAY_SIZE macro instead of sizeof
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Could lead to issues when arrays won't be 8 bit fields
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
Reviewed-by: Oleg Makarenko <oleg@makarenk.ooo>
---
drivers/hid/usbhid/hid-pidff.c | 46 +++++++++++++++++-----------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 75fc6dbe435c..2f9fbe4c52d7 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -158,20 +158,20 @@ struct pidff_usage {
struct pidff_device {
struct hid_device *hid;
- struct hid_report *reports[sizeof(pidff_reports)];
+ struct hid_report *reports[ARRAY_SIZE(pidff_reports)];
- struct pidff_usage set_effect[sizeof(pidff_set_effect)];
- struct pidff_usage set_envelope[sizeof(pidff_set_envelope)];
- struct pidff_usage set_condition[sizeof(pidff_set_condition)];
- struct pidff_usage set_periodic[sizeof(pidff_set_periodic)];
- struct pidff_usage set_constant[sizeof(pidff_set_constant)];
- struct pidff_usage set_ramp[sizeof(pidff_set_ramp)];
+ struct pidff_usage set_effect[ARRAY_SIZE(pidff_set_effect)];
+ struct pidff_usage set_envelope[ARRAY_SIZE(pidff_set_envelope)];
+ struct pidff_usage set_condition[ARRAY_SIZE(pidff_set_condition)];
+ struct pidff_usage set_periodic[ARRAY_SIZE(pidff_set_periodic)];
+ struct pidff_usage set_constant[ARRAY_SIZE(pidff_set_constant)];
+ struct pidff_usage set_ramp[ARRAY_SIZE(pidff_set_ramp)];
- struct pidff_usage device_gain[sizeof(pidff_device_gain)];
- struct pidff_usage block_load[sizeof(pidff_block_load)];
- struct pidff_usage pool[sizeof(pidff_pool)];
- struct pidff_usage effect_operation[sizeof(pidff_effect_operation)];
- struct pidff_usage block_free[sizeof(pidff_block_free)];
+ struct pidff_usage device_gain[ARRAY_SIZE(pidff_device_gain)];
+ struct pidff_usage block_load[ARRAY_SIZE(pidff_block_load)];
+ struct pidff_usage pool[ARRAY_SIZE(pidff_pool)];
+ struct pidff_usage effect_operation[ARRAY_SIZE(pidff_effect_operation)];
+ struct pidff_usage block_free[ARRAY_SIZE(pidff_block_free)];
/*
* Special field is a field that is not composed of
@@ -194,10 +194,10 @@ struct pidff_device {
/* Special field in effect_operation */
struct hid_field *effect_operation_status;
- int control_id[sizeof(pidff_device_control)];
- int type_id[sizeof(pidff_effect_types)];
- int status_id[sizeof(pidff_block_load_status)];
- int operation_id[sizeof(pidff_effect_operation_status)];
+ int control_id[ARRAY_SIZE(pidff_device_control)];
+ int type_id[ARRAY_SIZE(pidff_effect_types)];
+ int status_id[ARRAY_SIZE(pidff_block_load_status)];
+ int operation_id[ARRAY_SIZE(pidff_effect_operation_status)];
int pid_id[PID_EFFECTS_MAX];
@@ -583,7 +583,7 @@ static void pidff_set_device_control(struct pidff_device *pidff, int field)
hid_dbg(pidff->hid, "DEVICE_CONTROL is a bitmask\n");
/* Clear current bitmask */
- for (i = 0; i < sizeof(pidff_device_control); i++) {
+ for (i = 0; i < ARRAY_SIZE(pidff_device_control); i++) {
index = pidff->control_id[i];
if (index < 1)
continue;
@@ -999,7 +999,7 @@ static int pidff_check_usage(int usage)
{
int i;
- for (i = 0; i < sizeof(pidff_reports); i++)
+ for (i = 0; i < ARRAY_SIZE(pidff_reports); i++)
if (usage == (HID_UP_PID | pidff_reports[i]))
return i;
@@ -1117,7 +1117,7 @@ static int pidff_find_special_keys(int *keys, struct hid_field *fld,
#define PIDFF_FIND_SPECIAL_KEYS(keys, field, name) \
pidff_find_special_keys(pidff->keys, pidff->field, pidff_ ## name, \
- sizeof(pidff_ ## name))
+ ARRAY_SIZE(pidff_ ## name))
/*
* Find and check the special fields
@@ -1184,7 +1184,7 @@ static int pidff_find_special_fields(struct pidff_device *pidff)
if (PIDFF_FIND_SPECIAL_KEYS(status_id, block_load_status,
block_load_status) !=
- sizeof(pidff_block_load_status)) {
+ ARRAY_SIZE(pidff_block_load_status)) {
hid_err(pidff->hid,
"block load status identifiers not found\n");
return -1;
@@ -1192,7 +1192,7 @@ static int pidff_find_special_fields(struct pidff_device *pidff)
if (PIDFF_FIND_SPECIAL_KEYS(operation_id, effect_operation_status,
effect_operation_status) !=
- sizeof(pidff_effect_operation_status)) {
+ ARRAY_SIZE(pidff_effect_operation_status)) {
hid_err(pidff->hid, "effect operation identifiers not found\n");
return -1;
}
@@ -1208,7 +1208,7 @@ static int pidff_find_effects(struct pidff_device *pidff,
{
int i;
- for (i = 0; i < sizeof(pidff_effect_types); i++) {
+ for (i = 0; i < ARRAY_SIZE(pidff_effect_types); i++) {
int pidff_type = pidff->type_id[i];
if (pidff->set_effect_type->usage[pidff_type].hid !=
@@ -1258,7 +1258,7 @@ static int pidff_find_effects(struct pidff_device *pidff,
#define PIDFF_FIND_FIELDS(name, report, strict) \
pidff_find_fields(pidff->name, pidff_ ## name, \
pidff->reports[report], \
- sizeof(pidff_ ## name), strict)
+ ARRAY_SIZE(pidff_ ## name), strict)
/*
* Fill and check the pidff_usages
--
2.50.1
^ permalink raw reply related
* [PATCH 05/17] HID: pidff: Treat PID_REQUIRED_REPORTS as count, not max
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
It's naming suggests it's a count of the records required by the USB PID
standard and this driver.
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
Reviewed-by: Oleg Makarenko <oleg@makarenk.ooo>
---
drivers/hid/usbhid/hid-pidff.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 2f9fbe4c52d7..cff79e76c211 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -33,7 +33,7 @@
#define PID_DEVICE_CONTROL 6
#define PID_CREATE_NEW_EFFECT 7
-#define PID_REQUIRED_REPORTS 7
+#define PID_REQUIRED_REPORTS 8
#define PID_SET_ENVELOPE 8
#define PID_SET_CONDITION 9
@@ -1056,7 +1056,7 @@ static int pidff_reports_ok(struct pidff_device *pidff)
{
int i;
- for (i = 0; i <= PID_REQUIRED_REPORTS; i++) {
+ for (i = 0; i < PID_REQUIRED_REPORTS; i++) {
if (!pidff->reports[i]) {
hid_dbg(pidff->hid, "%d missing\n", i);
return 0;
--
2.50.1
^ permalink raw reply related
* [PATCH 06/17] HID: pidff: Better quirk assigment when searching for fields
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Assign quirks directly when they're discovered. Way easier to understand
without relying on return values.
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 59 +++++++++++-----------------------
1 file changed, 18 insertions(+), 41 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index cff79e76c211..c88442a087f1 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -943,7 +943,8 @@ static void pidff_set_autocenter(struct input_dev *dev, u16 magnitude)
* Find fields from a report and fill a pidff_usage
*/
static int pidff_find_fields(struct pidff_usage *usage, const u8 *table,
- struct hid_report *report, int count, int strict)
+ struct hid_report *report, int count, int strict,
+ u32 *quirks)
{
if (!report) {
pr_debug("%s, null report\n", __func__);
@@ -951,7 +952,6 @@ static int pidff_find_fields(struct pidff_usage *usage, const u8 *table,
}
int i, j, k, found;
- int return_value = 0;
for (k = 0; k < count; k++) {
found = 0;
@@ -979,17 +979,17 @@ static int pidff_find_fields(struct pidff_usage *usage, const u8 *table,
if (!found && table[k] == pidff_set_effect[PID_START_DELAY]) {
pr_debug("Delay field not found, but that's OK\n");
pr_debug("Setting MISSING_DELAY quirk\n");
- return_value |= HID_PIDFF_QUIRK_MISSING_DELAY;
+ *quirks |= HID_PIDFF_QUIRK_MISSING_DELAY;
} else if (!found && table[k] == pidff_set_condition[PID_PARAM_BLOCK_OFFSET]) {
pr_debug("PBO field not found, but that's OK\n");
pr_debug("Setting MISSING_PBO quirk\n");
- return_value |= HID_PIDFF_QUIRK_MISSING_PBO;
+ *quirks |= HID_PIDFF_QUIRK_MISSING_PBO;
} else if (!found && strict) {
pr_debug("failed to locate %d\n", k);
return -1;
}
}
- return return_value;
+ return 0;
}
/*
@@ -1258,26 +1258,17 @@ static int pidff_find_effects(struct pidff_device *pidff,
#define PIDFF_FIND_FIELDS(name, report, strict) \
pidff_find_fields(pidff->name, pidff_ ## name, \
pidff->reports[report], \
- ARRAY_SIZE(pidff_ ## name), strict)
+ ARRAY_SIZE(pidff_ ## name), strict, &pidff->quirks)
/*
* Fill and check the pidff_usages
*/
static int pidff_init_fields(struct pidff_device *pidff, struct input_dev *dev)
{
- int status = 0;
-
- /* Save info about the device not having the DELAY ffb field. */
- status = PIDFF_FIND_FIELDS(set_effect, PID_SET_EFFECT, 1);
- if (status == -1) {
+ if (PIDFF_FIND_FIELDS(set_effect, PID_SET_EFFECT, 1)) {
hid_err(pidff->hid, "unknown set_effect report layout\n");
return -ENODEV;
}
- pidff->quirks |= status;
-
- if (status & HID_PIDFF_QUIRK_MISSING_DELAY)
- hid_dbg(pidff->hid, "Adding MISSING_DELAY quirk\n");
-
PIDFF_FIND_FIELDS(block_load, PID_BLOCK_LOAD, 0);
if (!pidff->block_load[PID_EFFECT_BLOCK_INDEX].value) {
@@ -1311,39 +1302,25 @@ static int pidff_init_fields(struct pidff_device *pidff, struct input_dev *dev)
"has periodic effect but no envelope\n");
}
- if (test_bit(FF_CONSTANT, dev->ffbit) &&
- PIDFF_FIND_FIELDS(set_constant, PID_SET_CONSTANT, 1)) {
+ if (PIDFF_FIND_FIELDS(set_constant, PID_SET_CONSTANT, 1) &&
+ test_and_clear_bit(FF_CONSTANT, dev->ffbit))
hid_warn(pidff->hid, "unknown constant effect layout\n");
- clear_bit(FF_CONSTANT, dev->ffbit);
- }
- if (test_bit(FF_RAMP, dev->ffbit) &&
- PIDFF_FIND_FIELDS(set_ramp, PID_SET_RAMP, 1)) {
+ if (PIDFF_FIND_FIELDS(set_ramp, PID_SET_RAMP, 1) &&
+ test_and_clear_bit(FF_RAMP, dev->ffbit))
hid_warn(pidff->hid, "unknown ramp effect layout\n");
- clear_bit(FF_RAMP, dev->ffbit);
- }
-
- if (test_bit(FF_SPRING, dev->ffbit) ||
- test_bit(FF_DAMPER, dev->ffbit) ||
- test_bit(FF_FRICTION, dev->ffbit) ||
- test_bit(FF_INERTIA, dev->ffbit)) {
- status = PIDFF_FIND_FIELDS(set_condition, PID_SET_CONDITION, 1);
- if (status < 0) {
+ if (PIDFF_FIND_FIELDS(set_condition, PID_SET_CONDITION, 1)) {
+ if (test_and_clear_bit(FF_SPRING, dev->ffbit) ||
+ test_and_clear_bit(FF_DAMPER, dev->ffbit) ||
+ test_and_clear_bit(FF_FRICTION, dev->ffbit) ||
+ test_and_clear_bit(FF_INERTIA, dev->ffbit))
hid_warn(pidff->hid, "unknown condition effect layout\n");
- clear_bit(FF_SPRING, dev->ffbit);
- clear_bit(FF_DAMPER, dev->ffbit);
- clear_bit(FF_FRICTION, dev->ffbit);
- clear_bit(FF_INERTIA, dev->ffbit);
- }
- pidff->quirks |= status;
}
- if (test_bit(FF_PERIODIC, dev->ffbit) &&
- PIDFF_FIND_FIELDS(set_periodic, PID_SET_PERIODIC, 1)) {
+ if (PIDFF_FIND_FIELDS(set_periodic, PID_SET_PERIODIC, 1) &&
+ test_and_clear_bit(FF_PERIODIC, dev->ffbit))
hid_warn(pidff->hid, "unknown periodic effect layout\n");
- clear_bit(FF_PERIODIC, dev->ffbit);
- }
PIDFF_FIND_FIELDS(pool, PID_POOL, 0);
--
2.50.1
^ permalink raw reply related
* [PATCH 07/17] HID: pidff: Simplify HID field/usage searching logic
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Some deduplication and splitting into separate functions. This is now
way easier to comprehend and parse mentally.
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 105 +++++++++++++++++++--------------
1 file changed, 62 insertions(+), 43 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index c88442a087f1..2e8eac944be0 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -939,6 +939,43 @@ static void pidff_set_autocenter(struct input_dev *dev, u16 magnitude)
pidff_autocenter(dev->ff->private, magnitude);
}
+/*
+ * Find specific usage in a given hid_field
+ */
+static int pidff_find_usage(struct hid_field *fld, unsigned int usage_code)
+{
+ for (int i = 0; i < fld->maxusage; i++) {
+ if (fld->usage[i].hid == usage_code)
+ return i;
+ }
+ return -1;
+}
+
+/*
+ * Find hid_field with a specific usage. Return the usage index as well
+ */
+static int pidff_find_field_with_usage(int *usage_index,
+ struct hid_report *report,
+ unsigned int usage_code)
+{
+ for (int i = 0; i < report->maxfield; i++) {
+ struct hid_field *fld = report->field[i];
+
+ if (fld->maxusage != fld->report_count) {
+ pr_debug("maxusage and report_count do not match, skipping\n");
+ continue;
+ }
+
+ int index = pidff_find_usage(fld, usage_code);
+
+ if (index >= 0) {
+ *usage_index = index;
+ return i;
+ }
+ }
+ return -1;
+}
+
/*
* Find fields from a report and fill a pidff_usage
*/
@@ -946,46 +983,38 @@ static int pidff_find_fields(struct pidff_usage *usage, const u8 *table,
struct hid_report *report, int count, int strict,
u32 *quirks)
{
+ const u8 block_offset = pidff_set_condition[PID_PARAM_BLOCK_OFFSET];
+ const u8 delay = pidff_set_effect[PID_START_DELAY];
+
if (!report) {
pr_debug("%s, null report\n", __func__);
return -1;
}
- int i, j, k, found;
+ for (int i = 0; i < count; i++) {
+ int index;
+ int found = pidff_find_field_with_usage(&index, report,
+ HID_UP_PID | table[i]);
- for (k = 0; k < count; k++) {
- found = 0;
- for (i = 0; i < report->maxfield; i++) {
- if (report->field[i]->maxusage !=
- report->field[i]->report_count) {
- pr_debug("maxusage and report_count do not match, skipping\n");
- continue;
- }
- for (j = 0; j < report->field[i]->maxusage; j++) {
- if (report->field[i]->usage[j].hid ==
- (HID_UP_PID | table[k])) {
- pr_debug("found %d at %d->%d\n",
- k, i, j);
- usage[k].field = report->field[i];
- usage[k].value =
- &report->field[i]->value[j];
- found = 1;
- break;
- }
- }
- if (found)
- break;
+ if (found >= 0) {
+ pr_debug("found %d at %d->%d\n", i, found, index);
+ usage[i].field = report->field[found];
+ usage[i].value = &report->field[found]->value[index];
+ continue;
}
- if (!found && table[k] == pidff_set_effect[PID_START_DELAY]) {
+
+ if (table[i] == delay) {
pr_debug("Delay field not found, but that's OK\n");
pr_debug("Setting MISSING_DELAY quirk\n");
*quirks |= HID_PIDFF_QUIRK_MISSING_DELAY;
- } else if (!found && table[k] == pidff_set_condition[PID_PARAM_BLOCK_OFFSET]) {
+
+ } else if (table[i] == block_offset) {
pr_debug("PBO field not found, but that's OK\n");
pr_debug("Setting MISSING_PBO quirk\n");
*quirks |= HID_PIDFF_QUIRK_MISSING_PBO;
- } else if (!found && strict) {
- pr_debug("failed to locate %d\n", k);
+
+ } else if (strict) {
+ pr_debug("failed to locate %d\n", i);
return -1;
}
}
@@ -1054,9 +1083,7 @@ static void pidff_find_reports(struct hid_device *hid, int report_type,
*/
static int pidff_reports_ok(struct pidff_device *pidff)
{
- int i;
-
- for (i = 0; i < PID_REQUIRED_REPORTS; i++) {
+ for (int i = 0; i < PID_REQUIRED_REPORTS; i++) {
if (!pidff->reports[i]) {
hid_dbg(pidff->hid, "%d missing\n", i);
return 0;
@@ -1077,9 +1104,7 @@ static struct hid_field *pidff_find_special_field(struct hid_report *report,
return NULL;
}
- int i;
-
- for (i = 0; i < report->maxfield; i++) {
+ for (int i = 0; i < report->maxfield; i++) {
if (report->field[i]->logical == (HID_UP_PID | usage) &&
report->field[i]->report_count > 0) {
if (!enforce_min ||
@@ -1099,18 +1124,12 @@ static struct hid_field *pidff_find_special_field(struct hid_report *report,
static int pidff_find_special_keys(int *keys, struct hid_field *fld,
const u8 *usagetable, int count)
{
-
- int i, j;
int found = 0;
- for (i = 0; i < count; i++) {
- for (j = 0; j < fld->maxusage; j++) {
- if (fld->usage[j].hid == (HID_UP_PID | usagetable[i])) {
- keys[i] = j + 1;
- found++;
- break;
- }
- }
+ for (int i = 0; i < count; i++) {
+ keys[i] = pidff_find_usage(fld, HID_UP_PID | usagetable[i]) + 1;
+ if (keys[i])
+ found++;
}
return found;
}
--
2.50.1
^ permalink raw reply related
* [PATCH 08/17] HID: pidff: Add support for AXES_ENABLE field
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
AXES_ENABLE can be used in place of DIRECTION_ENABLE to indicate, which
FFB-enabled axes will be affected by a given effect. EFFECT_DIRECTION
enables all and uses the first direction only while AXES_ENABLE is a
bitmask and bit indexes are the same as the defined GD usages in the
EFFECT_DIRECTION array. Each axis can have it's own direction in this
case.
Search for AXES_ENABLE, set AXES_ENABLE for all axes if DIRECTION_ENABLE
is not used.
Search for specific axes in the direction array. Save their indexes. This
let us know what axes are actually available on the device and which bit
in the AXES_ENABLE field corresponds to which axis.
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 91 ++++++++++++++++++++++++++++++++--
1 file changed, 87 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 2e8eac944be0..0f49d2836e9e 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -51,6 +51,7 @@ static const u8 pidff_reports[] = {
/* PID special fields */
#define PID_EFFECT_TYPE 0x25
+#define PID_AXES_ENABLE 0x55
#define PID_DIRECTION 0x57
#define PID_EFFECT_OPERATION_ARRAY 0x78
#define PID_BLOCK_LOAD_STATUS 0x8b
@@ -150,6 +151,31 @@ static const u8 pidff_effect_operation_status[] = { 0x79, 0x7b };
/* Polar direction 90 degrees (East) */
#define PIDFF_FIXED_WHEEL_DIRECTION 0x4000
+/* AXES_ENABLE and DIRECTION axes */
+enum pid_axes {
+ PID_AXIS_X,
+ PID_AXIS_Y,
+ PID_AXIS_Z,
+ PID_AXIS_RX,
+ PID_AXIS_RY,
+ PID_AXIS_RZ,
+ PID_AXIS_SLIDER,
+ PID_AXIS_DIAL,
+ PID_AXIS_WHEEL,
+ PID_AXES_COUNT,
+};
+static const u8 pidff_direction_axis[] = {
+ HID_USAGE & HID_GD_X,
+ HID_USAGE & HID_GD_Y,
+ HID_USAGE & HID_GD_Z,
+ HID_USAGE & HID_GD_RX,
+ HID_USAGE & HID_GD_RY,
+ HID_USAGE & HID_GD_RZ,
+ HID_USAGE & HID_GD_SLIDER,
+ HID_USAGE & HID_GD_DIAL,
+ HID_USAGE & HID_GD_WHEEL,
+};
+
struct pidff_usage {
struct hid_field *field;
s32 *value;
@@ -184,6 +210,7 @@ struct pidff_device {
/* Special fields in set_effect */
struct hid_field *set_effect_type;
struct hid_field *effect_direction;
+ struct hid_field *axes_enable;
/* Special field in device_control */
struct hid_field *device_control;
@@ -198,11 +225,13 @@ struct pidff_device {
int type_id[ARRAY_SIZE(pidff_effect_types)];
int status_id[ARRAY_SIZE(pidff_block_load_status)];
int operation_id[ARRAY_SIZE(pidff_effect_operation_status)];
+ int direction_axis_id[ARRAY_SIZE(pidff_direction_axis)];
int pid_id[PID_EFFECTS_MAX];
u32 quirks;
u8 effect_count;
+ u8 axis_count;
};
static int pidff_is_effect_conditional(struct ff_effect *effect)
@@ -306,14 +335,37 @@ static void pidff_set_effect_direction(struct pidff_device *pidff,
struct ff_effect *effect)
{
u16 direction = effect->direction;
+ int direction_enable = 1;
/* Use fixed direction if needed */
if (pidff->quirks & HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION &&
pidff_is_effect_conditional(effect))
direction = PIDFF_FIXED_WHEEL_DIRECTION;
+ pidff->set_effect[PID_DIRECTION_ENABLE].value[0] = direction_enable;
pidff->effect_direction->value[0] =
pidff_rescale(direction, U16_MAX, pidff->effect_direction);
+
+ if (direction_enable)
+ return;
+
+ /*
+ * For use with improved FFB API
+ * We want to read the selected axes and their direction from the effect
+ * struct and only enable those. For now, enable all axes.
+ *
+ */
+ for (int i = 0; i < PID_AXES_COUNT; i++) {
+ /* HID index starts with 1 */
+ int index = pidff->direction_axis_id[i] - 1;
+
+ if (index < 0)
+ continue;
+
+ pidff->axes_enable->value[index] = 1;
+ pidff->effect_direction->value[index] = pidff_rescale(
+ direction, U16_MAX, pidff->effect_direction);
+ }
}
/*
@@ -411,7 +463,6 @@ static void pidff_set_effect_report(struct pidff_device *pidff,
effect->trigger.interval);
pidff->set_effect[PID_GAIN].value[0] =
pidff->set_effect[PID_GAIN].field->logical_maximum;
- pidff->set_effect[PID_DIRECTION_ENABLE].value[0] = 1;
pidff_set_effect_direction(pidff, effect);
@@ -1122,12 +1173,13 @@ static struct hid_field *pidff_find_special_field(struct hid_report *report,
* Fill a pidff->*_id struct table
*/
static int pidff_find_special_keys(int *keys, struct hid_field *fld,
- const u8 *usagetable, int count)
+ const u8 *usagetable, int count,
+ unsigned int usage_page)
{
int found = 0;
for (int i = 0; i < count; i++) {
- keys[i] = pidff_find_usage(fld, HID_UP_PID | usagetable[i]) + 1;
+ keys[i] = pidff_find_usage(fld, usage_page | usagetable[i]) + 1;
if (keys[i])
found++;
}
@@ -1136,7 +1188,11 @@ static int pidff_find_special_keys(int *keys, struct hid_field *fld,
#define PIDFF_FIND_SPECIAL_KEYS(keys, field, name) \
pidff_find_special_keys(pidff->keys, pidff->field, pidff_ ## name, \
- ARRAY_SIZE(pidff_ ## name))
+ ARRAY_SIZE(pidff_ ## name), HID_UP_PID)
+
+#define PIDFF_FIND_GENERAL_DESKTOP(keys, field, name) \
+ pidff_find_special_keys(pidff->keys, pidff->field, pidff_ ## name, \
+ ARRAY_SIZE(pidff_ ## name), HID_UP_GENDESK)
/*
* Find and check the special fields
@@ -1151,6 +1207,9 @@ static int pidff_find_special_fields(struct pidff_device *pidff)
pidff->set_effect_type =
pidff_find_special_field(pidff->reports[PID_SET_EFFECT],
PID_EFFECT_TYPE, 1);
+ pidff->axes_enable =
+ pidff_find_special_field(pidff->reports[PID_SET_EFFECT],
+ PID_AXES_ENABLE, 0);
pidff->effect_direction =
pidff_find_special_field(pidff->reports[PID_SET_EFFECT],
PID_DIRECTION, 0);
@@ -1216,6 +1275,30 @@ static int pidff_find_special_fields(struct pidff_device *pidff)
return -1;
}
+ if (!pidff->axes_enable)
+ hid_info(pidff->hid, "axes enable field not found!\n");
+ else
+ hid_dbg(pidff->hid, "axes enable report count: %u\n",
+ pidff->axes_enable->report_count);
+
+ uint found = PIDFF_FIND_GENERAL_DESKTOP(direction_axis_id, axes_enable,
+ direction_axis);
+
+ pidff->axis_count = found;
+ hid_dbg(pidff->hid, "found direction axes: %u", found);
+
+ for (int i = 0; i < sizeof(pidff_direction_axis); i++) {
+ if (!pidff->direction_axis_id[i])
+ continue;
+
+ hid_dbg(pidff->hid, "axis %d, usage: 0x%04x, index: %d", i + 1,
+ pidff_direction_axis[i], pidff->direction_axis_id[i]);
+ }
+
+ if (pidff->axes_enable && found != pidff->axes_enable->report_count)
+ hid_warn(pidff->hid, "axes_enable: %u != direction axes: %u",
+ pidff->axes_enable->report_count, found);
+
return 0;
}
--
2.50.1
^ permalink raw reply related
* [PATCH 09/17] HID: pidff: Update debug messages
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 0f49d2836e9e..689419b20bf0 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -623,8 +623,7 @@ static void pidff_set_gain_report(struct pidff_device *pidff, u16 gain)
*/
static void pidff_set_device_control(struct pidff_device *pidff, int field)
{
- int i, index;
- int field_index = pidff->control_id[field];
+ const int field_index = pidff->control_id[field];
if (field_index < 1)
return;
@@ -634,8 +633,9 @@ static void pidff_set_device_control(struct pidff_device *pidff, int field)
hid_dbg(pidff->hid, "DEVICE_CONTROL is a bitmask\n");
/* Clear current bitmask */
- for (i = 0; i < ARRAY_SIZE(pidff_device_control); i++) {
- index = pidff->control_id[i];
+ for (int i = 0; i < ARRAY_SIZE(pidff_device_control); i++) {
+ int index = pidff->control_id[i];
+
if (index < 1)
continue;
@@ -650,6 +650,8 @@ static void pidff_set_device_control(struct pidff_device *pidff, int field)
hid_hw_request(pidff->hid, pidff->reports[PID_DEVICE_CONTROL], HID_REQ_SET_REPORT);
hid_hw_wait(pidff->hid);
+ hid_dbg(pidff->hid, "Device control command 0x%02x sent",
+ pidff_device_control[field]);
}
/*
@@ -751,6 +753,9 @@ static void pidff_playback_pid(struct pidff_device *pidff, int pid_id, int n)
{
pidff->effect_operation[PID_EFFECT_BLOCK_INDEX].value[0] = pid_id;
+ hid_dbg(pidff->hid, "%s PID effect %d", n == 0 ? "stopping" : "playing",
+ pid_id);
+
if (n == 0) {
pidff->effect_operation_status->value[0] =
pidff->operation_id[PID_EFFECT_STOP];
@@ -772,6 +777,8 @@ static int pidff_playback(struct input_dev *dev, int effect_id, int value)
{
struct pidff_device *pidff = dev->ff->private;
+ hid_dbg(pidff->hid, "requesting %s on FF effect %d",
+ value == 0 ? "stop" : "playback", effect_id);
pidff_playback_pid(pidff, pidff->pid_id[effect_id], value);
return 0;
}
--
2.50.1
^ permalink raw reply related
* [PATCH 10/17] HID: pidff: Rework pidff_upload_effect
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
One of the more complicated functions. Expunge some of the logic to
separate functions (FF -> PID id conversion)
Add a macro for envelope check to make it more readable in the upload
function.
All this made it possible to to expunge common code from the big switch
statement and reduce the overall function size considerably. Now it can
fit on one screen.
Move the effect_cout logic from report functions to upload/erase
functions.
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 243 ++++++++++++++++-----------------
1 file changed, 115 insertions(+), 128 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 689419b20bf0..32d42792c95a 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -142,7 +142,7 @@ static const u8 pidff_effect_types[] = {
#define PID_BLOCK_LOAD_SUCCESS 0
#define PID_BLOCK_LOAD_FULL 1
#define PID_BLOCK_LOAD_ERROR 2
-static const u8 pidff_block_load_status[] = { 0x8c, 0x8d, 0x8e};
+static const u8 pidff_block_load_status[] = { 0x8c, 0x8d, 0x8e };
#define PID_EFFECT_START 0
#define PID_EFFECT_STOP 1
@@ -242,6 +242,62 @@ static int pidff_is_effect_conditional(struct ff_effect *effect)
effect->type == FF_FRICTION;
}
+/*
+ * Get PID effect index from FF effect type.
+ * Return 0 if invalid.
+ */
+static int pidff_effect_ff_to_pid(struct ff_effect *effect)
+{
+ switch (effect->type) {
+ case FF_CONSTANT:
+ return PID_CONSTANT;
+ case FF_RAMP:
+ return PID_RAMP;
+ case FF_SPRING:
+ return PID_SPRING;
+ case FF_DAMPER:
+ return PID_DAMPER;
+ case FF_INERTIA:
+ return PID_INERTIA;
+ case FF_FRICTION:
+ return PID_FRICTION;
+ case FF_PERIODIC:
+ switch (effect->u.periodic.waveform) {
+ case FF_SQUARE:
+ return PID_SQUARE;
+ case FF_TRIANGLE:
+ return PID_TRIANGLE;
+ case FF_SINE:
+ return PID_SINE;
+ case FF_SAW_UP:
+ return PID_SAW_UP;
+ case FF_SAW_DOWN:
+ return PID_SAW_DOWN;
+ }
+ }
+ pr_err("invalid effect type\n");
+ return -EINVAL;
+}
+
+/*
+ * Get effect id in the device descriptor.
+ * Return 0 if invalid.
+ */
+static int pidff_get_effect_type_id(struct pidff_device *pidff,
+ struct ff_effect *effect)
+{
+ int id = pidff_effect_ff_to_pid(effect);
+
+ if (id < 0)
+ return 0;
+
+ if (effect->type == FF_PERIODIC &&
+ pidff->quirks & HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY)
+ id = PID_SINE;
+
+ return pidff->type_id[id];
+}
+
/*
* Clamp value for a given field
*/
@@ -387,12 +443,12 @@ static void pidff_set_envelope_report(struct pidff_device *pidff,
pidff->set_envelope[PID_FADE_LEVEL].field);
pidff_set_time(&pidff->set_envelope[PID_ATTACK_TIME],
- envelope->attack_length);
+ envelope->attack_length);
pidff_set_time(&pidff->set_envelope[PID_FADE_TIME],
- envelope->fade_length);
+ envelope->fade_length);
hid_hw_request(pidff->hid, pidff->reports[PID_SET_ENVELOPE],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -401,7 +457,7 @@ static void pidff_set_envelope_report(struct pidff_device *pidff,
static int pidff_needs_set_envelope(struct ff_envelope *envelope,
struct ff_envelope *old)
{
- bool needs_new_envelope;
+ int needs_new_envelope;
needs_new_envelope = envelope->attack_level != 0 ||
envelope->fade_level != 0 ||
@@ -409,8 +465,7 @@ static int pidff_needs_set_envelope(struct ff_envelope *envelope,
envelope->fade_length != 0;
if (!needs_new_envelope)
- return false;
-
+ return 0;
if (!old)
return needs_new_envelope;
@@ -423,8 +478,8 @@ static int pidff_needs_set_envelope(struct ff_envelope *envelope,
/*
* Send constant force report to the device
*/
-static void pidff_set_constant_force_report(struct pidff_device *pidff,
- struct ff_effect *effect)
+static void pidff_set_constant_report(struct pidff_device *pidff,
+ struct ff_effect *effect)
{
pidff->set_constant[PID_EFFECT_BLOCK_INDEX].value[0] =
pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
@@ -432,7 +487,7 @@ static void pidff_set_constant_force_report(struct pidff_device *pidff,
effect->u.constant.level);
hid_hw_request(pidff->hid, pidff->reports[PID_SET_CONSTANT],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -583,8 +638,8 @@ static int pidff_needs_set_condition(struct ff_effect *effect,
/*
* Send ramp force report to the device
*/
-static void pidff_set_ramp_force_report(struct pidff_device *pidff,
- struct ff_effect *effect)
+static void pidff_set_ramp_report(struct pidff_device *pidff,
+ struct ff_effect *effect)
{
pidff->set_ramp[PID_EFFECT_BLOCK_INDEX].value[0] =
pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
@@ -593,7 +648,7 @@ static void pidff_set_ramp_force_report(struct pidff_device *pidff,
pidff_set_signed(&pidff->set_ramp[PID_RAMP_END],
effect->u.ramp.end_level);
hid_hw_request(pidff->hid, pidff->reports[PID_SET_RAMP],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -703,9 +758,6 @@ static int pidff_request_effect_upload(struct pidff_device *pidff, int efnum)
{
int j;
- if (!pidff->effect_count)
- pidff_reset(pidff);
-
pidff->create_new_effect_type->value[0] = efnum;
hid_hw_request(pidff->hid, pidff->reports[PID_CREATE_NEW_EFFECT],
HID_REQ_SET_REPORT);
@@ -725,8 +777,6 @@ static int pidff_request_effect_upload(struct pidff_device *pidff, int efnum)
hid_dbg(pidff->hid, "device reported free memory: %d bytes\n",
pidff->block_load[PID_RAM_POOL_AVAILABLE].value ?
pidff->block_load[PID_RAM_POOL_AVAILABLE].value[0] : -1);
-
- pidff->effect_count++;
return 0;
}
if (pidff->block_load_status->value[0] ==
@@ -767,7 +817,7 @@ static void pidff_playback_pid(struct pidff_device *pidff, int pid_id, int n)
}
hid_hw_request(pidff->hid, pidff->reports[PID_EFFECT_OPERATION],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -791,10 +841,7 @@ static void pidff_erase_pid(struct pidff_device *pidff, int pid_id)
{
pidff->block_free[PID_EFFECT_BLOCK_INDEX].value[0] = pid_id;
hid_hw_request(pidff->hid, pidff->reports[PID_BLOCK_FREE],
- HID_REQ_SET_REPORT);
-
- if (pidff->effect_count > 0)
- pidff->effect_count--;
+ HID_REQ_SET_REPORT);
}
/*
@@ -816,139 +863,79 @@ static int pidff_erase_effect(struct input_dev *dev, int effect_id)
pidff_playback_pid(pidff, pid_id, 0);
pidff_erase_pid(pidff, pid_id);
+ if (pidff->effect_count > 0)
+ pidff->effect_count--;
+
+ hid_dbg(pidff->hid, "current effect count: %d", pidff->effect_count);
return 0;
}
+#define PIDFF_SET_REPORT_IF_NEEDED(type, effect, old) \
+ ({ if (!old || pidff_needs_set_## type(effect, old)) \
+ pidff_set_ ##type## _report(pidff, effect); })
+
+#define PIDFF_SET_ENVELOPE_IF_NEEDED(type, effect, old) \
+ ({ if (pidff_needs_set_envelope(&effect->u.type.envelope, \
+ old ? &old->u.type.envelope : NULL)) \
+ pidff_set_envelope_report(pidff, &effect->u.type.envelope); })
+
/*
* Effect upload handler
*/
-static int pidff_upload_effect(struct input_dev *dev, struct ff_effect *effect,
+static int pidff_upload_effect(struct input_dev *dev, struct ff_effect *new,
struct ff_effect *old)
{
struct pidff_device *pidff = dev->ff->private;
- int type_id;
- int error;
+ const int type_id = pidff_get_effect_type_id(pidff, new);
- pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] = 0;
- if (old) {
- pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] =
- pidff->pid_id[effect->id];
+ if (!type_id) {
+ hid_err(pidff->hid, "effect type not supported\n");
+ return -EINVAL;
}
- switch (effect->type) {
+ if (!pidff->effect_count)
+ pidff_reset(pidff);
+
+ if (!old) {
+ int error = pidff_request_effect_upload(pidff, type_id);
+
+ if (error)
+ return error;
+
+ pidff->effect_count++;
+ hid_dbg(pidff->hid, "current effect count: %d", pidff->effect_count);
+ pidff->pid_id[new->id] =
+ pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
+ }
+
+ pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] =
+ pidff->pid_id[new->id];
+
+ PIDFF_SET_REPORT_IF_NEEDED(effect, new, old);
+ switch (new->type) {
case FF_CONSTANT:
- if (!old) {
- error = pidff_request_effect_upload(pidff,
- pidff->type_id[PID_CONSTANT]);
- if (error)
- return error;
- }
- if (!old || pidff_needs_set_effect(effect, old))
- pidff_set_effect_report(pidff, effect);
- if (!old || pidff_needs_set_constant(effect, old))
- pidff_set_constant_force_report(pidff, effect);
- if (pidff_needs_set_envelope(&effect->u.constant.envelope,
- old ? &old->u.constant.envelope : NULL))
- pidff_set_envelope_report(pidff, &effect->u.constant.envelope);
+ PIDFF_SET_REPORT_IF_NEEDED(constant, new, old);
+ PIDFF_SET_ENVELOPE_IF_NEEDED(constant, new, old);
break;
case FF_PERIODIC:
- if (!old) {
- switch (effect->u.periodic.waveform) {
- case FF_SQUARE:
- type_id = PID_SQUARE;
- break;
- case FF_TRIANGLE:
- type_id = PID_TRIANGLE;
- break;
- case FF_SINE:
- type_id = PID_SINE;
- break;
- case FF_SAW_UP:
- type_id = PID_SAW_UP;
- break;
- case FF_SAW_DOWN:
- type_id = PID_SAW_DOWN;
- break;
- default:
- hid_err(pidff->hid, "invalid waveform\n");
- return -EINVAL;
- }
-
- if (pidff->quirks & HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY)
- type_id = PID_SINE;
-
- error = pidff_request_effect_upload(pidff,
- pidff->type_id[type_id]);
- if (error)
- return error;
- }
- if (!old || pidff_needs_set_effect(effect, old))
- pidff_set_effect_report(pidff, effect);
- if (!old || pidff_needs_set_periodic(effect, old))
- pidff_set_periodic_report(pidff, effect);
- if (pidff_needs_set_envelope(&effect->u.periodic.envelope,
- old ? &old->u.periodic.envelope : NULL))
- pidff_set_envelope_report(pidff, &effect->u.periodic.envelope);
+ PIDFF_SET_REPORT_IF_NEEDED(periodic, new, old);
+ PIDFF_SET_ENVELOPE_IF_NEEDED(periodic, new, old);
break;
case FF_RAMP:
- if (!old) {
- error = pidff_request_effect_upload(pidff,
- pidff->type_id[PID_RAMP]);
- if (error)
- return error;
- }
- if (!old || pidff_needs_set_effect(effect, old))
- pidff_set_effect_report(pidff, effect);
- if (!old || pidff_needs_set_ramp(effect, old))
- pidff_set_ramp_force_report(pidff, effect);
- if (pidff_needs_set_envelope(&effect->u.ramp.envelope,
- old ? &old->u.ramp.envelope : NULL))
- pidff_set_envelope_report(pidff, &effect->u.ramp.envelope);
+ PIDFF_SET_REPORT_IF_NEEDED(ramp, new, old);
+ PIDFF_SET_ENVELOPE_IF_NEEDED(ramp, new, old);
break;
case FF_SPRING:
case FF_DAMPER:
case FF_INERTIA:
case FF_FRICTION:
- if (!old) {
- switch (effect->type) {
- case FF_SPRING:
- type_id = PID_SPRING;
- break;
- case FF_DAMPER:
- type_id = PID_DAMPER;
- break;
- case FF_INERTIA:
- type_id = PID_INERTIA;
- break;
- case FF_FRICTION:
- type_id = PID_FRICTION;
- break;
- }
- error = pidff_request_effect_upload(pidff,
- pidff->type_id[type_id]);
- if (error)
- return error;
- }
- if (!old || pidff_needs_set_effect(effect, old))
- pidff_set_effect_report(pidff, effect);
- if (!old || pidff_needs_set_condition(effect, old))
- pidff_set_condition_report(pidff, effect);
+ PIDFF_SET_REPORT_IF_NEEDED(condition, new, old);
break;
-
- default:
- hid_err(pidff->hid, "invalid type\n");
- return -EINVAL;
}
-
- if (!old)
- pidff->pid_id[effect->id] =
- pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
-
hid_dbg(pidff->hid, "uploaded\n");
-
return 0;
}
--
2.50.1
^ permalink raw reply related
* [PATCH 11/17] HID: pidff: Separate check for infinite duration
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
It will be used in a few more places so this makes sure it will always
work the same.
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 32d42792c95a..534fb28f6e55 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -242,6 +242,11 @@ static int pidff_is_effect_conditional(struct ff_effect *effect)
effect->type == FF_FRICTION;
}
+static int pidff_is_duration_infinite(u16 duration)
+{
+ return duration == FF_INFINITE || duration == PID_INFINITE;
+}
+
/*
* Get PID effect index from FF effect type.
* Return 0 if invalid.
@@ -374,12 +379,8 @@ static void pidff_set_time(struct pidff_usage *usage, u16 time)
static void pidff_set_duration(struct pidff_usage *usage, u16 duration)
{
- /* Infinite value conversion from Linux API -> PID */
- if (duration == FF_INFINITE)
- duration = PID_INFINITE;
-
/* PID defines INFINITE as the max possible value for duration field */
- if (duration == PID_INFINITE) {
+ if (pidff_is_duration_infinite(duration)) {
usage->value[0] = (1U << usage->field->report_size) - 1;
return;
}
--
2.50.1
^ permalink raw reply related
* [PATCH 12/17] HID: pidff: PERMISSIVE_CONTROL quirk autodetection
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Fixes force feedback for devices built with MMOS firmware and many more
not yet detected devices.
Update quirks mask debug message to always contain all 32 bits of data.
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 534fb28f6e55..3fd51ad5cf56 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -1210,8 +1210,16 @@ static int pidff_find_special_fields(struct pidff_device *pidff)
PID_DIRECTION, 0);
pidff->device_control =
pidff_find_special_field(pidff->reports[PID_DEVICE_CONTROL],
- PID_DEVICE_CONTROL_ARRAY,
- !(pidff->quirks & HID_PIDFF_QUIRK_PERMISSIVE_CONTROL));
+ PID_DEVICE_CONTROL_ARRAY, 1);
+
+ /* Detect and set permissive control quirk */
+ if (!pidff->device_control) {
+ pr_debug("Setting PERMISSIVE_CONTROL quirk\n");
+ pidff->quirks |= HID_PIDFF_QUIRK_PERMISSIVE_CONTROL;
+ pidff->device_control = pidff_find_special_field(
+ pidff->reports[PID_DEVICE_CONTROL],
+ PID_DEVICE_CONTROL_ARRAY, 0);
+ }
pidff->block_load_status =
pidff_find_special_field(pidff->reports[PID_BLOCK_LOAD],
@@ -1552,7 +1560,7 @@ int hid_pidff_init_with_quirks(struct hid_device *hid, u32 initial_quirks)
ff->playback = pidff_playback;
hid_info(dev, "Force feedback for USB HID PID devices by Anssi Hannula <anssi.hannula@gmail.com>\n");
- hid_dbg(dev, "Active quirks mask: 0x%x\n", pidff->quirks);
+ hid_dbg(dev, "Active quirks mask: 0x%08x\n", pidff->quirks);
hid_device_io_stop(hid);
--
2.50.1
^ permalink raw reply related
* [PATCH 13/17] HID: pidff: Remove Anssi's email address from info msg
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Some users might try to contact him about issues and he's no longer
active when it comes to the driver development/fixes.
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 3fd51ad5cf56..ebebac5c4384 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -1559,7 +1559,7 @@ int hid_pidff_init_with_quirks(struct hid_device *hid, u32 initial_quirks)
ff->set_autocenter = pidff_set_autocenter;
ff->playback = pidff_playback;
- hid_info(dev, "Force feedback for USB HID PID devices by Anssi Hannula <anssi.hannula@gmail.com>\n");
+ hid_info(dev, "Force feedback for USB HID PID devices by Anssi Hannula\n");
hid_dbg(dev, "Active quirks mask: 0x%08x\n", pidff->quirks);
hid_device_io_stop(hid);
--
2.50.1
^ permalink raw reply related
* [PATCH 14/17] HID: pidff: Define all cardinal directions
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Will be used by ff-effect based autocentering
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index ebebac5c4384..7f4c1186a44d 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -148,8 +148,12 @@ static const u8 pidff_block_load_status[] = { 0x8c, 0x8d, 0x8e };
#define PID_EFFECT_STOP 1
static const u8 pidff_effect_operation_status[] = { 0x79, 0x7b };
-/* Polar direction 90 degrees (East) */
-#define PIDFF_FIXED_WHEEL_DIRECTION 0x4000
+#define PID_DIRECTION_NORTH 0x0000
+#define PID_DIRECTION_EAST 0x4000
+#define PID_DIRECTION_SOUTH 0x8000
+#define PID_DIRECTION_WEST 0xc000
+
+#define PIDFF_FIXED_WHEEL_DIRECTION PID_DIRECTION_EAST
/* AXES_ENABLE and DIRECTION axes */
enum pid_axes {
--
2.50.1
^ permalink raw reply related
* [PATCH 15/17] HID: pidff: clang-format pass
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 55 ++++++++++++++++------------------
1 file changed, 26 insertions(+), 29 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 7f4c1186a44d..50a8924edfcc 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -9,12 +9,11 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include "hid-pidff.h"
+#include <linux/hid.h>
#include <linux/input.h>
+#include <linux/minmax.h>
#include <linux/slab.h>
#include <linux/usb.h>
-#include <linux/hid.h>
-#include <linux/minmax.h>
-
#define PID_EFFECTS_MAX 64
#define PID_INFINITE U16_MAX
@@ -321,7 +320,7 @@ static s32 pidff_clamp(s32 i, struct hid_field *field)
static int pidff_rescale(int i, int max, struct hid_field *field)
{
return i * (field->logical_maximum - field->logical_minimum) / max +
- field->logical_minimum;
+ field->logical_minimum;
}
/*
@@ -367,18 +366,18 @@ static void pidff_set_signed(struct pidff_usage *usage, s16 value)
else {
if (value < 0)
usage->value[0] =
- pidff_rescale(-value, -S16_MIN, usage->field);
+ pidff_rescale(-value, -S16_MIN, usage->field);
else
usage->value[0] =
- pidff_rescale(value, S16_MAX, usage->field);
+ pidff_rescale(value, S16_MAX, usage->field);
}
pr_debug("calculated from %d to %d\n", value, usage->value[0]);
}
static void pidff_set_time(struct pidff_usage *usage, u16 time)
{
- usage->value[0] = pidff_clamp(
- pidff_rescale_time(time, usage->field), usage->field);
+ usage->value[0] = pidff_clamp(pidff_rescale_time(time, usage->field),
+ usage->field);
}
static void pidff_set_duration(struct pidff_usage *usage, u16 duration)
@@ -516,11 +515,11 @@ static void pidff_set_effect_report(struct pidff_device *pidff,
pidff->create_new_effect_type->value[0];
pidff_set_duration(&pidff->set_effect[PID_DURATION],
- effect->replay.length);
+ effect->replay.length);
pidff->set_effect[PID_TRIGGER_BUTTON].value[0] = effect->trigger.button;
pidff_set_time(&pidff->set_effect[PID_TRIGGER_REPEAT_INT],
- effect->trigger.interval);
+ effect->trigger.interval);
pidff->set_effect[PID_GAIN].value[0] =
pidff->set_effect[PID_GAIN].field->logical_maximum;
@@ -529,10 +528,10 @@ static void pidff_set_effect_report(struct pidff_device *pidff,
/* Omit setting delay field if it's missing */
if (!(pidff->quirks & HID_PIDFF_QUIRK_MISSING_DELAY))
pidff_set_time(&pidff->set_effect[PID_START_DELAY],
- effect->replay.delay);
+ effect->replay.delay);
hid_hw_request(pidff->hid, pidff->reports[PID_SET_EFFECT],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -562,10 +561,10 @@ static void pidff_set_periodic_report(struct pidff_device *pidff,
effect->u.periodic.offset);
pidff_set(&pidff->set_periodic[PID_PHASE], effect->u.periodic.phase);
pidff_set_time(&pidff->set_periodic[PID_PERIOD],
- effect->u.periodic.period);
+ effect->u.periodic.period);
hid_hw_request(pidff->hid, pidff->reports[PID_SET_PERIODIC],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -612,7 +611,7 @@ static void pidff_set_condition_report(struct pidff_device *pidff,
pidff_set(&pidff->set_condition[PID_DEAD_BAND],
effect->u.condition[i].deadband);
hid_hw_request(pidff->hid, pidff->reports[PID_SET_CONDITION],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
}
@@ -675,7 +674,7 @@ static void pidff_set_gain_report(struct pidff_device *pidff, u16 gain)
pidff_set(&pidff->device_gain[PID_DEVICE_GAIN_FIELD], gain);
hid_hw_request(pidff->hid, pidff->reports[PID_DEVICE_GAIN],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -761,21 +760,19 @@ static void pidff_fetch_pool(struct pidff_device *pidff)
*/
static int pidff_request_effect_upload(struct pidff_device *pidff, int efnum)
{
- int j;
-
pidff->create_new_effect_type->value[0] = efnum;
hid_hw_request(pidff->hid, pidff->reports[PID_CREATE_NEW_EFFECT],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
hid_dbg(pidff->hid, "create_new_effect sent, type: %d\n", efnum);
pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] = 0;
pidff->block_load_status->value[0] = 0;
hid_hw_wait(pidff->hid);
- for (j = 0; j < 60; j++) {
+ for (int i = 0; i < 60; i++) {
hid_dbg(pidff->hid, "pid_block_load requested\n");
hid_hw_request(pidff->hid, pidff->reports[PID_BLOCK_LOAD],
- HID_REQ_GET_REPORT);
+ HID_REQ_GET_REPORT);
hid_hw_wait(pidff->hid);
if (pidff->block_load_status->value[0] ==
pidff->status_id[PID_BLOCK_LOAD_SUCCESS]) {
@@ -857,8 +854,8 @@ static int pidff_erase_effect(struct input_dev *dev, int effect_id)
struct pidff_device *pidff = dev->ff->private;
int pid_id = pidff->pid_id[effect_id];
- hid_dbg(pidff->hid, "starting to erase %d/%d\n",
- effect_id, pidff->pid_id[effect_id]);
+ hid_dbg(pidff->hid, "starting to erase %d/%d\n", effect_id,
+ pidff->pid_id[effect_id]);
/*
* Wait for the queue to clear. We do not want
@@ -978,7 +975,7 @@ static void pidff_autocenter(struct pidff_device *pidff, u16 magnitude)
pidff->set_effect[PID_START_DELAY].value[0] = 0;
hid_hw_request(pidff->hid, pidff->reports[PID_SET_EFFECT],
- HID_REQ_SET_REPORT);
+ HID_REQ_SET_REPORT);
}
/*
@@ -1269,7 +1266,7 @@ static int pidff_find_special_fields(struct pidff_device *pidff)
if (PIDFF_FIND_SPECIAL_KEYS(status_id, block_load_status,
block_load_status) !=
- ARRAY_SIZE(pidff_block_load_status)) {
+ ARRAY_SIZE(pidff_block_load_status)) {
hid_err(pidff->hid,
"block load status identifiers not found\n");
return -1;
@@ -1277,7 +1274,7 @@ static int pidff_find_special_fields(struct pidff_device *pidff)
if (PIDFF_FIND_SPECIAL_KEYS(operation_id, effect_operation_status,
effect_operation_status) !=
- ARRAY_SIZE(pidff_effect_operation_status)) {
+ ARRAY_SIZE(pidff_effect_operation_status)) {
hid_err(pidff->hid, "effect operation identifiers not found\n");
return -1;
}
@@ -1482,8 +1479,8 @@ static int pidff_check_autocenter(struct pidff_device *pidff,
int hid_pidff_init_with_quirks(struct hid_device *hid, u32 initial_quirks)
{
struct pidff_device *pidff;
- struct hid_input *hidinput = list_entry(hid->inputs.next,
- struct hid_input, list);
+ struct hid_input *hidinput =
+ list_entry(hid->inputs.next, struct hid_input, list);
struct input_dev *dev = hidinput->input;
struct ff_device *ff;
int max_effects;
@@ -1570,7 +1567,7 @@ int hid_pidff_init_with_quirks(struct hid_device *hid, u32 initial_quirks)
return 0;
- fail:
+fail:
hid_device_io_stop(hid);
kfree(pidff);
--
2.50.1
^ permalink raw reply related
* [PATCH 16/17] HID: universal-pidff: clang-format pass
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/hid-universal-pidff.c | 57 ++++++++++++++++---------------
1 file changed, 29 insertions(+), 28 deletions(-)
diff --git a/drivers/hid/hid-universal-pidff.c b/drivers/hid/hid-universal-pidff.c
index 70fce0f88e82..549dac555d40 100644
--- a/drivers/hid/hid-universal-pidff.c
+++ b/drivers/hid/hid-universal-pidff.c
@@ -8,12 +8,12 @@
* Copyright (c) 2024, 2025 Tomasz Pakuła
*/
+#include "hid-ids.h"
+#include "usbhid/hid-pidff.h"
#include <linux/device.h>
#include <linux/hid.h>
-#include <linux/module.h>
#include <linux/input-event-codes.h>
-#include "hid-ids.h"
-#include "usbhid/hid-pidff.h"
+#include <linux/module.h>
#define JOY_RANGE (BTN_DEAD - BTN_JOYSTICK + 1)
@@ -21,8 +21,10 @@
* Map buttons manually to extend the default joystick button limit
*/
static int universal_pidff_input_mapping(struct hid_device *hdev,
- struct hid_input *hi, struct hid_field *field, struct hid_usage *usage,
- unsigned long **bit, int *max)
+ struct hid_input *hi,
+ struct hid_field *field,
+ struct hid_usage *usage,
+ unsigned long **bit, int *max)
{
if ((usage->hid & HID_USAGE_PAGE) != HID_UP_BUTTON)
return 0;
@@ -126,65 +128,64 @@ static int universal_pidff_input_configured(struct hid_device *hdev,
if (!test_bit(axis, input->absbit))
continue;
- input_set_abs_params(input, axis,
- input->absinfo[axis].minimum,
- input->absinfo[axis].maximum,
- axis == ABS_X ? 0 : 8, 0);
+ input_set_abs_params(input, axis, input->absinfo[axis].minimum,
+ input->absinfo[axis].maximum,
+ axis == ABS_X ? 0 : 8, 0);
}
/* Remove fuzz and deadzone from the second joystick axis */
if (hdev->vendor == USB_VENDOR_ID_FFBEAST &&
hdev->product == USB_DEVICE_ID_FFBEAST_JOYSTICK)
input_set_abs_params(input, ABS_Y,
- input->absinfo[ABS_Y].minimum,
- input->absinfo[ABS_Y].maximum, 0, 0);
+ input->absinfo[ABS_Y].minimum,
+ input->absinfo[ABS_Y].maximum, 0, 0);
return 0;
}
static const struct hid_device_id universal_pidff_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R3),
- .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R3_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R5),
- .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R5_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R9),
- .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R9_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R12),
- .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R12_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R16_R21),
- .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R16_R21_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_CAMMUS, USB_DEVICE_ID_CAMMUS_C5) },
{ HID_USB_DEVICE(USB_VENDOR_ID_CAMMUS, USB_DEVICE_ID_CAMMUS_C12) },
{ HID_USB_DEVICE(USB_VENDOR_ID_VRS, USB_DEVICE_ID_VRS_DFP),
- .driver_data = HID_PIDFF_QUIRK_PERMISSIVE_CONTROL },
+ .driver_data = HID_PIDFF_QUIRK_PERMISSIVE_CONTROL },
{ HID_USB_DEVICE(USB_VENDOR_ID_FFBEAST, USB_DEVICE_ID_FFBEAST_JOYSTICK), },
{ HID_USB_DEVICE(USB_VENDOR_ID_FFBEAST, USB_DEVICE_ID_FFBEAST_RUDDER), },
{ HID_USB_DEVICE(USB_VENDOR_ID_FFBEAST, USB_DEVICE_ID_FFBEAST_WHEEL) },
{ HID_USB_DEVICE(USB_VENDOR_ID_LITE_STAR, USB_DEVICE_ID_PXN_V10),
- .driver_data = HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY },
+ .driver_data = HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY },
{ HID_USB_DEVICE(USB_VENDOR_ID_LITE_STAR, USB_DEVICE_ID_PXN_V12),
- .driver_data = HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY },
+ .driver_data = HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY },
{ HID_USB_DEVICE(USB_VENDOR_ID_LITE_STAR, USB_DEVICE_ID_PXN_V12_LITE),
- .driver_data = HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY },
+ .driver_data = HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY },
{ HID_USB_DEVICE(USB_VENDOR_ID_LITE_STAR, USB_DEVICE_ID_PXN_V12_LITE_2),
- .driver_data = HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY },
+ .driver_data = HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY },
{ HID_USB_DEVICE(USB_VENDOR_ID_LITE_STAR, USB_DEVICE_ID_LITE_STAR_GT987),
- .driver_data = HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY },
+ .driver_data = HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASETEK, USB_DEVICE_ID_ASETEK_INVICTA) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASETEK, USB_DEVICE_ID_ASETEK_FORTE) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASETEK, USB_DEVICE_ID_ASETEK_LA_PRIMA) },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASETEK, USB_DEVICE_ID_ASETEK_TONY_KANAAN) },
- { }
+ {}
};
MODULE_DEVICE_TABLE(hid, universal_pidff_devices);
--
2.50.1
^ permalink raw reply related
* [PATCH 17/17] HID: pidff: Reduce PID_EFFECT_OPERATION spam
From: Tomasz Pakuła @ 2025-08-03 18:13 UTC (permalink / raw)
To: jikos, bentiss; +Cc: oleg, linux-input
In-Reply-To: <20250803181354.60034-1-tomasz.pakula.oficjalny@gmail.com>
Keep track of effect's loop_count to reduce the spam of ffb play
commands coming from some games. This should speed up normal magnitude
etc updates and slightly increase max possible FFB refresh rate.
Helps games like Dirt Rally 2.0, F1 2023, WRC from KT
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
drivers/hid/usbhid/hid-pidff.c | 36 ++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 50a8924edfcc..0342c0a3f476 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -184,6 +184,12 @@ struct pidff_usage {
s32 *value;
};
+struct pidff_effect {
+ int pid_id;
+ int is_infinite;
+ unsigned int loop_count;
+};
+
struct pidff_device {
struct hid_device *hid;
@@ -202,6 +208,8 @@ struct pidff_device {
struct pidff_usage effect_operation[ARRAY_SIZE(pidff_effect_operation)];
struct pidff_usage block_free[ARRAY_SIZE(pidff_block_free)];
+ struct pidff_effect effect[PID_EFFECTS_MAX];
+
/*
* Special field is a field that is not composed of
* usage<->value pairs that pidff_usage values are
@@ -230,8 +238,6 @@ struct pidff_device {
int operation_id[ARRAY_SIZE(pidff_effect_operation_status)];
int direction_axis_id[ARRAY_SIZE(pidff_direction_axis)];
- int pid_id[PID_EFFECTS_MAX];
-
u32 quirks;
u8 effect_count;
u8 axis_count;
@@ -798,6 +804,12 @@ static int pidff_request_effect_upload(struct pidff_device *pidff, int efnum)
return -EIO;
}
+static int pidff_needs_playback(struct pidff_device *pidff, int effect_id, int n)
+{
+ return pidff->effect[effect_id].is_infinite ||
+ pidff->effect[effect_id].loop_count != n;
+}
+
/*
* Play the effect with PID id n times
*/
@@ -829,9 +841,14 @@ static int pidff_playback(struct input_dev *dev, int effect_id, int value)
{
struct pidff_device *pidff = dev->ff->private;
+ if (!pidff_needs_playback(pidff, effect_id, value))
+ return 0;
+
hid_dbg(pidff->hid, "requesting %s on FF effect %d",
value == 0 ? "stop" : "playback", effect_id);
- pidff_playback_pid(pidff, pidff->pid_id[effect_id], value);
+
+ pidff->effect[effect_id].loop_count = value;
+ pidff_playback_pid(pidff, pidff->effect[effect_id].pid_id, value);
return 0;
}
@@ -852,10 +869,9 @@ static void pidff_erase_pid(struct pidff_device *pidff, int pid_id)
static int pidff_erase_effect(struct input_dev *dev, int effect_id)
{
struct pidff_device *pidff = dev->ff->private;
- int pid_id = pidff->pid_id[effect_id];
+ int pid_id = pidff->effect[effect_id].pid_id;
- hid_dbg(pidff->hid, "starting to erase %d/%d\n", effect_id,
- pidff->pid_id[effect_id]);
+ hid_dbg(pidff->hid, "starting to erase %d/%d\n", effect_id, pid_id);
/*
* Wait for the queue to clear. We do not want
@@ -906,12 +922,16 @@ static int pidff_upload_effect(struct input_dev *dev, struct ff_effect *new,
pidff->effect_count++;
hid_dbg(pidff->hid, "current effect count: %d", pidff->effect_count);
- pidff->pid_id[new->id] =
+ pidff->effect[new->id].loop_count = 0;
+ pidff->effect[new->id].pid_id =
pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0];
}
+ pidff->effect[new->id].is_infinite =
+ pidff_is_duration_infinite(new->replay.length);
+
pidff->block_load[PID_EFFECT_BLOCK_INDEX].value[0] =
- pidff->pid_id[new->id];
+ pidff->effect[new->id].pid_id;
PIDFF_SET_REPORT_IF_NEEDED(effect, new, old);
switch (new->type) {
--
2.50.1
^ permalink raw reply related
* Re: [PATCH 05/11] i2c: apple: Drop default ARCH_APPLE in Kconfig
From: Wolfram Sang @ 2025-08-03 20:40 UTC (permalink / raw)
To: Sven Peter
Cc: Janne Grunau, Alyssa Rosenzweig, Neal Gompa, Ulf Hansson,
Michael Turquette, Stephen Boyd, Srinivas Kandagatla, Andi Shyti,
Rafael J. Wysocki, Viresh Kumar, Joerg Roedel, Will Deacon,
Robin Murphy, Dmitry Torokhov, Vinod Koul, Martin Povišer,
Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
Arnd Bergmann, asahi, linux-arm-kernel, linux-pm, linux-kernel,
linux-clk, linux-i2c, iommu, linux-input, dmaengine, linux-sound
In-Reply-To: <20250612-apple-kconfig-defconfig-v1-5-0e6f9cb512c1@kernel.org>
On Thu, Jun 12, 2025 at 09:11:29PM +0000, Sven Peter wrote:
> When the first driver for Apple Silicon was upstreamed we accidentally
> included `default ARCH_APPLE` in its Kconfig which then spread to almost
> every subsequent driver. As soon as ARCH_APPLE is set to y this will
> pull in many drivers as built-ins which is not what we want.
> Thus, drop `default ARCH_APPLE` from Kconfig.
>
> Signed-off-by: Sven Peter <sven@kernel.org>
Applied to for-next (for 6.17 mergewindow), thanks!
^ permalink raw reply
* Re: dev->uniq is not unique for individual USB interfaces
From: Jiri Kosina @ 2025-08-04 7:20 UTC (permalink / raw)
To: Andrey Smirnov; +Cc: Benjamin Tissoires, linux-usb, open list:HID CORE LAYER
In-Reply-To: <CAHQ1cqErtqZjSakSUppxKEPvK3xJYfKydM02RJTcDO0RV77r3g@mail.gmail.com>
On Mon, 21 Jul 2025, Andrey Smirnov wrote:
> Hey folks:
>
> I'm working on a custom USB device that presents N battery powered HID
> interfaces with each interface reporting its own battery life via
> standard HID Power Device report. The problem I'm running into is that
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-input.c?h=v6.16-rc7#n524
>
> assumes that "uniq" field of a "struct hid_device" is always unique, but
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/usbhid/hid-core.c?h=v6.16-rc7#n1415
>
> populates "uniq" with iSerialNumber which is only unique per USB
> device, not per USB interface. At the first glance the right way to
> fix this would be to change how uniq is generated by usbhid_probe()
> but that probably would break some userspace assumptions?
Hmm, actually, from top of my head I am not able to come up with any
userspace breakage this might cause. Do you have anything particular on
mind?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3] platform/x86: Add WMI driver for Redmibook keyboard.
From: Ilpo Järvinen @ 2025-08-04 7:31 UTC (permalink / raw)
To: Gladyshev Ilya
Cc: w_armin, linux-input, nikita.nikita.krasnov, Hans de Goede, LKML,
platform-driver-x86
In-Reply-To: <20250730185619.8725-1-foxido@foxido.dev>
On Wed, 30 Jul 2025, Gladyshev Ilya wrote:
> This driver implements support for various Fn keys (like Cut) and Xiaomi
> specific AI button.
>
> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
Thank you for the patch. Small correction requests below.
> ---
> Changes since v2:
> - Fix Kconfig dependencies
> - Accept WMI buffers that are bigger than expected
>
> Link to v2: https://lore.kernel.org/platform-driver-x86/20250729190528.8446-1-foxido@foxido.dev/
> ---
> MAINTAINERS | 6 ++
> drivers/platform/x86/Kconfig | 12 +++
> drivers/platform/x86/Makefile | 1 +
> drivers/platform/x86/redmi-wmi.c | 129 +++++++++++++++++++++++++++++++
> 4 files changed, 148 insertions(+)
> create mode 100644 drivers/platform/x86/redmi-wmi.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c0b444e5fd5a..eb25fb10e751 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20965,6 +20965,12 @@ S: Maintained
> T: git https://github.com/pkshih/rtw.git
> F: drivers/net/wireless/realtek/rtw89/
>
> +REDMIBOOK WMI DRIVERS
> +M: Gladyshev Ilya <foxido@foxido.dev>
> +L: platform-driver-x86@vger.kernel.org
> +S: Maintained
> +F: drivers/platform/x86/redmi-wmi.c
> +
> REDPINE WIRELESS DRIVER
> L: linux-wireless@vger.kernel.org
> S: Orphan
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index e5cbd58a99f3..9f98a7042e43 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -109,6 +109,18 @@ config XIAOMI_WMI
> To compile this driver as a module, choose M here: the module will
> be called xiaomi-wmi.
>
> +config REDMI_WMI
> + tristate "Redmibook WMI key driver"
> + depends on ACPI_WMI
> + depends on INPUT
> + select INPUT_SPARSEKMAP
> + help
> + Say Y here if you want support for WMI-based hotkey events on
> + Xiaomi Redmibook devices.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called redmi-wmi.
> +
> config GIGABYTE_WMI
> tristate "Gigabyte WMI temperature driver"
> depends on ACPI_WMI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index bea87a85ae75..406dd0807ba7 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_HUAWEI_WMI) += huawei-wmi.o
> obj-$(CONFIG_MXM_WMI) += mxm-wmi.o
> obj-$(CONFIG_NVIDIA_WMI_EC_BACKLIGHT) += nvidia-wmi-ec-backlight.o
> obj-$(CONFIG_XIAOMI_WMI) += xiaomi-wmi.o
> +obj-$(CONFIG_REDMI_WMI) += redmi-wmi.o
> obj-$(CONFIG_GIGABYTE_WMI) += gigabyte-wmi.o
>
> # Acer
> diff --git a/drivers/platform/x86/redmi-wmi.c b/drivers/platform/x86/redmi-wmi.c
> new file mode 100644
> index 000000000000..3dbf4a40a4ff
> --- /dev/null
> +++ b/drivers/platform/x86/redmi-wmi.c
> @@ -0,0 +1,129 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* WMI driver for Xiaomi Redmibooks */
> +
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/input/sparse-keymap.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/unaligned.h>
> +#include <linux/wmi.h>
> +
> +#include <uapi/linux/input-event-codes.h>
> +
> +#define WMI_REDMIBOOK_KEYBOARD_EVENT_GUID "46C93E13-EE9B-4262-8488-563BCA757FEF"
> +
> +#define AI_KEY_VALUE_MASK 0x00000100
> +
> +static const struct key_entry redmi_wmi_keymap[] = {
> + {KE_KEY, 0x00000201, {KEY_SELECTIVE_SCREENSHOT}},
> + {KE_KEY, 0x00000301, {KEY_ALL_APPLICATIONS}},
> + {KE_KEY, 0x00001b01, {KEY_SETUP}},
> +
> + /* AI button has code for each position */
> + {KE_KEY, 0x00011801, {KEY_ASSISTANT}},
> + {KE_KEY, 0x00011901, {KEY_ASSISTANT}},
> +
> + /* Keyboard backlight */
> + {KE_IGNORE, 0x00000501, {}},
> + {KE_IGNORE, 0x00800501, {}},
> + {KE_IGNORE, 0x00050501, {}},
> + {KE_IGNORE, 0x000a0501, {}},
> +
> + {KE_END}
> +};
> +
> +struct redmi_wmi {
> + struct input_dev *input_dev;
> + /* Protects the key event sequence */
> + struct mutex key_lock;
> +};
> +
> +static int redmi_wmi_probe(struct wmi_device *wdev, const void *context)
> +{
> + struct redmi_wmi *data;
> + int err;
> +
> + /* Init dev */
> + data = devm_kzalloc(&wdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&wdev->dev, data);
> +
> + err = devm_mutex_init(&wdev->dev, &data->key_lock);
> + if (err)
> + return err;
> +
> + /* Setup input device */
I'd drop this comment as it doesn't really add much value.
> + data->input_dev = devm_input_allocate_device(&wdev->dev);
> + if (!data->input_dev)
> + return -ENOMEM;
> + data->input_dev->name = "Redmibook WMI keys";
> + data->input_dev->phys = "wmi/input0";
> +
> + err = sparse_keymap_setup(data->input_dev, redmi_wmi_keymap, NULL);
> + if (err)
> + return err;
> +
> + return input_register_device(data->input_dev);
> +}
> +
> +static void redmi_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
> +{
> + struct redmi_wmi *data = dev_get_drvdata(&wdev->dev);
> + bool autorelease = true;
> + int value = 1;
> +
> + if (obj->type != ACPI_TYPE_BUFFER) {
> + dev_err(&wdev->dev, "Bad response type %u\n", obj->type);
To me it feels like these prints from the notify handlers should be
ratelimited just in case, but then I notice the other drivers don't
ratelimit them either so perhaps it has never caused an issue.
> + return;
> + }
> +
> + if (obj->buffer.length < 32) {
> + dev_err(&wdev->dev, "Invalid buffer length %u\n", obj->buffer.length);
> + return;
> + }
> +
> + /* For linearizability */
This comment doesn't really tell anything we don't know already from
seeing the mutex being taken.
> + guard(mutex)(&data->key_lock);
> +
> + u32 payload = get_unaligned_le32(obj->buffer.pointer);
Please always declare variables at the beginning of a function or a block,
not in the middle of it. The only allowed exception to this rule is
cleanup.h related variables as there the cleanup order really matters.
> + struct key_entry *entry = sparse_keymap_entry_from_scancode(data->input_dev, payload);
> +
> + if (!entry) {
> + dev_dbg(&wdev->dev, "Unknown WMI event with payload %u", payload);
> + return;
> + }
> +
> + /* AI key quirk */
> + if (entry->keycode == KEY_ASSISTANT) {
> + value = !(payload & AI_KEY_VALUE_MASK);
> + autorelease = false;
> + }
> +
> + sparse_keymap_report_entry(data->input_dev, entry, value, autorelease);
> +}
> +
> +static const struct wmi_device_id redmi_wmi_id_table[] = {
> + { .guid_string = WMI_REDMIBOOK_KEYBOARD_EVENT_GUID },
> + { }
> +};
> +
> +static struct wmi_driver redmi_wmi_driver = {
> + .driver = {
> + .name = "redmi-wmi",
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS
Please put a comma on any non-terminating entries so that a future
diff won't have to add it on that (unrelated) line.
> + },
> + .id_table = redmi_wmi_id_table,
> + .probe = redmi_wmi_probe,
> + .notify = redmi_wmi_notify,
> + .no_singleton = true,
> +};
> +module_wmi_driver(redmi_wmi_driver);
> +
> +MODULE_DEVICE_TABLE(wmi, redmi_wmi_id_table);
> +MODULE_AUTHOR("Gladyshev Ilya <foxido@foxido.dev>");
> +MODULE_DESCRIPTION("Redmibook WMI driver");
> +MODULE_LICENSE("GPL");
>
--
i.
^ permalink raw reply
* Re: [PATCH 3/3] arm64: dts: mediatek: mt8395-nio-12l: add support for blue and red LEDs
From: AngeloGioacchino Del Regno @ 2025-08-04 8:05 UTC (permalink / raw)
To: Julien Massot, kernel, Dmitry Torokhov, Matthias Brugger,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-kernel, linux-arm-kernel, linux-mediatek,
devicetree
In-Reply-To: <20250801-radxa-nio-12-l-gpio-v1-3-d0840f85d2c8@collabora.com>
Il 01/08/25 15:16, Julien Massot ha scritto:
> The Radxa NIO 12L board has an RGB LED, of which only red and blue
> are controllable.
>
> Red and blue LEDs: no need to choose, both are enabled.
>
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply
* Re: [PATCH 2/3] arm64: dts: mediatek: mt8395-nio-12l: add PMIC and GPIO keys support
From: AngeloGioacchino Del Regno @ 2025-08-04 8:05 UTC (permalink / raw)
To: Julien Massot, kernel, Dmitry Torokhov, Matthias Brugger,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-kernel, linux-arm-kernel, linux-mediatek,
devicetree
In-Reply-To: <20250801-radxa-nio-12-l-gpio-v1-2-d0840f85d2c8@collabora.com>
Il 01/08/25 15:16, Julien Massot ha scritto:
> Add support for PMIC and GPIO keys on the Radxa NIO 12L board:
> Declare a gpio-keys node for the Volume Up button using GPIO106.
> Add the corresponding pin configuration in the pinctrl node.
> Add a mediatek,mt6359-keys subnode under the PMIC to handle the
> power and home buttons exposed by the MT6359.
>
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply
* Re: [PATCH 1/3] Input: mtk-pmic-keys - MT6359 has a specific release irq
From: AngeloGioacchino Del Regno @ 2025-08-04 8:05 UTC (permalink / raw)
To: Julien Massot, kernel, Dmitry Torokhov, Matthias Brugger,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-input, linux-kernel, linux-arm-kernel, linux-mediatek,
devicetree
In-Reply-To: <20250801-radxa-nio-12-l-gpio-v1-1-d0840f85d2c8@collabora.com>
Il 01/08/25 15:16, Julien Massot ha scritto:
> A recent commit in linux-next added support for key events.
> However, the key release event is not properly handled: only key press events
> are generated, leaving key states stuck in "pressed".
>
> This patch ensures that both key press and key release events are properly
> emitted by handling the release logic correctly.
>
> Note: the code was introduced in linux-next by commit
> bc25e6bf032e ("Input: mtk-pmic-keys - add support for MT6359 PMIC keys")
> and is not yet present in mainline.
>
> Signed-off-by: Julien Massot <julien.massot@collabora.com>
Well, you are effectively fixing the commit that you pointed out, so this needs
Fixes: bc25e6bf032e ("Input: mtk-pmic-keys - add support for MT6359 PMIC keys")
and
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cheers,
Angelo
^ permalink raw reply
* [PATCH 1/2] HID: input: rename hidinput_set_battery_charge_status()
From: José Expósito @ 2025-08-04 9:11 UTC (permalink / raw)
To: jikos
Cc: bentiss, luguohong, linux-input, linux-kernel,
José Expósito
In preparation for a patch fixing a bug affecting
hidinput_set_battery_charge_status(), rename the function to
hidinput_update_battery_charge_status() and move it up so it can be used
by hidinput_update_battery().
Refactor, no functional changes.
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
drivers/hid/hid-input-test.c | 10 +++++-----
drivers/hid/hid-input.c | 38 ++++++++++++++++++------------------
2 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/hid-input-test.c b/drivers/hid/hid-input-test.c
index 77c2d45ac62a..6f5c71660d82 100644
--- a/drivers/hid/hid-input-test.c
+++ b/drivers/hid/hid-input-test.c
@@ -7,7 +7,7 @@
#include <kunit/test.h>
-static void hid_test_input_set_battery_charge_status(struct kunit *test)
+static void hid_test_input_update_battery_charge_status(struct kunit *test)
{
struct hid_device *dev;
bool handled;
@@ -15,15 +15,15 @@ static void hid_test_input_set_battery_charge_status(struct kunit *test)
dev = kunit_kzalloc(test, sizeof(*dev), GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
- handled = hidinput_set_battery_charge_status(dev, HID_DG_HEIGHT, 0);
+ handled = hidinput_update_battery_charge_status(dev, HID_DG_HEIGHT, 0);
KUNIT_EXPECT_FALSE(test, handled);
KUNIT_EXPECT_EQ(test, dev->battery_charge_status, POWER_SUPPLY_STATUS_UNKNOWN);
- handled = hidinput_set_battery_charge_status(dev, HID_BAT_CHARGING, 0);
+ handled = hidinput_update_battery_charge_status(dev, HID_BAT_CHARGING, 0);
KUNIT_EXPECT_TRUE(test, handled);
KUNIT_EXPECT_EQ(test, dev->battery_charge_status, POWER_SUPPLY_STATUS_DISCHARGING);
- handled = hidinput_set_battery_charge_status(dev, HID_BAT_CHARGING, 1);
+ handled = hidinput_update_battery_charge_status(dev, HID_BAT_CHARGING, 1);
KUNIT_EXPECT_TRUE(test, handled);
KUNIT_EXPECT_EQ(test, dev->battery_charge_status, POWER_SUPPLY_STATUS_CHARGING);
}
@@ -63,7 +63,7 @@ static void hid_test_input_get_battery_property(struct kunit *test)
}
static struct kunit_case hid_input_tests[] = {
- KUNIT_CASE(hid_test_input_set_battery_charge_status),
+ KUNIT_CASE(hid_test_input_update_battery_charge_status),
KUNIT_CASE(hid_test_input_get_battery_property),
{ }
};
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index ff1784b5c2a4..262787e6eb20 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -595,6 +595,20 @@ static void hidinput_cleanup_battery(struct hid_device *dev)
dev->battery = NULL;
}
+static bool hidinput_update_battery_charge_status(struct hid_device *dev,
+ unsigned int usage, int value)
+{
+ switch (usage) {
+ case HID_BAT_CHARGING:
+ dev->battery_charge_status = value ?
+ POWER_SUPPLY_STATUS_CHARGING :
+ POWER_SUPPLY_STATUS_DISCHARGING;
+ return true;
+ }
+
+ return false;
+}
+
static void hidinput_update_battery(struct hid_device *dev, int value)
{
int capacity;
@@ -617,20 +631,6 @@ static void hidinput_update_battery(struct hid_device *dev, int value)
power_supply_changed(dev->battery);
}
}
-
-static bool hidinput_set_battery_charge_status(struct hid_device *dev,
- unsigned int usage, int value)
-{
- switch (usage) {
- case HID_BAT_CHARGING:
- dev->battery_charge_status = value ?
- POWER_SUPPLY_STATUS_CHARGING :
- POWER_SUPPLY_STATUS_DISCHARGING;
- return true;
- }
-
- return false;
-}
#else /* !CONFIG_HID_BATTERY_STRENGTH */
static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
struct hid_field *field, bool is_percentage)
@@ -642,14 +642,14 @@ static void hidinput_cleanup_battery(struct hid_device *dev)
{
}
-static void hidinput_update_battery(struct hid_device *dev, int value)
+static bool hidinput_update_battery_charge_status(struct hid_device *dev,
+ unsigned int usage, int value)
{
+ return false;
}
-static bool hidinput_set_battery_charge_status(struct hid_device *dev,
- unsigned int usage, int value)
+static void hidinput_update_battery(struct hid_device *dev, int value)
{
- return false;
}
#endif /* CONFIG_HID_BATTERY_STRENGTH */
@@ -1515,7 +1515,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
return;
if (usage->type == EV_PWR) {
- bool handled = hidinput_set_battery_charge_status(hid, usage->hid, value);
+ bool handled = hidinput_update_battery_charge_status(hid, usage->hid, value);
if (!handled)
hidinput_update_battery(hid, value);
--
2.50.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox