* [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).