Re: [systemd-devel] [PATCH 1/5] udev-builtin-input_id: identify trackpoints and accelerometers

2015-04-03 Thread Hans de Goede 

Hi,

On 02-04-15 12:27, Lennart Poettering wrote:

On Thu, 02.04.15 12:21, Lennart Poettering (lenn...@poettering.net) wrote:


On Thu, 02.04.15 11:48, Hans de Goede (hdego...@redhat.com) wrote:


The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
on trackpoints for a while now, and this is useful information to have
in various places, so make input_id aware of this and make it set
ID_INPUT_POINTING_STICK on trackpoints.

While adding support for querying properties, also add support for the
recently added INPUT_PROP_ACCELEROMETER property bit.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
  src/udev/udev-builtin-input_id.c | 21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index 46f1c53..5b1790c 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -33,6 +33,14 @@
  #include udev.h
  #include util.h

+#ifndef INPUT_PROP_POINTING_STICK
+#define INPUT_PROP_POINTING_STICK   0x05
+#endif
+
+#ifndef INPUT_PROP_ACCELEROMETER
+#define INPUT_PROP_ACCELEROMETER0x06
+#endif


Are these definitions normally defined in input.h? If so, please add
them to missing.h, where we try to centralize definitions of this kind.


Also, INPUT_PROP_POINTING_STICK is already introduced by Peter
Hutterer's most recent patch. He adds it to missing.h, can you sync
the two patche sets up, please?


Actually Peter's patch pretty much does the same as my patch, except
that he does not add support for the recently added INPUT_PROP_ACCELEROMETER
bit.

So I'll completely drop this patch from v2 of my set.

Regards,

Hans
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/5] udev-builtin-input_id: identify trackpoints and accelerometers

2015-04-02 Thread Hans de Goede 
The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
on trackpoints for a while now, and this is useful information to have
in various places, so make input_id aware of this and make it set
ID_INPUT_POINTING_STICK on trackpoints.

While adding support for querying properties, also add support for the
recently added INPUT_PROP_ACCELEROMETER property bit.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 src/udev/udev-builtin-input_id.c | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index 46f1c53..5b1790c 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -33,6 +33,14 @@
 #include udev.h
 #include util.h
 
+#ifndef INPUT_PROP_POINTING_STICK
+#define INPUT_PROP_POINTING_STICK   0x05
+#endif
+
+#ifndef INPUT_PROP_ACCELEROMETER
+#define INPUT_PROP_ACCELEROMETER0x06
+#endif
+
 /* we must use this kernel-compatible implementation */
 #define BITS_PER_LONG (sizeof(unsigned long) * 8)
 #define NBITS(x) x)-1)/BITS_PER_LONG)+1)
@@ -131,6 +139,7 @@ static void test_pointers (struct udev_device *dev,
const unsigned long* bitmask_abs,
const unsigned long* bitmask_key,
const unsigned long* bitmask_rel,
+   unsigned long prop,
bool test) {
 int is_mouse = 0;
 int is_touchpad = 0;
@@ -182,6 +191,10 @@ static void test_pointers (struct udev_device *dev,
 udev_builtin_add_property(dev, test, ID_INPUT_MOUSE, 1);
 if (is_touchpad)
 udev_builtin_add_property(dev, test, ID_INPUT_TOUCHPAD, 1);
+if (prop  (1  INPUT_PROP_POINTING_STICK))
+udev_builtin_add_property(dev, test, ID_INPUT_TRACKPOINT, 
1);
+if (prop  (1  INPUT_PROP_ACCELEROMETER))
+udev_builtin_add_property(dev, test, ID_INPUT_ACCELEROMETER, 
1);
 }
 
 /* key like devices */
@@ -232,7 +245,8 @@ static int builtin_input_id(struct udev_device *dev, int 
argc, char *argv[], boo
 unsigned long bitmask_abs[NBITS(ABS_MAX)];
 unsigned long bitmask_key[NBITS(KEY_MAX)];
 unsigned long bitmask_rel[NBITS(REL_MAX)];
-const char *sysname, *devnode;
+unsigned long prop = 0;
+const char *sysname, *devnode, *prop_str;
 
 /* walk up the parental chain until we find the real input device; the
  * argument is very likely a subdevice of this, like eventN */
@@ -248,7 +262,10 @@ static int builtin_input_id(struct udev_device *dev, int 
argc, char *argv[], boo
 get_cap_mask(dev, pdev, capabilities/abs, bitmask_abs, 
sizeof(bitmask_abs), test);
 get_cap_mask(dev, pdev, capabilities/rel, bitmask_rel, 
sizeof(bitmask_rel), test);
 get_cap_mask(dev, pdev, capabilities/key, bitmask_key, 
sizeof(bitmask_key), test);
-test_pointers(dev, bitmask_ev, bitmask_abs, bitmask_key, 
bitmask_rel, test);
+prop_str = udev_device_get_property_value(pdev, PROP);
+if (prop_str)
+   prop = strtoul(prop_str, NULL, 16);
+test_pointers(dev, bitmask_ev, bitmask_abs, bitmask_key, 
bitmask_rel, prop, test);
 test_key(dev, bitmask_ev, bitmask_key, test);
 }
 
-- 
2.3.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/5] udev-builtin-input_id: identify trackpoints and accelerometers

2015-04-02 Thread Lennart Poettering 
On Thu, 02.04.15 11:48, Hans de Goede (hdego...@redhat.com) wrote:

 The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
 on trackpoints for a while now, and this is useful information to have
 in various places, so make input_id aware of this and make it set
 ID_INPUT_POINTING_STICK on trackpoints.
 
 While adding support for querying properties, also add support for the
 recently added INPUT_PROP_ACCELEROMETER property bit.
 
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
  src/udev/udev-builtin-input_id.c | 21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)
 
 diff --git a/src/udev/udev-builtin-input_id.c 
 b/src/udev/udev-builtin-input_id.c
 index 46f1c53..5b1790c 100644
 --- a/src/udev/udev-builtin-input_id.c
 +++ b/src/udev/udev-builtin-input_id.c
 @@ -33,6 +33,14 @@
  #include udev.h
  #include util.h
  
 +#ifndef INPUT_PROP_POINTING_STICK
 +#define INPUT_PROP_POINTING_STICK   0x05
 +#endif
 +
 +#ifndef INPUT_PROP_ACCELEROMETER
 +#define INPUT_PROP_ACCELEROMETER0x06
 +#endif

Are these definitions normally defined in input.h? If so, please add
them to missing.h, where we try to centralize definitions of this kind.

  /* we must use this kernel-compatible implementation */
  #define BITS_PER_LONG (sizeof(unsigned long) * 8)
  #define NBITS(x) x)-1)/BITS_PER_LONG)+1)
 @@ -131,6 +139,7 @@ static void test_pointers (struct udev_device *dev,
 const unsigned long* bitmask_abs,
 const unsigned long* bitmask_key,
 const unsigned long* bitmask_rel,
 +   unsigned long prop,

unsigned long? Is this really necessary? Shouldn't we just use
uint64_t? here?

 bool test) {
  int is_mouse = 0;
  int is_touchpad = 0;
 @@ -182,6 +191,10 @@ static void test_pointers (struct udev_device *dev,
  udev_builtin_add_property(dev, test, ID_INPUT_MOUSE, 1);
  if (is_touchpad)
  udev_builtin_add_property(dev, test, ID_INPUT_TOUCHPAD, 
 1);
 +if (prop  (1  INPUT_PROP_POINTING_STICK))
 +udev_builtin_add_property(dev, test, ID_INPUT_TRACKPOINT, 
 1);
 +if (prop  (1  INPUT_PROP_ACCELEROMETER))
 +udev_builtin_add_property(dev, test, 
 ID_INPUT_ACCELEROMETER, 1);
  }
  
  /* key like devices */
 @@ -232,7 +245,8 @@ static int builtin_input_id(struct udev_device *dev, int 
 argc, char *argv[], boo
  unsigned long bitmask_abs[NBITS(ABS_MAX)];
  unsigned long bitmask_key[NBITS(KEY_MAX)];
  unsigned long bitmask_rel[NBITS(REL_MAX)];
 -const char *sysname, *devnode;
 +unsigned long prop = 0;
 +const char *sysname, *devnode, *prop_str;
  
  /* walk up the parental chain until we find the real input device; 
 the
   * argument is very likely a subdevice of this, like eventN */
 @@ -248,7 +262,10 @@ static int builtin_input_id(struct udev_device *dev, int 
 argc, char *argv[], boo
  get_cap_mask(dev, pdev, capabilities/abs, bitmask_abs, 
 sizeof(bitmask_abs), test);
  get_cap_mask(dev, pdev, capabilities/rel, bitmask_rel, 
 sizeof(bitmask_rel), test);
  get_cap_mask(dev, pdev, capabilities/key, bitmask_key, 
 sizeof(bitmask_key), test);
 -test_pointers(dev, bitmask_ev, bitmask_abs, bitmask_key, 
 bitmask_rel, test);
 +prop_str = udev_device_get_property_value(pdev, PROP);
 +if (prop_str)
 +   prop = strtoul(prop_str, NULL, 16);

Hmm, we try to avoid direct invocations of strtoul() and friends, due
to the annoying error handling... Can't we use safe_atou64() here, or so?

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/5] udev-builtin-input_id: identify trackpoints and accelerometers

2015-04-02 Thread Hans de Goede 

Hi,

On 02-04-15 12:21, Lennart Poettering wrote:

On Thu, 02.04.15 11:48, Hans de Goede (hdego...@redhat.com) wrote:


The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
on trackpoints for a while now, and this is useful information to have
in various places, so make input_id aware of this and make it set
ID_INPUT_POINTING_STICK on trackpoints.

While adding support for querying properties, also add support for the
recently added INPUT_PROP_ACCELEROMETER property bit.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
  src/udev/udev-builtin-input_id.c | 21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index 46f1c53..5b1790c 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -33,6 +33,14 @@
  #include udev.h
  #include util.h

+#ifndef INPUT_PROP_POINTING_STICK
+#define INPUT_PROP_POINTING_STICK   0x05
+#endif
+
+#ifndef INPUT_PROP_ACCELEROMETER
+#define INPUT_PROP_ACCELEROMETER0x06
+#endif


Are these definitions normally defined in input.h?


Yes.


If so, please add them to missing.h, where we try to centralize definitions of 
this kind.


Ok, will fix for v2.


  /* we must use this kernel-compatible implementation */
  #define BITS_PER_LONG (sizeof(unsigned long) * 8)
  #define NBITS(x) x)-1)/BITS_PER_LONG)+1)
@@ -131,6 +139,7 @@ static void test_pointers (struct udev_device *dev,
 const unsigned long* bitmask_abs,
 const unsigned long* bitmask_key,
 const unsigned long* bitmask_rel,
+   unsigned long prop,


unsigned long? Is this really necessary? Shouldn't we just use
uint64_t? here?


unsigned long matches what is used in the kernel, it is a bit field,
I do not know what type is preferred for bit fields in systemd / udev.




 bool test) {
  int is_mouse = 0;
  int is_touchpad = 0;
@@ -182,6 +191,10 @@ static void test_pointers (struct udev_device *dev,
  udev_builtin_add_property(dev, test, ID_INPUT_MOUSE, 1);
  if (is_touchpad)
  udev_builtin_add_property(dev, test, ID_INPUT_TOUCHPAD, 
1);
+if (prop  (1  INPUT_PROP_POINTING_STICK))
+udev_builtin_add_property(dev, test, ID_INPUT_TRACKPOINT, 
1);
+if (prop  (1  INPUT_PROP_ACCELEROMETER))
+udev_builtin_add_property(dev, test, ID_INPUT_ACCELEROMETER, 
1);
  }

  /* key like devices */
@@ -232,7 +245,8 @@ static int builtin_input_id(struct udev_device *dev, int 
argc, char *argv[], boo
  unsigned long bitmask_abs[NBITS(ABS_MAX)];
  unsigned long bitmask_key[NBITS(KEY_MAX)];
  unsigned long bitmask_rel[NBITS(REL_MAX)];
-const char *sysname, *devnode;
+unsigned long prop = 0;
+const char *sysname, *devnode, *prop_str;

  /* walk up the parental chain until we find the real input device; the
   * argument is very likely a subdevice of this, like eventN */
@@ -248,7 +262,10 @@ static int builtin_input_id(struct udev_device *dev, int 
argc, char *argv[], boo
  get_cap_mask(dev, pdev, capabilities/abs, bitmask_abs, 
sizeof(bitmask_abs), test);
  get_cap_mask(dev, pdev, capabilities/rel, bitmask_rel, 
sizeof(bitmask_rel), test);
  get_cap_mask(dev, pdev, capabilities/key, bitmask_key, 
sizeof(bitmask_key), test);
-test_pointers(dev, bitmask_ev, bitmask_abs, bitmask_key, 
bitmask_rel, test);
+prop_str = udev_device_get_property_value(pdev, PROP);
+if (prop_str)
+   prop = strtoul(prop_str, NULL, 16);


Hmm, we try to avoid direct invocations of strtoul() and friends, due
to the annoying error handling... Can't we use safe_atou64() here, or so?


If that can handle hex without a 0x prefix then yes.

Regards,

Hans
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/5] udev-builtin-input_id: identify trackpoints and accelerometers

2015-04-02 Thread Hans de Goede 

Hi,

On 02-04-15 12:27, Lennart Poettering wrote:

On Thu, 02.04.15 12:21, Lennart Poettering (lenn...@poettering.net) wrote:


On Thu, 02.04.15 11:48, Hans de Goede (hdego...@redhat.com) wrote:


The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
on trackpoints for a while now, and this is useful information to have
in various places, so make input_id aware of this and make it set
ID_INPUT_POINTING_STICK on trackpoints.

While adding support for querying properties, also add support for the
recently added INPUT_PROP_ACCELEROMETER property bit.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
  src/udev/udev-builtin-input_id.c | 21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index 46f1c53..5b1790c 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -33,6 +33,14 @@
  #include udev.h
  #include util.h

+#ifndef INPUT_PROP_POINTING_STICK
+#define INPUT_PROP_POINTING_STICK   0x05
+#endif
+
+#ifndef INPUT_PROP_ACCELEROMETER
+#define INPUT_PROP_ACCELEROMETER0x06
+#endif


Are these definitions normally defined in input.h? If so, please add
them to missing.h, where we try to centralize definitions of this kind.


Also, INPUT_PROP_POINTING_STICK is already introduced by Peter
Hutterer's most recent patch. He adds it to missing.h, can you sync
the two patche sets up, please?


Sure I'll base my set on top of Peters for V2.

Regards,

Hans
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/5] udev-builtin-input_id: identify trackpoints and accelerometers

2015-04-02 Thread Lennart Poettering 
On Thu, 02.04.15 12:21, Lennart Poettering (lenn...@poettering.net) wrote:

 On Thu, 02.04.15 11:48, Hans de Goede (hdego...@redhat.com) wrote:
 
  The kernel has been setting the INPUT_PROP_POINTING_STICK property bit
  on trackpoints for a while now, and this is useful information to have
  in various places, so make input_id aware of this and make it set
  ID_INPUT_POINTING_STICK on trackpoints.
  
  While adding support for querying properties, also add support for the
  recently added INPUT_PROP_ACCELEROMETER property bit.
  
  Signed-off-by: Hans de Goede hdego...@redhat.com
  ---
   src/udev/udev-builtin-input_id.c | 21 +++--
   1 file changed, 19 insertions(+), 2 deletions(-)
  
  diff --git a/src/udev/udev-builtin-input_id.c 
  b/src/udev/udev-builtin-input_id.c
  index 46f1c53..5b1790c 100644
  --- a/src/udev/udev-builtin-input_id.c
  +++ b/src/udev/udev-builtin-input_id.c
  @@ -33,6 +33,14 @@
   #include udev.h
   #include util.h
   
  +#ifndef INPUT_PROP_POINTING_STICK
  +#define INPUT_PROP_POINTING_STICK   0x05
  +#endif
  +
  +#ifndef INPUT_PROP_ACCELEROMETER
  +#define INPUT_PROP_ACCELEROMETER0x06
  +#endif
 
 Are these definitions normally defined in input.h? If so, please add
 them to missing.h, where we try to centralize definitions of this kind.

Also, INPUT_PROP_POINTING_STICK is already introduced by Peter
Hutterer's most recent patch. He adds it to missing.h, can you sync
the two patche sets up, please?

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/5] udev-builtin-input_id: identify trackpoints and accelerometers

2015-04-02 Thread Lennart Poettering 
On Thu, 02.04.15 12:39, Hans de Goede (hdego...@redhat.com) wrote:

   /* we must use this kernel-compatible implementation */
   #define BITS_PER_LONG (sizeof(unsigned long) * 8)
   #define NBITS(x) x)-1)/BITS_PER_LONG)+1)
 @@ -131,6 +139,7 @@ static void test_pointers (struct udev_device *dev,
  const unsigned long* bitmask_abs,
  const unsigned long* bitmask_key,
  const unsigned long* bitmask_rel,
 +   unsigned long prop,
 
 unsigned long? Is this really necessary? Shouldn't we just use
 uint64_t? here?
 
 unsigned long matches what is used in the kernel, it is a bit field,
 I do not know what type is preferred for bit fields in systemd /
 udev.

Oh god, the kernel is stupid. Using variable size types for
kernel/userspace APIs is just stupid...

Anyway, if that's how it is, use unsigned long, and also use strtoul
then, as before...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel