Previously, it was assumed that DEVICE_CONTROL usage is always an array
but a lot of devices implements it as a bitmask variable. This led to
the pidff_reset function not working and causing errors in such cases.
Selectors can come in three types. One selection of a set, N selections
and Any selection in form of bitmask as from USB Hid Usage Tables v1.5,
subsection 3.4.2.1
Added pidff_send_device_control which handles usage flag check which
decides whether DEVICE_CONTROL should be handled as "One selection of a
set" or "Any selection of a set".
Reset was triggered once, on device initialization. Now, it's triggered
every time when uploading an effect to an empty device (no currently
stored effects), tracked by pidff->effect_count variable.
Co-developed-by: Makarenko Oleg <oleg@xxxxxxxxxxxx>
Signed-off-by: Makarenko Oleg <oleg@xxxxxxxxxxxx>
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@xxxxxxxxx>
---
drivers/hid/usbhid/hid-pidff.c | 139 +++++++++++++++++++++------------
1 file changed, 90 insertions(+), 49 deletions(-)
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index e6224e797dc6..689bf516ec55 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -109,9 +109,10 @@ static const u8 pidff_pool[] = { 0x80, 0x83, 0xa9 };
/* Special field key tables used to put special field keys into arrays */
#define PID_ENABLE_ACTUATORS 0
-#define PID_STOP_ALL_EFFECTS 1
-#define PID_RESET 2
-static const u8 pidff_device_control[] = { 0x97, 0x99, 0x9a };
+#define PID_DISABLE_ACTUATORS 1
+#define PID_STOP_ALL_EFFECTS 2
+#define PID_RESET 3
+static const u8 pidff_device_control[] = { 0x97, 0x98, 0x99, 0x9a };
#define PID_CONSTANT 0
#define PID_RAMP 1
@@ -190,6 +191,7 @@ struct pidff_device {
int pid_id[PID_EFFECTS_MAX];
u32 quirks;
+ u8 effect_count;
};
/*
@@ -490,9 +492,84 @@ static int pidff_needs_set_ramp(struct ff_effect *effect, struct ff_effect *old)
effect->u.ramp.end_level != old->u.ramp.end_level;
}
+/*
+ * Clear device control report
+ */
+static void pidff_send_device_control(struct pidff_device *pidff, int field)
+{
+ int i, tmp;
+ int field_index = pidff->control_id[field];
+
+ /* Detect if the field is a bitmask variable or an array */
+ if (pidff->device_control->flags & HID_MAIN_ITEM_VARIABLE) {
+ hid_dbg(pidff->hid, "DEVICE_CONTROL is a bitmask\n");
+ /* Clear current bitmask */
+ for(i = 0; i < sizeof(pidff_device_control); i++) {
+ tmp = pidff_device_control[i];
+ tmp = pidff->control_id[tmp];
+ pidff->device_control->value[tmp] = 0;
+ }
+ pidff->device_control->value[field_index - 1] = 1;
+ } else {
+ hid_dbg(pidff->hid, "DEVICE_CONTROL is an array\n");
+ pidff->device_control->value[0] = field_index;
+ }
+
+ hid_hw_request(pidff->hid, pidff->reports[PID_DEVICE_CONTROL], HID_REQ_SET_REPORT);
+ hid_hw_wait(pidff->hid);
+}
+
+/*
+ * Modify actuators state
+ */
+static void pidff_modify_actuators_state(struct pidff_device *pidff, bool enable)
+{
+ hid_dbg(pidff->hid, "%s actuators\n", enable ? "Enable" : "Disable");
+ pidff_send_device_control(pidff,
+ enable ? PID_ENABLE_ACTUATORS : PID_DISABLE_ACTUATORS);
+}
+
+/*
+ * Reset the device, stop all effects, enable actuators
+ * Refetch pool report
+ */
+static void pidff_reset(struct pidff_device *pidff)
+{
+ int i = 0;
+
+ /* We reset twice as sometimes hid_wait_io isn't waiting long enough */
+ pidff_send_device_control(pidff, PID_RESET);
+ pidff_send_device_control(pidff, PID_RESET);
+ pidff->effect_count = 0;
+
+ pidff_send_device_control(pidff, PID_STOP_ALL_EFFECTS);
+ pidff_modify_actuators_state(pidff, 1);
+
+ /* pool report is sometimes messed up, refetch it */
+ hid_hw_request(pidff->hid, pidff->reports[PID_POOL], HID_REQ_GET_REPORT);
+ hid_hw_wait(pidff->hid);
+
+ if (pidff->pool[PID_SIMULTANEOUS_MAX].value) {
+ while (pidff->pool[PID_SIMULTANEOUS_MAX].value[0] < 2) {
+ if (i++ > 20) {
+ hid_warn(pidff->hid,
+ "device reports %d simultaneous effects\n",
+ pidff->pool[PID_SIMULTANEOUS_MAX].value[0]);
+ break;
+ }
+ hid_dbg(pidff->hid, "pid_pool requested again\n");
+ hid_hw_request(pidff->hid, pidff->reports[PID_POOL],
+ HID_REQ_GET_REPORT);
+ hid_hw_wait(pidff->hid);
+ }
+ }
+}
+
/*
* Send a request for effect upload to the device
*
+ * Reset and enable actuators if no effects were present on the device
+ *
* Returns 0 if device reported success, -ENOSPC if the device reported memory
* is full. Upon unknown response the function will retry for 60 times, if
* still unsuccessful -EIO is returned.
@@ -501,6 +578,9 @@ 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);
@@ -520,6 +600,8 @@ 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] ==
@@ -568,12 +650,16 @@ static int pidff_playback(struct input_dev *dev, int effect_id, int value)
/*
* Erase effect with PID id
+ * Decrease the device effect counter
*/
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--;
}
/*
@@ -1211,50 +1297,6 @@ static int pidff_init_fields(struct pidff_device *pidff, struct input_dev *dev)
return 0;
}
-/*
- * Reset the device
- */
-static void pidff_reset(struct pidff_device *pidff)
-{
- struct hid_device *hid = pidff->hid;
- int i = 0;
-
- pidff->device_control->value[0] = pidff->control_id[PID_RESET];
- /* We reset twice as sometimes hid_wait_io isn't waiting long enough */
- hid_hw_request(hid, pidff->reports[PID_DEVICE_CONTROL], HID_REQ_SET_REPORT);
- hid_hw_wait(hid);
- hid_hw_request(hid, pidff->reports[PID_DEVICE_CONTROL], HID_REQ_SET_REPORT);
- hid_hw_wait(hid);
-
- pidff->device_control->value[0] = pidff->control_id[PID_STOP_ALL_EFFECTS];
- hid_hw_request(hid, pidff->reports[PID_DEVICE_CONTROL], HID_REQ_SET_REPORT);
- hid_hw_wait(hid);
-
- pidff->device_control->value[0] =
- pidff->control_id[PID_ENABLE_ACTUATORS];
- hid_hw_request(hid, pidff->reports[PID_DEVICE_CONTROL], HID_REQ_SET_REPORT);
- hid_hw_wait(hid);
-
- /* pool report is sometimes messed up, refetch it */
- hid_hw_request(hid, pidff->reports[PID_POOL], HID_REQ_GET_REPORT);
- hid_hw_wait(hid);
-
- if (pidff->pool[PID_SIMULTANEOUS_MAX].value) {
- while (pidff->pool[PID_SIMULTANEOUS_MAX].value[0] < 2) {
- if (i++ > 20) {
- hid_warn(pidff->hid,
- "device reports %d simultaneous effects\n",
- pidff->pool[PID_SIMULTANEOUS_MAX].value[0]);
- break;
- }
- hid_dbg(pidff->hid, "pid_pool requested again\n");
- hid_hw_request(hid, pidff->reports[PID_POOL],
- HID_REQ_GET_REPORT);
- hid_hw_wait(hid);
- }
- }
-}
-
/*
* Test if autocenter modification is using the supported method
*/
@@ -1320,6 +1362,7 @@ int hid_pidff_init_with_quirks(struct hid_device *hid, __u32 initial_quirks)
pidff->hid = hid;
pidff->quirks = initial_quirks;
+ pidff->effect_count = 0;
hid_device_io_start(hid);
@@ -1336,8 +1379,6 @@ int hid_pidff_init_with_quirks(struct hid_device *hid, __u32 initial_quirks)
if (error)
goto fail;
- pidff_reset(pidff);
-
if (test_bit(FF_GAIN, dev->ffbit)) {
pidff_set(&pidff->device_gain[PID_DEVICE_GAIN_FIELD], 0xffff);
hid_hw_request(hid, pidff->reports[PID_DEVICE_GAIN],
--
2.48.1
