Fix for Dell P57G touchpad found, need advice on patch

Fix for Dell P57G touchpad found, need advice on patch

[Pavel Roskin]

Hi!

I'm using Dell Inspiron 13, model P57G. It has a problem with the
touchpad under Linux. Approximately half of the time, the touchpad is
not working at all - the mouse cursor is not moving. In this case,
there are error messages written to the kernel log every second:

[   53.127339] i2c_designware INT3433:00: controller timed out
[   54.219336] i2c_designware INT3433:00: controller timed out
[   55.311346] i2c_designware INT3433:00: controller timed out
[   56.403326] i2c_designware INT3433:00: controller timed out

There is a simple fix - blacklist the driver
(i2c_designware_platform). In this case, the touchpad words as a mouse
and used IRQ 12. Googling for "blackist i2c_designware_platform" shows
that I'm not the one using that approach.

I started looking at the driver in git. A patch by Romain Baeriswyl
applied on 2014-08-20 adds support for the "standard" mode with the
100kHz clock, as opposed to the 400kHz "fast" mode. Unfortunately,
that patch only affects OpenFirmware systems, and I have ACPI. As soon
as I set the clock to 100kHz, the touchpad started working every time.

One fix would be to have a module parameter to force slower clock. It
would still require users to deal with modprobe, so it's not optimal.

I noticed that dev->get_clk_rate_khz(dev) returns 100000, but that
would be 100000kHz = 100MHz. Not sure is somebody confused the units
or it's the correct clock.

Considering that I have no other systems to test, what would be the
best approach? Set the clock to 100kHz for INT3433? Recognize Dell
P57G system specifically? How? Preserve the original clock by reading
the DW_IC_CON register and keeping some bits?

Pavel

Re: Fix for Dell P57G touchpad found, need advice on patch

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2230 bytes --]

Hi Pavel,

On Thu, Sep 10, 2015 at 11:11:41PM -0700, Pavel Roskin wrote:
> Hi!

Thanks for this detailed info. Adding Mika to CC who is an expert of
this driver. Mika, maybe we should add you to MAINTAINERS, too?

> I'm using Dell Inspiron 13, model P57G. It has a problem with the
> touchpad under Linux. Approximately half of the time, the touchpad is
> not working at all - the mouse cursor is not moving. In this case,
> there are error messages written to the kernel log every second:
> 
> [   53.127339] i2c_designware INT3433:00: controller timed out
> [   54.219336] i2c_designware INT3433:00: controller timed out
> [   55.311346] i2c_designware INT3433:00: controller timed out
> [   56.403326] i2c_designware INT3433:00: controller timed out
> 
> There is a simple fix - blacklist the driver
> (i2c_designware_platform). In this case, the touchpad words as a mouse
> and used IRQ 12. Googling for "blackist i2c_designware_platform" shows
> that I'm not the one using that approach.
> 
> I started looking at the driver in git. A patch by Romain Baeriswyl
> applied on 2014-08-20 adds support for the "standard" mode with the
> 100kHz clock, as opposed to the 400kHz "fast" mode. Unfortunately,
> that patch only affects OpenFirmware systems, and I have ACPI. As soon
> as I set the clock to 100kHz, the touchpad started working every time.
> 
> One fix would be to have a module parameter to force slower clock. It
> would still require users to deal with modprobe, so it's not optimal.
> 
> I noticed that dev->get_clk_rate_khz(dev) returns 100000, but that
> would be 100000kHz = 100MHz. Not sure is somebody confused the units
> or it's the correct clock.
> 
> Considering that I have no other systems to test, what would be the
> best approach? Set the clock to 100kHz for INT3433? Recognize Dell
> P57G system specifically? How? Preserve the original clock by reading
> the DW_IC_CON register and keeping some bits?

Note: The driver has been using 400kHz from its beginning. However,
100kHz is usually a better default for I2C busses. I wonder if we should
take the risk of a performance regression in favour of a sane and
working default?

Regards,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Fix for Dell P57G touchpad found, need advice on patch

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2428 bytes --]

On Tue, Sep 15, 2015 at 06:42:14PM +0200, Wolfram Sang wrote:
> Hi Pavel,
> 
> On Thu, Sep 10, 2015 at 11:11:41PM -0700, Pavel Roskin wrote:
> > Hi!
> 
> Thanks for this detailed info. Adding Mika to CC who is an expert of
> this driver. Mika, maybe we should add you to MAINTAINERS, too?
> 

And forgot to add Mika :)

> > I'm using Dell Inspiron 13, model P57G. It has a problem with the
> > touchpad under Linux. Approximately half of the time, the touchpad is
> > not working at all - the mouse cursor is not moving. In this case,
> > there are error messages written to the kernel log every second:
> > 
> > [   53.127339] i2c_designware INT3433:00: controller timed out
> > [   54.219336] i2c_designware INT3433:00: controller timed out
> > [   55.311346] i2c_designware INT3433:00: controller timed out
> > [   56.403326] i2c_designware INT3433:00: controller timed out
> > 
> > There is a simple fix - blacklist the driver
> > (i2c_designware_platform). In this case, the touchpad words as a mouse
> > and used IRQ 12. Googling for "blackist i2c_designware_platform" shows
> > that I'm not the one using that approach.
> > 
> > I started looking at the driver in git. A patch by Romain Baeriswyl
> > applied on 2014-08-20 adds support for the "standard" mode with the
> > 100kHz clock, as opposed to the 400kHz "fast" mode. Unfortunately,
> > that patch only affects OpenFirmware systems, and I have ACPI. As soon
> > as I set the clock to 100kHz, the touchpad started working every time.
> > 
> > One fix would be to have a module parameter to force slower clock. It
> > would still require users to deal with modprobe, so it's not optimal.
> > 
> > I noticed that dev->get_clk_rate_khz(dev) returns 100000, but that
> > would be 100000kHz = 100MHz. Not sure is somebody confused the units
> > or it's the correct clock.
> > 
> > Considering that I have no other systems to test, what would be the
> > best approach? Set the clock to 100kHz for INT3433? Recognize Dell
> > P57G system specifically? How? Preserve the original clock by reading
> > the DW_IC_CON register and keeping some bits?
> 
> Note: The driver has been using 400kHz from its beginning. However,
> 100kHz is usually a better default for I2C busses. I wonder if we should
> take the risk of a performance regression in favour of a sane and
> working default?
> 
> Regards,
> 
>    Wolfram
> 



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Fix for Dell P57G touchpad found, need advice on patch

[Mika Westerberg]

Adding Jarkko as well.

On Tue, Sep 15, 2015 at 07:11:51PM +0200, Wolfram Sang wrote:
> On Tue, Sep 15, 2015 at 06:42:14PM +0200, Wolfram Sang wrote:
> > Hi Pavel,
> > 
> > On Thu, Sep 10, 2015 at 11:11:41PM -0700, Pavel Roskin wrote:
> > > Hi!
> > 
> > Thanks for this detailed info. Adding Mika to CC who is an expert of
> > this driver. Mika, maybe we should add you to MAINTAINERS, too?
> > 
> 
> And forgot to add Mika :)
> 
> > > I'm using Dell Inspiron 13, model P57G. It has a problem with the
> > > touchpad under Linux. Approximately half of the time, the touchpad is
> > > not working at all - the mouse cursor is not moving. In this case,
> > > there are error messages written to the kernel log every second:
> > > 
> > > [   53.127339] i2c_designware INT3433:00: controller timed out
> > > [   54.219336] i2c_designware INT3433:00: controller timed out
> > > [   55.311346] i2c_designware INT3433:00: controller timed out
> > > [   56.403326] i2c_designware INT3433:00: controller timed out
> > > 
> > > There is a simple fix - blacklist the driver
> > > (i2c_designware_platform). In this case, the touchpad words as a mouse
> > > and used IRQ 12. Googling for "blackist i2c_designware_platform" shows
> > > that I'm not the one using that approach.
> > > 
> > > I started looking at the driver in git. A patch by Romain Baeriswyl
> > > applied on 2014-08-20 adds support for the "standard" mode with the
> > > 100kHz clock, as opposed to the 400kHz "fast" mode. Unfortunately,
> > > that patch only affects OpenFirmware systems, and I have ACPI. As soon
> > > as I set the clock to 100kHz, the touchpad started working every time.
> > > 
> > > One fix would be to have a module parameter to force slower clock. It
> > > would still require users to deal with modprobe, so it's not optimal.
> > > 
> > > I noticed that dev->get_clk_rate_khz(dev) returns 100000, but that
> > > would be 100000kHz = 100MHz. Not sure is somebody confused the units
> > > or it's the correct clock.
> > > 
> > > Considering that I have no other systems to test, what would be the
> > > best approach? Set the clock to 100kHz for INT3433? Recognize Dell
> > > P57G system specifically? How? Preserve the original clock by reading
> > > the DW_IC_CON register and keeping some bits?
> > 
> > Note: The driver has been using 400kHz from its beginning. However,
> > 100kHz is usually a better default for I2C busses. I wonder if we should
> > take the risk of a performance regression in favour of a sane and
> > working default?

Pavel,

Can you send me acpidump of that machine (or contents of
/sys/firmware/acpi/tables/DSDT)? It may be that the ACPI I2cSerialBus
connector actually has 100kHz there - we just don't use it currently.

Re: Fix for Dell P57G touchpad found, need advice on patch

[Mika Westerberg]

On Wed, Sep 16, 2015 at 11:40:39AM +0300, Mika Westerberg wrote:
> Can you send me acpidump of that machine (or contents of
> /sys/firmware/acpi/tables/DSDT)? It may be that the ACPI I2cSerialBus
> connector actually has 100kHz there - we just don't use it currently.

Thanks for the DSDT.

All touchpads seem to have 400kHz set in their I2cSerialBus connectors :-(

Can you try the below patch and see if it changes anything? One suspect
is that instead of running wrong frequency, timings for fast mode are
not correct. The below patch will force the driver to use calculated
ones (based on the DW I2C datasheet).

Please also attach dmesg from the test run.

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 3dd2de31a2f8..3ef19477e37e 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -91,6 +91,15 @@ static int dw_i2c_acpi_configure(struct platform_device *pdev)
 	dw_i2c_acpi_params(pdev, "FMCN", &dev->fs_hcnt, &dev->fs_lcnt,
 			   &dev->sda_hold_time);
 
+	dev_info(&pdev->dev, "ACPI SSCN %u:%u\n", dev->ss_hcnt, dev->ss_lcnt);
+	dev_info(&pdev->dev, "ACPI FMCN %u:%u\n", dev->fs_hcnt, dev->fs_lcnt);
+	dev_info(&pdev->dev, "ACPI SDA hold %u\n", dev->sda_hold_time);
+
+	/* Use defaults */
+	dev->ss_hcnt = dev->ss_lcnt = 0;
+	dev->fs_hcnt = dev->fs_lcnt = 0;
+	dev->sda_hold_time = 0;
+
 	/*
 	 * Provide a way for Designware I2C host controllers that are not
 	 * based on Intel LPSS to specify their input clock frequency via