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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 23 Jan 2015 13:48:50 -0500
From:	Benjamin Tissoires <benjamin.tissoires@...il.com>
To:	Jim Keir <jimkeir@...cledbadirect.com>
Cc:	linux-input <linux-input@...r.kernel.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Jiri Kosina <jkosina@...e.cz>
Subject: Re: [PATCH 1/2] Fix initialisation for the Microsoft Sidewinder Force
 Feedback Pro 2 joystick

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@...cledbadirect.com>

---

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

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@...cledbadirect.com> 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@...cledbadirect.com>
>
>  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@...il.com>\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@...r.kernel.org
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/