| Message ID | 20170310230114.788-7-dmitry.torokhov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mar 10 2017 or thereabouts, Dmitry Torokhov wrote: > From: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > This provides glue between PS/2 devices that enumerate the RMI4 devices > and Elan touchpads to the RMI4 (or Elan) SMBus driver. > > The SMBus devices keep their PS/2 connection alive. If the initialization > process goes too far (psmouse_activate called), the device disconnects > from the I2C bus and stays on the PS/2 bus, that is why we explicitly > disable PS/2 device reporting (by calling psmouse_deactivate) before > trying to register SMBus companion device. > > The HID over I2C devices are enumerated through the ACPI DSDT, and > their PS/2 device also exports the InterTouch bit in the extended > capability 0x0C. However, the firmware keeps its I2C connection open > even after going further in the PS/2 initialization. We don't need > to take extra precautions with those device, especially because they > block their PS/2 communication when HID over I2C is used. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > --- > drivers/input/mouse/Kconfig | 4 + > drivers/input/mouse/Makefile | 2 + > drivers/input/mouse/psmouse-base.c | 16 +- > drivers/input/mouse/psmouse-smbus.c | 291 ++++++++++++++++++++++++++++++++++++ > drivers/input/mouse/psmouse.h | 37 +++++ > 5 files changed, 348 insertions(+), 2 deletions(-) > create mode 100644 drivers/input/mouse/psmouse-smbus.c > > diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig > index 096abb4ad5cd..87bde8a210c7 100644 > --- a/drivers/input/mouse/Kconfig > +++ b/drivers/input/mouse/Kconfig > @@ -171,6 +171,10 @@ config MOUSE_PS2_VMMOUSE > > If unsure, say N. > > +config MOUSE_PS2_SMBUS > + bool > + depends on MOUSE_PS2 > + > config MOUSE_SERIAL > tristate "Serial mouse" > select SERIO > diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile > index 6168b134937b..56bf0ad877c6 100644 > --- a/drivers/input/mouse/Makefile > +++ b/drivers/input/mouse/Makefile > @@ -39,6 +39,8 @@ psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT) += touchkit_ps2.o > psmouse-$(CONFIG_MOUSE_PS2_CYPRESS) += cypress_ps2.o > psmouse-$(CONFIG_MOUSE_PS2_VMMOUSE) += vmmouse.o > > +psmouse-$(CONFIG_MOUSE_PS2_SMBUS) += psmouse-smbus.o > + > elan_i2c-objs := elan_i2c_core.o > elan_i2c-$(CONFIG_MOUSE_ELAN_I2C_I2C) += elan_i2c_i2c.o > elan_i2c-$(CONFIG_MOUSE_ELAN_I2C_SMBUS) += elan_i2c_smbus.o > diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c > index 40f09ce84f14..bdb48b2acc57 100644 > --- a/drivers/input/mouse/psmouse-base.c > +++ b/drivers/input/mouse/psmouse-base.c > @@ -1996,16 +1996,27 @@ static int __init psmouse_init(void) > synaptics_module_init(); > hgpk_module_init(); > > + err = psmouse_smbus_module_init(); > + if (err) > + return err; > + > kpsmoused_wq = alloc_ordered_workqueue("kpsmoused", 0); > if (!kpsmoused_wq) { > pr_err("failed to create kpsmoused workqueue\n"); > - return -ENOMEM; > + err = -ENOMEM; > + goto err_smbus_exit; > } > > err = serio_register_driver(&psmouse_drv); > if (err) > - destroy_workqueue(kpsmoused_wq); > + goto err_destroy_wq; > > + return 0; > + > +err_destroy_wq: > + destroy_workqueue(kpsmoused_wq); > +err_smbus_exit: > + psmouse_smbus_module_exit(); > return err; > } > > @@ -2013,6 +2024,7 @@ static void __exit psmouse_exit(void) > { > serio_unregister_driver(&psmouse_drv); > destroy_workqueue(kpsmoused_wq); > + psmouse_smbus_module_exit(); > } > > module_init(psmouse_init); > diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c > new file mode 100644 > index 000000000000..cc716977fbf9 > --- /dev/null > +++ b/drivers/input/mouse/psmouse-smbus.c > @@ -0,0 +1,291 @@ > +/* > + * Copyright (c) 2017 Red Hat, Inc > + * > + * 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. > + */ > + > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/libps2.h> > +#include <linux/i2c.h> > +#include <linux/serio.h> > +#include <linux/slab.h> > +#include <linux/workqueue.h> > +#include "psmouse.h" > + > +struct psmouse_smbus_dev { > + struct i2c_board_info board; > + struct psmouse *psmouse; > + struct i2c_client *client; > + struct list_head node; > + bool dead; > +}; > + > +static LIST_HEAD(psmouse_smbus_list); > +static DEFINE_MUTEX(psmouse_smbus_mutex); > + > +static void psmouse_smbus_check_adapter(struct i2c_adapter *adapter) > +{ > + struct psmouse_smbus_dev *smbdev; > + > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_HOST_NOTIFY)) > + return; > + > + mutex_lock(&psmouse_smbus_mutex); > + > + list_for_each_entry(smbdev, &psmouse_smbus_list, node) { > + if (smbdev->dead) > + continue; > + > + if (smbdev->client) > + continue; > + > + if (i2c_probe_func_quick_read(adapter, smbdev->board.addr) < 0) > + continue; These 2 lines above seem to be the culprit of the non-working-on-the-t450s-while-in-bzImage :) First, i2c_probe_func_quick_read() returns a boolean, so the value can never be negative. And second, if you try to probe the device on the I2C bus while psmouse_activate has been called, the device won't answer. So we can simply ignore the call all together and let i2c_new_probed_device() checks for the validity of the device. Bonus point, if I remove the call to i2c_probe_func_quick_read(), I seem to have reliable detection of SMBus even when psmouse in compiled in the bzImage. > + > + /* Device seems to be there, let's try switching over */ > + psmouse_dbg(smbdev->psmouse, > + "SMBus companion appeared, triggering rescan\n"); > + serio_rescan(smbdev->psmouse->ps2dev.serio); > + } > + > + mutex_unlock(&psmouse_smbus_mutex); > +} > + > +static void psmouse_smbus_detach_i2c_client(struct i2c_client *client) > +{ > + struct psmouse_smbus_dev *smbdev; > + > + mutex_lock(&psmouse_smbus_mutex); > + > + list_for_each_entry(smbdev, &psmouse_smbus_list, node) { > + if (smbdev->client == client) { > + psmouse_dbg(smbdev->psmouse, > + "Marking SMBus companion %s as gone\n", > + dev_name(&smbdev->client->dev)); > + smbdev->client = NULL; > + smbdev->dead = true; > + serio_rescan(smbdev->psmouse->ps2dev.serio); > + } > + } > + > + kfree(client->dev.platform_data); > + client->dev.platform_data = NULL; > + > + mutex_unlock(&psmouse_smbus_mutex); > +} > + > +static int psmouse_smbus_notifier_call(struct notifier_block *nb, > + unsigned long action, void *data) > +{ > + struct device *dev = data; > + > + switch (action) { > + case BUS_NOTIFY_ADD_DEVICE: > + if (dev->type == &i2c_adapter_type) > + psmouse_smbus_check_adapter(to_i2c_adapter(dev)); > + break; > + > + case BUS_NOTIFY_REMOVED_DEVICE: > + if (dev->type == &i2c_client_type) > + psmouse_smbus_detach_i2c_client(to_i2c_client(dev)); > + break; > + } > + > + return 0; > +} > + > +static struct notifier_block psmouse_smbus_notifier = { > + .notifier_call = psmouse_smbus_notifier_call, > +}; > + > +static psmouse_ret_t psmouse_smbus_process_byte(struct psmouse *psmouse) > +{ > + return PSMOUSE_FULL_PACKET; > +} > + > +static int psmouse_smbus_reconnect(struct psmouse *psmouse) > +{ > + psmouse_deactivate(psmouse); > + > + return 0; > +} > + > +struct psmouse_smbus_removal_work { > + struct work_struct work; > + struct i2c_client *client; > +}; > + > +static void psmouse_smbus_remove_i2c_device(struct work_struct *work) > +{ > + struct psmouse_smbus_removal_work *rwork = > + container_of(work, struct psmouse_smbus_removal_work, work); > + > + dev_dbg(&rwork->client->dev, "destroying SMBus companion device\n"); > + i2c_unregister_device(rwork->client); > + > + kfree(rwork); > +} > + > +/* > + * This schedules removal of SMBus companion device. We have to do > + * it in a separate tread to avoid deadlocking on psmouse_mutex in > + * case the device has a trackstick (which is also driven by psmouse). > + * > + * Note that this may be racing with i2c adapter removal, but we > + * can't do anything about that: i2c automatically destroys clients > + * attached to an adapter that is being removed. This has to be > + * fixed in i2c core. > + */ > +static void psmouse_smbus_schedule_remove(struct i2c_client *client) > +{ > + struct psmouse_smbus_removal_work *rwork; > + > + rwork = kzalloc(sizeof(*rwork), GFP_KERNEL); > + if (rwork) { > + INIT_WORK(&rwork->work, psmouse_smbus_remove_i2c_device); > + rwork->client = client; > + > + schedule_work(&rwork->work); > + } > +} > + > +static void psmouse_smbus_disconnect(struct psmouse *psmouse) > +{ > + struct psmouse_smbus_dev *smbdev = psmouse->private; > + > + mutex_lock(&psmouse_smbus_mutex); > + list_del(&smbdev->node); > + mutex_unlock(&psmouse_smbus_mutex); > + > + if (smbdev->client) { > + psmouse_dbg(smbdev->psmouse, > + "posting removal request for SMBus companion %s\n", > + dev_name(&smbdev->client->dev)); > + psmouse_smbus_schedule_remove(smbdev->client); > + } > + > + kfree(smbdev); > + psmouse->private = NULL; > +} > + > +static int psmouse_smbus_create_companion(struct device *dev, void *data) > +{ > + struct psmouse_smbus_dev *smbdev = data; > + unsigned short addr_list[] = { smbdev->board.addr, I2C_CLIENT_END }; > + struct i2c_adapter *adapter; > + > + adapter = i2c_verify_adapter(dev); > + if (!adapter) > + return 0; > + > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_HOST_NOTIFY)) > + return 0; > + > + smbdev->client = i2c_new_probed_device(adapter, &smbdev->board, > + addr_list, NULL); > + if (!smbdev->client) > + return 0; > + > + /* We have our(?) device, stop iterating i2c bus. */ > + return 1; > +} > + > +void psmouse_smbus_cleanup(struct psmouse *psmouse) > +{ > + struct psmouse_smbus_dev *smbdev, *tmp; > + > + mutex_lock(&psmouse_smbus_mutex); > + > + list_for_each_entry_safe(smbdev, tmp, &psmouse_smbus_list, node) { > + if (psmouse == smbdev->psmouse) { > + list_del(&smbdev->node); > + kfree(smbdev); > + } > + } > + > + mutex_unlock(&psmouse_smbus_mutex); > +} > + > +int psmouse_smbus_init(struct psmouse *psmouse, > + const struct i2c_board_info *board, > + const void *pdata, size_t pdata_size, > + bool leave_breadcrumbs) > +{ > + struct psmouse_smbus_dev *smbdev; > + int error; > + > + smbdev = kzalloc(sizeof(*smbdev), GFP_KERNEL); > + if (!smbdev) > + return -ENOMEM; > + > + smbdev->psmouse = psmouse; > + smbdev->board = *board; > + > + smbdev->board.platform_data = kmemdup(pdata, pdata_size, GFP_KERNEL); > + if (!smbdev->board.platform_data) { > + kfree(smbdev); > + return -ENOMEM; > + } > + > + psmouse->private = smbdev; > + psmouse->protocol_handler = psmouse_smbus_process_byte; > + psmouse->reconnect = psmouse_smbus_reconnect; > + psmouse->fast_reconnect = psmouse_smbus_reconnect; > + psmouse->disconnect = psmouse_smbus_disconnect; > + psmouse->resync_time = 0; > + > + psmouse_deactivate(psmouse); > + > + mutex_lock(&psmouse_smbus_mutex); > + list_add_tail(&smbdev->node, &psmouse_smbus_list); > + mutex_unlock(&psmouse_smbus_mutex); > + > + /* Bind to already existing adapters right away */ > + error = i2c_for_each_dev(smbdev, psmouse_smbus_create_companion); > + > + if (smbdev->client) { > + /* We have our companion device */ > + return 0; > + } > + > + /* > + * If we did not create i2c device we will not need platform > + * data even if we are leaving breadcrumbs. > + */ > + kfree(smbdev->board.platform_data); > + smbdev->board.platform_data = NULL; > + > + if (error < 0 || !leave_breadcrumbs) { > + mutex_lock(&psmouse_smbus_mutex); > + list_del(&smbdev->node); > + mutex_unlock(&psmouse_smbus_mutex); > + > + kfree(smbdev); > + } > + > + return error < 0 ? error : -EAGAIN; > +} > + > +int __init psmouse_smbus_module_init(void) > +{ > + int error; > + > + error = bus_register_notifier(&i2c_bus_type, &psmouse_smbus_notifier); > + if (error) { > + pr_err("failed to register i2c bus notifier: %d\n", error); > + return error; > + } > + > + return 0; > +} > + > +void __exit psmouse_smbus_module_exit(void) This raises a new section mismatch warning: WARNING: drivers/input/mouse/psmouse.o(.init.text+0x7c): Section mismatch in reference from the function psmouse_init() to the function .exit.text:psmouse_s The function __init psmouse_init() references a function __exit psmouse_smbus_module_exit(). This is often seen when error handling in the init function uses functionality in the exit path. The fix is often to remove the __exit annotation of psmouse_smbus_module_exit() so it may be used outside an exit section. As mentioned, dropping the __exit tag solves the issue. With these 2 changes, the series is: Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Thanks again for the respin! Cheers, Benjamin > +{ > + bus_unregister_notifier(&i2c_bus_type, &psmouse_smbus_notifier); > + flush_scheduled_work(); > +} > diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h > index e853dee05e79..77fcd361c8d6 100644 > --- a/drivers/input/mouse/psmouse.h > +++ b/drivers/input/mouse/psmouse.h > @@ -209,5 +209,42 @@ static struct psmouse_attribute psmouse_attr_##_name = { \ > &(psmouse)->ps2dev.serio->dev, \ > psmouse_fmt(format), ##__VA_ARGS__) > > +#ifdef CONFIG_MOUSE_PS2_SMBUS > + > +int psmouse_smbus_module_init(void); > +void psmouse_smbus_module_exit(void); > + > +struct i2c_board_info; > + > +int psmouse_smbus_init(struct psmouse *psmouse, > + const struct i2c_board_info *board, > + const void *pdata, size_t pdata_size, > + bool leave_breadcrumbs); > +void psmouse_smbus_cleanup(struct psmouse *psmouse); > + > +#else /* !CONFIG_MOUSE_PS2_SMBUS */ > + > +static inline int psmouse_smbus_module_init(void) > +{ > + return 0; > +} > + > +static inline void psmouse_smbus_module_exit(void) > +{ > +} > + > +int psmouse_smbus_init(struct psmouse *psmouse, > + const struct i2c_board_info *board, > + const void *pdata, size_t pdata_size, > + bool leave_breadcrumbs) > +{ > + return -ENOSYS; > +} > + > +void psmouse_smbus_cleanup(struct psmouse *psmouse) > +{ > +} > + > +#endif /* CONFIG_MOUSE_PS2_SMBUS */ > > #endif /* _PSMOUSE_H */ > -- > 2.12.0.246.ga2ecc84866-goog > -- 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 Mar 10 2017 or thereabouts, Dmitry Torokhov wrote: > From: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > This provides glue between PS/2 devices that enumerate the RMI4 devices > and Elan touchpads to the RMI4 (or Elan) SMBus driver. > > The SMBus devices keep their PS/2 connection alive. If the initialization > process goes too far (psmouse_activate called), the device disconnects > from the I2C bus and stays on the PS/2 bus, that is why we explicitly > disable PS/2 device reporting (by calling psmouse_deactivate) before > trying to register SMBus companion device. > > The HID over I2C devices are enumerated through the ACPI DSDT, and > their PS/2 device also exports the InterTouch bit in the extended > capability 0x0C. However, the firmware keeps its I2C connection open > even after going further in the PS/2 initialization. We don't need > to take extra precautions with those device, especially because they > block their PS/2 communication when HID over I2C is used. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > --- Actually, one thing I noticed but forgot to say in my previous email: If you call rescan from the sysfs, there is a chance the i2c device is still there while we are trying to reconnect the device. Which means that we fail at creating the i2c device, and then fall back on PS/2. It's not a big issue, but still it will show some warning in the dmesg while attempting to create some sysfs files that already exist. S solution could be to unlock the mutexes and wait for the termination of i2c_unregister_device, but I would believe the best approach would be to remove the mutexes in serio and psmouse, and directly call i2c_unregister_device in .disconnect. Cheers, Benjamin -- 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 Mon, Mar 13, 2017 at 11:31:48AM +0100, Benjamin Tissoires wrote: > On Mar 10 2017 or thereabouts, Dmitry Torokhov wrote: > > From: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > > > This provides glue between PS/2 devices that enumerate the RMI4 devices > > and Elan touchpads to the RMI4 (or Elan) SMBus driver. > > > > The SMBus devices keep their PS/2 connection alive. If the initialization > > process goes too far (psmouse_activate called), the device disconnects > > from the I2C bus and stays on the PS/2 bus, that is why we explicitly > > disable PS/2 device reporting (by calling psmouse_deactivate) before > > trying to register SMBus companion device. > > > > The HID over I2C devices are enumerated through the ACPI DSDT, and > > their PS/2 device also exports the InterTouch bit in the extended > > capability 0x0C. However, the firmware keeps its I2C connection open > > even after going further in the PS/2 initialization. We don't need > > to take extra precautions with those device, especially because they > > block their PS/2 communication when HID over I2C is used. > > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > --- > > drivers/input/mouse/Kconfig | 4 + > > drivers/input/mouse/Makefile | 2 + > > drivers/input/mouse/psmouse-base.c | 16 +- > > drivers/input/mouse/psmouse-smbus.c | 291 ++++++++++++++++++++++++++++++++++++ > > drivers/input/mouse/psmouse.h | 37 +++++ > > 5 files changed, 348 insertions(+), 2 deletions(-) > > create mode 100644 drivers/input/mouse/psmouse-smbus.c > > > > diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig > > index 096abb4ad5cd..87bde8a210c7 100644 > > --- a/drivers/input/mouse/Kconfig > > +++ b/drivers/input/mouse/Kconfig > > @@ -171,6 +171,10 @@ config MOUSE_PS2_VMMOUSE > > > > If unsure, say N. > > > > +config MOUSE_PS2_SMBUS > > + bool > > + depends on MOUSE_PS2 > > + > > config MOUSE_SERIAL > > tristate "Serial mouse" > > select SERIO > > diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile > > index 6168b134937b..56bf0ad877c6 100644 > > --- a/drivers/input/mouse/Makefile > > +++ b/drivers/input/mouse/Makefile > > @@ -39,6 +39,8 @@ psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT) += touchkit_ps2.o > > psmouse-$(CONFIG_MOUSE_PS2_CYPRESS) += cypress_ps2.o > > psmouse-$(CONFIG_MOUSE_PS2_VMMOUSE) += vmmouse.o > > > > +psmouse-$(CONFIG_MOUSE_PS2_SMBUS) += psmouse-smbus.o > > + > > elan_i2c-objs := elan_i2c_core.o > > elan_i2c-$(CONFIG_MOUSE_ELAN_I2C_I2C) += elan_i2c_i2c.o > > elan_i2c-$(CONFIG_MOUSE_ELAN_I2C_SMBUS) += elan_i2c_smbus.o > > diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c > > index 40f09ce84f14..bdb48b2acc57 100644 > > --- a/drivers/input/mouse/psmouse-base.c > > +++ b/drivers/input/mouse/psmouse-base.c > > @@ -1996,16 +1996,27 @@ static int __init psmouse_init(void) > > synaptics_module_init(); > > hgpk_module_init(); > > > > + err = psmouse_smbus_module_init(); > > + if (err) > > + return err; > > + > > kpsmoused_wq = alloc_ordered_workqueue("kpsmoused", 0); > > if (!kpsmoused_wq) { > > pr_err("failed to create kpsmoused workqueue\n"); > > - return -ENOMEM; > > + err = -ENOMEM; > > + goto err_smbus_exit; > > } > > > > err = serio_register_driver(&psmouse_drv); > > if (err) > > - destroy_workqueue(kpsmoused_wq); > > + goto err_destroy_wq; > > > > + return 0; > > + > > +err_destroy_wq: > > + destroy_workqueue(kpsmoused_wq); > > +err_smbus_exit: > > + psmouse_smbus_module_exit(); > > return err; > > } > > > > @@ -2013,6 +2024,7 @@ static void __exit psmouse_exit(void) > > { > > serio_unregister_driver(&psmouse_drv); > > destroy_workqueue(kpsmoused_wq); > > + psmouse_smbus_module_exit(); > > } > > > > module_init(psmouse_init); > > diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c > > new file mode 100644 > > index 000000000000..cc716977fbf9 > > --- /dev/null > > +++ b/drivers/input/mouse/psmouse-smbus.c > > @@ -0,0 +1,291 @@ > > +/* > > + * Copyright (c) 2017 Red Hat, Inc > > + * > > + * 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. > > + */ > > + > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/libps2.h> > > +#include <linux/i2c.h> > > +#include <linux/serio.h> > > +#include <linux/slab.h> > > +#include <linux/workqueue.h> > > +#include "psmouse.h" > > + > > +struct psmouse_smbus_dev { > > + struct i2c_board_info board; > > + struct psmouse *psmouse; > > + struct i2c_client *client; > > + struct list_head node; > > + bool dead; > > +}; > > + > > +static LIST_HEAD(psmouse_smbus_list); > > +static DEFINE_MUTEX(psmouse_smbus_mutex); > > + > > +static void psmouse_smbus_check_adapter(struct i2c_adapter *adapter) > > +{ > > + struct psmouse_smbus_dev *smbdev; > > + > > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_HOST_NOTIFY)) > > + return; > > + > > + mutex_lock(&psmouse_smbus_mutex); > > + > > + list_for_each_entry(smbdev, &psmouse_smbus_list, node) { > > + if (smbdev->dead) > > + continue; > > + > > + if (smbdev->client) > > + continue; > > + > > + if (i2c_probe_func_quick_read(adapter, smbdev->board.addr) < 0) > > + continue; > > These 2 lines above seem to be the culprit of the > non-working-on-the-t450s-while-in-bzImage :) > > First, i2c_probe_func_quick_read() returns a boolean, so the value can > never be negative. And second, if you try to probe the device on the I2C > bus while psmouse_activate has been called, the device won't answer. So > we can simply ignore the call all together and let i2c_new_probed_device() > checks for the validity of the device. > > Bonus point, if I remove the call to i2c_probe_func_quick_read(), I seem > to have reliable detection of SMBus even when psmouse in compiled in the > bzImage. Yeah, this is a bit unfortunate, but I think we'll have to live with it. I tried adding calls to psmouse_disable and try different probe routines, but the device keeps hiding if we went past psmouse_activate, and haven't gone through entire reset/detect sequence again. So I guess if we see an adapter that is host notify capable and we have our "breadcrumb" we will be issuing rescan request and hope for the best. We do not really expect many such adapters and many companions in a single system. Thanks.
On Mon, Mar 13, 2017 at 12:01:59PM +0100, Benjamin Tissoires wrote: > On Mar 10 2017 or thereabouts, Dmitry Torokhov wrote: > > From: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > > > This provides glue between PS/2 devices that enumerate the RMI4 devices > > and Elan touchpads to the RMI4 (or Elan) SMBus driver. > > > > The SMBus devices keep their PS/2 connection alive. If the initialization > > process goes too far (psmouse_activate called), the device disconnects > > from the I2C bus and stays on the PS/2 bus, that is why we explicitly > > disable PS/2 device reporting (by calling psmouse_deactivate) before > > trying to register SMBus companion device. > > > > The HID over I2C devices are enumerated through the ACPI DSDT, and > > their PS/2 device also exports the InterTouch bit in the extended > > capability 0x0C. However, the firmware keeps its I2C connection open > > even after going further in the PS/2 initialization. We don't need > > to take extra precautions with those device, especially because they > > block their PS/2 communication when HID over I2C is used. > > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > --- > > Actually, one thing I noticed but forgot to say in my previous email: > > If you call rescan from the sysfs, there is a chance the i2c device is > still there while we are trying to reconnect the device. Which means > that we fail at creating the i2c device, and then fall back on PS/2. > > It's not a big issue, but still it will show some warning in the dmesg > while attempting to create some sysfs files that already exist. > > S solution could be to unlock the mutexes and wait for the termination > of i2c_unregister_device, but I would believe the best approach would be > to remove the mutexes in serio and psmouse, and directly call > i2c_unregister_device in .disconnect. Yeah, I think we'll have to live with rescan from sysfs hiccuping for now. I want to do a bit of face-lift for serio/psmouse, but it will take a bit of time. Thanks.
diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig index 096abb4ad5cd..87bde8a210c7 100644 --- a/drivers/input/mouse/Kconfig +++ b/drivers/input/mouse/Kconfig @@ -171,6 +171,10 @@ config MOUSE_PS2_VMMOUSE If unsure, say N. +config MOUSE_PS2_SMBUS + bool + depends on MOUSE_PS2 + config MOUSE_SERIAL tristate "Serial mouse" select SERIO diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile index 6168b134937b..56bf0ad877c6 100644 --- a/drivers/input/mouse/Makefile +++ b/drivers/input/mouse/Makefile @@ -39,6 +39,8 @@ psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT) += touchkit_ps2.o psmouse-$(CONFIG_MOUSE_PS2_CYPRESS) += cypress_ps2.o psmouse-$(CONFIG_MOUSE_PS2_VMMOUSE) += vmmouse.o +psmouse-$(CONFIG_MOUSE_PS2_SMBUS) += psmouse-smbus.o + elan_i2c-objs := elan_i2c_core.o elan_i2c-$(CONFIG_MOUSE_ELAN_I2C_I2C) += elan_i2c_i2c.o elan_i2c-$(CONFIG_MOUSE_ELAN_I2C_SMBUS) += elan_i2c_smbus.o diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c index 40f09ce84f14..bdb48b2acc57 100644 --- a/drivers/input/mouse/psmouse-base.c +++ b/drivers/input/mouse/psmouse-base.c @@ -1996,16 +1996,27 @@ static int __init psmouse_init(void) synaptics_module_init(); hgpk_module_init(); + err = psmouse_smbus_module_init(); + if (err) + return err; + kpsmoused_wq = alloc_ordered_workqueue("kpsmoused", 0); if (!kpsmoused_wq) { pr_err("failed to create kpsmoused workqueue\n"); - return -ENOMEM; + err = -ENOMEM; + goto err_smbus_exit; } err = serio_register_driver(&psmouse_drv); if (err) - destroy_workqueue(kpsmoused_wq); + goto err_destroy_wq; + return 0; + +err_destroy_wq: + destroy_workqueue(kpsmoused_wq); +err_smbus_exit: + psmouse_smbus_module_exit(); return err; } @@ -2013,6 +2024,7 @@ static void __exit psmouse_exit(void) { serio_unregister_driver(&psmouse_drv); destroy_workqueue(kpsmoused_wq); + psmouse_smbus_module_exit(); } module_init(psmouse_init); diff --git a/drivers/input/mouse/psmouse-smbus.c b/drivers/input/mouse/psmouse-smbus.c new file mode 100644 index 000000000000..cc716977fbf9 --- /dev/null +++ b/drivers/input/mouse/psmouse-smbus.c @@ -0,0 +1,291 @@ +/* + * Copyright (c) 2017 Red Hat, Inc + * + * 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. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/libps2.h> +#include <linux/i2c.h> +#include <linux/serio.h> +#include <linux/slab.h> +#include <linux/workqueue.h> +#include "psmouse.h" + +struct psmouse_smbus_dev { + struct i2c_board_info board; + struct psmouse *psmouse; + struct i2c_client *client; + struct list_head node; + bool dead; +}; + +static LIST_HEAD(psmouse_smbus_list); +static DEFINE_MUTEX(psmouse_smbus_mutex); + +static void psmouse_smbus_check_adapter(struct i2c_adapter *adapter) +{ + struct psmouse_smbus_dev *smbdev; + + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_HOST_NOTIFY)) + return; + + mutex_lock(&psmouse_smbus_mutex); + + list_for_each_entry(smbdev, &psmouse_smbus_list, node) { + if (smbdev->dead) + continue; + + if (smbdev->client) + continue; + + if (i2c_probe_func_quick_read(adapter, smbdev->board.addr) < 0) + continue; + + /* Device seems to be there, let's try switching over */ + psmouse_dbg(smbdev->psmouse, + "SMBus companion appeared, triggering rescan\n"); + serio_rescan(smbdev->psmouse->ps2dev.serio); + } + + mutex_unlock(&psmouse_smbus_mutex); +} + +static void psmouse_smbus_detach_i2c_client(struct i2c_client *client) +{ + struct psmouse_smbus_dev *smbdev; + + mutex_lock(&psmouse_smbus_mutex); + + list_for_each_entry(smbdev, &psmouse_smbus_list, node) { + if (smbdev->client == client) { + psmouse_dbg(smbdev->psmouse, + "Marking SMBus companion %s as gone\n", + dev_name(&smbdev->client->dev)); + smbdev->client = NULL; + smbdev->dead = true; + serio_rescan(smbdev->psmouse->ps2dev.serio); + } + } + + kfree(client->dev.platform_data); + client->dev.platform_data = NULL; + + mutex_unlock(&psmouse_smbus_mutex); +} + +static int psmouse_smbus_notifier_call(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct device *dev = data; + + switch (action) { + case BUS_NOTIFY_ADD_DEVICE: + if (dev->type == &i2c_adapter_type) + psmouse_smbus_check_adapter(to_i2c_adapter(dev)); + break; + + case BUS_NOTIFY_REMOVED_DEVICE: + if (dev->type == &i2c_client_type) + psmouse_smbus_detach_i2c_client(to_i2c_client(dev)); + break; + } + + return 0; +} + +static struct notifier_block psmouse_smbus_notifier = { + .notifier_call = psmouse_smbus_notifier_call, +}; + +static psmouse_ret_t psmouse_smbus_process_byte(struct psmouse *psmouse) +{ + return PSMOUSE_FULL_PACKET; +} + +static int psmouse_smbus_reconnect(struct psmouse *psmouse) +{ + psmouse_deactivate(psmouse); + + return 0; +} + +struct psmouse_smbus_removal_work { + struct work_struct work; + struct i2c_client *client; +}; + +static void psmouse_smbus_remove_i2c_device(struct work_struct *work) +{ + struct psmouse_smbus_removal_work *rwork = + container_of(work, struct psmouse_smbus_removal_work, work); + + dev_dbg(&rwork->client->dev, "destroying SMBus companion device\n"); + i2c_unregister_device(rwork->client); + + kfree(rwork); +} + +/* + * This schedules removal of SMBus companion device. We have to do + * it in a separate tread to avoid deadlocking on psmouse_mutex in + * case the device has a trackstick (which is also driven by psmouse). + * + * Note that this may be racing with i2c adapter removal, but we + * can't do anything about that: i2c automatically destroys clients + * attached to an adapter that is being removed. This has to be + * fixed in i2c core. + */ +static void psmouse_smbus_schedule_remove(struct i2c_client *client) +{ + struct psmouse_smbus_removal_work *rwork; + + rwork = kzalloc(sizeof(*rwork), GFP_KERNEL); + if (rwork) { + INIT_WORK(&rwork->work, psmouse_smbus_remove_i2c_device); + rwork->client = client; + + schedule_work(&rwork->work); + } +} + +static void psmouse_smbus_disconnect(struct psmouse *psmouse) +{ + struct psmouse_smbus_dev *smbdev = psmouse->private; + + mutex_lock(&psmouse_smbus_mutex); + list_del(&smbdev->node); + mutex_unlock(&psmouse_smbus_mutex); + + if (smbdev->client) { + psmouse_dbg(smbdev->psmouse, + "posting removal request for SMBus companion %s\n", + dev_name(&smbdev->client->dev)); + psmouse_smbus_schedule_remove(smbdev->client); + } + + kfree(smbdev); + psmouse->private = NULL; +} + +static int psmouse_smbus_create_companion(struct device *dev, void *data) +{ + struct psmouse_smbus_dev *smbdev = data; + unsigned short addr_list[] = { smbdev->board.addr, I2C_CLIENT_END }; + struct i2c_adapter *adapter; + + adapter = i2c_verify_adapter(dev); + if (!adapter) + return 0; + + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_HOST_NOTIFY)) + return 0; + + smbdev->client = i2c_new_probed_device(adapter, &smbdev->board, + addr_list, NULL); + if (!smbdev->client) + return 0; + + /* We have our(?) device, stop iterating i2c bus. */ + return 1; +} + +void psmouse_smbus_cleanup(struct psmouse *psmouse) +{ + struct psmouse_smbus_dev *smbdev, *tmp; + + mutex_lock(&psmouse_smbus_mutex); + + list_for_each_entry_safe(smbdev, tmp, &psmouse_smbus_list, node) { + if (psmouse == smbdev->psmouse) { + list_del(&smbdev->node); + kfree(smbdev); + } + } + + mutex_unlock(&psmouse_smbus_mutex); +} + +int psmouse_smbus_init(struct psmouse *psmouse, + const struct i2c_board_info *board, + const void *pdata, size_t pdata_size, + bool leave_breadcrumbs) +{ + struct psmouse_smbus_dev *smbdev; + int error; + + smbdev = kzalloc(sizeof(*smbdev), GFP_KERNEL); + if (!smbdev) + return -ENOMEM; + + smbdev->psmouse = psmouse; + smbdev->board = *board; + + smbdev->board.platform_data = kmemdup(pdata, pdata_size, GFP_KERNEL); + if (!smbdev->board.platform_data) { + kfree(smbdev); + return -ENOMEM; + } + + psmouse->private = smbdev; + psmouse->protocol_handler = psmouse_smbus_process_byte; + psmouse->reconnect = psmouse_smbus_reconnect; + psmouse->fast_reconnect = psmouse_smbus_reconnect; + psmouse->disconnect = psmouse_smbus_disconnect; + psmouse->resync_time = 0; + + psmouse_deactivate(psmouse); + + mutex_lock(&psmouse_smbus_mutex); + list_add_tail(&smbdev->node, &psmouse_smbus_list); + mutex_unlock(&psmouse_smbus_mutex); + + /* Bind to already existing adapters right away */ + error = i2c_for_each_dev(smbdev, psmouse_smbus_create_companion); + + if (smbdev->client) { + /* We have our companion device */ + return 0; + } + + /* + * If we did not create i2c device we will not need platform + * data even if we are leaving breadcrumbs. + */ + kfree(smbdev->board.platform_data); + smbdev->board.platform_data = NULL; + + if (error < 0 || !leave_breadcrumbs) { + mutex_lock(&psmouse_smbus_mutex); + list_del(&smbdev->node); + mutex_unlock(&psmouse_smbus_mutex); + + kfree(smbdev); + } + + return error < 0 ? error : -EAGAIN; +} + +int __init psmouse_smbus_module_init(void) +{ + int error; + + error = bus_register_notifier(&i2c_bus_type, &psmouse_smbus_notifier); + if (error) { + pr_err("failed to register i2c bus notifier: %d\n", error); + return error; + } + + return 0; +} + +void __exit psmouse_smbus_module_exit(void) +{ + bus_unregister_notifier(&i2c_bus_type, &psmouse_smbus_notifier); + flush_scheduled_work(); +} diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h index e853dee05e79..77fcd361c8d6 100644 --- a/drivers/input/mouse/psmouse.h +++ b/drivers/input/mouse/psmouse.h @@ -209,5 +209,42 @@ static struct psmouse_attribute psmouse_attr_##_name = { \ &(psmouse)->ps2dev.serio->dev, \ psmouse_fmt(format), ##__VA_ARGS__) +#ifdef CONFIG_MOUSE_PS2_SMBUS + +int psmouse_smbus_module_init(void); +void psmouse_smbus_module_exit(void); + +struct i2c_board_info; + +int psmouse_smbus_init(struct psmouse *psmouse, + const struct i2c_board_info *board, + const void *pdata, size_t pdata_size, + bool leave_breadcrumbs); +void psmouse_smbus_cleanup(struct psmouse *psmouse); + +#else /* !CONFIG_MOUSE_PS2_SMBUS */ + +static inline int psmouse_smbus_module_init(void) +{ + return 0; +} + +static inline void psmouse_smbus_module_exit(void) +{ +} + +int psmouse_smbus_init(struct psmouse *psmouse, + const struct i2c_board_info *board, + const void *pdata, size_t pdata_size, + bool leave_breadcrumbs) +{ + return -ENOSYS; +} + +void psmouse_smbus_cleanup(struct psmouse *psmouse) +{ +} + +#endif /* CONFIG_MOUSE_PS2_SMBUS */ #endif /* _PSMOUSE_H */