Message ID | 1480414104-8524-8-git-send-email-benjamin.tissoires@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Benjamin, On Tue, Nov 29, 2016 at 11:08:18AM +0100, Benjamin Tissoires wrote: > From: Lyude Paul <thatslyude@gmail.com> > > This adds basic functionality for PS/2 passthrough on Synaptics > Touchpads using RMI4 through smbus. > > Reviewed-by: Andrew Duggan <aduggan@synaptics.com> > Signed-off-by: Lyude Paul <thatslyude@gmail.com> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > --- > drivers/input/mouse/psmouse-base.c | 6 + > drivers/input/rmi4/Kconfig | 9 ++ > drivers/input/rmi4/Makefile | 1 + > drivers/input/rmi4/rmi_bus.c | 3 + > drivers/input/rmi4/rmi_driver.h | 1 + > drivers/input/rmi4/rmi_f03.c | 227 +++++++++++++++++++++++++++++++++++++ > include/uapi/linux/serio.h | 1 + > 7 files changed, 248 insertions(+) > create mode 100644 drivers/input/rmi4/rmi_f03.c > > diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c > index fb4b185..691dd74 100644 > --- a/drivers/input/mouse/psmouse-base.c > +++ b/drivers/input/mouse/psmouse-base.c > @@ -1663,6 +1663,12 @@ static struct serio_device_id psmouse_serio_ids[] = { > .id = SERIO_ANY, > .extra = SERIO_ANY, > }, > + { > + .type = SERIO_RMI_PSTHRU, Why do we need new serio type here? We had SERIO_PS_PSTHRU because we needed some quirks in how it interacted with the parent PS/2 port, but here it seems SERIO_I8042 (which could be called SERIO_STANDARD_PS2) would suffice? > + .proto = SERIO_ANY, > + .id = SERIO_ANY, > + .extra = SERIO_ANY, > + }, > { 0 } > }; > > diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig > index a9c36a5..b8189a3 100644 > --- a/drivers/input/rmi4/Kconfig > +++ b/drivers/input/rmi4/Kconfig > @@ -39,6 +39,15 @@ config RMI4_SMB > To compile this driver as a module, choose M here: the module will be > called rmi_smbus. > > +config RMI4_F03 > + bool "RMI4 Function 03 (PS2 Guest)" > + depends on RMI4_CORE > + help > + Say Y here if you want to add support for RMI4 function 03. > + > + Function 03 provides PS2 guest support for RMI4 devices. This > + includes support for TrackPoints on TouchPads. > + > config RMI4_2D_SENSOR > bool > depends on RMI4_CORE > diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile > index e7f4ca6..a199cbe 100644 > --- a/drivers/input/rmi4/Makefile > +++ b/drivers/input/rmi4/Makefile > @@ -4,6 +4,7 @@ rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o > rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o > > # Function drivers > +rmi_core-$(CONFIG_RMI4_F03) += rmi_f03.o > rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o > rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o > rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o > diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c > index 81be9c1..1c40d94 100644 > --- a/drivers/input/rmi4/rmi_bus.c > +++ b/drivers/input/rmi4/rmi_bus.c > @@ -305,6 +305,9 @@ struct bus_type rmi_bus_type = { > > static struct rmi_function_handler *fn_handlers[] = { > &rmi_f01_handler, > +#ifdef CONFIG_RMI4_F03 > + &rmi_f03_handler, > +#endif > #ifdef CONFIG_RMI4_F11 > &rmi_f11_handler, > #endif > diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h > index cc94585..24f8f76 100644 > --- a/drivers/input/rmi4/rmi_driver.h > +++ b/drivers/input/rmi4/rmi_driver.h > @@ -121,6 +121,7 @@ static inline void rmi_f34_remove_sysfs(struct rmi_device *rmi_dev) > #endif /* CONFIG_RMI_F34 */ > > extern struct rmi_function_handler rmi_f01_handler; > +extern struct rmi_function_handler rmi_f03_handler; > extern struct rmi_function_handler rmi_f11_handler; > extern struct rmi_function_handler rmi_f12_handler; > extern struct rmi_function_handler rmi_f30_handler; > diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c > new file mode 100644 > index 0000000..a7e1b98 > --- /dev/null > +++ b/drivers/input/rmi4/rmi_f03.c > @@ -0,0 +1,227 @@ > +/* > + * Copyright (C) 2015-2016 Red Hat > + * Copyright (C) 2015 Lyude Paul <thatslyude@gmail.com> > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License version 2 as published by > + * the Free Software Foundation. > + */ > + > +#include <linux/kernel.h> > +#include <linux/slab.h> > +#include <linux/serio.h> > +#include <linux/notifier.h> > +#include "rmi_driver.h" > + > +#define RMI_F03_RX_DATA_OFB 0x01 > +#define RMI_F03_OB_SIZE 2 > + > +#define RMI_F03_OB_OFFSET 2 > +#define RMI_F03_OB_DATA_OFFSET 1 > +#define RMI_F03_OB_FLAG_TIMEOUT (1 << 6) > +#define RMI_F03_OB_FLAG_PARITY (1 << 7) BIT()? > + > +#define RMI_F03_DEVICE_COUNT 0x07 > +#define RMI_F03_BYTES_PER_DEVICE_MASK 0x70 > +#define RMI_F03_BYTES_PER_DEVICE_SHIFT 4 > +#define RMI_F03_QUEUE_LENGTH 0x0F > + > +struct f03_data { > + struct rmi_function *fn; > + > + struct serio *serio; > + > + unsigned int overwrite_buttons; Unused? > + > + u8 device_count; > + u8 rx_queue_length; > +}; > + > +static int rmi_f03_pt_write(struct serio *id, unsigned char val) > +{ > + struct f03_data *f03 = id->port_data; > + int rc; > + > + rmi_dbg(RMI_DEBUG_FN, &f03->fn->dev, > + "%s: Wrote %.2hhx to PS/2 passthrough address", > + __func__, val); > + > + rc = rmi_write(f03->fn->rmi_dev, f03->fn->fd.data_base_addr, val); error = rmi_write()? > + if (rc) { > + dev_err(&f03->fn->dev, > + "%s: Failed to write to F03 TX register.\n", __func__); Please report error code as well. > + return rc; > + } > + > + return 0; > +} > + > +static inline int rmi_f03_initialize(struct rmi_function *fn) Why inline? > +{ > + struct f03_data *f03; > + struct device *dev = &fn->dev; > + int rc; > + u8 bytes_per_device; > + u8 query1; > + size_t query2_len; > + > + rc = rmi_read(fn->rmi_dev, fn->fd.query_base_addr, &query1); error = rmi_read()? > + if (rc) { > + dev_err(dev, "Failed to read query register.\n"); > + return rc; > + } > + > + f03 = devm_kzalloc(dev, sizeof(struct f03_data), GFP_KERNEL); > + if (!f03) > + return -ENOMEM; > + > + f03->device_count = query1 & RMI_F03_DEVICE_COUNT; > + bytes_per_device = (query1 & RMI_F03_BYTES_PER_DEVICE_MASK) >> > + RMI_F03_BYTES_PER_DEVICE_SHIFT; > + > + query2_len = f03->device_count * bytes_per_device; > + > + /* > + * The first generation of image sensors don't have a second part to > + * their f03 query, as such we have to set some of these values manually > + */ > + if (query2_len < 1) { > + f03->device_count = 1; > + f03->rx_queue_length = 7; > + } else { > + u8 query2[query2_len]; > + > + rc = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr + 1, > + query2, query2_len); > + if (rc) { > + dev_err(dev, "Failed to read second set of query registers.\n"); > + return rc; > + } > + > + f03->rx_queue_length = query2[0] & RMI_F03_QUEUE_LENGTH; > + } > + > + f03->fn = fn; > + > + dev_set_drvdata(dev, f03); > + > + return f03->device_count; I'd rather we returned customary 0 here. > +} > + > +static inline int rmi_f03_register_pt(struct rmi_function *fn) > +{ > + struct f03_data *f03 = dev_get_drvdata(&fn->dev); > + struct serio *serio = kzalloc(sizeof(struct serio), GFP_KERNEL); Please do not do allocations at declaration; limit to declarations with initialization to operations without side effect. So: struct serio *serio; serio = kzalloc(sizeof(struct serio), GFP_KERNEL); if (!serio) return -ENOMEM; > + > + if (!serio) > + return -ENOMEM; > + > + serio->id.type = SERIO_RMI_PSTHRU; > + serio->write = rmi_f03_pt_write; > + serio->port_data = f03; > + > + strlcpy(serio->name, "Synaptics RMI4 PS2 pass-through", > + sizeof(serio->name)); > + strlcpy(serio->phys, "synaptics-rmi4-pt/serio1", > + sizeof(serio->phys)); > + serio->dev.parent = &fn->dev; Extra space after tab in indentation. > + > + f03->serio = serio; > + > + serio_register_port(serio); > + > + return 0; > +} > + > +static int rmi_f03_probe(struct rmi_function *fn) > +{ > + int rc; int error; Maybe allocate the memory here... > + > + rc = rmi_f03_initialize(fn); > + if (rc < 0) > + return rc; > + > + rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%d devices on PS/2 passthrough", rc); so you can use f03->device_count here. Do we need to warn if we see device count greater than 1? > + > + rc = rmi_f03_register_pt(fn); > + if (rc) > + return rc; > + > + return 0; > +} > + > +static int rmi_f03_config(struct rmi_function *fn) > +{ > + fn->rmi_dev->driver->set_irq_bits(fn->rmi_dev, fn->irq_mask); > + > + return 0; > +} > + > +static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits) > +{ > + struct f03_data *f03 = dev_get_drvdata(&fn->dev); > + u16 data_addr = fn->fd.data_base_addr; > + const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE; > + u8 obs[ob_len]; > + u8 ob_status; > + u8 ob_data; > + unsigned int serio_flags; > + int i; > + int retval; > + > + /* Grab all of the data registers, and check them for data */ > + retval = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET, > + &obs, ob_len); > + if (retval) { > + dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n", > + __func__); > + serio_interrupt(f03->serio, 0, SERIO_TIMEOUT); > + return retval; > + } > + > + for (i = 0; i < ob_len; i += RMI_F03_OB_SIZE) { > + ob_status = obs[i]; > + ob_data = obs[i + RMI_F03_OB_DATA_OFFSET]; > + serio_flags = 0; > + > + if (!(ob_status & RMI_F03_RX_DATA_OFB)) > + continue; > + > + if (ob_status & RMI_F03_OB_FLAG_TIMEOUT) > + serio_flags |= SERIO_TIMEOUT; > + if (ob_status & RMI_F03_OB_FLAG_PARITY) > + serio_flags |= SERIO_PARITY; > + > + rmi_dbg(RMI_DEBUG_FN, &fn->dev, > + "%s: Received %.2hhx from PS2 guest T: %c P: %c\n", > + __func__, ob_data, > + serio_flags & SERIO_TIMEOUT ? 'Y' : 'N', > + serio_flags & SERIO_PARITY ? 'Y' : 'N'); > + > + serio_interrupt(f03->serio, ob_data, serio_flags); > + } > + > + return 0; > +} > + > +static void rmi_f03_remove(struct rmi_function *fn) > +{ > + struct f03_data *f03 = dev_get_drvdata(&fn->dev); > + > + serio_unregister_port(f03->serio); > +} > + > +struct rmi_function_handler rmi_f03_handler = { > + .driver = { > + .name = "rmi4_f03", > + }, > + .func = 0x03, > + .probe = rmi_f03_probe, > + .config = rmi_f03_config, > + .attention = rmi_f03_attention, > + .remove = rmi_f03_remove, > +}; > + > +MODULE_AUTHOR("Lyude Paul <thatslyude@gmail.com>"); > +MODULE_DESCRIPTION("RMI F03 module"); > +MODULE_LICENSE("GPL"); > diff --git a/include/uapi/linux/serio.h b/include/uapi/linux/serio.h > index f2447a8..7012178 100644 > --- a/include/uapi/linux/serio.h > +++ b/include/uapi/linux/serio.h > @@ -30,6 +30,7 @@ > #define SERIO_HIL_MLC 0x03 > #define SERIO_PS_PSTHRU 0x05 > #define SERIO_8042_XL 0x06 > +#define SERIO_RMI_PSTHRU 0x07 > > /* > * Serio protocols > -- > 2.7.4 > Thanks.
Hi Dmitry, On Nov 30 2016 or thereabouts, Dmitry Torokhov wrote: > Hi Benjamin, > > On Tue, Nov 29, 2016 at 11:08:18AM +0100, Benjamin Tissoires wrote: > > From: Lyude Paul <thatslyude@gmail.com> > > > > This adds basic functionality for PS/2 passthrough on Synaptics > > Touchpads using RMI4 through smbus. > > > > Reviewed-by: Andrew Duggan <aduggan@synaptics.com> > > Signed-off-by: Lyude Paul <thatslyude@gmail.com> > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > > > --- > > drivers/input/mouse/psmouse-base.c | 6 + > > drivers/input/rmi4/Kconfig | 9 ++ > > drivers/input/rmi4/Makefile | 1 + > > drivers/input/rmi4/rmi_bus.c | 3 + > > drivers/input/rmi4/rmi_driver.h | 1 + > > drivers/input/rmi4/rmi_f03.c | 227 +++++++++++++++++++++++++++++++++++++ > > include/uapi/linux/serio.h | 1 + > > 7 files changed, 248 insertions(+) > > create mode 100644 drivers/input/rmi4/rmi_f03.c > > > > diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c > > index fb4b185..691dd74 100644 > > --- a/drivers/input/mouse/psmouse-base.c > > +++ b/drivers/input/mouse/psmouse-base.c > > @@ -1663,6 +1663,12 @@ static struct serio_device_id psmouse_serio_ids[] = { > > .id = SERIO_ANY, > > .extra = SERIO_ANY, > > }, > > + { > > + .type = SERIO_RMI_PSTHRU, > > Why do we need new serio type here? We had SERIO_PS_PSTHRU because we > needed some quirks in how it interacted with the parent PS/2 port, but > here it seems SERIO_I8042 (which could be called SERIO_STANDARD_PS2) > would suffice? I guess this must be some kind of left over from the time we were trying to fix the suspend/resume interactions between this driver and the PS/2 connection of the rmi-smbus devices. Given the rest of the code which seems to be indeed not dependent of SERIO_RMI_PSTHRU, I'll switch over to SERIO_I8042. > > > + .proto = SERIO_ANY, > > + .id = SERIO_ANY, > > + .extra = SERIO_ANY, > > + }, > > { 0 } > > }; > > > > diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig > > index a9c36a5..b8189a3 100644 > > --- a/drivers/input/rmi4/Kconfig > > +++ b/drivers/input/rmi4/Kconfig > > @@ -39,6 +39,15 @@ config RMI4_SMB > > To compile this driver as a module, choose M here: the module will be > > called rmi_smbus. > > > > +config RMI4_F03 > > + bool "RMI4 Function 03 (PS2 Guest)" > > + depends on RMI4_CORE > > + help > > + Say Y here if you want to add support for RMI4 function 03. > > + > > + Function 03 provides PS2 guest support for RMI4 devices. This > > + includes support for TrackPoints on TouchPads. > > + > > config RMI4_2D_SENSOR > > bool > > depends on RMI4_CORE > > diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile > > index e7f4ca6..a199cbe 100644 > > --- a/drivers/input/rmi4/Makefile > > +++ b/drivers/input/rmi4/Makefile > > @@ -4,6 +4,7 @@ rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o > > rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o > > > > # Function drivers > > +rmi_core-$(CONFIG_RMI4_F03) += rmi_f03.o > > rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o > > rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o > > rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o > > diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c > > index 81be9c1..1c40d94 100644 > > --- a/drivers/input/rmi4/rmi_bus.c > > +++ b/drivers/input/rmi4/rmi_bus.c > > @@ -305,6 +305,9 @@ struct bus_type rmi_bus_type = { > > > > static struct rmi_function_handler *fn_handlers[] = { > > &rmi_f01_handler, > > +#ifdef CONFIG_RMI4_F03 > > + &rmi_f03_handler, > > +#endif > > #ifdef CONFIG_RMI4_F11 > > &rmi_f11_handler, > > #endif > > diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h > > index cc94585..24f8f76 100644 > > --- a/drivers/input/rmi4/rmi_driver.h > > +++ b/drivers/input/rmi4/rmi_driver.h > > @@ -121,6 +121,7 @@ static inline void rmi_f34_remove_sysfs(struct rmi_device *rmi_dev) > > #endif /* CONFIG_RMI_F34 */ > > > > extern struct rmi_function_handler rmi_f01_handler; > > +extern struct rmi_function_handler rmi_f03_handler; > > extern struct rmi_function_handler rmi_f11_handler; > > extern struct rmi_function_handler rmi_f12_handler; > > extern struct rmi_function_handler rmi_f30_handler; > > diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c > > new file mode 100644 > > index 0000000..a7e1b98 > > --- /dev/null > > +++ b/drivers/input/rmi4/rmi_f03.c > > @@ -0,0 +1,227 @@ > > +/* > > + * Copyright (C) 2015-2016 Red Hat > > + * Copyright (C) 2015 Lyude Paul <thatslyude@gmail.com> > > + * > > + * This program is free software; you can redistribute it and/or modify it > > + * under the terms of the GNU General Public License version 2 as published by > > + * the Free Software Foundation. > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/slab.h> > > +#include <linux/serio.h> > > +#include <linux/notifier.h> > > +#include "rmi_driver.h" > > + > > +#define RMI_F03_RX_DATA_OFB 0x01 > > +#define RMI_F03_OB_SIZE 2 > > + > > +#define RMI_F03_OB_OFFSET 2 > > +#define RMI_F03_OB_DATA_OFFSET 1 > > +#define RMI_F03_OB_FLAG_TIMEOUT (1 << 6) > > +#define RMI_F03_OB_FLAG_PARITY (1 << 7) > > BIT()? > > > + > > +#define RMI_F03_DEVICE_COUNT 0x07 > > +#define RMI_F03_BYTES_PER_DEVICE_MASK 0x70 > > +#define RMI_F03_BYTES_PER_DEVICE_SHIFT 4 > > +#define RMI_F03_QUEUE_LENGTH 0x0F > > + > > +struct f03_data { > > + struct rmi_function *fn; > > + > > + struct serio *serio; > > + > > + unsigned int overwrite_buttons; > > Unused? Looks like it's a bad split between the F03 core implementation and the tweaks we have to do for the trackstick "buttons-that-are-wired-into-the- touchpad-instead-of-the-trackstick". > > > + > > + u8 device_count; > > + u8 rx_queue_length; > > +}; > > + > > +static int rmi_f03_pt_write(struct serio *id, unsigned char val) > > +{ > > + struct f03_data *f03 = id->port_data; > > + int rc; > > + > > + rmi_dbg(RMI_DEBUG_FN, &f03->fn->dev, > > + "%s: Wrote %.2hhx to PS/2 passthrough address", > > + __func__, val); > > + > > + rc = rmi_write(f03->fn->rmi_dev, f03->fn->fd.data_base_addr, val); > > error = rmi_write()? > > > + if (rc) { > > + dev_err(&f03->fn->dev, > > + "%s: Failed to write to F03 TX register.\n", __func__); > > Please report error code as well. > > > + return rc; > > + } > > + > > + return 0; > > +} > > + > > +static inline int rmi_f03_initialize(struct rmi_function *fn) > > Why inline? > > > +{ > > + struct f03_data *f03; > > + struct device *dev = &fn->dev; > > + int rc; > > + u8 bytes_per_device; > > + u8 query1; > > + size_t query2_len; > > + > > + rc = rmi_read(fn->rmi_dev, fn->fd.query_base_addr, &query1); > > error = rmi_read()? > > > + if (rc) { > > + dev_err(dev, "Failed to read query register.\n"); > > + return rc; > > + } > > + > > + f03 = devm_kzalloc(dev, sizeof(struct f03_data), GFP_KERNEL); > > + if (!f03) > > + return -ENOMEM; > > + > > + f03->device_count = query1 & RMI_F03_DEVICE_COUNT; > > + bytes_per_device = (query1 & RMI_F03_BYTES_PER_DEVICE_MASK) >> > > + RMI_F03_BYTES_PER_DEVICE_SHIFT; > > + > > + query2_len = f03->device_count * bytes_per_device; > > + > > + /* > > + * The first generation of image sensors don't have a second part to > > + * their f03 query, as such we have to set some of these values manually > > + */ > > + if (query2_len < 1) { > > + f03->device_count = 1; > > + f03->rx_queue_length = 7; > > + } else { > > + u8 query2[query2_len]; > > + > > + rc = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr + 1, > > + query2, query2_len); > > + if (rc) { > > + dev_err(dev, "Failed to read second set of query registers.\n"); > > + return rc; > > + } > > + > > + f03->rx_queue_length = query2[0] & RMI_F03_QUEUE_LENGTH; > > + } > > + > > + f03->fn = fn; > > + > > + dev_set_drvdata(dev, f03); > > + > > + return f03->device_count; > > I'd rather we returned customary 0 here. > > > +} > > + > > +static inline int rmi_f03_register_pt(struct rmi_function *fn) > > +{ > > + struct f03_data *f03 = dev_get_drvdata(&fn->dev); > > + struct serio *serio = kzalloc(sizeof(struct serio), GFP_KERNEL); > > Please do not do allocations at declaration; limit to declarations with > initialization to operations without side effect. So: > > struct serio *serio; > > serio = kzalloc(sizeof(struct serio), GFP_KERNEL); > if (!serio) > return -ENOMEM; > > > + > > + if (!serio) > > + return -ENOMEM; > > + > > + serio->id.type = SERIO_RMI_PSTHRU; > > + serio->write = rmi_f03_pt_write; > > + serio->port_data = f03; > > + > > + strlcpy(serio->name, "Synaptics RMI4 PS2 pass-through", > > + sizeof(serio->name)); > > + strlcpy(serio->phys, "synaptics-rmi4-pt/serio1", > > + sizeof(serio->phys)); > > + serio->dev.parent = &fn->dev; > > Extra space after tab in indentation. > > > + > > + f03->serio = serio; > > + > > + serio_register_port(serio); > > + > > + return 0; > > +} > > + > > +static int rmi_f03_probe(struct rmi_function *fn) > > +{ > > + int rc; > > int error; > > Maybe allocate the memory here... > > > + > > + rc = rmi_f03_initialize(fn); > > + if (rc < 0) > > + return rc; > > + > > + rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%d devices on PS/2 passthrough", rc); > > so you can use f03->device_count here. > > Do we need to warn if we see device count greater than 1? Good quesstion. I can't recall the exact protocol, and we certainly never seen anything with more than one device. So probably yes, we should shout something if it's not what we expect. > > > + > > + rc = rmi_f03_register_pt(fn); > > + if (rc) > > + return rc; > > + > > + return 0; > > +} > > + > > +static int rmi_f03_config(struct rmi_function *fn) > > +{ > > + fn->rmi_dev->driver->set_irq_bits(fn->rmi_dev, fn->irq_mask); > > + > > + return 0; > > +} > > + > > +static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits) > > +{ > > + struct f03_data *f03 = dev_get_drvdata(&fn->dev); > > + u16 data_addr = fn->fd.data_base_addr; > > + const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE; > > + u8 obs[ob_len]; > > + u8 ob_status; > > + u8 ob_data; > > + unsigned int serio_flags; > > + int i; > > + int retval; > > + > > + /* Grab all of the data registers, and check them for data */ > > + retval = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET, > > + &obs, ob_len); > > + if (retval) { > > + dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n", > > + __func__); > > + serio_interrupt(f03->serio, 0, SERIO_TIMEOUT); > > + return retval; > > + } > > + > > + for (i = 0; i < ob_len; i += RMI_F03_OB_SIZE) { > > + ob_status = obs[i]; > > + ob_data = obs[i + RMI_F03_OB_DATA_OFFSET]; > > + serio_flags = 0; > > + > > + if (!(ob_status & RMI_F03_RX_DATA_OFB)) > > + continue; > > + > > + if (ob_status & RMI_F03_OB_FLAG_TIMEOUT) > > + serio_flags |= SERIO_TIMEOUT; > > + if (ob_status & RMI_F03_OB_FLAG_PARITY) > > + serio_flags |= SERIO_PARITY; > > + > > + rmi_dbg(RMI_DEBUG_FN, &fn->dev, > > + "%s: Received %.2hhx from PS2 guest T: %c P: %c\n", > > + __func__, ob_data, > > + serio_flags & SERIO_TIMEOUT ? 'Y' : 'N', > > + serio_flags & SERIO_PARITY ? 'Y' : 'N'); > > + > > + serio_interrupt(f03->serio, ob_data, serio_flags); > > + } > > + > > + return 0; > > +} > > + > > +static void rmi_f03_remove(struct rmi_function *fn) > > +{ > > + struct f03_data *f03 = dev_get_drvdata(&fn->dev); > > + > > + serio_unregister_port(f03->serio); > > +} > > + > > +struct rmi_function_handler rmi_f03_handler = { > > + .driver = { > > + .name = "rmi4_f03", > > + }, > > + .func = 0x03, > > + .probe = rmi_f03_probe, > > + .config = rmi_f03_config, > > + .attention = rmi_f03_attention, > > + .remove = rmi_f03_remove, > > +}; > > + > > +MODULE_AUTHOR("Lyude Paul <thatslyude@gmail.com>"); > > +MODULE_DESCRIPTION("RMI F03 module"); > > +MODULE_LICENSE("GPL"); > > diff --git a/include/uapi/linux/serio.h b/include/uapi/linux/serio.h > > index f2447a8..7012178 100644 > > --- a/include/uapi/linux/serio.h > > +++ b/include/uapi/linux/serio.h > > @@ -30,6 +30,7 @@ > > #define SERIO_HIL_MLC 0x03 > > #define SERIO_PS_PSTHRU 0x05 > > #define SERIO_8042_XL 0x06 > > +#define SERIO_RMI_PSTHRU 0x07 > > > > /* > > * Serio protocols > > -- > > 2.7.4 > > > > Thanks. > I saw that you pushed part of the series. Many thanks for that. I'll work on the fixes for this one and submit v2 ASAP. Cheers, Benjamin > -- > Dmitry -- 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
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c index fb4b185..691dd74 100644 --- a/drivers/input/mouse/psmouse-base.c +++ b/drivers/input/mouse/psmouse-base.c @@ -1663,6 +1663,12 @@ static struct serio_device_id psmouse_serio_ids[] = { .id = SERIO_ANY, .extra = SERIO_ANY, }, + { + .type = SERIO_RMI_PSTHRU, + .proto = SERIO_ANY, + .id = SERIO_ANY, + .extra = SERIO_ANY, + }, { 0 } }; diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig index a9c36a5..b8189a3 100644 --- a/drivers/input/rmi4/Kconfig +++ b/drivers/input/rmi4/Kconfig @@ -39,6 +39,15 @@ config RMI4_SMB To compile this driver as a module, choose M here: the module will be called rmi_smbus. +config RMI4_F03 + bool "RMI4 Function 03 (PS2 Guest)" + depends on RMI4_CORE + help + Say Y here if you want to add support for RMI4 function 03. + + Function 03 provides PS2 guest support for RMI4 devices. This + includes support for TrackPoints on TouchPads. + config RMI4_2D_SENSOR bool depends on RMI4_CORE diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile index e7f4ca6..a199cbe 100644 --- a/drivers/input/rmi4/Makefile +++ b/drivers/input/rmi4/Makefile @@ -4,6 +4,7 @@ rmi_core-y := rmi_bus.o rmi_driver.o rmi_f01.o rmi_core-$(CONFIG_RMI4_2D_SENSOR) += rmi_2d_sensor.o # Function drivers +rmi_core-$(CONFIG_RMI4_F03) += rmi_f03.o rmi_core-$(CONFIG_RMI4_F11) += rmi_f11.o rmi_core-$(CONFIG_RMI4_F12) += rmi_f12.o rmi_core-$(CONFIG_RMI4_F30) += rmi_f30.o diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c index 81be9c1..1c40d94 100644 --- a/drivers/input/rmi4/rmi_bus.c +++ b/drivers/input/rmi4/rmi_bus.c @@ -305,6 +305,9 @@ struct bus_type rmi_bus_type = { static struct rmi_function_handler *fn_handlers[] = { &rmi_f01_handler, +#ifdef CONFIG_RMI4_F03 + &rmi_f03_handler, +#endif #ifdef CONFIG_RMI4_F11 &rmi_f11_handler, #endif diff --git a/drivers/input/rmi4/rmi_driver.h b/drivers/input/rmi4/rmi_driver.h index cc94585..24f8f76 100644 --- a/drivers/input/rmi4/rmi_driver.h +++ b/drivers/input/rmi4/rmi_driver.h @@ -121,6 +121,7 @@ static inline void rmi_f34_remove_sysfs(struct rmi_device *rmi_dev) #endif /* CONFIG_RMI_F34 */ extern struct rmi_function_handler rmi_f01_handler; +extern struct rmi_function_handler rmi_f03_handler; extern struct rmi_function_handler rmi_f11_handler; extern struct rmi_function_handler rmi_f12_handler; extern struct rmi_function_handler rmi_f30_handler; diff --git a/drivers/input/rmi4/rmi_f03.c b/drivers/input/rmi4/rmi_f03.c new file mode 100644 index 0000000..a7e1b98 --- /dev/null +++ b/drivers/input/rmi4/rmi_f03.c @@ -0,0 +1,227 @@ +/* + * Copyright (C) 2015-2016 Red Hat + * Copyright (C) 2015 Lyude Paul <thatslyude@gmail.com> + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + */ + +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/serio.h> +#include <linux/notifier.h> +#include "rmi_driver.h" + +#define RMI_F03_RX_DATA_OFB 0x01 +#define RMI_F03_OB_SIZE 2 + +#define RMI_F03_OB_OFFSET 2 +#define RMI_F03_OB_DATA_OFFSET 1 +#define RMI_F03_OB_FLAG_TIMEOUT (1 << 6) +#define RMI_F03_OB_FLAG_PARITY (1 << 7) + +#define RMI_F03_DEVICE_COUNT 0x07 +#define RMI_F03_BYTES_PER_DEVICE_MASK 0x70 +#define RMI_F03_BYTES_PER_DEVICE_SHIFT 4 +#define RMI_F03_QUEUE_LENGTH 0x0F + +struct f03_data { + struct rmi_function *fn; + + struct serio *serio; + + unsigned int overwrite_buttons; + + u8 device_count; + u8 rx_queue_length; +}; + +static int rmi_f03_pt_write(struct serio *id, unsigned char val) +{ + struct f03_data *f03 = id->port_data; + int rc; + + rmi_dbg(RMI_DEBUG_FN, &f03->fn->dev, + "%s: Wrote %.2hhx to PS/2 passthrough address", + __func__, val); + + rc = rmi_write(f03->fn->rmi_dev, f03->fn->fd.data_base_addr, val); + if (rc) { + dev_err(&f03->fn->dev, + "%s: Failed to write to F03 TX register.\n", __func__); + return rc; + } + + return 0; +} + +static inline int rmi_f03_initialize(struct rmi_function *fn) +{ + struct f03_data *f03; + struct device *dev = &fn->dev; + int rc; + u8 bytes_per_device; + u8 query1; + size_t query2_len; + + rc = rmi_read(fn->rmi_dev, fn->fd.query_base_addr, &query1); + if (rc) { + dev_err(dev, "Failed to read query register.\n"); + return rc; + } + + f03 = devm_kzalloc(dev, sizeof(struct f03_data), GFP_KERNEL); + if (!f03) + return -ENOMEM; + + f03->device_count = query1 & RMI_F03_DEVICE_COUNT; + bytes_per_device = (query1 & RMI_F03_BYTES_PER_DEVICE_MASK) >> + RMI_F03_BYTES_PER_DEVICE_SHIFT; + + query2_len = f03->device_count * bytes_per_device; + + /* + * The first generation of image sensors don't have a second part to + * their f03 query, as such we have to set some of these values manually + */ + if (query2_len < 1) { + f03->device_count = 1; + f03->rx_queue_length = 7; + } else { + u8 query2[query2_len]; + + rc = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr + 1, + query2, query2_len); + if (rc) { + dev_err(dev, "Failed to read second set of query registers.\n"); + return rc; + } + + f03->rx_queue_length = query2[0] & RMI_F03_QUEUE_LENGTH; + } + + f03->fn = fn; + + dev_set_drvdata(dev, f03); + + return f03->device_count; +} + +static inline int rmi_f03_register_pt(struct rmi_function *fn) +{ + struct f03_data *f03 = dev_get_drvdata(&fn->dev); + struct serio *serio = kzalloc(sizeof(struct serio), GFP_KERNEL); + + if (!serio) + return -ENOMEM; + + serio->id.type = SERIO_RMI_PSTHRU; + serio->write = rmi_f03_pt_write; + serio->port_data = f03; + + strlcpy(serio->name, "Synaptics RMI4 PS2 pass-through", + sizeof(serio->name)); + strlcpy(serio->phys, "synaptics-rmi4-pt/serio1", + sizeof(serio->phys)); + serio->dev.parent = &fn->dev; + + f03->serio = serio; + + serio_register_port(serio); + + return 0; +} + +static int rmi_f03_probe(struct rmi_function *fn) +{ + int rc; + + rc = rmi_f03_initialize(fn); + if (rc < 0) + return rc; + + rmi_dbg(RMI_DEBUG_FN, &fn->dev, "%d devices on PS/2 passthrough", rc); + + rc = rmi_f03_register_pt(fn); + if (rc) + return rc; + + return 0; +} + +static int rmi_f03_config(struct rmi_function *fn) +{ + fn->rmi_dev->driver->set_irq_bits(fn->rmi_dev, fn->irq_mask); + + return 0; +} + +static int rmi_f03_attention(struct rmi_function *fn, unsigned long *irq_bits) +{ + struct f03_data *f03 = dev_get_drvdata(&fn->dev); + u16 data_addr = fn->fd.data_base_addr; + const u8 ob_len = f03->rx_queue_length * RMI_F03_OB_SIZE; + u8 obs[ob_len]; + u8 ob_status; + u8 ob_data; + unsigned int serio_flags; + int i; + int retval; + + /* Grab all of the data registers, and check them for data */ + retval = rmi_read_block(fn->rmi_dev, data_addr + RMI_F03_OB_OFFSET, + &obs, ob_len); + if (retval) { + dev_err(&fn->dev, "%s: Failed to read F03 output buffers.\n", + __func__); + serio_interrupt(f03->serio, 0, SERIO_TIMEOUT); + return retval; + } + + for (i = 0; i < ob_len; i += RMI_F03_OB_SIZE) { + ob_status = obs[i]; + ob_data = obs[i + RMI_F03_OB_DATA_OFFSET]; + serio_flags = 0; + + if (!(ob_status & RMI_F03_RX_DATA_OFB)) + continue; + + if (ob_status & RMI_F03_OB_FLAG_TIMEOUT) + serio_flags |= SERIO_TIMEOUT; + if (ob_status & RMI_F03_OB_FLAG_PARITY) + serio_flags |= SERIO_PARITY; + + rmi_dbg(RMI_DEBUG_FN, &fn->dev, + "%s: Received %.2hhx from PS2 guest T: %c P: %c\n", + __func__, ob_data, + serio_flags & SERIO_TIMEOUT ? 'Y' : 'N', + serio_flags & SERIO_PARITY ? 'Y' : 'N'); + + serio_interrupt(f03->serio, ob_data, serio_flags); + } + + return 0; +} + +static void rmi_f03_remove(struct rmi_function *fn) +{ + struct f03_data *f03 = dev_get_drvdata(&fn->dev); + + serio_unregister_port(f03->serio); +} + +struct rmi_function_handler rmi_f03_handler = { + .driver = { + .name = "rmi4_f03", + }, + .func = 0x03, + .probe = rmi_f03_probe, + .config = rmi_f03_config, + .attention = rmi_f03_attention, + .remove = rmi_f03_remove, +}; + +MODULE_AUTHOR("Lyude Paul <thatslyude@gmail.com>"); +MODULE_DESCRIPTION("RMI F03 module"); +MODULE_LICENSE("GPL"); diff --git a/include/uapi/linux/serio.h b/include/uapi/linux/serio.h index f2447a8..7012178 100644 --- a/include/uapi/linux/serio.h +++ b/include/uapi/linux/serio.h @@ -30,6 +30,7 @@ #define SERIO_HIL_MLC 0x03 #define SERIO_PS_PSTHRU 0x05 #define SERIO_8042_XL 0x06 +#define SERIO_RMI_PSTHRU 0x07 /* * Serio protocols