| Message ID | 20180525125106.18008-1-benjamin.tissoires@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 25, 2018 at 02:51:06PM +0200, Benjamin Tissoires wrote: > When we receive a RMI4 report, we should not unconditionally send an > input_sync event. Instead, we should let the rmi4 transport layer do it > for us. > > This fixes a situation where we might receive X in a report and the rest > in a subsequent one. And this messes up user space. > > Link: https://bugs.freedesktop.org/show_bug.cgi?id=100436 > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- yes please! Acked-by: Peter Hutterer <peter.hutterer@who-t.net> Cheers, Peter > > Hi, > > Oscar, do you mind if we add your "Tested-by: Oscar Morante <your@email>"? > > Andrew, can you check for any sides effects please? > > Cheers, > Benjamin > > drivers/hid/hid-rmi.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c > index 9c9362149641..9e33165250a3 100644 > --- a/drivers/hid/hid-rmi.c > +++ b/drivers/hid/hid-rmi.c > @@ -413,6 +413,24 @@ static int rmi_event(struct hid_device *hdev, struct hid_field *field, > return 0; > } > > +static void rmi_report(struct hid_device *hid, struct hid_report *report) > +{ > + struct hid_field *field = report->field[0]; > + > + if (!(hid->claimed & HID_CLAIMED_INPUT)) > + return; > + > + switch (report->id) { > + case RMI_READ_DATA_REPORT_ID: > + /* fall-through */ > + case RMI_ATTN_REPORT_ID: > + return; > + } > + > + if (field && field->hidinput && field->hidinput->input) > + input_sync(field->hidinput->input); > +} > + > #ifdef CONFIG_PM > static int rmi_suspend(struct hid_device *hdev, pm_message_t message) > { > @@ -637,6 +655,7 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id) > hid_set_drvdata(hdev, data); > > hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS; > + hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC; > > ret = hid_parse(hdev); > if (ret) { > @@ -744,6 +763,7 @@ static struct hid_driver rmi_driver = { > .remove = rmi_remove, > .event = rmi_event, > .raw_event = rmi_raw_event, > + .report = rmi_report, > .input_mapping = rmi_input_mapping, > .input_configured = rmi_input_configured, > #ifdef CONFIG_PM > -- > 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 Fri, May 25, 2018 at 2:51 PM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > When we receive a RMI4 report, we should not unconditionally send an > input_sync event. Instead, we should let the rmi4 transport layer do it > for us. > > This fixes a situation where we might receive X in a report and the rest > in a subsequent one. And this messes up user space. > > Link: https://bugs.freedesktop.org/show_bug.cgi?id=100436 > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > > Hi, > > Oscar, do you mind if we add your "Tested-by: Oscar Morante <your@email>"? As mentioned in comment 25 of the bug above (https://bugs.freedesktop.org/show_bug.cgi?id=100436#c25), oscar is fine adding his tested-by line. Cheers, Benjamin > > Andrew, can you check for any sides effects please? > > Cheers, > Benjamin > > drivers/hid/hid-rmi.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c > index 9c9362149641..9e33165250a3 100644 > --- a/drivers/hid/hid-rmi.c > +++ b/drivers/hid/hid-rmi.c > @@ -413,6 +413,24 @@ static int rmi_event(struct hid_device *hdev, struct hid_field *field, > return 0; > } > > +static void rmi_report(struct hid_device *hid, struct hid_report *report) > +{ > + struct hid_field *field = report->field[0]; > + > + if (!(hid->claimed & HID_CLAIMED_INPUT)) > + return; > + > + switch (report->id) { > + case RMI_READ_DATA_REPORT_ID: > + /* fall-through */ > + case RMI_ATTN_REPORT_ID: > + return; > + } > + > + if (field && field->hidinput && field->hidinput->input) > + input_sync(field->hidinput->input); > +} > + > #ifdef CONFIG_PM > static int rmi_suspend(struct hid_device *hdev, pm_message_t message) > { > @@ -637,6 +655,7 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id) > hid_set_drvdata(hdev, data); > > hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS; > + hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC; > > ret = hid_parse(hdev); > if (ret) { > @@ -744,6 +763,7 @@ static struct hid_driver rmi_driver = { > .remove = rmi_remove, > .event = rmi_event, > .raw_event = rmi_raw_event, > + .report = rmi_report, > .input_mapping = rmi_input_mapping, > .input_configured = rmi_input_configured, > #ifdef CONFIG_PM > -- > 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 Fri, 25 May 2018, Benjamin Tissoires wrote: > When we receive a RMI4 report, we should not unconditionally send an > input_sync event. Instead, we should let the rmi4 transport layer do it > for us. > > This fixes a situation where we might receive X in a report and the rest > in a subsequent one. And this messes up user space. > > Link: https://bugs.freedesktop.org/show_bug.cgi?id=100436 > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > > Hi, > > Oscar, do you mind if we add your "Tested-by: Oscar Morante <your@email>"? > > Andrew, can you check for any sides effects please? I have now added Oscar's Tested-by: line and queued in for-4.18/rmi. In case any sideeffects are discovered, I'll either not include for-4.18/rmi in the push to Linus, or we'll do incremental fixups on top (depending on the nature of the side-effects :) ). Thanks,
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c index 9c9362149641..9e33165250a3 100644 --- a/drivers/hid/hid-rmi.c +++ b/drivers/hid/hid-rmi.c @@ -413,6 +413,24 @@ static int rmi_event(struct hid_device *hdev, struct hid_field *field, return 0; } +static void rmi_report(struct hid_device *hid, struct hid_report *report) +{ + struct hid_field *field = report->field[0]; + + if (!(hid->claimed & HID_CLAIMED_INPUT)) + return; + + switch (report->id) { + case RMI_READ_DATA_REPORT_ID: + /* fall-through */ + case RMI_ATTN_REPORT_ID: + return; + } + + if (field && field->hidinput && field->hidinput->input) + input_sync(field->hidinput->input); +} + #ifdef CONFIG_PM static int rmi_suspend(struct hid_device *hdev, pm_message_t message) { @@ -637,6 +655,7 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id) hid_set_drvdata(hdev, data); hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS; + hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC; ret = hid_parse(hdev); if (ret) { @@ -744,6 +763,7 @@ static struct hid_driver rmi_driver = { .remove = rmi_remove, .event = rmi_event, .raw_event = rmi_raw_event, + .report = rmi_report, .input_mapping = rmi_input_mapping, .input_configured = rmi_input_configured, #ifdef CONFIG_PM
When we receive a RMI4 report, we should not unconditionally send an input_sync event. Instead, we should let the rmi4 transport layer do it for us. This fixes a situation where we might receive X in a report and the rest in a subsequent one. And this messes up user space. Link: https://bugs.freedesktop.org/show_bug.cgi?id=100436 Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- Hi, Oscar, do you mind if we add your "Tested-by: Oscar Morante <your@email>"? Andrew, can you check for any sides effects please? Cheers, Benjamin drivers/hid/hid-rmi.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)