| Message ID | 20180531114929.30545-1-benjamin.tissoires@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Benjamin, I had to make a minor change since I didn't see > #define HID_STAT_DUP_DETECTED BIT(2) In Linus's tree. I tested with master today though and it seems to be working now with your patch across several boot attempts. Tested-by: Mario Limonciello <mario.limonciello@dell.com> Thanks, > -----Original Message----- > From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com] > Sent: Thursday, May 31, 2018 6:49 AM > To: Jiri Kosina; Limonciello, Mario > Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Benjamin Tissoires > Subject: [PATCH] HID: core: allow concurrent registration of drivers > > Detected on the Dell XPS 9365. > The laptop has 2 devices that benefit from the hid-generic auto-unbinding. > When those 2 devices are presented to the userspace, udev loads both > wacom and hid-multitouch. When this happens, the code in > __hid_bus_reprobe_drivers() is called concurrently and the second device > gets reprobed twice. > An other bug in the power_supply subsystem prevent to remove the wacom > driver if it just finished its initialization, which basically kills > the wacom node. > > Fixes c17a7476e4c4 ("HID: core: rewrite the hid-generic automatic unbind") > > Cc: stable@vger.kernel.org # v4.17 > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > > Hi Mario, > > can you please test this on your faulty XPS? > I think it'll fix the issue, but I can not reproduce, so better wait for > your confirmation. > > Jiri, ideally I would love to see this in v4.17 final, but Mario seems > to be on PTO until next week. I guess we'll just push this in v4.17.1 > then. Also, if you can double check that it makes sense, that would be > nice :) > > Cheers, > Benjamin > > drivers/hid/hid-core.c | 5 ++++- > include/linux/hid.h | 3 ++- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 2f7367b1de00..7afed0c0f9e5 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -1960,6 +1960,8 @@ static int hid_device_probe(struct device *dev) > } > hdev->io_started = false; > > + clear_bit(ffs(HID_STAT_REPROBED), &hdev->status); > + > if (!hdev->driver) { > id = hid_match_device(hdev, hdrv); > if (id == NULL) { > @@ -2223,7 +2225,8 @@ static int __hid_bus_reprobe_drivers(struct device *dev, > void *data) > struct hid_device *hdev = to_hid_device(dev); > > if (hdev->driver == hdrv && > - !hdrv->match(hdev, hid_ignore_special_drivers)) > + !hdrv->match(hdev, hid_ignore_special_drivers) && > + !test_and_set_bit(ffs(HID_STAT_REPROBED), &hdev->status)) > return device_reprobe(dev); > > return 0; > diff --git a/include/linux/hid.h b/include/linux/hid.h > index ee2510019033..aee281522c6d 100644 > --- a/include/linux/hid.h > +++ b/include/linux/hid.h > @@ -517,6 +517,7 @@ struct hid_output_fifo { > #define HID_STAT_ADDED BIT(0) > #define HID_STAT_PARSED BIT(1) > #define HID_STAT_DUP_DETECTED BIT(2) > +#define HID_STAT_REPROBED BIT(3) > > struct hid_input { > struct list_head list; > @@ -585,7 +586,7 @@ struct hid_device { > /* device report descriptor */ > bool battery_avoid_query; > #endif > > - unsigned int status; /* see > STAT flags above */ > + unsigned long status; /* see > STAT flags above */ > unsigned claimed; /* > Claimed by hidinput, hiddev? */ > unsigned quirks; /* Various > quirks the device can pull on us */ > bool io_started; /* If IO has started > */ > -- > 2.14.3 -- 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
Hi, On Mon, Jun 4, 2018 at 10:37 PM, <Mario.Limonciello@dell.com> wrote: > Benjamin, > > I had to make a minor change since I didn't see >> #define HID_STAT_DUP_DETECTED BIT(2) > In Linus's tree. > > I tested with master today though and it seems to be working now with your patch across > several boot attempts. > > Tested-by: Mario Limonciello <mario.limonciello@dell.com> Thanks a lot Mario. Jiri, I do not see this patch in your tree. Could you take it soon-ish so we can backport it in 4.17 ASAP? Cheers, Benjamin > > Thanks, > >> -----Original Message----- >> From: Benjamin Tissoires [mailto:benjamin.tissoires@redhat.com] >> Sent: Thursday, May 31, 2018 6:49 AM >> To: Jiri Kosina; Limonciello, Mario >> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Benjamin Tissoires >> Subject: [PATCH] HID: core: allow concurrent registration of drivers >> >> Detected on the Dell XPS 9365. >> The laptop has 2 devices that benefit from the hid-generic auto-unbinding. >> When those 2 devices are presented to the userspace, udev loads both >> wacom and hid-multitouch. When this happens, the code in >> __hid_bus_reprobe_drivers() is called concurrently and the second device >> gets reprobed twice. >> An other bug in the power_supply subsystem prevent to remove the wacom >> driver if it just finished its initialization, which basically kills >> the wacom node. >> >> Fixes c17a7476e4c4 ("HID: core: rewrite the hid-generic automatic unbind") >> >> Cc: stable@vger.kernel.org # v4.17 >> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> >> --- >> >> Hi Mario, >> >> can you please test this on your faulty XPS? >> I think it'll fix the issue, but I can not reproduce, so better wait for >> your confirmation. >> >> Jiri, ideally I would love to see this in v4.17 final, but Mario seems >> to be on PTO until next week. I guess we'll just push this in v4.17.1 >> then. Also, if you can double check that it makes sense, that would be >> nice :) >> >> Cheers, >> Benjamin >> >> drivers/hid/hid-core.c | 5 ++++- >> include/linux/hid.h | 3 ++- >> 2 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c >> index 2f7367b1de00..7afed0c0f9e5 100644 >> --- a/drivers/hid/hid-core.c >> +++ b/drivers/hid/hid-core.c >> @@ -1960,6 +1960,8 @@ static int hid_device_probe(struct device *dev) >> } >> hdev->io_started = false; >> >> + clear_bit(ffs(HID_STAT_REPROBED), &hdev->status); >> + >> if (!hdev->driver) { >> id = hid_match_device(hdev, hdrv); >> if (id == NULL) { >> @@ -2223,7 +2225,8 @@ static int __hid_bus_reprobe_drivers(struct device *dev, >> void *data) >> struct hid_device *hdev = to_hid_device(dev); >> >> if (hdev->driver == hdrv && >> - !hdrv->match(hdev, hid_ignore_special_drivers)) >> + !hdrv->match(hdev, hid_ignore_special_drivers) && >> + !test_and_set_bit(ffs(HID_STAT_REPROBED), &hdev->status)) >> return device_reprobe(dev); >> >> return 0; >> diff --git a/include/linux/hid.h b/include/linux/hid.h >> index ee2510019033..aee281522c6d 100644 >> --- a/include/linux/hid.h >> +++ b/include/linux/hid.h >> @@ -517,6 +517,7 @@ struct hid_output_fifo { >> #define HID_STAT_ADDED BIT(0) >> #define HID_STAT_PARSED BIT(1) >> #define HID_STAT_DUP_DETECTED BIT(2) >> +#define HID_STAT_REPROBED BIT(3) >> >> struct hid_input { >> struct list_head list; >> @@ -585,7 +586,7 @@ struct hid_device { >> /* device report descriptor */ >> bool battery_avoid_query; >> #endif >> >> - unsigned int status; /* see >> STAT flags above */ >> + unsigned long status; /* see >> STAT flags above */ >> unsigned claimed; /* >> Claimed by hidinput, hiddev? */ >> unsigned quirks; /* Various >> quirks the device can pull on us */ >> bool io_started; /* If IO has started >> */ >> -- >> 2.14.3 > -- 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
On Thu, 31 May 2018, Benjamin Tissoires wrote: > Detected on the Dell XPS 9365. > The laptop has 2 devices that benefit from the hid-generic auto-unbinding. > When those 2 devices are presented to the userspace, udev loads both > wacom and hid-multitouch. When this happens, the code in > __hid_bus_reprobe_drivers() is called concurrently and the second device > gets reprobed twice. > An other bug in the power_supply subsystem prevent to remove the wacom > driver if it just finished its initialization, which basically kills > the wacom node. > > Fixes c17a7476e4c4 ("HID: core: rewrite the hid-generic automatic unbind") > > Cc: stable@vger.kernel.org # v4.17 > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Now staged in for-4.18/upstream-fixes_v2, with the plan to send it to Linus this week. Thanks,
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 2f7367b1de00..7afed0c0f9e5 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1960,6 +1960,8 @@ static int hid_device_probe(struct device *dev) } hdev->io_started = false; + clear_bit(ffs(HID_STAT_REPROBED), &hdev->status); + if (!hdev->driver) { id = hid_match_device(hdev, hdrv); if (id == NULL) { @@ -2223,7 +2225,8 @@ static int __hid_bus_reprobe_drivers(struct device *dev, void *data) struct hid_device *hdev = to_hid_device(dev); if (hdev->driver == hdrv && - !hdrv->match(hdev, hid_ignore_special_drivers)) + !hdrv->match(hdev, hid_ignore_special_drivers) && + !test_and_set_bit(ffs(HID_STAT_REPROBED), &hdev->status)) return device_reprobe(dev); return 0; diff --git a/include/linux/hid.h b/include/linux/hid.h index ee2510019033..aee281522c6d 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -517,6 +517,7 @@ struct hid_output_fifo { #define HID_STAT_ADDED BIT(0) #define HID_STAT_PARSED BIT(1) #define HID_STAT_DUP_DETECTED BIT(2) +#define HID_STAT_REPROBED BIT(3) struct hid_input { struct list_head list; @@ -585,7 +586,7 @@ struct hid_device { /* device report descriptor */ bool battery_avoid_query; #endif - unsigned int status; /* see STAT flags above */ + unsigned long status; /* see STAT flags above */ unsigned claimed; /* Claimed by hidinput, hiddev? */ unsigned quirks; /* Various quirks the device can pull on us */ bool io_started; /* If IO has started */
Detected on the Dell XPS 9365. The laptop has 2 devices that benefit from the hid-generic auto-unbinding. When those 2 devices are presented to the userspace, udev loads both wacom and hid-multitouch. When this happens, the code in __hid_bus_reprobe_drivers() is called concurrently and the second device gets reprobed twice. An other bug in the power_supply subsystem prevent to remove the wacom driver if it just finished its initialization, which basically kills the wacom node. Fixes c17a7476e4c4 ("HID: core: rewrite the hid-generic automatic unbind") Cc: stable@vger.kernel.org # v4.17 Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- Hi Mario, can you please test this on your faulty XPS? I think it'll fix the issue, but I can not reproduce, so better wait for your confirmation. Jiri, ideally I would love to see this in v4.17 final, but Mario seems to be on PTO until next week. I guess we'll just push this in v4.17.1 then. Also, if you can double check that it makes sense, that would be nice :) Cheers, Benjamin drivers/hid/hid-core.c | 5 ++++- include/linux/hid.h | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-)