| Message ID | 1397033270-29597-3-git-send-email-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 09, 2014 at 10:47:50AM +0200, Hans de Goede wrote: > Fill in the new serio firmware_id sysfs attribute for pnp instantiated > 8042 serio ports. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > Acked-by: Peter Hutterer <peter.hutterer@who-t.net> > --- > 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)); Do we need all IDs? I'd expect we only interested in HID, not CIDs? Thanks.
On Wed, Apr 09, 2014 at 11:24:34AM -0700, Dmitry Torokhov wrote:
> Do we need all IDs? I'd expect we only interested in HID, not CIDs?
I think HID handles the cases we've seen so far, but we could imagine a
system vendor providing their own HID, a trackpad vendor's CID and then
the generic mouse CID. It seems better to err on the side of including
them.
On Wed, Apr 09, 2014 at 07:29:26PM +0100, Matthew Garrett wrote: > On Wed, Apr 09, 2014 at 11:24:34AM -0700, Dmitry Torokhov wrote: > > > Do we need all IDs? I'd expect we only interested in HID, not CIDs? > > I think HID handles the cases we've seen so far, but we could imagine a > system vendor providing their own HID, a trackpad vendor's CID and then > the generic mouse CID. It seems better to err on the side of including > them. OK, fair enough. Another question - do we want to prefix IDs with "PNP:" prefix so that if we add device tree in the future we'll know what kind of IDs we are dealing with?
Hi, On 04/09/2014 10:09 PM, Dmitry Torokhov wrote: > On Wed, Apr 09, 2014 at 07:29:26PM +0100, Matthew Garrett wrote: >> On Wed, Apr 09, 2014 at 11:24:34AM -0700, Dmitry Torokhov wrote: >> >>> Do we need all IDs? I'd expect we only interested in HID, not CIDs? >> >> I think HID handles the cases we've seen so far, but we could imagine a >> system vendor providing their own HID, a trackpad vendor's CID and then >> the generic mouse CID. It seems better to err on the side of including >> them. > > OK, fair enough. Another question - do we want to prefix IDs with "PNP:" > prefix so that if we add device tree in the future we'll know what kind > of IDs we are dealing with? I'm a bit divided on this, adding a "PNP: " prefix will make it a bit harder to parse, OTOH once we will have other users like devicetree knowing where the info comes from will be very useful. To me in the end the latter argument wins. Let me know if you agree and I'll do a v3 adding the PNP: prefix. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 10, 2014 at 11:02:17AM +0200, Hans de Goede wrote: > Hi, > > On 04/09/2014 10:09 PM, Dmitry Torokhov wrote: > > On Wed, Apr 09, 2014 at 07:29:26PM +0100, Matthew Garrett wrote: > >> On Wed, Apr 09, 2014 at 11:24:34AM -0700, Dmitry Torokhov wrote: > >> > >>> Do we need all IDs? I'd expect we only interested in HID, not CIDs? > >> > >> I think HID handles the cases we've seen so far, but we could imagine a > >> system vendor providing their own HID, a trackpad vendor's CID and then > >> the generic mouse CID. It seems better to err on the side of including > >> them. > > > > OK, fair enough. Another question - do we want to prefix IDs with "PNP:" > > prefix so that if we add device tree in the future we'll know what kind > > of IDs we are dealing with? > > I'm a bit divided on this, adding a "PNP: " prefix will make it a bit harder > to parse, OTOH once we will have other users like devicetree knowing where > the info comes from will be very useful. To me in the end the latter argument > wins. Let me know if you agree and I'll do a v3 adding the PNP: prefix. Yes please.
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);