* [PATCH 0/1] Input: joydev blacklist feature @ 2017-10-06 1:03 Roderick Colenbrander 2017-10-06 1:03 ` [PATCH 1/1] Input: joydev - blacklist ds3/ds4/udraw motion sensors Roderick Colenbrander 0 siblings, 1 reply; 6+ messages in thread From: Roderick Colenbrander @ 2017-10-06 1:03 UTC (permalink / raw) To: linux-input Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Roderick Colenbrander From: Roderick Colenbrander <roderick.colenbrander@sony.com> Hi, The joydev changes introduced by this patch are the result of a discussion on linux-input about handling of features in newer devices, which classic joydev applications may not pick up and confuse. The original motivation was dualshock 3 /4 and thq udraw devices, which expose motion sensors on a subdevice. These got picked up by joydev and confused older applications and users and has been a regression since 4.12. The original patch proposed to filter out motion sensor capability from joydev, but this was 'too wide' of a filter. Ultimately Dmitry suggested for joydev to reject certain devices. This patch introduces an initial blacklist mechanism for joydev. It is used to filter out the motion sensor subdevice on dualshock 3 / 4 and thq udraw. We hope this feature could still make it into 4.14 to minimize impact on our users as this will be a long LTS release. Thanks, Roderick Roderick Colenbrander (1): Input: joydev - blacklist ds3/ds4/udraw motion sensors drivers/input/joydev.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) -- 2.13.6 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] Input: joydev - blacklist ds3/ds4/udraw motion sensors 2017-10-06 1:03 [PATCH 0/1] Input: joydev blacklist feature Roderick Colenbrander @ 2017-10-06 1:03 ` Roderick Colenbrander 2017-10-07 18:44 ` Dmitry Torokhov 0 siblings, 1 reply; 6+ messages in thread From: Roderick Colenbrander @ 2017-10-06 1:03 UTC (permalink / raw) To: linux-input Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Roderick Colenbrander, stable From: Roderick Colenbrander <roderick.colenbrander@sony.com> Introduce a device table used for blacklisting devices. We currently blacklist the motion sensor subdevice of THQ Udraw and Sony ds3/ds4. Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> Cc: stable@vger.kernel.org --- drivers/input/joydev.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c index 29d677c714d2..e2a0f63d5656 100644 --- a/drivers/input/joydev.c +++ b/drivers/input/joydev.c @@ -70,6 +70,40 @@ struct joydev_client { struct list_head node; }; +/* These codes are copied from from hid-ids.h, unfortunately there is no common + * usb_ids/bt_ids.h header. + */ +#define USB_VENDOR_ID_SONY 0x54c +#define USB_VENDOR_ID_THQ 0x20d6 +#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268 +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER 0x05c4 +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER_2 0x09cc +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE 0x0ba0 +#define USB_DEVICE_ID_THQ_PS3_UDRAW 0xcb17 + +/* List of devices blacklisted from joydev. A reason for blacklisting a + * device is to support (legacy) software supporting joydev, but which will + * never get updated to support these devices or features. An example would + * be handling of motion sensors, which these applications could not handle + * resulting in undefined behavior. + */ +static const struct joydev_blacklist { + __u16 bustype; + __u16 vendor; + __u16 product; + unsigned long propbit; /* Allow for filtering based on device properties. */ +} joydev_blacklist[] = { + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2, BIT(INPUT_PROP_ACCELEROMETER) }, + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2, BIT(INPUT_PROP_ACCELEROMETER) }, + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE, BIT(INPUT_PROP_ACCELEROMETER) }, + { BUS_USB, USB_VENDOR_ID_THQ, USB_DEVICE_ID_THQ_PS3_UDRAW, BIT(INPUT_PROP_ACCELEROMETER) }, + { 0, 0, 0, 0} +}; + static int joydev_correct(int value, struct js_corr *corr) { switch (corr->type) { @@ -805,6 +839,25 @@ static bool joydev_dev_is_absolute_mouse(struct input_dev *dev) return true; } + +static bool joydev_dev_is_blacklisted(struct input_dev *dev) +{ + int i; + + for (i = 0; joydev_blacklist[i].vendor; i++) { + if (dev->id.bustype == joydev_blacklist[i].bustype && + dev->id.vendor == joydev_blacklist[i].vendor && + dev->id.product == joydev_blacklist[i].product && + dev->propbit[0] == joydev_blacklist[i].propbit) + { + dev_info(&dev->dev, "joydev: Blacklisting '%s'\n", dev->name); + return true; + } + } + + return false; +} + static bool joydev_match(struct input_handler *handler, struct input_dev *dev) { /* Avoid touchpads and touchscreens */ @@ -819,6 +872,10 @@ static bool joydev_match(struct input_handler *handler, struct input_dev *dev) if (joydev_dev_is_absolute_mouse(dev)) return false; + /* Disable blacklisted devices */ + if (joydev_dev_is_blacklisted(dev)) + return false; + return true; } -- 2.13.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Input: joydev - blacklist ds3/ds4/udraw motion sensors 2017-10-06 1:03 ` [PATCH 1/1] Input: joydev - blacklist ds3/ds4/udraw motion sensors Roderick Colenbrander @ 2017-10-07 18:44 ` Dmitry Torokhov 2017-10-11 0:53 ` Colenbrander, Roelof 0 siblings, 1 reply; 6+ messages in thread From: Dmitry Torokhov @ 2017-10-07 18:44 UTC (permalink / raw) To: Roderick Colenbrander Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Roderick Colenbrander, stable Hi Roderick, On Thu, Oct 05, 2017 at 06:03:33PM -0700, Roderick Colenbrander wrote: > From: Roderick Colenbrander <roderick.colenbrander@sony.com> > > Introduce a device table used for blacklisting devices. We currently > blacklist the motion sensor subdevice of THQ Udraw and Sony ds3/ds4. > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> > Cc: stable@vger.kernel.org > --- > drivers/input/joydev.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 57 insertions(+) > > diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c > index 29d677c714d2..e2a0f63d5656 100644 > --- a/drivers/input/joydev.c > +++ b/drivers/input/joydev.c > @@ -70,6 +70,40 @@ struct joydev_client { > struct list_head node; > }; > > +/* These codes are copied from from hid-ids.h, unfortunately there is no common > + * usb_ids/bt_ids.h header. > + */ > +#define USB_VENDOR_ID_SONY 0x54c > +#define USB_VENDOR_ID_THQ 0x20d6 > +#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268 > +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER 0x05c4 > +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER_2 0x09cc > +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE 0x0ba0 > +#define USB_DEVICE_ID_THQ_PS3_UDRAW 0xcb17 > + > +/* List of devices blacklisted from joydev. A reason for blacklisting a > + * device is to support (legacy) software supporting joydev, but which will > + * never get updated to support these devices or features. An example would > + * be handling of motion sensors, which these applications could not handle > + * resulting in undefined behavior. > + */ > +static const struct joydev_blacklist { > + __u16 bustype; > + __u16 vendor; > + __u16 product; > + unsigned long propbit; /* Allow for filtering based on device properties. */ > +} joydev_blacklist[] = { > + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_USB, USB_VENDOR_ID_THQ, USB_DEVICE_ID_THQ_PS3_UDRAW, BIT(INPUT_PROP_ACCELEROMETER) }, > + { 0, 0, 0, 0} I would assume that regardless of connection (USB, BT, SPI, I2C, the accelerometer in PS4 controller should not be handled by joydev. I wonder if we should not factor out input device id matching from input_match_device(), add propbit handling to input device id and use it here? Then your blacklist would be: #define ACCEL_DEV(vendor, product) \ { \ .flags = INPUT_DEVICE_ID_MATCH_VENDOR | \ INPUT_DEVICE_ID_MATCH_PRODUCT | \ INPUT_DEVICE_ID_MATCH_PROPBIT, \ .vendor = (vendor), \ .product = (product), \ .propbit[0] = BIT(INPUT_PROP_ACCELEROMETER), \ } static const struct input_device_id joydev_blacklist { ACCEL_DEV(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER), ACCEL_DEV(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER), ... { } }; > +}; > + > static int joydev_correct(int value, struct js_corr *corr) > { > switch (corr->type) { > @@ -805,6 +839,25 @@ static bool joydev_dev_is_absolute_mouse(struct input_dev *dev) > return true; > } > > + > +static bool joydev_dev_is_blacklisted(struct input_dev *dev) > +{ > + int i; > + > + for (i = 0; joydev_blacklist[i].vendor; i++) { > + if (dev->id.bustype == joydev_blacklist[i].bustype && > + dev->id.vendor == joydev_blacklist[i].vendor && > + dev->id.product == joydev_blacklist[i].product && > + dev->propbit[0] == joydev_blacklist[i].propbit) You probably want "&" and not strict match? > + { > + dev_info(&dev->dev, "joydev: Blacklisting '%s'\n", dev->name); > + return true; > + } > + } > + > + return false; > +} > + > static bool joydev_match(struct input_handler *handler, struct input_dev *dev) > { > /* Avoid touchpads and touchscreens */ > @@ -819,6 +872,10 @@ static bool joydev_match(struct input_handler *handler, struct input_dev *dev) > if (joydev_dev_is_absolute_mouse(dev)) > return false; > > + /* Disable blacklisted devices */ > + if (joydev_dev_is_blacklisted(dev)) > + return false; > + > return true; > } > > -- > 2.13.6 > Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 1/1] Input: joydev - blacklist ds3/ds4/udraw motion sensors 2017-10-07 18:44 ` Dmitry Torokhov @ 2017-10-11 0:53 ` Colenbrander, Roelof 2017-10-13 23:50 ` Dmitry Torokhov 0 siblings, 1 reply; 6+ messages in thread From: Colenbrander, Roelof @ 2017-10-11 0:53 UTC (permalink / raw) To: Dmitry Torokhov, roderick@gaikai.com Cc: linux-input@vger.kernel.org, Benjamin Tissoires, Jiri Kosina, stable@vger.kernel.org Hi Dmitry, Thanks for your suggestions the feedback looks good. Just got back to office after a long weekend, then was surprised to see you already made a few changes to the series. The changes look. Thanks for the assistance! Thanks, Roderick ________________________________________ From: Dmitry Torokhov [dmitry.torokhov@gmail.com] Sent: Saturday, October 07, 2017 11:44 AM To: roderick@gaikai.com Cc: linux-input@vger.kernel.org; Benjamin Tissoires; Jiri Kosina; Colenbrander, Roelof; stable@vger.kernel.org Subject: Re: [PATCH 1/1] Input: joydev - blacklist ds3/ds4/udraw motion sensors Hi Roderick, On Thu, Oct 05, 2017 at 06:03:33PM -0700, Roderick Colenbrander wrote: > From: Roderick Colenbrander <roderick.colenbrander@sony.com> > > Introduce a device table used for blacklisting devices. We currently > blacklist the motion sensor subdevice of THQ Udraw and Sony ds3/ds4. > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> > Cc: stable@vger.kernel.org > --- > drivers/input/joydev.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 57 insertions(+) > > diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c > index 29d677c714d2..e2a0f63d5656 100644 > --- a/drivers/input/joydev.c > +++ b/drivers/input/joydev.c > @@ -70,6 +70,40 @@ struct joydev_client { > struct list_head node; > }; > > +/* These codes are copied from from hid-ids.h, unfortunately there is no common > + * usb_ids/bt_ids.h header. > + */ > +#define USB_VENDOR_ID_SONY 0x54c > +#define USB_VENDOR_ID_THQ 0x20d6 > +#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268 > +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER 0x05c4 > +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER_2 0x09cc > +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE 0x0ba0 > +#define USB_DEVICE_ID_THQ_PS3_UDRAW 0xcb17 > + > +/* List of devices blacklisted from joydev. A reason for blacklisting a > + * device is to support (legacy) software supporting joydev, but which will > + * never get updated to support these devices or features. An example would > + * be handling of motion sensors, which these applications could not handle > + * resulting in undefined behavior. > + */ > +static const struct joydev_blacklist { > + __u16 bustype; > + __u16 vendor; > + __u16 product; > + unsigned long propbit; /* Allow for filtering based on device properties. */ > +} joydev_blacklist[] = { > + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE, BIT(INPUT_PROP_ACCELEROMETER) }, > + { BUS_USB, USB_VENDOR_ID_THQ, USB_DEVICE_ID_THQ_PS3_UDRAW, BIT(INPUT_PROP_ACCELEROMETER) }, > + { 0, 0, 0, 0} I would assume that regardless of connection (USB, BT, SPI, I2C, the accelerometer in PS4 controller should not be handled by joydev. I wonder if we should not factor out input device id matching from input_match_device(), add propbit handling to input device id and use it here? Then your blacklist would be: #define ACCEL_DEV(vendor, product) \ { \ .flags = INPUT_DEVICE_ID_MATCH_VENDOR | \ INPUT_DEVICE_ID_MATCH_PRODUCT | \ INPUT_DEVICE_ID_MATCH_PROPBIT, \ .vendor = (vendor), \ .product = (product), \ .propbit[0] = BIT(INPUT_PROP_ACCELEROMETER), \ } static const struct input_device_id joydev_blacklist { ACCEL_DEV(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER), ACCEL_DEV(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER), ... { } }; > +}; > + > static int joydev_correct(int value, struct js_corr *corr) > { > switch (corr->type) { > @@ -805,6 +839,25 @@ static bool joydev_dev_is_absolute_mouse(struct input_dev *dev) > return true; > } > > + > +static bool joydev_dev_is_blacklisted(struct input_dev *dev) > +{ > + int i; > + > + for (i = 0; joydev_blacklist[i].vendor; i++) { > + if (dev->id.bustype == joydev_blacklist[i].bustype && > + dev->id.vendor == joydev_blacklist[i].vendor && > + dev->id.product == joydev_blacklist[i].product && > + dev->propbit[0] == joydev_blacklist[i].propbit) You probably want "&" and not strict match? > + { > + dev_info(&dev->dev, "joydev: Blacklisting '%s'\n", dev->name); > + return true; > + } > + } > + > + return false; > +} > + > static bool joydev_match(struct input_handler *handler, struct input_dev *dev) > { > /* Avoid touchpads and touchscreens */ > @@ -819,6 +872,10 @@ static bool joydev_match(struct input_handler *handler, struct input_dev *dev) > if (joydev_dev_is_absolute_mouse(dev)) > return false; > > + /* Disable blacklisted devices */ > + if (joydev_dev_is_blacklisted(dev)) > + return false; > + > return true; > } > > -- > 2.13.6 > Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Input: joydev - blacklist ds3/ds4/udraw motion sensors 2017-10-11 0:53 ` Colenbrander, Roelof @ 2017-10-13 23:50 ` Dmitry Torokhov 2017-10-17 0:48 ` Colenbrander, Roelof 0 siblings, 1 reply; 6+ messages in thread From: Dmitry Torokhov @ 2017-10-13 23:50 UTC (permalink / raw) To: Colenbrander, Roelof Cc: roderick@gaikai.com, linux-input@vger.kernel.org, Benjamin Tissoires, Jiri Kosina, stable@vger.kernel.org Hi Roderick, On Wed, Oct 11, 2017 at 12:53:53AM +0000, Colenbrander, Roelof wrote: > Hi Dmitry, > > Thanks for your suggestions the feedback looks good. Just got back to office after a long weekend, then was surprised to see you already made a few changes to the series. The changes look. Thanks for the assistance! > OK, if that works for you then I will queue it up. Thanks. > Thanks, > Roderick > ________________________________________ > From: Dmitry Torokhov [dmitry.torokhov@gmail.com] > Sent: Saturday, October 07, 2017 11:44 AM > To: roderick@gaikai.com > Cc: linux-input@vger.kernel.org; Benjamin Tissoires; Jiri Kosina; Colenbrander, Roelof; stable@vger.kernel.org > Subject: Re: [PATCH 1/1] Input: joydev - blacklist ds3/ds4/udraw motion sensors > > Hi Roderick, > > On Thu, Oct 05, 2017 at 06:03:33PM -0700, Roderick Colenbrander wrote: > > From: Roderick Colenbrander <roderick.colenbrander@sony.com> > > > > Introduce a device table used for blacklisting devices. We currently > > blacklist the motion sensor subdevice of THQ Udraw and Sony ds3/ds4. > > > > Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> > > Cc: stable@vger.kernel.org > > --- > > drivers/input/joydev.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 57 insertions(+) > > > > diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c > > index 29d677c714d2..e2a0f63d5656 100644 > > --- a/drivers/input/joydev.c > > +++ b/drivers/input/joydev.c > > @@ -70,6 +70,40 @@ struct joydev_client { > > struct list_head node; > > }; > > > > +/* These codes are copied from from hid-ids.h, unfortunately there is no common > > + * usb_ids/bt_ids.h header. > > + */ > > +#define USB_VENDOR_ID_SONY 0x54c > > +#define USB_VENDOR_ID_THQ 0x20d6 > > +#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268 > > +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER 0x05c4 > > +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER_2 0x09cc > > +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE 0x0ba0 > > +#define USB_DEVICE_ID_THQ_PS3_UDRAW 0xcb17 > > + > > +/* List of devices blacklisted from joydev. A reason for blacklisting a > > + * device is to support (legacy) software supporting joydev, but which will > > + * never get updated to support these devices or features. An example would > > + * be handling of motion sensors, which these applications could not handle > > + * resulting in undefined behavior. > > + */ > > +static const struct joydev_blacklist { > > + __u16 bustype; > > + __u16 vendor; > > + __u16 product; > > + unsigned long propbit; /* Allow for filtering based on device properties. */ > > +} joydev_blacklist[] = { > > + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > > + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > > + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2, BIT(INPUT_PROP_ACCELEROMETER) }, > > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, > > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2, BIT(INPUT_PROP_ACCELEROMETER) }, > > + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE, BIT(INPUT_PROP_ACCELEROMETER) }, > > + { BUS_USB, USB_VENDOR_ID_THQ, USB_DEVICE_ID_THQ_PS3_UDRAW, BIT(INPUT_PROP_ACCELEROMETER) }, > > + { 0, 0, 0, 0} > > I would assume that regardless of connection (USB, BT, SPI, I2C, the > accelerometer in PS4 controller should not be handled by joydev. I > wonder if we should not factor out input device id matching from > input_match_device(), add propbit handling to input device id and use it > here? Then your blacklist would be: > > #define ACCEL_DEV(vendor, product) \ > { \ > .flags = INPUT_DEVICE_ID_MATCH_VENDOR | \ > INPUT_DEVICE_ID_MATCH_PRODUCT | \ > INPUT_DEVICE_ID_MATCH_PROPBIT, \ > .vendor = (vendor), \ > .product = (product), \ > .propbit[0] = BIT(INPUT_PROP_ACCELEROMETER), \ > } > > static const struct input_device_id joydev_blacklist { > ACCEL_DEV(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER), > ACCEL_DEV(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER), > ... > { } > }; > > > +}; > > + > > static int joydev_correct(int value, struct js_corr *corr) > > { > > switch (corr->type) { > > @@ -805,6 +839,25 @@ static bool joydev_dev_is_absolute_mouse(struct input_dev *dev) > > return true; > > } > > > > + > > +static bool joydev_dev_is_blacklisted(struct input_dev *dev) > > +{ > > + int i; > > + > > + for (i = 0; joydev_blacklist[i].vendor; i++) { > > + if (dev->id.bustype == joydev_blacklist[i].bustype && > > + dev->id.vendor == joydev_blacklist[i].vendor && > > + dev->id.product == joydev_blacklist[i].product && > > + dev->propbit[0] == joydev_blacklist[i].propbit) > > You probably want "&" and not strict match? > > > + { > > + dev_info(&dev->dev, "joydev: Blacklisting '%s'\n", dev->name); > > + return true; > > + } > > + } > > + > > + return false; > > +} > > + > > static bool joydev_match(struct input_handler *handler, struct input_dev *dev) > > { > > /* Avoid touchpads and touchscreens */ > > @@ -819,6 +872,10 @@ static bool joydev_match(struct input_handler *handler, struct input_dev *dev) > > if (joydev_dev_is_absolute_mouse(dev)) > > return false; > > > > + /* Disable blacklisted devices */ > > + if (joydev_dev_is_blacklisted(dev)) > > + return false; > > + > > return true; > > } > > > > -- > > 2.13.6 > > > > Thanks. > > -- > Dmitry > -- Dmitry ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Input: joydev - blacklist ds3/ds4/udraw motion sensors 2017-10-13 23:50 ` Dmitry Torokhov @ 2017-10-17 0:48 ` Colenbrander, Roelof 0 siblings, 0 replies; 6+ messages in thread From: Colenbrander, Roelof @ 2017-10-17 0:48 UTC (permalink / raw) To: Dmitry Torokhov Cc: roderick@gaikai.com, linux-input@vger.kernel.org, Benjamin Tissoires, Jiri Kosina, stable@vger.kernel.org Hi Dmitry, I would like to confirm that the changes are working correctly. The code looked code before, but hadn't validated the changes yet. We did so today and things work accordingly, so I think it is ready to merge. Thanks for the help! Thanks, Roderick On 10/13/2017 04:50 PM, Dmitry Torokhov wrote: > Hi Roderick, > > On Wed, Oct 11, 2017 at 12:53:53AM +0000, Colenbrander, Roelof wrote: >> Hi Dmitry, >> >> Thanks for your suggestions the feedback looks good. Just got back to office after a long weekend, then was surprised to see you already made a few changes to the series. The changes look. Thanks for the assistance! >> > > OK, if that works for you then I will queue it up. > > Thanks. > >> Thanks, >> Roderick >> ________________________________________ >> From: Dmitry Torokhov [dmitry.torokhov@gmail.com] >> Sent: Saturday, October 07, 2017 11:44 AM >> To: roderick@gaikai.com >> Cc: linux-input@vger.kernel.org; Benjamin Tissoires; Jiri Kosina; Colenbrander, Roelof; stable@vger.kernel.org >> Subject: Re: [PATCH 1/1] Input: joydev - blacklist ds3/ds4/udraw motion sensors >> >> Hi Roderick, >> >> On Thu, Oct 05, 2017 at 06:03:33PM -0700, Roderick Colenbrander wrote: >>> From: Roderick Colenbrander <roderick.colenbrander@sony.com> >>> >>> Introduce a device table used for blacklisting devices. We currently >>> blacklist the motion sensor subdevice of THQ Udraw and Sony ds3/ds4. >>> >>> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com> >>> Cc: stable@vger.kernel.org >>> --- >>> drivers/input/joydev.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 57 insertions(+) >>> >>> diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c >>> index 29d677c714d2..e2a0f63d5656 100644 >>> --- a/drivers/input/joydev.c >>> +++ b/drivers/input/joydev.c >>> @@ -70,6 +70,40 @@ struct joydev_client { >>> struct list_head node; >>> }; >>> >>> +/* These codes are copied from from hid-ids.h, unfortunately there is no common >>> + * usb_ids/bt_ids.h header. >>> + */ >>> +#define USB_VENDOR_ID_SONY 0x54c >>> +#define USB_VENDOR_ID_THQ 0x20d6 >>> +#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268 >>> +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER 0x05c4 >>> +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER_2 0x09cc >>> +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE 0x0ba0 >>> +#define USB_DEVICE_ID_THQ_PS3_UDRAW 0xcb17 >>> + >>> +/* List of devices blacklisted from joydev. A reason for blacklisting a >>> + * device is to support (legacy) software supporting joydev, but which will >>> + * never get updated to support these devices or features. An example would >>> + * be handling of motion sensors, which these applications could not handle >>> + * resulting in undefined behavior. >>> + */ >>> +static const struct joydev_blacklist { >>> + __u16 bustype; >>> + __u16 vendor; >>> + __u16 product; >>> + unsigned long propbit; /* Allow for filtering based on device properties. */ >>> +} joydev_blacklist[] = { >>> + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, >>> + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, >>> + { BUS_BLUETOOTH, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2, BIT(INPUT_PROP_ACCELEROMETER) }, >>> + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, >>> + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER, BIT(INPUT_PROP_ACCELEROMETER) }, >>> + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_2, BIT(INPUT_PROP_ACCELEROMETER) }, >>> + { BUS_USB, USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER_DONGLE, BIT(INPUT_PROP_ACCELEROMETER) }, >>> + { BUS_USB, USB_VENDOR_ID_THQ, USB_DEVICE_ID_THQ_PS3_UDRAW, BIT(INPUT_PROP_ACCELEROMETER) }, >>> + { 0, 0, 0, 0} >> >> I would assume that regardless of connection (USB, BT, SPI, I2C, the >> accelerometer in PS4 controller should not be handled by joydev. I >> wonder if we should not factor out input device id matching from >> input_match_device(), add propbit handling to input device id and use it >> here? Then your blacklist would be: >> >> #define ACCEL_DEV(vendor, product) \ >> { \ >> .flags = INPUT_DEVICE_ID_MATCH_VENDOR | \ >> INPUT_DEVICE_ID_MATCH_PRODUCT | \ >> INPUT_DEVICE_ID_MATCH_PROPBIT, \ >> .vendor = (vendor), \ >> .product = (product), \ >> .propbit[0] = BIT(INPUT_PROP_ACCELEROMETER), \ >> } >> >> static const struct input_device_id joydev_blacklist { >> ACCEL_DEV(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER), >> ACCEL_DEV(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER), >> ... >> { } >> }; >> >>> +}; >>> + >>> static int joydev_correct(int value, struct js_corr *corr) >>> { >>> switch (corr->type) { >>> @@ -805,6 +839,25 @@ static bool joydev_dev_is_absolute_mouse(struct input_dev *dev) >>> return true; >>> } >>> >>> + >>> +static bool joydev_dev_is_blacklisted(struct input_dev *dev) >>> +{ >>> + int i; >>> + >>> + for (i = 0; joydev_blacklist[i].vendor; i++) { >>> + if (dev->id.bustype == joydev_blacklist[i].bustype && >>> + dev->id.vendor == joydev_blacklist[i].vendor && >>> + dev->id.product == joydev_blacklist[i].product && >>> + dev->propbit[0] == joydev_blacklist[i].propbit) >> >> You probably want "&" and not strict match? >> >>> + { >>> + dev_info(&dev->dev, "joydev: Blacklisting '%s'\n", dev->name); >>> + return true; >>> + } >>> + } >>> + >>> + return false; >>> +} >>> + >>> static bool joydev_match(struct input_handler *handler, struct input_dev *dev) >>> { >>> /* Avoid touchpads and touchscreens */ >>> @@ -819,6 +872,10 @@ static bool joydev_match(struct input_handler *handler, struct input_dev *dev) >>> if (joydev_dev_is_absolute_mouse(dev)) >>> return false; >>> >>> + /* Disable blacklisted devices */ >>> + if (joydev_dev_is_blacklisted(dev)) >>> + return false; >>> + >>> return true; >>> } >>> >>> -- >>> 2.13.6 >>> >> >> Thanks. >> >> -- >> Dmitry >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-17 0:49 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-06 1:03 [PATCH 0/1] Input: joydev blacklist feature Roderick Colenbrander 2017-10-06 1:03 ` [PATCH 1/1] Input: joydev - blacklist ds3/ds4/udraw motion sensors Roderick Colenbrander 2017-10-07 18:44 ` Dmitry Torokhov 2017-10-11 0:53 ` Colenbrander, Roelof 2017-10-13 23:50 ` Dmitry Torokhov 2017-10-17 0:48 ` Colenbrander, Roelof
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).