https://bugs.launchpad.net/bugs/1513297

2015-11-18 Thread Rich K 
XE500C12-K01US Samsung Chromebook 2 Ubuntu 14.04 touchpad not responsive
The chromebook is new and the touchpad worked great with chromiumos
but not at all with Ubuntu.
I used John Lewis's script to flash seabios to the chromebook and then
installed Ubuntu like normal from a USB drive.
I filed a bug report for it on launchpad and Christopher M. Penalver
responded asking me to try Ubuntu 16.04 with the latest version of the
kernel. I used the kernel from
http://kernel.ubuntu.com/~kernel-ppa/mainline/v4.3-wily/ and the
problem persisted.
Before filing this report, I noticed a new folder for
http://kernel.ubuntu.com/~kernel-ppa/mainline/v4.4-rc1-wily/ with only
linux-headers-4.4.0-040400rc1_4.4.0-040400rc1.201511152030_all.deb so
I ran
$ sudo dpkg -i 
linux-headers-4.4.0-040400rc1_4.4.0-040400rc1.201511152030_all.deb
and rebooted before getting the information for this report. I still
only saw the 4.3 entry in grub. I'm guessing that's because this was
just the headers file and not the image file.

Link to bug report on launchpad.
https://bugs.launchpad.net/bugs/1513297

I'm following the format found on.
https://wiki.ubuntu.com/Bugs/Upstream/kernel

$ cat /proc/version
Linux version 4.3.0-040300-generic (kernel@gomeisa) (gcc version 5.2.1
20151010 (Ubuntu 5.2.1-22ubuntu2) ) #201511020949 SMP Mon Nov 2
14:50:44 UTC 2015

$ lsb_release -rd
Description:Ubuntu Xenial Xerus (development branch)
Release:16.04

$ /usr/src/linux-headers-4.4.0-040400rc1/scripts/ver_linux
If some fields are empty or look unusual you may have an old version.
Compare to the current minimal requirements in Documentation/Changes.

Linux winky 4.3.0-040300-generic #201511020949 SMP Mon Nov 2 14:50:44
UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

GNU C5.2.1
GNU Make4.0
Binutils2.25.51.20151106
Util-linux2.27.1
Mount2.27.1
Module-init-tools21
E2fsprogs1.42.13
Pcmciautils018
PPP2.4.6
Linux C Library2.21
Dynamic linker (ldd)2.21
Linux C++ Library6.0.21
Procps3.3.9
Net-tools1.60
Kbd1.15.5
Console-tools1.15.5
Sh-utils8.23
Udev227
Wireless-tools30
Modules Loaded8250_dw ac97_bus acpi_thermal_rel ansi_cprng
arc4 atmel_mxt_ts autofs4 binfmt_misc bluetooth bnep btbcm btintel
btrtl btusb ccm cfg80211 coretemp crc32_pclmul crct10dif_pclmul cryptd
ctr drbg drm drm_kms_helper dw_dmac dw_dmac_core fb_sys_fops fjes hid
hid_logitech_dj hid_logitech_hidpp i2c_algo_bit i2c_designware_core
i2c_designware_platform i915 input_leds int3400_thermal
int3403_thermal int340x_thermal_zone intel_powerclamp intel_rapl
intel_soc_dts_iosf iosf_mbi iwlmvm iwlwifi joydev kvm kvm_intel lp
lpc_ich mac80211 mac_hid media mei mei_txe mmc_block parport
parport_pc ppdev processor_thermal_device punit_atom_debug pwm_lpss
pwm_lpss_platform rfcomm sdhci sdhci_acpi serio_raw shpchp snd
snd_compress snd_hda_codec snd_hda_codec_hdmi snd_hda_core
snd_hda_intel snd_hwdep snd_intel_sst_acpi snd_intel_sst_core snd_pcm
snd_pcm_dmaengine snd_rawmidi snd_seq snd_seq_device snd_seq_midi
snd_seq_midi_event snd_soc_core snd_soc_max98090 snd_soc_sst_acpi
snd_soc_sst_baytrail_pcm snd_soc_sst_byt_max98090_mach snd_soc_sst_dsp
snd_soc_sst_ipc snd_soc_sst_mfld_platform snd_timer soundcore
spi_pxa2xx_platform syscopyarea sysfillrect sysimgblt tpm_infineon
usbhid uvcvideo v4l2_common video videobuf2_core videobuf2_memops
videobuf2_vmalloc videodev

$ /proc/cpuinfo
processor: 0
vendor_id: GenuineIntel
cpu family: 6
model: 55
model name: Intel(R) Celeron(R) CPU  N2840  @ 2.16GHz
stepping: 8
microcode: 0x829
cpu MHz: 1482.642
cache size: 1024 KB
physical id: 0
siblings: 2
core id: 0
cpu cores: 2
apicid: 0
initial apicid: 0
fpu: yes
fpu_exception: yes
cpuid level: 11
wp: yes
flags: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge
mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe
syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl
xtopology nonstop_tsc aperfmperf pni pclmulqdq dtes64 monitor ds_cpl
vmx est tm2 ssse3 cx16 xtpr pdcm sse4_1 sse4_2 movbe popcnt
tsc_deadline_timer rdrand lahf_lm 3dnowprefetch ida arat epb dtherm
tpr_shadow vnmi flexpriority ept vpid tsc_adjust smep erms
bugs:
bogomips: 4326.40
clflush size: 64
cache_alignment: 64
address sizes: 36 bits physical, 48 bits virtual
power management:

processor: 1
vendor_id: GenuineIntel
cpu family: 6
model: 55
model name: Intel(R) Celeron(R) CPU  N2840  @ 2.16GHz
stepping: 8
microcode: 0x829
cpu MHz: 1462.422
cache size: 1024 KB
physical id: 0
siblings: 2
core id: 1
cpu cores: 2
apicid: 2
initial apicid: 2
fpu: yes
fpu_exception: yes
cpuid level: 11
wp: yes
flags: fpu vme de pse tsc msr pae mce cx8 apic

[PATCH v10 1/8] Input: goodix - use actual config length for each device type

2015-11-18 Thread Irina Tirdea 
Each of the Goodix devices supported by this driver has a fixed size for
the configuration information registers. The size varies depending on the
device and is specified in the datasheet.

Use the proper configuration length as specified in the datasheet for
each device model, so we do not read more than the actual size of the
configuration registers.

Signed-off-by: Irina Tirdea 
Acked-by: Bastien Nocera 
Tested-by: Bastien Nocera 
Tested-by: Aleksei Mamlin 
---
 drivers/input/touchscreen/goodix.c | 25 +++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c 
b/drivers/input/touchscreen/goodix.c
index 4d113c9..56d0330 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -36,6 +36,7 @@ struct goodix_ts_data {
unsigned int max_touch_num;
unsigned int int_trigger_type;
bool rotated_screen;
+   int cfg_len;
 };
 
 #define GOODIX_MAX_HEIGHT  4096
@@ -45,6 +46,8 @@ struct goodix_ts_data {
 #define GOODIX_MAX_CONTACTS10
 
 #define GOODIX_CONFIG_MAX_LENGTH   240
+#define GOODIX_CONFIG_911_LENGTH   186
+#define GOODIX_CONFIG_967_LENGTH   228
 
 /* Register defines */
 #define GOODIX_READ_COOR_ADDR  0x814E
@@ -115,6 +118,23 @@ static int goodix_i2c_read(struct i2c_client *client,
return ret < 0 ? ret : (ret != ARRAY_SIZE(msgs) ? -EIO : 0);
 }
 
+static int goodix_get_cfg_len(u16 id)
+{
+   switch (id) {
+   case 911:
+   case 9271:
+   case 9110:
+   case 927:
+   case 928:
+   return GOODIX_CONFIG_911_LENGTH;
+   case 912:
+   case 967:
+   return GOODIX_CONFIG_967_LENGTH;
+   default:
+   return GOODIX_CONFIG_MAX_LENGTH;
+   }
+}
+
 static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
 {
int touch_num;
@@ -230,8 +250,7 @@ static void goodix_read_config(struct goodix_ts_data *ts)
int error;
 
error = goodix_i2c_read(ts->client, GOODIX_REG_CONFIG_DATA,
-   config,
-   GOODIX_CONFIG_MAX_LENGTH);
+   config, ts->cfg_len);
if (error) {
dev_warn(>client->dev,
 "Error reading config (%d), using defaults\n",
@@ -398,6 +417,8 @@ static int goodix_ts_probe(struct i2c_client *client,
return error;
}
 
+   ts->cfg_len = goodix_get_cfg_len(id_info);
+
goodix_read_config(ts);
 
error = goodix_request_input_dev(ts, version_info, id_info);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 0/8] Goodix touchscreen enhancements

2015-11-18 Thread Irina Tirdea 
This patchset depends on Dmitry's patch that fixes the
GPIO ACPI API[1], so devm_gpiod_get_optional can be properly
used in the code. Thanks for taking care of this, Dmitry!

Bastien and Aleksei, I have removed your Acked-by and Tested-by
tags from the patches I have changed.

Thanks,
Irina

[1] https://lkml.org/lkml/2015/11/11/465

Changes in v10:
 - use devm_gpiod_get_optional and no longer ignore errors returned
by it
 - use completion to signal other code paths in the driver that
the config firmware is loaded on the device
 - dropped the patch that ordered the includes

Changes in v9:
 - add GPIOLIB to driver dependencies
 - add Tested-by tag from Bastien and Aleksei

Changes in v8:
 - only allow new functionality for devices that declare named
gpios (using _DSD properties in ACPI or named DT properties)

Changes in v7:
 - add dmi quirk to skip gpio pins setup and functionality that
depends on them for Onda v975w, WinBook TW100 and WinBook TW700.
 - add support for named gpio pins
 - rework the runtime pm patch to fix a couple of issues
 - sort includes using inverse Xmas tree ordering

Changes in v6:
 - skip runtime power manangent calls in open/close if the device
ACPI/DT configuration does not declare interrupt and reset gpio pins.
 - reset the device before starting i2c communication
 - add Bastien's ack to the first 2 patches

Changes in v5:
 - add some more style cleanup (reorder includes, use error instead
of ret for return values)
 - add runtime power management patch

Changes in v4:
 - use dmi quirk to determine the order of irq and reset pins
 - use actual config length depending on device
 - add sysfs interface to dump config
 - initialize esd timeout from ACPI/DT properly

Changes in v3:
 - dropped the first 3 patches that got merged
 - handle -EPROBE_DEFER and -ENOENT for gpio pins
 - skip functionality depending on the gpio pins if the pins are not
properly initialized from ACPI/DT (reset, write config, power management,
ESD)
 - dropped #ifdef CONFIG_PM_SLEEP and annotated with __maybe_unused instead
 - use sysfs property to set ESD timeout instead of ACPI/DT property
 - use request_firmware_nowait to read configuration firmware and use
defaults if firmware is not found
 - use ACPI IDs to determine the order of the GPIO pins in the ACPI tables
(interrupt pin first or reset pin first)

Changes in v2:
 - use request_firmware instead of ACPI/DT property for config
 - dropped "input: goodix: add ACPI IDs for GT911 and GT9271" patch
 - add ACPI DSDT excerpt in commit message where necessary
 - add comments for suspend/resume sleep values
 - dropped the checkpatch fixes that did not make sense
 - added Bastien's ack to the first patch

Irina Tirdea (8):
  Input: goodix - use actual config length for each device type
  Input: goodix - reset device at init
  Input: goodix - write configuration data to device
  Input: goodix - add power management support
  Input: goodix - use goodix_i2c_write_u8 instead of i2c_master_send
  Input: goodix - add support for ESD
  Input: goodix - add sysfs interface to dump config
  Input: goodix - add runtime power management support

 .../bindings/input/touchscreen/goodix.txt  |  11 +
 drivers/input/touchscreen/Kconfig  |   1 +
 drivers/input/touchscreen/goodix.c | 766 -
 3 files changed, 740 insertions(+), 38 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 4/8] Input: goodix - add power management support

2015-11-18 Thread Irina Tirdea 
Implement suspend/resume for goodix driver.

The suspend and resume process uses the gpio pins.
If the device ACPI/DT information does not declare gpio pins,
suspend/resume will not be available for these devices.

This is based on Goodix datasheets for GT911 and GT9271
and on Goodix driver gt9xx.c for Android (publicly available
in Android kernel trees for various devices).

Signed-off-by: Octavian Purdila 
Signed-off-by: Irina Tirdea 
---
 drivers/input/touchscreen/goodix.c | 96 --
 1 file changed, 91 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c 
b/drivers/input/touchscreen/goodix.c
index 0911b0c9..0fd472d 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -45,6 +45,7 @@ struct goodix_ts_data {
u16 version;
char *cfg_name;
struct completion firmware_loading_complete;
+   unsigned long irq_flags;
 };
 
 #define GOODIX_GPIO_INT_NAME   "irq"
@@ -61,6 +62,9 @@ struct goodix_ts_data {
 #define GOODIX_CONFIG_967_LENGTH   228
 
 /* Register defines */
+#define GOODIX_REG_COMMAND 0x8040
+#define GOODIX_CMD_SCREEN_OFF  0x05
+
 #define GOODIX_READ_COOR_ADDR  0x814E
 #define GOODIX_REG_CONFIG_DATA 0x8047
 #define GOODIX_REG_ID  0x8140
@@ -162,6 +166,11 @@ static int goodix_i2c_write(struct i2c_client *client, u16 
reg, const u8 *buf,
return ret < 0 ? ret : (ret != 1 ? -EIO : 0);
 }
 
+static int goodix_i2c_write_u8(struct i2c_client *client, u16 reg, u8 value)
+{
+   return goodix_i2c_write(client, reg, , sizeof(value));
+}
+
 static int goodix_get_cfg_len(u16 id)
 {
switch (id) {
@@ -281,6 +290,18 @@ static irqreturn_t goodix_ts_irq_handler(int irq, void 
*dev_id)
return IRQ_HANDLED;
 }
 
+static void goodix_free_irq(struct goodix_ts_data *ts)
+{
+   devm_free_irq(>client->dev, ts->client->irq, ts);
+}
+
+static int goodix_request_irq(struct goodix_ts_data *ts)
+{
+   return devm_request_threaded_irq(>client->dev, ts->client->irq,
+NULL, goodix_ts_irq_handler,
+ts->irq_flags, ts->client->name, ts);
+}
+
 /**
  * goodix_check_cfg - Checks if config fw is valid
  *
@@ -585,7 +606,6 @@ static int goodix_request_input_dev(struct goodix_ts_data 
*ts)
 static int goodix_configure_dev(struct goodix_ts_data *ts)
 {
int error;
-   unsigned long irq_flags;
 
goodix_read_config(ts);
 
@@ -593,10 +613,8 @@ static int goodix_configure_dev(struct goodix_ts_data *ts)
if (error)
return error;
 
-   irq_flags = goodix_irq_flags[ts->int_trigger_type] | IRQF_ONESHOT;
-   error = devm_request_threaded_irq(>client->dev, ts->client->irq,
- NULL, goodix_ts_irq_handler,
- irq_flags, ts->client->name, ts);
+   ts->irq_flags = goodix_irq_flags[ts->int_trigger_type] | IRQF_ONESHOT;
+   error = goodix_request_irq(ts);
if (error) {
dev_err(>client->dev, "request IRQ failed: %d\n", error);
return error;
@@ -720,6 +738,73 @@ static int goodix_ts_remove(struct i2c_client *client)
return 0;
 }
 
+static int __maybe_unused goodix_suspend(struct device *dev)
+{
+   struct i2c_client *client = to_i2c_client(dev);
+   struct goodix_ts_data *ts = i2c_get_clientdata(client);
+   int error;
+
+   /* We need gpio pins to suspend/resume */
+   if (!ts->gpiod_int || !ts->gpiod_rst)
+   return 0;
+
+   wait_for_completion(>firmware_loading_complete);
+
+   /* Free IRQ as IRQ pin is used as output in the suspend sequence */
+   goodix_free_irq(ts);
+   /* Output LOW on the INT pin for 5 ms */
+   error = gpiod_direction_output(ts->gpiod_int, 0);
+   if (error) {
+   goodix_request_irq(ts);
+   return error;
+   }
+   usleep_range(5000, 6000);
+
+   error = goodix_i2c_write_u8(ts->client, GOODIX_REG_COMMAND,
+   GOODIX_CMD_SCREEN_OFF);
+   if (error) {
+   dev_err(>client->dev, "Screen off command failed\n");
+   gpiod_direction_input(ts->gpiod_int);
+   goodix_request_irq(ts);
+   return -EAGAIN;
+   }
+
+   /*
+* The datasheet specifies that the interval between sending screen-off
+* command and wake-up should be longer than 58 ms. To avoid waking up
+* sooner, delay 58ms here.
+*/
+   msleep(58);
+   return 0;
+}
+
+static int __maybe_unused goodix_resume(struct device *dev)
+{
+   struct i2c_client *client = to_i2c_client(dev);
+   struct goodix_ts_data *ts = i2c_get_clientdata(client);
+   int error;
+
+   if (!ts->gpiod_int || !ts->gpiod_rst)
+   return 0;

[PATCH v10 3/8] Input: goodix - write configuration data to device

2015-11-18 Thread Irina Tirdea 
Goodix devices can be configured by writing custom data to the device at
init. The configuration data is read with request_firmware from
"goodix__cfg.bin", where  is the product id read from the device
(e.g.: goodix_911_cfg.bin for Goodix GT911, goodix_9271_cfg.bin for
GT9271).

The configuration information has a specific format described in the Goodix
datasheet. It includes X/Y resolution, maximum supported touch points,
interrupt flags, various sensitivity factors and settings for advanced
features (like gesture recognition).

Before writing the firmware, it is necessary to reset the device. If
the device ACPI/DT information does not declare gpio pins (needed for
reset), writing the firmware will not be available for these devices.

This is based on Goodix datasheets for GT911 and GT9271 and on Goodix
driver gt9xx.c for Android (publicly available in Android kernel
trees for various devices).

Signed-off-by: Octavian Purdila 
Signed-off-by: Irina Tirdea 
---
 drivers/input/touchscreen/goodix.c | 247 -
 1 file changed, 215 insertions(+), 32 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c 
b/drivers/input/touchscreen/goodix.c
index 4744032..0911b0c9 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -16,6 +16,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -40,6 +41,10 @@ struct goodix_ts_data {
int cfg_len;
struct gpio_desc *gpiod_int;
struct gpio_desc *gpiod_rst;
+   u16 id;
+   u16 version;
+   char *cfg_name;
+   struct completion firmware_loading_complete;
 };
 
 #define GOODIX_GPIO_INT_NAME   "irq"
@@ -124,6 +129,39 @@ static int goodix_i2c_read(struct i2c_client *client,
return ret < 0 ? ret : (ret != ARRAY_SIZE(msgs) ? -EIO : 0);
 }
 
+/**
+ * goodix_i2c_write - write data to a register of the i2c slave device.
+ *
+ * @client: i2c device.
+ * @reg: the register to write to.
+ * @buf: raw data buffer to write.
+ * @len: length of the buffer to write
+ */
+static int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf,
+   unsigned len)
+{
+   u8 *addr_buf;
+   struct i2c_msg msg;
+   int ret;
+
+   addr_buf = kmalloc(len + 2, GFP_KERNEL);
+   if (!addr_buf)
+   return -ENOMEM;
+
+   addr_buf[0] = reg >> 8;
+   addr_buf[1] = reg & 0xFF;
+   memcpy(_buf[2], buf, len);
+
+   msg.flags = 0;
+   msg.addr = client->addr;
+   msg.buf = addr_buf;
+   msg.len = len + 2;
+
+   ret = i2c_transfer(client->adapter, , 1);
+   kfree(addr_buf);
+   return ret < 0 ? ret : (ret != 1 ? -EIO : 0);
+}
+
 static int goodix_get_cfg_len(u16 id)
 {
switch (id) {
@@ -243,6 +281,73 @@ static irqreturn_t goodix_ts_irq_handler(int irq, void 
*dev_id)
return IRQ_HANDLED;
 }
 
+/**
+ * goodix_check_cfg - Checks if config fw is valid
+ *
+ * @ts: goodix_ts_data pointer
+ * @cfg: firmware config data
+ */
+static int goodix_check_cfg(struct goodix_ts_data *ts,
+   const struct firmware *cfg)
+{
+   int i, raw_cfg_len;
+   u8 check_sum = 0;
+
+   if (cfg->size > GOODIX_CONFIG_MAX_LENGTH) {
+   dev_err(>client->dev,
+   "The length of the config fw is not correct");
+   return -EINVAL;
+   }
+
+   raw_cfg_len = cfg->size - 2;
+   for (i = 0; i < raw_cfg_len; i++)
+   check_sum += cfg->data[i];
+   check_sum = (~check_sum) + 1;
+   if (check_sum != cfg->data[raw_cfg_len]) {
+   dev_err(>client->dev,
+   "The checksum of the config fw is not correct");
+   return -EINVAL;
+   }
+
+   if (cfg->data[raw_cfg_len + 1] != 1) {
+   dev_err(>client->dev,
+   "Config fw must have Config_Fresh register set");
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+/**
+ * goodix_send_cfg - Write fw config to device
+ *
+ * @ts: goodix_ts_data pointer
+ * @cfg: config firmware to write to device
+ */
+static int goodix_send_cfg(struct goodix_ts_data *ts,
+  const struct firmware *cfg)
+{
+   int error;
+
+   error = goodix_check_cfg(ts, cfg);
+   if (error)
+   return error;
+
+   error = goodix_i2c_write(ts->client, GOODIX_REG_CONFIG_DATA, cfg->data,
+cfg->size);
+   if (error) {
+   dev_err(>client->dev, "Failed to write config data: %d",
+   error);
+   return error;
+   }
+   dev_dbg(>client->dev, "Config sent successfully.");
+
+   /* Let the firmware reconfigure itself, so sleep for 10ms */
+   usleep_range(1, 11000);
+
+   return 0;
+}
+
 static int goodix_int_sync(struct goodix_ts_data *ts)
 {
int error;
@@ -371,30 +476,29

[PATCH v10 2/8] Input: goodix - reset device at init

2015-11-18 Thread Irina Tirdea 
After power on, it is recommended that the driver resets the device.
The reset procedure timing is described in the datasheet and is used
at device init (before writing device configuration) and
for power management. It is a sequence of setting the interrupt
and reset pins high/low at specific timing intervals. This procedure
also includes setting the slave address to the one specified in the
ACPI/device tree.

This is based on Goodix datasheets for GT911 and GT9271 and on Goodix
driver gt9xx.c for Android (publicly available in Android kernel
trees for various devices).

For reset the driver needs to control the interrupt and
reset gpio pins (configured through ACPI/device tree). For devices
that do not have the gpio pins properly declared, the functionality
depending on these pins will not be available, but the device can still
be used with basic functionality.

For both device tree and ACPI, the interrupt gpio pin configuration is
read from the "irq-gpio" property and the reset pin configuration is
read from the "reset-gpio" property. For ACPI 5.1, named properties
can be specified using the _DSD section. This functionality will not be
available for devices that use indexed gpio pins declared in the _CRS
section (we need to provide backward compatibility with devices
that do not support using the interrupt gpio pin as output).

For ACPI, the pins can be specified using ACPI 5.1:
Device (STAC)
{
Name (_HID, "GDIX1001")
...

Method (_CRS, 0, Serialized)
{
Name (RBUF, ResourceTemplate ()
{
I2cSerialBus (0x0014, ControllerInitiated, 0x00061A80,
AddressingMode7Bit, "\\I2C0",
0x00, ResourceConsumer, ,
)

GpioInt (Edge, ActiveHigh, Exclusive, PullNone, 0x,
"\\I2C0", 0x00, ResourceConsumer, ,
 )
 {   // Pin list
 0
 }

GpioIo (Exclusive, PullDown, 0x, 0x,
IoRestrictionOutputOnly, "\\I2C0", 0x00,
ResourceConsumer, ,
)
{
 1
}
})
Return (RBUF)
}

Name (_DSD,  Package ()
{
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package ()
{
Package (2) {"irq-gpio", Package() {^STAC, 0, 0, 0 }},
Package (2) {"reset-gpio", Package() {^STAC, 1, 0, 0 }},
...
}
}

Signed-off-by: Octavian Purdila 
Signed-off-by: Irina Tirdea 
---
 .../bindings/input/touchscreen/goodix.txt  |   5 +
 drivers/input/touchscreen/Kconfig  |   1 +
 drivers/input/touchscreen/goodix.c | 101 +
 3 files changed, 107 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt 
b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
index 8ba98ee..7137881 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
@@ -12,6 +12,8 @@ Required properties:
  - reg : I2C address of the chip. Should be 0x5d or 0x14
  - interrupt-parent: Interrupt controller to which the chip is connected
  - interrupts  : Interrupt to which the chip is connected
+ - irq-gpio: GPIO pin used for IRQ
+ - reset-gpio  : GPIO pin used for reset
 
 Example:
 
@@ -23,6 +25,9 @@ Example:
reg = <0x5d>;
interrupt-parent = <>;
interrupts = <0 0>;
+
+   irq-gpio = < 0 0>;
+   reset-gpio = < 1 0>;
};
 
/* ... */
diff --git a/drivers/input/touchscreen/Kconfig 
b/drivers/input/touchscreen/Kconfig
index 2ccc522..121a0ac 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -324,6 +324,7 @@ config TOUCHSCREEN_FUJITSU
 config TOUCHSCREEN_GOODIX
tristate "Goodix I2C touchscreen"
depends on I2C
+   depends on GPIOLIB
help
  Say Y here if you have the Goodix touchscreen (such as one
  installed in Onda v975w tablets) connected to your
diff --git a/drivers/input/touchscreen/goodix.c 
b/drivers/input/touchscreen/goodix.c
index 56d0330..4744032 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -16,6 +16,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -37,8 +38,13 @@ struct goodix_ts_data {
unsigned int int_trigger_type;
bool rotated_screen;
int cfg_len;
+   struct gpio_desc *gpiod_int;
+   struct gpio_desc *gpiod_rst;
 };
 
+#define GOODIX_GPIO_INT_NAME   "irq"
+#define GOODIX_GPIO_RST_NAME   "reset"
+
 #define GOODIX_MAX_HEIGHT  4096
 #define GOODIX_MAX_WIDTH

[PATCH v10 5/8] Input: goodix - use goodix_i2c_write_u8 instead of i2c_master_send

2015-11-18 Thread Irina Tirdea 
Use goodix_i2c_write_u8 instead of i2c_master_send to simplify code.

Signed-off-by: Irina Tirdea 
Tested-by: Bastien Nocera 
Tested-by: Aleksei Mamlin 
---
 drivers/input/touchscreen/goodix.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c 
b/drivers/input/touchscreen/goodix.c
index 0fd472d..9b7f2ad 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -275,16 +275,11 @@ static void goodix_process_events(struct goodix_ts_data 
*ts)
  */
 static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
 {
-   static const u8 end_cmd[] = {
-   GOODIX_READ_COOR_ADDR >> 8,
-   GOODIX_READ_COOR_ADDR & 0xff,
-   0
-   };
struct goodix_ts_data *ts = dev_id;
 
goodix_process_events(ts);
 
-   if (i2c_master_send(ts->client, end_cmd, sizeof(end_cmd)) < 0)
+   if (goodix_i2c_write_u8(ts->client, GOODIX_READ_COOR_ADDR, 0) < 0)
dev_err(>client->dev, "I2C write end_cmd error\n");
 
return IRQ_HANDLED;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10 8/8] Input: goodix - add runtime power management support

2015-11-18 Thread Irina Tirdea 
Add support for runtime power management so that the device is
turned off when not used (when the userspace holds no open
handles of the input device). The device uses autosuspend with a
default delay of 2 seconds, so the device will suspend if no
handles to it are open for 2 seconds.

The runtime management support is only available if the gpio pins
are properly initialized from ACPI/DT.

Signed-off-by: Irina Tirdea 
---
 drivers/input/touchscreen/goodix.c | 159 +
 1 file changed, 145 insertions(+), 14 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c 
b/drivers/input/touchscreen/goodix.c
index ea5042f..a793b8c 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 struct goodix_ts_data {
@@ -48,6 +49,10 @@ struct goodix_ts_data {
unsigned long irq_flags;
atomic_t esd_timeout;
struct delayed_work esd_work;
+   bool suspended;
+   atomic_t open_count;
+   /* Protects power management calls and access to suspended flag */
+   struct mutex mutex;
 };
 
 #define GOODIX_GPIO_INT_NAME   "irq"
@@ -78,6 +83,8 @@ struct goodix_ts_data {
 #define MAX_CONTACTS_LOC   5
 #define TRIGGER_LOC6
 
+#define GOODIX_AUTOSUSPEND_DELAY_MS2000
+
 static const unsigned long goodix_irq_flags[] = {
IRQ_TYPE_EDGE_RISING,
IRQ_TYPE_EDGE_FALLING,
@@ -193,6 +200,29 @@ static int goodix_get_cfg_len(u16 id)
}
 }
 
+static int goodix_set_power_state(struct goodix_ts_data *ts, bool on)
+{
+   int error;
+
+   if (on) {
+   error = pm_runtime_get_sync(>client->dev);
+   } else {
+   pm_runtime_mark_last_busy(>client->dev);
+   error = pm_runtime_put_autosuspend(>client->dev);
+   }
+
+   if (error < 0) {
+   dev_err(>client->dev,
+   "failed to change power state to %d\n", on);
+   if (on)
+   pm_runtime_put_noidle(>client->dev);
+
+   return error;
+   }
+
+   return 0;
+}
+
 static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
 {
int touch_num;
@@ -501,11 +531,13 @@ static ssize_t goodix_esd_timeout_store(struct device 
*dev,
return error;
 
esd_timeout = atomic_read(>esd_timeout);
-   if (esd_timeout && !new_esd_timeout)
+   if (esd_timeout && !new_esd_timeout &&
+   pm_runtime_active(>client->dev))
goodix_disable_esd(ts);
 
atomic_set(>esd_timeout, new_esd_timeout);
-   if (!esd_timeout && new_esd_timeout)
+   if (!esd_timeout && new_esd_timeout &&
+   pm_runtime_active(>client->dev))
goodix_enable_esd(ts);
 
return count;
@@ -518,17 +550,23 @@ static ssize_t goodix_dump_config_show(struct device *dev,
u8 config[GOODIX_CONFIG_MAX_LENGTH];
int error, count = 0, i;
 
+   error = goodix_set_power_state(ts, true);
+   if (error)
+   return error;
error = goodix_i2c_read(ts->client, GOODIX_REG_CONFIG_DATA,
config, ts->cfg_len);
if (error) {
dev_warn(>client->dev,
 "Error reading config (%d)\n",  error);
+   goodix_set_power_state(ts, false);
return error;
}
+   goodix_set_power_state(ts, false);
 
for (i = 0; i < ts->cfg_len; i++)
count += scnprintf(buf + count, PAGE_SIZE - count, "%02x ",
   config[i]);
+
return count;
 }
 
@@ -547,6 +585,34 @@ static const struct attribute_group goodix_attr_group = {
.attrs = goodix_attrs,
 };
 
+static int goodix_open(struct input_dev *input_dev)
+{
+   struct goodix_ts_data *ts = input_get_drvdata(input_dev);
+   int error;
+
+   if (!ts->gpiod_int || !ts->gpiod_rst)
+   return 0;
+
+   wait_for_completion(>firmware_loading_complete);
+
+   error = goodix_set_power_state(ts, true);
+   if (error)
+   return error;
+   atomic_inc(>open_count);
+   return 0;
+}
+
+static void goodix_close(struct input_dev *input_dev)
+{
+   struct goodix_ts_data *ts = input_get_drvdata(input_dev);
+
+   if (!ts->gpiod_int || !ts->gpiod_rst)
+   return;
+
+   goodix_set_power_state(ts, false);
+   atomic_dec(>open_count);
+}
+
 /**
  * goodix_get_gpio_config - Get GPIO config from ACPI/DT
  *
@@ -718,6 +784,9 @@ static int goodix_request_input_dev(struct goodix_ts_data 
*ts)
ts->input_dev->id.vendor = 0x0416;
ts->input_dev->id.product = ts->id;
ts->input_dev->id.version = ts->version;
+   ts->input_dev->open = goodix_open;
+   ts->input_dev->close = goodix_close;
+   input_set_drvdata(ts->input_dev, ts);
 
error =

[PATCH v10 6/8] Input: goodix - add support for ESD

2015-11-18 Thread Irina Tirdea 
Add ESD (Electrostatic Discharge) protection mechanism.

The driver enables ESD protection in HW and checks a register
to determine if ESD occurred. If ESD is signalled by the HW,
the driver will reset the device.

The ESD poll time (in ms) can be set through the sysfs property
esd_timeout. If it is set to 0, ESD protection is disabled.
Recommended value is 2000 ms. The initial value for ESD timeout
can be set through esd-recovery-timeout-ms ACPI/DT property.
If there is no such property defined, ESD protection is disabled.
For ACPI 5.1, the property can be specified using _DSD properties:
 Device (STAC)
 {
 Name (_HID, "GDIX1001")
 ...

 Name (_DSD,  Package ()
 {
 ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
 Package ()
 {
 Package (2) { "esd-recovery-timeout-ms", Package(1) { 2000 }},
 ...
 }
 })
 }

The ESD protection mechanism is only available if the gpio pins
are properly initialized from ACPI/DT.

This is based on Goodix datasheets for GT911 and GT9271 and on Goodix
driver gt9xx.c for Android (publicly available in Android kernel
trees for various devices).

Signed-off-by: Irina Tirdea 
---
 .../bindings/input/touchscreen/goodix.txt  |   6 +
 drivers/input/touchscreen/goodix.c | 160 -
 2 files changed, 161 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt 
b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
index 7137881..4db3393 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
@@ -14,6 +14,12 @@ Required properties:
  - interrupts  : Interrupt to which the chip is connected
  - irq-gpio: GPIO pin used for IRQ
  - reset-gpio  : GPIO pin used for reset
+Optional properties:
+
+ - esd-recovery-timeout-ms : ESD poll time (in milli seconds) for the driver to
+check if ESD occurred and in that case reset the
+device. ESD is disabled if this property is not set
+or is set to 0.
 
 Example:
 
diff --git a/drivers/input/touchscreen/goodix.c 
b/drivers/input/touchscreen/goodix.c
index 9b7f2ad..28cbfa9 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -46,10 +46,13 @@ struct goodix_ts_data {
char *cfg_name;
struct completion firmware_loading_complete;
unsigned long irq_flags;
+   atomic_t esd_timeout;
+   struct delayed_work esd_work;
 };
 
 #define GOODIX_GPIO_INT_NAME   "irq"
 #define GOODIX_GPIO_RST_NAME   "reset"
+#define GOODIX_DEVICE_ESD_TIMEOUT_PROPERTY "esd-recovery-timeout-ms"
 
 #define GOODIX_MAX_HEIGHT  4096
 #define GOODIX_MAX_WIDTH   4096
@@ -64,6 +67,8 @@ struct goodix_ts_data {
 /* Register defines */
 #define GOODIX_REG_COMMAND 0x8040
 #define GOODIX_CMD_SCREEN_OFF  0x05
+#define GOODIX_CMD_ESD_ENABLED 0xAA
+#define GOODIX_REG_ESD_CHECK   0x8041
 
 #define GOODIX_READ_COOR_ADDR  0x814E
 #define GOODIX_REG_CONFIG_DATA 0x8047
@@ -406,6 +411,119 @@ static int goodix_reset(struct goodix_ts_data *ts)
return goodix_int_sync(ts);
 }
 
+static void goodix_disable_esd(struct goodix_ts_data *ts)
+{
+   if (!atomic_read(>esd_timeout))
+   return;
+   cancel_delayed_work_sync(>esd_work);
+}
+
+static int goodix_enable_esd(struct goodix_ts_data *ts)
+{
+   int error, esd_timeout;
+
+   esd_timeout = atomic_read(>esd_timeout);
+   if (!esd_timeout)
+   return 0;
+
+   error = goodix_i2c_write_u8(ts->client, GOODIX_REG_ESD_CHECK,
+   GOODIX_CMD_ESD_ENABLED);
+   if (error) {
+   dev_err(>client->dev, "Failed to enable ESD: %d\n", error);
+   return error;
+   }
+
+   schedule_delayed_work(>esd_work, round_jiffies_relative(
+ msecs_to_jiffies(esd_timeout)));
+   return 0;
+}
+
+static void goodix_esd_work(struct work_struct *work)
+{
+   struct goodix_ts_data *ts = container_of(work, struct goodix_ts_data,
+esd_work.work);
+   int retries = 3, error;
+   u8 esd_data[2];
+   const struct firmware *cfg = NULL;
+
+   wait_for_completion(>firmware_loading_complete);
+
+   while (--retries) {
+   error = goodix_i2c_read(ts->client, GOODIX_REG_COMMAND,
+   esd_data, sizeof(esd_data));
+   if (error)
+   continue;
+   if (esd_data[0] != GOODIX_CMD_ESD_ENABLED &&
+   esd_data[1] == GOODIX_CMD_ESD_ENABLED) {
+   /* feed the watchdog */
+

[PATCH v10 7/8] Input: goodix - add sysfs interface to dump config

2015-11-18 Thread Irina Tirdea 
Goodix devices have a configuration information register area that
specify various parameters for the device. The configuration information
has a specific format described in the Goodix datasheet. It includes X/Y
resolution, maximum supported touch points, interrupt flags, various
sesitivity factors and settings for advanced features (like gesture
recognition).

Export a sysfs interface that would allow reading the configuration
information. The default device configuration can be used as a starting
point for creating a valid configuration firmware used by the device at
init time to update its configuration.

This sysfs interface will be exported only if the gpio pins are properly
initialized from ACPI/DT.

Signed-off-by: Irina Tirdea 
Tested-by: Bastien Nocera 
Tested-by: Aleksei Mamlin 
---
 drivers/input/touchscreen/goodix.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/input/touchscreen/goodix.c 
b/drivers/input/touchscreen/goodix.c
index 28cbfa9..ea5042f 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -511,12 +511,35 @@ static ssize_t goodix_esd_timeout_store(struct device 
*dev,
return count;
 }
 
+static ssize_t goodix_dump_config_show(struct device *dev,
+  struct device_attribute *attr, char *buf)
+{
+   struct goodix_ts_data *ts = dev_get_drvdata(dev);
+   u8 config[GOODIX_CONFIG_MAX_LENGTH];
+   int error, count = 0, i;
+
+   error = goodix_i2c_read(ts->client, GOODIX_REG_CONFIG_DATA,
+   config, ts->cfg_len);
+   if (error) {
+   dev_warn(>client->dev,
+"Error reading config (%d)\n",  error);
+   return error;
+   }
+
+   for (i = 0; i < ts->cfg_len; i++)
+   count += scnprintf(buf + count, PAGE_SIZE - count, "%02x ",
+  config[i]);
+   return count;
+}
+
 /* ESD timeout in ms. Default disabled (0). Recommended 2000 ms. */
 static DEVICE_ATTR(esd_timeout, S_IRUGO | S_IWUSR, goodix_esd_timeout_show,
   goodix_esd_timeout_store);
+static DEVICE_ATTR(dump_config, S_IRUGO, goodix_dump_config_show, NULL);
 
 static struct attribute *goodix_attrs[] = {
_attr_esd_timeout.attr,
+   _attr_dump_config.attr,
NULL
 };
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Chromebook touchpad not responsive on latest kernel

2015-11-18 Thread Dmitry Torokhov 
Hi Rich,

On Wed, Nov 18, 2015 at 11:44:03AM -0500, Rich K wrote:
> Dear Dmitry Torokhov,
> 
> I have filed a bug report on launchpad and gathered the information from
> wiki.ubuntu.com/Bugs/Upstream/kernel. I believe that you are the correct
> person to email with this concern.
> 
> https://bugs.launchpad.net/bugs/1513297

...

> XE500C12-K01US Samsung Chromebook 2 Ubuntu 16.04 touchpad not responsive
> 
> The chromebook is new and the touchpad worked great with chromiumos but not
> at all with Ubuntu.

I believe this model uses Atmel touchpad, for which we do have a driver
in mainline (even though it is not the most up-to-date one). I see that
Ubuntu has it configured to be a module, does it help if you do:

sudo modprobe atmel_mxt_ts

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] hid: usbhid: hid-core: fix recursive deadlock

2015-11-18 Thread Ioan-Adrian Ratiu 
The critical section protected by usbhid->lock in hid_ctrl() is too
big and in rare cases causes a recursive deadlock because of its call
to hid_input_report().

This deadlock reproduces on newer wacom tablets like 056a:033c because
the wacom driver in its irq handler ends up calling hid_hw_request()
from wacom_intuos_schedule_prox_event() in wacom_wac.c. What this means
is that it submits a report to reschedule a proximity read through a
sync ctrl call which grabs the lock in hid_ctrl(struct urb *urb)
before calling hid_input_report(). When the irq kicks in on the same
cpu, it also tries to grab the lock resulting in a recursive deadlock.

The proper fix is to shrink the critical section in hid_ctrl() to
protect only the instructions which modify usbhid, thus move the lock
after the hid_input_report() call and the deadlock dissapears.

Signed-off-by: Ioan-Adrian Ratiu 
---
 drivers/hid/usbhid/hid-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 36712e9..5dd426f 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -477,8 +477,6 @@ static void hid_ctrl(struct urb *urb)
struct usbhid_device *usbhid = hid->driver_data;
int unplug = 0, status = urb->status;
 
-   spin_lock(>lock);
-
switch (status) {
case 0: /* success */
if (usbhid->ctrl[usbhid->ctrltail].dir == USB_DIR_IN)
@@ -498,6 +496,8 @@ static void hid_ctrl(struct urb *urb)
hid_warn(urb->dev, "ctrl urb status %d received\n", status);
}
 
+   spin_lock(>lock);
+
if (unplug) {
usbhid->ctrltail = usbhid->ctrlhead;
} else {
-- 
2.6.3

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock

2015-11-18 Thread Ioan-Adrian Ratiu 
Here are some images with more information on this deadlock, might be helpful.

First part of lockdep report:
http://imgur.com/clLsCWe

Second part:
http://imgur.com/Wa2PzRl

Here are some printk's of mine while reproducing + debugging the issue:
http://imgur.com/SETOHT7
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock

2015-11-18 Thread Jiri Kosina 
On Wed, 18 Nov 2015, Ioan-Adrian Ratiu wrote:

> The critical section protected by usbhid->lock in hid_ctrl() is too
> big and in rare cases causes a recursive deadlock because of its call
> to hid_input_report().
> 
> This deadlock reproduces on newer wacom tablets like 056a:033c because
> the wacom driver in its irq handler ends up calling hid_hw_request()
> from wacom_intuos_schedule_prox_event() in wacom_wac.c. What this means
> is that it submits a report to reschedule a proximity read through a
> sync ctrl call which grabs the lock in hid_ctrl(struct urb *urb)
> before calling hid_input_report(). When the irq kicks in on the same
> cpu, it also tries to grab the lock resulting in a recursive deadlock.
> 
> The proper fix is to shrink the critical section in hid_ctrl() to
> protect only the instructions which modify usbhid, thus move the lock
> after the hid_input_report() call and the deadlock dissapears.

I think the proper fix actually is to spin_lock_irqsave() in hid_ctrl(), 
isn't it?

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] HID: multitouch: enable palm rejection if device implements confidence usage

2015-11-18 Thread Benjamin Tissoires 
On Wed, Nov 18, 2015 at 10:41 AM, Allen Hung  wrote:
> The usage Confidence is mandary to Windows Precision Touchpad devices. The
> appearance of this usage is checked in hidinput_connect but the quirk
> MT_QUIRK_VALID_IS_CONFIDENCE is not applied to the device accordingly.
> Apply this quirk and also remove quirk MT_QUIRK_ALWAYS_VALID to enable the
> palm rejection for the devices which implement the usage confidence in its
> input reports.
>
> Tested on Dell XPS 13 laptop.
>
> Signed-off-by: Allen Hung 
> ---
>  drivers/hid/hid-multitouch.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 426b2f1..661de1d 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -448,6 +448,8 @@ static int mt_touch_input_mapping(struct hid_device 
> *hdev, struct hid_input *hi,
> mt_store_field(usage, td, hi);
> return 1;
> case HID_DG_CONFIDENCE:
> +   cls->quirks &= ~MT_QUIRK_ALWAYS_VALID;
> +   cls->quirks |= MT_QUIRK_VALID_IS_CONFIDENCE;

NACK: you are going to break a *lot* of multitouch panels with this change.

Please target the modification to only touchpads that are following
the Precision Touchpad spec. Even there, I will need to be sure that
there will be no side effect, but we will have a much thiner spectre
than blindly enabling this.

For the record, Windows 7 touchscreens used to put any meaning in the
confidence tag.

Cheers,
Benjamin

> mt_store_field(usage, td, hi);
> return 1;
> case HID_DG_TIPSWITCH:
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] HID: multitouch: enable palm rejection if device implements confidence usage

2015-11-18 Thread Allen Hung 
The usage Confidence is mandary to Windows Precision Touchpad devices. The
appearance of this usage is checked in hidinput_connect but the quirk
MT_QUIRK_VALID_IS_CONFIDENCE is not applied to the device accordingly.
Apply this quirk and also remove quirk MT_QUIRK_ALWAYS_VALID to enable the
palm rejection for the devices which implement the usage confidence in its
input reports.

Tested on Dell XPS 13 laptop.

Signed-off-by: Allen Hung 
---
 drivers/hid/hid-multitouch.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 426b2f1..661de1d 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -448,6 +448,8 @@ static int mt_touch_input_mapping(struct hid_device *hdev, 
struct hid_input *hi,
mt_store_field(usage, td, hi);
return 1;
case HID_DG_CONFIDENCE:
+   cls->quirks &= ~MT_QUIRK_ALWAYS_VALID;
+   cls->quirks |= MT_QUIRK_VALID_IS_CONFIDENCE;
mt_store_field(usage, td, hi);
return 1;
case HID_DG_TIPSWITCH:
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock

2015-11-18 Thread Josh Cartwright 
On Wed, Nov 18, 2015 at 11:05:44PM +0200, Ioan-Adrian Ratiu wrote:
> On Wed, 18 Nov 2015 21:37:42 +0100 (CET)
> Jiri Kosina  wrote:
> 
> > On Wed, 18 Nov 2015, Ioan-Adrian Ratiu wrote:
> > 
> > > The critical section protected by usbhid->lock in hid_ctrl() is too
> > > big and in rare cases causes a recursive deadlock because of its call
> > > to hid_input_report().
> > > 
> > > This deadlock reproduces on newer wacom tablets like 056a:033c because
> > > the wacom driver in its irq handler ends up calling hid_hw_request()
> > > from wacom_intuos_schedule_prox_event() in wacom_wac.c. What this means
> > > is that it submits a report to reschedule a proximity read through a
> > > sync ctrl call which grabs the lock in hid_ctrl(struct urb *urb)
> > > before calling hid_input_report(). When the irq kicks in on the same
> > > cpu, it also tries to grab the lock resulting in a recursive deadlock.
> > > 
> > > The proper fix is to shrink the critical section in hid_ctrl() to
> > > protect only the instructions which modify usbhid, thus move the lock
> > > after the hid_input_report() call and the deadlock dissapears.  
> > 
> > I think the proper fix actually is to spin_lock_irqsave() in hid_ctrl(), 
> > isn't it?
> > 
> 
> That was my first attempt, yes, but the deadlock still happens with interrupts
> disabled. It is very weird, I know.

I think your best course of action is to figure out why this is the
case, instead of continuing with trying to solve the symptoms.  Do you
have actual callstacks showing the cases where you hit?  That might be
useful to share (your lockdep picture cuts out the callstacks).

Also, have you tried without the PREEMPT_RT patch in the picture at all?

  Josh


signature.asc
Description: PGP signature

[RESEND PATCH] hid: Drop owner assignment from i2c_driver

2015-11-18 Thread Krzysztof Kozlowski 
i2c_driver does not need to set an owner because i2c_register_driver()
will set it.

Signed-off-by: Krzysztof Kozlowski 
Acked-by: Benjamin Tissoires 

---

The coccinelle script which generated the patch was sent here:
http://www.spinics.net/lists/kernel/msg2029903.html
---
 drivers/hid/i2c-hid/i2c-hid.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 10bd8e6e4c9c..55d8f9d25696 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -1184,7 +1184,6 @@ MODULE_DEVICE_TABLE(i2c, i2c_hid_id_table);
 static struct i2c_driver i2c_hid_driver = {
.driver = {
.name   = "i2c_hid",
-   .owner  = THIS_MODULE,
.pm = _hid_pm,
.acpi_match_table = ACPI_PTR(i2c_hid_acpi_match),
.of_match_table = of_match_ptr(i2c_hid_of_match),
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock

2015-11-18 Thread Ioan-Adrian Ratiu 
On Wed, 18 Nov 2015 17:58:56 -0600
Josh Cartwright  wrote:

> On Wed, Nov 18, 2015 at 11:05:44PM +0200, Ioan-Adrian Ratiu wrote:
> > On Wed, 18 Nov 2015 21:37:42 +0100 (CET)
> > Jiri Kosina  wrote:
> >   
> > > On Wed, 18 Nov 2015, Ioan-Adrian Ratiu wrote:
> > >   
> > > > The critical section protected by usbhid->lock in hid_ctrl() is too
> > > > big and in rare cases causes a recursive deadlock because of its call
> > > > to hid_input_report().
> > > > 
> > > > This deadlock reproduces on newer wacom tablets like 056a:033c because
> > > > the wacom driver in its irq handler ends up calling hid_hw_request()
> > > > from wacom_intuos_schedule_prox_event() in wacom_wac.c. What this means
> > > > is that it submits a report to reschedule a proximity read through a
> > > > sync ctrl call which grabs the lock in hid_ctrl(struct urb *urb)
> > > > before calling hid_input_report(). When the irq kicks in on the same
> > > > cpu, it also tries to grab the lock resulting in a recursive deadlock.
> > > > 
> > > > The proper fix is to shrink the critical section in hid_ctrl() to
> > > > protect only the instructions which modify usbhid, thus move the lock
> > > > after the hid_input_report() call and the deadlock dissapears.
> > > 
> > > I think the proper fix actually is to spin_lock_irqsave() in hid_ctrl(), 
> > > isn't it?
> > >   
> > 
> > That was my first attempt, yes, but the deadlock still happens with
> > interrupts disabled. It is very weird, I know.  
> 
> I think your best course of action is to figure out why this is the
> case, instead of continuing with trying to solve the symptoms.  Do you
> have actual callstacks showing the cases where you hit?  That might be
> useful to share (your lockdep picture cuts out the callstacks).
> 
> Also, have you tried without the PREEMPT_RT patch in the picture at all?
> 
>   Josh

Yes, of course I tried it without PREEMPT_RT_FULL :) This happens on vanilla
mainline kernels (only after 4.4-rc1 which introduced support for this kind of
tablets).

I also backported all the wacom patches to 4.1 non-RT and the same deadlock
happens.

I've sent another email with some lockdep traces and printk's on a running
vanilla linux-next, maybe it didn't get through, here are the links again:

First part of lockdep report:
http://imgur.com/clLsCWe

Second part:
http://imgur.com/Wa2PzRl

Here are some printk's of mine while reproducing + debugging the issue:
http://imgur.com/SETOHT7

I'll continue to research this more in depth, but progress is slow because I
don't have much time, I'm doing this in my spare time because it's my
girlfriend's tablet.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 2/8] Input: goodix - reset device at init

2015-11-18 Thread Rob Herring 
On Wed, Nov 18, 2015 at 06:31:35PM +0200, Irina Tirdea wrote:
> After power on, it is recommended that the driver resets the device.
> The reset procedure timing is described in the datasheet and is used
> at device init (before writing device configuration) and
> for power management. It is a sequence of setting the interrupt
> and reset pins high/low at specific timing intervals. This procedure
> also includes setting the slave address to the one specified in the
> ACPI/device tree.
> 
> This is based on Goodix datasheets for GT911 and GT9271 and on Goodix
> driver gt9xx.c for Android (publicly available in Android kernel
> trees for various devices).
> 
> For reset the driver needs to control the interrupt and
> reset gpio pins (configured through ACPI/device tree). For devices
> that do not have the gpio pins properly declared, the functionality
> depending on these pins will not be available, but the device can still
> be used with basic functionality.
> 
> For both device tree and ACPI, the interrupt gpio pin configuration is
> read from the "irq-gpio" property and the reset pin configuration is
> read from the "reset-gpio" property. For ACPI 5.1, named properties
> can be specified using the _DSD section. This functionality will not be
> available for devices that use indexed gpio pins declared in the _CRS
> section (we need to provide backward compatibility with devices
> that do not support using the interrupt gpio pin as output).
> 
> For ACPI, the pins can be specified using ACPI 5.1:
> Device (STAC)
> {
> Name (_HID, "GDIX1001")
> ...
> 
> Method (_CRS, 0, Serialized)
> {
> Name (RBUF, ResourceTemplate ()
> {
> I2cSerialBus (0x0014, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\I2C0",
> 0x00, ResourceConsumer, ,
> )
> 
> GpioInt (Edge, ActiveHigh, Exclusive, PullNone, 0x,
> "\\I2C0", 0x00, ResourceConsumer, ,
>  )
>  {   // Pin list
>  0
>  }
> 
> GpioIo (Exclusive, PullDown, 0x, 0x,
> IoRestrictionOutputOnly, "\\I2C0", 0x00,
> ResourceConsumer, ,
> )
> {
>  1
> }
> })
> Return (RBUF)
> }
> 
> Name (_DSD,  Package ()
> {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package ()
> {
> Package (2) {"irq-gpio", Package() {^STAC, 0, 0, 0 }},
> Package (2) {"reset-gpio", Package() {^STAC, 1, 0, 0 }},
> ...
> }
> }
> 
> Signed-off-by: Octavian Purdila 
> Signed-off-by: Irina Tirdea 
> ---
>  .../bindings/input/touchscreen/goodix.txt  |   5 +
>  drivers/input/touchscreen/Kconfig  |   1 +
>  drivers/input/touchscreen/goodix.c | 101 
> +
>  3 files changed, 107 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt 
> b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> index 8ba98ee..7137881 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> @@ -12,6 +12,8 @@ Required properties:
>   - reg   : I2C address of the chip. Should be 0x5d or 
> 0x14
>   - interrupt-parent  : Interrupt controller to which the chip is connected
>   - interrupts: Interrupt to which the chip is connected
> + - irq-gpio  : GPIO pin used for IRQ

Please note here why you need this in addition to just "interrupts". 
Also, it should be irq-gpios instead.

> + - reset-gpio: GPIO pin used for reset

Should be reset-gpios instead.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 6/8] Input: goodix - add support for ESD

2015-11-18 Thread Rob Herring 
On Wed, Nov 18, 2015 at 06:31:39PM +0200, Irina Tirdea wrote:
> Add ESD (Electrostatic Discharge) protection mechanism.
> 
> The driver enables ESD protection in HW and checks a register
> to determine if ESD occurred. If ESD is signalled by the HW,
> the driver will reset the device.
> 
> The ESD poll time (in ms) can be set through the sysfs property
> esd_timeout. If it is set to 0, ESD protection is disabled.
> Recommended value is 2000 ms. The initial value for ESD timeout
> can be set through esd-recovery-timeout-ms ACPI/DT property.
> If there is no such property defined, ESD protection is disabled.
> For ACPI 5.1, the property can be specified using _DSD properties:
>  Device (STAC)
>  {
>  Name (_HID, "GDIX1001")
>  ...
> 
>  Name (_DSD,  Package ()
>  {
>  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>  Package ()
>  {
>  Package (2) { "esd-recovery-timeout-ms", Package(1) { 2000 }},
>  ...
>  }
>  })
>  }
> 
> The ESD protection mechanism is only available if the gpio pins
> are properly initialized from ACPI/DT.
> 
> This is based on Goodix datasheets for GT911 and GT9271 and on Goodix
> driver gt9xx.c for Android (publicly available in Android kernel
> trees for various devices).
> 
> Signed-off-by: Irina Tirdea 
> ---
>  .../bindings/input/touchscreen/goodix.txt  |   6 +

For the binding:

Acked-by: Rob Herring 

>  drivers/input/touchscreen/goodix.c | 160 
> -
>  2 files changed, 161 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt 
> b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> index 7137881..4db3393 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> @@ -14,6 +14,12 @@ Required properties:
>   - interrupts: Interrupt to which the chip is connected
>   - irq-gpio  : GPIO pin used for IRQ
>   - reset-gpio: GPIO pin used for reset
> +Optional properties:
> +
> + - esd-recovery-timeout-ms : ESD poll time (in milli seconds) for the driver 
> to
> +check if ESD occurred and in that case reset the
> +device. ESD is disabled if this property is not 
> set
> +or is set to 0.
>  
>  Example:
>  
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html