All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Input/HID: introduce joydev ignore feature
@ 2017-08-24 23:11 Roderick Colenbrander
  2017-08-24 23:11 ` [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE Roderick Colenbrander
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Roderick Colenbrander @ 2017-08-24 23:11 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Bastien Nocera,
	Roderick Colenbrander

From: Roderick Colenbrander <roderick.colenbrander@sony.com>

Hi,

This is a revised version of a patch set, which was originally meant
to prevent devices ds3/ds4 motion sensors to get picked up by joydev.

Originally we added a filter based on INPUT_PROP_ACCELEROMETER, but
in review it was felt not to be narrow enough. It was suggested to
only limit in case of composite devices, which led to a revised
series.

The v2 series added a new property INPUT_PROP_COMPOSITE based on the
feedback, but the discussion led to the ability to enumerate composite
devices (a very useful use case) e.g. by means of an ioctl.

Tackling the composite device problem is an interesting problem, but
needs a bit of thought. For ds3/ds4 in particular, we have reports
(even on reddit) of various users having problems with joydev picking
up the device, so ideally for 4.14 (and maybe even 4.13) we need some
solution as it will be used in the next wave of distributions.

For v3, we are going back to the basics. Instead as Bastien suggested
we are adding a flag INPUT_PROP_JOYDEV_IGNORE, which devices can use
to get blacklist from joydev. We think such a flag is helpful as the
heuristics in joydev are generic and in some exceptional cases it is
tricky to find a generic way to narrow devices down. In addition
as requested by Bastien we are also setting the ignore flag for the
udraw-ps3 driver.

Thanks,
Roderick

Roderick Colenbrander (4):
  Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  HID: sony: Set INPUT_PROP_JOYDEV_IGNORE flag for motion sensors.
  HID: udraw-ps3: Set INPUT_PROP_JOYDEV_IGNORE for motion sensor
  Input: joydev - ignore devices which don't want joydev

 Documentation/input/event-codes.rst    | 9 +++++++++
 drivers/hid/hid-sony.c                 | 1 +
 drivers/hid/hid-udraw-ps3.c            | 1 +
 drivers/input/joydev.c                 | 4 ++++
 include/uapi/linux/input-event-codes.h | 1 +
 5 files changed, 16 insertions(+)

-- 
2.9.4


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-24 23:11 [PATCH v3 0/4] Input/HID: introduce joydev ignore feature Roderick Colenbrander
@ 2017-08-24 23:11 ` Roderick Colenbrander
  2017-08-25  8:45   ` Bastien Nocera
  2017-08-24 23:11 ` [PATCH 2/4] HID: sony: Set INPUT_PROP_JOYDEV_IGNORE flag for motion sensors Roderick Colenbrander
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Roderick Colenbrander @ 2017-08-24 23:11 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Bastien Nocera,
	Roderick Colenbrander

From: Roderick Colenbrander <roderick.colenbrander@sony.com>

This new property can be set on input devices to blacklist them
from getting picked up by joydev. This is meant for devices, which
pass joydev its heuristics, but for which there is no good generic
way of updating the heuristics.

Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
---
 Documentation/input/event-codes.rst    | 9 +++++++++
 include/uapi/linux/input-event-codes.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
index a8c0873..ae8c546 100644
--- a/Documentation/input/event-codes.rst
+++ b/Documentation/input/event-codes.rst
@@ -356,6 +356,15 @@ can report through the rotational axes (absolute and/or relative rx, ry, rz).
 All other axes retain their meaning. A device must not mix
 regular directional axes and accelerometer axes on the same event node.
 
+INPUT_PROP_JOYDEV_IGNORE
+------------------------
+
+The joydev interface uses heuristics to determine whether it should expose an
+input device through joydev. Some devices pass its heuristics, but don't
+make sense to expose. In some cases the generic heuristics can be updated,
+but in other cases this is not easy. The INPUT_PROP_JOYDEV_IGNORE flag can
+be set by drivers to explicit request blacklisting by joydev.
+
 Guidelines
 ==========
 
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 1798910..f6aac0e 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -26,6 +26,7 @@
 #define INPUT_PROP_TOPBUTTONPAD		0x04	/* softbuttons at top of pad */
 #define INPUT_PROP_POINTING_STICK	0x05	/* is a pointing stick */
 #define INPUT_PROP_ACCELEROMETER	0x06	/* has accelerometer */
+#define INPUT_PROP_JOYDEV_IGNORE	0x07	/* hint to ignore device by joydev */
 
 #define INPUT_PROP_MAX			0x1f
 #define INPUT_PROP_CNT			(INPUT_PROP_MAX + 1)
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/4] HID: sony: Set INPUT_PROP_JOYDEV_IGNORE flag for motion sensors.
  2017-08-24 23:11 [PATCH v3 0/4] Input/HID: introduce joydev ignore feature Roderick Colenbrander
  2017-08-24 23:11 ` [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE Roderick Colenbrander
@ 2017-08-24 23:11 ` Roderick Colenbrander
  2017-08-25  8:46   ` Bastien Nocera
  2017-08-24 23:11 ` [PATCH 3/4] HID: udraw-ps3: Set INPUT_PROP_JOYDEV_IGNORE for motion sensor Roderick Colenbrander
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Roderick Colenbrander @ 2017-08-24 23:11 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Bastien Nocera,
	Roderick Colenbrander

From: Roderick Colenbrander <roderick.colenbrander@sony.com>

Configure the motion sensor sub device with the INPUT_PROP_JOYDEV_IGNORE
flag to prevent joydev from picking it up.

Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
---
 drivers/hid/hid-sony.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index d03203a..6c290c6 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1375,6 +1375,7 @@ static int sony_register_sensors(struct sony_sc *sc)
 	}
 
 	__set_bit(INPUT_PROP_ACCELEROMETER, sc->sensor_dev->propbit);
+	__set_bit(INPUT_PROP_JOYDEV_IGNORE, sc->sensor_dev->propbit);
 
 	ret = input_register_device(sc->sensor_dev);
 	if (ret < 0)
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/4] HID: udraw-ps3: Set INPUT_PROP_JOYDEV_IGNORE for motion sensor
  2017-08-24 23:11 [PATCH v3 0/4] Input/HID: introduce joydev ignore feature Roderick Colenbrander
  2017-08-24 23:11 ` [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE Roderick Colenbrander
  2017-08-24 23:11 ` [PATCH 2/4] HID: sony: Set INPUT_PROP_JOYDEV_IGNORE flag for motion sensors Roderick Colenbrander
@ 2017-08-24 23:11 ` Roderick Colenbrander
  2017-08-25  8:46   ` Bastien Nocera
  2017-08-24 23:11 ` [PATCH 4/4] Input: joydev - ignore devices which don't want joydev Roderick Colenbrander
  2017-08-25  8:49 ` [PATCH v3 0/4] Input/HID: introduce joydev ignore feature Bastien Nocera
  4 siblings, 1 reply; 20+ messages in thread
From: Roderick Colenbrander @ 2017-08-24 23:11 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Bastien Nocera,
	Roderick Colenbrander

From: Roderick Colenbrander <roderick.colenbrander@sony.com>

Set the INPUT_PROP_JOYDEV_IGNORE flag for the motion sensor device,
so it won't get picked up by joydev.

Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
---
 drivers/hid/hid-udraw-ps3.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/hid-udraw-ps3.c b/drivers/hid/hid-udraw-ps3.c
index 88ea390..7ea4066 100644
--- a/drivers/hid/hid-udraw-ps3.c
+++ b/drivers/hid/hid-udraw-ps3.c
@@ -379,6 +379,7 @@ static bool udraw_setup_accel(struct udraw *udraw,
 	input_set_abs_params(input_dev, ABS_Z, -512, 512, 0, 0);
 
 	set_bit(INPUT_PROP_ACCELEROMETER, input_dev->propbit);
+	set_bit(INPUT_PROP_JOYDEV_IGNORE, input_dev->propbit);
 
 	udraw->accel_input_dev = input_dev;
 
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/4] Input: joydev - ignore devices which don't want joydev
  2017-08-24 23:11 [PATCH v3 0/4] Input/HID: introduce joydev ignore feature Roderick Colenbrander
                   ` (2 preceding siblings ...)
  2017-08-24 23:11 ` [PATCH 3/4] HID: udraw-ps3: Set INPUT_PROP_JOYDEV_IGNORE for motion sensor Roderick Colenbrander
@ 2017-08-24 23:11 ` Roderick Colenbrander
  2017-08-25  8:48   ` Bastien Nocera
  2017-08-25  8:49 ` [PATCH v3 0/4] Input/HID: introduce joydev ignore feature Bastien Nocera
  4 siblings, 1 reply; 20+ messages in thread
From: Roderick Colenbrander @ 2017-08-24 23:11 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Bastien Nocera,
	Roderick Colenbrander

From: Roderick Colenbrander <roderick.colenbrander@sony.com>

Gamepads like DualShock 3 / 4 as of 4.12 started reporting motion
sensors on a separate evdev node. Joydev is picking these devices
up as well, but they don't make sense for the joydev interface.
This patch leverages INPUT_PROP_JOYDEV_IGNORE to not create joydev
devices for devices, which don't want them.

Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
---
 drivers/input/joydev.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
index 29d677c..a60cfb3 100644
--- a/drivers/input/joydev.c
+++ b/drivers/input/joydev.c
@@ -819,6 +819,10 @@ static bool joydev_match(struct input_handler *handler, struct input_dev *dev)
 	if (joydev_dev_is_absolute_mouse(dev))
 		return false;
 
+	/* Avoid devices, which don't want to be reported by joydev. */
+	if (test_bit(INPUT_PROP_JOYDEV_IGNORE, dev->propbit))
+		return false;
+
 	return true;
 }
 
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-24 23:11 ` [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE Roderick Colenbrander
@ 2017-08-25  8:45   ` Bastien Nocera
  2017-08-28 21:08     ` Roderick Colenbrander
  0 siblings, 1 reply; 20+ messages in thread
From: Bastien Nocera @ 2017-08-25  8:45 UTC (permalink / raw)
  To: Roderick Colenbrander, linux-input
  Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Roderick Colenbrander

On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander wrote:
> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
> 
> This new property can be set on input devices to blacklist them
> from getting picked up by joydev. This is meant for devices, which
> pass joydev its heuristics, but for which there is no good generic
> way of updating the heuristics.

I can't make sense of that last sentence, and the possessive for
"heuristics" (here and below in the documentation) is, IMO,
unnecessary.

> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> ---
>  Documentation/input/event-codes.rst    | 9 +++++++++
>  include/uapi/linux/input-event-codes.h | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/Documentation/input/event-codes.rst
> b/Documentation/input/event-codes.rst
> index a8c0873..ae8c546 100644
> --- a/Documentation/input/event-codes.rst
> +++ b/Documentation/input/event-codes.rst
> @@ -356,6 +356,15 @@ can report through the rotational axes (absolute
> and/or relative rx, ry, rz).
>  All other axes retain their meaning. A device must not mix
>  regular directional axes and accelerometer axes on the same event
> node.
>  
> +INPUT_PROP_JOYDEV_IGNORE
> +------------------------
> +
> +The joydev interface uses heuristics to determine whether it should
> expose an
> +input device through joydev. Some devices pass its heuristics, but
> don't
> +make sense to expose. In some cases the generic heuristics can be
> updated,
> +but in other cases this is not easy. The INPUT_PROP_JOYDEV_IGNORE
> flag can
> +be set by drivers to explicit request blacklisting by joydev.

The "don't make sense to expose" is not what we're trying to do here
though. The problem is rather that "we used not to show this device
through joydev, but programs using joydev are limited and usually not
updated so we should only show what we used to".

>  Guidelines
>  ==========
>  
> diff --git a/include/uapi/linux/input-event-codes.h
> b/include/uapi/linux/input-event-codes.h
> index 1798910..f6aac0e 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -26,6 +26,7 @@
>  #define INPUT_PROP_TOPBUTTONPAD		0x04	/*
> softbuttons at top of pad */
>  #define INPUT_PROP_POINTING_STICK	0x05	/* is a
> pointing stick */
>  #define INPUT_PROP_ACCELEROMETER	0x06	/* has
> accelerometer */
> +#define INPUT_PROP_JOYDEV_IGNORE	0x07	/* hint to
> ignore device by joydev */
>  
>  #define INPUT_PROP_MAX			0x1f
>  #define INPUT_PROP_CNT			(INPUT_PROP_MAX + 1)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/4] HID: sony: Set INPUT_PROP_JOYDEV_IGNORE flag for motion sensors.
  2017-08-24 23:11 ` [PATCH 2/4] HID: sony: Set INPUT_PROP_JOYDEV_IGNORE flag for motion sensors Roderick Colenbrander
@ 2017-08-25  8:46   ` Bastien Nocera
  0 siblings, 0 replies; 20+ messages in thread
From: Bastien Nocera @ 2017-08-25  8:46 UTC (permalink / raw)
  To: Roderick Colenbrander, linux-input
  Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Roderick Colenbrander

On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander wrote:
> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
> 
> Configure the motion sensor sub device with the
> INPUT_PROP_JOYDEV_IGNORE
> flag to prevent joydev from picking it up.

Can you please add the reason why to the commit message?

> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> ---
>  drivers/hid/hid-sony.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index d03203a..6c290c6 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -1375,6 +1375,7 @@ static int sony_register_sensors(struct sony_sc
> *sc)
>  	}
>  
>  	__set_bit(INPUT_PROP_ACCELEROMETER, sc->sensor_dev-
> >propbit);
> +	__set_bit(INPUT_PROP_JOYDEV_IGNORE, sc->sensor_dev-
> >propbit);
>  
>  	ret = input_register_device(sc->sensor_dev);
>  	if (ret < 0)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/4] HID: udraw-ps3: Set INPUT_PROP_JOYDEV_IGNORE for motion sensor
  2017-08-24 23:11 ` [PATCH 3/4] HID: udraw-ps3: Set INPUT_PROP_JOYDEV_IGNORE for motion sensor Roderick Colenbrander
@ 2017-08-25  8:46   ` Bastien Nocera
  0 siblings, 0 replies; 20+ messages in thread
From: Bastien Nocera @ 2017-08-25  8:46 UTC (permalink / raw)
  To: Roderick Colenbrander, linux-input
  Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Roderick Colenbrander

On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander wrote:
> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
> 
> Set the INPUT_PROP_JOYDEV_IGNORE flag for the motion sensor device,
> so it won't get picked up by joydev.

Ditto on the reason.

> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> ---
>  drivers/hid/hid-udraw-ps3.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/hid/hid-udraw-ps3.c b/drivers/hid/hid-udraw-
> ps3.c
> index 88ea390..7ea4066 100644
> --- a/drivers/hid/hid-udraw-ps3.c
> +++ b/drivers/hid/hid-udraw-ps3.c
> @@ -379,6 +379,7 @@ static bool udraw_setup_accel(struct udraw
> *udraw,
>  	input_set_abs_params(input_dev, ABS_Z, -512, 512, 0, 0);
>  
>  	set_bit(INPUT_PROP_ACCELEROMETER, input_dev->propbit);
> +	set_bit(INPUT_PROP_JOYDEV_IGNORE, input_dev->propbit);
>  
>  	udraw->accel_input_dev = input_dev;
>  

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/4] Input: joydev - ignore devices which don't want joydev
  2017-08-24 23:11 ` [PATCH 4/4] Input: joydev - ignore devices which don't want joydev Roderick Colenbrander
@ 2017-08-25  8:48   ` Bastien Nocera
  0 siblings, 0 replies; 20+ messages in thread
From: Bastien Nocera @ 2017-08-25  8:48 UTC (permalink / raw)
  To: Roderick Colenbrander, linux-input
  Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Roderick Colenbrander

On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander wrote:
> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
> 
> Gamepads like DualShock 3 / 4 as of 4.12 started reporting motion
> sensors on a separate evdev node. Joydev is picking these devices
> up as well, but they don't make sense for the joydev interface.

Again, they do make sense, but programs using joydev are unlikely to
see updates any time soon. See the comment for patch 1, this needs to
be reworded.

> This patch leverages INPUT_PROP_JOYDEV_IGNORE to not create joydev
> devices for devices, which don't want them.

"to not create joydev device nodes for devices which don't want it".

> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> ---
>  drivers/input/joydev.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c
> index 29d677c..a60cfb3 100644
> --- a/drivers/input/joydev.c
> +++ b/drivers/input/joydev.c
> @@ -819,6 +819,10 @@ static bool joydev_match(struct input_handler
> *handler, struct input_dev *dev)
>  	if (joydev_dev_is_absolute_mouse(dev))
>  		return false;
>  
> +	/* Avoid devices, which don't want to be reported by joydev.
> */
> +	if (test_bit(INPUT_PROP_JOYDEV_IGNORE, dev->propbit))
> +		return false;
> +
>  	return true;
>  }
>  

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v3 0/4] Input/HID: introduce joydev ignore feature
  2017-08-24 23:11 [PATCH v3 0/4] Input/HID: introduce joydev ignore feature Roderick Colenbrander
                   ` (3 preceding siblings ...)
  2017-08-24 23:11 ` [PATCH 4/4] Input: joydev - ignore devices which don't want joydev Roderick Colenbrander
@ 2017-08-25  8:49 ` Bastien Nocera
  4 siblings, 0 replies; 20+ messages in thread
From: Bastien Nocera @ 2017-08-25  8:49 UTC (permalink / raw)
  To: Roderick Colenbrander, linux-input
  Cc: Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina, Roderick Colenbrander

On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander wrote:
> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
> 
> Hi,
> 
> This is a revised version of a patch set, which was originally meant
> to prevent devices ds3/ds4 motion sensors to get picked up by joydev.
<snip>

It's all nitpick and wording problems for me.

Dmitry, you can add:
Reviewed-by: Bastien Nocera <hadess@hadess.net>

to the whole patchset once you're happy with the wording.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-25  8:45   ` Bastien Nocera
@ 2017-08-28 21:08     ` Roderick Colenbrander
  2017-08-28 21:16       ` Dmitry Torokhov
  0 siblings, 1 reply; 20+ messages in thread
From: Roderick Colenbrander @ 2017-08-28 21:08 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: linux-input, Dmitry Torokhov, Benjamin Tissoires, Jiri Kosina,
	Roderick Colenbrander

On Fri, Aug 25, 2017 at 1:45 AM, Bastien Nocera <hadess@hadess.net> wrote:
> On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander wrote:
>> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
>>
>> This new property can be set on input devices to blacklist them
>> from getting picked up by joydev. This is meant for devices, which
>> pass joydev its heuristics, but for which there is no good generic
>> way of updating the heuristics.
>
> I can't make sense of that last sentence, and the possessive for
> "heuristics" (here and below in the documentation) is, IMO,
> unnecessary.
>
>> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
>> ---
>>  Documentation/input/event-codes.rst    | 9 +++++++++
>>  include/uapi/linux/input-event-codes.h | 1 +
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/Documentation/input/event-codes.rst
>> b/Documentation/input/event-codes.rst
>> index a8c0873..ae8c546 100644
>> --- a/Documentation/input/event-codes.rst
>> +++ b/Documentation/input/event-codes.rst
>> @@ -356,6 +356,15 @@ can report through the rotational axes (absolute
>> and/or relative rx, ry, rz).
>>  All other axes retain their meaning. A device must not mix
>>  regular directional axes and accelerometer axes on the same event
>> node.
>>
>> +INPUT_PROP_JOYDEV_IGNORE
>> +------------------------
>> +
>> +The joydev interface uses heuristics to determine whether it should
>> expose an
>> +input device through joydev. Some devices pass its heuristics, but
>> don't
>> +make sense to expose. In some cases the generic heuristics can be
>> updated,
>> +but in other cases this is not easy. The INPUT_PROP_JOYDEV_IGNORE
>> flag can
>> +be set by drivers to explicit request blacklisting by joydev.
>
> The "don't make sense to expose" is not what we're trying to do here
> though. The problem is rather that "we used not to show this device
> through joydev, but programs using joydev are limited and usually not
> updated so we should only show what we used to".
>

Thanks, I will change the wording. Originally I wrote it like this,
because I thought joydev applications could not determine at all which
axes were being used except for 'an axis number' and for that reason
thought that the match function had some heuristics (e.g. filtering
out touchpad devices and others), making sure a joystick has buttons
etcetera. I wasn't aware of JSIOCGAXMAP, which does allow applications
to get more information about a device, but you can't easily determine
if something is e.g. a motion sensor device you would need to do a
string compare on known strings or make assumptions if you see a
device with axes, but no buttons.

Thanks,
Roderick

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-28 21:08     ` Roderick Colenbrander
@ 2017-08-28 21:16       ` Dmitry Torokhov
  2017-08-28 21:35         ` Roderick Colenbrander
  0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2017-08-28 21:16 UTC (permalink / raw)
  To: Roderick Colenbrander
  Cc: Bastien Nocera, linux-input, Benjamin Tissoires, Jiri Kosina,
	Roderick Colenbrander

Hi,

On Mon, Aug 28, 2017 at 02:08:54PM -0700, Roderick Colenbrander wrote:
> On Fri, Aug 25, 2017 at 1:45 AM, Bastien Nocera <hadess@hadess.net> wrote:
> > On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander wrote:
> >> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
> >>
> >> This new property can be set on input devices to blacklist them
> >> from getting picked up by joydev. This is meant for devices, which
> >> pass joydev its heuristics, but for which there is no good generic
> >> way of updating the heuristics.
> >
> > I can't make sense of that last sentence, and the possessive for
> > "heuristics" (here and below in the documentation) is, IMO,
> > unnecessary.
> >
> >> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> >> ---
> >>  Documentation/input/event-codes.rst    | 9 +++++++++
> >>  include/uapi/linux/input-event-codes.h | 1 +
> >>  2 files changed, 10 insertions(+)
> >>
> >> diff --git a/Documentation/input/event-codes.rst
> >> b/Documentation/input/event-codes.rst
> >> index a8c0873..ae8c546 100644
> >> --- a/Documentation/input/event-codes.rst
> >> +++ b/Documentation/input/event-codes.rst
> >> @@ -356,6 +356,15 @@ can report through the rotational axes (absolute
> >> and/or relative rx, ry, rz).
> >>  All other axes retain their meaning. A device must not mix
> >>  regular directional axes and accelerometer axes on the same event
> >> node.
> >>
> >> +INPUT_PROP_JOYDEV_IGNORE
> >> +------------------------
> >> +
> >> +The joydev interface uses heuristics to determine whether it should
> >> expose an
> >> +input device through joydev. Some devices pass its heuristics, but
> >> don't
> >> +make sense to expose. In some cases the generic heuristics can be
> >> updated,
> >> +but in other cases this is not easy. The INPUT_PROP_JOYDEV_IGNORE
> >> flag can
> >> +be set by drivers to explicit request blacklisting by joydev.
> >
> > The "don't make sense to expose" is not what we're trying to do here
> > though. The problem is rather that "we used not to show this device
> > through joydev, but programs using joydev are limited and usually not
> > updated so we should only show what we used to".
> >
> 
> Thanks, I will change the wording. Originally I wrote it like this,
> because I thought joydev applications could not determine at all which
> axes were being used except for 'an axis number' and for that reason
> thought that the match function had some heuristics (e.g. filtering
> out touchpad devices and others), making sure a joystick has buttons
> etcetera. I wasn't aware of JSIOCGAXMAP, which does allow applications
> to get more information about a device, but you can't easily determine
> if something is e.g. a motion sensor device you would need to do a
> string compare on known strings or make assumptions if you see a
> device with axes, but no buttons.

Sorry for the delay, but exposing the internal kernel decisions to
userspace is not something that we need to do. Why would userspace care
to see this in device properties?

Also, this whole thing puts knowledge of interfaces into the drivers,
and driver should not care at all what interfaces kernel might
implement. Do drivers need to be aware that there is SysRq handler? Or
that on some versions of ChromeOS there is a handler that bumps up
CPU frequency in response to user activity?

If you really want to stop joydev from attaching to some devices then
the decision should go in joydev itself, not spread across multiple
drivers.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-28 21:16       ` Dmitry Torokhov
@ 2017-08-28 21:35         ` Roderick Colenbrander
  2017-08-28 21:38           ` Dmitry Torokhov
  0 siblings, 1 reply; 20+ messages in thread
From: Roderick Colenbrander @ 2017-08-28 21:35 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Bastien Nocera, linux-input, Benjamin Tissoires, Jiri Kosina,
	Roderick Colenbrander

On Mon, Aug 28, 2017 at 2:16 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi,
>
> On Mon, Aug 28, 2017 at 02:08:54PM -0700, Roderick Colenbrander wrote:
>> On Fri, Aug 25, 2017 at 1:45 AM, Bastien Nocera <hadess@hadess.net> wrote:
>> > On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander wrote:
>> >> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
>> >>
>> >> This new property can be set on input devices to blacklist them
>> >> from getting picked up by joydev. This is meant for devices, which
>> >> pass joydev its heuristics, but for which there is no good generic
>> >> way of updating the heuristics.
>> >
>> > I can't make sense of that last sentence, and the possessive for
>> > "heuristics" (here and below in the documentation) is, IMO,
>> > unnecessary.
>> >
>> >> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
>> >> ---
>> >>  Documentation/input/event-codes.rst    | 9 +++++++++
>> >>  include/uapi/linux/input-event-codes.h | 1 +
>> >>  2 files changed, 10 insertions(+)
>> >>
>> >> diff --git a/Documentation/input/event-codes.rst
>> >> b/Documentation/input/event-codes.rst
>> >> index a8c0873..ae8c546 100644
>> >> --- a/Documentation/input/event-codes.rst
>> >> +++ b/Documentation/input/event-codes.rst
>> >> @@ -356,6 +356,15 @@ can report through the rotational axes (absolute
>> >> and/or relative rx, ry, rz).
>> >>  All other axes retain their meaning. A device must not mix
>> >>  regular directional axes and accelerometer axes on the same event
>> >> node.
>> >>
>> >> +INPUT_PROP_JOYDEV_IGNORE
>> >> +------------------------
>> >> +
>> >> +The joydev interface uses heuristics to determine whether it should
>> >> expose an
>> >> +input device through joydev. Some devices pass its heuristics, but
>> >> don't
>> >> +make sense to expose. In some cases the generic heuristics can be
>> >> updated,
>> >> +but in other cases this is not easy. The INPUT_PROP_JOYDEV_IGNORE
>> >> flag can
>> >> +be set by drivers to explicit request blacklisting by joydev.
>> >
>> > The "don't make sense to expose" is not what we're trying to do here
>> > though. The problem is rather that "we used not to show this device
>> > through joydev, but programs using joydev are limited and usually not
>> > updated so we should only show what we used to".
>> >
>>
>> Thanks, I will change the wording. Originally I wrote it like this,
>> because I thought joydev applications could not determine at all which
>> axes were being used except for 'an axis number' and for that reason
>> thought that the match function had some heuristics (e.g. filtering
>> out touchpad devices and others), making sure a joystick has buttons
>> etcetera. I wasn't aware of JSIOCGAXMAP, which does allow applications
>> to get more information about a device, but you can't easily determine
>> if something is e.g. a motion sensor device you would need to do a
>> string compare on known strings or make assumptions if you see a
>> device with axes, but no buttons.
>
> Sorry for the delay, but exposing the internal kernel decisions to
> userspace is not something that we need to do. Why would userspace care
> to see this in device properties?
>
> Also, this whole thing puts knowledge of interfaces into the drivers,
> and driver should not care at all what interfaces kernel might
> implement. Do drivers need to be aware that there is SysRq handler? Or
> that on some versions of ChromeOS there is a handler that bumps up
> CPU frequency in response to user activity?
>
> If you really want to stop joydev from attaching to some devices then
> the decision should go in joydev itself, not spread across multiple
> drivers.
>
> Thanks.
>
> --
> Dmitry

Correct user space should not have to be aware. Originally the patch
add a composite device flag, but that term was more loaded and needed
ioctls. That field would have made sense for user space, but this flag
not, we just piggy-backed on the the properties field in the
input_dev.

In my case of ds3/ds4 to fix old applications, I want to blacklist
joydev in some way, but joydev doesn't have access to enough
information except for INPUT_PROP_ACCELEROMETER which I think you felt
was not narrow enough in scope.

Would the solution be to add some new private quirks/flags field to
'struct input_dev', which joydev could use? Or is there another
solution you have in mind.

Thanks,
Roderick

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-28 21:35         ` Roderick Colenbrander
@ 2017-08-28 21:38           ` Dmitry Torokhov
  2017-08-28 22:02             ` Roderick Colenbrander
  0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2017-08-28 21:38 UTC (permalink / raw)
  To: Roderick Colenbrander
  Cc: Bastien Nocera, linux-input, Benjamin Tissoires, Jiri Kosina,
	Roderick Colenbrander

On Mon, Aug 28, 2017 at 02:35:15PM -0700, Roderick Colenbrander wrote:
> On Mon, Aug 28, 2017 at 2:16 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > Hi,
> >
> > On Mon, Aug 28, 2017 at 02:08:54PM -0700, Roderick Colenbrander wrote:
> >> On Fri, Aug 25, 2017 at 1:45 AM, Bastien Nocera <hadess@hadess.net> wrote:
> >> > On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander wrote:
> >> >> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
> >> >>
> >> >> This new property can be set on input devices to blacklist them
> >> >> from getting picked up by joydev. This is meant for devices, which
> >> >> pass joydev its heuristics, but for which there is no good generic
> >> >> way of updating the heuristics.
> >> >
> >> > I can't make sense of that last sentence, and the possessive for
> >> > "heuristics" (here and below in the documentation) is, IMO,
> >> > unnecessary.
> >> >
> >> >> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> >> >> ---
> >> >>  Documentation/input/event-codes.rst    | 9 +++++++++
> >> >>  include/uapi/linux/input-event-codes.h | 1 +
> >> >>  2 files changed, 10 insertions(+)
> >> >>
> >> >> diff --git a/Documentation/input/event-codes.rst
> >> >> b/Documentation/input/event-codes.rst
> >> >> index a8c0873..ae8c546 100644
> >> >> --- a/Documentation/input/event-codes.rst
> >> >> +++ b/Documentation/input/event-codes.rst
> >> >> @@ -356,6 +356,15 @@ can report through the rotational axes (absolute
> >> >> and/or relative rx, ry, rz).
> >> >>  All other axes retain their meaning. A device must not mix
> >> >>  regular directional axes and accelerometer axes on the same event
> >> >> node.
> >> >>
> >> >> +INPUT_PROP_JOYDEV_IGNORE
> >> >> +------------------------
> >> >> +
> >> >> +The joydev interface uses heuristics to determine whether it should
> >> >> expose an
> >> >> +input device through joydev. Some devices pass its heuristics, but
> >> >> don't
> >> >> +make sense to expose. In some cases the generic heuristics can be
> >> >> updated,
> >> >> +but in other cases this is not easy. The INPUT_PROP_JOYDEV_IGNORE
> >> >> flag can
> >> >> +be set by drivers to explicit request blacklisting by joydev.
> >> >
> >> > The "don't make sense to expose" is not what we're trying to do here
> >> > though. The problem is rather that "we used not to show this device
> >> > through joydev, but programs using joydev are limited and usually not
> >> > updated so we should only show what we used to".
> >> >
> >>
> >> Thanks, I will change the wording. Originally I wrote it like this,
> >> because I thought joydev applications could not determine at all which
> >> axes were being used except for 'an axis number' and for that reason
> >> thought that the match function had some heuristics (e.g. filtering
> >> out touchpad devices and others), making sure a joystick has buttons
> >> etcetera. I wasn't aware of JSIOCGAXMAP, which does allow applications
> >> to get more information about a device, but you can't easily determine
> >> if something is e.g. a motion sensor device you would need to do a
> >> string compare on known strings or make assumptions if you see a
> >> device with axes, but no buttons.
> >
> > Sorry for the delay, but exposing the internal kernel decisions to
> > userspace is not something that we need to do. Why would userspace care
> > to see this in device properties?
> >
> > Also, this whole thing puts knowledge of interfaces into the drivers,
> > and driver should not care at all what interfaces kernel might
> > implement. Do drivers need to be aware that there is SysRq handler? Or
> > that on some versions of ChromeOS there is a handler that bumps up
> > CPU frequency in response to user activity?
> >
> > If you really want to stop joydev from attaching to some devices then
> > the decision should go in joydev itself, not spread across multiple
> > drivers.
> >
> > Thanks.
> >
> > --
> > Dmitry
> 
> Correct user space should not have to be aware. Originally the patch
> add a composite device flag, but that term was more loaded and needed
> ioctls. That field would have made sense for user space, but this flag
> not, we just piggy-backed on the the properties field in the
> input_dev.
> 
> In my case of ds3/ds4 to fix old applications, I want to blacklist
> joydev in some way, but joydev doesn't have access to enough
> information except for INPUT_PROP_ACCELEROMETER which I think you felt
> was not narrow enough in scope.
> 
> Would the solution be to add some new private quirks/flags field to
> 'struct input_dev', which joydev could use? Or is there another
> solution you have in mind.

What kind of data joydev is missing? There is the input_id with bus,
vendor, product and version, device capabilities, plus you have full
access to the input device itself, so you can look up name, phys, etc.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-28 21:38           ` Dmitry Torokhov
@ 2017-08-28 22:02             ` Roderick Colenbrander
  2017-08-28 22:08               ` Bastien Nocera
  2017-08-28 22:09               ` Dmitry Torokhov
  0 siblings, 2 replies; 20+ messages in thread
From: Roderick Colenbrander @ 2017-08-28 22:02 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Bastien Nocera, linux-input, Benjamin Tissoires, Jiri Kosina,
	Roderick Colenbrander

On Mon, Aug 28, 2017 at 2:38 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Aug 28, 2017 at 02:35:15PM -0700, Roderick Colenbrander wrote:
>> On Mon, Aug 28, 2017 at 2:16 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > Hi,
>> >
>> > On Mon, Aug 28, 2017 at 02:08:54PM -0700, Roderick Colenbrander wrote:
>> >> On Fri, Aug 25, 2017 at 1:45 AM, Bastien Nocera <hadess@hadess.net> wrote:
>> >> > On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander wrote:
>> >> >> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
>> >> >>
>> >> >> This new property can be set on input devices to blacklist them
>> >> >> from getting picked up by joydev. This is meant for devices, which
>> >> >> pass joydev its heuristics, but for which there is no good generic
>> >> >> way of updating the heuristics.
>> >> >
>> >> > I can't make sense of that last sentence, and the possessive for
>> >> > "heuristics" (here and below in the documentation) is, IMO,
>> >> > unnecessary.
>> >> >
>> >> >> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
>> >> >> ---
>> >> >>  Documentation/input/event-codes.rst    | 9 +++++++++
>> >> >>  include/uapi/linux/input-event-codes.h | 1 +
>> >> >>  2 files changed, 10 insertions(+)
>> >> >>
>> >> >> diff --git a/Documentation/input/event-codes.rst
>> >> >> b/Documentation/input/event-codes.rst
>> >> >> index a8c0873..ae8c546 100644
>> >> >> --- a/Documentation/input/event-codes.rst
>> >> >> +++ b/Documentation/input/event-codes.rst
>> >> >> @@ -356,6 +356,15 @@ can report through the rotational axes (absolute
>> >> >> and/or relative rx, ry, rz).
>> >> >>  All other axes retain their meaning. A device must not mix
>> >> >>  regular directional axes and accelerometer axes on the same event
>> >> >> node.
>> >> >>
>> >> >> +INPUT_PROP_JOYDEV_IGNORE
>> >> >> +------------------------
>> >> >> +
>> >> >> +The joydev interface uses heuristics to determine whether it should
>> >> >> expose an
>> >> >> +input device through joydev. Some devices pass its heuristics, but
>> >> >> don't
>> >> >> +make sense to expose. In some cases the generic heuristics can be
>> >> >> updated,
>> >> >> +but in other cases this is not easy. The INPUT_PROP_JOYDEV_IGNORE
>> >> >> flag can
>> >> >> +be set by drivers to explicit request blacklisting by joydev.
>> >> >
>> >> > The "don't make sense to expose" is not what we're trying to do here
>> >> > though. The problem is rather that "we used not to show this device
>> >> > through joydev, but programs using joydev are limited and usually not
>> >> > updated so we should only show what we used to".
>> >> >
>> >>
>> >> Thanks, I will change the wording. Originally I wrote it like this,
>> >> because I thought joydev applications could not determine at all which
>> >> axes were being used except for 'an axis number' and for that reason
>> >> thought that the match function had some heuristics (e.g. filtering
>> >> out touchpad devices and others), making sure a joystick has buttons
>> >> etcetera. I wasn't aware of JSIOCGAXMAP, which does allow applications
>> >> to get more information about a device, but you can't easily determine
>> >> if something is e.g. a motion sensor device you would need to do a
>> >> string compare on known strings or make assumptions if you see a
>> >> device with axes, but no buttons.
>> >
>> > Sorry for the delay, but exposing the internal kernel decisions to
>> > userspace is not something that we need to do. Why would userspace care
>> > to see this in device properties?
>> >
>> > Also, this whole thing puts knowledge of interfaces into the drivers,
>> > and driver should not care at all what interfaces kernel might
>> > implement. Do drivers need to be aware that there is SysRq handler? Or
>> > that on some versions of ChromeOS there is a handler that bumps up
>> > CPU frequency in response to user activity?
>> >
>> > If you really want to stop joydev from attaching to some devices then
>> > the decision should go in joydev itself, not spread across multiple
>> > drivers.
>> >
>> > Thanks.
>> >
>> > --
>> > Dmitry
>>
>> Correct user space should not have to be aware. Originally the patch
>> add a composite device flag, but that term was more loaded and needed
>> ioctls. That field would have made sense for user space, but this flag
>> not, we just piggy-backed on the the properties field in the
>> input_dev.
>>
>> In my case of ds3/ds4 to fix old applications, I want to blacklist
>> joydev in some way, but joydev doesn't have access to enough
>> information except for INPUT_PROP_ACCELEROMETER which I think you felt
>> was not narrow enough in scope.
>>
>> Would the solution be to add some new private quirks/flags field to
>> 'struct input_dev', which joydev could use? Or is there another
>> solution you have in mind.
>
> What kind of data joydev is missing? There is the input_id with bus,
> vendor, product and version, device capabilities, plus you have full
> access to the input device itself, so you can look up name, phys, etc.
>
> Thanks.
>
> --
> Dmitry


Thanks for getting back so quickly. The input_dev has all the
information as you could do something with product / vendor ids, which
I wanted to avoid as there are 5 DS4 ids I need to handle and 2 DS3
ids. We still want to expose the 'gamepad' subdevice, but not the
motion device (INPUT_PROP_ACCELEROMETER), so it would be quite some
logic. Overall I thought it would be cleaner to not have this device
knowledge in joydev and maybe expose a new flag.

Thanks,
Roderick

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-28 22:02             ` Roderick Colenbrander
@ 2017-08-28 22:08               ` Bastien Nocera
  2017-08-28 22:16                 ` Dmitry Torokhov
  2017-08-28 22:09               ` Dmitry Torokhov
  1 sibling, 1 reply; 20+ messages in thread
From: Bastien Nocera @ 2017-08-28 22:08 UTC (permalink / raw)
  To: Roderick Colenbrander, Dmitry Torokhov
  Cc: linux-input, Benjamin Tissoires, Jiri Kosina, Roderick Colenbrander

On Mon, 2017-08-28 at 15:02 -0700, Roderick Colenbrander wrote:
> On Mon, Aug 28, 2017 at 2:38 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Mon, Aug 28, 2017 at 02:35:15PM -0700, Roderick Colenbrander
> > wrote:
> > > On Mon, Aug 28, 2017 at 2:16 PM, Dmitry Torokhov
> > > <dmitry.torokhov@gmail.com> wrote:
> > > > Hi,
> > > > 
> > > > On Mon, Aug 28, 2017 at 02:08:54PM -0700, Roderick Colenbrander
> > > > wrote:
> > > > > On Fri, Aug 25, 2017 at 1:45 AM, Bastien Nocera <hadess@hades
> > > > > s.net> wrote:
> > > > > > On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander
> > > > > > wrote:
> > > > > > > From: Roderick Colenbrander <roderick.colenbrander@sony.c
> > > > > > > om>
> > > > > > > 
> > > > > > > This new property can be set on input devices to
> > > > > > > blacklist them
> > > > > > > from getting picked up by joydev. This is meant for
> > > > > > > devices, which
> > > > > > > pass joydev its heuristics, but for which there is no
> > > > > > > good generic
> > > > > > > way of updating the heuristics.
> > > > > > 
> > > > > > I can't make sense of that last sentence, and the
> > > > > > possessive for
> > > > > > "heuristics" (here and below in the documentation) is, IMO,
> > > > > > unnecessary.
> > > > > > 
> > > > > > > Signed-off-by: Roderick Colenbrander <roderick.colenbrand
> > > > > > > er@sony.com>
> > > > > > > ---
> > > > > > >  Documentation/input/event-codes.rst    | 9 +++++++++
> > > > > > >  include/uapi/linux/input-event-codes.h | 1 +
> > > > > > >  2 files changed, 10 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/Documentation/input/event-codes.rst
> > > > > > > b/Documentation/input/event-codes.rst
> > > > > > > index a8c0873..ae8c546 100644
> > > > > > > --- a/Documentation/input/event-codes.rst
> > > > > > > +++ b/Documentation/input/event-codes.rst
> > > > > > > @@ -356,6 +356,15 @@ can report through the rotational
> > > > > > > axes (absolute
> > > > > > > and/or relative rx, ry, rz).
> > > > > > >  All other axes retain their meaning. A device must not
> > > > > > > mix
> > > > > > >  regular directional axes and accelerometer axes on the
> > > > > > > same event
> > > > > > > node.
> > > > > > > 
> > > > > > > +INPUT_PROP_JOYDEV_IGNORE
> > > > > > > +------------------------
> > > > > > > +
> > > > > > > +The joydev interface uses heuristics to determine
> > > > > > > whether it should
> > > > > > > expose an
> > > > > > > +input device through joydev. Some devices pass its
> > > > > > > heuristics, but
> > > > > > > don't
> > > > > > > +make sense to expose. In some cases the generic
> > > > > > > heuristics can be
> > > > > > > updated,
> > > > > > > +but in other cases this is not easy. The
> > > > > > > INPUT_PROP_JOYDEV_IGNORE
> > > > > > > flag can
> > > > > > > +be set by drivers to explicit request blacklisting by
> > > > > > > joydev.
> > > > > > 
> > > > > > The "don't make sense to expose" is not what we're trying
> > > > > > to do here
> > > > > > though. The problem is rather that "we used not to show
> > > > > > this device
> > > > > > through joydev, but programs using joydev are limited and
> > > > > > usually not
> > > > > > updated so we should only show what we used to".
> > > > > > 
> > > > > 
> > > > > Thanks, I will change the wording. Originally I wrote it like
> > > > > this,
> > > > > because I thought joydev applications could not determine at
> > > > > all which
> > > > > axes were being used except for 'an axis number' and for that
> > > > > reason
> > > > > thought that the match function had some heuristics (e.g.
> > > > > filtering
> > > > > out touchpad devices and others), making sure a joystick has
> > > > > buttons
> > > > > etcetera. I wasn't aware of JSIOCGAXMAP, which does allow
> > > > > applications
> > > > > to get more information about a device, but you can't easily
> > > > > determine
> > > > > if something is e.g. a motion sensor device you would need to
> > > > > do a
> > > > > string compare on known strings or make assumptions if you
> > > > > see a
> > > > > device with axes, but no buttons.
> > > > 
> > > > Sorry for the delay, but exposing the internal kernel decisions
> > > > to
> > > > userspace is not something that we need to do. Why would
> > > > userspace care
> > > > to see this in device properties?
> > > > 
> > > > Also, this whole thing puts knowledge of interfaces into the
> > > > drivers,
> > > > and driver should not care at all what interfaces kernel might
> > > > implement. Do drivers need to be aware that there is SysRq
> > > > handler? Or
> > > > that on some versions of ChromeOS there is a handler that bumps
> > > > up
> > > > CPU frequency in response to user activity?
> > > > 
> > > > If you really want to stop joydev from attaching to some
> > > > devices then
> > > > the decision should go in joydev itself, not spread across
> > > > multiple
> > > > drivers.
> > > > 
> > > > Thanks.
> > > > 
> > > > --
> > > > Dmitry
> > > 
> > > Correct user space should not have to be aware. Originally the
> > > patch
> > > add a composite device flag, but that term was more loaded and
> > > needed
> > > ioctls. That field would have made sense for user space, but this
> > > flag
> > > not, we just piggy-backed on the the properties field in the
> > > input_dev.
> > > 
> > > In my case of ds3/ds4 to fix old applications, I want to
> > > blacklist
> > > joydev in some way, but joydev doesn't have access to enough
> > > information except for INPUT_PROP_ACCELEROMETER which I think you
> > > felt
> > > was not narrow enough in scope.
> > > 
> > > Would the solution be to add some new private quirks/flags field
> > > to
> > > 'struct input_dev', which joydev could use? Or is there another
> > > solution you have in mind.
> > 
> > What kind of data joydev is missing? There is the input_id with
> > bus,
> > vendor, product and version, device capabilities, plus you have
> > full
> > access to the input device itself, so you can look up name, phys,
> > etc.
> > 
> > Thanks.
> > 
> > --
> > Dmitry
> 
> 
> Thanks for getting back so quickly. The input_dev has all the
> information as you could do something with product / vendor ids,
> which
> I wanted to avoid as there are 5 DS4 ids I need to handle and 2 DS3
> ids. We still want to expose the 'gamepad' subdevice, but not the
> motion device (INPUT_PROP_ACCELEROMETER), so it would be quite some
> logic. Overall I thought it would be cleaner to not have this device
> knowledge in joydev and maybe expose a new flag.

Apart from the fact that the INPUT_PROP_JOYDEV_IGNORE property is made
visible in user-space (it could be there as a debug mechanism to show
why the device is not exported through joydev), I think having the
device driver tell joydev not to export it is the right mechanism.

The problem of devices having sub-components exported through joydev
should only exist when 1) the device driver is extended to be more
capable (like the DS4 accelerometers), 2) the device driver is extended
to support more hardware (whether a simple vid/pid combination, or
something more involved) 3) new device drivers are added.

joydev changes are thus kept to a minimum, the drivers (and their
changes) are self-sufficient.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-28 22:02             ` Roderick Colenbrander
  2017-08-28 22:08               ` Bastien Nocera
@ 2017-08-28 22:09               ` Dmitry Torokhov
  1 sibling, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2017-08-28 22:09 UTC (permalink / raw)
  To: Roderick Colenbrander
  Cc: Bastien Nocera, linux-input, Benjamin Tissoires, Jiri Kosina,
	Roderick Colenbrander

On Mon, Aug 28, 2017 at 03:02:16PM -0700, Roderick Colenbrander wrote:
> On Mon, Aug 28, 2017 at 2:38 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Mon, Aug 28, 2017 at 02:35:15PM -0700, Roderick Colenbrander wrote:
> >> On Mon, Aug 28, 2017 at 2:16 PM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> wrote:
> >> > Hi,
> >> >
> >> > On Mon, Aug 28, 2017 at 02:08:54PM -0700, Roderick Colenbrander wrote:
> >> >> On Fri, Aug 25, 2017 at 1:45 AM, Bastien Nocera <hadess@hadess.net> wrote:
> >> >> > On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander wrote:
> >> >> >> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
> >> >> >>
> >> >> >> This new property can be set on input devices to blacklist them
> >> >> >> from getting picked up by joydev. This is meant for devices, which
> >> >> >> pass joydev its heuristics, but for which there is no good generic
> >> >> >> way of updating the heuristics.
> >> >> >
> >> >> > I can't make sense of that last sentence, and the possessive for
> >> >> > "heuristics" (here and below in the documentation) is, IMO,
> >> >> > unnecessary.
> >> >> >
> >> >> >> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
> >> >> >> ---
> >> >> >>  Documentation/input/event-codes.rst    | 9 +++++++++
> >> >> >>  include/uapi/linux/input-event-codes.h | 1 +
> >> >> >>  2 files changed, 10 insertions(+)
> >> >> >>
> >> >> >> diff --git a/Documentation/input/event-codes.rst
> >> >> >> b/Documentation/input/event-codes.rst
> >> >> >> index a8c0873..ae8c546 100644
> >> >> >> --- a/Documentation/input/event-codes.rst
> >> >> >> +++ b/Documentation/input/event-codes.rst
> >> >> >> @@ -356,6 +356,15 @@ can report through the rotational axes (absolute
> >> >> >> and/or relative rx, ry, rz).
> >> >> >>  All other axes retain their meaning. A device must not mix
> >> >> >>  regular directional axes and accelerometer axes on the same event
> >> >> >> node.
> >> >> >>
> >> >> >> +INPUT_PROP_JOYDEV_IGNORE
> >> >> >> +------------------------
> >> >> >> +
> >> >> >> +The joydev interface uses heuristics to determine whether it should
> >> >> >> expose an
> >> >> >> +input device through joydev. Some devices pass its heuristics, but
> >> >> >> don't
> >> >> >> +make sense to expose. In some cases the generic heuristics can be
> >> >> >> updated,
> >> >> >> +but in other cases this is not easy. The INPUT_PROP_JOYDEV_IGNORE
> >> >> >> flag can
> >> >> >> +be set by drivers to explicit request blacklisting by joydev.
> >> >> >
> >> >> > The "don't make sense to expose" is not what we're trying to do here
> >> >> > though. The problem is rather that "we used not to show this device
> >> >> > through joydev, but programs using joydev are limited and usually not
> >> >> > updated so we should only show what we used to".
> >> >> >
> >> >>
> >> >> Thanks, I will change the wording. Originally I wrote it like this,
> >> >> because I thought joydev applications could not determine at all which
> >> >> axes were being used except for 'an axis number' and for that reason
> >> >> thought that the match function had some heuristics (e.g. filtering
> >> >> out touchpad devices and others), making sure a joystick has buttons
> >> >> etcetera. I wasn't aware of JSIOCGAXMAP, which does allow applications
> >> >> to get more information about a device, but you can't easily determine
> >> >> if something is e.g. a motion sensor device you would need to do a
> >> >> string compare on known strings or make assumptions if you see a
> >> >> device with axes, but no buttons.
> >> >
> >> > Sorry for the delay, but exposing the internal kernel decisions to
> >> > userspace is not something that we need to do. Why would userspace care
> >> > to see this in device properties?
> >> >
> >> > Also, this whole thing puts knowledge of interfaces into the drivers,
> >> > and driver should not care at all what interfaces kernel might
> >> > implement. Do drivers need to be aware that there is SysRq handler? Or
> >> > that on some versions of ChromeOS there is a handler that bumps up
> >> > CPU frequency in response to user activity?
> >> >
> >> > If you really want to stop joydev from attaching to some devices then
> >> > the decision should go in joydev itself, not spread across multiple
> >> > drivers.
> >> >
> >> > Thanks.
> >> >
> >> > --
> >> > Dmitry
> >>
> >> Correct user space should not have to be aware. Originally the patch
> >> add a composite device flag, but that term was more loaded and needed
> >> ioctls. That field would have made sense for user space, but this flag
> >> not, we just piggy-backed on the the properties field in the
> >> input_dev.
> >>
> >> In my case of ds3/ds4 to fix old applications, I want to blacklist
> >> joydev in some way, but joydev doesn't have access to enough
> >> information except for INPUT_PROP_ACCELEROMETER which I think you felt
> >> was not narrow enough in scope.
> >>
> >> Would the solution be to add some new private quirks/flags field to
> >> 'struct input_dev', which joydev could use? Or is there another
> >> solution you have in mind.
> >
> > What kind of data joydev is missing? There is the input_id with bus,
> > vendor, product and version, device capabilities, plus you have full
> > access to the input device itself, so you can look up name, phys, etc.
> >
> > Thanks.
> >
> > --
> > Dmitry
> 
> 
> Thanks for getting back so quickly. The input_dev has all the
> information as you could do something with product / vendor ids, which
> I wanted to avoid as there are 5 DS4 ids I need to handle and 2 DS3
> ids. We still want to expose the 'gamepad' subdevice, but not the
> motion device (INPUT_PROP_ACCELEROMETER), so it would be quite some
> logic. Overall I thought it would be cleaner to not have this device
> knowledge in joydev and maybe expose a new flag.

I believe conceptually this would be wrong, as it would push the
knowledge of existing interfaces into the drivers. So just use input ID
(and I guess you need to parse phys a bit to see which device you want
to handle and which to ignore).

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-28 22:08               ` Bastien Nocera
@ 2017-08-28 22:16                 ` Dmitry Torokhov
  2017-08-28 22:19                   ` Bastien Nocera
  2017-08-29  0:02                   ` Roderick Colenbrander
  0 siblings, 2 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2017-08-28 22:16 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: Roderick Colenbrander, linux-input, Benjamin Tissoires,
	Jiri Kosina, Roderick Colenbrander

On Tue, Aug 29, 2017 at 12:08:08AM +0200, Bastien Nocera wrote:
> On Mon, 2017-08-28 at 15:02 -0700, Roderick Colenbrander wrote:
> > On Mon, Aug 28, 2017 at 2:38 PM, Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> > > On Mon, Aug 28, 2017 at 02:35:15PM -0700, Roderick Colenbrander
> > > wrote:
> > > > On Mon, Aug 28, 2017 at 2:16 PM, Dmitry Torokhov
> > > > <dmitry.torokhov@gmail.com> wrote:
> > > > > Hi,
> > > > > 
> > > > > On Mon, Aug 28, 2017 at 02:08:54PM -0700, Roderick Colenbrander
> > > > > wrote:
> > > > > > On Fri, Aug 25, 2017 at 1:45 AM, Bastien Nocera <hadess@hades
> > > > > > s.net> wrote:
> > > > > > > On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander
> > > > > > > wrote:
> > > > > > > > From: Roderick Colenbrander <roderick.colenbrander@sony.c
> > > > > > > > om>
> > > > > > > > 
> > > > > > > > This new property can be set on input devices to
> > > > > > > > blacklist them
> > > > > > > > from getting picked up by joydev. This is meant for
> > > > > > > > devices, which
> > > > > > > > pass joydev its heuristics, but for which there is no
> > > > > > > > good generic
> > > > > > > > way of updating the heuristics.
> > > > > > > 
> > > > > > > I can't make sense of that last sentence, and the
> > > > > > > possessive for
> > > > > > > "heuristics" (here and below in the documentation) is, IMO,
> > > > > > > unnecessary.
> > > > > > > 
> > > > > > > > Signed-off-by: Roderick Colenbrander <roderick.colenbrand
> > > > > > > > er@sony.com>
> > > > > > > > ---
> > > > > > > >  Documentation/input/event-codes.rst    | 9 +++++++++
> > > > > > > >  include/uapi/linux/input-event-codes.h | 1 +
> > > > > > > >  2 files changed, 10 insertions(+)
> > > > > > > > 
> > > > > > > > diff --git a/Documentation/input/event-codes.rst
> > > > > > > > b/Documentation/input/event-codes.rst
> > > > > > > > index a8c0873..ae8c546 100644
> > > > > > > > --- a/Documentation/input/event-codes.rst
> > > > > > > > +++ b/Documentation/input/event-codes.rst
> > > > > > > > @@ -356,6 +356,15 @@ can report through the rotational
> > > > > > > > axes (absolute
> > > > > > > > and/or relative rx, ry, rz).
> > > > > > > >  All other axes retain their meaning. A device must not
> > > > > > > > mix
> > > > > > > >  regular directional axes and accelerometer axes on the
> > > > > > > > same event
> > > > > > > > node.
> > > > > > > > 
> > > > > > > > +INPUT_PROP_JOYDEV_IGNORE
> > > > > > > > +------------------------
> > > > > > > > +
> > > > > > > > +The joydev interface uses heuristics to determine
> > > > > > > > whether it should
> > > > > > > > expose an
> > > > > > > > +input device through joydev. Some devices pass its
> > > > > > > > heuristics, but
> > > > > > > > don't
> > > > > > > > +make sense to expose. In some cases the generic
> > > > > > > > heuristics can be
> > > > > > > > updated,
> > > > > > > > +but in other cases this is not easy. The
> > > > > > > > INPUT_PROP_JOYDEV_IGNORE
> > > > > > > > flag can
> > > > > > > > +be set by drivers to explicit request blacklisting by
> > > > > > > > joydev.
> > > > > > > 
> > > > > > > The "don't make sense to expose" is not what we're trying
> > > > > > > to do here
> > > > > > > though. The problem is rather that "we used not to show
> > > > > > > this device
> > > > > > > through joydev, but programs using joydev are limited and
> > > > > > > usually not
> > > > > > > updated so we should only show what we used to".
> > > > > > > 
> > > > > > 
> > > > > > Thanks, I will change the wording. Originally I wrote it like
> > > > > > this,
> > > > > > because I thought joydev applications could not determine at
> > > > > > all which
> > > > > > axes were being used except for 'an axis number' and for that
> > > > > > reason
> > > > > > thought that the match function had some heuristics (e.g.
> > > > > > filtering
> > > > > > out touchpad devices and others), making sure a joystick has
> > > > > > buttons
> > > > > > etcetera. I wasn't aware of JSIOCGAXMAP, which does allow
> > > > > > applications
> > > > > > to get more information about a device, but you can't easily
> > > > > > determine
> > > > > > if something is e.g. a motion sensor device you would need to
> > > > > > do a
> > > > > > string compare on known strings or make assumptions if you
> > > > > > see a
> > > > > > device with axes, but no buttons.
> > > > > 
> > > > > Sorry for the delay, but exposing the internal kernel decisions
> > > > > to
> > > > > userspace is not something that we need to do. Why would
> > > > > userspace care
> > > > > to see this in device properties?
> > > > > 
> > > > > Also, this whole thing puts knowledge of interfaces into the
> > > > > drivers,
> > > > > and driver should not care at all what interfaces kernel might
> > > > > implement. Do drivers need to be aware that there is SysRq
> > > > > handler? Or
> > > > > that on some versions of ChromeOS there is a handler that bumps
> > > > > up
> > > > > CPU frequency in response to user activity?
> > > > > 
> > > > > If you really want to stop joydev from attaching to some
> > > > > devices then
> > > > > the decision should go in joydev itself, not spread across
> > > > > multiple
> > > > > drivers.
> > > > > 
> > > > > Thanks.
> > > > > 
> > > > > --
> > > > > Dmitry
> > > > 
> > > > Correct user space should not have to be aware. Originally the
> > > > patch
> > > > add a composite device flag, but that term was more loaded and
> > > > needed
> > > > ioctls. That field would have made sense for user space, but this
> > > > flag
> > > > not, we just piggy-backed on the the properties field in the
> > > > input_dev.
> > > > 
> > > > In my case of ds3/ds4 to fix old applications, I want to
> > > > blacklist
> > > > joydev in some way, but joydev doesn't have access to enough
> > > > information except for INPUT_PROP_ACCELEROMETER which I think you
> > > > felt
> > > > was not narrow enough in scope.
> > > > 
> > > > Would the solution be to add some new private quirks/flags field
> > > > to
> > > > 'struct input_dev', which joydev could use? Or is there another
> > > > solution you have in mind.
> > > 
> > > What kind of data joydev is missing? There is the input_id with
> > > bus,
> > > vendor, product and version, device capabilities, plus you have
> > > full
> > > access to the input device itself, so you can look up name, phys,
> > > etc.
> > > 
> > > Thanks.
> > > 
> > > --
> > > Dmitry
> > 
> > 
> > Thanks for getting back so quickly. The input_dev has all the
> > information as you could do something with product / vendor ids,
> > which
> > I wanted to avoid as there are 5 DS4 ids I need to handle and 2 DS3
> > ids. We still want to expose the 'gamepad' subdevice, but not the
> > motion device (INPUT_PROP_ACCELEROMETER), so it would be quite some
> > logic. Overall I thought it would be cleaner to not have this device
> > knowledge in joydev and maybe expose a new flag.
> 
> Apart from the fact that the INPUT_PROP_JOYDEV_IGNORE property is made
> visible in user-space (it could be there as a debug mechanism to show
> why the device is not exported through joydev), I think having the
> device driver tell joydev not to export it is the right mechanism.
> 
> The problem of devices having sub-components exported through joydev
> should only exist when 1) the device driver is extended to be more
> capable (like the DS4 accelerometers), 2) the device driver is extended
> to support more hardware (whether a simple vid/pid combination, or
> something more involved) 3) new device drivers are added.
> 
> joydev changes are thus kept to a minimum, the drivers (and their
> changes) are self-sufficient.

What will happen if there is joydev2? Or some other interface (input
handler) that you decide should not bind to certain devices?

The handlers need to decide what to do with input devices, and input
driver should only expose the capabilities/caracteristics/whatever. So
"composite" property is good (and I am open to looking into how we can
help userspace identify all parts of composite device, maybe just
convention on "phys" format will work well enough), and maybe "leader"
device property is also acceptable, but "ignore joydev", "ignore
cpufreq", "ignore sysrq", "ignore cpuboost" is not.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-28 22:16                 ` Dmitry Torokhov
@ 2017-08-28 22:19                   ` Bastien Nocera
  2017-08-29  0:02                   ` Roderick Colenbrander
  1 sibling, 0 replies; 20+ messages in thread
From: Bastien Nocera @ 2017-08-28 22:19 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Roderick Colenbrander, linux-input, Benjamin Tissoires,
	Jiri Kosina, Roderick Colenbrander

On Mon, 2017-08-28 at 15:16 -0700, Dmitry Torokhov wrote:
> On Tue, Aug 29, 2017 at 12:08:08AM +0200, Bastien Nocera wrote:
> > On Mon, 2017-08-28 at 15:02 -0700, Roderick Colenbrander wrote:
> > > On Mon, Aug 28, 2017 at 2:38 PM, Dmitry Torokhov
> > > <dmitry.torokhov@gmail.com> wrote:
> > > > On Mon, Aug 28, 2017 at 02:35:15PM -0700, Roderick Colenbrander
> > > > wrote:
> > > > > On Mon, Aug 28, 2017 at 2:16 PM, Dmitry Torokhov
> > > > > <dmitry.torokhov@gmail.com> wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On Mon, Aug 28, 2017 at 02:08:54PM -0700, Roderick
> > > > > > Colenbrander
> > > > > > wrote:
> > > > > > > On Fri, Aug 25, 2017 at 1:45 AM, Bastien Nocera <hadess@h
> > > > > > > ades
> > > > > > > s.net> wrote:
> > > > > > > > On Thu, 2017-08-24 at 16:11 -0700, Roderick
> > > > > > > > Colenbrander
> > > > > > > > wrote:
> > > > > > > > > From: Roderick Colenbrander <roderick.colenbrander@so
> > > > > > > > > ny.c
> > > > > > > > > om>
> > > > > > > > > 
> > > > > > > > > This new property can be set on input devices to
> > > > > > > > > blacklist them
> > > > > > > > > from getting picked up by joydev. This is meant for
> > > > > > > > > devices, which
> > > > > > > > > pass joydev its heuristics, but for which there is no
> > > > > > > > > good generic
> > > > > > > > > way of updating the heuristics.
> > > > > > > > 
> > > > > > > > I can't make sense of that last sentence, and the
> > > > > > > > possessive for
> > > > > > > > "heuristics" (here and below in the documentation) is,
> > > > > > > > IMO,
> > > > > > > > unnecessary.
> > > > > > > > 
> > > > > > > > > Signed-off-by: Roderick Colenbrander
> > > > > > > > > <roderick.colenbrand
> > > > > > > > > er@sony.com>
> > > > > > > > > ---
> > > > > > > > >  Documentation/input/event-codes.rst    | 9 +++++++++
> > > > > > > > >  include/uapi/linux/input-event-codes.h | 1 +
> > > > > > > > >  2 files changed, 10 insertions(+)
> > > > > > > > > 
> > > > > > > > > diff --git a/Documentation/input/event-codes.rst
> > > > > > > > > b/Documentation/input/event-codes.rst
> > > > > > > > > index a8c0873..ae8c546 100644
> > > > > > > > > --- a/Documentation/input/event-codes.rst
> > > > > > > > > +++ b/Documentation/input/event-codes.rst
> > > > > > > > > @@ -356,6 +356,15 @@ can report through the
> > > > > > > > > rotational
> > > > > > > > > axes (absolute
> > > > > > > > > and/or relative rx, ry, rz).
> > > > > > > > >  All other axes retain their meaning. A device must
> > > > > > > > > not
> > > > > > > > > mix
> > > > > > > > >  regular directional axes and accelerometer axes on
> > > > > > > > > the
> > > > > > > > > same event
> > > > > > > > > node.
> > > > > > > > > 
> > > > > > > > > +INPUT_PROP_JOYDEV_IGNORE
> > > > > > > > > +------------------------
> > > > > > > > > +
> > > > > > > > > +The joydev interface uses heuristics to determine
> > > > > > > > > whether it should
> > > > > > > > > expose an
> > > > > > > > > +input device through joydev. Some devices pass its
> > > > > > > > > heuristics, but
> > > > > > > > > don't
> > > > > > > > > +make sense to expose. In some cases the generic
> > > > > > > > > heuristics can be
> > > > > > > > > updated,
> > > > > > > > > +but in other cases this is not easy. The
> > > > > > > > > INPUT_PROP_JOYDEV_IGNORE
> > > > > > > > > flag can
> > > > > > > > > +be set by drivers to explicit request blacklisting
> > > > > > > > > by
> > > > > > > > > joydev.
> > > > > > > > 
> > > > > > > > The "don't make sense to expose" is not what we're
> > > > > > > > trying
> > > > > > > > to do here
> > > > > > > > though. The problem is rather that "we used not to show
> > > > > > > > this device
> > > > > > > > through joydev, but programs using joydev are limited
> > > > > > > > and
> > > > > > > > usually not
> > > > > > > > updated so we should only show what we used to".
> > > > > > > > 
> > > > > > > 
> > > > > > > Thanks, I will change the wording. Originally I wrote it
> > > > > > > like
> > > > > > > this,
> > > > > > > because I thought joydev applications could not determine
> > > > > > > at
> > > > > > > all which
> > > > > > > axes were being used except for 'an axis number' and for
> > > > > > > that
> > > > > > > reason
> > > > > > > thought that the match function had some heuristics (e.g.
> > > > > > > filtering
> > > > > > > out touchpad devices and others), making sure a joystick
> > > > > > > has
> > > > > > > buttons
> > > > > > > etcetera. I wasn't aware of JSIOCGAXMAP, which does allow
> > > > > > > applications
> > > > > > > to get more information about a device, but you can't
> > > > > > > easily
> > > > > > > determine
> > > > > > > if something is e.g. a motion sensor device you would
> > > > > > > need to
> > > > > > > do a
> > > > > > > string compare on known strings or make assumptions if
> > > > > > > you
> > > > > > > see a
> > > > > > > device with axes, but no buttons.
> > > > > > 
> > > > > > Sorry for the delay, but exposing the internal kernel
> > > > > > decisions
> > > > > > to
> > > > > > userspace is not something that we need to do. Why would
> > > > > > userspace care
> > > > > > to see this in device properties?
> > > > > > 
> > > > > > Also, this whole thing puts knowledge of interfaces into
> > > > > > the
> > > > > > drivers,
> > > > > > and driver should not care at all what interfaces kernel
> > > > > > might
> > > > > > implement. Do drivers need to be aware that there is SysRq
> > > > > > handler? Or
> > > > > > that on some versions of ChromeOS there is a handler that
> > > > > > bumps
> > > > > > up
> > > > > > CPU frequency in response to user activity?
> > > > > > 
> > > > > > If you really want to stop joydev from attaching to some
> > > > > > devices then
> > > > > > the decision should go in joydev itself, not spread across
> > > > > > multiple
> > > > > > drivers.
> > > > > > 
> > > > > > Thanks.
> > > > > > 
> > > > > > --
> > > > > > Dmitry
> > > > > 
> > > > > Correct user space should not have to be aware. Originally
> > > > > the
> > > > > patch
> > > > > add a composite device flag, but that term was more loaded
> > > > > and
> > > > > needed
> > > > > ioctls. That field would have made sense for user space, but
> > > > > this
> > > > > flag
> > > > > not, we just piggy-backed on the the properties field in the
> > > > > input_dev.
> > > > > 
> > > > > In my case of ds3/ds4 to fix old applications, I want to
> > > > > blacklist
> > > > > joydev in some way, but joydev doesn't have access to enough
> > > > > information except for INPUT_PROP_ACCELEROMETER which I think
> > > > > you
> > > > > felt
> > > > > was not narrow enough in scope.
> > > > > 
> > > > > Would the solution be to add some new private quirks/flags
> > > > > field
> > > > > to
> > > > > 'struct input_dev', which joydev could use? Or is there
> > > > > another
> > > > > solution you have in mind.
> > > > 
> > > > What kind of data joydev is missing? There is the input_id with
> > > > bus,
> > > > vendor, product and version, device capabilities, plus you have
> > > > full
> > > > access to the input device itself, so you can look up name,
> > > > phys,
> > > > etc.
> > > > 
> > > > Thanks.
> > > > 
> > > > --
> > > > Dmitry
> > > 
> > > 
> > > Thanks for getting back so quickly. The input_dev has all the
> > > information as you could do something with product / vendor ids,
> > > which
> > > I wanted to avoid as there are 5 DS4 ids I need to handle and 2
> > > DS3
> > > ids. We still want to expose the 'gamepad' subdevice, but not the
> > > motion device (INPUT_PROP_ACCELEROMETER), so it would be quite
> > > some
> > > logic. Overall I thought it would be cleaner to not have this
> > > device
> > > knowledge in joydev and maybe expose a new flag.
> > 
> > Apart from the fact that the INPUT_PROP_JOYDEV_IGNORE property is
> > made
> > visible in user-space (it could be there as a debug mechanism to
> > show
> > why the device is not exported through joydev), I think having the
> > device driver tell joydev not to export it is the right mechanism.
> > 
> > The problem of devices having sub-components exported through
> > joydev
> > should only exist when 1) the device driver is extended to be more
> > capable (like the DS4 accelerometers), 2) the device driver is
> > extended
> > to support more hardware (whether a simple vid/pid combination, or
> > something more involved) 3) new device drivers are added.
> > 
> > joydev changes are thus kept to a minimum, the drivers (and their
> > changes) are self-sufficient.
> 
> What will happen if there is joydev2? Or some other interface (input
> handler) that you decide should not bind to certain devices?
> 
> The handlers need to decide what to do with input devices, and input
> driver should only expose the capabilities/caracteristics/whatever.
> So
> "composite" property is good (and I am open to looking into how we
> can
> help userspace identify all parts of composite device, maybe just
> convention on "phys" format will work well enough), and maybe
> "leader"
> device property is also acceptable, but "ignore joydev", "ignore
> cpufreq", "ignore sysrq", "ignore cpuboost" is not.

"slippery slope" argument doesn't work here. "joydev" is a
compatibility interface. If there weren't any old applications relying
on it, we'd probably want to remove it. The reason why we need to make
those changes in the first place is because we cannot change the user-
space, and that user-space gets confused by new capabilities in
devices.

But up to you, your code to maintain in the long run.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE
  2017-08-28 22:16                 ` Dmitry Torokhov
  2017-08-28 22:19                   ` Bastien Nocera
@ 2017-08-29  0:02                   ` Roderick Colenbrander
  1 sibling, 0 replies; 20+ messages in thread
From: Roderick Colenbrander @ 2017-08-29  0:02 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Bastien Nocera, linux-input, Benjamin Tissoires, Jiri Kosina,
	Roderick Colenbrander

On Mon, Aug 28, 2017 at 3:16 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tue, Aug 29, 2017 at 12:08:08AM +0200, Bastien Nocera wrote:
>> On Mon, 2017-08-28 at 15:02 -0700, Roderick Colenbrander wrote:
>> > On Mon, Aug 28, 2017 at 2:38 PM, Dmitry Torokhov
>> > <dmitry.torokhov@gmail.com> wrote:
>> > > On Mon, Aug 28, 2017 at 02:35:15PM -0700, Roderick Colenbrander
>> > > wrote:
>> > > > On Mon, Aug 28, 2017 at 2:16 PM, Dmitry Torokhov
>> > > > <dmitry.torokhov@gmail.com> wrote:
>> > > > > Hi,
>> > > > >
>> > > > > On Mon, Aug 28, 2017 at 02:08:54PM -0700, Roderick Colenbrander
>> > > > > wrote:
>> > > > > > On Fri, Aug 25, 2017 at 1:45 AM, Bastien Nocera <hadess@hades
>> > > > > > s.net> wrote:
>> > > > > > > On Thu, 2017-08-24 at 16:11 -0700, Roderick Colenbrander
>> > > > > > > wrote:
>> > > > > > > > From: Roderick Colenbrander <roderick.colenbrander@sony.c
>> > > > > > > > om>
>> > > > > > > >
>> > > > > > > > This new property can be set on input devices to
>> > > > > > > > blacklist them
>> > > > > > > > from getting picked up by joydev. This is meant for
>> > > > > > > > devices, which
>> > > > > > > > pass joydev its heuristics, but for which there is no
>> > > > > > > > good generic
>> > > > > > > > way of updating the heuristics.
>> > > > > > >
>> > > > > > > I can't make sense of that last sentence, and the
>> > > > > > > possessive for
>> > > > > > > "heuristics" (here and below in the documentation) is, IMO,
>> > > > > > > unnecessary.
>> > > > > > >
>> > > > > > > > Signed-off-by: Roderick Colenbrander <roderick.colenbrand
>> > > > > > > > er@sony.com>
>> > > > > > > > ---
>> > > > > > > >  Documentation/input/event-codes.rst    | 9 +++++++++
>> > > > > > > >  include/uapi/linux/input-event-codes.h | 1 +
>> > > > > > > >  2 files changed, 10 insertions(+)
>> > > > > > > >
>> > > > > > > > diff --git a/Documentation/input/event-codes.rst
>> > > > > > > > b/Documentation/input/event-codes.rst
>> > > > > > > > index a8c0873..ae8c546 100644
>> > > > > > > > --- a/Documentation/input/event-codes.rst
>> > > > > > > > +++ b/Documentation/input/event-codes.rst
>> > > > > > > > @@ -356,6 +356,15 @@ can report through the rotational
>> > > > > > > > axes (absolute
>> > > > > > > > and/or relative rx, ry, rz).
>> > > > > > > >  All other axes retain their meaning. A device must not
>> > > > > > > > mix
>> > > > > > > >  regular directional axes and accelerometer axes on the
>> > > > > > > > same event
>> > > > > > > > node.
>> > > > > > > >
>> > > > > > > > +INPUT_PROP_JOYDEV_IGNORE
>> > > > > > > > +------------------------
>> > > > > > > > +
>> > > > > > > > +The joydev interface uses heuristics to determine
>> > > > > > > > whether it should
>> > > > > > > > expose an
>> > > > > > > > +input device through joydev. Some devices pass its
>> > > > > > > > heuristics, but
>> > > > > > > > don't
>> > > > > > > > +make sense to expose. In some cases the generic
>> > > > > > > > heuristics can be
>> > > > > > > > updated,
>> > > > > > > > +but in other cases this is not easy. The
>> > > > > > > > INPUT_PROP_JOYDEV_IGNORE
>> > > > > > > > flag can
>> > > > > > > > +be set by drivers to explicit request blacklisting by
>> > > > > > > > joydev.
>> > > > > > >
>> > > > > > > The "don't make sense to expose" is not what we're trying
>> > > > > > > to do here
>> > > > > > > though. The problem is rather that "we used not to show
>> > > > > > > this device
>> > > > > > > through joydev, but programs using joydev are limited and
>> > > > > > > usually not
>> > > > > > > updated so we should only show what we used to".
>> > > > > > >
>> > > > > >
>> > > > > > Thanks, I will change the wording. Originally I wrote it like
>> > > > > > this,
>> > > > > > because I thought joydev applications could not determine at
>> > > > > > all which
>> > > > > > axes were being used except for 'an axis number' and for that
>> > > > > > reason
>> > > > > > thought that the match function had some heuristics (e.g.
>> > > > > > filtering
>> > > > > > out touchpad devices and others), making sure a joystick has
>> > > > > > buttons
>> > > > > > etcetera. I wasn't aware of JSIOCGAXMAP, which does allow
>> > > > > > applications
>> > > > > > to get more information about a device, but you can't easily
>> > > > > > determine
>> > > > > > if something is e.g. a motion sensor device you would need to
>> > > > > > do a
>> > > > > > string compare on known strings or make assumptions if you
>> > > > > > see a
>> > > > > > device with axes, but no buttons.
>> > > > >
>> > > > > Sorry for the delay, but exposing the internal kernel decisions
>> > > > > to
>> > > > > userspace is not something that we need to do. Why would
>> > > > > userspace care
>> > > > > to see this in device properties?
>> > > > >
>> > > > > Also, this whole thing puts knowledge of interfaces into the
>> > > > > drivers,
>> > > > > and driver should not care at all what interfaces kernel might
>> > > > > implement. Do drivers need to be aware that there is SysRq
>> > > > > handler? Or
>> > > > > that on some versions of ChromeOS there is a handler that bumps
>> > > > > up
>> > > > > CPU frequency in response to user activity?
>> > > > >
>> > > > > If you really want to stop joydev from attaching to some
>> > > > > devices then
>> > > > > the decision should go in joydev itself, not spread across
>> > > > > multiple
>> > > > > drivers.
>> > > > >
>> > > > > Thanks.
>> > > > >
>> > > > > --
>> > > > > Dmitry
>> > > >
>> > > > Correct user space should not have to be aware. Originally the
>> > > > patch
>> > > > add a composite device flag, but that term was more loaded and
>> > > > needed
>> > > > ioctls. That field would have made sense for user space, but this
>> > > > flag
>> > > > not, we just piggy-backed on the the properties field in the
>> > > > input_dev.
>> > > >
>> > > > In my case of ds3/ds4 to fix old applications, I want to
>> > > > blacklist
>> > > > joydev in some way, but joydev doesn't have access to enough
>> > > > information except for INPUT_PROP_ACCELEROMETER which I think you
>> > > > felt
>> > > > was not narrow enough in scope.
>> > > >
>> > > > Would the solution be to add some new private quirks/flags field
>> > > > to
>> > > > 'struct input_dev', which joydev could use? Or is there another
>> > > > solution you have in mind.
>> > >
>> > > What kind of data joydev is missing? There is the input_id with
>> > > bus,
>> > > vendor, product and version, device capabilities, plus you have
>> > > full
>> > > access to the input device itself, so you can look up name, phys,
>> > > etc.
>> > >
>> > > Thanks.
>> > >
>> > > --
>> > > Dmitry
>> >
>> >
>> > Thanks for getting back so quickly. The input_dev has all the
>> > information as you could do something with product / vendor ids,
>> > which
>> > I wanted to avoid as there are 5 DS4 ids I need to handle and 2 DS3
>> > ids. We still want to expose the 'gamepad' subdevice, but not the
>> > motion device (INPUT_PROP_ACCELEROMETER), so it would be quite some
>> > logic. Overall I thought it would be cleaner to not have this device
>> > knowledge in joydev and maybe expose a new flag.
>>
>> Apart from the fact that the INPUT_PROP_JOYDEV_IGNORE property is made
>> visible in user-space (it could be there as a debug mechanism to show
>> why the device is not exported through joydev), I think having the
>> device driver tell joydev not to export it is the right mechanism.
>>
>> The problem of devices having sub-components exported through joydev
>> should only exist when 1) the device driver is extended to be more
>> capable (like the DS4 accelerometers), 2) the device driver is extended
>> to support more hardware (whether a simple vid/pid combination, or
>> something more involved) 3) new device drivers are added.
>>
>> joydev changes are thus kept to a minimum, the drivers (and their
>> changes) are self-sufficient.
>
> What will happen if there is joydev2? Or some other interface (input
> handler) that you decide should not bind to certain devices?
>
> The handlers need to decide what to do with input devices, and input
> driver should only expose the capabilities/caracteristics/whatever. So
> "composite" property is good (and I am open to looking into how we can
> help userspace identify all parts of composite device, maybe just
> convention on "phys" format will work well enough), and maybe "leader"
> device property is also acceptable, but "ignore joydev", "ignore
> cpufreq", "ignore sysrq", "ignore cpuboost" is not.
>
> Thanks.
>
> --
> Dmitry

I understand the design from your perspective in that it should be the
handlers, which should make decisions on what devices to support or
not.

The original patch added INPUT_PROP_COMPOSITE to allow joydev to do
'INPUT_PROP_COMPOSITE && INPUT_PROP_ACCELEROMETER'. Do you consider
this new flag a proper stepping stone towards to a complete solution
in where we also allow to query 'sibling devices'? If so I can
resubmit the patch as it would really help with our users on upcoming
distributions.

Thanks,
Roderick

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-08-29  0:02 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-24 23:11 [PATCH v3 0/4] Input/HID: introduce joydev ignore feature Roderick Colenbrander
2017-08-24 23:11 ` [PATCH 1/4] Input: Add new property INPUT_PROP_JOYDEV_IGNORE Roderick Colenbrander
2017-08-25  8:45   ` Bastien Nocera
2017-08-28 21:08     ` Roderick Colenbrander
2017-08-28 21:16       ` Dmitry Torokhov
2017-08-28 21:35         ` Roderick Colenbrander
2017-08-28 21:38           ` Dmitry Torokhov
2017-08-28 22:02             ` Roderick Colenbrander
2017-08-28 22:08               ` Bastien Nocera
2017-08-28 22:16                 ` Dmitry Torokhov
2017-08-28 22:19                   ` Bastien Nocera
2017-08-29  0:02                   ` Roderick Colenbrander
2017-08-28 22:09               ` Dmitry Torokhov
2017-08-24 23:11 ` [PATCH 2/4] HID: sony: Set INPUT_PROP_JOYDEV_IGNORE flag for motion sensors Roderick Colenbrander
2017-08-25  8:46   ` Bastien Nocera
2017-08-24 23:11 ` [PATCH 3/4] HID: udraw-ps3: Set INPUT_PROP_JOYDEV_IGNORE for motion sensor Roderick Colenbrander
2017-08-25  8:46   ` Bastien Nocera
2017-08-24 23:11 ` [PATCH 4/4] Input: joydev - ignore devices which don't want joydev Roderick Colenbrander
2017-08-25  8:48   ` Bastien Nocera
2017-08-25  8:49 ` [PATCH v3 0/4] Input/HID: introduce joydev ignore feature Bastien Nocera

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.