| Message ID | 1406119378-24551-3-git-send-email-spbnick@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> wrote: > Limit inverting the in-range bit in raw reports to tablet product ID > only. This will make adding handling of other, non-tablet products, > easier. > > Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com> > --- I am not particularly a big fan of this one. You are here adding a test which will be called at each raw_event but currently only tablet products are bound to hid-huion. Even in the rest of the series, you add another VID/PID, but it still has the same PID. So I would say that this will be nice to have when we really have the problem, not now. But if you tell me that you already have the need for it, I am fine with it. It's just that this commit message + the rest of the patch series makes me feel like this is just a superflous test. So, in its current state: NACK Cheers, Benjamin > drivers/hid/hid-huion.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c > index 25d01cd..db24472 100644 > --- a/drivers/hid/hid-huion.c > +++ b/drivers/hid/hid-huion.c > @@ -151,9 +151,15 @@ err: > static int huion_raw_event(struct hid_device *hdev, struct hid_report *report, > u8 *data, int size) > { > - /* If this is a pen input report then invert the in-range bit */ > - if (report->type == HID_INPUT_REPORT && report->id == 0x07 && size >= 2) > - data[1] ^= 0x40; > + switch (hdev->product) { > + case USB_DEVICE_ID_HUION_TABLET: > + /* If this is a pen input report */ > + if (report->type == HID_INPUT_REPORT && > + report->id == 0x07 && size >= 2) > + /* Invert the in-range bit */ > + data[1] ^= 0x40; > + break; > + } > > return 0; > } > -- > 2.0.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 -- 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 07/23/2014 05:34 PM, Benjamin Tissoires wrote: > On Wed, Jul 23, 2014 at 8:42 AM, Nikolai Kondrashov <spbnick@gmail.com> wrote: >> Limit inverting the in-range bit in raw reports to tablet product ID >> only. This will make adding handling of other, non-tablet products, >> easier. >> >> Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com> >> --- > > I am not particularly a big fan of this one. You are here adding a > test which will be called at each raw_event but currently only tablet > products are bound to hid-huion. Even in the rest of the series, you > add another VID/PID, but it still has the same PID. > > So I would say that this will be nice to have when we really have the > problem, not now. > > But if you tell me that you already have the need for it, I am fine > with it. It's just that this commit message + the rest of the patch > series makes me feel like this is just a superflous test. > > So, in its current state: > NACK I had doubts about this one myself, but left it in just for consistency with some other drivers. I'll drop it in the next version then. Thank you. Nick -- 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 everyone, This is the second version of the patch set adding support for more Huion tablets [1]. This version has the "hid: huion: Invert in-range on specific product" patch removed as requested by Benjamin Tissoires. Tested with Huion H610N. Nick [PATCH 1/4] hid: huion: Use "tablet" instead of specific model [PATCH 2/4] hid: huion: Don't ignore other interfaces [PATCH 3/4] hid: huion: Switch to generating report descriptor [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID drivers/hid/hid-core.c | 2 +- drivers/hid/hid-huion.c | 265 ++++++++++++++++++++++++++++++++---------------- drivers/hid/hid-ids.h | 2 +- 3 files changed, 177 insertions(+), 92 deletions(-) [1] http://thread.gmane.org/gmane.linux.kernel.input/37187 -- 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 Wed, Jul 23, 2014 at 12:31 PM, Nikolai Kondrashov <spbnick@gmail.com> wrote: > Hi everyone, > > This is the second version of the patch set adding support for more Huion > tablets [1]. > > This version has the "hid: huion: Invert in-range on specific product" patch > removed as requested by Benjamin Tissoires. > > Tested with Huion H610N. > > Nick > > [PATCH 1/4] hid: huion: Use "tablet" instead of specific model > [PATCH 2/4] hid: huion: Don't ignore other interfaces > [PATCH 3/4] hid: huion: Switch to generating report descriptor > [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID > > drivers/hid/hid-core.c | 2 +- > drivers/hid/hid-huion.c | 265 ++++++++++++++++++++++++++++++++---------------- > drivers/hid/hid-ids.h | 2 +- > 3 files changed, 177 insertions(+), 92 deletions(-) > > [1] http://thread.gmane.org/gmane.linux.kernel.input/37187 Just so that the info does not get lost, the v2 of the patch series is still: 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 Mon, 28 Jul 2014, Benjamin Tissoires wrote: > > This is the second version of the patch set adding support for more Huion > > tablets [1]. > > > > This version has the "hid: huion: Invert in-range on specific product" patch > > removed as requested by Benjamin Tissoires. > > > > Tested with Huion H610N. > > > > Nick > > > > [PATCH 1/4] hid: huion: Use "tablet" instead of specific model > > [PATCH 2/4] hid: huion: Don't ignore other interfaces > > [PATCH 3/4] hid: huion: Switch to generating report descriptor > > [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID > > > > drivers/hid/hid-core.c | 2 +- > > drivers/hid/hid-huion.c | 265 ++++++++++++++++++++++++++++++++---------------- > > drivers/hid/hid-ids.h | 2 +- > > 3 files changed, 177 insertions(+), 92 deletions(-) > > > > [1] http://thread.gmane.org/gmane.linux.kernel.input/37187 > > Just so that the info does not get lost, the v2 of the patch series is still: > > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Queuing for 3.17, thanks.
diff --git a/drivers/hid/hid-huion.c b/drivers/hid/hid-huion.c index 25d01cd..db24472 100644 --- a/drivers/hid/hid-huion.c +++ b/drivers/hid/hid-huion.c @@ -151,9 +151,15 @@ err: static int huion_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, int size) { - /* If this is a pen input report then invert the in-range bit */ - if (report->type == HID_INPUT_REPORT && report->id == 0x07 && size >= 2) - data[1] ^= 0x40; + switch (hdev->product) { + case USB_DEVICE_ID_HUION_TABLET: + /* If this is a pen input report */ + if (report->type == HID_INPUT_REPORT && + report->id == 0x07 && size >= 2) + /* Invert the in-range bit */ + data[1] ^= 0x40; + break; + } return 0; }
Limit inverting the in-range bit in raw reports to tablet product ID only. This will make adding handling of other, non-tablet products, easier. Signed-off-by: Nikolai Kondrashov <spbnick@gmail.com> --- drivers/hid/hid-huion.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)