| Message ID | 1411495768-5956-1-git-send-email-killertofu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Tue, Sep 23, 2014 at 11:09 AM, Jason Gerecke <killertofu@gmail.com> wrote: > The commit which introduced TransducerSerialNumber (368c966) is missing > two crucial implementation details. Firstly, the commit does not set the > type/code/bit/max fields as expected later down the code which can cause > the driver to crash when a tablet with this usage is connected. Secondly, > the call to 'set_bit' causes MSC_PULSELED to be sent instead of the > expected MSC_SERIAL. This commit addreses both issues. > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > --- > drivers/hid/hid-input.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c > index 2619f7f..cb1b3fa 100644 > --- a/drivers/hid/hid-input.c > +++ b/drivers/hid/hid-input.c > @@ -689,7 +689,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel > break; > > case 0x5b: /* TransducerSerialNumber */ > - set_bit(MSC_SERIAL, input->mscbit); > + usage->type = EV_MSC; > + usage->code = MSC_SERIAL; > + bit = input->mscbit; > + max = MSC_MAX; > break; > > default: goto unknown; > -- > 2.1.0 > This patch still seems to be in limbo. Anyone (Ping? Benjamin?) care to provide an ACK or review? Jason --- Now instead of four in the eights place / you’ve got three, ‘Cause you added one / (That is to say, eight) to the two, / But you can’t take seven from three, / So you look at the sixty-fours.... -- 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 Tue, Sep 23, 2014 at 11:09 AM, Jason Gerecke <killertofu@gmail.com> wrote: > > The commit which introduced TransducerSerialNumber (368c966) is missing > two crucial implementation details. Firstly, the commit does not set the > type/code/bit/max fields as expected later down the code which can cause > the driver to crash when a tablet with this usage is connected. Secondly, > the call to 'set_bit' causes MSC_PULSELED to be sent instead of the > expected MSC_SERIAL. This commit addreses both issues. > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> Reviewed-by: Ping Cheng <pingc@wacom.com> Thank you, Jason, for the fix. Ping > --- > drivers/hid/hid-input.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c > index 2619f7f..cb1b3fa 100644 > --- a/drivers/hid/hid-input.c > +++ b/drivers/hid/hid-input.c > @@ -689,7 +689,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel > break; > > case 0x5b: /* TransducerSerialNumber */ > - set_bit(MSC_SERIAL, input->mscbit); > + usage->type = EV_MSC; > + usage->code = MSC_SERIAL; > + bit = input->mscbit; > + max = MSC_MAX; > break; > > default: goto unknown; > -- > 2.1.0 > -- 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 Oct 28 2014 or thereabouts, Jason Gerecke wrote: > On Tue, Sep 23, 2014 at 11:09 AM, Jason Gerecke <killertofu@gmail.com> wrote: > > The commit which introduced TransducerSerialNumber (368c966) is missing > > two crucial implementation details. Firstly, the commit does not set the > > type/code/bit/max fields as expected later down the code which can cause > > the driver to crash when a tablet with this usage is connected. Secondly, > > the call to 'set_bit' causes MSC_PULSELED to be sent instead of the > > expected MSC_SERIAL. This commit addreses both issues. > > > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > > --- > > drivers/hid/hid-input.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c > > index 2619f7f..cb1b3fa 100644 > > --- a/drivers/hid/hid-input.c > > +++ b/drivers/hid/hid-input.c > > @@ -689,7 +689,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel > > break; > > > > case 0x5b: /* TransducerSerialNumber */ > > - set_bit(MSC_SERIAL, input->mscbit); > > + usage->type = EV_MSC; > > + usage->code = MSC_SERIAL; > > + bit = input->mscbit; > > + max = MSC_MAX; > > break; > > > > default: goto unknown; > > -- > > 2.1.0 > > > > This patch still seems to be in limbo. Anyone (Ping? Benjamin?) care > to provide an ACK or review? > Sorry for that, I was persuaded I already have answered to this one. The patch makes sense, but I think it will be better to extend hid_map_usage() with MSC bits and define map_msc_clear(). The code will be more consistent with the rest this way. Anyway, if Jiri does not find this to be mandatory, then this code is: Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Cheers, Benjamin -- 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 Tue, 23 Sep 2014, Jason Gerecke wrote: > The commit which introduced TransducerSerialNumber (368c966) is missing > two crucial implementation details. Firstly, the commit does not set the > type/code/bit/max fields as expected later down the code which can cause > the driver to crash when a tablet with this usage is connected. Secondly, > the call to 'set_bit' causes MSC_PULSELED to be sent instead of the > expected MSC_SERIAL. This commit addreses both issues. > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > --- > drivers/hid/hid-input.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c > index 2619f7f..cb1b3fa 100644 > --- a/drivers/hid/hid-input.c > +++ b/drivers/hid/hid-input.c > @@ -689,7 +689,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel > break; > > case 0x5b: /* TransducerSerialNumber */ > - set_bit(MSC_SERIAL, input->mscbit); > + usage->type = EV_MSC; > + usage->code = MSC_SERIAL; > + bit = input->mscbit; > + max = MSC_MAX; > break; > Apologizes for the delay. I have now queued this for 3.18 still.
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 2619f7f..cb1b3fa 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -689,7 +689,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel break; case 0x5b: /* TransducerSerialNumber */ - set_bit(MSC_SERIAL, input->mscbit); + usage->type = EV_MSC; + usage->code = MSC_SERIAL; + bit = input->mscbit; + max = MSC_MAX; break; default: goto unknown;
The commit which introduced TransducerSerialNumber (368c966) is missing two crucial implementation details. Firstly, the commit does not set the type/code/bit/max fields as expected later down the code which can cause the driver to crash when a tablet with this usage is connected. Secondly, the call to 'set_bit' causes MSC_PULSELED to be sent instead of the expected MSC_SERIAL. This commit addreses both issues. Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> --- drivers/hid/hid-input.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)