linux-kernel - Re: [Bug 203297] Synaptics touchpad TM-3127 functionality broken by PCI runtime power management patch on 4.20.2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3a1139ef-10ed-6923-73c5-30fbf0c065c3@linux.intel.com>
Date:   Fri, 26 Apr 2019 15:12:42 +0300
From:   Jarkko Nikula <jarkko.nikula@...ux.intel.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Keijo Vaara <ferdasyn@...ketmail.com>,
        Jean Delvare <jdelvare@...e.de>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        linux-pm@...r.kernel.org, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-input@...r.kernel.org
Subject: Re: [Bug 203297] Synaptics touchpad TM-3127 functionality broken by
 PCI runtime power management patch on 4.20.2

On 4/22/19 4:08 PM, Bjorn Helgaas wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=203297
> 
> Regression, suspected but as yet unconfirmed cause:
> 
>    c5eb1190074c ("PCI / PM: Allow runtime PM without callback functions")
> 
> backported to 4.20 stable as 39e1be324c2f.
> 
With help of Keijo it was confirmed above patch broke the Synaptics 
touchpad. Not bisected but touchpad works again by forcing the i2c-i801 
SMBus controller always on:
"echo on >/sys/bus/pci/devices/0000\:00\:1f.3/power/control"

Above patch is a generalized fix that fixed the runtime PM regression on 
i2c-i801 and re-allow the controller go to runtime suspend when idle. So 
most probably Synaptics touchpad was broken by i2c-i801 runtime PM also 
before but got unnoticed. Which is easy since on many platforms SMBus 
controller doesn't necessarily have the PCI PM capabilities.

I would like to ask help from input subsystem experts what kind of SMBus 
power state dependency Synaptics RMI4 SMBus devices have since it cease 
to work if SMBus controllers idles between transfers and how this is 
best to fix?

Instead of revert I think we'd need to have some method to force SMBus 
controller on whenever the touchpad is active, like when there is a 
userspace listening.

I'm not expert in this area so as quick proof of concept I had a 
following hack which forces the I2C/SMBus adapter, and eventually the 
parent PCI device of it on when the RMI4 SMBus device is probed and let 
the SMBus controller to idle when removed.

According to Keijo it fixes the issue but I like to hear input experts 
for better place to put these.

diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index b6ccf39c6a7b..2b11d69be313 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -16,6 +16,7 @@
  #include <linux/lockdep.h>
  #include <linux/module.h>
  #include <linux/pm.h>
+#include <linux/pm_runtime.h>
  #include <linux/rmi.h>
  #include <linux/slab.h>
  #include "rmi_driver.h"
@@ -332,6 +333,9 @@ static int rmi_smb_probe(struct i2c_client *client,

  	dev_info(&client->dev, "registering SMbus-connected sensor\n");

+	/* Force SMBus adapter on while RMI4 device is connected */
+	pm_runtime_get(&client->adapter->dev);
+
  	error = rmi_register_transport_device(&rmi_smb->xport);
  	if (error) {
  		dev_err(&client->dev, "failed to register sensor: %d\n", error);
@@ -346,6 +350,7 @@ static int rmi_smb_remove(struct i2c_client *client)
  	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);

  	rmi_unregister_transport_device(&rmi_smb->xport);
+	pm_runtime_put(&client->adapter->dev);

  	return 0;
  }

-- 
Jarkko