Re: [PATCH 1/2] Fix initialisation for the Microsoft Sidewinder Force Feedback Pro 2 joystick

From: Benjamin Tissoires
Date: Fri Jan 23 2015 - 13:48:58 EST


You are missing here (before the first '---' the commit message which
should state why you want to have this patch merged.

To rephrase and continue what you wrote in your very first submission
of this series, this could be (including your Signed-of-by):

The FF2 driver (usbhid/hid-pidff.c) sends commands to the stick
during ff_init. However, this is called inside a block where
driver_input_lock is locked, so the results of these initial commands
are discarded. This behavior is the "killer", without this nothing else works.

ff_init issues commands using "hid_hw_request". This eventually goes to
hid_input_report, which returns -EBUSY because driver_input_lock is
locked. The change is to delay the ff_init call in hid-core.c until
after this lock has been released.

Calling hid_device_io_start() releases the lock so the device can be configured.
We also need to call hid_device_io_stop() on exit for the lock to
remain locked while ending the init of the drivers.

Signed-off-by: Jim Keir <jimkeir@xxxxxxxxxxxxxxxxxxx>

---

With the changes above (or if you fix my typos), the patch is
Reviewed-by: Benjamin.tissoires <benjamin.tissoires@xxxxxxxxxx>

Jiri, could you amend the commit with the above so that Jim won't be
desperate by submitting a patch?

Cheers,
Benjamin


On Fri, Jan 23, 2015 at 12:21 PM, Jim Keir <jimkeir@xxxxxxxxxxxxxxxxxxx> wrote:
> ---
> Hi,
>
> Changes from previous patches:
> - All changes removed from higher-level files
> - Calls to hid_device_io_start/stop added here
>
> Signed-off-by: Jim Keir <jimkeir@xxxxxxxxxxxxxxxxxxx>
>
> drivers/hid/usbhid/hid-pidff.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
> index 10b6167..0b531c6 100644
> --- a/drivers/hid/usbhid/hid-pidff.c
> +++ b/drivers/hid/usbhid/hid-pidff.c
> @@ -1252,6 +1252,8 @@ int hid_pidff_init(struct hid_device *hid)
>
> pidff->hid = hid;
>
> + hid_device_io_start(hid);
> +
> pidff_find_reports(hid, HID_OUTPUT_REPORT, pidff);
> pidff_find_reports(hid, HID_FEATURE_REPORT, pidff);
>
> @@ -1315,9 +1317,13 @@ int hid_pidff_init(struct hid_device *hid)
>
> hid_info(dev, "Force feedback for USB HID PID devices by Anssi Hannula <anssi.hannula@xxxxxxxxx>\n");
>
> + hid_device_io_stop(hid);
> +
> return 0;
>
> fail:
> + hid_device_io_stop(hid);
> +
> kfree(pidff);
> return error;
> }
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/