LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Benjamin Tissoires <benjamin.tissoires@gmail.com>
To: Jim Keir <jimkeir@oracledbadirect.com>
Cc: linux-input <linux-input@vger.kernel.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Jiri Kosina <jkosina@suse.cz>
Subject: Re: [PATCH 1/2] Fix initialisation for the Microsoft Sidewinder Force Feedback Pro 2 joystick
Date: Fri, 23 Jan 2015 13:48:50 -0500	[thread overview]
Message-ID: <CAN+gG=FC1gOCY1FQhHa16NALuyx2z5w7088k87O3eFM5518_Ug@mail.gmail.com> (raw)
In-Reply-To: <0000014b17cfda93-ba54b0fb-23f4-4005-95c1-6ae695a48369-000000@email.amazonses.com>

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

---

With the changes above (or if you fix my typos), the patch is
Reviewed-by: Benjamin.tissoires <benjamin.tissoires@redhat.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@oracledbadirect.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@oracledbadirect.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@gmail.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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2015-01-23 18:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-23 17:21 Jim Keir
2015-01-23 18:48 ` Benjamin Tissoires [this message]
2015-01-26 20:28   ` Jiri Kosina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAN+gG=FC1gOCY1FQhHa16NALuyx2z5w7088k87O3eFM5518_Ug@mail.gmail.com' \
    --to=benjamin.tissoires@gmail.com \
    --cc=jimkeir@oracledbadirect.com \
    --cc=jkosina@suse.cz \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --subject='Re: [PATCH 1/2] Fix initialisation for the Microsoft Sidewinder Force Feedback Pro 2 joystick' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).