* [PATCH] input/hid: Add a new Saitek mouse device ID (RAT 9) @ 2016-07-30 11:53 Mayeul Cantan 2016-08-01 9:21 ` Benjamin Tissoires 0 siblings, 1 reply; 4+ messages in thread From: Mayeul Cantan @ 2016-07-30 11:53 UTC (permalink / raw) To: jikos, benjamin.tissoires; +Cc: linux-input, linux-kernel, Mayeul Cantan From: Mayeul Cantan <mayeul.cantan@gmail.com> The new device has 06a3:0cfa as identifiers, and the same quirks as the other RAT models. It needs this fix in order not to confuse the xorg server with its tristate button, which is reported as three different buttons, one of which is always on. This commit also fixes a small trailing whitespace issue in hid/hid-core.c Signed-off-by: Mayeul Cantan <mayeul.cantan@gmail.com> --- This patch is quite trivial, I am using it as a mean to try the submission process. As such, please don't hesitate to make any comment on both the form and substance. Best regards, MC drivers/hid/hid-core.c | 3 ++- drivers/hid/hid-ids.h | 1 + drivers/hid/hid-saitek.c | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 8ea3a26..f5b8fbf 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -2030,6 +2030,7 @@ static const struct hid_device_id hid_have_special_driver[] = { { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_PS1000) }, { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7_OLD) }, { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7) }, + { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT9) }, { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_MMO7) }, { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_RAT5) }, { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_RAT9) }, @@ -2289,7 +2290,7 @@ __ATTRIBUTE_GROUPS(hid_dev); static int hid_uevent(struct device *dev, struct kobj_uevent_env *env) { - struct hid_device *hdev = to_hid_device(dev); + struct hid_device *hdev = to_hid_device(dev); if (add_uevent_var(env, "HID_ID=%04X:%08X:%08X", hdev->bus, hdev->vendor, hdev->product)) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 3eec09a1..6db4079 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -849,6 +849,7 @@ #define USB_DEVICE_ID_SAITEK_PS1000 0x0621 #define USB_DEVICE_ID_SAITEK_RAT7_OLD 0x0ccb #define USB_DEVICE_ID_SAITEK_RAT7 0x0cd7 +#define USB_DEVICE_ID_SAITEK_RAT9 0x0cfa #define USB_DEVICE_ID_SAITEK_MMO7 0x0cd0 #define USB_VENDOR_ID_SAMSUNG 0x0419 diff --git a/drivers/hid/hid-saitek.c b/drivers/hid/hid-saitek.c index 2f84b26..39e6426 100644 --- a/drivers/hid/hid-saitek.c +++ b/drivers/hid/hid-saitek.c @@ -183,6 +183,8 @@ static const struct hid_device_id saitek_devices[] = { .driver_data = SAITEK_RELEASE_MODE_RAT7 }, { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7), .driver_data = SAITEK_RELEASE_MODE_RAT7 }, + { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT9), + .driver_data = SAITEK_RELEASE_MODE_RAT7 }, { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_RAT9), .driver_data = SAITEK_RELEASE_MODE_RAT7 }, { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_MMO7), -- 2.9.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] input/hid: Add a new Saitek mouse device ID (RAT 9) 2016-07-30 11:53 [PATCH] input/hid: Add a new Saitek mouse device ID (RAT 9) Mayeul Cantan @ 2016-08-01 9:21 ` Benjamin Tissoires 2016-08-02 8:48 ` Mayeul Cantan 2016-08-02 14:46 ` Jiri Kosina 0 siblings, 2 replies; 4+ messages in thread From: Benjamin Tissoires @ 2016-08-01 9:21 UTC (permalink / raw) To: Mayeul Cantan; +Cc: jikos, linux-input, linux-kernel Hi Mayeul, On Jul 30 2016 or thereabouts, Mayeul Cantan wrote: > From: Mayeul Cantan <mayeul.cantan@gmail.com> > > The new device has 06a3:0cfa as identifiers, and the same quirks as the > other RAT models. It needs this fix in order not to confuse the xorg server > with its tristate button, which is reported as three different buttons, one > of which is always on. > This commit also fixes a small trailing whitespace issue in hid/hid-core.c > > Signed-off-by: Mayeul Cantan <mayeul.cantan@gmail.com> > --- > This patch is quite trivial, I am using it as a mean to try the submission process. > As such, please don't hesitate to make any comment on both the form and substance. Substance looks good, and the form too :) I have a few nitpicks (given that you asked for those). But with them fixed or not, the patch is: Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > Best regards, > MC > > drivers/hid/hid-core.c | 3 ++- > drivers/hid/hid-ids.h | 1 + > drivers/hid/hid-saitek.c | 2 ++ > 3 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > index 8ea3a26..f5b8fbf 100644 > --- a/drivers/hid/hid-core.c > +++ b/drivers/hid/hid-core.c > @@ -2030,6 +2030,7 @@ static const struct hid_device_id hid_have_special_driver[] = { > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_PS1000) }, > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7_OLD) }, > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7) }, > + { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT9) }, > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_MMO7) }, > { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_RAT5) }, > { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_RAT9) }, > @@ -2289,7 +2290,7 @@ __ATTRIBUTE_GROUPS(hid_dev); > > static int hid_uevent(struct device *dev, struct kobj_uevent_env *env) > { > - struct hid_device *hdev = to_hid_device(dev); > + struct hid_device *hdev = to_hid_device(dev); Usually, if the change is not related to the patch, it needs to be in its own patch. The rational being that if we need to revert the patch, the cleanups won't. Here you are removing trailing whitespace, which is good but it's up to Jiri to take it as it this or amend the patch I guess :) > > if (add_uevent_var(env, "HID_ID=%04X:%08X:%08X", > hdev->bus, hdev->vendor, hdev->product)) > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index 3eec09a1..6db4079 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -849,6 +849,7 @@ > #define USB_DEVICE_ID_SAITEK_PS1000 0x0621 > #define USB_DEVICE_ID_SAITEK_RAT7_OLD 0x0ccb > #define USB_DEVICE_ID_SAITEK_RAT7 0x0cd7 > +#define USB_DEVICE_ID_SAITEK_RAT9 0x0cfa The ordering is weird here (not your fault though): The initial was wrong in the first place (_SAITEK_RAT7/0x0cd7 then _SAITEK_MMO7/0x0cd0). Then _SAITEK_RAT7_OLD was added, somewhat in a better place (by looking at the PID), and then you have to add yours... I think that's fine but at some point we will have to decide once for all the ordering of this file :) Cheers, Benjamin > #define USB_DEVICE_ID_SAITEK_MMO7 0x0cd0 > > #define USB_VENDOR_ID_SAMSUNG 0x0419 > diff --git a/drivers/hid/hid-saitek.c b/drivers/hid/hid-saitek.c > index 2f84b26..39e6426 100644 > --- a/drivers/hid/hid-saitek.c > +++ b/drivers/hid/hid-saitek.c > @@ -183,6 +183,8 @@ static const struct hid_device_id saitek_devices[] = { > .driver_data = SAITEK_RELEASE_MODE_RAT7 }, > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7), > .driver_data = SAITEK_RELEASE_MODE_RAT7 }, > + { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT9), > + .driver_data = SAITEK_RELEASE_MODE_RAT7 }, > { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_RAT9), > .driver_data = SAITEK_RELEASE_MODE_RAT7 }, > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_MMO7), > -- > 2.9.0 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] input/hid: Add a new Saitek mouse device ID (RAT 9) 2016-08-01 9:21 ` Benjamin Tissoires @ 2016-08-02 8:48 ` Mayeul Cantan 2016-08-02 14:46 ` Jiri Kosina 1 sibling, 0 replies; 4+ messages in thread From: Mayeul Cantan @ 2016-08-02 8:48 UTC (permalink / raw) To: Benjamin Tissoires; +Cc: jikos, linux-input, linux-kernel Hi Benjamin, 2016-08-01 11:21 GMT+02:00 Benjamin Tissoires <benjamin.tissoires@redhat.com>: > > Hi Mayeul, > > On Jul 30 2016 or thereabouts, Mayeul Cantan wrote: > > From: Mayeul Cantan <mayeul.cantan@gmail.com> > > > > The new device has 06a3:0cfa as identifiers, and the same quirks as the > > other RAT models. It needs this fix in order not to confuse the xorg server > > with its tristate button, which is reported as three different buttons, one > > of which is always on. > > This commit also fixes a small trailing whitespace issue in hid/hid-core.c > > > > Signed-off-by: Mayeul Cantan <mayeul.cantan@gmail.com> > > --- > > This patch is quite trivial, I am using it as a mean to try the submission process. > > As such, please don't hesitate to make any comment on both the form and substance. > > Substance looks good, and the form too :) Thank you, hopefully this is only the beginning of a longer series. I have some other patches in the works. > I have a few nitpicks (given that you asked for those). > But with them fixed or not, the patch is: > > Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > > Best regards, > > MC > > > > drivers/hid/hid-core.c | 3 ++- > > drivers/hid/hid-ids.h | 1 + > > drivers/hid/hid-saitek.c | 2 ++ > > 3 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c > > index 8ea3a26..f5b8fbf 100644 > > --- a/drivers/hid/hid-core.c > > +++ b/drivers/hid/hid-core.c > > @@ -2030,6 +2030,7 @@ static const struct hid_device_id hid_have_special_driver[] = { > > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_PS1000) }, > > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7_OLD) }, > > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7) }, > > + { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT9) }, > > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_MMO7) }, > > { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_RAT5) }, > > { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_RAT9) }, > > @@ -2289,7 +2290,7 @@ __ATTRIBUTE_GROUPS(hid_dev); > > > > static int hid_uevent(struct device *dev, struct kobj_uevent_env *env) > > { > > - struct hid_device *hdev = to_hid_device(dev); > > + struct hid_device *hdev = to_hid_device(dev); > > Usually, if the change is not related to the patch, it needs to be in > its own patch. The rational being that if we need to revert the patch, > the cleanups won't. > > Here you are removing trailing whitespace, which is good but it's up to > Jiri to take it as it this or amend the patch I guess :) Thanks, I hesitated a bit, about this one, but didn't see myself doing a separate commit for this fix (maybe I should?) nor just letting this in place. > > > > > if (add_uevent_var(env, "HID_ID=%04X:%08X:%08X", > > hdev->bus, hdev->vendor, hdev->product)) > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > > index 3eec09a1..6db4079 100644 > > --- a/drivers/hid/hid-ids.h > > +++ b/drivers/hid/hid-ids.h > > @@ -849,6 +849,7 @@ > > #define USB_DEVICE_ID_SAITEK_PS1000 0x0621 > > #define USB_DEVICE_ID_SAITEK_RAT7_OLD 0x0ccb > > #define USB_DEVICE_ID_SAITEK_RAT7 0x0cd7 > > +#define USB_DEVICE_ID_SAITEK_RAT9 0x0cfa > > The ordering is weird here (not your fault though): > The initial was wrong in the first place (_SAITEK_RAT7/0x0cd7 then > _SAITEK_MMO7/0x0cd0). Then _SAITEK_RAT7_OLD was added, somewhat in a > better place (by looking at the PID), and then you have to add yours... > > I think that's fine but at some point we will have to decide once for > all the ordering of this file :) I think the order for those devices was more of a chronological one (at some point, Mad Catz bough Saitek, hence the VID/branding change). That said, I am not even sure it stays consistent for those products. Having some guidelines here could definitely help. Thank you for taking time to review my patch. Best regards, Mayeul > Cheers, > Benjamin > > > > #define USB_DEVICE_ID_SAITEK_MMO7 0x0cd0 > > > > #define USB_VENDOR_ID_SAMSUNG 0x0419 > > diff --git a/drivers/hid/hid-saitek.c b/drivers/hid/hid-saitek.c > > index 2f84b26..39e6426 100644 > > --- a/drivers/hid/hid-saitek.c > > +++ b/drivers/hid/hid-saitek.c > > @@ -183,6 +183,8 @@ static const struct hid_device_id saitek_devices[] = { > > .driver_data = SAITEK_RELEASE_MODE_RAT7 }, > > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT7), > > .driver_data = SAITEK_RELEASE_MODE_RAT7 }, > > + { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RAT9), > > + .driver_data = SAITEK_RELEASE_MODE_RAT7 }, > > { HID_USB_DEVICE(USB_VENDOR_ID_MADCATZ, USB_DEVICE_ID_MADCATZ_RAT9), > > .driver_data = SAITEK_RELEASE_MODE_RAT7 }, > > { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_MMO7), > > -- > > 2.9.0 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] input/hid: Add a new Saitek mouse device ID (RAT 9) 2016-08-01 9:21 ` Benjamin Tissoires 2016-08-02 8:48 ` Mayeul Cantan @ 2016-08-02 14:46 ` Jiri Kosina 1 sibling, 0 replies; 4+ messages in thread From: Jiri Kosina @ 2016-08-02 14:46 UTC (permalink / raw) To: Benjamin Tissoires, Mayeul Cantan; +Cc: linux-input, linux-kernel On Mon, 1 Aug 2016, Benjamin Tissoires wrote: > > static int hid_uevent(struct device *dev, struct kobj_uevent_env *env) > > { > > - struct hid_device *hdev = to_hid_device(dev); > > + struct hid_device *hdev = to_hid_device(dev); > > Usually, if the change is not related to the patch, it needs to be in > its own patch. The rational being that if we need to revert the patch, > the cleanups won't. Yeah, let's fix this once someone is actually touching the code in question. I've removed this hunk, and applied the rest of the patch to for-4.8/upstream-fixes. Thanks, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-08-02 15:25 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-07-30 11:53 [PATCH] input/hid: Add a new Saitek mouse device ID (RAT 9) Mayeul Cantan 2016-08-01 9:21 ` Benjamin Tissoires 2016-08-02 8:48 ` Mayeul Cantan 2016-08-02 14:46 ` Jiri Kosina
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.