* [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute @ 2014-03-20 10:12 Hans de Goede 2014-03-20 10:12 ` [PATCH 1/2] " Hans de Goede ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Hans de Goede @ 2014-03-20 10:12 UTC (permalink / raw) To: Dmitry Torokhov Cc: Matthew Garrett, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input Hi All, For firmware instantiated serio devices, such as devices instantiated through ACPI, it may be useful for userspace to know the firmware-id (pnp-id in case of ACPI) through which the device was instantiated. One concrete example of this is the new ps/2 touchpads found in Lenovo Thinkpad X240 T440 and T540 laptops, which not only have a special bottom right click area to emulate right clicks, but also top middle and top right areas to emulate middle and right clicks for the trackpoint. Lenove has given these touchpads a unique LENxxxx pnp-id and we would like to use this to automatically enable emulation of middle and right buttons in the top area of the clickpad. Matthew Garret wrote a patch to set the parent of the serio port to the /sys/devices/pnp0/00:xx device so that we could get the necessary info that way, but Dmitry rightfully pointed out that that would break suspend/resume ordering, see: https://lkml.org/lkml/2014/2/23/63 https://lkml.org/lkml/2014/3/7/428 So while discussing this with Peter Hutterer I made the plan to add a /dev/devices/platform/i8042/serioX/firmware_parent symlink pointing to the relevant /sys/devices/pnp0/00:xx device, and add a little udev-helper + rules file to read this symlink and add an attribute to the udevdb with the pnp-id this way. This however is harder then it sounds, I spend a couple of hours on how to do this in a race free manner and I could not come up with one. The problem is that the sysfs_create_symlink call needs to be done after the device_add call, at which point an add uevent has already been fired. Causing a theoretical race where the udev-helper would not find the symlink, this can be fixed with an extra change event after adding the symlink, but then any higher userspace levels need to re-check on a change event as well. So after spending too much time on this I decided to go for an alternative solution. Which in the end turns out to be much nicer too, since it gets rid of needing a udev-helper too. After this much too long introduction I'll let the patches speak for themselves. With this patch-set one can now do: [hans@shalem linux]$ udevadm info -p /devices/platform/i8042/serio1 P: /devices/platform/i8042/serio1 E: DEVPATH=/devices/platform/i8042/serio1 E: MODALIAS=serio:ty01pr00id00ex00 E: SERIO_EXTRA=00 E: SERIO_FIRMWARE_ID=PNP0f03 PNP0f13 E: SERIO_ID=00 E: SERIO_PROTO=00 E: SERIO_TYPE=01 E: SUBSYSTEM=serio And the info we need is just there in the new SERIO_FIRMWARE_ID attribute, for completeness the patch-set also adds: [hans@shalem linux]$ cat /sys/devices/platform/i8042/serio1/firmware_id PNP0f03 PNP0f13 Regards, Hans ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] input/serio: Add a firmware_id sysfs attribute 2014-03-20 10:12 [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute Hans de Goede @ 2014-03-20 10:12 ` Hans de Goede 2014-03-20 10:12 ` [PATCH 2/2] input/serio/8042: Add firmware_id support Hans de Goede 2014-03-20 17:21 ` [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute Matthew Garrett 2 siblings, 0 replies; 17+ messages in thread From: Hans de Goede @ 2014-03-20 10:12 UTC (permalink / raw) To: Dmitry Torokhov Cc: Matthew Garrett, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input, Hans de Goede serio devices exposed via platform firmware interfaces such as ACPI may provide additional identifying information of use to userspace. We don't associate the serio devices with the firmware device (we don't set it as parent), so there's no way for userspace to make use of this information. We cannot change the parent for serio devices instantiated though a firmware interface as that would break suspend / resume ordering. Therefor this patch adds a new firmware_id sysfs attribute so that userspace can get a string from there with any additional identifying information the firmware interface may provide. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/input/serio/serio.c | 12 ++++++++++++ include/linux/serio.h | 1 + 2 files changed, 13 insertions(+) diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c index 8f4c4ab..1788a4d 100644 --- a/drivers/input/serio/serio.c +++ b/drivers/input/serio/serio.c @@ -451,6 +451,13 @@ static ssize_t serio_set_bind_mode(struct device *dev, struct device_attribute * return retval; } +static ssize_t firmware_id_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct serio *serio = to_serio_port(dev); + + return sprintf(buf, "%s\n", serio->firmware_id); +} + static DEVICE_ATTR_RO(type); static DEVICE_ATTR_RO(proto); static DEVICE_ATTR_RO(id); @@ -473,12 +480,14 @@ static DEVICE_ATTR_RO(modalias); static DEVICE_ATTR_WO(drvctl); static DEVICE_ATTR(description, S_IRUGO, serio_show_description, NULL); static DEVICE_ATTR(bind_mode, S_IWUSR | S_IRUGO, serio_show_bind_mode, serio_set_bind_mode); +static DEVICE_ATTR_RO(firmware_id); static struct attribute *serio_device_attrs[] = { &dev_attr_modalias.attr, &dev_attr_description.attr, &dev_attr_drvctl.attr, &dev_attr_bind_mode.attr, + &dev_attr_firmware_id.attr, NULL }; @@ -923,6 +932,9 @@ static int serio_uevent(struct device *dev, struct kobj_uevent_env *env) SERIO_ADD_UEVENT_VAR("SERIO_EXTRA=%02x", serio->id.extra); SERIO_ADD_UEVENT_VAR("MODALIAS=serio:ty%02Xpr%02Xid%02Xex%02X", serio->id.type, serio->id.proto, serio->id.id, serio->id.extra); + if (serio->firmware_id[0]) + SERIO_ADD_UEVENT_VAR("SERIO_FIRMWARE_ID=%s", + serio->firmware_id); return 0; } diff --git a/include/linux/serio.h b/include/linux/serio.h index 36aac73..9f779c7 100644 --- a/include/linux/serio.h +++ b/include/linux/serio.h @@ -23,6 +23,7 @@ struct serio { char name[32]; char phys[32]; + char firmware_id[128]; bool manual_bind; -- 1.9.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] input/serio/8042: Add firmware_id support 2014-03-20 10:12 [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute Hans de Goede 2014-03-20 10:12 ` [PATCH 1/2] " Hans de Goede @ 2014-03-20 10:12 ` Hans de Goede 2014-03-20 17:21 ` [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute Matthew Garrett 2 siblings, 0 replies; 17+ messages in thread From: Hans de Goede @ 2014-03-20 10:12 UTC (permalink / raw) To: Dmitry Torokhov Cc: Matthew Garrett, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input, Hans de Goede Fill in the new serio firmware_id sysfs attribute for pnp instantiated 8042 serio ports. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/input/serio/i8042-x86ia64io.h | 26 ++++++++++++++++++++++++++ drivers/input/serio/i8042.c | 6 ++++++ 2 files changed, 32 insertions(+) diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h index 0ec9abb..3f9da83 100644 --- a/drivers/input/serio/i8042-x86ia64io.h +++ b/drivers/input/serio/i8042-x86ia64io.h @@ -704,6 +704,8 @@ static char i8042_pnp_aux_name[32]; static int i8042_pnp_kbd_probe(struct pnp_dev *dev, const struct pnp_device_id *did) { + struct pnp_id *id = dev->id; + if (pnp_port_valid(dev, 0) && pnp_port_len(dev, 0) == 1) i8042_pnp_data_reg = pnp_port_start(dev,0); @@ -719,6 +721,17 @@ static int i8042_pnp_kbd_probe(struct pnp_dev *dev, const struct pnp_device_id * strlcat(i8042_pnp_kbd_name, pnp_dev_name(dev), sizeof(i8042_pnp_kbd_name)); } + if (id) { + strlcpy(i8042_kbd_firmware_id, id->id, + sizeof(i8042_kbd_firmware_id)); + for (id = id->next; id; id = id->next) { + strlcat(i8042_kbd_firmware_id, " ", + sizeof(i8042_kbd_firmware_id)); + strlcat(i8042_kbd_firmware_id, id->id, + sizeof(i8042_kbd_firmware_id)); + } + } + /* Keyboard ports are always supposed to be wakeup-enabled */ device_set_wakeup_enable(&dev->dev, true); @@ -728,6 +741,8 @@ static int i8042_pnp_kbd_probe(struct pnp_dev *dev, const struct pnp_device_id * static int i8042_pnp_aux_probe(struct pnp_dev *dev, const struct pnp_device_id *did) { + struct pnp_id *id = dev->id; + if (pnp_port_valid(dev, 0) && pnp_port_len(dev, 0) == 1) i8042_pnp_data_reg = pnp_port_start(dev,0); @@ -743,6 +758,17 @@ static int i8042_pnp_aux_probe(struct pnp_dev *dev, const struct pnp_device_id * strlcat(i8042_pnp_aux_name, pnp_dev_name(dev), sizeof(i8042_pnp_aux_name)); } + if (id) { + strlcpy(i8042_aux_firmware_id, id->id, + sizeof(i8042_aux_firmware_id)); + for (id = id->next; id; id = id->next) { + strlcat(i8042_aux_firmware_id, " ", + sizeof(i8042_aux_firmware_id)); + strlcat(i8042_aux_firmware_id, id->id, + sizeof(i8042_aux_firmware_id)); + } + } + i8042_pnp_aux_devices++; return 0; } diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c index 020053f..3807c3e 100644 --- a/drivers/input/serio/i8042.c +++ b/drivers/input/serio/i8042.c @@ -87,6 +87,8 @@ MODULE_PARM_DESC(debug, "Turn i8042 debugging mode on and off"); #endif static bool i8042_bypass_aux_irq_test; +static char i8042_kbd_firmware_id[128]; +static char i8042_aux_firmware_id[128]; #include "i8042.h" @@ -1218,6 +1220,8 @@ static int __init i8042_create_kbd_port(void) serio->dev.parent = &i8042_platform_device->dev; strlcpy(serio->name, "i8042 KBD port", sizeof(serio->name)); strlcpy(serio->phys, I8042_KBD_PHYS_DESC, sizeof(serio->phys)); + strlcpy(serio->firmware_id, i8042_kbd_firmware_id, + sizeof(serio->firmware_id)); port->serio = serio; port->irq = I8042_KBD_IRQ; @@ -1244,6 +1248,8 @@ static int __init i8042_create_aux_port(int idx) if (idx < 0) { strlcpy(serio->name, "i8042 AUX port", sizeof(serio->name)); strlcpy(serio->phys, I8042_AUX_PHYS_DESC, sizeof(serio->phys)); + strlcpy(serio->firmware_id, i8042_aux_firmware_id, + sizeof(serio->firmware_id)); serio->close = i8042_port_close; } else { snprintf(serio->name, sizeof(serio->name), "i8042 AUX%d port", idx); -- 1.9.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-20 10:12 [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute Hans de Goede 2014-03-20 10:12 ` [PATCH 1/2] " Hans de Goede 2014-03-20 10:12 ` [PATCH 2/2] input/serio/8042: Add firmware_id support Hans de Goede @ 2014-03-20 17:21 ` Matthew Garrett 2014-03-24 1:07 ` Peter Hutterer 2014-03-28 7:56 ` Dmitry Torokhov 2 siblings, 2 replies; 17+ messages in thread From: Matthew Garrett @ 2014-03-20 17:21 UTC (permalink / raw) To: Hans de Goede Cc: Dmitry Torokhov, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote: > Which in the end turns out to be much nicer too, since it gets rid of needing > a udev-helper too. After this much too long introduction I'll let the patches > speak for themselves. Yeah, I was coming to the conclusion that this was probably the best we could do. It's unfortunate that "id" is already in use - we'd be able to get away without any X server modifications otherwise. Long term we probably still want to tie serio devices to the ACPI devices in case the vendor provides power management calls there, but we can leave that until there's an actual example. -- Matthew Garrett | mjg59@srcf.ucam.org ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-20 17:21 ` [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute Matthew Garrett @ 2014-03-24 1:07 ` Peter Hutterer 2014-03-28 7:56 ` Dmitry Torokhov 1 sibling, 0 replies; 17+ messages in thread From: Peter Hutterer @ 2014-03-24 1:07 UTC (permalink / raw) To: Matthew Garrett, Hans de Goede Cc: Dmitry Torokhov, Benjamin Tissoires, platform-driver-x86, linux-input On 21/03/14 03:21, Matthew Garrett wrote: > On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote: > >> Which in the end turns out to be much nicer too, since it gets rid of needing >> a udev-helper too. After this much too long introduction I'll let the patches >> speak for themselves. > > Yeah, I was coming to the conclusion that this was probably the best we > could do. It's unfortunate that "id" is already in use - we'd be able to > get away without any X server modifications otherwise. IMO we don't need to worry about that. this is needed for a set of devices that need a kernel patch, several synaptics patches and a server patch to work properly anyway, so changes are already required. Approach looks good, ACK from me. Cheers, Peter > > Long term we probably still want to tie serio devices to the ACPI > devices in case the vendor provides power management calls there, but we > can leave that until there's an actual example. > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-20 17:21 ` [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute Matthew Garrett 2014-03-24 1:07 ` Peter Hutterer @ 2014-03-28 7:56 ` Dmitry Torokhov 2014-03-28 8:12 ` Hans de Goede 2014-03-28 8:20 ` Matthew Garrett 1 sibling, 2 replies; 17+ messages in thread From: Dmitry Torokhov @ 2014-03-28 7:56 UTC (permalink / raw) To: Matthew Garrett Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input On Thu, Mar 20, 2014 at 05:21:59PM +0000, Matthew Garrett wrote: > On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote: > > > Which in the end turns out to be much nicer too, since it gets rid of needing > > a udev-helper too. After this much too long introduction I'll let the patches > > speak for themselves. > > Yeah, I was coming to the conclusion that this was probably the best we > could do. It's unfortunate that "id" is already in use - we'd be able to > get away without any X server modifications otherwise. > > Long term we probably still want to tie serio devices to the ACPI > devices in case the vendor provides power management calls there, but we > can leave that until there's an actual example. I am still unsure if we shoudl be adding these new IDs to serio core... Can't the X driver take a peek at ACPI devices on it's own? Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-28 7:56 ` Dmitry Torokhov @ 2014-03-28 8:12 ` Hans de Goede 2014-03-28 8:17 ` Dmitry Torokhov 2014-03-28 8:20 ` Matthew Garrett 1 sibling, 1 reply; 17+ messages in thread From: Hans de Goede @ 2014-03-28 8:12 UTC (permalink / raw) To: Dmitry Torokhov, Matthew Garrett Cc: Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input Hi, On 03/28/2014 08:56 AM, Dmitry Torokhov wrote: > On Thu, Mar 20, 2014 at 05:21:59PM +0000, Matthew Garrett wrote: >> On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote: >> >>> Which in the end turns out to be much nicer too, since it gets rid of needing >>> a udev-helper too. After this much too long introduction I'll let the patches >>> speak for themselves. >> >> Yeah, I was coming to the conclusion that this was probably the best we >> could do. It's unfortunate that "id" is already in use - we'd be able to >> get away without any X server modifications otherwise. >> >> Long term we probably still want to tie serio devices to the ACPI >> devices in case the vendor provides power management calls there, but we >> can leave that until there's an actual example. > > I am still unsure if we shoudl be adding these new IDs to serio core... > Can't the X driver take a peek at ACPI devices on it's own? The problem is there is no way for userspace to know which /sys/devices/pnp0/00:xx device is the serio bus host. Regards, Hans ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-28 8:12 ` Hans de Goede @ 2014-03-28 8:17 ` Dmitry Torokhov 2014-03-28 8:29 ` Hans de Goede 0 siblings, 1 reply; 17+ messages in thread From: Dmitry Torokhov @ 2014-03-28 8:17 UTC (permalink / raw) To: Hans de Goede Cc: Matthew Garrett, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input On Fri, Mar 28, 2014 at 09:12:43AM +0100, Hans de Goede wrote: > Hi, > > On 03/28/2014 08:56 AM, Dmitry Torokhov wrote: > > On Thu, Mar 20, 2014 at 05:21:59PM +0000, Matthew Garrett wrote: > >> On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote: > >> > >>> Which in the end turns out to be much nicer too, since it gets rid of needing > >>> a udev-helper too. After this much too long introduction I'll let the patches > >>> speak for themselves. > >> > >> Yeah, I was coming to the conclusion that this was probably the best we > >> could do. It's unfortunate that "id" is already in use - we'd be able to > >> get away without any X server modifications otherwise. > >> > >> Long term we probably still want to tie serio devices to the ACPI > >> devices in case the vendor provides power management calls there, but we > >> can leave that until there's an actual example. > > > > I am still unsure if we shoudl be adding these new IDs to serio core... > > Can't the X driver take a peek at ACPI devices on it's own? > > The problem is there is no way for userspace to know which /sys/devices/pnp0/00:xx > device is the serio bus host. Practically speaking you should not care - there is only one touchpad in Lenovos. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-28 8:17 ` Dmitry Torokhov @ 2014-03-28 8:29 ` Hans de Goede 2014-03-28 8:52 ` Dmitry Torokhov 0 siblings, 1 reply; 17+ messages in thread From: Hans de Goede @ 2014-03-28 8:29 UTC (permalink / raw) To: Dmitry Torokhov Cc: Matthew Garrett, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input Hi, On 03/28/2014 09:17 AM, Dmitry Torokhov wrote: > On Fri, Mar 28, 2014 at 09:12:43AM +0100, Hans de Goede wrote: >> Hi, >> >> On 03/28/2014 08:56 AM, Dmitry Torokhov wrote: >>> On Thu, Mar 20, 2014 at 05:21:59PM +0000, Matthew Garrett wrote: >>>> On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote: >>>> >>>>> Which in the end turns out to be much nicer too, since it gets rid of needing >>>>> a udev-helper too. After this much too long introduction I'll let the patches >>>>> speak for themselves. >>>> >>>> Yeah, I was coming to the conclusion that this was probably the best we >>>> could do. It's unfortunate that "id" is already in use - we'd be able to >>>> get away without any X server modifications otherwise. >>>> >>>> Long term we probably still want to tie serio devices to the ACPI >>>> devices in case the vendor provides power management calls there, but we >>>> can leave that until there's an actual example. >>> >>> I am still unsure if we shoudl be adding these new IDs to serio core... >>> Can't the X driver take a peek at ACPI devices on it's own? >> >> The problem is there is no way for userspace to know which /sys/devices/pnp0/00:xx >> device is the serio bus host. > > Practically speaking you should not care - there is only one touchpad in > Lenovos. So are you suggesting we simply go over all /sys/devices/pnp0/00:xx devices looking for a pnp-id we're interested in ? I'm sorry but that is just a non-sense solution, which reminds me of the good old days of random poking io-ports to probe stuff. We're not blindly going to read every /sys/devices/pnp0/00:xx/id attribute on a system, assuming that if it contains a pnp-id we're interested in it happens to belong to the input device we're enumerating at that time. Regards, Hans ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-28 8:29 ` Hans de Goede @ 2014-03-28 8:52 ` Dmitry Torokhov 2014-03-28 9:00 ` Matthew Garrett 2014-03-28 9:05 ` Hans de Goede 0 siblings, 2 replies; 17+ messages in thread From: Dmitry Torokhov @ 2014-03-28 8:52 UTC (permalink / raw) To: Hans de Goede Cc: Matthew Garrett, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input On Fri, Mar 28, 2014 at 09:29:50AM +0100, Hans de Goede wrote: > Hi, > > On 03/28/2014 09:17 AM, Dmitry Torokhov wrote: > > On Fri, Mar 28, 2014 at 09:12:43AM +0100, Hans de Goede wrote: > >> Hi, > >> > >> On 03/28/2014 08:56 AM, Dmitry Torokhov wrote: > >>> On Thu, Mar 20, 2014 at 05:21:59PM +0000, Matthew Garrett wrote: > >>>> On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote: > >>>> > >>>>> Which in the end turns out to be much nicer too, since it gets rid of needing > >>>>> a udev-helper too. After this much too long introduction I'll let the patches > >>>>> speak for themselves. > >>>> > >>>> Yeah, I was coming to the conclusion that this was probably the best we > >>>> could do. It's unfortunate that "id" is already in use - we'd be able to > >>>> get away without any X server modifications otherwise. > >>>> > >>>> Long term we probably still want to tie serio devices to the ACPI > >>>> devices in case the vendor provides power management calls there, but we > >>>> can leave that until there's an actual example. > >>> > >>> I am still unsure if we shoudl be adding these new IDs to serio core... > >>> Can't the X driver take a peek at ACPI devices on it's own? > >> > >> The problem is there is no way for userspace to know which /sys/devices/pnp0/00:xx > >> device is the serio bus host. > > > > Practically speaking you should not care - there is only one touchpad in > > Lenovos. > > So are you suggesting we simply go over all /sys/devices/pnp0/00:xx devices looking > for a pnp-id we're interested in ? I'm sorry but that is just a non-sense solution, > which reminds me of the good old days of random poking io-ports to probe stuff. > > We're not blindly going to read every /sys/devices/pnp0/00:xx/id attribute on a > system, assuming that if it contains a pnp-id we're interested in it happens to > belong to the input device we're enumerating at that time. Are we even certain that they will be consistent in use of these special PNP ID's? Maybe you should really do DMI match... Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-28 8:52 ` Dmitry Torokhov @ 2014-03-28 9:00 ` Matthew Garrett 2014-03-28 16:04 ` Dmitry Torokhov 2014-03-28 9:05 ` Hans de Goede 1 sibling, 1 reply; 17+ messages in thread From: Matthew Garrett @ 2014-03-28 9:00 UTC (permalink / raw) To: Dmitry Torokhov Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input On Fri, Mar 28, 2014 at 01:52:07AM -0700, Dmitry Torokhov wrote: > > Are we even certain that they will be consistent in use of these special > PNP ID's? Maybe you should really do DMI match... The Windows drivers bind on PNP IDs, not DMI. -- Matthew Garrett | mjg59@srcf.ucam.org ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-28 9:00 ` Matthew Garrett @ 2014-03-28 16:04 ` Dmitry Torokhov 0 siblings, 0 replies; 17+ messages in thread From: Dmitry Torokhov @ 2014-03-28 16:04 UTC (permalink / raw) To: Matthew Garrett Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input On Fri, Mar 28, 2014 at 09:00:53AM +0000, Matthew Garrett wrote: > On Fri, Mar 28, 2014 at 01:52:07AM -0700, Dmitry Torokhov wrote: > > > > Are we even certain that they will be consistent in use of these special > > PNP ID's? Maybe you should really do DMI match... > > The Windows drivers bind on PNP IDs, not DMI. OK, fair enough. -- Dmitry ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-28 8:52 ` Dmitry Torokhov 2014-03-28 9:00 ` Matthew Garrett @ 2014-03-28 9:05 ` Hans de Goede 1 sibling, 0 replies; 17+ messages in thread From: Hans de Goede @ 2014-03-28 9:05 UTC (permalink / raw) To: Dmitry Torokhov Cc: Matthew Garrett, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input Hi, On 03/28/2014 09:52 AM, Dmitry Torokhov wrote: > On Fri, Mar 28, 2014 at 09:29:50AM +0100, Hans de Goede wrote: >> Hi, >> >> On 03/28/2014 09:17 AM, Dmitry Torokhov wrote: >>> On Fri, Mar 28, 2014 at 09:12:43AM +0100, Hans de Goede wrote: >>>> Hi, >>>> >>>> On 03/28/2014 08:56 AM, Dmitry Torokhov wrote: >>>>> On Thu, Mar 20, 2014 at 05:21:59PM +0000, Matthew Garrett wrote: >>>>>> On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote: >>>>>> >>>>>>> Which in the end turns out to be much nicer too, since it gets rid of needing >>>>>>> a udev-helper too. After this much too long introduction I'll let the patches >>>>>>> speak for themselves. >>>>>> >>>>>> Yeah, I was coming to the conclusion that this was probably the best we >>>>>> could do. It's unfortunate that "id" is already in use - we'd be able to >>>>>> get away without any X server modifications otherwise. >>>>>> >>>>>> Long term we probably still want to tie serio devices to the ACPI >>>>>> devices in case the vendor provides power management calls there, but we >>>>>> can leave that until there's an actual example. >>>>> >>>>> I am still unsure if we shoudl be adding these new IDs to serio core... >>>>> Can't the X driver take a peek at ACPI devices on it's own? >>>> >>>> The problem is there is no way for userspace to know which /sys/devices/pnp0/00:xx >>>> device is the serio bus host. >>> >>> Practically speaking you should not care - there is only one touchpad in >>> Lenovos. >> >> So are you suggesting we simply go over all /sys/devices/pnp0/00:xx devices looking >> for a pnp-id we're interested in ? I'm sorry but that is just a non-sense solution, >> which reminds me of the good old days of random poking io-ports to probe stuff. >> >> We're not blindly going to read every /sys/devices/pnp0/00:xx/id attribute on a >> system, assuming that if it contains a pnp-id we're interested in it happens to >> belong to the input device we're enumerating at that time. > > Are we even certain that they will be consistent in use of these special > PNP ID's? Maybe you should really do DMI match... They are more consistent with PNP ids then with their DMI strings, that is if they re-use the exact same touchpad the PNP id stays the same. So PNP ids give us a better chance of things just working without the user needing to wait for a distro update which has the new DMI strings in there. And yes so far the PNP ids use we want to-do is Lenovo only, as we've just discovered that they are actually storing some useful info in there. If we need quirks for other vendor laptops in the future, chances are we may want to do pnp-id matching there too. I've deliberately tried to write this patch-set so that it adds a generic way to export extra info the platform specific code may have. I could have added a pnp-id pointer to the serio struct, but I deliberately went with a free-form string so as to make this as generic as possible. As Matthew has already said we may want to also export extra info from devicetree through this mechanism for example. Regards, Hans ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-28 7:56 ` Dmitry Torokhov 2014-03-28 8:12 ` Hans de Goede @ 2014-03-28 8:20 ` Matthew Garrett 2014-03-28 8:24 ` Dmitry Torokhov 1 sibling, 1 reply; 17+ messages in thread From: Matthew Garrett @ 2014-03-28 8:20 UTC (permalink / raw) To: Dmitry Torokhov Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input On Fri, Mar 28, 2014 at 12:56:55AM -0700, Dmitry Torokhov wrote: > I am still unsure if we shoudl be adding these new IDs to serio core... > Can't the X driver take a peek at ACPI devices on it's own? In the (admittedly unlikely) event of multiple PS/2 trackpads, how do you know which one corresponds to which ACPI device? -- Matthew Garrett | mjg59@srcf.ucam.org ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-28 8:20 ` Matthew Garrett @ 2014-03-28 8:24 ` Dmitry Torokhov 2014-03-28 8:27 ` Matthew Garrett 0 siblings, 1 reply; 17+ messages in thread From: Dmitry Torokhov @ 2014-03-28 8:24 UTC (permalink / raw) To: Matthew Garrett Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input On Fri, Mar 28, 2014 at 08:20:04AM +0000, Matthew Garrett wrote: > On Fri, Mar 28, 2014 at 12:56:55AM -0700, Dmitry Torokhov wrote: > > > I am still unsure if we shoudl be adding these new IDs to serio core... > > Can't the X driver take a peek at ACPI devices on it's own? > > In the (admittedly unlikely) event of multiple PS/2 trackpads, how do > you know which one corresponds to which ACPI device? So far I have not seen a single instance of a laptop with 2 touchpads and I doubt external PS/2 ones will ever make come back. -- Dmitry ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-28 8:24 ` Dmitry Torokhov @ 2014-03-28 8:27 ` Matthew Garrett 2014-03-28 8:50 ` Dmitry Torokhov 0 siblings, 1 reply; 17+ messages in thread From: Matthew Garrett @ 2014-03-28 8:27 UTC (permalink / raw) To: Dmitry Torokhov Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input On Fri, Mar 28, 2014 at 01:24:52AM -0700, Dmitry Torokhov wrote: > On Fri, Mar 28, 2014 at 08:20:04AM +0000, Matthew Garrett wrote: > > On Fri, Mar 28, 2014 at 12:56:55AM -0700, Dmitry Torokhov wrote: > > > > > I am still unsure if we shoudl be adding these new IDs to serio core... > > > Can't the X driver take a peek at ACPI devices on it's own? > > > > In the (admittedly unlikely) event of multiple PS/2 trackpads, how do > > you know which one corresponds to which ACPI device? > > So far I have not seen a single instance of a laptop with 2 touchpads > and I doubt external PS/2 ones will ever make come back. Right, we can hack around it based on what we've seen so far. But it seems more attractive to fix it in such a way that we won't behave inappropriately even if someone does do something utterly unexpected in future. For instance, some ARM devices have i8042-like serio - if the underlying device is exposed in device tree, it'd be nice to be able to expose that to userspace without having to modify the core X code. -- Matthew Garrett | mjg59@srcf.ucam.org ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute 2014-03-28 8:27 ` Matthew Garrett @ 2014-03-28 8:50 ` Dmitry Torokhov 0 siblings, 0 replies; 17+ messages in thread From: Dmitry Torokhov @ 2014-03-28 8:50 UTC (permalink / raw) To: Matthew Garrett Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer, platform-driver-x86, linux-input On Fri, Mar 28, 2014 at 08:27:04AM +0000, Matthew Garrett wrote: > On Fri, Mar 28, 2014 at 01:24:52AM -0700, Dmitry Torokhov wrote: > > On Fri, Mar 28, 2014 at 08:20:04AM +0000, Matthew Garrett wrote: > > > On Fri, Mar 28, 2014 at 12:56:55AM -0700, Dmitry Torokhov wrote: > > > > > > > I am still unsure if we shoudl be adding these new IDs to serio core... > > > > Can't the X driver take a peek at ACPI devices on it's own? > > > > > > In the (admittedly unlikely) event of multiple PS/2 trackpads, how do > > > you know which one corresponds to which ACPI device? > > > > So far I have not seen a single instance of a laptop with 2 touchpads > > and I doubt external PS/2 ones will ever make come back. > > Right, we can hack around it based on what we've seen so far. But it > seems more attractive to fix it in such a way that we won't behave > inappropriately even if someone does do something utterly unexpected in > future. For instance, some ARM devices have i8042-like serio - if the > underlying device is exposed in device tree, it'd be nice to be able to > expose that to userspace without having to modify the core X code. I wonder if on ARM they have the same split description for AUX and KBD ports or of they have proper i8042 parent. ACPI way of describing PS/2 devices is ugly - keyboard gets ports and mice/touchpads are portless and everyone knows that they should use the same as keyboard. I am unconvinced that just storing a string as firmware ID would solve anything other than quirky Lenovos. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2014-03-28 16:04 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-03-20 10:12 [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute Hans de Goede 2014-03-20 10:12 ` [PATCH 1/2] " Hans de Goede 2014-03-20 10:12 ` [PATCH 2/2] input/serio/8042: Add firmware_id support Hans de Goede 2014-03-20 17:21 ` [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute Matthew Garrett 2014-03-24 1:07 ` Peter Hutterer 2014-03-28 7:56 ` Dmitry Torokhov 2014-03-28 8:12 ` Hans de Goede 2014-03-28 8:17 ` Dmitry Torokhov 2014-03-28 8:29 ` Hans de Goede 2014-03-28 8:52 ` Dmitry Torokhov 2014-03-28 9:00 ` Matthew Garrett 2014-03-28 16:04 ` Dmitry Torokhov 2014-03-28 9:05 ` Hans de Goede 2014-03-28 8:20 ` Matthew Garrett 2014-03-28 8:24 ` Dmitry Torokhov 2014-03-28 8:27 ` Matthew Garrett 2014-03-28 8:50 ` Dmitry Torokhov
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.