* Re: [PATCH] Input: driver for the Goodix touchpanel
From: Dmitry Torokhov @ 2014-09-24 0:45 UTC (permalink / raw)
To: Bastien Nocera; +Cc: Henrik Rydberg, linux-input, Benjamin Tissoires
In-Reply-To: <1411511293.29315.16.camel@hadess.net>
Hi Bastien,
On Wed, Sep 24, 2014 at 12:28:13AM +0200, Bastien Nocera wrote:
> Add a driver for the Goodix touchscreen panel found in Onda v975w
> tablets. The driver is based off the Android driver gt9xx.c found
> in some Android code dumps, but now bears no resemblance to the original
> driver.
>
> The driver was tested on the aforementioned tablet.
>
> Signed-off-by: Bastien Nocera <hadess@hadess.net>
> Tested-by: Bastien Nocera <hadess@hadess.net>
> ---
> MAINTAINERS | 6 +
> drivers/input/touchscreen/Kconfig | 13 ++
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/goodix.c | 452 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 472 insertions(+)
> create mode 100644 drivers/input/touchscreen/goodix.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 670b3dc..7a37464 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4057,6 +4057,12 @@ L: linux-media@vger.kernel.org
> S: Maintained
> F: drivers/media/usb/go7007/
>
> +GOODIX TOUCHSCREEN
> +M: Bastien Nocera <hadess@hadess.net>
> +L: linux-input@vger.kernel.org
> +S: Maintained
> +F: drivers/input/touchscreen/goodix.c
> +
> GPIO SUBSYSTEM
> M: Linus Walleij <linus.walleij@linaro.org>
> M: Alexandre Courbot <gnurou@gmail.com>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 6bb9a7d..c328df3 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -283,6 +283,19 @@ config TOUCHSCREEN_FUJITSU
> To compile this driver as a module, choose M here: the
> module will be called fujitsu-ts.
>
> +config TOUCHSCREEN_GOODIX
> + tristate "Goodix I2C touchscreen"
> + depends on I2C && ACPI
> + help
> + Say Y here if you have the Goodix touchscreen (such as one
> + installed in Onda v975w tablets) connected to your
> + system.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called goodix.
> +
> config TOUCHSCREEN_ILI210X
> tristate "Ilitek ILI210X based touchscreen"
> depends on I2C
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 4be94fc..55af212 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_TOUCHSCREEN_EETI) += eeti_ts.o
> obj-$(CONFIG_TOUCHSCREEN_ELO) += elo.o
> obj-$(CONFIG_TOUCHSCREEN_EGALAX) += egalax_ts.o
> obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
> +obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o
> obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
> obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
> obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> new file mode 100644
> index 0000000..bda6482
> --- /dev/null
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -0,0 +1,452 @@
> +/*
> + * driver for Goodix Touchscreens
> + *
> + * Copyright (c) 2014 Red Hat Inc.
> + *
> + * This code is based on gt9xx.c authored by andrew@goodix.com:
> + *
> + * 2010 - 2012 Goodix Technology.
> + */
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; version 2 of the License.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/slab.h>
> +#include <asm/unaligned.h>
> +
> +struct goodix_ts_data {
> + struct i2c_client *client;
> + struct input_dev *input_dev;
> + int abs_x_max;
> + int abs_y_max;
> + unsigned int max_touch_num;
> + unsigned int int_trigger_type;
> +};
> +
> +#define GOODIX_MAX_HEIGHT 4096
> +#define GOODIX_MAX_WIDTH 4096
> +#define GOODIX_INT_TRIGGER 1
> +#define GOODIX_MAX_TOUCH 10
> +
> +#define GOODIX_CONFIG_MAX_LENGTH 240
> +
> +/* Register defineS */
> +#define GOODIX_READ_COOR_ADDR 0x814E
> +#define GOODIX_REG_CONFIG_DATA 0x8047
> +#define GOODIX_REG_VERSION 0x8140
> +
> +#define RESOLUTION_LOC 1
> +#define TRIGGER_LOC 6
> +
> +#define GOODIX_INFO(fmt, arg...) pr_info("<<-GTP-INFO->> "fmt"\n", ##arg)
> +#define GOODIX_ERROR(fmt, arg...) pr_err("<<-GTP-ERROR->> "fmt"\n", ##arg)
Let's use standard dev_xxx()
> +
> +static const char *goodix_ts_name = "Goodix Capacitive TouchScreen";
> +static const unsigned long goodix_irq_flags[] = {
> + IRQ_TYPE_EDGE_RISING | IRQF_ONESHOT,
> + IRQ_TYPE_EDGE_FALLING | IRQF_ONESHOT,
> + IRQ_TYPE_LEVEL_LOW | IRQF_ONESHOT,
> + IRQ_TYPE_LEVEL_HIGH | IRQF_ONESHOT
> +};
I'd rather you pulled out IRQF_ONESHOT and specified it explicitly in
request_threaded_irq().
> +
> +/**
> + * goodix_i2c_read - read data from a register of the i2c slave device.
> + *
> + * @client: i2c device.
> + * @reg: the register to read from.
> + * @buf: raw write data buffer.
> + * @len: length of the buffer to write
> + */
> +static int goodix_i2c_read(struct i2c_client *client,
> + u16 reg, u8 *buf, int len)
> +{
> + struct i2c_msg msgs[2];
> + u8 wbuf[2] = { reg >> 8, reg & 0xff };
cpu_to_be16()?
> +
> + msgs[0].flags = !I2C_M_RD;
I2C_M_RD is not a boolean, do not negate it.
> + msgs[0].addr = client->addr;
> + msgs[0].len = 2;
> + msgs[0].buf = wbuf;
> +
> + msgs[1].flags = I2C_M_RD;
> + msgs[1].addr = client->addr;
> + msgs[1].len = len;
> + msgs[1].buf = buf;
> +
> + return i2c_transfer(client->adapter, msgs, 2);
return ret < 0 ? ret : (ret != ARRAY_SIZE(msgs) ? -EIO : 0);
I really need to get that i2c_transfer_exact() going again.
> +}
> +
> +/**
> + * goodix_i2c_write - write data to the i2c slave device.
> + *
> + * @client: i2c device.
> + * @reg: the register to read to.
> + * @buf: raw write data buffer.
> + * @len: length of the buffer to write
> + */
> +static int goodix_i2c_write(struct i2c_client *client,
> + u16 reg, u8 *buf, int len)
> +{
> + struct i2c_msg msg;
> + int ret;
> + u8 *wbuf;
> +
> + wbuf = kzalloc(len + 2, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + wbuf[0] = reg >> 8;
> + wbuf[1] = reg & 0xFF;
Again cpu_to_be16().
> + memcpy(&wbuf[2], buf, len);
> +
> + msg.flags = !I2C_M_RD;
Same here, not boolean.
> + msg.addr = client->addr;
> + msg.len = len + 2;
> + msg.buf = wbuf;
> +
> + ret = i2c_transfer(client->adapter, &msg, 1);
> + kfree(wbuf);
> + return ret;
> +}
> +
> +static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
> +{
> + int touch_num;
> + int ret;
> +
> + ret = goodix_i2c_read(ts->client, GOODIX_READ_COOR_ADDR, data, 10);
> + if (ret < 0) {
> + GOODIX_ERROR("I2C transfer error (%d)\n", ret);
> + return ret;
> + }
> +
> + touch_num = data[0] & 0x0f;
> + if (touch_num > GOODIX_MAX_TOUCH)
> + return -EPROTO;
> +
> + if (touch_num > 1) {
> + ret = goodix_i2c_read(ts->client, GOODIX_READ_COOR_ADDR + 10,
> + &data[10], 8 * (touch_num - 1));
> + if (ret < 0)
> + return ret;
> + }
> +
> + return touch_num;
> +}
> +
> +static void goodix_ts_parse_touch(struct goodix_ts_data *ts, u8 *coor_data)
> +{
> + int id = coor_data[0] & 0x0F;
> + int input_x = get_unaligned_le16(&coor_data[1]);
> + int input_y = get_unaligned_le16(&coor_data[3]);
> + int input_w = get_unaligned_le16(&coor_data[5]);
> +
> + input_mt_slot(ts->input_dev, id);
> + input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, true);
> + input_report_abs(ts->input_dev, ABS_MT_POSITION_X, input_x);
> + input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, input_y);
> + input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, input_w);
> + input_report_abs(ts->input_dev, ABS_MT_WIDTH_MAJOR, input_w);
> +}
> +
> +/**
> + * goodix_ts_work_func - Process incoming IRQ
> + *
> + * @ts: our goodix_ts_data pointer
> + *
> + * Called when the IRQ is triggered. Read the current device state, and push
> + * the input events to the user space.
> + */
> +static void goodix_ts_work_func(struct goodix_ts_data *ts)
> +{
> + u8 end_cmd[1] = {0};
> + u8 point_data[1 + 8 * GOODIX_MAX_TOUCH + 1];
> + int touch_num;
> + int i;
> +
> + touch_num = goodix_ts_read_input_report(ts, point_data);
> + if (touch_num < 0)
> + goto exit_work_func;
> +
> + for (i = 0; i < touch_num; i++)
> + goodix_ts_parse_touch(ts, &point_data[1 + 8 * i]);
> +
> + input_mt_sync_frame(ts->input_dev);
> + input_sync(ts->input_dev);
> +
> +exit_work_func:
> + if (goodix_i2c_write(ts->client,
> + GOODIX_READ_COOR_ADDR, end_cmd, 1) < 0)
> + GOODIX_INFO("I2C write end_cmd error");
Why did you need to pull it out of goodix_ts_irq_handler()?
> +}
> +
> +/**
> + * goodix_ts_irq_handler - The IRQ handler
> + *
> + * @irq: interrupt number.
> + * @dev_id: private data pointer.
> + */
> +static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
> +{
> + struct goodix_ts_data *ts = dev_id;
> +
> + goodix_ts_work_func(ts);
> +
> + return IRQ_HANDLED;
> +}
> +
> +/**
> + * goodix_read_config - Read the embedded configuration of the panel
> + *
> + * @ts: our goodix_ts_data pointer
> + *
> + * Must be called during probe
> + */
> +static void goodix_read_config(struct goodix_ts_data *ts)
> +{
> + int ret;
> + u8 config[GOODIX_CONFIG_MAX_LENGTH];
> +
> + ret = goodix_i2c_read(ts->client, GOODIX_REG_CONFIG_DATA, config,
> + GOODIX_CONFIG_MAX_LENGTH);
> + if (ret < 0) {
> + GOODIX_ERROR("Error reading config, use default value!");
> + ts->abs_x_max = GOODIX_MAX_WIDTH;
> + ts->abs_y_max = GOODIX_MAX_HEIGHT;
> + ts->int_trigger_type = GOODIX_INT_TRIGGER;
> + return;
> + }
> +
> + ts->abs_x_max = get_unaligned_le16(&config[RESOLUTION_LOC]);
> + ts->abs_y_max = get_unaligned_le16(&config[RESOLUTION_LOC + 2]);
> + ts->int_trigger_type = (config[TRIGGER_LOC]) & 0x03;
> + if ((!ts->abs_x_max) || (!ts->abs_y_max)) {
> + GOODIX_ERROR("Invalid config, use default value!");
> + ts->abs_x_max = GOODIX_MAX_WIDTH;
> + ts->abs_y_max = GOODIX_MAX_HEIGHT;
> + }
> +}
> +
> +
> +/**
> + * goodix_read_version - Read goodix touchscreen version
> + *
> + * @client: the i2c client
> + * @version: output buffer containing the version on success
> + */
> +static int goodix_read_version(struct i2c_client *client, u16 *version)
> +{
> + int ret;
> + int i;
> + u8 buf[6];
> +
> + ret = goodix_i2c_read(client, GOODIX_REG_VERSION, buf, sizeof(buf));
> + if (ret < 0) {
> + GOODIX_ERROR("GTP read version failed");
> + return ret;
> + }
> +
> + if (version)
> + *version = get_unaligned_le16(&buf[4]);
> +
> + for (i = 0; i < 4; i++) {
> + if (!buf[i])
> + buf[i] = 0x30;
buf[i] = '0';
but what if they happen to have some other nonprintable garbage there?
> + }
> + GOODIX_INFO("IC VERSION: %c%c%c%c_%02x%02x",
> + buf[0], buf[1], buf[2], buf[3], buf[5], buf[4]);
> +
> + return ret;
> +}
> +
> +/**
> + * goodix_i2c_test - I2C test function to check if the device answers.
> + *
> + * @client: the i2c client
> + */
> +static int goodix_i2c_test(struct i2c_client *client)
> +{
> + u8 test;
> + int ret;
> + int retry = 0;
> +
> + while (retry++ < 2) {
> + ret = goodix_i2c_read(client, GOODIX_REG_CONFIG_DATA,
> + &test, 1);
> + if (ret > 0)
> + return ret;
> +
> + GOODIX_ERROR("GTP i2c test failed time %d.", retry);
> + msleep(20);
> + }
> + return ret;
> +}
> +
> +/**
> + * goodix_request_irq - Request the IRQ handler
> + *
> + * @ts: our goodix_ts_data pointer
> + *
> + * Must be called during probe
> + */
> +static int goodix_request_irq(struct goodix_ts_data *ts)
> +{
> + int ret;
> +
> + ret = devm_request_threaded_irq(&ts->client->dev,
> + ts->client->irq, NULL,
> + goodix_ts_irq_handler,
> + goodix_irq_flags[ts->int_trigger_type],
> + ts->client->name,
> + ts);
> + if (ret) {
> + GOODIX_ERROR("Request IRQ failed! ERRNO:%d.", ret);
> + return -1;
> + }
> +
> + disable_irq_nosync(ts->client->irq);
Why nosync? And why do you need to disable? Yo just need to request it
later, after you've done querying the device. And why a separate funcion
for basically one statement?
> + return 0;
> +}
> +
> +/**
> + * goodix_request_input_dev - Allocate, populate and register the input device
> + *
> + * @ts: our goodix_ts_data pointer
> + *
> + * Must be called during probe
> + */
> +static int goodix_request_input_dev(struct goodix_ts_data *ts)
> +{
> + int ret;
> +
> + ts->input_dev = devm_input_allocate_device(&ts->client->dev);
> + if (ts->input_dev == NULL) {
> + GOODIX_ERROR("Failed to allocate input device.");
> + return -ENOMEM;
> + }
> +
> + ts->input_dev->evbit[0] = BIT_MASK(EV_SYN) |
> + BIT_MASK(EV_KEY) |
> + BIT_MASK(EV_ABS);
> +
> + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, 0,
> + ts->abs_x_max, 0, 0);
> + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, 0,
> + ts->abs_y_max, 0, 0);
> + input_set_abs_params(ts->input_dev, ABS_MT_WIDTH_MAJOR, 0, 255, 0, 0);
> + input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
> +
> + input_mt_init_slots(ts->input_dev, GOODIX_MAX_TOUCH,
> + INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
> +
> + ts->input_dev->name = goodix_ts_name;
> + ts->input_dev->phys = "input/ts";
> + ts->input_dev->id.bustype = BUS_I2C;
> + ts->input_dev->id.vendor = 0x0416;
> + ts->input_dev->id.product = 0x1001;
> + ts->input_dev->id.version = 10427;
> +
> + ret = input_register_device(ts->input_dev);
> + if (ret) {
> + GOODIX_ERROR("Failed to register %s input device",
> + ts->input_dev->name);
> + return -ENODEV;
Why -ENODEV instead of ret?
> + }
> +
> + return 0;
> +}
> +
> +static int goodix_ts_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + int ret;
> + struct goodix_ts_data *ts;
> + u16 version_info;
> +
> + GOODIX_INFO("GTP I2C Address: 0x%02x", client->addr);
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> + GOODIX_ERROR("I2C check functionality failed.");
> + return -ENODEV;
> + }
> + ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
> + if (ts == NULL) {
if (!ts)
is a preferred form (by me at least).
> + GOODIX_ERROR("Alloc GFP_KERNEL memory failed.");
> + return -ENOMEM;
> + }
> +
> + ts->client = client;
> + i2c_set_clientdata(client, ts);
> +
> + ret = goodix_i2c_test(client);
> + if (ret < 0) {
> + client->addr = 0x5d;
Umm, why? Let's trust board code/i2c core to configure the i2c device
properly.
> + ret = goodix_i2c_test(client);
> + if (ret < 0) {
> + GOODIX_ERROR("I2C communication ERROR!");
> + return -ENODEV;
> + }
> + GOODIX_INFO("GTP I2C new Address: 0x%02x", client->addr);
> + }
> +
> + goodix_read_config(ts);
> +
> + ret = goodix_request_input_dev(ts);
> + if (ret < 0) {
> + GOODIX_ERROR("GTP request input dev failed");
> + return ret;
> + }
> +
> + ret = goodix_request_irq(ts);
> + if (ret < 0)
> + return ret;
> +
> + ret = goodix_read_version(client, &version_info);
> + if (ret < 0) {
> + GOODIX_ERROR("Read version failed.");
> + return ret;
> + }
> +
> + enable_irq(ts->client->irq);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id goodix_ts_id[] = {
> + { "GDIX1001:00", 0 },
> + { }
Do we have to have this table?
> +};
> +
> +static const struct acpi_device_id goodix_acpi_match[] = {
> + { "GDIX1001", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
> +
> +static struct i2c_driver goodix_ts_driver = {
> + .probe = goodix_ts_probe,
> + .id_table = goodix_ts_id,
> + .driver = {
> + .name = "Goodix-TS",
> + .owner = THIS_MODULE,
> + .acpi_match_table = goodix_acpi_match,
> + },
> +};
> +
> +module_i2c_driver(goodix_ts_driver);
> +
> +MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
> +MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
> +MODULE_DESCRIPTION("Goodix touchscreen driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.1.0
>
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: HID: wacom: regression - system freezes when resuming from S3 suspend
From: Jonas Jelten @ 2014-09-24 1:31 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina; +Cc: linux-input
In-Reply-To: <CAN+gG=EfKHqo2++ejxJMqDxpOnav5qiVRJMRjXoDOrbWQVMnBQ@mail.gmail.com>
On 2014-09-22 17:52, Benjamin Tissoires wrote:
> On Mon, Sep 22, 2014 at 11:01 AM, Jonas Jelten <jelten@in.tum.de> wrote:
>> Hi!
>>
>> I encountered that my system freezes when resuming my Lenovo X220t with
>> integrated Wacom ISDv4 tablet from S3 suspend.
>>
>> Currently running 3.17.0-rc6, I started to bisect and found that this
>> commit causes the issue:
>>
>> commit 29b4739134c73a2873adec93346f09bb76d6a794
>> Author: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> Date: Thu Jul 24 12:52:23 2014 -0700
>>
>> Input: wacom - switch from an USB driver to a HID driver
>>
>>
>> Running 3.17.0-rc6, right after boot, this shows up in the log:
>>
>> [ 25.843484] wacom 0003:056A:00E6.0001: usb_submit_urb(ctrl) failed: -1
>> [ 25.843531] wacom 0003:056A:00E6.0001: timeout initializing reports
>> [ 25.843886] wacom 0003:056A:00E6.0001: hidraw0: USB HID v1.11 Device
>> [Tablet ISD-V4] on usb-0000:00:1d.0-1.5/input0
>> [ 25.846036] input: Wacom ISDv4 E6 Finger as
>> /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.5/2-1.5:1.1/0003:056A:00E6.0002/input/input18
>>
>> When suspending, the screen goes off and about 8 seconds later, the
>> machine actually suspends.
>>
>> Right after the (instant) resume, I can see the frame buffer I had when
>> suspending (X screen) for about 10 seconds. During that time, no sysrq
>> works. After this short period, the tty1-framebuffer appears, with
>> messed up linebreaks (no carriage returns occur any more), and the
>> capslock-led starts blinking. Sysrq keys still don't work. The system is
>> completely frozen.
>>
>> Running the kernel from the bisected first good commit, the suspend is
>> faster (under 1 second) and the resume does not crash.
>>
>> I just saw the "HID: wacom: fix timeout on probe for some wacoms" mail,
>> which probably has to do something with my problem.
>>
>> Any ideas?
>
> Yes, this commit will fix your problem. It has just been delayed for
> 3.17.1 as mentioned by Jiri today.
>
Are you sure it's a good idea to let the kernel crash into 3.17.0 and
fix it later in stable? I suspect many X220t users could be affected.
Cheers,
Jonas
^ permalink raw reply
* Re: [PATCH v2 1/2] cap1106: Add support for various cap11xx devices
From: Matt Ranostay @ 2014-09-24 3:28 UTC (permalink / raw)
To: Daniel Mack
Cc: Kumar Gala, Dmitry Torokhov, Daniel Mack, linux-input,
linux-kernel@vger.kernel.org, Rob Herring,
devicetree@vger.kernel.org
In-Reply-To: <5421174A.7060101@zonque.org>
On Mon, Sep 22, 2014 at 11:46 PM, Daniel Mack <daniel@zonque.org> wrote:
> On 09/23/2014 06:04 AM, Matt Ranostay wrote:
>> diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
>> index d70b65a..07f9e88 100644
>> --- a/drivers/input/keyboard/cap1106.c
>> +++ b/drivers/input/keyboard/cap1106.c
>> @@ -55,8 +55,6 @@
>> #define CAP1106_REG_MANUFACTURER_ID 0xfe
>> #define CAP1106_REG_REVISION 0xff
>>
>> -#define CAP1106_NUM_CHN 6
>> -#define CAP1106_PRODUCT_ID 0x55
>> #define CAP1106_MANUFACTURER_ID 0x5d
>>
>> struct cap1106_priv {
>> @@ -64,7 +62,25 @@ struct cap1106_priv {
>> struct input_dev *idev;
>>
>> /* config */
>> - unsigned short keycodes[CAP1106_NUM_CHN];
>> + u32 *keycodes;
>> + unsigned int num_channels;
>> +};
>> +
>> +struct cap11xx_hw_model {
>> + uint8_t product_id;
>> + unsigned int num_channels;
>> +};
>> +
>> +enum {
>> + CAP1106,
>> + CAP1126,
>> + CAP1188,
>> +};
>> +
>> +struct cap11xx_hw_model cap11xx_devices[] = {
>
> This can be static const.
>
D'oh how did I miss that?
> And, as I said, there should be a 3rd patch that renames the file and
> its functions, structs, defines etc to the more generic "11xx" variants.
>
Ok will do and resubmit.
>
> Thanks,
> Daniel
>
^ permalink raw reply
* Re: How to indicate hover touch when exact distance unknown?
From: Peter Hutterer @ 2014-09-24 4:52 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Andrew de los Reyes, hdegoede, rydberg, jikos,
David Herrmann, Linux Input
In-Reply-To: <20140923162811.GB40700@core.coreip.homeip.net>
On Tue, Sep 23, 2014 at 09:28:11AM -0700, Dmitry Torokhov wrote:
> Hi Benjamin,
>
> On Tue, Sep 23, 2014 at 10:59:16AM -0400, Benjamin Tissoires wrote:
> > Hi Andrew,
> >
> > On Sep 23 2014 or thereabouts, Andrew de los Reyes wrote:
> > > Hi folks,
> > >
> > > More and more we're seeing touch input devices that support detection of
> > > fingers that are above the surface and not touching. We were thinking of
> > > writing a kernel patch for one such device. Normally we would indicate
> > > distance with ABS_MT_DISTANCE, but we ran into the problem that sometimes
> > > the hardware will not be able to report a distance (or will stop reporting
> > > a distance when it's too great a number), and we had the (bad) idea to
> > > simply report a value or 1 or 255 or something like that when distance is
> > > unknown.
> >
> > FWIW, hid-multitouch already support those devices.
> > ABS_MT_DISTANCE is set with a min/max of 0/1 when we detect win8
> > certified panels with hovering capability. By default the spec does not
> > provide the distance IIRC, and you only have one byte: InRange.
>
> Hmm, I missed that and this is unfortunate. The ABS capabilities
> advertised by the devices should match their real capabilities. If
> device can't properly report distance it should not be using
> ABS_MT_DISTANCE/ABS_DISTANCE...
I think the hid-mt behaviour makes sense. Without explicit resolution (and
no device sets that anyway, IIRC) any distance value > min tells us only that a
tool is within detectable range, but not yet touching. Anything between
min/max is only useful as a relative scale, but effectively that [min,max]
range could be a metre or a millimeter, we can't know. So a device with a
0/1 range simply has low granularity and is only able to detect whether
something is within range or touching the surface.
Cheers,
Peter
>
> >
> > I think Xorg can deal with that (the touch emulation discards the
> > ABS_MT_DISTANCE, but the xorg-evdev driver should be smart enough to not
> > take BTN_TOUCH into account).
> >
> > >
> > > Dmitry warned that a similar thing happened with PRESSURE a while ago, and
> > > it was a mess as different drivers did different things.
> >
> > As I said, IIRC, xorg-evdev is fine with that. For generic desktop, we
> > have to make sure libinput is aware, and then you only have to handle
> > your own xorg chromebook driver. This is not something which scares me
> > that much, especially given that this is what hid-multitouch reports
> > since the v3.9 kernel.
> > libinput is based on a per slot device information, so it is really easy
> > to implement if it is not already in place. xorg-Synaptics and xorg-Wacom
> > should not be ported to wayland from what I understood, so the mess with
> > several implementations can be solved easily.
> >
> > >
> > > We are now wondering if we should come up with a standard way to indicate
> > > hover with or without distance (or with distance being optional). Dmitry
> > > had the idea for a new tool type, HOVER; FINGER would be for touches that
> > > are actually touching.
> >
> > The tool is still the finger.
>
> Do we reliably know that it is a finger and not a pen or eraser or some
> other tool? If we do then obviously introducing new tool will not fly.
>
> > So I am not very happy with having a new
> > tool. In the end, it may also disturb older clients which will not know
> > what to do with HOVER.
> > And the ABS_MT_DISTANCE approach used to be fully retro-compatible
> > (assuming that the hovering distance is small enough for the user not to
> > detect it).
>
> Except that if we start getting devices that can actually tell the
> distance we'd need special casing 0/1 handling, no?
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: How to indicate hover touch when exact distance unknown?
From: Henrik Rydberg @ 2014-09-24 5:28 UTC (permalink / raw)
To: Peter Hutterer, Dmitry Torokhov
Cc: Benjamin Tissoires, Andrew de los Reyes, hdegoede, jikos,
David Herrmann, Linux Input
In-Reply-To: <20140924045213.GA4194@jelly.redhat.com>
>>> FWIW, hid-multitouch already support those devices.
>>> ABS_MT_DISTANCE is set with a min/max of 0/1 when we detect win8
>>> certified panels with hovering capability. By default the spec does not
>>> provide the distance IIRC, and you only have one byte: InRange.
>>
>> Hmm, I missed that and this is unfortunate. The ABS capabilities
>> advertised by the devices should match their real capabilities. If
>> device can't properly report distance it should not be using
>> ABS_MT_DISTANCE/ABS_DISTANCE...
>
> I think the hid-mt behaviour makes sense. Without explicit resolution (and
> no device sets that anyway, IIRC) any distance value > min tells us only that a
> tool is within detectable range, but not yet touching. Anything between
> min/max is only useful as a relative scale, but effectively that [min,max]
> range could be a metre or a millimeter, we can't know. So a device with a
> 0/1 range simply has low granularity and is only able to detect whether
> something is within range or touching the surface.
I agree with this, but I also share Dmitry's concern.
A device that can detect hovering, if only binary, does in fact coarsely
estimate the distance from the touching surface. A device allowing for a smooth
approach of objects would simply support a better resolution. From that
perspective, using the ABS_MT_DISTANCE capability makes sense. Pragmatically.
However, at no point are we really changing the coordinate system, which remains
euclidian space. We are simply changing resolution and thresholds for what
constitues a touch. Forcing userland to step away from the simple interpretation
is what eventually makes the capability impossible to use as intended.
So, if we cannot express, using the abs_info data, something like "contains a
detector which can coarsely estimate the distance and then uses a detector
threshold to set that distance to zero or one", we had better express it in some
other way, which is less ambiguous.
How about ABS_MT_PRESENT and/or ABS_PRESENT? It would complement TOUCH in the
case of hovering, allow the state where the tool is there but not touching, and
would unambiguously advertise the capability of detecting presence. It would
also be forward compatible with additional capabilities, such as reporting the
actual distance to the surface.
Henrik
^ permalink raw reply
* RE: [PATCH v4 1/14] input: cyapa: re-architecture driver to support multi-trackpads in one driver
From: Dudley Du @ 2014-09-24 7:08 UTC (permalink / raw)
To: 'Dmitry Torokhov'
Cc: Rafael J. Wysocki, Benson Leung, Patrik Fimml, linux-input,
linux-kernel
In-Reply-To: <20140924001839.GA16747@core.coreip.homeip.net>
Hi Dmitry,
Thanks for your careful consideration.
I added my idea and comments below.
Thanks,
Dudley
> -----Original Message-----
> From: linux-input-owner@vger.kernel.org [mailto:linux-input-
> owner@vger.kernel.org] On Behalf Of Dmitry Torokhov
> Sent: Wednesday, September 24, 2014 8:19 AM
> To: Dudley Du
> Cc: Rafael J. Wysocki; Benson Leung; Patrik Fimml; linux-input@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 1/14] input: cyapa: re-architecture driver to support
> multi-trackpads in one driver
>
> Hi Dudley,
>
> On Fri, Aug 22, 2014 at 04:41:07PM +0800, Dudley Du wrote:
> > Dmitry,
> >
> > Thanks for your feedback.
> >
> > Thanks,
> > Dudley
> >
> > > -----Original Message-----
> > > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > > Sent: Friday, August 22, 2014 2:21 AM
> > > To: Dudley Du
> > > Cc: Rafael J. Wysocki; Benson Leung; Patrik Fimml; linux-
> input@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; Dudley Du
> > > Subject: Re: [PATCH v4 1/14] input: cyapa: re-architecture driver to
> support
> > > multi-trackpads in one driver
> > >
> > > On Thu, Jul 17, 2014 at 02:47:24PM +0800, Dudley Du wrote:
> > > > In order to support two different communication protocol based trackpad
> > > > device in one cyapa, the new cyapa driver is re-designed with
> > > > one cyapa driver core and two devices' functions component.
> > > > The cyapa driver core is contained in this patch, it supplies the basic
> > > > function with input and kernel system and also defined the interfaces
> > > > that the devices' functions component needs to apply and support.
> > > > Also, in order to speed up the system boot time, the device states
> > > > detecting and probing process is put into the async thread.
> > > > TEST=test on Chromebooks.
> > > >
> > > > Signed-off-by: Dudley Du <dudl@cypress.com>
> > > > ---
> > > > drivers/input/mouse/Makefile | 4 +-
> > > > drivers/input/mouse/cyapa.c | 1083 ++++++++++++++---------------------
> ----
> > > ---
> > > > drivers/input/mouse/cyapa.h | 275 +++++++++++
> > > > 3 files changed, 646 insertions(+), 716 deletions(-)
> > > > create mode 100644 drivers/input/mouse/cyapa.h
> > > >
> > > > diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
> > > > index c25efdb..8608eb7 100644
> > > > --- a/drivers/input/mouse/Makefile
> > > > +++ b/drivers/input/mouse/Makefile
> > > > @@ -8,7 +8,7 @@ obj-$(CONFIG_MOUSE_AMIGA) += amimouse.o
> > > > obj-$(CONFIG_MOUSE_APPLETOUCH) += appletouch.o
> > > > obj-$(CONFIG_MOUSE_ATARI) += atarimouse.o
> > > > obj-$(CONFIG_MOUSE_BCM5974) += bcm5974.o
> > > > -obj-$(CONFIG_MOUSE_CYAPA) += cyapa.o
> > > > +obj-$(CONFIG_MOUSE_CYAPA) += cyapatp.o
> > > > obj-$(CONFIG_MOUSE_GPIO) += gpio_mouse.o
> > > > obj-$(CONFIG_MOUSE_INPORT) += inport.o
> > > > obj-$(CONFIG_MOUSE_LOGIBM) += logibm.o
> > > > @@ -34,3 +34,5 @@ psmouse-$(CONFIG_MOUSE_PS2_SENTELIC) +=
> sentelic.o
> > > > psmouse-$(CONFIG_MOUSE_PS2_TRACKPOINT) += trackpoint.o
> > > > psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT) += touchkit_ps2.o
> > > > psmouse-$(CONFIG_MOUSE_PS2_CYPRESS) += cypress_ps2.o
> > > > +
> > > > +cyapatp-y := cyapa.o
> > > > diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c
> > > > index b409c3d..5fc8dbe 100644
> > > > --- a/drivers/input/mouse/cyapa.c
> > > > +++ b/drivers/input/mouse/cyapa.c
> > > > @@ -6,7 +6,7 @@
> > > > * Daniel Kurtz <djkurtz@chromium.org>
> > > > * Benson Leung <bleung@chromium.org>
> > > > *
> > > > - * Copyright (C) 2011-2012 Cypress Semiconductor, Inc.
> > > > + * Copyright (C) 2011-2014 Cypress Semiconductor, Inc.
> > > > * Copyright (C) 2011-2012 Google, Inc.
> > > > *
> > > > * This file is subject to the terms and conditions of the GNU General
> > > Public
> > > > @@ -14,731 +14,114 @@
> > > > * more details.
> > > > */
> > > >
> > > > +#include <linux/debugfs.h>
> > > > #include <linux/delay.h>
> > > > #include <linux/i2c.h>
> > > > #include <linux/input.h>
> > > > #include <linux/input/mt.h>
> > > > #include <linux/interrupt.h>
> > > > #include <linux/module.h>
> > > > +#include <linux/mutex.h>
> > > > #include <linux/slab.h>
> > > > +#include <linux/uaccess.h>
> > > > +#include <linux/pm_runtime.h>
> > > > +#include "cyapa.h"
> > > >
> > > > -/* APA trackpad firmware generation */
> > > > -#define CYAPA_GEN3 0x03 /* support MT-protocol B with tracking ID.
> */
> > > > -
> > > > -#define CYAPA_NAME "Cypress APA Trackpad (cyapa)"
> > > > -
> > > > -/* commands for read/write registers of Cypress trackpad */
> > > > -#define CYAPA_CMD_SOFT_RESET 0x00
> > > > -#define CYAPA_CMD_POWER_MODE 0x01
> > > > -#define CYAPA_CMD_DEV_STATUS 0x02
> > > > -#define CYAPA_CMD_GROUP_DATA 0x03
> > > > -#define CYAPA_CMD_GROUP_CMD 0x04
> > > > -#define CYAPA_CMD_GROUP_QUERY 0x05
> > > > -#define CYAPA_CMD_BL_STATUS 0x06
> > > > -#define CYAPA_CMD_BL_HEAD 0x07
> > > > -#define CYAPA_CMD_BL_CMD 0x08
> > > > -#define CYAPA_CMD_BL_DATA 0x09
> > > > -#define CYAPA_CMD_BL_ALL 0x0a
> > > > -#define CYAPA_CMD_BLK_PRODUCT_ID 0x0b
> > > > -#define CYAPA_CMD_BLK_HEAD 0x0c
> > > > -
> > > > -/* report data start reg offset address. */
> > > > -#define DATA_REG_START_OFFSET 0x0000
> > > > -
> > > > -#define BL_HEAD_OFFSET 0x00
> > > > -#define BL_DATA_OFFSET 0x10
> > > > -
> > > > -/*
> > > > - * Operational Device Status Register
> > > > - *
> > > > - * bit 7: Valid interrupt source
> > > > - * bit 6 - 4: Reserved
> > > > - * bit 3 - 2: Power status
> > > > - * bit 1 - 0: Device status
> > > > - */
> > > > -#define REG_OP_STATUS 0x00
> > > > -#define OP_STATUS_SRC 0x80
> > > > -#define OP_STATUS_POWER 0x0c
> > > > -#define OP_STATUS_DEV 0x03
> > > > -#define OP_STATUS_MASK (OP_STATUS_SRC | OP_STATUS_POWER | OP_STATUS_DEV)
> > > > -
> > > > -/*
> > > > - * Operational Finger Count/Button Flags Register
> > > > - *
> > > > - * bit 7 - 4: Number of touched finger
> > > > - * bit 3: Valid data
> > > > - * bit 2: Middle Physical Button
> > > > - * bit 1: Right Physical Button
> > > > - * bit 0: Left physical Button
> > > > - */
> > > > -#define REG_OP_DATA1 0x01
> > > > -#define OP_DATA_VALID 0x08
> > > > -#define OP_DATA_MIDDLE_BTN 0x04
> > > > -#define OP_DATA_RIGHT_BTN 0x02
> > > > -#define OP_DATA_LEFT_BTN 0x01
> > > > -#define OP_DATA_BTN_MASK (OP_DATA_MIDDLE_BTN | OP_DATA_RIGHT_BTN | \
> > > > - OP_DATA_LEFT_BTN)
> > > > -
> > > > -/*
> > > > - * Bootloader Status Register
> > > > - *
> > > > - * bit 7: Busy
> > > > - * bit 6 - 5: Reserved
> > > > - * bit 4: Bootloader running
> > > > - * bit 3 - 1: Reserved
> > > > - * bit 0: Checksum valid
> > > > - */
> > > > -#define REG_BL_STATUS 0x01
> > > > -#define BL_STATUS_BUSY 0x80
> > > > -#define BL_STATUS_RUNNING 0x10
> > > > -#define BL_STATUS_DATA_VALID 0x08
> > > > -#define BL_STATUS_CSUM_VALID 0x01
> > > > -
> > > > -/*
> > > > - * Bootloader Error Register
> > > > - *
> > > > - * bit 7: Invalid
> > > > - * bit 6: Invalid security key
> > > > - * bit 5: Bootloading
> > > > - * bit 4: Command checksum
> > > > - * bit 3: Flash protection error
> > > > - * bit 2: Flash checksum error
> > > > - * bit 1 - 0: Reserved
> > > > - */
> > > > -#define REG_BL_ERROR 0x02
> > > > -#define BL_ERROR_INVALID 0x80
> > > > -#define BL_ERROR_INVALID_KEY 0x40
> > > > -#define BL_ERROR_BOOTLOADING 0x20
> > > > -#define BL_ERROR_CMD_CSUM 0x10
> > > > -#define BL_ERROR_FLASH_PROT 0x08
> > > > -#define BL_ERROR_FLASH_CSUM 0x04
> > > > -
> > > > -#define BL_STATUS_SIZE 3 /* length of bootloader status registers */
> > > > -#define BLK_HEAD_BYTES 32
> > > > -
> > > > -#define PRODUCT_ID_SIZE 16
> > > > -#define QUERY_DATA_SIZE 27
> > > > -#define REG_PROTOCOL_GEN_QUERY_OFFSET 20
> > > > -
> > > > -#define REG_OFFSET_DATA_BASE 0x0000
> > > > -#define REG_OFFSET_COMMAND_BASE 0x0028
> > > > -#define REG_OFFSET_QUERY_BASE 0x002a
> > > > -
> > > > -#define CAPABILITY_LEFT_BTN_MASK (0x01 << 3)
> > > > -#define CAPABILITY_RIGHT_BTN_MASK (0x01 << 4)
> > > > -#define CAPABILITY_MIDDLE_BTN_MASK (0x01 << 5)
> > > > -#define CAPABILITY_BTN_MASK (CAPABILITY_LEFT_BTN_MASK | \
> > > > - CAPABILITY_RIGHT_BTN_MASK | \
> > > > - CAPABILITY_MIDDLE_BTN_MASK)
> > > > -
> > > > -#define CYAPA_OFFSET_SOFT_RESET REG_OFFSET_COMMAND_BASE
> > > > -
> > > > -#define REG_OFFSET_POWER_MODE (REG_OFFSET_COMMAND_BASE + 1)
> > > > -
> > > > -#define PWR_MODE_MASK 0xfc
> > > > -#define PWR_MODE_FULL_ACTIVE (0x3f << 2)
> > > > -#define PWR_MODE_IDLE (0x05 << 2) /* default sleep time is 50 ms.
> */
> > > > -#define PWR_MODE_OFF (0x00 << 2)
> > > > -
> > > > -#define PWR_STATUS_MASK 0x0c
> > > > -#define PWR_STATUS_ACTIVE (0x03 << 2)
> > > > -#define PWR_STATUS_IDLE (0x02 << 2)
> > > > -#define PWR_STATUS_OFF (0x00 << 2)
> > > > -
> > > > -/*
> > > > - * CYAPA trackpad device states.
> > > > - * Used in register 0x00, bit1-0, DeviceStatus field.
> > > > - * Other values indicate device is in an abnormal state and must be
> reset.
> > > > - */
> > > > -#define CYAPA_DEV_NORMAL 0x03
> > > > -#define CYAPA_DEV_BUSY 0x01
> > > > -
> > > > -enum cyapa_state {
> > > > - CYAPA_STATE_OP,
> > > > - CYAPA_STATE_BL_IDLE,
> > > > - CYAPA_STATE_BL_ACTIVE,
> > > > - CYAPA_STATE_BL_BUSY,
> > > > - CYAPA_STATE_NO_DEVICE,
> > > > -};
> > > > -
> > > > -
> > > > -struct cyapa_touch {
> > > > - /*
> > > > - * high bits or x/y position value
> > > > - * bit 7 - 4: high 4 bits of x position value
> > > > - * bit 3 - 0: high 4 bits of y position value
> > > > - */
> > > > - u8 xy_hi;
> > > > - u8 x_lo; /* low 8 bits of x position value. */
> > > > - u8 y_lo; /* low 8 bits of y position value. */
> > > > - u8 pressure;
> > > > - /* id range is 1 - 15. It is incremented with every new touch.
> */
> > > > - u8 id;
> > > > -} __packed;
> > > > -
> > > > -/* The touch.id is used as the MT slot id, thus max MT slot is 15 */
> > > > -#define CYAPA_MAX_MT_SLOTS 15
> > > > -
> > > > -struct cyapa_reg_data {
> > > > - /*
> > > > - * bit 0 - 1: device status
> > > > - * bit 3 - 2: power mode
> > > > - * bit 6 - 4: reserved
> > > > - * bit 7: interrupt valid bit
> > > > - */
> > > > - u8 device_status;
> > > > - /*
> > > > - * bit 7 - 4: number of fingers currently touching pad
> > > > - * bit 3: valid data check bit
> > > > - * bit 2: middle mechanism button state if exists
> > > > - * bit 1: right mechanism button state if exists
> > > > - * bit 0: left mechanism button state if exists
> > > > - */
> > > > - u8 finger_btn;
> > > > - /* CYAPA reports up to 5 touches per packet. */
> > > > - struct cyapa_touch touches[5];
> > > > -} __packed;
> > > > -
> > > > -/* The main device structure */
> > > > -struct cyapa {
> > > > - enum cyapa_state state;
> > > > -
> > > > - struct i2c_client *client;
> > > > - struct input_dev *input;
> > > > - char phys[32]; /* device physical location */
> > > > - int irq;
> > > > - bool irq_wake; /* irq wake is enabled */
> > > > - bool smbus;
> > > > -
> > > > - /* read from query data region. */
> > > > - char product_id[16];
> > > > - u8 btn_capability;
> > > > - u8 gen;
> > > > - int max_abs_x;
> > > > - int max_abs_y;
> > > > - int physical_size_x;
> > > > - int physical_size_y;
> > > > -};
> > > > -
> > > > -static const u8 bl_deactivate[] = { 0x00, 0xff, 0x3b, 0x00, 0x01, 0x02,
> > > 0x03,
> > > > - 0x04, 0x05, 0x06, 0x07 };
> > > > -static const u8 bl_exit[] = { 0x00, 0xff, 0xa5, 0x00, 0x01, 0x02, 0x03,
> > > 0x04,
> > > > - 0x05, 0x06, 0x07 };
> > > > -
> > > > -struct cyapa_cmd_len {
> > > > - u8 cmd;
> > > > - u8 len;
> > > > -};
> > > >
> > > > #define CYAPA_ADAPTER_FUNC_NONE 0
> > > > #define CYAPA_ADAPTER_FUNC_I2C 1
> > > > #define CYAPA_ADAPTER_FUNC_SMBUS 2
> > > > #define CYAPA_ADAPTER_FUNC_BOTH 3
> > > >
> > > > -/*
> > > > - * macros for SMBus communication
> > > > - */
> > > > -#define SMBUS_READ 0x01
> > > > -#define SMBUS_WRITE 0x00
> > > > -#define SMBUS_ENCODE_IDX(cmd, idx) ((cmd) | (((idx) & 0x03) << 1))
> > > > -#define SMBUS_ENCODE_RW(cmd, rw) ((cmd) | ((rw) & 0x01))
> > > > -#define SMBUS_BYTE_BLOCK_CMD_MASK 0x80
> > > > -#define SMBUS_GROUP_BLOCK_CMD_MASK 0x40
> > > > -
> > > > - /* for byte read/write command */
> > > > -#define CMD_RESET 0
> > > > -#define CMD_POWER_MODE 1
> > > > -#define CMD_DEV_STATUS 2
> > > > -#define SMBUS_BYTE_CMD(cmd) (((cmd) & 0x3f) << 1)
> > > > -#define CYAPA_SMBUS_RESET SMBUS_BYTE_CMD(CMD_RESET)
> > > > -#define CYAPA_SMBUS_POWER_MODE SMBUS_BYTE_CMD(CMD_POWER_MODE)
> > > > -#define CYAPA_SMBUS_DEV_STATUS SMBUS_BYTE_CMD(CMD_DEV_STATUS)
> > > > -
> > > > - /* for group registers read/write command */
> > > > -#define REG_GROUP_DATA 0
> > > > -#define REG_GROUP_CMD 2
> > > > -#define REG_GROUP_QUERY 3
> > > > -#define SMBUS_GROUP_CMD(grp) (0x80 | (((grp) & 0x07) << 3))
> > > > -#define CYAPA_SMBUS_GROUP_DATA SMBUS_GROUP_CMD(REG_GROUP_DATA)
> > > > -#define CYAPA_SMBUS_GROUP_CMD SMBUS_GROUP_CMD(REG_GROUP_CMD)
> > > > -#define CYAPA_SMBUS_GROUP_QUERY SMBUS_GROUP_CMD(REG_GROUP_QUERY)
> > > > -
> > > > - /* for register block read/write command */
> > > > -#define CMD_BL_STATUS 0
> > > > -#define CMD_BL_HEAD 1
> > > > -#define CMD_BL_CMD 2
> > > > -#define CMD_BL_DATA 3
> > > > -#define CMD_BL_ALL 4
> > > > -#define CMD_BLK_PRODUCT_ID 5
> > > > -#define CMD_BLK_HEAD 6
> > > > -#define SMBUS_BLOCK_CMD(cmd) (0xc0 | (((cmd) & 0x1f) << 1))
> > > > -
> > > > -/* register block read/write command in bootloader mode */
> > > > -#define CYAPA_SMBUS_BL_STATUS SMBUS_BLOCK_CMD(CMD_BL_STATUS)
> > > > -#define CYAPA_SMBUS_BL_HEAD SMBUS_BLOCK_CMD(CMD_BL_HEAD)
> > > > -#define CYAPA_SMBUS_BL_CMD SMBUS_BLOCK_CMD(CMD_BL_CMD)
> > > > -#define CYAPA_SMBUS_BL_DATA SMBUS_BLOCK_CMD(CMD_BL_DATA)
> > > > -#define CYAPA_SMBUS_BL_ALL SMBUS_BLOCK_CMD(CMD_BL_ALL)
> > > > -
> > > > -/* register block read/write command in operational mode */
> > > > -#define CYAPA_SMBUS_BLK_PRODUCT_ID SMBUS_BLOCK_CMD(CMD_BLK_PRODUCT_ID)
> > > > -#define CYAPA_SMBUS_BLK_HEAD SMBUS_BLOCK_CMD(CMD_BLK_HEAD)
> > > > -
> > > > -static const struct cyapa_cmd_len cyapa_i2c_cmds[] = {
> > > > - { CYAPA_OFFSET_SOFT_RESET, 1 },
> > > > - { REG_OFFSET_COMMAND_BASE + 1, 1 },
> > > > - { REG_OFFSET_DATA_BASE, 1 },
> > > > - { REG_OFFSET_DATA_BASE, sizeof(struct cyapa_reg_data) },
> > > > - { REG_OFFSET_COMMAND_BASE, 0 },
> > > > - { REG_OFFSET_QUERY_BASE, QUERY_DATA_SIZE },
> > > > - { BL_HEAD_OFFSET, 3 },
> > > > - { BL_HEAD_OFFSET, 16 },
> > > > - { BL_HEAD_OFFSET, 16 },
> > > > - { BL_DATA_OFFSET, 16 },
> > > > - { BL_HEAD_OFFSET, 32 },
> > > > - { REG_OFFSET_QUERY_BASE, PRODUCT_ID_SIZE },
> > > > - { REG_OFFSET_DATA_BASE, 32 }
> > > > -};
> > > > +#define CYAPA_DEBUGFS_READ_FW "read_fw"
> > > > +#define CYAPA_DEBUGFS_RAW_DATA "raw_data"
> > > > +#define CYAPA_FW_NAME "cyapa.bin"
> > > > +
> > > > +const char unique_str[] = "CYTRA";
> > > > +
> > > >
> > > > -static const struct cyapa_cmd_len cyapa_smbus_cmds[] = {
> > > > - { CYAPA_SMBUS_RESET, 1 },
> > > > - { CYAPA_SMBUS_POWER_MODE, 1 },
> > > > - { CYAPA_SMBUS_DEV_STATUS, 1 },
> > > > - { CYAPA_SMBUS_GROUP_DATA, sizeof(struct cyapa_reg_data) },
> > > > - { CYAPA_SMBUS_GROUP_CMD, 2 },
> > > > - { CYAPA_SMBUS_GROUP_QUERY, QUERY_DATA_SIZE },
> > > > - { CYAPA_SMBUS_BL_STATUS, 3 },
> > > > - { CYAPA_SMBUS_BL_HEAD, 16 },
> > > > - { CYAPA_SMBUS_BL_CMD, 16 },
> > > > - { CYAPA_SMBUS_BL_DATA, 16 },
> > > > - { CYAPA_SMBUS_BL_ALL, 32 },
> > > > - { CYAPA_SMBUS_BLK_PRODUCT_ID, PRODUCT_ID_SIZE },
> > > > - { CYAPA_SMBUS_BLK_HEAD, 16 },
> > > > -};
> > > >
> > > > -static ssize_t cyapa_i2c_reg_read_block(struct cyapa *cyapa, u8 reg,
> size_t
> > > len,
> > > > +ssize_t cyapa_i2c_reg_read_block(struct cyapa *cyapa, u8 reg, size_t
> len,
> > > > u8 *values)
> > > > {
> > > > return i2c_smbus_read_i2c_block_data(cyapa->client, reg, len,
> values);
> > > > }
> > > >
> > > > -static ssize_t cyapa_i2c_reg_write_block(struct cyapa *cyapa, u8 reg,
> > > > +ssize_t cyapa_i2c_reg_write_block(struct cyapa *cyapa, u8 reg,
> > > > size_t len, const u8 *values)
> > > > {
> > > > return i2c_smbus_write_i2c_block_data(cyapa->client, reg, len,
> values);
> > > > }
> > > >
> > > > -/*
> > > > - * cyapa_smbus_read_block - perform smbus block read command
> > > > - * @cyapa - private data structure of the driver
> > > > - * @cmd - the properly encoded smbus command
> > > > - * @len - expected length of smbus command result
> > > > - * @values - buffer to store smbus command result
> > > > - *
> > > > - * Returns negative errno, else the number of bytes written.
> > > > - *
> > > > - * Note:
> > > > - * In trackpad device, the memory block allocated for I2C register map
> > > > - * is 256 bytes, so the max read block for I2C bus is 256 bytes.
> > > > - */
> > > > -static ssize_t cyapa_smbus_read_block(struct cyapa *cyapa, u8 cmd,
> size_t
> > > len,
> > > > - u8 *values)
> > > > -{
> > > > - ssize_t ret;
> > > > - u8 index;
> > > > - u8 smbus_cmd;
> > > > - u8 *buf;
> > > > - struct i2c_client *client = cyapa->client;
> > > > -
> > > > - if (!(SMBUS_BYTE_BLOCK_CMD_MASK & cmd))
> > > > - return -EINVAL;
> > > > -
> > > > - if (SMBUS_GROUP_BLOCK_CMD_MASK & cmd) {
> > > > - /* read specific block registers command. */
> > > > - smbus_cmd = SMBUS_ENCODE_RW(cmd, SMBUS_READ);
> > > > - ret = i2c_smbus_read_block_data(client, smbus_cmd, values);
> > > > - goto out;
> > > > - }
> > > > -
> > > > - ret = 0;
> > > > - for (index = 0; index * I2C_SMBUS_BLOCK_MAX < len; index++) {
> > > > - smbus_cmd = SMBUS_ENCODE_IDX(cmd, index);
> > > > - smbus_cmd = SMBUS_ENCODE_RW(smbus_cmd, SMBUS_READ);
> > > > - buf = values + I2C_SMBUS_BLOCK_MAX * index;
> > > > - ret = i2c_smbus_read_block_data(client, smbus_cmd, buf);
> > > > - if (ret < 0)
> > > > - goto out;
> > > > - }
> > > > -
> > > > -out:
> > > > - return ret > 0 ? len : ret;
> > > > -}
> > > > -
> > > > -static s32 cyapa_read_byte(struct cyapa *cyapa, u8 cmd_idx)
> > > > -{
> > > > - u8 cmd;
> > > > -
> > > > - if (cyapa->smbus) {
> > > > - cmd = cyapa_smbus_cmds[cmd_idx].cmd;
> > > > - cmd = SMBUS_ENCODE_RW(cmd, SMBUS_READ);
> > > > - } else {
> > > > - cmd = cyapa_i2c_cmds[cmd_idx].cmd;
> > > > - }
> > > > - return i2c_smbus_read_byte_data(cyapa->client, cmd);
> > > > -}
> > > > -
> > > > -static s32 cyapa_write_byte(struct cyapa *cyapa, u8 cmd_idx, u8 value)
> > > > -{
> > > > - u8 cmd;
> > > > -
> > > > - if (cyapa->smbus) {
> > > > - cmd = cyapa_smbus_cmds[cmd_idx].cmd;
> > > > - cmd = SMBUS_ENCODE_RW(cmd, SMBUS_WRITE);
> > > > - } else {
> > > > - cmd = cyapa_i2c_cmds[cmd_idx].cmd;
> > > > - }
> > > > - return i2c_smbus_write_byte_data(cyapa->client, cmd, value);
> > > > -}
> > > > -
> > > > -static ssize_t cyapa_read_block(struct cyapa *cyapa, u8 cmd_idx, u8
> *values)
> > > > -{
> > > > - u8 cmd;
> > > > - size_t len;
> > > > -
> > > > - if (cyapa->smbus) {
> > > > - cmd = cyapa_smbus_cmds[cmd_idx].cmd;
> > > > - len = cyapa_smbus_cmds[cmd_idx].len;
> > > > - return cyapa_smbus_read_block(cyapa, cmd, len, values);
> > > > - } else {
> > > > - cmd = cyapa_i2c_cmds[cmd_idx].cmd;
> > > > - len = cyapa_i2c_cmds[cmd_idx].len;
> > > > - return cyapa_i2c_reg_read_block(cyapa, cmd, len, values);
> > > > - }
> > > > -}
> > > > -
> > > > -/*
> > > > - * Query device for its current operating state.
> > > > - *
> > > > - */
> > > > -static int cyapa_get_state(struct cyapa *cyapa)
> > > > +/* Returns 0 on success, else negative errno on failure. */
> > > > +ssize_t cyapa_i2c_read(struct cyapa *cyapa, u8 reg, size_t len,
> > > > + u8 *values)
> > > > {
> > > > int ret;
> > > > - u8 status[BL_STATUS_SIZE];
> > > > -
> > > > - cyapa->state = CYAPA_STATE_NO_DEVICE;
> > > > -
> > > > - /*
> > > > - * Get trackpad status by reading 3 registers starting from 0.
> > > > - * If the device is in the bootloader, this will be BL_HEAD.
> > > > - * If the device is in operation mode, this will be the DATA regs.
> > > > - *
> > > > - */
> > > > - ret = cyapa_i2c_reg_read_block(cyapa, BL_HEAD_OFFSET,
> BL_STATUS_SIZE,
> > > > - status);
> > > > -
> > > > - /*
> > > > - * On smbus systems in OP mode, the i2c_reg_read will fail with
> > > > - * -ETIMEDOUT. In this case, try again using the smbus
> equivalent
> > > > - * command. This should return a BL_HEAD indicating
> CYAPA_STATE_OP.
> > > > - */
> > > > - if (cyapa->smbus && (ret == -ETIMEDOUT || ret == -ENXIO))
> > > > - ret = cyapa_read_block(cyapa, CYAPA_CMD_BL_STATUS, status);
> > > > -
> > > > - if (ret != BL_STATUS_SIZE)
> > > > - goto error;
> > > > -
> > > > - if ((status[REG_OP_STATUS] & OP_STATUS_SRC) == OP_STATUS_SRC) {
> > > > - switch (status[REG_OP_STATUS] & OP_STATUS_DEV) {
> > > > - case CYAPA_DEV_NORMAL:
> > > > - case CYAPA_DEV_BUSY:
> > > > - cyapa->state = CYAPA_STATE_OP;
> > > > - break;
> > > > - default:
> > > > - ret = -EAGAIN;
> > > > - goto error;
> > > > - }
> > > > - } else {
> > > > - if (status[REG_BL_STATUS] & BL_STATUS_BUSY)
> > > > - cyapa->state = CYAPA_STATE_BL_BUSY;
> > > > - else if (status[REG_BL_ERROR] & BL_ERROR_BOOTLOADING)
> > > > - cyapa->state = CYAPA_STATE_BL_ACTIVE;
> > > > - else
> > > > - cyapa->state = CYAPA_STATE_BL_IDLE;
> > > > - }
> > > > -
> > > > - return 0;
> > > > -error:
> > > > - return (ret < 0) ? ret : -EAGAIN;
> > > > + struct i2c_client *client = cyapa->client;
> > > > + struct i2c_msg msgs[] = {
> > > > + {
> > > > + .addr = client->addr,
> > > > + .flags = 0,
> > > > + .len = 1,
> > > > + .buf = ®,
> > > > + },
> > > > + {
> > > > + .addr = client->addr,
> > > > + .flags = I2C_M_RD,
> > > > + .len = len,
> > > > + .buf = values,
> > > > + },
> > > > + };
> > > > +
> > > > + ret = i2c_transfer(client->adapter, msgs, 2);
> > >
> > > You need to watch out for partial transfers. It should be:
> > >
> > > if (ret != ARRAY_SIZE(msgs))
> > > return ret < 0 ? ret : -EIO;
> > >
> > > return 0;
> > >
> >
> > Thanks, I will update in next release.
> >
> > > > +
> > > > + return ret < 0 ? ret : 0;
> > > > }
> > > >
> > > > -/*
> > > > - * Poll device for its status in a loop, waiting up to timeout for a
> > > response.
> > > > - *
> > > > - * When the device switches state, it usually takes ~300 ms.
> > > > - * However, when running a new firmware image, the device must
> calibrate
> > > its
> > > > - * sensors, which can take as long as 2 seconds.
> > > > +/**
> > > > + * cyapa_i2c_write - Execute i2c block data write operation
> > > > + * @cyapa: Handle to this driver
> > > > + * @ret: Offset of the data to written in the register map
> > > > + * @len: number of bytes to write
> > > > + * @values: Data to be written
> > > > *
> > > > - * Note: The timeout has granularity of the polling rate, which is 100
> ms.
> > > > - *
> > > > - * Returns:
> > > > - * 0 when the device eventually responds with a valid non-busy state.
> > > > - * -ETIMEDOUT if device never responds (too many -EAGAIN)
> > > > - * < 0 other errors
> > > > + * Return negative errno code on error; return zero when success.
> > > > */
> > > > -static int cyapa_poll_state(struct cyapa *cyapa, unsigned int timeout)
> > > > +ssize_t cyapa_i2c_write(struct cyapa *cyapa, u8 reg,
> > > > + size_t len, const void *values)
> > > > {
> > > > int ret;
> > > > - int tries = timeout / 100;
> > > > -
> > > > - ret = cyapa_get_state(cyapa);
> > > > - while ((ret || cyapa->state >= CYAPA_STATE_BL_BUSY) && tries--) {
> > > > - msleep(100);
> > > > - ret = cyapa_get_state(cyapa);
> > > > - }
> > > > - return (ret == -EAGAIN || ret == -ETIMEDOUT) ? -ETIMEDOUT : ret;
> > > > -}
> > > > + struct i2c_client *client = cyapa->client;
> > > > + char data[32], *buf;
> > > >
> > > > -static int cyapa_bl_deactivate(struct cyapa *cyapa)
> > > > -{
> > > > - int ret;
> > > > + if (len > 31)
> > > > + buf = kzalloc(len + 1, GFP_KERNEL);
> > > > + else
> > > > + buf = data;
> > > >
> > > > - ret = cyapa_i2c_reg_write_block(cyapa, 0, sizeof(bl_deactivate),
> > > > - bl_deactivate);
> > > > - if (ret < 0)
> > > > - return ret;
> > > > + buf[0] = reg;
> > > > + memcpy(&buf[1], values, len);
> > > > + ret = i2c_master_send(client, buf, len + 1);
> > > >
> > > > - /* wait for bootloader to switch to idle state; should take <
> 100ms */
> > > > - msleep(100);
> > > > - ret = cyapa_poll_state(cyapa, 500);
> > > > - if (ret < 0)
> > > > - return ret;
> > > > - if (cyapa->state != CYAPA_STATE_BL_IDLE)
> > > > - return -EAGAIN;
> > > > - return 0;
> > > > + if (buf != data)
> > > > + kfree(buf);
> > > > + return (ret == (len + 1)) ? 0 : ((ret < 0) ? ret : -EIO);
> > > > }
> > > >
> > > > -/*
> > > > - * Exit bootloader
> > > > - *
> > > > - * Send bl_exit command, then wait 50 - 100 ms to let device transition
> to
> > > > - * operational mode. If this is the first time the device's firmware
> is
> > > > - * running, it can take up to 2 seconds to calibrate its sensors. So,
> poll
> > > > - * the device's new state for up to 2 seconds.
> > > > - *
> > > > - * Returns:
> > > > - * -EIO failure while reading from device
> > > > - * -EAGAIN device is stuck in bootloader, b/c it has invalid firmware
> > > > - * 0 device is supported and in operational mode
> > > > - */
> > > > -static int cyapa_bl_exit(struct cyapa *cyapa)
> > > > +void cyapa_default_irq_handler(struct cyapa *cyapa)
> > > > {
> > > > - int ret;
> > > > -
> > > > - ret = cyapa_i2c_reg_write_block(cyapa, 0, sizeof(bl_exit),
> bl_exit);
> > > > - if (ret < 0)
> > > > - return ret;
> > > > -
> > > > - /*
> > > > - * Wait for bootloader to exit, and operation mode to start.
> > > > - * Normally, this takes at least 50 ms.
> > > > - */
> > > > - usleep_range(50000, 100000);
> > > > /*
> > > > - * In addition, when a device boots for the first time after
> being
> > > > - * updated to new firmware, it must first calibrate its sensors,
> which
> > > > - * can take up to an additional 2 seconds.
> > > > + * Do redetecting when device states is still unknown and
> > > > + * interrupt envent is received from device.
> > > > */
> > > > - ret = cyapa_poll_state(cyapa, 2000);
> > > > - if (ret < 0)
> > > > - return ret;
> > > > - if (cyapa->state != CYAPA_STATE_OP)
> > > > - return -EAGAIN;
> > > > -
> > > > - return 0;
> > > > -}
> > > > -
> > > > -/*
> > > > - * Set device power mode
> > > > - *
> > > > - */
> > > > -static int cyapa_set_power_mode(struct cyapa *cyapa, u8 power_mode)
> > > > -{
> > > > - struct device *dev = &cyapa->client->dev;
> > > > - int ret;
> > > > - u8 power;
> > > > -
> > > > - if (cyapa->state != CYAPA_STATE_OP)
> > > > - return 0;
> > > > -
> > > > - ret = cyapa_read_byte(cyapa, CYAPA_CMD_POWER_MODE);
> > > > - if (ret < 0)
> > > > - return ret;
> > > > -
> > > > - power = ret & ~PWR_MODE_MASK;
> > > > - power |= power_mode & PWR_MODE_MASK;
> > > > - ret = cyapa_write_byte(cyapa, CYAPA_CMD_POWER_MODE, power);
> > > > - if (ret < 0)
> > > > - dev_err(dev, "failed to set power_mode 0x%02x err = %d\n",
> > > > - power_mode, ret);
> > > > - return ret;
> > > > -}
> > > > -
> > > > -static int cyapa_get_query_data(struct cyapa *cyapa)
> > > > -{
> > > > - u8 query_data[QUERY_DATA_SIZE];
> > > > - int ret;
> > > > -
> > > > - if (cyapa->state != CYAPA_STATE_OP)
> > > > - return -EBUSY;
> > > > -
> > > > - ret = cyapa_read_block(cyapa, CYAPA_CMD_GROUP_QUERY, query_data);
> > > > - if (ret < 0)
> > > > - return ret;
> > > > - if (ret != QUERY_DATA_SIZE)
> > > > - return -EIO;
> > > > -
> > > > - memcpy(&cyapa->product_id[0], &query_data[0], 5);
> > > > - cyapa->product_id[5] = '-';
> > > > - memcpy(&cyapa->product_id[6], &query_data[5], 6);
> > > > - cyapa->product_id[12] = '-';
> > > > - memcpy(&cyapa->product_id[13], &query_data[11], 2);
> > > > - cyapa->product_id[15] = '\0';
> > > > -
> > > > - cyapa->btn_capability = query_data[19] & CAPABILITY_BTN_MASK;
> > > > -
> > > > - cyapa->gen = query_data[20] & 0x0f;
> > > > -
> > > > - cyapa->max_abs_x = ((query_data[21] & 0xf0) << 4) |
> query_data[22];
> > > > - cyapa->max_abs_y = ((query_data[21] & 0x0f) << 8) |
> query_data[23];
> > > > -
> > > > - cyapa->physical_size_x =
> > > > - ((query_data[24] & 0xf0) << 4) | query_data[25];
> > > > - cyapa->physical_size_y =
> > > > - ((query_data[24] & 0x0f) << 8) | query_data[26];
> > > > -
> > > > - return 0;
> > > > + async_schedule(cyapa_detect_async, cyapa);
> > >
> > > I think I already asked this before - why do we need to try and schedule
> async
> > > detect from interrupt handler. In what cases we will fail to initialize
> the
> > > device during normal probing, but then are fine when stray interrupt
> arrives?
> > >
> >
> > 1) Because gen5 TP devices use interrupt to sync the command and response,
> and
> > in detecting, some commands must be sent and executed in attached devices,
> so
> > the interrupt must be usable in detecting.
> > So if do not schedule async detect from interrupt handler, the interrupt
> > handler will be taken, and cannot enter again, which will cause the command
> to
> > the device failed, it will also cause long time detecting.
> > 2) detecting will fail when no device attached or the attached device is not
> > supported gen3 or gen5 TP devices or there is some issue with the device
> > that cannot enter into active working mode or stay in bootloader mode
> > for invalid firmware detected.
> > And it's tested that it's save when stray interrupt arrives in detecting.
>
>
> I am sorry, I have trouble parsing this. I understand that you may need
> interrupt to know when command response is ready, but I do not see how
> kicking asynchronous detect helps there. During probe you can figure out
> if you are talking to a Cypress device and whether it is operable. If it
> is not operable you can try flashing a new firmware and then kick off
> detection again after flash is successful. But I do not see any reason
> in trying to re-detect the device after random interrupt arrives in hopes
> that maybe this time stars will align and we'll be able to work with it.
Considering two situations:
1) For some machines, when system get into sleep mode, the power of the trackpad device
will be cut off, and after system resumed, the power to trackpad device is on again.
2) For the trackpad device itself, if internal error encounter, it will reset itself,
similar result as power off and power on again.
When either of above situation happened, for gen3 Cypress trackpad device, it will get
into bootloader mode (it cannot get into operational mode automatically), and also assert
interrupts to host.
So at this time, cyapa driver must kick off the detection again to determine the real status
of the trackpad device, and command it go to the operational mode.
Otherwise, it will be out of work until reboot.
>
> >
> > Thanks.
> >
> >
> > > > }
> > > >
> > > > -/*
> > > > - * Check if device is operational.
> > > > - *
> > > > - * An operational device is responding, has exited bootloader, and has
> > > > - * firmware supported by this driver.
> > > > - *
> > > > - * Returns:
> > > > - * -EBUSY no device or in bootloader
> > > > - * -EIO failure while reading from device
> > > > - * -EAGAIN device is still in bootloader
> > > > - * if ->state = CYAPA_STATE_BL_IDLE, device has invalid
> firmware
> > > > - * -EINVAL device is in operational mode, but not supported by this
> > > driver
> > > > - * 0 device is supported
> > > > - */
> > > > -static int cyapa_check_is_operational(struct cyapa *cyapa)
> > > > -{
> > > > - struct device *dev = &cyapa->client->dev;
> > > > - static const char unique_str[] = "CYTRA";
> > > > - int ret;
> > > > -
> > > > - ret = cyapa_poll_state(cyapa, 2000);
> > > > - if (ret < 0)
> > > > - return ret;
> > > > - switch (cyapa->state) {
> > > > - case CYAPA_STATE_BL_ACTIVE:
> > > > - ret = cyapa_bl_deactivate(cyapa);
> > > > - if (ret)
> > > > - return ret;
> > > > -
> > > > - /* Fallthrough state */
> > > > - case CYAPA_STATE_BL_IDLE:
> > > > - ret = cyapa_bl_exit(cyapa);
> > > > - if (ret)
> > > > - return ret;
> > > > -
> > > > - /* Fallthrough state */
> > > > - case CYAPA_STATE_OP:
> > > > - ret = cyapa_get_query_data(cyapa);
> > > > - if (ret < 0)
> > > > - return ret;
> > > > -
> > > > - /* only support firmware protocol gen3 */
> > > > - if (cyapa->gen != CYAPA_GEN3) {
> > > > - dev_err(dev, "unsupported protocol version (%d)",
> > > > - cyapa->gen);
> > > > - return -EINVAL;
> > > > - }
> > > > -
> > > > - /* only support product ID starting with CYTRA */
> > > > - if (memcmp(cyapa->product_id, unique_str,
> > > > - sizeof(unique_str) - 1) != 0) {
> > > > - dev_err(dev, "unsupported product ID (%s)\n",
> > > > - cyapa->product_id);
> > > > - return -EINVAL;
> > > > - }
> > > > - return 0;
> > > > -
> > > > - default:
> > > > - return -EIO;
> > > > - }
> > > > - return 0;
> > > > -}
> > > > -
> > > > -static irqreturn_t cyapa_irq(int irq, void *dev_id)
> > > > -{
> > > > - struct cyapa *cyapa = dev_id;
> > > > - struct device *dev = &cyapa->client->dev;
> > > > - struct input_dev *input = cyapa->input;
> > > > - struct cyapa_reg_data data;
> > > > - int i;
> > > > - int ret;
> > > > - int num_fingers;
> > > > -
> > > > - if (device_may_wakeup(dev))
> > > > - pm_wakeup_event(dev, 0);
> > > > -
> > > > - ret = cyapa_read_block(cyapa, CYAPA_CMD_GROUP_DATA, (u8 *)&data);
> > > > - if (ret != sizeof(data))
> > > > - goto out;
> > > > -
> > > > - if ((data.device_status & OP_STATUS_SRC) != OP_STATUS_SRC ||
> > > > - (data.device_status & OP_STATUS_DEV) != CYAPA_DEV_NORMAL ||
> > > > - (data.finger_btn & OP_DATA_VALID) != OP_DATA_VALID) {
> > > > - goto out;
> > > > - }
> > > > -
> > > > - num_fingers = (data.finger_btn >> 4) & 0x0f;
> > > > - for (i = 0; i < num_fingers; i++) {
> > > > - const struct cyapa_touch *touch = &data.touches[i];
> > > > - /* Note: touch->id range is 1 to 15; slots are 0 to 14. */
> > > > - int slot = touch->id - 1;
> > > > -
> > > > - input_mt_slot(input, slot);
> > > > - input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
> > > > - input_report_abs(input, ABS_MT_POSITION_X,
> > > > - ((touch->xy_hi & 0xf0) << 4) | touch->x_lo);
> > > > - input_report_abs(input, ABS_MT_POSITION_Y,
> > > > - ((touch->xy_hi & 0x0f) << 8) | touch->y_lo);
> > > > - input_report_abs(input, ABS_MT_PRESSURE, touch->pressure);
> > > > - }
> > > > -
> > > > - input_mt_sync_frame(input);
> > > > -
> > > > - if (cyapa->btn_capability & CAPABILITY_LEFT_BTN_MASK)
> > > > - input_report_key(input, BTN_LEFT,
> > > > - data.finger_btn & OP_DATA_LEFT_BTN);
> > > > -
> > > > - if (cyapa->btn_capability & CAPABILITY_MIDDLE_BTN_MASK)
> > > > - input_report_key(input, BTN_MIDDLE,
> > > > - data.finger_btn & OP_DATA_MIDDLE_BTN);
> > > > -
> > > > - if (cyapa->btn_capability & CAPABILITY_RIGHT_BTN_MASK)
> > > > - input_report_key(input, BTN_RIGHT,
> > > > - data.finger_btn & OP_DATA_RIGHT_BTN);
> > > > -
> > > > - input_sync(input);
> > > > +const struct cyapa_dev_ops cyapa_default_ops = {
> > > > + .irq_handler = cyapa_default_irq_handler
> > > > +};
> > > >
> > > > -out:
> > > > - return IRQ_HANDLED;
> > > > -}
> > > >
> > > > static u8 cyapa_check_adapter_functionality(struct i2c_client *client)
> > > > {
> > > > @@ -772,19 +155,40 @@ static int cyapa_create_input_dev(struct cyapa
> *cyapa)
> > > > input->phys = cyapa->phys;
> > > > input->id.bustype = BUS_I2C;
> > > > input->id.version = 1;
> > > > - input->id.product = 0; /* means any product in eventcomm. */
> > > > + input->id.product = 0; /* Means any product in eventcomm. */
> > > > input->dev.parent = &cyapa->client->dev;
> > > >
> > > > input_set_drvdata(input, cyapa);
> > > >
> > > > __set_bit(EV_ABS, input->evbit);
> > > >
> > > > - /* finger position */
> > > > + /* Finger position */
> > > > input_set_abs_params(input, ABS_MT_POSITION_X, 0, cyapa-
> >max_abs_x, 0,
> > > > 0);
> > > > input_set_abs_params(input, ABS_MT_POSITION_Y, 0, cyapa-
> >max_abs_y, 0,
> > > > 0);
> > > > - input_set_abs_params(input, ABS_MT_PRESSURE, 0, 255, 0, 0);
> > > > + input_set_abs_params(input, ABS_MT_PRESSURE, 0, cyapa->max_z, 0,
> 0);
> > > > + if (cyapa->gen > CYAPA_GEN3) {
> > > > + input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255, 0,
> 0);
> > > > + input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, 255, 0,
> 0);
> > > > + /*
> > > > + * Orientation is the angle between the vertical axis and
> > > > + * the major axis of the contact ellipse.
> > > > + * The range is -127 to 127.
> > > > + * the positive direction is clockwise form the vertical
> axis.
> > > > + * If the ellipse of contact degenerates into a circle,
> > > > + * orientation is reported as 0.
> > > > + *
> > > > + * Also, for Gen5 trackpad the accurate of this
> orientation
> > > > + * value is value + (-30 ~ 30).
> > > > + */
> > > > + input_set_abs_params(input, ABS_MT_ORIENTATION,
> > > > + -127, 127, 0, 0);
> > > > + }
> > > > + if (cyapa->gen >= CYAPA_GEN5) {
> > > > + input_set_abs_params(input, ABS_MT_WIDTH_MAJOR, 0, 255, 0,
> 0);
> > > > + input_set_abs_params(input, ABS_MT_WIDTH_MINOR, 0, 255, 0,
> 0);
> > > > + }
> > > >
> > > > input_abs_set_res(input, ABS_MT_POSITION_X,
> > > > cyapa->max_abs_x / cyapa->physical_size_x);
> > > > @@ -801,7 +205,7 @@ static int cyapa_create_input_dev(struct cyapa
> *cyapa)
> > > > if (cyapa->btn_capability == CAPABILITY_LEFT_BTN_MASK)
> > > > __set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
> > > >
> > > > - /* handle pointer emulation and unused slots in core */
> > > > + /* Handle pointer emulation and unused slots in core */
> > > > ret = input_mt_init_slots(input, CYAPA_MAX_MT_SLOTS,
> > > > INPUT_MT_POINTER | INPUT_MT_DROP_UNUSED);
> > > > if (ret) {
> > > > @@ -823,6 +227,224 @@ err_free_device:
> > > > return ret;
> > > > }
> > > >
> > > > +/*
> > > > + * Check if device is operational.
> > > > + *
> > > > + * An operational device is responding, has exited bootloader, and has
> > > > + * firmware supported by this driver.
> > > > + *
> > > > + * Returns:
> > > > + * -EBUSY no device or in bootloader
> > > > + * -EIO failure while reading from device
> > > > + * -EAGAIN device is still in bootloader
> > > > + * if ->state = CYAPA_STATE_BL_IDLE, device has invalid
> firmware
> > > > + * -EINVAL device is in operational mode, but not supported by this
> > > driver
> > > > + * 0 device is supported
> > > > + */
> > > > +static int cyapa_check_is_operational(struct cyapa *cyapa)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + ret = cyapa_poll_state(cyapa, 4000);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + switch (cyapa->gen) {
> > > > + default:
> > > > + cyapa->ops = &cyapa_default_ops;
> > > > + cyapa->gen = CYAPA_GEN_UNKNOWN;
> > > > + break;
> > > > + }
> > > > +
> > > > + if (cyapa->ops->operational_check)
> > > > + ret = cyapa->ops->operational_check(cyapa);
> > > > + else
> > > > + ret = -EBUSY;
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +
> > > > +static irqreturn_t cyapa_irq(int irq, void *dev_id)
> > > > +{
> > > > + struct cyapa *cyapa = dev_id;
> > > > + struct input_dev *input = cyapa->input;
> > > > + bool cont;
> > > > +
> > > > + /* Interrupt event maybe cuased by host command to trackpad
> device. */
> > > > + cont = true;
> > > > + if (cyapa->ops->irq_cmd_handler)
> > > > + cont = cyapa->ops->irq_cmd_handler(cyapa);
> > > > +
> > > > + /* Interrupt event maybe from trackpad device input reporting. */
> > > > + if (cont && cyapa->ops->irq_handler) {
> > > > + if (!mutex_trylock(&cyapa->state_sync_lock)) {
> > > > + if (cyapa->ops->sort_empty_output_data)
> > > > + cyapa->ops->sort_empty_output_data(cyapa,
> > > > + NULL, NULL, NULL);
> > > > + goto out;
> > > > + }
> > > > +
> > > > + if (!input) {
> > > > + if (cyapa->ops->sort_empty_output_data)
> > > > + cyapa->ops->sort_empty_output_data(cyapa,
> > > > + NULL, NULL, NULL);
> > > > + } else
> > > > + cyapa->ops->irq_handler(cyapa);
> > > > +
> > > > + mutex_unlock(&cyapa->state_sync_lock);
> > > > + }
> > > > +out:
> > > > + return IRQ_HANDLED;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Query device for its current operating state.
> > > > + */
> > > > +static int cyapa_get_state(struct cyapa *cyapa)
> > > > +{
> > > > + cyapa->state = CYAPA_STATE_NO_DEVICE;
> > > > +
> > > > + return -ENODEV;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Poll device for its status in a loop, waiting up to timeout for a
> > > response.
> > > > + *
> > > > + * When the device switches state, it usually takes ~300 ms.
> > > > + * However, when running a new firmware image, the device must
> calibrate
> > > its
> > > > + * sensors, which can take as long as 2 seconds.
> > > > + *
> > > > + * Note: The timeout has granularity of the polling rate, which is 100
> ms.
> > > > + *
> > > > + * Returns:
> > > > + * 0 when the device eventually responds with a valid non-busy state.
> > > > + * -ETIMEDOUT if device never responds (too many -EAGAIN)
> > > > + * < 0 other errors
> > > > + */
> > > > +int cyapa_poll_state(struct cyapa *cyapa, unsigned int timeout)
> > > > +{
> > > > + int ret;
> > > > + int tries = timeout / 100;
> > > > +
> > > > + ret = cyapa_get_state(cyapa);
> > > > + while ((ret || cyapa->state >= CYAPA_STATE_BL_BUSY) && tries--) {
> > > > + msleep(100);
> > > > + ret = cyapa_get_state(cyapa);
> > > > + }
> > > > +
> > > > + return (ret == -EAGAIN || ret == -ETIMEDOUT) ? -ETIMEDOUT : ret;
> > > > +}
> > > > +
> > > > +static void cyapa_detect(struct cyapa *cyapa)
> > > > +{
> > > > + struct device *dev = &cyapa->client->dev;
> > > > + char *envp[2] = {"ERROR=1", NULL};
> > > > + int ret;
> > > > +
> > > > + ret = cyapa_check_is_operational(cyapa);
> > > > + if (ret == -ETIMEDOUT)
> > > > + dev_err(dev, "no device detected, %d\n", ret);
> > > > + else if (ret)
> > > > + dev_err(dev, "device detected, but not operational, %d\n",
> ret);
> > > > +
> > > > + if (ret) {
> > > > + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
> > > > + return;
> > > > + }
> > > > +
> > > > + if (!cyapa->input) {
> > > > + ret = cyapa_create_input_dev(cyapa);
> > > > + if (ret)
> > > > + dev_err(dev, "create input_dev instance
> failed, %d\n",
> > > > + ret);
> > > > +
> > > > + /*
> > > > + * On some systems, a system crash / warm boot does not
> reset
> > > > + * the device's current power mode to FULL_ACTIVE.
> > > > + * If such an event happens during suspend, after the
> device
> > > > + * has been put in a low power mode, the device will still
> be
> > > > + * in low power mode on a subsequent boot, since there was
> > > > + * never a matching resume().
> > > > + * Handle this by always forcing full power here, when a
> > > > + * device is first detected to be in operational mode.
> > > > + */
> > > > + if (cyapa->ops->set_power_mode) {
> > > > + ret = cyapa->ops->set_power_mode(cyapa,
> > > > + PWR_MODE_FULL_ACTIVE, 0);
> > > > + if (ret)
> > > > + dev_warn(dev, "set active power failed, %d\n",
> > > > + ret);
> > > > + }
> > > > + }
> > > > +}
> > > > +
> > > > +/*
> > > > + * Sysfs Interface.
> > > > + */
> > > > +
> > > > +/*
> > > > + * cyapa_sleep_time_to_pwr_cmd and cyapa_pwr_cmd_to_sleep_time
> > > > + *
> > > > + * These are helper functions that convert to and from integer idle
> > > > + * times and register settings to write to the PowerMode register.
> > > > + * The trackpad supports between 20ms to 1000ms scan intervals.
> > > > + * The time will be increased in increments of 10ms from 20ms to 100ms.
> > > > + * From 100ms to 1000ms, time will be increased in increments of 20ms.
> > > > + *
> > > > + * When Idle_Time < 100, the format to convert Idle_Time to
> Idle_Command is:
> > > > + * Idle_Command = Idle Time / 10;
> > > > + * When Idle_Time >= 100, the format to convert Idle_Time to
> Idle_Command
> > > is:
> > > > + * Idle_Command = Idle Time / 20 + 5;
> > > > + */
> > > > +u8 cyapa_sleep_time_to_pwr_cmd(u16 sleep_time)
> > > > +{
> > > > + if (sleep_time < 20)
> > > > + sleep_time = 20; /* minimal sleep time. */
> > > > + else if (sleep_time > 1000)
> > > > + sleep_time = 1000; /* maximal sleep time. */
> > >
> > > clamp_val()
> >
> > Thanks. clamp_pwr_cmd_sleep_time() will be used to above code.
>
> No, we have a macro called clamp_val() which does exactly what you are
> doing here by hand. So please use
>
> sleep_time = clamp_val(sleep_time, 20, 1000);
>
Got it, thanks.
>
> >
> > >
> > > > +
> > > > + if (sleep_time < 100)
> > > > + return ((sleep_time / 10) << 2) & PWR_MODE_MASK;
> > > > + else
> > > > + return ((sleep_time / 20 + 5) << 2) & PWR_MODE_MASK;
> > > > +}
> > > > +
> > > > +u16 cyapa_pwr_cmd_to_sleep_time(u8 pwr_mode)
> > > > +{
> > > > + u8 encoded_time = pwr_mode >> 2;
> > > > +
> > > > + return (encoded_time < 10) ? encoded_time * 10
> > > > + : (encoded_time - 5) * 20;
> > > > +}
> > > > +
> > > > +void cyapa_detect_async(void *data, async_cookie_t cookie)
> > > > +{
> > > > + struct cyapa *cyapa = (struct cyapa *)data;
> > > > +
> > > > + mutex_lock(&cyapa->state_sync_lock);
> > > > +
> > > > + /* Keep synchronized with sys interface process threads. */
> > > > + cyapa_detect(cyapa);
> > > > +
> > > > + mutex_unlock(&cyapa->state_sync_lock);
> > > > +}
> > > > +
> > > > +static void cyapa_detect_and_start(void *data, async_cookie_t cookie)
> > > > +{
> > > > + cyapa_detect_async(data, cookie);
> > > > +}
> > > > +
> > > > +static int cyapa_tp_modules_init(struct cyapa *cyapa)
> > > > +{
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int cyapa_tp_modules_uninit(struct cyapa *cyapa)
> > > > +{
> > > > + return 0;
> > > > +}
> > > > +
> > > > static int cyapa_probe(struct i2c_client *client,
> > > > const struct i2c_device_id *dev_id)
> > > > {
> > > > @@ -830,6 +452,7 @@ static int cyapa_probe(struct i2c_client *client,
> > > > u8 adapter_func;
> > > > struct cyapa *cyapa;
> > > > struct device *dev = &client->dev;
> > > > + union i2c_smbus_data dummy;
> > > >
> > > > adapter_func = cyapa_check_adapter_functionality(client);
> > > > if (adapter_func == CYAPA_ADAPTER_FUNC_NONE) {
> > > > @@ -837,39 +460,43 @@ static int cyapa_probe(struct i2c_client *client,
> > > > return -EIO;
> > > > }
> > > >
> > > > + /* Make sure there is something at this address */
> > > > + if (dev->of_node && i2c_smbus_xfer(client->adapter, client->addr,
> 0,
> > > > + I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &dummy) < 0)
> > > > + return -ENODEV;
> > > > +
> > > > cyapa = kzalloc(sizeof(struct cyapa), GFP_KERNEL);
> > > > if (!cyapa) {
> > > > dev_err(dev, "allocate memory for cyapa failed\n");
> > > > return -ENOMEM;
> > > > }
> > > >
> > > > - cyapa->gen = CYAPA_GEN3;
> > > > cyapa->client = client;
> > > > i2c_set_clientdata(client, cyapa);
> > > > sprintf(cyapa->phys, "i2c-%d-%04x/input0", client->adapter->nr,
> > > > client->addr);
> > > >
> > > > - /* i2c isn't supported, use smbus */
> > > > - if (adapter_func == CYAPA_ADAPTER_FUNC_SMBUS)
> > > > - cyapa->smbus = true;
> > > > - cyapa->state = CYAPA_STATE_NO_DEVICE;
> > > > - ret = cyapa_check_is_operational(cyapa);
> > > > + ret = cyapa_tp_modules_init(cyapa);
> > > > if (ret) {
> > > > - dev_err(dev, "device not operational, %d\n", ret);
> > > > - goto err_mem_free;
> > > > + dev_err(dev, "initialize device modules failed.\n");
> > > > + goto err_unregister_device;
> > > > }
> > > >
> > > > - ret = cyapa_create_input_dev(cyapa);
> > > > - if (ret) {
> > > > - dev_err(dev, "create input_dev instance failed, %d\n",
> ret);
> > > > - goto err_mem_free;
> > > > - }
> > > > + cyapa->gen = CYAPA_GEN_UNKNOWN;
> > > > + cyapa->ops = &cyapa_default_ops;
> > > > + mutex_init(&cyapa->state_sync_lock);
> > > >
> > > > - ret = cyapa_set_power_mode(cyapa, PWR_MODE_FULL_ACTIVE);
> > > > - if (ret) {
> > > > - dev_err(dev, "set active power failed, %d\n", ret);
> > > > - goto err_unregister_device;
> > > > - }
> > > > + /* i2c isn't supported, use smbus */
> > > > + if (adapter_func == CYAPA_ADAPTER_FUNC_SMBUS)
> > > > + cyapa->smbus = true;
> > > > + cyapa->state = CYAPA_STATE_NO_DEVICE;
> > > > + /*
> > > > + * Set to hard code default, they will be updated with trackpad
> set
> > > > + * default values after probe and initialized.
> > > > + */
> > > > + cyapa->suspend_power_mode = PWR_MODE_SLEEP;
> > > > + cyapa->suspend_sleep_time =
> > > > + cyapa_pwr_cmd_to_sleep_time(cyapa->suspend_power_mode);
> > > >
> > > > cyapa->irq = client->irq;
> > > > ret = request_threaded_irq(cyapa->irq,
> > > > @@ -880,14 +507,17 @@ static int cyapa_probe(struct i2c_client *client,
> > > > cyapa);
> > > > if (ret) {
> > > > dev_err(dev, "IRQ request failed: %d\n, ", ret);
> > > > - goto err_unregister_device;
> > > > + goto err_uninit_tp_modules;
> > > > }
> > > >
> > > > + async_schedule(cyapa_detect_and_start, cyapa);
> > > > return 0;
> > > >
> > > > +err_uninit_tp_modules:
> > > > + cyapa_tp_modules_uninit(cyapa);
> > > > err_unregister_device:
> > > > input_unregister_device(cyapa->input);
> > > > -err_mem_free:
> > > > + i2c_set_clientdata(client, NULL);
> > > > kfree(cyapa);
> > > >
> > > > return ret;
> > > > @@ -898,8 +528,12 @@ static int cyapa_remove(struct i2c_client *client)
> > > > struct cyapa *cyapa = i2c_get_clientdata(client);
> > > >
> > >
> > > You schedule detect asynchronously so you need to make sure it is not
> running
> > > (and will not be running) when you try to unbind the driver here.
> >
> > Thanks. I will update it in next release.
> >
> > To avoid this problem, a removed flag is introduced and is also synced by
> the
> > cyapa->state_sync_lock mutex lock. So when this driver is removing, it's
> sure
> > that the detecting asynchronous thread is not running, and if there's any
> event
> > cause the asynchronous thread is entered, it will do nothing and will exit
> immediately.
>
> Peeking at the other version of the patch you do:
>
> cyapa_remove()
> {
> ...
> cyapa->removed = true;
> ...
> kfree(cyapa);
> }
>
> and
>
> void cyapa_detect_async(void *data, async_cookie_t cookie)
> {
> ...
> if (cyapa->removed) {
> mutex_unlock(&cyapa->state_sync_lock);
> return;
> }
> ...
> }
>
> That's not going to work.
>
> Getting asynchronous behavior by individual driver is hard and I believe
> Luis Rordiguez is working on allowing to do that is the driver core. So
> I would recommend to switch driver to do synchronous probing, at least
> for now, and figure out other kinks first.
>
> Thanks.
>
The consideration of the thread asynchronous issue is that:
The detection thread has been scheduled, but not run yet, and at the time,
the driver itself has been removed, so when later the detection thread is
triggered to run, the memory of cyapa driver has been freed, and cause error, right?
I have an idea as below, please help review.
Add variable cyapa->detect_thread_actived to trace the number of the threads were scheduled,
and when the driver module is in removing, it will wait all scheduled threads has been run.
That is when a thread is scheduled, cyapa->detect_thread_actived will be increased,
when the thread is executed and finished, the cyapa->detect_thread_actived will be decrreased,
so, when the value of cyapa->detect_thread_actived is 0, it must be no thread scheduled for running.
So when at this time, if the driver module is marked for removed, the remove() thread that waiting
on the cyapa->detect_thread_completed will be issue, and then driver is safe to be removed.
The code is similar as below.
For each time, before the cyapa_detect_async() function is called or scheduled,
... {
...
atomic_inc(&cyapa->detect_thread_actived);
async_schedule(cyapa_detect_async, cyapa);
...
}
void cyapa_detect_async(void *data, async_cookie_t cookie)
{
...
if (cyapa->removed) {
mutex_unlock(&cyapa->state_sync_lock);
return;
}
...
free_irq(cyapa->irq, cyapa);
/* Wait until all scheduled thread exited. */
if (atomic_read(&cyapa->detect_thread_actived)
wait_for_completion_timeout(&cyapa->detect_thread_completed,
msecs_to_jiffies(4000));
...
}
void cyapa_detect_async(void *data, async_cookie_t cookie)
{
struct cyapa *cyapa = (struct cyapa *)data;
mutex_lock(&cyapa->state_sync_lock);
if (cyapa->removed) {
mutex_unlock(&cyapa->state_sync_lock);
if (atomic_dec_and_test(&cyapa->detect_thread_actived)) {
/* Now, no thread runing, safe to remove this driver. */
complete(&cyapa->detect_thread_completed);
}
goto return;
}
/* Keep synchronized with sys interface process threads. */
cyapa_detect(cyapa);
mutex_unlock(&cyapa->state_sync_lock);
atomic_dec(&cyapa->detect_thread_actived);
}
> --
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: HID: wacom: regression - system freezes when resuming from S3 suspend
From: Jiri Kosina @ 2014-09-24 8:34 UTC (permalink / raw)
To: Jonas Jelten; +Cc: Benjamin Tissoires, linux-input
In-Reply-To: <54221EFF.2060501@in.tum.de>
On Wed, 24 Sep 2014, Jonas Jelten wrote:
> >> I encountered that my system freezes when resuming my Lenovo X220t with
> >> integrated Wacom ISDv4 tablet from S3 suspend.
> >>
> >> Currently running 3.17.0-rc6, I started to bisect and found that this
> >> commit causes the issue:
> >>
> >> commit 29b4739134c73a2873adec93346f09bb76d6a794
> >> Author: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >> Date: Thu Jul 24 12:52:23 2014 -0700
> >>
> >> Input: wacom - switch from an USB driver to a HID driver
> >>
> >>
> >> Running 3.17.0-rc6, right after boot, this shows up in the log:
> >>
> >> [ 25.843484] wacom 0003:056A:00E6.0001: usb_submit_urb(ctrl) failed: -1
> >> [ 25.843531] wacom 0003:056A:00E6.0001: timeout initializing reports
> >> [ 25.843886] wacom 0003:056A:00E6.0001: hidraw0: USB HID v1.11 Device
> >> [Tablet ISD-V4] on usb-0000:00:1d.0-1.5/input0
> >> [ 25.846036] input: Wacom ISDv4 E6 Finger as
> >> /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.5/2-1.5:1.1/0003:056A:00E6.0002/input/input18
> >>
> >> When suspending, the screen goes off and about 8 seconds later, the
> >> machine actually suspends.
> >>
> >> Right after the (instant) resume, I can see the frame buffer I had when
> >> suspending (X screen) for about 10 seconds. During that time, no sysrq
> >> works. After this short period, the tty1-framebuffer appears, with
> >> messed up linebreaks (no carriage returns occur any more), and the
> >> capslock-led starts blinking. Sysrq keys still don't work. The system is
> >> completely frozen.
> >>
> >> Running the kernel from the bisected first good commit, the suspend is
> >> faster (under 1 second) and the resume does not crash.
> >>
> >> I just saw the "HID: wacom: fix timeout on probe for some wacoms" mail,
> >> which probably has to do something with my problem.
> >>
> >> Any ideas?
> >
> > Yes, this commit will fix your problem. It has just been delayed for
> > 3.17.1 as mentioned by Jiri today.
>
> Are you sure it's a good idea to let the kernel crash into 3.17.0 and
> fix it later in stable? I suspect many X220t users could be affected.
Okay, my initial understanding was that this is just about the 10s delay.
How exactly does it crash on resume, and do we have any idea why it
crashes, and why "HID: wacom: fix timeout on probe for some wacoms" fixes
it?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] mfd: ti_am335x_tscadc: Update logic in CTRL register for 5-wire TS
From: Lee Jones @ 2014-09-24 11:26 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: linux-kernel, linux-input, Felipe Balbi, linux-omap, Samuel Ortiz,
Jeff Lance
In-Reply-To: <1409850117-5156-1-git-send-email-bigeasy@linutronix.de>
On Thu, 04 Sep 2014, Sebastian Andrzej Siewior wrote:
> From: Jeff Lance <j-lance1@ti.com>
>
> The logic in AFE_Pen_Ctrl bitmask in the CTRL register is different for five
> wire versus four or eight wire touschscreens. This patch should fix this for
> five-wire touch screens. There should be no change needed here for four and
> eight wire tousch screens.
>
> Signed-off-by: Jeff Lance <j-lance1@ti.com>
> [bigeasy: keep the change mfd only]
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> I've been looking at moving this piece and the idle mode setting into the
> TSC driver. Kinda odd that I have to patch within the MFD driver for this
> TSC related change.
> Anyone disagrees?
>
> drivers/mfd/ti_am335x_tscadc.c | 30 +++++++++++++++++-------------
> include/linux/mfd/ti_am335x_tscadc.h | 1 +
> 2 files changed, 18 insertions(+), 13 deletions(-)
I hope this has been fully tested.
Looks like it could work though, so applied, thanks.
> diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c
> index dd4bf5816221..6c9cfb8c1d01 100644
> --- a/drivers/mfd/ti_am335x_tscadc.c
> +++ b/drivers/mfd/ti_am335x_tscadc.c
> @@ -241,18 +241,20 @@ static int ti_tscadc_probe(struct platform_device *pdev)
> tscadc_writel(tscadc, REG_CLKDIV, tscadc->clk_div);
>
> /* Set the control register bits */
> - ctrl = CNTRLREG_STEPCONFIGWRT |
> - CNTRLREG_STEPID;
> - if (tsc_wires > 0)
> - ctrl |= CNTRLREG_4WIRE | CNTRLREG_TSCENB;
> + ctrl = CNTRLREG_STEPCONFIGWRT | CNTRLREG_STEPID;
> tscadc_writel(tscadc, REG_CTRL, ctrl);
>
> /* Set register bits for Idle Config Mode */
> - if (tsc_wires > 0)
> + if (tsc_wires > 0) {
> + tscadc->tsc_wires = tsc_wires;
> + if (tsc_wires == 5)
> + ctrl |= CNTRLREG_5WIRE | CNTRLREG_TSCENB;
> + else
> + ctrl |= CNTRLREG_4WIRE | CNTRLREG_TSCENB;
> tscadc_idle_config(tscadc);
> + }
>
> /* Enable the TSC module enable bit */
> - ctrl = tscadc_readl(tscadc, REG_CTRL);
> ctrl |= CNTRLREG_TSCSSENB;
> tscadc_writel(tscadc, REG_CTRL, ctrl);
>
> @@ -324,21 +326,23 @@ static int tscadc_suspend(struct device *dev)
> static int tscadc_resume(struct device *dev)
> {
> struct ti_tscadc_dev *tscadc_dev = dev_get_drvdata(dev);
> - unsigned int restore, ctrl;
> + u32 ctrl;
>
> pm_runtime_get_sync(dev);
>
> /* context restore */
> ctrl = CNTRLREG_STEPCONFIGWRT | CNTRLREG_STEPID;
> - if (tscadc_dev->tsc_cell != -1)
> - ctrl |= CNTRLREG_TSCENB | CNTRLREG_4WIRE;
> tscadc_writel(tscadc_dev, REG_CTRL, ctrl);
>
> - if (tscadc_dev->tsc_cell != -1)
> + if (tscadc_dev->tsc_cell != -1) {
> + if (tscadc_dev->tsc_wires == 5)
> + ctrl |= CNTRLREG_5WIRE | CNTRLREG_TSCENB;
> + else
> + ctrl |= CNTRLREG_4WIRE | CNTRLREG_TSCENB;
> tscadc_idle_config(tscadc_dev);
> - restore = tscadc_readl(tscadc_dev, REG_CTRL);
> - tscadc_writel(tscadc_dev, REG_CTRL,
> - (restore | CNTRLREG_TSCSSENB));
> + }
> + ctrl |= CNTRLREG_TSCSSENB;
> + tscadc_writel(tscadc_dev, REG_CTRL, ctrl);
>
> tscadc_writel(tscadc_dev, REG_CLKDIV, tscadc_dev->clk_div);
>
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index fb96c84dada5..e2e70053470e 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -155,6 +155,7 @@ struct ti_tscadc_dev {
> void __iomem *tscadc_base;
> int irq;
> int used_cells; /* 1-2 */
> + int tsc_wires;
> int tsc_cell; /* -1 if not used */
> int adc_cell; /* -1 if not used */
> struct mfd_cell cells[TSCADC_CELLS];
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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
^ permalink raw reply
* [PATCH] hid: sony: Update the DualShock 4 touchpad resolution
From: Frank Praznik @ 2014-09-24 13:38 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, Frank Praznik
The DualShock 4 touchpad has been measured to have a resolution of
44.86 dots/mm which equates to 1920x942.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
drivers/hid/hid-sony.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 5baa311..bc4269e 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1107,10 +1107,10 @@ static void sony_input_configured(struct hid_device *hdev,
/*
* The Dualshock 4 touchpad supports 2 touches and has a
- * resolution of 1920x940.
+ * resolution of 1920x942 (44.86 dots/mm).
*/
if (sc->quirks & DUALSHOCK4_CONTROLLER) {
- if (sony_register_touchpad(hidinput, 2, 1920, 940) != 0)
+ if (sony_register_touchpad(hidinput, 2, 1920, 942) != 0)
hid_err(sc->hdev,
"Unable to initialize multi-touch slots\n");
}
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] Input: driver for the Goodix touchpanel
From: Bastien Nocera @ 2014-09-24 14:05 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Henrik Rydberg, linux-input, Benjamin Tissoires
In-Reply-To: <20140924004510.GC16747@core.coreip.homeip.net>
On Tue, 2014-09-23 at 17:45 -0700, Dmitry Torokhov wrote:
> Hi Bastien,
Hey Dmitry,
As a reference, Benjamin and I have a downstream/stand-alone repository
for the driver, as we needed to greatly modify the original Android
driver.
I'm making all the changes there before pushing them for review to you,
which might make it easier for you to review.
See: https://github.com/hadess/gt9xx
<snip>
> > +#define GOODIX_INFO(fmt, arg...) pr_info("<<-GTP-INFO->> "fmt"\n", ##arg)
> > +#define GOODIX_ERROR(fmt, arg...) pr_err("<<-GTP-ERROR->> "fmt"\n", ##arg)
>
> Let's use standard dev_xxx()
Done.
> > +
> > +static const char *goodix_ts_name = "Goodix Capacitive TouchScreen";
> > +static const unsigned long goodix_irq_flags[] = {
> > + IRQ_TYPE_EDGE_RISING | IRQF_ONESHOT,
> > + IRQ_TYPE_EDGE_FALLING | IRQF_ONESHOT,
> > + IRQ_TYPE_LEVEL_LOW | IRQF_ONESHOT,
> > + IRQ_TYPE_LEVEL_HIGH | IRQF_ONESHOT
> > +};
>
> I'd rather you pulled out IRQF_ONESHOT and specified it explicitly in
> request_threaded_irq().
Done.
> > +
> > +/**
> > + * goodix_i2c_read - read data from a register of the i2c slave device.
> > + *
> > + * @client: i2c device.
> > + * @reg: the register to read from.
> > + * @buf: raw write data buffer.
> > + * @len: length of the buffer to write
> > + */
> > +static int goodix_i2c_read(struct i2c_client *client,
> > + u16 reg, u8 *buf, int len)
> > +{
> > + struct i2c_msg msgs[2];
> > + u8 wbuf[2] = { reg >> 8, reg & 0xff };
>
> cpu_to_be16()?
Fixed (hopefully)
> > +
> > + msgs[0].flags = !I2C_M_RD;
>
> I2C_M_RD is not a boolean, do not negate it.
Done.
> > + msgs[0].addr = client->addr;
> > + msgs[0].len = 2;
> > + msgs[0].buf = wbuf;
> > +
> > + msgs[1].flags = I2C_M_RD;
> > + msgs[1].addr = client->addr;
> > + msgs[1].len = len;
> > + msgs[1].buf = buf;
> > +
> > + return i2c_transfer(client->adapter, msgs, 2);
>
> return ret < 0 ? ret : (ret != ARRAY_SIZE(msgs) ? -EIO : 0);
>
> I really need to get that i2c_transfer_exact() going again.
Done.
> > +}
> > +
> > +/**
> > + * goodix_i2c_write - write data to the i2c slave device.
> > + *
> > + * @client: i2c device.
> > + * @reg: the register to read to.
> > + * @buf: raw write data buffer.
> > + * @len: length of the buffer to write
> > + */
> > +static int goodix_i2c_write(struct i2c_client *client,
> > + u16 reg, u8 *buf, int len)
> > +{
> > + struct i2c_msg msg;
> > + int ret;
> > + u8 *wbuf;
> > +
> > + wbuf = kzalloc(len + 2, GFP_KERNEL);
> > + if (!buf)
> > + return -ENOMEM;
> > +
> > + wbuf[0] = reg >> 8;
> > + wbuf[1] = reg & 0xFF;
>
> Again cpu_to_be16().
Done.
> > + memcpy(&wbuf[2], buf, len);
> > +
> > + msg.flags = !I2C_M_RD;
>
> Same here, not boolean.
Done.
<snip>
> > +/**
> > + * goodix_ts_work_func - Process incoming IRQ
> > + *
> > + * @ts: our goodix_ts_data pointer
> > + *
> > + * Called when the IRQ is triggered. Read the current device state, and push
> > + * the input events to the user space.
> > + */
> > +static void goodix_ts_work_func(struct goodix_ts_data *ts)
> > +{
> > + u8 end_cmd[1] = {0};
> > + u8 point_data[1 + 8 * GOODIX_MAX_TOUCH + 1];
> > + int touch_num;
> > + int i;
> > +
> > + touch_num = goodix_ts_read_input_report(ts, point_data);
> > + if (touch_num < 0)
> > + goto exit_work_func;
> > +
> > + for (i = 0; i < touch_num; i++)
> > + goodix_ts_parse_touch(ts, &point_data[1 + 8 * i]);
> > +
> > + input_mt_sync_frame(ts->input_dev);
> > + input_sync(ts->input_dev);
> > +
> > +exit_work_func:
> > + if (goodix_i2c_write(ts->client,
> > + GOODIX_READ_COOR_ADDR, end_cmd, 1) < 0)
> > + GOODIX_INFO("I2C write end_cmd error");
>
> Why did you need to pull it out of goodix_ts_irq_handler()?
True, moved now.
> > +}
<snip>
> > +/**
> > + * goodix_read_version - Read goodix touchscreen version
> > + *
> > + * @client: the i2c client
> > + * @version: output buffer containing the version on success
> > + */
> > +static int goodix_read_version(struct i2c_client *client, u16 *version)
> > +{
> > + int ret;
> > + int i;
> > + u8 buf[6];
> > +
> > + ret = goodix_i2c_read(client, GOODIX_REG_VERSION, buf, sizeof(buf));
> > + if (ret < 0) {
> > + GOODIX_ERROR("GTP read version failed");
> > + return ret;
> > + }
> > +
> > + if (version)
> > + *version = get_unaligned_le16(&buf[4]);
> > +
> > + for (i = 0; i < 4; i++) {
> > + if (!buf[i])
> > + buf[i] = 0x30;
>
> buf[i] = '0';
>
> but what if they happen to have some other nonprintable garbage there?
True. Not sure what to do here, Benjamin?
> > + }
> > + GOODIX_INFO("IC VERSION: %c%c%c%c_%02x%02x",
> > + buf[0], buf[1], buf[2], buf[3], buf[5], buf[4]);
> > +
> > + return ret;
> > +}
<snip>
> > +/**
> > + * goodix_request_irq - Request the IRQ handler
> > + *
> > + * @ts: our goodix_ts_data pointer
> > + *
> > + * Must be called during probe
> > + */
> > +static int goodix_request_irq(struct goodix_ts_data *ts)
> > +{
> > + int ret;
> > +
> > + ret = devm_request_threaded_irq(&ts->client->dev,
> > + goodix_i2c_writets->client->irq, NULL,
> > + goodix_ts_irq_handler,
> > + goodix_irq_flags[ts->int_trigger_type],
> > + ts->client->name,
> > + ts);
> > + if (ret) {
> > + GOODIX_ERROR("Request IRQ failed! ERRNO:%d.", ret);
> > + return -1;
> > + }
> > +
> > + disable_irq_nosync(ts->client->irq);
>
> Why nosync? And why do you need to disable? Yo just need to request it
> later, after you've done querying the device. And why a separate funcion
> for basically one statement?
Removed, and merged into the calling function.
> > + return 0;
> > +}
> > +
> > +/**
> > + * goodix_request_input_dev - Allocate, populate and register the input device
> > + *
> > + * @ts: our goodix_ts_data pointer
> > + *
> > + * Must be called during probe
> > + */
> > +static int goodix_request_input_dev(struct goodix_ts_data *ts)
> > +{
> > + int ret;
> > +
> > + ts->input_dev = devm_input_allocate_device(&ts->client->dev);
> > + if (ts->input_dev == NULL) {
> > + GOODIX_ERROR("Failed to allocate input device.");
> > + return -ENOMEM;
> > + }
> > +
> > + ts->input_dev->evbit[0] = BIT_MASK(EV_SYN) |
> > + BIT_MASK(EV_KEY) |
> > + BIT_MASK(EV_ABS);
> > +
> > + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, 0,
> > + ts->abs_x_max, 0, 0);
> > + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, 0,
> > + ts->abs_y_max, 0, 0);
> > + input_set_abs_params(ts->input_dev, ABS_MT_WIDTH_MAJOR, 0, 255, 0, 0);
> > + input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
> > +
> > + input_mt_init_slots(ts->input_dev, GOODIX_MAX_TOUCH,
> > + INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
> > +
> > + ts->input_dev->name = goodix_ts_name;
> > + ts->input_dev->phys = "input/ts";
> > + ts->input_dev->id.bustype = BUS_I2C;
> > + ts->input_dev->id.vendor = 0x0416;
> > + ts->input_dev->id.product = 0x1001;
> > + ts->input_dev->id.version = 10427;
> > +
> > + ret = input_register_device(ts->input_dev);
> > + if (ret) {
> > + GOODIX_ERROR("Failed to register %s input device",
> > + ts->input_dev->name);
> > + return -ENODEV;
>
> Why -ENODEV instead of ret?
Fixed.
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int goodix_ts_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + int ret;
> > + struct goodix_ts_data *ts;
> > + u16 version_info;
> > +
> > + GOODIX_INFO("GTP I2C Address: 0x%02x", client->addr);
> > +
> > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> > + GOODIX_ERROR("I2C check functionality failed.");
> > + return -ENODEV;
> > + }
> > + ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
> > + if (ts == NULL) {
>
> if (!ts)
>
> is a preferred form (by me at least).
Fixed here and in goodix_request_input_dev().
> > + GOODIX_ERROR("Alloc GFP_KERNEL memory failed.");
> > + return -ENOMEM;
> > + }
> > +
> > + ts->client = client;
> > + i2c_set_clientdata(client, ts);
> > +
> > + ret = goodix_i2c_test(client);
> > + if (ret < 0) {
> > + client->addr = 0x5d;
>
> Umm, why? Let's trust board code/i2c core to configure the i2c device
> properly.
Done.
> > + ret = goodix_i2c_test(client);
> > + if (ret < 0) {
> > + GOODIX_ERROR("I2C communication ERROR!");
> > + return -ENODEV;
> > + }
> > + GOODIX_INFO("GTP I2C new Address: 0x%02x", client->addr);
> > + }
> > +
> > + goodix_read_config(ts);
> > +
> > + ret = goodix_request_input_dev(ts);
> > + if (ret < 0) {
> > + GOODIX_ERROR("GTP request input dev failed");
> > + return ret;
> > + }
> > +
> > + ret = goodix_request_irq(ts);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = goodix_read_version(client, &version_info);
> > + if (ret < 0) {
> > + GOODIX_ERROR("Read version failed.");
> > + return ret;
> > + }
> > +
> > + enable_irq(ts->client->irq);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct i2c_device_id goodix_ts_id[] = {
> > + { "GDIX1001:00", 0 },
> > + { }
>
> Do we have to have this table?
Yep, otherwise the driver won't detect the device. Benjamin tried to
remove it as well ;)
<snip>
>
> Thanks.
Thank YOU for the thorough review.
^ permalink raw reply
* Re: [PATCH] Input: driver for the Goodix touchpanel
From: Benjamin Tissoires @ 2014-09-24 14:14 UTC (permalink / raw)
To: Bastien Nocera; +Cc: Dmitry Torokhov, Henrik Rydberg, linux-input
In-Reply-To: <1411567548.29315.19.camel@hadess.net>
Hi Dmitry,
Thanks for the review. As a foreword, most of the things you mentioned
were left over from the original driver that we did not spot. Anyway,
enough of excuses, let's go in details.
On Wed, Sep 24, 2014 at 10:05 AM, Bastien Nocera <hadess@hadess.net> wrote:
> On Tue, 2014-09-23 at 17:45 -0700, Dmitry Torokhov wrote:
>> Hi Bastien,
>
> Hey Dmitry,
>
> As a reference, Benjamin and I have a downstream/stand-alone repository
> for the driver, as we needed to greatly modify the original Android
> driver.
>
> I'm making all the changes there before pushing them for review to you,
> which might make it easier for you to review.
>
> See: https://github.com/hadess/gt9xx
>
> <snip>
>> > +#define GOODIX_INFO(fmt, arg...) pr_info("<<-GTP-INFO->> "fmt"\n", ##arg)
>> > +#define GOODIX_ERROR(fmt, arg...) pr_err("<<-GTP-ERROR->> "fmt"\n", ##arg)
>>
>> Let's use standard dev_xxx()
>
> Done.
Yep. I was not sure about keeping that or not, but we can drop the
defines entirely and rely on dev_xxx as you suggest.
>
>> > +
>> > +static const char *goodix_ts_name = "Goodix Capacitive TouchScreen";
>> > +static const unsigned long goodix_irq_flags[] = {
>> > + IRQ_TYPE_EDGE_RISING | IRQF_ONESHOT,
>> > + IRQ_TYPE_EDGE_FALLING | IRQF_ONESHOT,
>> > + IRQ_TYPE_LEVEL_LOW | IRQF_ONESHOT,
>> > + IRQ_TYPE_LEVEL_HIGH | IRQF_ONESHOT
>> > +};
>>
>> I'd rather you pulled out IRQF_ONESHOT and specified it explicitly in
>> request_threaded_irq().
>
> Done.
>
>> > +
>> > +/**
>> > + * goodix_i2c_read - read data from a register of the i2c slave device.
>> > + *
>> > + * @client: i2c device.
>> > + * @reg: the register to read from.
>> > + * @buf: raw write data buffer.
>> > + * @len: length of the buffer to write
>> > + */
>> > +static int goodix_i2c_read(struct i2c_client *client,
>> > + u16 reg, u8 *buf, int len)
>> > +{
>> > + struct i2c_msg msgs[2];
>> > + u8 wbuf[2] = { reg >> 8, reg & 0xff };
>>
>> cpu_to_be16()?
>
> Fixed (hopefully)
You should be more confident Bastien :)
>
>> > +
>> > + msgs[0].flags = !I2C_M_RD;
>>
>> I2C_M_RD is not a boolean, do not negate it.
Ouch, I have seen that and forgot about it. We will set the flags to 0 directly.
>
> Done.
>
>> > + msgs[0].addr = client->addr;
>> > + msgs[0].len = 2;
>> > + msgs[0].buf = wbuf;
>> > +
>> > + msgs[1].flags = I2C_M_RD;
>> > + msgs[1].addr = client->addr;
>> > + msgs[1].len = len;
>> > + msgs[1].buf = buf;
>> > +
>> > + return i2c_transfer(client->adapter, msgs, 2);
>>
>> return ret < 0 ? ret : (ret != ARRAY_SIZE(msgs) ? -EIO : 0);
>>
>> I really need to get that i2c_transfer_exact() going again.
>
> Done.
>
>> > +}
>> > +
>> > +/**
>> > + * goodix_i2c_write - write data to the i2c slave device.
>> > + *
>> > + * @client: i2c device.
>> > + * @reg: the register to read to.
>> > + * @buf: raw write data buffer.
>> > + * @len: length of the buffer to write
>> > + */
>> > +static int goodix_i2c_write(struct i2c_client *client,
>> > + u16 reg, u8 *buf, int len)
>> > +{
>> > + struct i2c_msg msg;
>> > + int ret;
>> > + u8 *wbuf;
>> > +
>> > + wbuf = kzalloc(len + 2, GFP_KERNEL);
>> > + if (!buf)
>> > + return -ENOMEM;
>> > +
>> > + wbuf[0] = reg >> 8;
>> > + wbuf[1] = reg & 0xFF;
>>
>> Again cpu_to_be16().
>
> Done.
>
>> > + memcpy(&wbuf[2], buf, len);
>> > +
>> > + msg.flags = !I2C_M_RD;
>>
>> Same here, not boolean.
>
sigh. sorry.
> Done.
>
> <snip>
>> > +/**
>> > + * goodix_ts_work_func - Process incoming IRQ
>> > + *
>> > + * @ts: our goodix_ts_data pointer
>> > + *
>> > + * Called when the IRQ is triggered. Read the current device state, and push
>> > + * the input events to the user space.
>> > + */
>> > +static void goodix_ts_work_func(struct goodix_ts_data *ts)
>> > +{
>> > + u8 end_cmd[1] = {0};
>> > + u8 point_data[1 + 8 * GOODIX_MAX_TOUCH + 1];
>> > + int touch_num;
>> > + int i;
>> > +
>> > + touch_num = goodix_ts_read_input_report(ts, point_data);
>> > + if (touch_num < 0)
>> > + goto exit_work_func;
>> > +
>> > + for (i = 0; i < touch_num; i++)
>> > + goodix_ts_parse_touch(ts, &point_data[1 + 8 * i]);
>> > +
>> > + input_mt_sync_frame(ts->input_dev);
>> > + input_sync(ts->input_dev);
>> > +
>> > +exit_work_func:
>> > + if (goodix_i2c_write(ts->client,
>> > + GOODIX_READ_COOR_ADDR, end_cmd, 1) < 0)
>> > + GOODIX_INFO("I2C write end_cmd error");
>>
>> Why did you need to pull it out of goodix_ts_irq_handler()?
I can surely get a very good reason, but I will have to digg too much
for it to be acceptable :)
We will drop goodix_ts_work_func() (I did not liked the name BTW) and
merge the code in the irq handler directly.
>
> True, moved now.
>
>> > +}
> <snip>
>> > +/**
>> > + * goodix_read_version - Read goodix touchscreen version
>> > + *
>> > + * @client: the i2c client
>> > + * @version: output buffer containing the version on success
>> > + */
>> > +static int goodix_read_version(struct i2c_client *client, u16 *version)
>> > +{
>> > + int ret;
>> > + int i;
>> > + u8 buf[6];
>> > +
>> > + ret = goodix_i2c_read(client, GOODIX_REG_VERSION, buf, sizeof(buf));
>> > + if (ret < 0) {
>> > + GOODIX_ERROR("GTP read version failed");
>> > + return ret;
>> > + }
>> > +
>> > + if (version)
>> > + *version = get_unaligned_le16(&buf[4]);
>> > +
>> > + for (i = 0; i < 4; i++) {
>> > + if (!buf[i])
>> > + buf[i] = 0x30;
>>
>> buf[i] = '0';
>>
>> but what if they happen to have some other nonprintable garbage there?
>
> True. Not sure what to do here, Benjamin?
So at first i was thinking at just using %*ph in the following
message, but then I realized about the char part of it.
I guess we will just drop the previous for loop, and have the message as follow:
dev_info(..., "IC VERSION: %6ph", buf);
>
>> > + }
>> > + GOODIX_INFO("IC VERSION: %c%c%c%c_%02x%02x",
>> > + buf[0], buf[1], buf[2], buf[3], buf[5], buf[4]);
>> > +
>> > + return ret;
>> > +}
> <snip>
>> > +/**
>> > + * goodix_request_irq - Request the IRQ handler
>> > + *
>> > + * @ts: our goodix_ts_data pointer
>> > + *
>> > + * Must be called during probe
>> > + */
>> > +static int goodix_request_irq(struct goodix_ts_data *ts)
>> > +{
>> > + int ret;
>> > +
>> > + ret = devm_request_threaded_irq(&ts->client->dev,
>> > + goodix_i2c_writets->client->irq, NULL,
>> > + goodix_ts_irq_handler,
>> > + goodix_irq_flags[ts->int_trigger_type],
>> > + ts->client->name,
>> > + ts);
>> > + if (ret) {
>> > + GOODIX_ERROR("Request IRQ failed! ERRNO:%d.", ret);
>> > + return -1;
>> > + }
>> > +
>> > + disable_irq_nosync(ts->client->irq);
>>
>> Why nosync? And why do you need to disable? Yo just need to request it
>> later, after you've done querying the device. And why a separate funcion
>> for basically one statement?
>
> Removed, and merged into the calling function.
>
>> > + return 0;
>> > +}
>> > +
>> > +/**
>> > + * goodix_request_input_dev - Allocate, populate and register the input device
>> > + *
>> > + * @ts: our goodix_ts_data pointer
>> > + *
>> > + * Must be called during probe
>> > + */
>> > +static int goodix_request_input_dev(struct goodix_ts_data *ts)
>> > +{
>> > + int ret;
>> > +
>> > + ts->input_dev = devm_input_allocate_device(&ts->client->dev);
>> > + if (ts->input_dev == NULL) {
>> > + GOODIX_ERROR("Failed to allocate input device.");
>> > + return -ENOMEM;
>> > + }
>> > +
>> > + ts->input_dev->evbit[0] = BIT_MASK(EV_SYN) |
>> > + BIT_MASK(EV_KEY) |
>> > + BIT_MASK(EV_ABS);
>> > +
>> > + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, 0,
>> > + ts->abs_x_max, 0, 0);
>> > + input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, 0,
>> > + ts->abs_y_max, 0, 0);
>> > + input_set_abs_params(ts->input_dev, ABS_MT_WIDTH_MAJOR, 0, 255, 0, 0);
>> > + input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
>> > +
>> > + input_mt_init_slots(ts->input_dev, GOODIX_MAX_TOUCH,
>> > + INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
>> > +
>> > + ts->input_dev->name = goodix_ts_name;
>> > + ts->input_dev->phys = "input/ts";
>> > + ts->input_dev->id.bustype = BUS_I2C;
>> > + ts->input_dev->id.vendor = 0x0416;
>> > + ts->input_dev->id.product = 0x1001;
>> > + ts->input_dev->id.version = 10427;
>> > +
>> > + ret = input_register_device(ts->input_dev);
>> > + if (ret) {
>> > + GOODIX_ERROR("Failed to register %s input device",
>> > + ts->input_dev->name);
>> > + return -ENODEV;
>>
>> Why -ENODEV instead of ret?
>
> Fixed.
>
>> > + }
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int goodix_ts_probe(struct i2c_client *client,
>> > + const struct i2c_device_id *id)
>> > +{
>> > + int ret;
>> > + struct goodix_ts_data *ts;
>> > + u16 version_info;
>> > +
>> > + GOODIX_INFO("GTP I2C Address: 0x%02x", client->addr);
>> > +
>> > + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>> > + GOODIX_ERROR("I2C check functionality failed.");
>> > + return -ENODEV;
>> > + }
>> > + ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
>> > + if (ts == NULL) {
>>
>> if (!ts)
>>
>> is a preferred form (by me at least).
>
> Fixed here and in goodix_request_input_dev().
>
>> > + GOODIX_ERROR("Alloc GFP_KERNEL memory failed.");
>> > + return -ENOMEM;
>> > + }
>> > +
>> > + ts->client = client;
>> > + i2c_set_clientdata(client, ts);
>> > +
>> > + ret = goodix_i2c_test(client);
>> > + if (ret < 0) {
>> > + client->addr = 0x5d;
>>
>> Umm, why? Let's trust board code/i2c core to configure the i2c device
>> properly.
This is also a left over from the original driver. They used to change
the address in case of a reset of the device to check if it was in
bootloader mode or not. We just missed this one.
>
> Done.
>
>> > + ret = goodix_i2c_test(client);
>> > + if (ret < 0) {
>> > + GOODIX_ERROR("I2C communication ERROR!");
>> > + return -ENODEV;
>> > + }
>> > + GOODIX_INFO("GTP I2C new Address: 0x%02x", client->addr);
>> > + }
>> > +
>> > + goodix_read_config(ts);
>> > +
>> > + ret = goodix_request_input_dev(ts);
>> > + if (ret < 0) {
>> > + GOODIX_ERROR("GTP request input dev failed");
>> > + return ret;
>> > + }
>> > +
>> > + ret = goodix_request_irq(ts);
>> > + if (ret < 0)
>> > + return ret;
>> > +
>> > + ret = goodix_read_version(client, &version_info);
>> > + if (ret < 0) {
>> > + GOODIX_ERROR("Read version failed.");
>> > + return ret;
>> > + }
>> > +
>> > + enable_irq(ts->client->irq);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static const struct i2c_device_id goodix_ts_id[] = {
>> > + { "GDIX1001:00", 0 },
>> > + { }
>>
>> Do we have to have this table?
>
> Yep, otherwise the driver won't detect the device. Benjamin tried to
> remove it as well ;)
>
> <snip>
>>
>> Thanks.
>
> Thank YOU for the thorough review.
>
Cheers,
Benjamin
^ permalink raw reply
* [PATCH v2] Input: driver for the Goodix touchpanel
From: Bastien Nocera @ 2014-09-24 14:43 UTC (permalink / raw)
To: Dmitry Torokhov, Henrik Rydberg, linux-input; +Cc: Benjamin Tissoires
Add a driver for the Goodix touchscreen panel found in Onda v975w
tablets. The driver is based off the Android driver gt9xx.c found
in some Android code dumps, but now bears no resemblance to the original
driver.
The driver was tested on the aforementioned tablet.
Signed-off-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Tested-by: Bastien Nocera <hadess@hadess.net>
---
MAINTAINERS | 6 +
drivers/input/touchscreen/Kconfig | 13 ++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix.c | 423 +++++++++++++++++++++++++++++++++++++
4 files changed, 443 insertions(+)
create mode 100644 drivers/input/touchscreen/goodix.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 670b3dc..7a37464 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4057,6 +4057,12 @@ L: linux-media@vger.kernel.org
S: Maintained
F: drivers/media/usb/go7007/
+GOODIX TOUCHSCREEN
+M: Bastien Nocera <hadess@hadess.net>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/input/touchscreen/goodix.c
+
GPIO SUBSYSTEM
M: Linus Walleij <linus.walleij@linaro.org>
M: Alexandre Courbot <gnurou@gmail.com>
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 6bb9a7d..c328df3 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -283,6 +283,19 @@ config TOUCHSCREEN_FUJITSU
To compile this driver as a module, choose M here: the
module will be called fujitsu-ts.
+config TOUCHSCREEN_GOODIX
+ tristate "Goodix I2C touchscreen"
+ depends on I2C && ACPI
+ help
+ Say Y here if you have the Goodix touchscreen (such as one
+ installed in Onda v975w tablets) connected to your
+ system.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix.
+
config TOUCHSCREEN_ILI210X
tristate "Ilitek ILI210X based touchscreen"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 4be94fc..55af212 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_TOUCHSCREEN_EETI) += eeti_ts.o
obj-$(CONFIG_TOUCHSCREEN_ELO) += elo.o
obj-$(CONFIG_TOUCHSCREEN_EGALAX) += egalax_ts.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
obj-$(CONFIG_TOUCHSCREEN_INEXIO) += inexio.o
obj-$(CONFIG_TOUCHSCREEN_INTEL_MID) += intel-mid-touch.o
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
new file mode 100644
index 0000000..fd02b5e
--- /dev/null
+++ b/drivers/input/touchscreen/goodix.c
@@ -0,0 +1,423 @@
+/*
+ * driver for Goodix Touchscreens
+ *
+ * Copyright (c) 2014 Red Hat Inc.
+ *
+ * This code is based on gt9xx.c authored by andrew@goodix.com:
+ *
+ * 2010 - 2012 Goodix Technology.
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; version 2 of the License.
+ */
+
+#include <linux/kernel.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+#include <asm/unaligned.h>
+
+struct goodix_ts_data {
+ struct i2c_client *client;
+ struct input_dev *input_dev;
+ int abs_x_max;
+ int abs_y_max;
+ unsigned int max_touch_num;
+ unsigned int int_trigger_type;
+};
+
+#define GOODIX_MAX_HEIGHT 4096
+#define GOODIX_MAX_WIDTH 4096
+#define GOODIX_INT_TRIGGER 1
+#define GOODIX_MAX_TOUCH 10
+
+#define GOODIX_CONFIG_MAX_LENGTH 240
+
+/* Register defineS */
+#define GOODIX_READ_COOR_ADDR 0x814E
+#define GOODIX_REG_CONFIG_DATA 0x8047
+#define GOODIX_REG_VERSION 0x8140
+
+#define RESOLUTION_LOC 1
+#define TRIGGER_LOC 6
+
+static const char *goodix_ts_name = "Goodix Capacitive TouchScreen";
+static const unsigned long goodix_irq_flags[] = {
+ IRQ_TYPE_EDGE_RISING,
+ IRQ_TYPE_EDGE_FALLING,
+ IRQ_TYPE_LEVEL_LOW,
+ IRQ_TYPE_LEVEL_HIGH,
+};
+
+/**
+ * goodix_i2c_read - read data from a register of the i2c slave device.
+ *
+ * @client: i2c device.
+ * @reg: the register to read from.
+ * @buf: raw write data buffer.
+ * @len: length of the buffer to write
+ */
+static int goodix_i2c_read(struct i2c_client *client,
+ u16 reg, u8 *buf, int len)
+{
+ struct i2c_msg msgs[2];
+ u16 wbuf = cpu_to_be16(reg);
+ int ret;
+
+ msgs[0].flags = 0;
+ msgs[0].addr = client->addr;
+ msgs[0].len = 2;
+ msgs[0].buf = (u8 *) &wbuf;
+
+ msgs[1].flags = I2C_M_RD;
+ msgs[1].addr = client->addr;
+ msgs[1].len = len;
+ msgs[1].buf = buf;
+
+ ret = i2c_transfer(client->adapter, msgs, 2);
+ return ret < 0 ? ret : (ret != ARRAY_SIZE(msgs) ? -EIO : 0);
+}
+
+/**
+ * goodix_i2c_write - write data to the i2c slave device.
+ *
+ * @client: i2c device.
+ * @reg: the register to read to.
+ * @buf: raw write data buffer.
+ * @len: length of the buffer to write
+ */
+static int goodix_i2c_write(struct i2c_client *client,
+ u16 reg, u8 *buf, int len)
+{
+ struct i2c_msg msg;
+ int ret;
+ u8 *wbuf;
+ u16 *addr;
+
+ wbuf = kzalloc(len + 2, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ addr = ((u16 *) &wbuf[0]);
+ *addr = cpu_to_be16(reg);
+ memcpy(&wbuf[2], buf, len);
+
+ msg.flags = 0;
+ msg.addr = client->addr;
+ msg.len = len + 2;
+ msg.buf = wbuf;
+
+ ret = i2c_transfer(client->adapter, &msg, 1);
+ kfree(wbuf);
+ return ret < 0 ? ret : (ret != 1 ? -EIO : 0);
+}
+
+static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
+{
+ int touch_num;
+ int ret;
+
+ ret = goodix_i2c_read(ts->client, GOODIX_READ_COOR_ADDR, data, 10);
+ if (ret < 0) {
+ dev_err(&ts->client->dev, "I2C transfer error (%d)\n", ret);
+ return ret;
+ }
+
+ touch_num = data[0] & 0x0f;
+ if (touch_num > GOODIX_MAX_TOUCH)
+ return -EPROTO;
+
+ if (touch_num > 1) {
+ ret = goodix_i2c_read(ts->client, GOODIX_READ_COOR_ADDR + 10,
+ &data[10], 8 * (touch_num - 1));
+ if (ret < 0)
+ return ret;
+ }
+
+ return touch_num;
+}
+
+static void goodix_ts_parse_touch(struct goodix_ts_data *ts, u8 *coor_data)
+{
+ int id = coor_data[0] & 0x0F;
+ int input_x = get_unaligned_le16(&coor_data[1]);
+ int input_y = get_unaligned_le16(&coor_data[3]);
+ int input_w = get_unaligned_le16(&coor_data[5]);
+
+ input_mt_slot(ts->input_dev, id);
+ input_mt_report_slot_state(ts->input_dev, MT_TOOL_FINGER, true);
+ input_report_abs(ts->input_dev, ABS_MT_POSITION_X, input_x);
+ input_report_abs(ts->input_dev, ABS_MT_POSITION_Y, input_y);
+ input_report_abs(ts->input_dev, ABS_MT_TOUCH_MAJOR, input_w);
+ input_report_abs(ts->input_dev, ABS_MT_WIDTH_MAJOR, input_w);
+}
+
+/**
+ * goodix_process_events - Process incoming events
+ *
+ * @ts: our goodix_ts_data pointer
+ *
+ * Called when the IRQ is triggered. Read the current device state, and push
+ * the input events to the user space.
+ */
+static void goodix_process_events(struct goodix_ts_data *ts)
+{
+ u8 point_data[1 + 8 * GOODIX_MAX_TOUCH + 1];
+ int touch_num;
+ int i;
+
+ touch_num = goodix_ts_read_input_report(ts, point_data);
+ if (touch_num < 0)
+ return;
+
+ for (i = 0; i < touch_num; i++)
+ goodix_ts_parse_touch(ts, &point_data[1 + 8 * i]);
+
+ input_mt_sync_frame(ts->input_dev);
+ input_sync(ts->input_dev);
+}
+
+/**
+ * goodix_ts_irq_handler - The IRQ handler
+ *
+ * @irq: interrupt number.
+ * @dev_id: private data pointer.
+ */
+static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
+{
+ struct goodix_ts_data *ts = dev_id;
+ u8 end_cmd[1] = {0};
+
+ goodix_process_events(ts);
+
+ if (goodix_i2c_write(ts->client,
+ GOODIX_READ_COOR_ADDR, end_cmd, 1) < 0)
+ dev_err(&ts->client->dev, "I2C write end_cmd error");
+
+ return IRQ_HANDLED;
+}
+
+/**
+ * goodix_read_config - Read the embedded configuration of the panel
+ *
+ * @ts: our goodix_ts_data pointer
+ *
+ * Must be called during probe
+ */
+static void goodix_read_config(struct goodix_ts_data *ts)
+{
+ int ret;
+ u8 config[GOODIX_CONFIG_MAX_LENGTH];
+
+ ret = goodix_i2c_read(ts->client, GOODIX_REG_CONFIG_DATA, config,
+ GOODIX_CONFIG_MAX_LENGTH);
+ if (ret < 0) {
+ dev_err(&ts->client->dev,
+ "Error reading config, use default value!");
+ ts->abs_x_max = GOODIX_MAX_WIDTH;
+ ts->abs_y_max = GOODIX_MAX_HEIGHT;
+ ts->int_trigger_type = GOODIX_INT_TRIGGER;
+ return;
+ }
+
+ ts->abs_x_max = get_unaligned_le16(&config[RESOLUTION_LOC]);
+ ts->abs_y_max = get_unaligned_le16(&config[RESOLUTION_LOC + 2]);
+ ts->int_trigger_type = (config[TRIGGER_LOC]) & 0x03;
+ if ((!ts->abs_x_max) || (!ts->abs_y_max)) {
+ dev_err(&ts->client->dev,
+ "Invalid config, use default value!");
+ ts->abs_x_max = GOODIX_MAX_WIDTH;
+ ts->abs_y_max = GOODIX_MAX_HEIGHT;
+ }
+}
+
+
+/**
+ * goodix_read_version - Read goodix touchscreen version
+ *
+ * @client: the i2c client
+ * @version: output buffer containing the version on success
+ */
+static int goodix_read_version(struct i2c_client *client, u16 *version)
+{
+ int ret;
+ u8 buf[6];
+
+ ret = goodix_i2c_read(client, GOODIX_REG_VERSION, buf, sizeof(buf));
+ if (ret < 0) {
+ dev_err(&client->dev, "read version failed");
+ return ret;
+ }
+
+ if (version)
+ *version = get_unaligned_le16(&buf[4]);
+
+ dev_info(&client->dev, "IC VERSION: %6ph", buf);
+
+ return ret;
+}
+
+/**
+ * goodix_i2c_test - I2C test function to check if the device answers.
+ *
+ * @client: the i2c client
+ */
+static int goodix_i2c_test(struct i2c_client *client)
+{
+ u8 test;
+ int ret;
+ int retry = 0;
+
+ while (retry++ < 2) {
+ ret = goodix_i2c_read(client, GOODIX_REG_CONFIG_DATA,
+ &test, 1);
+ if (ret >= 0)
+ return ret;
+
+ dev_err(&client->dev, "i2c test failed time %d.", retry);
+ msleep(20);
+ }
+ return ret;
+}
+
+/**
+ * goodix_request_input_dev - Allocate, populate and register the input device
+ *
+ * @ts: our goodix_ts_data pointer
+ *
+ * Must be called during probe
+ */
+static int goodix_request_input_dev(struct goodix_ts_data *ts)
+{
+ int ret;
+
+ ts->input_dev = devm_input_allocate_device(&ts->client->dev);
+ if (!ts->input_dev) {
+ dev_err(&ts->client->dev, "Failed to allocate input device.");
+ return -ENOMEM;
+ }
+
+ ts->input_dev->evbit[0] = BIT_MASK(EV_SYN) |
+ BIT_MASK(EV_KEY) |
+ BIT_MASK(EV_ABS);
+
+ input_set_abs_params(ts->input_dev, ABS_MT_POSITION_X, 0,
+ ts->abs_x_max, 0, 0);
+ input_set_abs_params(ts->input_dev, ABS_MT_POSITION_Y, 0,
+ ts->abs_y_max, 0, 0);
+ input_set_abs_params(ts->input_dev, ABS_MT_WIDTH_MAJOR, 0, 255, 0, 0);
+ input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+
+ input_mt_init_slots(ts->input_dev, GOODIX_MAX_TOUCH,
+ INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+
+ ts->input_dev->name = goodix_ts_name;
+ ts->input_dev->phys = "input/ts";
+ ts->input_dev->id.bustype = BUS_I2C;
+ ts->input_dev->id.vendor = 0x0416;
+ ts->input_dev->id.product = 0x1001;
+ ts->input_dev->id.version = 10427;
+
+ ret = input_register_device(ts->input_dev);
+ if (ret) {
+ dev_err(&ts->client->dev, "Failed to register %s input device",
+ ts->input_dev->name);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int goodix_ts_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ int ret;
+ struct goodix_ts_data *ts;
+ u16 version_info;
+
+ dev_info(&client->dev, "I2C Address: 0x%02x", client->addr);
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+ dev_err(&client->dev, "I2C check functionality failed.");
+ return -ENODEV;
+ }
+ ts = devm_kzalloc(&client->dev, sizeof(*ts), GFP_KERNEL);
+ if (!ts) {
+ dev_err(&client->dev, "Alloc GFP_KERNEL memory failed.");
+ return -ENOMEM;
+ }
+
+ ts->client = client;
+ i2c_set_clientdata(client, ts);
+
+ ret = goodix_i2c_test(client);
+ if (ret < 0) {
+ dev_err(&client->dev, "I2C communication ERROR!");
+ return ret;
+ }
+
+ goodix_read_config(ts);
+
+ ret = goodix_request_input_dev(ts);
+ if (ret < 0) {
+ dev_err(&client->dev, "request input dev failed");
+ return ret;
+ }
+
+ ret = devm_request_threaded_irq(&ts->client->dev,
+ ts->client->irq, NULL,
+ goodix_ts_irq_handler,
+ goodix_irq_flags[ts->int_trigger_type]
+ | IRQF_ONESHOT,
+ ts->client->name,
+ ts);
+ if (ret < 0) {
+ dev_err(&ts->client->dev,
+ "Request IRQ failed! ERRNO: %d.", ret);
+ return ret;
+ }
+
+ ret = goodix_read_version(client, &version_info);
+ if (ret < 0) {
+ dev_err(&client->dev, "Read version failed.");
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct i2c_device_id goodix_ts_id[] = {
+ { "GDIX1001:00", 0 },
+ { }
+};
+
+static const struct acpi_device_id goodix_acpi_match[] = {
+ { "GDIX1001", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
+
+static struct i2c_driver goodix_ts_driver = {
+ .probe = goodix_ts_probe,
+ .id_table = goodix_ts_id,
+ .driver = {
+ .name = "Goodix-TS",
+ .owner = THIS_MODULE,
+ .acpi_match_table = goodix_acpi_match,
+ },
+};
+
+module_i2c_driver(goodix_ts_driver);
+
+MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
+MODULE_DESCRIPTION("Goodix touchscreen driver");
+MODULE_LICENSE("GPL v2");
--
2.1.0
^ permalink raw reply related
* [PATCH v3 0/2] Add ROHM BU21023/24 Dual touch support resistive touchscreens
From: Yoichi Yuasa @ 2014-09-24 15:16 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: yuasa, linux-input
Hi Dmitry,
I updated the driver according to your comments.
Do you have any comments?
v3 Changes:
- fix multi touch slots initialization
- fix set_bit BTN_TOUCH
- remove input_unregister_device()
- switch to __set_bit()
v2 Changes:
- remove polling mode
- switch to threaded interrupt
- switch to managed resources
- use MT-B protocol with input_mt_assign_slots
- provide ST emulation
- firmware load and device initialization are shifted at opening the device
- add error handling for IO operations
Yoichi Yuasa (2):
input: Add ROHM BU21023/24 Dual touch support resistive touchscreens
input: bu21023_ts: Add calibration function
drivers/input/touchscreen/Kconfig | 11 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/rohm_bu21023.c | 930 ++++++++++++++++++++++++++++++
drivers/input/touchscreen/rohm_bu21023.h | 258 +++++++++
4 files changed, 1200 insertions(+)
create mode 100644 drivers/input/touchscreen/rohm_bu21023.c
create mode 100644 drivers/input/touchscreen/rohm_bu21023.h
--
1.7.9.5
^ permalink raw reply
* [PATCH v3 1/2] input: Add ROHM BU21023/24 Dual touch support resistive touchscreens
From: Yoichi Yuasa @ 2014-09-24 15:18 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: yuasa, linux-input
In-Reply-To: <20140925001647.dd48571a3c0ceda5c25ab669@linux-mips.org>
Signed-off-by: Yoichi Yuasa <yuasa@linux-mips.org>
---
drivers/input/touchscreen/Kconfig | 11 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/rohm_bu21023.c | 680 ++++++++++++++++++++++++++++++
drivers/input/touchscreen/rohm_bu21023.h | 258 ++++++++++++
4 files changed, 950 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 6bb9a7d..9ae5c88 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -950,4 +950,15 @@ config TOUCHSCREEN_ZFORCE
To compile this driver as a module, choose M here: the
module will be called zforce_ts.
+config TOUCHSCREEN_ROHM_BU21023
+ tristate "ROHM BU21023/24 Dual touch support resistive touchscreens"
+ depends on I2C
+ help
+ Say Y here if you have a touchscreen using ROHM BU21023/24.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called bu21023_ts.
+
endif
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 4be94fc..af766d0 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_TOUCHSCREEN_USB_COMPOSITE) += usbtouchscreen.o
obj-$(CONFIG_TOUCHSCREEN_PCAP) += pcap_ts.o
obj-$(CONFIG_TOUCHSCREEN_PENMOUNT) += penmount.o
obj-$(CONFIG_TOUCHSCREEN_PIXCIR) += pixcir_i2c_ts.o
+obj-$(CONFIG_TOUCHSCREEN_ROHM_BU21023) += rohm_bu21023.o
obj-$(CONFIG_TOUCHSCREEN_S3C2410) += s3c2410_ts.o
obj-$(CONFIG_TOUCHSCREEN_ST1232) += st1232.o
obj-$(CONFIG_TOUCHSCREEN_STMPE) += stmpe-ts.o
diff --git a/drivers/input/touchscreen/rohm_bu21023.c b/drivers/input/touchscreen/rohm_bu21023.c
new file mode 100644
index 0000000..a35a914
--- /dev/null
+++ b/drivers/input/touchscreen/rohm_bu21023.c
@@ -0,0 +1,680 @@
+/*
+ * ROHM BU21023/24 Dual touch support resistive touch screen driver
+ * Copyright (C) 2012 ROHM CO.,LTD.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#include <linux/delay.h>
+#include <linux/firmware.h>
+#include <linux/hrtimer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include "rohm_bu21023.h"
+
+struct rohm_ts_data {
+ struct i2c_client *client;
+ struct input_dev *input_dev;
+
+ bool initialized;
+
+ unsigned int untouch_count;
+ unsigned int single_touch_count;
+ unsigned int dual_touch_count;
+ unsigned int prev_touch_report;
+};
+
+/*
+ * rohm_i2c_burst_read - execute combined I2C message for ROHM BU21023/24
+ * @adap: Handle to I2C bus
+ * @msgs: combined messages to execute
+ * @num: Number of messages to be executed.
+ *
+ * Returns negative errno, else the number of messages executed.
+ *
+ * Note
+ * In BU21023/24 burst read, stop condition is needed after "address write".
+ * Therefore, transmission is performed in 2 steps.
+ */
+static inline int rohm_i2c_burst_read(struct i2c_adapter *adap,
+ struct i2c_msg *msgs, int num)
+{
+ int ret, i;
+
+ if (!adap->algo->master_xfer) {
+ dev_err(&adap->dev, "I2C level transfers not supported\n");
+ return -EOPNOTSUPP;
+ }
+
+ i2c_lock_adapter(adap);
+
+ for (i = 0; i < num; i++) {
+ ret = __i2c_transfer(adap, &msgs[i], 1);
+ if (ret < 0)
+ break;
+
+ ret = i;
+ }
+
+ i2c_unlock_adapter(adap);
+
+ return ret;
+}
+
+static unsigned long inactive_polling_interval[2] = { 1, 0 };
+static unsigned long active_polling_interval[2] = { 0, 10000000 };
+
+module_param_array(inactive_polling_interval, ulong, NULL, S_IRUGO | S_IWUSR);
+module_param_array(active_polling_interval, ulong, NULL, S_IRUGO | S_IWUSR);
+
+MODULE_PARM_DESC(inactive_polling_interval,
+ "Polling interval for un-touch detection");
+MODULE_PARM_DESC(active_polling_interval,
+ "Polling interval for touch detection");
+
+#define INACTIVE_POLLING_INTERVAL_KTIME \
+ ktime_set(inactive_polling_interval[0], inactive_polling_interval[1])
+#define ACTIVE_POLLING_INTERVAL_KTIME \
+ ktime_set(active_polling_interval[0], active_polling_interval[1])
+
+static unsigned int untouch_threshold[3] = { 0, 1, 5 };
+static unsigned int single_touch_threshold[3] = { 0, 0, 4 };
+static unsigned int dual_touch_threshold[3] = { 10, 8, 0 };
+
+module_param_array(untouch_threshold, uint, NULL, S_IRUGO | S_IWUSR);
+module_param_array(single_touch_threshold, uint, NULL, S_IRUGO | S_IWUSR);
+module_param_array(dual_touch_threshold, uint, NULL, S_IRUGO | S_IWUSR);
+
+MODULE_PARM_DESC(untouch_threshold, "Thresholds for un-touch detection");
+MODULE_PARM_DESC(single_touch_threshold,
+ "Thresholds for single touch detection");
+MODULE_PARM_DESC(dual_touch_threshold, "Thresholds for dual touch detection");
+
+static irqreturn_t rohm_ts_soft_irq(int irq, void *dev_id)
+{
+ struct rohm_ts_data *ts = dev_id;
+ struct i2c_client *client = ts->client;
+ struct input_dev *input_dev = ts->input_dev;
+ struct device *dev = &client->dev;
+
+ struct i2c_msg msg[2];
+ u16 addr = client->addr;
+ u8 addr_buf;
+ u8 buf[10]; /* for 0x20-0x29 */
+
+ struct input_mt_pos pos[BU21023_MAX_SLOTS];
+ int slots[BU21023_MAX_SLOTS];
+ u8 touch_flags;
+ unsigned int threshold;
+ int touch_report = 0;
+ unsigned int prev_touch_report = ts->prev_touch_report;
+
+ s32 status;
+ int i, ret;
+
+ status = i2c_smbus_read_byte_data(client, INT_STATUS);
+ if (!status)
+ return IRQ_NONE;
+
+ if (status < 0)
+ return IRQ_HANDLED;
+
+ ret = i2c_smbus_write_byte_data(client, INT_MASK, INT_ALL);
+ if (ret)
+ return IRQ_HANDLED;
+
+ /* Clear all interrupts */
+ ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
+ if (ret)
+ return IRQ_HANDLED;
+
+ addr_buf = POS_X1_H;
+ msg[0].addr = addr;
+ msg[0].flags = 0;
+ msg[0].len = 1;
+ msg[0].buf = &addr_buf;
+
+ msg[1].addr = addr;
+ msg[1].flags = I2C_M_RD;
+ msg[1].len = sizeof(buf);
+ msg[1].buf = buf;
+
+#define READ_POS_BUF(reg) ((u16)buf[((reg) - POS_X1_H)])
+
+ ret = rohm_i2c_burst_read(client->adapter, msg, 2);
+ if (ret < 0)
+ return IRQ_HANDLED;
+
+ touch_flags = READ_POS_BUF(TOUCH_GESTURE) & TOUCH_MASK;
+ if (touch_flags) {
+ /* generate coordinates */
+ pos[0].x = (READ_POS_BUF(POS_X1_H) << 2) |
+ READ_POS_BUF(POS_X1_L);
+ pos[0].y = (READ_POS_BUF(POS_Y1_H) << 2) |
+ READ_POS_BUF(POS_Y1_L);
+ pos[1].x = (READ_POS_BUF(POS_X2_H) << 2) |
+ READ_POS_BUF(POS_X2_L);
+ pos[1].y = (READ_POS_BUF(POS_Y2_H) << 2) |
+ READ_POS_BUF(POS_Y2_L);
+
+ switch (touch_flags) {
+ case SINGLE_TOUCH:
+ ts->untouch_count = 0;
+ ts->single_touch_count++;
+ ts->dual_touch_count = 0;
+
+ threshold = single_touch_threshold[prev_touch_report];
+ if (ts->single_touch_count > threshold)
+ touch_report = 1;
+
+ if (touch_report == 1) {
+ if (pos[1].x != 0 && pos[1].y != 0) {
+ pos[0].x = pos[1].x;
+ pos[0].y = pos[1].y;
+ pos[1].x = 0;
+ pos[1].y = 0;
+ }
+ }
+ break;
+ case DUAL_TOUCH:
+ ts->untouch_count = 0;
+ ts->single_touch_count = 0;
+ ts->dual_touch_count++;
+
+ threshold = dual_touch_threshold[prev_touch_report];
+ if (ts->dual_touch_count > threshold)
+ touch_report = 2;
+ break;
+ default:
+ dev_err(dev,
+ "Three or more touches are not supported\n");
+ return IRQ_HANDLED;
+ break;
+ }
+ } else {
+ ts->untouch_count++;
+
+ threshold = untouch_threshold[prev_touch_report];
+ if (ts->untouch_count > threshold) {
+ ts->untouch_count = 0;
+ ts->single_touch_count = 0;
+ ts->dual_touch_count = 0;
+
+ touch_report = 0;
+ }
+ }
+
+ input_mt_assign_slots(input_dev, slots, pos, touch_report);
+
+ for (i = 0; i < touch_report; i++) {
+ input_mt_slot(input_dev, slots[i]);
+ input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, true);
+ input_report_abs(input_dev, ABS_MT_POSITION_X, pos[i].x);
+ input_report_abs(input_dev, ABS_MT_POSITION_Y, pos[i].y);
+ }
+
+ input_mt_sync_frame(input_dev);
+ input_mt_report_pointer_emulation(input_dev, true);
+ input_sync(input_dev);
+
+ i2c_smbus_write_byte_data(client, INT_MASK,
+ CALIBRATION_DONE | SLEEP_OUT | SLEEP_IN |
+ PROGRAM_LOAD_DONE);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t rohm_ts_hard_irq(int irq, void *dev_id)
+{
+ return IRQ_WAKE_THREAD;
+}
+
+static int rohm_ts_load_firmware(struct i2c_client *client,
+ const char *firmware_name)
+{
+ struct device *dev = &client->dev;
+ const struct firmware *firmware = NULL;
+ s32 status;
+ int blocks, remainder, retry = 0, offset;
+ int err = 0, ret;
+ int i;
+
+ ret = request_firmware(&firmware, firmware_name, dev);
+ if (ret) {
+ dev_err(dev, "Unable to open firmware %s\n", firmware_name);
+ return ret;
+ }
+
+ blocks = firmware->size / FIRMWARE_BLOCK_SIZE;
+ remainder = firmware->size % FIRMWARE_BLOCK_SIZE;
+
+ ret = i2c_smbus_write_byte_data(client, INT_MASK,
+ COORD_UPDATE | CALIBRATION_DONE |
+ SLEEP_IN | SLEEP_OUT);
+ if (ret) {
+ err = ret;
+ goto err_int_mask_exit;
+ }
+
+ while (retry < FIRMWARE_RETRY_MAX) {
+ ret = i2c_smbus_write_byte_data(client, COMMON_SETUP1,
+ COMMON_SETUP1_DEFAULT);
+ if (ret) {
+ err = ret;
+ goto err_int_mask_exit;
+ }
+
+ ret = i2c_smbus_write_byte_data(client, EX_ADDR_H, 0);
+ if (ret) {
+ err = ret;
+ goto err_int_mask_exit;
+ }
+
+ ret = i2c_smbus_write_byte_data(client, EX_ADDR_L, 0);
+ if (ret) {
+ err = ret;
+ goto err_int_mask_exit;
+ }
+
+ offset = 0;
+
+ /* firmware load to the device */
+ for (i = 0; i < blocks; i++) {
+ ret = i2c_smbus_write_i2c_block_data(client, EX_WDAT,
+ FIRMWARE_BLOCK_SIZE, &firmware->data[offset]);
+ if (ret) {
+ err = ret;
+ goto err_int_mask_exit;
+ }
+
+ offset += FIRMWARE_BLOCK_SIZE;
+ }
+
+ if (remainder) {
+ ret = i2c_smbus_write_i2c_block_data(client, EX_WDAT,
+ remainder, &firmware->data[offset]);
+ if (ret) {
+ err = ret;
+ goto err_int_mask_exit;
+ }
+ }
+
+ /* check formware load result */
+ status = i2c_smbus_read_byte_data(client, INT_STATUS);
+ if (status < 0) {
+ err = status;
+ goto err_int_mask_exit;
+ }
+
+ /* clear all interrupts */
+ ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
+ if (ret) {
+ err = ret;
+ goto err_int_mask_exit;
+ }
+
+ if (status == PROGRAM_LOAD_DONE)
+ break;
+
+ /* settings for retry */
+ ret = i2c_smbus_write_byte_data(client, EX_WDAT, 0);
+ if (ret) {
+ err = ret;
+ goto err_int_mask_exit;
+ }
+
+ retry++;
+ dev_warn(dev, "Retry firmware load\n");
+ }
+
+err_int_mask_exit:
+ ret = i2c_smbus_write_byte_data(client, INT_MASK, INT_ALL);
+ if (ret)
+ err = ret;
+
+ release_firmware(firmware);
+
+ if (retry >= FIRMWARE_RETRY_MAX)
+ return -EBUSY;
+
+ return err;
+}
+
+static bool swap_xy;
+module_param(swap_xy, bool, S_IRUGO);
+MODULE_PARM_DESC(swap_xy, "Swap X-axis and Y-axis");
+
+static bool inv_x;
+module_param(inv_x, bool, S_IRUGO);
+MODULE_PARM_DESC(inv_x, "Invert X-axis");
+
+static bool inv_y;
+module_param(inv_y, bool, S_IRUGO);
+MODULE_PARM_DESC(inv_y, "Invert Y-axis");
+
+static int rohm_ts_device_init(struct rohm_ts_data *ts)
+{
+ struct i2c_client *client = ts->client;
+ struct device *dev = &client->dev;
+ u8 val;
+ int ret;
+
+ /*
+ * Wait 200usec for reset
+ */
+ udelay(200);
+
+ /* Release analog reset */
+ ret = i2c_smbus_write_byte_data(client, SYSTEM, ANALOG_POWER_ON);
+ if (ret)
+ return ret;
+
+ /* Waiting for the analog warm-up, max. 200usec */
+ udelay(200);
+
+ /* clear all interrupts */
+ ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
+ if (ret)
+ return ret;
+
+ val = MAF_1SAMPLE;
+ if (swap_xy)
+ val |= SWAP_XY;
+ if (inv_x)
+ val |= INV_X;
+ if (inv_y)
+ val |= INV_Y;
+
+ ret = i2c_smbus_write_byte_data(client, EX_WDAT, 0);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, COMMON_SETUP1, 0);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, COMMON_SETUP2, val);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, COMMON_SETUP3,
+ SEL_TBL_DEFAULT | EN_MULTI);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, THRESHOLD_GESTURE,
+ THRESHOLD_GESTURE_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, INTERVAL_TIME,
+ INTERVAL_TIME_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, CPU_FREQ, CPU_FREQ_10MHZ);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, PRM_SWOFF_TIME,
+ PRM_SWOFF_TIME_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, ADC_CTRL, ADC_DIV_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, ADC_WAIT, ADC_WAIT_DEFAULT);
+ if (ret)
+ return ret;
+
+ /*
+ * Panel setup, these values change with the panel.
+ */
+ ret = i2c_smbus_write_byte_data(client, STEP_X, STEP_X_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, STEP_Y, STEP_Y_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, OFFSET_X, OFFSET_X_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, OFFSET_Y, OFFSET_Y_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, THRESHOLD_TOUCH,
+ THRESHOLD_TOUCH_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, EVR_XY, EVR_XY_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, EVR_X, EVR_X_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, EVR_Y, EVR_Y_DEFAULT);
+ if (ret)
+ return ret;
+
+ /* Fixed value settings */
+ ret = i2c_smbus_write_byte_data(client, CALIBRATION_ADJUST,
+ CALIBRATION_ADJUST_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, SWCONT, SWCONT_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, TEST1,
+ DUALTOUCH_STABILIZE_ON |
+ DUALTOUCH_REG_ON);
+ if (ret)
+ return ret;
+
+ ret = rohm_ts_load_firmware(client, BU21023_FIRMWARE_NAME);
+ if (ret) {
+ dev_err(dev, "Failed to firmware load\n");
+ return ret;
+ }
+
+ /*
+ * Manual calibration results are not changed in same environment.
+ * If the force calibration is performed,
+ * the controller will not require calibration request interrupt
+ * when the typical values are set to the calibration registers.
+ */
+ ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG1,
+ CALIBRATION_REG1_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG2,
+ CALIBRATION_REG2_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG3,
+ CALIBRATION_REG3_DEFAULT);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, FORCE_CALIBRATION,
+ FORCE_CALIBRATION_OFF);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, FORCE_CALIBRATION,
+ FORCE_CALIBRATION_ON);
+ if (ret)
+ return ret;
+
+ /* Clear all interrupts */
+ ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
+ if (ret)
+ return ret;
+
+ ret = devm_request_threaded_irq(dev, client->irq, rohm_ts_hard_irq,
+ rohm_ts_soft_irq, IRQF_TRIGGER_FALLING,
+ client->name, ts);
+ if (ret) {
+ dev_err(dev, "Unable to request IRQ\n");
+ return ret;
+ }
+
+ /* Enable coordinates update interrupt */
+ ret = i2c_smbus_write_byte_data(client, INT_MASK,
+ CALIBRATION_DONE |
+ SLEEP_OUT | SLEEP_IN |
+ PROGRAM_LOAD_DONE);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, ERR_MASK,
+ PROGRAM_LOAD_ERR | CPU_TIMEOUT |
+ ADC_TIMEOUT);
+ if (ret)
+ return ret;
+
+ /* controller CPU power on */
+ ret = i2c_smbus_write_byte_data(client, SYSTEM,
+ CPU_POWER_ON | ANALOG_POWER_ON);
+
+ return ret;
+}
+
+static int rohm_ts_open(struct input_dev *input_dev)
+{
+ struct rohm_ts_data *ts = input_get_drvdata(input_dev);
+ struct i2c_client *client = ts->client;
+ int ret;
+
+ if (!ts->initialized) {
+ ret = rohm_ts_device_init(ts);
+ if (ret) {
+ dev_err(&client->dev,
+ "Failed to device initialization\n");
+ return ret;
+ }
+
+ ts->initialized = true;
+ }
+
+ return 0;
+}
+
+static int rohm_bu21023_i2c_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct rohm_ts_data *ts;
+ struct device *dev = &client->dev;
+ struct input_dev *input_dev;
+ int ret;
+
+ if (!client->irq) {
+ dev_err(dev, "IRQ is not assigned\n");
+ return -EBUSY;
+ }
+
+ ts = devm_kzalloc(dev, sizeof(struct rohm_ts_data), GFP_KERNEL);
+ if (!ts)
+ return -ENOMEM;
+
+ ts->client = client;
+ i2c_set_clientdata(client, ts);
+
+ input_dev = devm_input_allocate_device(dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ input_dev->name = BU21023_NAME;
+ input_dev->id.bustype = BUS_I2C;
+ input_dev->open = rohm_ts_open;
+
+ ts->input_dev = input_dev;
+ input_set_drvdata(input_dev, ts);
+
+ __set_bit(EV_SYN, input_dev->evbit);
+ __set_bit(EV_KEY, input_dev->evbit);
+ __set_bit(EV_ABS, input_dev->evbit);
+
+ __set_bit(BTN_TOUCH, input_dev->keybit);
+
+ input_set_abs_params(input_dev, ABS_MT_POSITION_X,
+ ROHM_TS_ABS_X_MIN, ROHM_TS_ABS_X_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
+ ROHM_TS_ABS_Y_MIN, ROHM_TS_ABS_Y_MAX, 0, 0);
+
+ input_set_abs_params(input_dev, ABS_X,
+ ROHM_TS_ABS_X_MIN, ROHM_TS_ABS_X_MAX, 0, 0);
+ input_set_abs_params(input_dev, ABS_Y,
+ ROHM_TS_ABS_Y_MIN, ROHM_TS_ABS_Y_MAX, 0, 0);
+
+ ret = input_mt_init_slots(input_dev, BU21023_MAX_SLOTS,
+ INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+ if (ret) {
+ dev_err(dev, "Failed to multi touch slots initialization\n");
+ return ret;
+ }
+
+ ret = input_register_device(input_dev);
+ if (ret)
+ dev_err(dev, "Unable to register input device\n");
+
+ return ret;
+}
+
+static int rohm_bu21023_i2c_remove(struct i2c_client *client)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(client, SYSTEM,
+ ANALOG_POWER_ON | CPU_POWER_OFF);
+ if (ret)
+ return ret;
+
+ return i2c_smbus_write_byte_data(client, SYSTEM,
+ ANALOG_POWER_OFF | CPU_POWER_OFF);
+}
+
+static const struct i2c_device_id rohm_bu21023_i2c_id[] = {
+ {BU21023_NAME, 0},
+ {},
+};
+
+MODULE_DEVICE_TABLE(i2c, rohm_bu21023_i2c_id);
+
+static struct i2c_driver rohm_bu21023_i2c_driver = {
+ .driver = {
+ .name = BU21023_NAME,
+ },
+ .probe = rohm_bu21023_i2c_probe,
+ .remove = rohm_bu21023_i2c_remove,
+ .id_table = rohm_bu21023_i2c_id,
+};
+
+module_i2c_driver(rohm_bu21023_i2c_driver);
+
+MODULE_DESCRIPTION("ROHM BU21023/24 Touchscreen driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("ROHM Co., Ltd.");
diff --git a/drivers/input/touchscreen/rohm_bu21023.h b/drivers/input/touchscreen/rohm_bu21023.h
new file mode 100644
index 0000000..302581b
--- /dev/null
+++ b/drivers/input/touchscreen/rohm_bu21023.h
@@ -0,0 +1,258 @@
+/*
+ * ROHM BU21023/24 Dual touch support resistive touch screen driver
+ * Copyright (C) 2012 ROHM CO.,LTD.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+#ifndef _BU21023_TS_H
+#define _BU21023_TS_H
+
+#define BU21023_NAME "bu21023_ts"
+#define BU21023_FIRMWARE_NAME "bu21023.bin"
+
+#define BU21023_MAX_SLOTS 2
+
+#define AXIS_ADJUST 4
+#define AXIS_OFFSET 8
+
+#define FIRMWARE_BLOCK_SIZE 32
+#define FIRMWARE_RETRY_MAX 4
+
+#define SAMPLING_DELAY 12 /* msec */
+
+#define CALIBRATION_RETRY_MAX 6
+
+#define ROHM_TS_ABS_X_MIN 40
+#define ROHM_TS_ABS_X_MAX 990
+#define ROHM_TS_ABS_Y_MIN 160
+#define ROHM_TS_ABS_Y_MAX 920
+
+/*
+ * BU21023GUL/BU21023MUV/BU21024FV-M registers map
+ */
+#define VADOUT_YP_H 0x00
+#define VADOUT_YP_L 0x01
+#define VADOUT_XP_H 0x02
+#define VADOUT_XP_L 0x03
+#define VADOUT_YN_H 0x04
+#define VADOUT_YN_L 0x05
+#define VADOUT_XN_H 0x06
+#define VADOUT_XN_L 0x07
+
+#define PRM1_X_H 0x08
+#define PRM1_X_L 0x09
+#define PRM1_Y_H 0x0a
+#define PRM1_Y_L 0x0b
+#define PRM2_X_H 0x0c
+#define PRM2_X_L 0x0d
+#define PRM2_Y_H 0x0e
+#define PRM2_Y_L 0x0f
+
+#define MLT_PRM_MONI_X 0x10
+#define MLT_PRM_MONI_Y 0x11
+
+#define DEBUG_MONI_1 0x12
+#define DEBUG_MONI_2 0x13
+
+#define VADOUT_ZX_H 0x14
+#define VADOUT_ZX_L 0x15
+#define VADOUT_ZY_H 0x16
+#define VADOUT_ZY_L 0x17
+
+#define Z_PARAM_H 0x18
+#define Z_PARAM_L 0x19
+
+/*
+ * Value for VADOUT_*_L
+ */
+#define VADOUT_L_MASK 0x01
+
+/*
+ * Value for PRM*_*_L
+ */
+#define PRM_L_MASK 0x01
+
+#define POS_X1_H 0x20
+#define POS_X1_L 0x21
+#define POS_Y1_H 0x22
+#define POS_Y1_L 0x23
+#define POS_X2_H 0x24
+#define POS_X2_L 0x25
+#define POS_Y2_H 0x26
+#define POS_Y2_L 0x27
+
+/*
+ * Value for POS_*_L
+ */
+#define POS_L_MASK 0x01
+
+#define TOUCH 0x28
+#define TOUCH_DETECT 0x01
+
+#define TOUCH_GESTURE 0x29
+#define SINGLE_TOUCH 0x01
+#define DUAL_TOUCH 0x03
+#define TOUCH_MASK 0x03
+#define CALIBRATION_REQUEST 0x04
+#define CALIBRATION_STATUS 0x08
+#define CALIBRATION_MASK 0x0c
+#define GESTURE_SPREAD 0x10
+#define GESTURE_PINCH 0x20
+#define GESTURE_ROTATE_R 0x40
+#define GESTURE_ROTATE_L 0x80
+
+#define INT_STATUS 0x2a
+#define INT_MASK 0x3d
+#define INT_CLEAR 0x3e
+
+/*
+ * Values for INT_*
+ */
+#define COORD_UPDATE 0x01
+#define CALIBRATION_DONE 0x02
+#define SLEEP_IN 0x04
+#define SLEEP_OUT 0x08
+#define PROGRAM_LOAD_DONE 0x10
+#define ERROR 0x80
+#define INT_ALL 0x9f
+
+#define ERR_STATUS 0x2b
+#define ERR_MASK 0x3f
+
+/*
+ * Values for ERR_*
+ */
+#define ADC_TIMEOUT 0x01
+#define CPU_TIMEOUT 0x02
+#define CALIBRATION_ERR 0x04
+#define PROGRAM_LOAD_ERR 0x10
+
+#define COMMON_SETUP1 0x30
+#define PROGRAM_LOAD_HOST 0x02
+#define PROGRAM_LOAD_EEPROM 0x03
+#define CENSOR_4PORT 0x04
+#define CENSOR_8PORT 0x00 /* Not supported by BU21023 */
+#define CALIBRATION_TYPE_DEFAULT 0x08
+#define CALIBRATION_TYPE_SPECIAL 0x00
+#define INT_ACTIVE_HIGH 0x10
+#define INT_ACTIVE_LOW 0x00
+#define AUTO_CALIBRATION 0x40
+#define MANUAL_CALIBRATION 0x00
+#define COMMON_SETUP1_DEFAULT 0x4e
+
+#define COMMON_SETUP2 0x31
+#define MAF_NONE 0x00
+#define MAF_1SAMPLE 0x01
+#define MAF_3SAMPLES 0x02
+#define MAF_5SAMPLES 0x03
+#define INV_Y 0x04
+#define INV_X 0x08
+#define SWAP_XY 0x10
+
+#define COMMON_SETUP3 0x32
+#define EN_SLEEP 0x01
+#define EN_MULTI 0x02
+#define EN_GESTURE 0x04
+#define EN_INTVL 0x08
+#define SEL_STEP 0x10
+#define SEL_MULTI 0x20
+#define SEL_TBL_DEFAULT 0x40
+
+#define INTERVAL_TIME 0x33
+#define INTERVAL_TIME_DEFAULT 0x10
+
+#define STEP_X 0x34
+#define STEP_X_DEFAULT 0x41
+
+#define STEP_Y 0x35
+#define STEP_Y_DEFAULT 0x8d
+
+#define OFFSET_X 0x38
+#define OFFSET_X_DEFAULT 0x0c
+
+#define OFFSET_Y 0x39
+#define OFFSET_Y_DEFAULT 0x0c
+
+#define THRESHOLD_TOUCH 0x3a
+#define THRESHOLD_TOUCH_DEFAULT 0xa0
+
+#define THRESHOLD_GESTURE 0x3b
+#define THRESHOLD_GESTURE_DEFAULT 0x17
+
+#define SYSTEM 0x40
+#define ANALOG_POWER_ON 0x01
+#define ANALOG_POWER_OFF 0x00
+#define CPU_POWER_ON 0x02
+#define CPU_POWER_OFF 0x00
+
+#define FORCE_CALIBRATION 0x42
+#define FORCE_CALIBRATION_ON 0x01
+#define FORCE_CALIBRATION_OFF 0x00
+
+#define CPU_FREQ 0x50 /* 10 / (reg + 1) MHz */
+#define CPU_FREQ_10MHZ 0x00
+#define CPU_FREQ_5MHZ 0x01
+#define CPU_FREQ_1MHZ 0x09
+
+#define EEPROM_ADDR 0x51
+
+#define CALIBRATION_ADJUST 0x52
+#define CALIBRATION_ADJUST_DEFAULT 0x00
+
+#define THRESHOLD_SLEEP_IN 0x53
+
+#define EVR_XY 0x56
+#define EVR_XY_DEFAULT 0x10
+
+#define PRM_SWOFF_TIME 0x57
+#define PRM_SWOFF_TIME_DEFAULT 0x04
+
+#define PROGRAM_VERSION 0x5f
+
+#define ADC_CTRL 0x60
+#define ADC_DIV_MASK 0x1f /* The minimum value is 4 */
+#define ADC_DIV_DEFAULT 0x08
+
+#define ADC_WAIT 0x61
+#define ADC_WAIT_DEFAULT 0x0a
+
+#define SWCONT 0x62
+#define SWCONT_DEFAULT 0x0f
+
+#define EVR_X 0x63
+#define EVR_X_DEFAULT 0x86
+
+#define EVR_Y 0x64
+#define EVR_Y_DEFAULT 0x64
+
+#define TEST1 0x65
+#define DUALTOUCH_STABILIZE_ON 0x01
+#define DUALTOUCH_STABILIZE_OFF 0x00
+#define DUALTOUCH_REG_ON 0x20
+#define DUALTOUCH_REG_OFF 0x00
+
+#define CALIBRATION_REG1 0x68
+#define CALIBRATION_REG1_DEFAULT 0xd9
+
+#define CALIBRATION_REG2 0x69
+#define CALIBRATION_REG2_DEFAULT 0x36
+
+#define CALIBRATION_REG3 0x6a
+#define CALIBRATION_REG3_DEFAULT 0x32
+
+#define EX_ADDR_H 0x70
+#define EX_ADDR_L 0x71
+#define EX_WDAT 0x72
+#define EX_RDAT 0x73
+#define EX_CHK_SUM1 0x74
+#define EX_CHK_SUM2 0x75
+#define EX_CHK_SUM3 0x76
+
+#endif /* _BU21023_TS_H */
--
1.7.9.5
^ permalink raw reply related
* [PATCH v3 2/2] input: bu21023_ts: Add calibration function
From: Yoichi Yuasa @ 2014-09-24 15:19 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: yuasa, linux-input
In-Reply-To: <20140925001647.dd48571a3c0ceda5c25ab669@linux-mips.org>
Signed-off-by: Yoichi Yuasa <yuasa@linux-mips.org>
---
drivers/input/touchscreen/rohm_bu21023.c | 250 ++++++++++++++++++++++++++++++
1 file changed, 250 insertions(+)
diff --git a/drivers/input/touchscreen/rohm_bu21023.c b/drivers/input/touchscreen/rohm_bu21023.c
index a35a914..de6717f 100644
--- a/drivers/input/touchscreen/rohm_bu21023.c
+++ b/drivers/input/touchscreen/rohm_bu21023.c
@@ -72,6 +72,251 @@ static inline int rohm_i2c_burst_read(struct i2c_adapter *adap,
return ret;
}
+static int rohm_ts_manual_calibration(struct rohm_ts_data *ts)
+{
+ struct i2c_client *client = ts->client;
+ struct device *dev = &client->dev;
+ struct i2c_msg msg[2];
+ u8 buf[33];
+ u8 addr_buf; /* burst read start address */
+
+ int retry;
+ bool success = false;
+ bool first_time = true;
+ bool calibration_done;
+
+ u8 reg1, reg2, reg3;
+ s32 reg1_orig, reg2_orig, reg3_orig;
+ s32 val;
+
+ int calib_x = 0, calib_y = 0;
+ int reg_x, reg_y;
+ int err_x, err_y;
+
+ int err = 0, ret;
+ int i;
+
+ addr_buf = PRM1_X_H;
+ msg[0].addr = client->addr;
+ msg[0].flags = 0;
+ msg[0].len = 1;
+ msg[0].buf = &addr_buf;
+
+ msg[1].addr = client->addr;
+ msg[1].flags = I2C_M_RD;
+ msg[1].len = sizeof(buf);
+ msg[1].buf = buf;
+
+#define READ_CALIB_BUF(reg) ((u16)buf[((reg) - PRM1_X_H)])
+
+ reg1_orig = i2c_smbus_read_byte_data(client, CALIBRATION_REG1);
+ if (reg1_orig < 0)
+ return reg1_orig;
+
+ reg2_orig = i2c_smbus_read_byte_data(client, CALIBRATION_REG2);
+ if (reg2_orig < 0)
+ return reg2_orig;
+
+ reg3_orig = i2c_smbus_read_byte_data(client, CALIBRATION_REG3);
+ if (reg3_orig < 0)
+ return reg3_orig;
+
+ ret = i2c_smbus_write_byte_data(client, INT_MASK,
+ COORD_UPDATE | SLEEP_IN | SLEEP_OUT |
+ PROGRAM_LOAD_DONE);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ ret = i2c_smbus_write_byte_data(client, TEST1, DUALTOUCH_STABILIZE_ON);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ for (retry = 0; retry < CALIBRATION_RETRY_MAX; retry++) {
+ /* wait 2 sampling for update */
+ mdelay(2 * SAMPLING_DELAY);
+
+ ret = rohm_i2c_burst_read(client->adapter, msg, 2);
+ if (ret < 0) {
+ err = ret;
+ goto err_exit;
+ }
+
+ if (READ_CALIB_BUF(TOUCH) & TOUCH_DETECT)
+ continue;
+
+ if (first_time) {
+ /* generate calibration parameter */
+ calib_x =
+ (READ_CALIB_BUF(PRM1_X_H) << 2 |
+ READ_CALIB_BUF(PRM1_X_L)) - AXIS_OFFSET;
+ calib_y =
+ (READ_CALIB_BUF(PRM1_Y_H) << 2 |
+ READ_CALIB_BUF(PRM1_Y_L)) - AXIS_OFFSET;
+
+ ret = i2c_smbus_write_byte_data(client, TEST1,
+ DUALTOUCH_STABILIZE_ON |
+ DUALTOUCH_REG_ON);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ first_time = false;
+ } else {
+ /* generate adjustment parameter */
+ err_x = READ_CALIB_BUF(PRM1_X_H) << 2 |
+ READ_CALIB_BUF(PRM1_X_L);
+ err_y = READ_CALIB_BUF(PRM1_Y_H) << 2 |
+ READ_CALIB_BUF(PRM1_Y_L);
+
+ /* X axis ajust */
+ if (err_x <= 4)
+ calib_x -= AXIS_ADJUST;
+ else if (err_x >= 60)
+ calib_x += AXIS_ADJUST;
+
+ /* Y axis ajust */
+ if (err_y <= 4)
+ calib_y -= AXIS_ADJUST;
+ else if (err_y >= 60)
+ calib_y += AXIS_ADJUST;
+ }
+
+ /* generate calibration setting value */
+ reg_x = calib_x + ((calib_x & 0x200) << 1);
+ reg_y = calib_y + ((calib_y & 0x200) << 1);
+
+ /* convert for register format */
+ reg1 = reg_x >> 3;
+ reg2 = (reg_y & 0x7) << 4 | (reg_x & 0x7);
+ reg3 = reg_y >> 3;
+
+ ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG1, reg1);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG2, reg2);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG3, reg3);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ /*
+ * force calibration sequcence
+ */
+ ret = i2c_smbus_write_byte_data(client, FORCE_CALIBRATION,
+ FORCE_CALIBRATION_OFF);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ ret = i2c_smbus_write_byte_data(client, FORCE_CALIBRATION,
+ FORCE_CALIBRATION_ON);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ /* clear all interrupts */
+ ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ /*
+ * Wait for the status change of calibration, max 10 sampling
+ */
+ calibration_done = false;
+
+ for (i = 0; i < 10; i++) {
+ mdelay(SAMPLING_DELAY);
+
+ val = i2c_smbus_read_byte_data(client, TOUCH_GESTURE);
+ if (!(val & CALIBRATION_MASK)) {
+ calibration_done = true;
+ break;
+ } else if (val < 0) {
+ err = val;
+ goto err_exit;
+ }
+ }
+
+ if (calibration_done) {
+ val = i2c_smbus_read_byte_data(client, INT_STATUS);
+ if (val == CALIBRATION_DONE) {
+ success = true;
+ break;
+ } else if (val < 0) {
+ err = val;
+ goto err_exit;
+ }
+ } else
+ dev_warn(dev, "Calibration timeout\n");
+ }
+
+ if (!success) {
+ ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG1,
+ reg1_orig);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG2,
+ reg2_orig);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG3,
+ reg3_orig);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ /* calibration data enable */
+ ret = i2c_smbus_write_byte_data(client, TEST1,
+ DUALTOUCH_STABILIZE_ON |
+ DUALTOUCH_REG_ON);
+ if (ret) {
+ err = ret;
+ goto err_exit;
+ }
+
+ /* wait 10 sampling */
+ mdelay(10 * SAMPLING_DELAY);
+
+ err = -EBUSY;
+ }
+
+err_exit:
+ ret = i2c_smbus_write_byte_data(client, INT_MASK, INT_ALL);
+ if (!ret)
+ /* Clear all interrupts */
+ ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
+
+ if (!err && ret)
+ err = ret;
+
+ return err;
+}
+
static unsigned long inactive_polling_interval[2] = { 1, 0 };
static unsigned long active_polling_interval[2] = { 0, 10000000 };
@@ -228,6 +473,11 @@ static irqreturn_t rohm_ts_soft_irq(int irq, void *dev_id)
input_mt_report_pointer_emulation(input_dev, true);
input_sync(input_dev);
+ if (READ_POS_BUF(TOUCH_GESTURE) & CALIBRATION_REQUEST) {
+ if (rohm_ts_manual_calibration(ts) < 0)
+ dev_warn(dev, "Failed to manual calibration\n");
+ }
+
i2c_smbus_write_byte_data(client, INT_MASK,
CALIBRATION_DONE | SLEEP_OUT | SLEEP_IN |
PROGRAM_LOAD_DONE);
--
1.7.9.5
^ permalink raw reply related
* Re: HID: wacom: regression - system freezes when resuming from S3 suspend
From: Benjamin Tissoires @ 2014-09-24 19:27 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Jonas Jelten, linux-input
In-Reply-To: <alpine.LNX.2.00.1409241032490.32397@pobox.suse.cz>
On Wed, Sep 24, 2014 at 4:34 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 24 Sep 2014, Jonas Jelten wrote:
>
>> >> I encountered that my system freezes when resuming my Lenovo X220t with
>> >> integrated Wacom ISDv4 tablet from S3 suspend.
>> >>
>> >> Currently running 3.17.0-rc6, I started to bisect and found that this
>> >> commit causes the issue:
>> >>
>> >> commit 29b4739134c73a2873adec93346f09bb76d6a794
>> >> Author: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> >> Date: Thu Jul 24 12:52:23 2014 -0700
>> >>
>> >> Input: wacom - switch from an USB driver to a HID driver
>> >>
>> >>
>> >> Running 3.17.0-rc6, right after boot, this shows up in the log:
>> >>
>> >> [ 25.843484] wacom 0003:056A:00E6.0001: usb_submit_urb(ctrl) failed: -1
>> >> [ 25.843531] wacom 0003:056A:00E6.0001: timeout initializing reports
>> >> [ 25.843886] wacom 0003:056A:00E6.0001: hidraw0: USB HID v1.11 Device
>> >> [Tablet ISD-V4] on usb-0000:00:1d.0-1.5/input0
>> >> [ 25.846036] input: Wacom ISDv4 E6 Finger as
>> >> /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.5/2-1.5:1.1/0003:056A:00E6.0002/input/input18
>> >>
>> >> When suspending, the screen goes off and about 8 seconds later, the
>> >> machine actually suspends.
>> >>
>> >> Right after the (instant) resume, I can see the frame buffer I had when
>> >> suspending (X screen) for about 10 seconds. During that time, no sysrq
>> >> works. After this short period, the tty1-framebuffer appears, with
>> >> messed up linebreaks (no carriage returns occur any more), and the
>> >> capslock-led starts blinking. Sysrq keys still don't work. The system is
>> >> completely frozen.
>> >>
>> >> Running the kernel from the bisected first good commit, the suspend is
>> >> faster (under 1 second) and the resume does not crash.
>> >>
>> >> I just saw the "HID: wacom: fix timeout on probe for some wacoms" mail,
>> >> which probably has to do something with my problem.
>> >>
>> >> Any ideas?
>> >
>> > Yes, this commit will fix your problem. It has just been delayed for
>> > 3.17.1 as mentioned by Jiri today.
>>
>> Are you sure it's a good idea to let the kernel crash into 3.17.0 and
>> fix it later in stable? I suspect many X220t users could be affected.
>
> Okay, my initial understanding was that this is just about the 10s delay.
> How exactly does it crash on resume, and do we have any idea why it
> crashes, and why "HID: wacom: fix timeout on probe for some wacoms" fixes
> it?
>
I am not sure (I can not see how actually) that there is a kernel oops
on resume (what you intend by "crash").
However, if the system disable/enable the USB ports both on suspend &
resume (this is where I do not understand), the hid core layer will
try to get the current report states from the device. The device does
not answer, so this adds the delay on both suspend and resume, which
can be quite annoying.
It's just a ugly lock with a 5-8 sec timeouts, not an actual crash.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH] input/serio/i8042-x86ia64io.h: Fix Asus X450LCP touchpad detection
From: Dmitry Torokhov @ 2014-09-25 0:01 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: linux-kernel, Hans de Goede, Jiri Kosina, Tom Gundersen,
Peter Hutterer, linux-input, stable
In-Reply-To: <1411485873-4245-1-git-send-email-marcos.souza.org@gmail.com>
On Tue, Sep 23, 2014 at 12:24:33PM -0300, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
>
> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: Tom Gundersen <teg@jklm.no>
> Cc: Peter Hutterer <peter.hutterer@who-t.net>
> Cc: linux-input@vger.kernel.org
> Cc: stable@vger.kernel.org
Applied, thank you.
> ---
> drivers/input/serio/i8042-x86ia64io.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
> index 136b7b20..ec6cb72 100644
> --- a/drivers/input/serio/i8042-x86ia64io.h
> +++ b/drivers/input/serio/i8042-x86ia64io.h
> @@ -465,6 +465,15 @@ static const struct dmi_system_id __initconst i8042_dmi_nomux_table[] = {
> DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion dv4 Notebook PC"),
> },
> },
> + {
> + /* Asus X450LCP */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> + DMI_MATCH(DMI_PRODUCT_NAME, "X450LCP"),
> + },
> + },
> +
> +
> { }
> };
>
> --
> 1.9.3
>
--
Dmitry
^ permalink raw reply
* Re: HID: wacom: regression - system freezes when resuming from S3 suspend
From: Jonas Jelten @ 2014-09-25 0:20 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina; +Cc: linux-input
In-Reply-To: <CAN+gG=HqiV1omKjhUEV22sWO8DVjiTY0t04qxmO7YtE540RsOw@mail.gmail.com>
On 2014-09-24 21:27, Benjamin Tissoires wrote:
> On Wed, Sep 24, 2014 at 4:34 AM, Jiri Kosina <jkosina@suse.cz> wrote:
>> On Wed, 24 Sep 2014, Jonas Jelten wrote:
>>
>>>>> I encountered that my system freezes when resuming my Lenovo X220t with
>>>>> integrated Wacom ISDv4 tablet from S3 suspend.
>>>>>
>>>>> Currently running 3.17.0-rc6, I started to bisect and found that this
>>>>> commit causes the issue:
>>>>>
>>>>> commit 29b4739134c73a2873adec93346f09bb76d6a794
>>>>> Author: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>>>>> Date: Thu Jul 24 12:52:23 2014 -0700
>>>>>
>>>>> Input: wacom - switch from an USB driver to a HID driver
>>>>>
>>>>>
>>>>> Running 3.17.0-rc6, right after boot, this shows up in the log:
>>>>>
>>>>> [ 25.843484] wacom 0003:056A:00E6.0001: usb_submit_urb(ctrl) failed: -1
>>>>> [ 25.843531] wacom 0003:056A:00E6.0001: timeout initializing reports
>>>>> [ 25.843886] wacom 0003:056A:00E6.0001: hidraw0: USB HID v1.11 Device
>>>>> [Tablet ISD-V4] on usb-0000:00:1d.0-1.5/input0
>>>>> [ 25.846036] input: Wacom ISDv4 E6 Finger as
>>>>> /devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.5/2-1.5:1.1/0003:056A:00E6.0002/input/input18
>>>>>
>>>>> When suspending, the screen goes off and about 8 seconds later, the
>>>>> machine actually suspends.
>>>>>
>>>>> Right after the (instant) resume, I can see the frame buffer I had when
>>>>> suspending (X screen) for about 10 seconds. During that time, no sysrq
>>>>> works. After this short period, the tty1-framebuffer appears, with
>>>>> messed up linebreaks (no carriage returns occur any more), and the
>>>>> capslock-led starts blinking. Sysrq keys still don't work. The system is
>>>>> completely frozen.
>>>>>
>>>>> Running the kernel from the bisected first good commit, the suspend is
>>>>> faster (under 1 second) and the resume does not crash.
>>>>>
>>>>> I just saw the "HID: wacom: fix timeout on probe for some wacoms" mail,
>>>>> which probably has to do something with my problem.
>>>>>
>>>>> Any ideas?
>>>>
>>>> Yes, this commit will fix your problem. It has just been delayed for
>>>> 3.17.1 as mentioned by Jiri today.
>>>
>>> Are you sure it's a good idea to let the kernel crash into 3.17.0 and
>>> fix it later in stable? I suspect many X220t users could be affected.
>>
>> Okay, my initial understanding was that this is just about the 10s delay.
>> How exactly does it crash on resume, and do we have any idea why it
>> crashes, and why "HID: wacom: fix timeout on probe for some wacoms" fixes
>> it?
>>
>
> I am not sure (I can not see how actually) that there is a kernel oops
> on resume (what you intend by "crash").
> However, if the system disable/enable the USB ports both on suspend &
> resume (this is where I do not understand), the hid core layer will
> try to get the current report states from the device. The device does
> not answer, so this adds the delay on both suspend and resume, which
> can be quite annoying.
>
> It's just a ugly lock with a 5-8 sec timeouts, not an actual crash.
>
> Cheers,
> Benjamin
>
I have no idea about the reasons yet, however my kernel indeed panics 10
seconds after resume from S3. During the 10 seconds, the on-suspend X
screen is visible, but frozen (no mouse/keyboard input) and no sysrqs
are possible. After the 10s passed, I see old tty1 framebuffer parts
from the init system log, the kernel seems to have paniced as the
capslock led starts blinking, but I can't see any backtrace.
I could not test whether "HID: wacom: fix timeout on probe for some
wacoms" fixes the panic, as it does not apply for 3.17-rc6.
Cheers,
Jonas
^ permalink raw reply
* Re: HID: wacom: regression - system freezes when resuming from S3 suspend
From: Jiri Kosina @ 2014-09-25 0:24 UTC (permalink / raw)
To: Jonas Jelten; +Cc: Benjamin Tissoires, linux-input
In-Reply-To: <54235FB9.5040706@in.tum.de>
On Thu, 25 Sep 2014, Jonas Jelten wrote:
> I have no idea about the reasons yet, however my kernel indeed panics 10
> seconds after resume from S3. During the 10 seconds, the on-suspend X
> screen is visible, but frozen (no mouse/keyboard input) and no sysrqs
> are possible. After the 10s passed, I see old tty1 framebuffer parts
> from the init system log, the kernel seems to have paniced as the
> capslock led starts blinking, but I can't see any backtrace.
Is there any way you could setup serial console or at least netconsole on
that system?
> I could not test whether "HID: wacom: fix timeout on probe for some
> wacoms" fixes the panic, as it does not apply for 3.17-rc6.
Just compiling for-3.18/wacom branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
should be enough.
I still fail to see how this would be fixing any panic, so either there is
a very strange race condition somewhere, or this is a different problem
that still needs to be solved.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v4 1/14] input: cyapa: re-architecture driver to support multi-trackpads in one driver
From: Dmitry Torokhov @ 2014-09-25 0:48 UTC (permalink / raw)
To: Dudley Du
Cc: Rafael J. Wysocki, Benson Leung, Patrik Fimml, linux-input,
linux-kernel
In-Reply-To: <BAY176-DS93497D6BEA95BFE466BC1BDB10@phx.gbl>
On Wed, Sep 24, 2014 at 03:08:31PM +0800, Dudley Du wrote:
> > > > > + async_schedule(cyapa_detect_async, cyapa);
> > > >
> > > > I think I already asked this before - why do we need to try and schedule
> > async
> > > > detect from interrupt handler. In what cases we will fail to initialize
> > the
> > > > device during normal probing, but then are fine when stray interrupt
> > arrives?
> > > >
> > >
> > > 1) Because gen5 TP devices use interrupt to sync the command and response,
> > and
> > > in detecting, some commands must be sent and executed in attached devices,
> > so
> > > the interrupt must be usable in detecting.
> > > So if do not schedule async detect from interrupt handler, the interrupt
> > > handler will be taken, and cannot enter again, which will cause the command
> > to
> > > the device failed, it will also cause long time detecting.
> > > 2) detecting will fail when no device attached or the attached device is not
> > > supported gen3 or gen5 TP devices or there is some issue with the device
> > > that cannot enter into active working mode or stay in bootloader mode
> > > for invalid firmware detected.
> > > And it's tested that it's save when stray interrupt arrives in detecting.
> >
> >
> > I am sorry, I have trouble parsing this. I understand that you may need
> > interrupt to know when command response is ready, but I do not see how
> > kicking asynchronous detect helps there. During probe you can figure out
> > if you are talking to a Cypress device and whether it is operable. If it
> > is not operable you can try flashing a new firmware and then kick off
> > detection again after flash is successful. But I do not see any reason
> > in trying to re-detect the device after random interrupt arrives in hopes
> > that maybe this time stars will align and we'll be able to work with it.
>
> Considering two situations:
> 1) For some machines, when system get into sleep mode, the power of the trackpad device
> will be cut off, and after system resumed, the power to trackpad device is on again.
And cyapa_resume() is responsible for bringing the device back into
operational state.
> 2) For the trackpad device itself, if internal error encounter, it will reset itself,
> similar result as power off and power on again.
> When either of above situation happened, for gen3 Cypress trackpad device, it will get
> into bootloader mode (it cannot get into operational mode automatically), and also assert
> interrupts to host.
> So at this time, cyapa driver must kick off the detection again to determine the real status
> of the trackpad device, and command it go to the operational mode.
> Otherwise, it will be out of work until reboot.
In this case you won't have cyapa_default_irq_handler installed, right?
The specific generation IRQ handler will have to detect this condition
and reinitialize the touchpad.
...
> > > > > @@ -898,8 +528,12 @@ static int cyapa_remove(struct i2c_client *client)
> > > > > struct cyapa *cyapa = i2c_get_clientdata(client);
> > > > >
> > > >
> > > > You schedule detect asynchronously so you need to make sure it is not
> > running
> > > > (and will not be running) when you try to unbind the driver here.
> > >
> > > Thanks. I will update it in next release.
> > >
> > > To avoid this problem, a removed flag is introduced and is also synced by
> > the
> > > cyapa->state_sync_lock mutex lock. So when this driver is removing, it's
> > sure
> > > that the detecting asynchronous thread is not running, and if there's any
> > event
> > > cause the asynchronous thread is entered, it will do nothing and will exit
> > immediately.
> >
> > Peeking at the other version of the patch you do:
> >
> > cyapa_remove()
> > {
> > ...
> > cyapa->removed = true;
> > ...
> > kfree(cyapa);
> > }
> >
> > and
> >
> > void cyapa_detect_async(void *data, async_cookie_t cookie)
> > {
> > ...
> > if (cyapa->removed) {
> > mutex_unlock(&cyapa->state_sync_lock);
> > return;
> > }
> > ...
> > }
> >
> > That's not going to work.
> >
> > Getting asynchronous behavior by individual driver is hard and I believe
> > Luis Rordiguez is working on allowing to do that is the driver core. So
> > I would recommend to switch driver to do synchronous probing, at least
> > for now, and figure out other kinks first.
> >
> > Thanks.
> >
>
> The consideration of the thread asynchronous issue is that:
> The detection thread has been scheduled, but not run yet, and at the time,
> the driver itself has been removed, so when later the detection thread is
> triggered to run, the memory of cyapa driver has been freed, and cause error, right?
Right.
>
> I have an idea as below, please help review.
> Add variable cyapa->detect_thread_actived to trace the number of the threads were scheduled,
> and when the driver module is in removing, it will wait all scheduled threads has been run.
> That is when a thread is scheduled, cyapa->detect_thread_actived will be increased,
> when the thread is executed and finished, the cyapa->detect_thread_actived will be decrreased,
> so, when the value of cyapa->detect_thread_actived is 0, it must be no thread scheduled for running.
> So when at this time, if the driver module is marked for removed, the remove() thread that waiting
> on the cyapa->detect_thread_completed will be issue, and then driver is safe to be removed.
> The code is similar as below.
>
> For each time, before the cyapa_detect_async() function is called or scheduled,
> ... {
> ...
> atomic_inc(&cyapa->detect_thread_actived);
> async_schedule(cyapa_detect_async, cyapa);
> ...
> }
>
> void cyapa_detect_async(void *data, async_cookie_t cookie)
> {
> ...
> if (cyapa->removed) {
> mutex_unlock(&cyapa->state_sync_lock);
> return;
> }
> ...
> free_irq(cyapa->irq, cyapa);
>
> /* Wait until all scheduled thread exited. */
> if (atomic_read(&cyapa->detect_thread_actived)
> wait_for_completion_timeout(&cyapa->detect_thread_completed,
> msecs_to_jiffies(4000));
> ...
> }
>
> void cyapa_detect_async(void *data, async_cookie_t cookie)
> {
> struct cyapa *cyapa = (struct cyapa *)data;
>
> mutex_lock(&cyapa->state_sync_lock);
> if (cyapa->removed) {
> mutex_unlock(&cyapa->state_sync_lock);
> if (atomic_dec_and_test(&cyapa->detect_thread_actived)) {
> /* Now, no thread runing, safe to remove this driver. */
> complete(&cyapa->detect_thread_completed);
> }
> goto return;
> }
>
> /* Keep synchronized with sys interface process threads. */
> cyapa_detect(cyapa);
>
> mutex_unlock(&cyapa->state_sync_lock);
> atomic_dec(&cyapa->detect_thread_actived);
> }
It can be made to work, but as I mentioned we are working on bringing
asynchronous probing into device core in one form or another so I'd
rather not be doing it in the driver at all at this time.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v3 0/3] cap1106: add support for cap11xx variants
From: Matt Ranostay @ 2014-09-25 5:24 UTC (permalink / raw)
To: galak, dmitry.torokhov, zonque, linux-input, linux-kernel,
robh+dt
Cc: devicetree, Matt Ranostay
Changes from v2:
* Rename apply relevant CAP1106 references to CAP11XX to make generic
for variant support
Matt Ranostay (3):
cap11xx: make driver generic for variant support
cap11xx: Add support for various cap11xx devices
cap11xx: support for irq-active-high option
.../devicetree/bindings/input/cap1106.txt | 53 ---
.../devicetree/bindings/input/cap11xx.txt | 59 ++++
drivers/input/keyboard/Kconfig | 8 +-
drivers/input/keyboard/Makefile | 2 +-
drivers/input/keyboard/cap1106.c | 341 -------------------
drivers/input/keyboard/cap11xx.c | 372 +++++++++++++++++++++
6 files changed, 436 insertions(+), 399 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/cap1106.txt
create mode 100644 Documentation/devicetree/bindings/input/cap11xx.txt
delete mode 100644 drivers/input/keyboard/cap1106.c
create mode 100644 drivers/input/keyboard/cap11xx.c
--
1.9.1
^ permalink raw reply
* [PATCH v3 2/3] cap11xx: Add support for various cap11xx devices
From: Matt Ranostay @ 2014-09-25 5:24 UTC (permalink / raw)
To: galak, dmitry.torokhov, zonque, linux-input, linux-kernel,
robh+dt
Cc: devicetree, Matt Ranostay
In-Reply-To: <1411622662-16581-1-git-send-email-mranostay@gmail.com>
Several other variants of the cap11xx device exists with a varying
number of capacitance detection channels. Add support for creating
the channels dynamically.
Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
.../devicetree/bindings/input/cap11xx.txt | 3 +-
drivers/input/keyboard/cap11xx.c | 65 +++++++++++++++-------
2 files changed, 46 insertions(+), 22 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/cap11xx.txt b/Documentation/devicetree/bindings/input/cap11xx.txt
index 738f5f3..8031aa5 100644
--- a/Documentation/devicetree/bindings/input/cap11xx.txt
+++ b/Documentation/devicetree/bindings/input/cap11xx.txt
@@ -7,9 +7,10 @@ Required properties:
compatible: Must be on the following, depending on the model:
"microchip,cap1106"
+ "microchip,cap1126"
+ "microchip,cap1188"
reg: The I2C slave address of the device.
- Only 0x28 is valid.
interrupts: Property describing the interrupt line the
device's ALERT#/CM_IRQ# pin is connected to.
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index 0da2e83..adb979e 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -1,7 +1,6 @@
/*
* Input driver for Microchip CAP11xx based capacitive touch sensors
*
- *
* (c) 2014 Daniel Mack <linux@zonque.org>
*
* This program is free software; you can redistribute it and/or modify
@@ -54,8 +53,6 @@
#define CAP11XX_REG_MANUFACTURER_ID 0xfe
#define CAP11XX_REG_REVISION 0xff
-#define CAP11XX_NUM_CHN 6
-#define CAP11XX_PRODUCT_ID 0x55
#define CAP11XX_MANUFACTURER_ID 0x5d
struct cap11xx_priv {
@@ -63,7 +60,25 @@ struct cap11xx_priv {
struct input_dev *idev;
/* config */
- unsigned short keycodes[CAP11XX_NUM_CHN];
+ u32 *keycodes;
+ unsigned int num_channels;
+};
+
+struct cap11xx_hw_model {
+ uint8_t product_id;
+ unsigned int num_channels;
+};
+
+enum {
+ CAP1106,
+ CAP1126,
+ CAP1188,
+};
+
+struct cap11xx_hw_model cap11xx_devices[] = {
+ [CAP1106] = { .product_id = 0x55, .num_channels = 6 },
+ [CAP1126] = { .product_id = 0x53, .num_channels = 6 },
+ [CAP1188] = { .product_id = 0x50, .num_channels = 8 },
};
static const struct reg_default cap11xx_reg_defaults[] = {
@@ -150,7 +165,7 @@ static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
if (ret < 0)
goto out;
- for (i = 0; i < CAP11XX_NUM_CHN; i++)
+ for (i = 0; i < priv->num_channels; i++)
input_report_key(priv->idev, priv->keycodes[i],
status & (1 << i));
@@ -187,14 +202,23 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
struct device *dev = &i2c_client->dev;
struct cap11xx_priv *priv;
struct device_node *node;
+ struct cap11xx_hw_model *cap = &cap11xx_devices[id->driver_data];
int i, error, irq, gain = 0;
unsigned int val, rev;
- u32 gain32, keycodes[CAP11XX_NUM_CHN];
+ u32 gain32;
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
+ BUG_ON(!cap->num_channels);
+
+ priv->num_channels = cap->num_channels;
+ priv->keycodes = devm_kcalloc(dev,
+ priv->num_channels, sizeof(u32), GFP_KERNEL);
+ if (!priv->keycodes)
+ return -ENOMEM;
+
priv->regmap = devm_regmap_init_i2c(i2c_client, &cap11xx_regmap_config);
if (IS_ERR(priv->regmap))
return PTR_ERR(priv->regmap);
@@ -203,9 +227,9 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
if (error)
return error;
- if (val != CAP11XX_PRODUCT_ID) {
+ if (val != cap->product_id) {
dev_err(dev, "Product ID: Got 0x%02x, expected 0x%02x\n",
- val, CAP11XX_PRODUCT_ID);
+ val, cap->product_id);
return -ENODEV;
}
@@ -234,17 +258,12 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
}
- BUILD_BUG_ON(ARRAY_SIZE(keycodes) != ARRAY_SIZE(priv->keycodes));
-
/* Provide some useful defaults */
- for (i = 0; i < ARRAY_SIZE(keycodes); i++)
- keycodes[i] = KEY_A + i;
+ for (i = 0; i < priv->num_channels; i++)
+ priv->keycodes[i] = KEY_A + i;
of_property_read_u32_array(node, "linux,keycodes",
- keycodes, ARRAY_SIZE(keycodes));
-
- for (i = 0; i < ARRAY_SIZE(keycodes); i++)
- priv->keycodes[i] = keycodes[i];
+ priv->keycodes, priv->num_channels);
error = regmap_update_bits(priv->regmap, CAP11XX_REG_MAIN_CONTROL,
CAP11XX_REG_MAIN_CONTROL_GAIN_MASK,
@@ -268,17 +287,17 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
if (of_property_read_bool(node, "autorepeat"))
__set_bit(EV_REP, priv->idev->evbit);
- for (i = 0; i < CAP11XX_NUM_CHN; i++)
+ for (i = 0; i < priv->num_channels; i++)
__set_bit(priv->keycodes[i], priv->idev->keybit);
__clear_bit(KEY_RESERVED, priv->idev->keybit);
priv->idev->keycode = priv->keycodes;
- priv->idev->keycodesize = sizeof(priv->keycodes[0]);
- priv->idev->keycodemax = ARRAY_SIZE(priv->keycodes);
+ priv->idev->keycodesize = sizeof(u32);
+ priv->idev->keycodemax = priv->num_channels;
priv->idev->id.vendor = CAP11XX_MANUFACTURER_ID;
- priv->idev->id.product = CAP11XX_PRODUCT_ID;
+ priv->idev->id.product = cap->product_id;
priv->idev->id.version = rev;
priv->idev->open = cap11xx_input_open;
@@ -312,12 +331,16 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
static const struct of_device_id cap11xx_dt_ids[] = {
{ .compatible = "microchip,cap1106", },
+ { .compatible = "microchip,cap1126", },
+ { .compatible = "microchip,cap1188", },
{}
};
MODULE_DEVICE_TABLE(of, cap11xx_dt_ids);
static const struct i2c_device_id cap11xx_i2c_ids[] = {
- { "cap1106", 0 },
+ { "cap1106", CAP1106 },
+ { "cap1126", CAP1126 },
+ { "cap1188", CAP1188 },
{}
};
MODULE_DEVICE_TABLE(i2c, cap11xx_i2c_ids);
--
1.9.1
^ permalink raw reply related
* [PATCH v3 1/3] cap11xx: make driver generic for variant support
From: Matt Ranostay @ 2014-09-25 5:24 UTC (permalink / raw)
To: galak, dmitry.torokhov, zonque, linux-input, linux-kernel,
robh+dt
Cc: devicetree, Matt Ranostay
In-Reply-To: <1411622662-16581-1-git-send-email-mranostay@gmail.com>
cap1106 driver can support much more one device make the driver
generic for support of similiar parts.
Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
.../devicetree/bindings/input/cap1106.txt | 53 ----
.../devicetree/bindings/input/cap11xx.txt | 54 ++++
drivers/input/keyboard/Kconfig | 8 +-
drivers/input/keyboard/Makefile | 2 +-
drivers/input/keyboard/cap1106.c | 341 ---------------------
drivers/input/keyboard/cap11xx.c | 340 ++++++++++++++++++++
6 files changed, 399 insertions(+), 399 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/cap1106.txt
create mode 100644 Documentation/devicetree/bindings/input/cap11xx.txt
delete mode 100644 drivers/input/keyboard/cap1106.c
create mode 100644 drivers/input/keyboard/cap11xx.c
diff --git a/Documentation/devicetree/bindings/input/cap1106.txt b/Documentation/devicetree/bindings/input/cap1106.txt
deleted file mode 100644
index 4b46390..0000000
--- a/Documentation/devicetree/bindings/input/cap1106.txt
+++ /dev/null
@@ -1,53 +0,0 @@
-Device tree bindings for Microchip CAP1106, 6 channel capacitive touch sensor
-
-The node for this driver must be a child of a I2C controller node, as the
-device communication via I2C only.
-
-Required properties:
-
- compatible: Must be "microchip,cap1106"
-
- reg: The I2C slave address of the device.
- Only 0x28 is valid.
-
- interrupts: Property describing the interrupt line the
- device's ALERT#/CM_IRQ# pin is connected to.
- The device only has one interrupt source.
-
-Optional properties:
-
- autorepeat: Enables the Linux input system's autorepeat
- feature on the input device.
-
- microchip,sensor-gain: Defines the gain of the sensor circuitry. This
- effectively controls the sensitivity, as a
- smaller delta capacitance is required to
- generate the same delta count values.
- Valid values are 1, 2, 4, and 8.
- By default, a gain of 1 is set.
-
- linux,keycodes: Specifies an array of numeric keycode values to
- be used for the channels. If this property is
- omitted, KEY_A, KEY_B, etc are used as
- defaults. The array must have exactly six
- entries.
-
-Example:
-
-i2c_controller {
- cap1106@28 {
- compatible = "microchip,cap1106";
- interrupt-parent = <&gpio1>;
- interrupts = <0 0>;
- reg = <0x28>;
- autorepeat;
- microchip,sensor-gain = <2>;
-
- linux,keycodes = <103 /* KEY_UP */
- 106 /* KEY_RIGHT */
- 108 /* KEY_DOWN */
- 105 /* KEY_LEFT */
- 109 /* KEY_PAGEDOWN */
- 104>; /* KEY_PAGEUP */
- };
-}
diff --git a/Documentation/devicetree/bindings/input/cap11xx.txt b/Documentation/devicetree/bindings/input/cap11xx.txt
new file mode 100644
index 0000000..738f5f3
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cap11xx.txt
@@ -0,0 +1,54 @@
+Device tree bindings for Microchip CAP1106 based capacitive touch sensor
+
+The node for this driver must be a child of a I2C controller node, as the
+device communication via I2C only.
+
+Required properties:
+
+ compatible: Must be on the following, depending on the model:
+ "microchip,cap1106"
+
+ reg: The I2C slave address of the device.
+ Only 0x28 is valid.
+
+ interrupts: Property describing the interrupt line the
+ device's ALERT#/CM_IRQ# pin is connected to.
+ The device only has one interrupt source.
+
+Optional properties:
+
+ autorepeat: Enables the Linux input system's autorepeat
+ feature on the input device.
+
+ microchip,sensor-gain: Defines the gain of the sensor circuitry. This
+ effectively controls the sensitivity, as a
+ smaller delta capacitance is required to
+ generate the same delta count values.
+ Valid values are 1, 2, 4, and 8.
+ By default, a gain of 1 is set.
+
+ linux,keycodes: Specifies an array of numeric keycode values to
+ be used for the channels. If this property is
+ omitted, KEY_A, KEY_B, etc are used as
+ defaults. The array must have exactly six
+ entries.
+
+Example:
+
+i2c_controller {
+ cap1106@28 {
+ compatible = "microchip,cap1106";
+ interrupt-parent = <&gpio1>;
+ interrupts = <0 0>;
+ reg = <0x28>;
+ autorepeat;
+ microchip,sensor-gain = <2>;
+
+ linux,keycodes = <103 /* KEY_UP */
+ 106 /* KEY_RIGHT */
+ 108 /* KEY_DOWN */
+ 105 /* KEY_LEFT */
+ 109 /* KEY_PAGEDOWN */
+ 104>; /* KEY_PAGEUP */
+ };
+}
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index a3958c6..96ee26c 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -665,14 +665,14 @@ config KEYBOARD_CROS_EC
To compile this driver as a module, choose M here: the
module will be called cros_ec_keyb.
-config KEYBOARD_CAP1106
- tristate "Microchip CAP1106 touch sensor"
+config KEYBOARD_CAP11XX
+ tristate "Microchip CAP11XX based touch sensors"
depends on OF && I2C
select REGMAP_I2C
help
- Say Y here to enable the CAP1106 touch sensor driver.
+ Say Y here to enable the CAP11XX touch sensor driver.
To compile this driver as a module, choose M here: the
- module will be called cap1106.
+ module will be called cap11xx.
endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 0a33456..febafa5 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -11,7 +11,7 @@ obj-$(CONFIG_KEYBOARD_AMIGA) += amikbd.o
obj-$(CONFIG_KEYBOARD_ATARI) += atakbd.o
obj-$(CONFIG_KEYBOARD_ATKBD) += atkbd.o
obj-$(CONFIG_KEYBOARD_BFIN) += bf54x-keys.o
-obj-$(CONFIG_KEYBOARD_CAP1106) += cap1106.o
+obj-$(CONFIG_KEYBOARD_CAP11XX) += cap11xx.o
obj-$(CONFIG_KEYBOARD_CLPS711X) += clps711x-keypad.o
obj-$(CONFIG_KEYBOARD_CROS_EC) += cros_ec_keyb.o
obj-$(CONFIG_KEYBOARD_DAVINCI) += davinci_keyscan.o
diff --git a/drivers/input/keyboard/cap1106.c b/drivers/input/keyboard/cap1106.c
deleted file mode 100644
index d70b65a..0000000
--- a/drivers/input/keyboard/cap1106.c
+++ /dev/null
@@ -1,341 +0,0 @@
-/*
- * Input driver for Microchip CAP1106, 6 channel capacitive touch sensor
- *
- * http://www.microchip.com/wwwproducts/Devices.aspx?product=CAP1106
- *
- * (c) 2014 Daniel Mack <linux@zonque.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/input.h>
-#include <linux/of_irq.h>
-#include <linux/regmap.h>
-#include <linux/i2c.h>
-#include <linux/gpio/consumer.h>
-
-#define CAP1106_REG_MAIN_CONTROL 0x00
-#define CAP1106_REG_MAIN_CONTROL_GAIN_SHIFT (6)
-#define CAP1106_REG_MAIN_CONTROL_GAIN_MASK (0xc0)
-#define CAP1106_REG_MAIN_CONTROL_DLSEEP BIT(4)
-#define CAP1106_REG_GENERAL_STATUS 0x02
-#define CAP1106_REG_SENSOR_INPUT 0x03
-#define CAP1106_REG_NOISE_FLAG_STATUS 0x0a
-#define CAP1106_REG_SENOR_DELTA(X) (0x10 + (X))
-#define CAP1106_REG_SENSITIVITY_CONTROL 0x1f
-#define CAP1106_REG_CONFIG 0x20
-#define CAP1106_REG_SENSOR_ENABLE 0x21
-#define CAP1106_REG_SENSOR_CONFIG 0x22
-#define CAP1106_REG_SENSOR_CONFIG2 0x23
-#define CAP1106_REG_SAMPLING_CONFIG 0x24
-#define CAP1106_REG_CALIBRATION 0x26
-#define CAP1106_REG_INT_ENABLE 0x27
-#define CAP1106_REG_REPEAT_RATE 0x28
-#define CAP1106_REG_MT_CONFIG 0x2a
-#define CAP1106_REG_MT_PATTERN_CONFIG 0x2b
-#define CAP1106_REG_MT_PATTERN 0x2d
-#define CAP1106_REG_RECALIB_CONFIG 0x2f
-#define CAP1106_REG_SENSOR_THRESH(X) (0x30 + (X))
-#define CAP1106_REG_SENSOR_NOISE_THRESH 0x38
-#define CAP1106_REG_STANDBY_CHANNEL 0x40
-#define CAP1106_REG_STANDBY_CONFIG 0x41
-#define CAP1106_REG_STANDBY_SENSITIVITY 0x42
-#define CAP1106_REG_STANDBY_THRESH 0x43
-#define CAP1106_REG_CONFIG2 0x44
-#define CAP1106_REG_SENSOR_BASE_CNT(X) (0x50 + (X))
-#define CAP1106_REG_SENSOR_CALIB (0xb1 + (X))
-#define CAP1106_REG_SENSOR_CALIB_LSB1 0xb9
-#define CAP1106_REG_SENSOR_CALIB_LSB2 0xba
-#define CAP1106_REG_PRODUCT_ID 0xfd
-#define CAP1106_REG_MANUFACTURER_ID 0xfe
-#define CAP1106_REG_REVISION 0xff
-
-#define CAP1106_NUM_CHN 6
-#define CAP1106_PRODUCT_ID 0x55
-#define CAP1106_MANUFACTURER_ID 0x5d
-
-struct cap1106_priv {
- struct regmap *regmap;
- struct input_dev *idev;
-
- /* config */
- unsigned short keycodes[CAP1106_NUM_CHN];
-};
-
-static const struct reg_default cap1106_reg_defaults[] = {
- { CAP1106_REG_MAIN_CONTROL, 0x00 },
- { CAP1106_REG_GENERAL_STATUS, 0x00 },
- { CAP1106_REG_SENSOR_INPUT, 0x00 },
- { CAP1106_REG_NOISE_FLAG_STATUS, 0x00 },
- { CAP1106_REG_SENSITIVITY_CONTROL, 0x2f },
- { CAP1106_REG_CONFIG, 0x20 },
- { CAP1106_REG_SENSOR_ENABLE, 0x3f },
- { CAP1106_REG_SENSOR_CONFIG, 0xa4 },
- { CAP1106_REG_SENSOR_CONFIG2, 0x07 },
- { CAP1106_REG_SAMPLING_CONFIG, 0x39 },
- { CAP1106_REG_CALIBRATION, 0x00 },
- { CAP1106_REG_INT_ENABLE, 0x3f },
- { CAP1106_REG_REPEAT_RATE, 0x3f },
- { CAP1106_REG_MT_CONFIG, 0x80 },
- { CAP1106_REG_MT_PATTERN_CONFIG, 0x00 },
- { CAP1106_REG_MT_PATTERN, 0x3f },
- { CAP1106_REG_RECALIB_CONFIG, 0x8a },
- { CAP1106_REG_SENSOR_THRESH(0), 0x40 },
- { CAP1106_REG_SENSOR_THRESH(1), 0x40 },
- { CAP1106_REG_SENSOR_THRESH(2), 0x40 },
- { CAP1106_REG_SENSOR_THRESH(3), 0x40 },
- { CAP1106_REG_SENSOR_THRESH(4), 0x40 },
- { CAP1106_REG_SENSOR_THRESH(5), 0x40 },
- { CAP1106_REG_SENSOR_NOISE_THRESH, 0x01 },
- { CAP1106_REG_STANDBY_CHANNEL, 0x00 },
- { CAP1106_REG_STANDBY_CONFIG, 0x39 },
- { CAP1106_REG_STANDBY_SENSITIVITY, 0x02 },
- { CAP1106_REG_STANDBY_THRESH, 0x40 },
- { CAP1106_REG_CONFIG2, 0x40 },
- { CAP1106_REG_SENSOR_CALIB_LSB1, 0x00 },
- { CAP1106_REG_SENSOR_CALIB_LSB2, 0x00 },
-};
-
-static bool cap1106_volatile_reg(struct device *dev, unsigned int reg)
-{
- switch (reg) {
- case CAP1106_REG_MAIN_CONTROL:
- case CAP1106_REG_SENSOR_INPUT:
- case CAP1106_REG_SENOR_DELTA(0):
- case CAP1106_REG_SENOR_DELTA(1):
- case CAP1106_REG_SENOR_DELTA(2):
- case CAP1106_REG_SENOR_DELTA(3):
- case CAP1106_REG_SENOR_DELTA(4):
- case CAP1106_REG_SENOR_DELTA(5):
- case CAP1106_REG_PRODUCT_ID:
- case CAP1106_REG_MANUFACTURER_ID:
- case CAP1106_REG_REVISION:
- return true;
- }
-
- return false;
-}
-
-static const struct regmap_config cap1106_regmap_config = {
- .reg_bits = 8,
- .val_bits = 8,
-
- .max_register = CAP1106_REG_REVISION,
- .reg_defaults = cap1106_reg_defaults,
-
- .num_reg_defaults = ARRAY_SIZE(cap1106_reg_defaults),
- .cache_type = REGCACHE_RBTREE,
- .volatile_reg = cap1106_volatile_reg,
-};
-
-static irqreturn_t cap1106_thread_func(int irq_num, void *data)
-{
- struct cap1106_priv *priv = data;
- unsigned int status;
- int ret, i;
-
- /*
- * Deassert interrupt. This needs to be done before reading the status
- * registers, which will not carry valid values otherwise.
- */
- ret = regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL, 1, 0);
- if (ret < 0)
- goto out;
-
- ret = regmap_read(priv->regmap, CAP1106_REG_SENSOR_INPUT, &status);
- if (ret < 0)
- goto out;
-
- for (i = 0; i < CAP1106_NUM_CHN; i++)
- input_report_key(priv->idev, priv->keycodes[i],
- status & (1 << i));
-
- input_sync(priv->idev);
-
-out:
- return IRQ_HANDLED;
-}
-
-static int cap1106_set_sleep(struct cap1106_priv *priv, bool sleep)
-{
- return regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL,
- CAP1106_REG_MAIN_CONTROL_DLSEEP,
- sleep ? CAP1106_REG_MAIN_CONTROL_DLSEEP : 0);
-}
-
-static int cap1106_input_open(struct input_dev *idev)
-{
- struct cap1106_priv *priv = input_get_drvdata(idev);
-
- return cap1106_set_sleep(priv, false);
-}
-
-static void cap1106_input_close(struct input_dev *idev)
-{
- struct cap1106_priv *priv = input_get_drvdata(idev);
-
- cap1106_set_sleep(priv, true);
-}
-
-static int cap1106_i2c_probe(struct i2c_client *i2c_client,
- const struct i2c_device_id *id)
-{
- struct device *dev = &i2c_client->dev;
- struct cap1106_priv *priv;
- struct device_node *node;
- int i, error, irq, gain = 0;
- unsigned int val, rev;
- u32 gain32, keycodes[CAP1106_NUM_CHN];
-
- priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
-
- priv->regmap = devm_regmap_init_i2c(i2c_client, &cap1106_regmap_config);
- if (IS_ERR(priv->regmap))
- return PTR_ERR(priv->regmap);
-
- error = regmap_read(priv->regmap, CAP1106_REG_PRODUCT_ID, &val);
- if (error)
- return error;
-
- if (val != CAP1106_PRODUCT_ID) {
- dev_err(dev, "Product ID: Got 0x%02x, expected 0x%02x\n",
- val, CAP1106_PRODUCT_ID);
- return -ENODEV;
- }
-
- error = regmap_read(priv->regmap, CAP1106_REG_MANUFACTURER_ID, &val);
- if (error)
- return error;
-
- if (val != CAP1106_MANUFACTURER_ID) {
- dev_err(dev, "Manufacturer ID: Got 0x%02x, expected 0x%02x\n",
- val, CAP1106_MANUFACTURER_ID);
- return -ENODEV;
- }
-
- error = regmap_read(priv->regmap, CAP1106_REG_REVISION, &rev);
- if (error < 0)
- return error;
-
- dev_info(dev, "CAP1106 detected, revision 0x%02x\n", rev);
- i2c_set_clientdata(i2c_client, priv);
- node = dev->of_node;
-
- if (!of_property_read_u32(node, "microchip,sensor-gain", &gain32)) {
- if (is_power_of_2(gain32) && gain32 <= 8)
- gain = ilog2(gain32);
- else
- dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
- }
-
- BUILD_BUG_ON(ARRAY_SIZE(keycodes) != ARRAY_SIZE(priv->keycodes));
-
- /* Provide some useful defaults */
- for (i = 0; i < ARRAY_SIZE(keycodes); i++)
- keycodes[i] = KEY_A + i;
-
- of_property_read_u32_array(node, "linux,keycodes",
- keycodes, ARRAY_SIZE(keycodes));
-
- for (i = 0; i < ARRAY_SIZE(keycodes); i++)
- priv->keycodes[i] = keycodes[i];
-
- error = regmap_update_bits(priv->regmap, CAP1106_REG_MAIN_CONTROL,
- CAP1106_REG_MAIN_CONTROL_GAIN_MASK,
- gain << CAP1106_REG_MAIN_CONTROL_GAIN_SHIFT);
- if (error)
- return error;
-
- /* Disable autorepeat. The Linux input system has its own handling. */
- error = regmap_write(priv->regmap, CAP1106_REG_REPEAT_RATE, 0);
- if (error)
- return error;
-
- priv->idev = devm_input_allocate_device(dev);
- if (!priv->idev)
- return -ENOMEM;
-
- priv->idev->name = "CAP1106 capacitive touch sensor";
- priv->idev->id.bustype = BUS_I2C;
- priv->idev->evbit[0] = BIT_MASK(EV_KEY);
-
- if (of_property_read_bool(node, "autorepeat"))
- __set_bit(EV_REP, priv->idev->evbit);
-
- for (i = 0; i < CAP1106_NUM_CHN; i++)
- __set_bit(priv->keycodes[i], priv->idev->keybit);
-
- __clear_bit(KEY_RESERVED, priv->idev->keybit);
-
- priv->idev->keycode = priv->keycodes;
- priv->idev->keycodesize = sizeof(priv->keycodes[0]);
- priv->idev->keycodemax = ARRAY_SIZE(priv->keycodes);
-
- priv->idev->id.vendor = CAP1106_MANUFACTURER_ID;
- priv->idev->id.product = CAP1106_PRODUCT_ID;
- priv->idev->id.version = rev;
-
- priv->idev->open = cap1106_input_open;
- priv->idev->close = cap1106_input_close;
-
- input_set_drvdata(priv->idev, priv);
-
- /*
- * Put the device in deep sleep mode for now.
- * ->open() will bring it back once the it is actually needed.
- */
- cap1106_set_sleep(priv, true);
-
- error = input_register_device(priv->idev);
- if (error)
- return error;
-
- irq = irq_of_parse_and_map(node, 0);
- if (!irq) {
- dev_err(dev, "Unable to parse or map IRQ\n");
- return -ENXIO;
- }
-
- error = devm_request_threaded_irq(dev, irq, NULL, cap1106_thread_func,
- IRQF_ONESHOT, dev_name(dev), priv);
- if (error)
- return error;
-
- return 0;
-}
-
-static const struct of_device_id cap1106_dt_ids[] = {
- { .compatible = "microchip,cap1106", },
- {}
-};
-MODULE_DEVICE_TABLE(of, cap1106_dt_ids);
-
-static const struct i2c_device_id cap1106_i2c_ids[] = {
- { "cap1106", 0 },
- {}
-};
-MODULE_DEVICE_TABLE(i2c, cap1106_i2c_ids);
-
-static struct i2c_driver cap1106_i2c_driver = {
- .driver = {
- .name = "cap1106",
- .owner = THIS_MODULE,
- .of_match_table = cap1106_dt_ids,
- },
- .id_table = cap1106_i2c_ids,
- .probe = cap1106_i2c_probe,
-};
-
-module_i2c_driver(cap1106_i2c_driver);
-
-MODULE_ALIAS("platform:cap1106");
-MODULE_DESCRIPTION("Microchip CAP1106 driver");
-MODULE_AUTHOR("Daniel Mack <linux@zonque.org>");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
new file mode 100644
index 0000000..0da2e83
--- /dev/null
+++ b/drivers/input/keyboard/cap11xx.c
@@ -0,0 +1,340 @@
+/*
+ * Input driver for Microchip CAP11xx based capacitive touch sensors
+ *
+ *
+ * (c) 2014 Daniel Mack <linux@zonque.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/input.h>
+#include <linux/of_irq.h>
+#include <linux/regmap.h>
+#include <linux/i2c.h>
+#include <linux/gpio/consumer.h>
+
+#define CAP11XX_REG_MAIN_CONTROL 0x00
+#define CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT (6)
+#define CAP11XX_REG_MAIN_CONTROL_GAIN_MASK (0xc0)
+#define CAP11XX_REG_MAIN_CONTROL_DLSEEP BIT(4)
+#define CAP11XX_REG_GENERAL_STATUS 0x02
+#define CAP11XX_REG_SENSOR_INPUT 0x03
+#define CAP11XX_REG_NOISE_FLAG_STATUS 0x0a
+#define CAP11XX_REG_SENOR_DELTA(X) (0x10 + (X))
+#define CAP11XX_REG_SENSITIVITY_CONTROL 0x1f
+#define CAP11XX_REG_CONFIG 0x20
+#define CAP11XX_REG_SENSOR_ENABLE 0x21
+#define CAP11XX_REG_SENSOR_CONFIG 0x22
+#define CAP11XX_REG_SENSOR_CONFIG2 0x23
+#define CAP11XX_REG_SAMPLING_CONFIG 0x24
+#define CAP11XX_REG_CALIBRATION 0x26
+#define CAP11XX_REG_INT_ENABLE 0x27
+#define CAP11XX_REG_REPEAT_RATE 0x28
+#define CAP11XX_REG_MT_CONFIG 0x2a
+#define CAP11XX_REG_MT_PATTERN_CONFIG 0x2b
+#define CAP11XX_REG_MT_PATTERN 0x2d
+#define CAP11XX_REG_RECALIB_CONFIG 0x2f
+#define CAP11XX_REG_SENSOR_THRESH(X) (0x30 + (X))
+#define CAP11XX_REG_SENSOR_NOISE_THRESH 0x38
+#define CAP11XX_REG_STANDBY_CHANNEL 0x40
+#define CAP11XX_REG_STANDBY_CONFIG 0x41
+#define CAP11XX_REG_STANDBY_SENSITIVITY 0x42
+#define CAP11XX_REG_STANDBY_THRESH 0x43
+#define CAP11XX_REG_CONFIG2 0x44
+#define CAP11XX_REG_SENSOR_BASE_CNT(X) (0x50 + (X))
+#define CAP11XX_REG_SENSOR_CALIB (0xb1 + (X))
+#define CAP11XX_REG_SENSOR_CALIB_LSB1 0xb9
+#define CAP11XX_REG_SENSOR_CALIB_LSB2 0xba
+#define CAP11XX_REG_PRODUCT_ID 0xfd
+#define CAP11XX_REG_MANUFACTURER_ID 0xfe
+#define CAP11XX_REG_REVISION 0xff
+
+#define CAP11XX_NUM_CHN 6
+#define CAP11XX_PRODUCT_ID 0x55
+#define CAP11XX_MANUFACTURER_ID 0x5d
+
+struct cap11xx_priv {
+ struct regmap *regmap;
+ struct input_dev *idev;
+
+ /* config */
+ unsigned short keycodes[CAP11XX_NUM_CHN];
+};
+
+static const struct reg_default cap11xx_reg_defaults[] = {
+ { CAP11XX_REG_MAIN_CONTROL, 0x00 },
+ { CAP11XX_REG_GENERAL_STATUS, 0x00 },
+ { CAP11XX_REG_SENSOR_INPUT, 0x00 },
+ { CAP11XX_REG_NOISE_FLAG_STATUS, 0x00 },
+ { CAP11XX_REG_SENSITIVITY_CONTROL, 0x2f },
+ { CAP11XX_REG_CONFIG, 0x20 },
+ { CAP11XX_REG_SENSOR_ENABLE, 0x3f },
+ { CAP11XX_REG_SENSOR_CONFIG, 0xa4 },
+ { CAP11XX_REG_SENSOR_CONFIG2, 0x07 },
+ { CAP11XX_REG_SAMPLING_CONFIG, 0x39 },
+ { CAP11XX_REG_CALIBRATION, 0x00 },
+ { CAP11XX_REG_INT_ENABLE, 0x3f },
+ { CAP11XX_REG_REPEAT_RATE, 0x3f },
+ { CAP11XX_REG_MT_CONFIG, 0x80 },
+ { CAP11XX_REG_MT_PATTERN_CONFIG, 0x00 },
+ { CAP11XX_REG_MT_PATTERN, 0x3f },
+ { CAP11XX_REG_RECALIB_CONFIG, 0x8a },
+ { CAP11XX_REG_SENSOR_THRESH(0), 0x40 },
+ { CAP11XX_REG_SENSOR_THRESH(1), 0x40 },
+ { CAP11XX_REG_SENSOR_THRESH(2), 0x40 },
+ { CAP11XX_REG_SENSOR_THRESH(3), 0x40 },
+ { CAP11XX_REG_SENSOR_THRESH(4), 0x40 },
+ { CAP11XX_REG_SENSOR_THRESH(5), 0x40 },
+ { CAP11XX_REG_SENSOR_NOISE_THRESH, 0x01 },
+ { CAP11XX_REG_STANDBY_CHANNEL, 0x00 },
+ { CAP11XX_REG_STANDBY_CONFIG, 0x39 },
+ { CAP11XX_REG_STANDBY_SENSITIVITY, 0x02 },
+ { CAP11XX_REG_STANDBY_THRESH, 0x40 },
+ { CAP11XX_REG_CONFIG2, 0x40 },
+ { CAP11XX_REG_SENSOR_CALIB_LSB1, 0x00 },
+ { CAP11XX_REG_SENSOR_CALIB_LSB2, 0x00 },
+};
+
+static bool cap11xx_volatile_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case CAP11XX_REG_MAIN_CONTROL:
+ case CAP11XX_REG_SENSOR_INPUT:
+ case CAP11XX_REG_SENOR_DELTA(0):
+ case CAP11XX_REG_SENOR_DELTA(1):
+ case CAP11XX_REG_SENOR_DELTA(2):
+ case CAP11XX_REG_SENOR_DELTA(3):
+ case CAP11XX_REG_SENOR_DELTA(4):
+ case CAP11XX_REG_SENOR_DELTA(5):
+ case CAP11XX_REG_PRODUCT_ID:
+ case CAP11XX_REG_MANUFACTURER_ID:
+ case CAP11XX_REG_REVISION:
+ return true;
+ }
+
+ return false;
+}
+
+static const struct regmap_config cap11xx_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+
+ .max_register = CAP11XX_REG_REVISION,
+ .reg_defaults = cap11xx_reg_defaults,
+
+ .num_reg_defaults = ARRAY_SIZE(cap11xx_reg_defaults),
+ .cache_type = REGCACHE_RBTREE,
+ .volatile_reg = cap11xx_volatile_reg,
+};
+
+static irqreturn_t cap11xx_thread_func(int irq_num, void *data)
+{
+ struct cap11xx_priv *priv = data;
+ unsigned int status;
+ int ret, i;
+
+ /*
+ * Deassert interrupt. This needs to be done before reading the status
+ * registers, which will not carry valid values otherwise.
+ */
+ ret = regmap_update_bits(priv->regmap, CAP11XX_REG_MAIN_CONTROL, 1, 0);
+ if (ret < 0)
+ goto out;
+
+ ret = regmap_read(priv->regmap, CAP11XX_REG_SENSOR_INPUT, &status);
+ if (ret < 0)
+ goto out;
+
+ for (i = 0; i < CAP11XX_NUM_CHN; i++)
+ input_report_key(priv->idev, priv->keycodes[i],
+ status & (1 << i));
+
+ input_sync(priv->idev);
+
+out:
+ return IRQ_HANDLED;
+}
+
+static int cap11xx_set_sleep(struct cap11xx_priv *priv, bool sleep)
+{
+ return regmap_update_bits(priv->regmap, CAP11XX_REG_MAIN_CONTROL,
+ CAP11XX_REG_MAIN_CONTROL_DLSEEP,
+ sleep ? CAP11XX_REG_MAIN_CONTROL_DLSEEP : 0);
+}
+
+static int cap11xx_input_open(struct input_dev *idev)
+{
+ struct cap11xx_priv *priv = input_get_drvdata(idev);
+
+ return cap11xx_set_sleep(priv, false);
+}
+
+static void cap11xx_input_close(struct input_dev *idev)
+{
+ struct cap11xx_priv *priv = input_get_drvdata(idev);
+
+ cap11xx_set_sleep(priv, true);
+}
+
+static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &i2c_client->dev;
+ struct cap11xx_priv *priv;
+ struct device_node *node;
+ int i, error, irq, gain = 0;
+ unsigned int val, rev;
+ u32 gain32, keycodes[CAP11XX_NUM_CHN];
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->regmap = devm_regmap_init_i2c(i2c_client, &cap11xx_regmap_config);
+ if (IS_ERR(priv->regmap))
+ return PTR_ERR(priv->regmap);
+
+ error = regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);
+ if (error)
+ return error;
+
+ if (val != CAP11XX_PRODUCT_ID) {
+ dev_err(dev, "Product ID: Got 0x%02x, expected 0x%02x\n",
+ val, CAP11XX_PRODUCT_ID);
+ return -ENODEV;
+ }
+
+ error = regmap_read(priv->regmap, CAP11XX_REG_MANUFACTURER_ID, &val);
+ if (error)
+ return error;
+
+ if (val != CAP11XX_MANUFACTURER_ID) {
+ dev_err(dev, "Manufacturer ID: Got 0x%02x, expected 0x%02x\n",
+ val, CAP11XX_MANUFACTURER_ID);
+ return -ENODEV;
+ }
+
+ error = regmap_read(priv->regmap, CAP11XX_REG_REVISION, &rev);
+ if (error < 0)
+ return error;
+
+ dev_info(dev, "CAP11XX detected, revision 0x%02x\n", rev);
+ i2c_set_clientdata(i2c_client, priv);
+ node = dev->of_node;
+
+ if (!of_property_read_u32(node, "microchip,sensor-gain", &gain32)) {
+ if (is_power_of_2(gain32) && gain32 <= 8)
+ gain = ilog2(gain32);
+ else
+ dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
+ }
+
+ BUILD_BUG_ON(ARRAY_SIZE(keycodes) != ARRAY_SIZE(priv->keycodes));
+
+ /* Provide some useful defaults */
+ for (i = 0; i < ARRAY_SIZE(keycodes); i++)
+ keycodes[i] = KEY_A + i;
+
+ of_property_read_u32_array(node, "linux,keycodes",
+ keycodes, ARRAY_SIZE(keycodes));
+
+ for (i = 0; i < ARRAY_SIZE(keycodes); i++)
+ priv->keycodes[i] = keycodes[i];
+
+ error = regmap_update_bits(priv->regmap, CAP11XX_REG_MAIN_CONTROL,
+ CAP11XX_REG_MAIN_CONTROL_GAIN_MASK,
+ gain << CAP11XX_REG_MAIN_CONTROL_GAIN_SHIFT);
+ if (error)
+ return error;
+
+ /* Disable autorepeat. The Linux input system has its own handling. */
+ error = regmap_write(priv->regmap, CAP11XX_REG_REPEAT_RATE, 0);
+ if (error)
+ return error;
+
+ priv->idev = devm_input_allocate_device(dev);
+ if (!priv->idev)
+ return -ENOMEM;
+
+ priv->idev->name = "CAP11XX capacitive touch sensor";
+ priv->idev->id.bustype = BUS_I2C;
+ priv->idev->evbit[0] = BIT_MASK(EV_KEY);
+
+ if (of_property_read_bool(node, "autorepeat"))
+ __set_bit(EV_REP, priv->idev->evbit);
+
+ for (i = 0; i < CAP11XX_NUM_CHN; i++)
+ __set_bit(priv->keycodes[i], priv->idev->keybit);
+
+ __clear_bit(KEY_RESERVED, priv->idev->keybit);
+
+ priv->idev->keycode = priv->keycodes;
+ priv->idev->keycodesize = sizeof(priv->keycodes[0]);
+ priv->idev->keycodemax = ARRAY_SIZE(priv->keycodes);
+
+ priv->idev->id.vendor = CAP11XX_MANUFACTURER_ID;
+ priv->idev->id.product = CAP11XX_PRODUCT_ID;
+ priv->idev->id.version = rev;
+
+ priv->idev->open = cap11xx_input_open;
+ priv->idev->close = cap11xx_input_close;
+
+ input_set_drvdata(priv->idev, priv);
+
+ /*
+ * Put the device in deep sleep mode for now.
+ * ->open() will bring it back once the it is actually needed.
+ */
+ cap11xx_set_sleep(priv, true);
+
+ error = input_register_device(priv->idev);
+ if (error)
+ return error;
+
+ irq = irq_of_parse_and_map(node, 0);
+ if (!irq) {
+ dev_err(dev, "Unable to parse or map IRQ\n");
+ return -ENXIO;
+ }
+
+ error = devm_request_threaded_irq(dev, irq, NULL, cap11xx_thread_func,
+ IRQF_ONESHOT, dev_name(dev), priv);
+ if (error)
+ return error;
+
+ return 0;
+}
+
+static const struct of_device_id cap11xx_dt_ids[] = {
+ { .compatible = "microchip,cap1106", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, cap11xx_dt_ids);
+
+static const struct i2c_device_id cap11xx_i2c_ids[] = {
+ { "cap1106", 0 },
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, cap11xx_i2c_ids);
+
+static struct i2c_driver cap11xx_i2c_driver = {
+ .driver = {
+ .name = "cap11xx",
+ .owner = THIS_MODULE,
+ .of_match_table = cap11xx_dt_ids,
+ },
+ .id_table = cap11xx_i2c_ids,
+ .probe = cap11xx_i2c_probe,
+};
+
+module_i2c_driver(cap11xx_i2c_driver);
+
+MODULE_ALIAS("platform:cap11xx");
+MODULE_DESCRIPTION("Microchip CAP11XX driver");
+MODULE_AUTHOR("Daniel Mack <linux@zonque.org>");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related
* [PATCH v3 3/3] cap11xx: support for irq-active-high option
From: Matt Ranostay @ 2014-09-25 5:24 UTC (permalink / raw)
To: galak, dmitry.torokhov, zonque, linux-input, linux-kernel,
robh+dt
Cc: devicetree, Matt Ranostay
In-Reply-To: <1411622662-16581-1-git-send-email-mranostay@gmail.com>
Some applications need to use the irq-active-high push-pull option.
This allows it be enabled in the device tree child node.
Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
Documentation/devicetree/bindings/input/cap11xx.txt | 4 ++++
drivers/input/keyboard/cap11xx.c | 8 ++++++++
2 files changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/cap11xx.txt b/Documentation/devicetree/bindings/input/cap11xx.txt
index 8031aa5..57407be 100644
--- a/Documentation/devicetree/bindings/input/cap11xx.txt
+++ b/Documentation/devicetree/bindings/input/cap11xx.txt
@@ -28,6 +28,10 @@ Optional properties:
Valid values are 1, 2, 4, and 8.
By default, a gain of 1 is set.
+ microchip,irq-active-high: By default the interrupt pin is active low
+ open drain. This property allows using the active
+ high push-pull output.
+
linux,keycodes: Specifies an array of numeric keycode values to
be used for the channels. If this property is
omitted, KEY_A, KEY_B, etc are used as
diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
index a8fe806..b4fc9f6 100644
--- a/drivers/input/keyboard/cap11xx.c
+++ b/drivers/input/keyboard/cap11xx.c
@@ -46,6 +46,7 @@
#define CAP11XX_REG_STANDBY_SENSITIVITY 0x42
#define CAP11XX_REG_STANDBY_THRESH 0x43
#define CAP11XX_REG_CONFIG2 0x44
+#define CAP11XX_REG_CONFIG2_ALT_POL BIT(6)
#define CAP11XX_REG_SENSOR_BASE_CNT(X) (0x50 + (X))
#define CAP11XX_REG_SENSOR_CALIB (0xb1 + (X))
#define CAP11XX_REG_SENSOR_CALIB_LSB1 0xb9
@@ -259,6 +260,13 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client,
dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
}
+ if (of_property_read_bool(node, "microchip,irq-active-high")) {
+ error = regmap_update_bits(priv->regmap, CAP11XX_REG_CONFIG2,
+ CAP11XX_REG_CONFIG2_ALT_POL, 0);
+ if (error)
+ return error;
+ }
+
/* Provide some useful defaults */
for (i = 0; i < priv->num_channels; i++)
priv->keycodes[i] = KEY_A + i;
--
1.9.1
^ permalink raw reply related
* RE: [PATCH v4 1/14] input: cyapa: re-architecture driver to support multi-trackpads in one driver
From: Dudley Du @ 2014-09-25 5:53 UTC (permalink / raw)
To: 'Dmitry Torokhov'
Cc: Rafael J. Wysocki, Benson Leung, Patrik Fimml, linux-input,
linux-kernel
In-Reply-To: <20140925004838.GB38891@core.coreip.homeip.net>
> -----Original Message-----
> From: linux-input-owner@vger.kernel.org [mailto:linux-input-
> owner@vger.kernel.org] On Behalf Of Dmitry Torokhov
> Sent: Thursday, September 25, 2014 8:49 AM
> To: Dudley Du
> Cc: Rafael J. Wysocki; Benson Leung; Patrik Fimml; linux-input@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 1/14] input: cyapa: re-architecture driver to support
> multi-trackpads in one driver
>
> On Wed, Sep 24, 2014 at 03:08:31PM +0800, Dudley Du wrote:
> > > > > > + async_schedule(cyapa_detect_async, cyapa);
> > > > >
> > > > > I think I already asked this before - why do we need to try and
> schedule
> > > async
> > > > > detect from interrupt handler. In what cases we will fail to
> initialize
> > > the
> > > > > device during normal probing, but then are fine when stray interrupt
> > > arrives?
> > > > >
> > > >
> > > > 1) Because gen5 TP devices use interrupt to sync the command and
> response,
> > > and
> > > > in detecting, some commands must be sent and executed in attached
> devices,
> > > so
> > > > the interrupt must be usable in detecting.
> > > > So if do not schedule async detect from interrupt handler, the interrupt
> > > > handler will be taken, and cannot enter again, which will cause the
> command
> > > to
> > > > the device failed, it will also cause long time detecting.
> > > > 2) detecting will fail when no device attached or the attached device is
> not
> > > > supported gen3 or gen5 TP devices or there is some issue with the device
> > > > that cannot enter into active working mode or stay in bootloader mode
> > > > for invalid firmware detected.
> > > > And it's tested that it's save when stray interrupt arrives in detecting.
> > >
> > >
> > > I am sorry, I have trouble parsing this. I understand that you may need
> > > interrupt to know when command response is ready, but I do not see how
> > > kicking asynchronous detect helps there. During probe you can figure out
> > > if you are talking to a Cypress device and whether it is operable. If it
> > > is not operable you can try flashing a new firmware and then kick off
> > > detection again after flash is successful. But I do not see any reason
> > > in trying to re-detect the device after random interrupt arrives in hopes
> > > that maybe this time stars will align and we'll be able to work with it.
> >
> > Considering two situations:
> > 1) For some machines, when system get into sleep mode, the power of the
> trackpad device
> > will be cut off, and after system resumed, the power to trackpad device is
> on again.
>
> And cyapa_resume() is responsible for bringing the device back into
> operational state.
>
> > 2) For the trackpad device itself, if internal error encounter, it will
> reset itself,
> > similar result as power off and power on again.
> > When either of above situation happened, for gen3 Cypress trackpad device,
> it will get
> > into bootloader mode (it cannot get into operational mode automatically),
> and also assert
> > interrupts to host.
> > So at this time, cyapa driver must kick off the detection again to determine
> the real status
> > of the trackpad device, and command it go to the operational mode.
> > Otherwise, it will be out of work until reboot.
>
> In this case you won't have cyapa_default_irq_handler installed, right?
> The specific generation IRQ handler will have to detect this condition
> and reinitialize the touchpad.
Yes, you are correct.
The async detection will be removed from cyapa_default_irq_handler().
>
> ...
>
> > > > > > @@ -898,8 +528,12 @@ static int cyapa_remove(struct i2c_client
> *client)
> > > > > > struct cyapa *cyapa = i2c_get_clientdata(client);
> > > > > >
> > > > >
> > > > > You schedule detect asynchronously so you need to make sure it is not
> > > running
> > > > > (and will not be running) when you try to unbind the driver here.
> > > >
> > > > Thanks. I will update it in next release.
> > > >
> > > > To avoid this problem, a removed flag is introduced and is also synced
> by
> > > the
> > > > cyapa->state_sync_lock mutex lock. So when this driver is removing, it's
> > > sure
> > > > that the detecting asynchronous thread is not running, and if there's
> any
> > > event
> > > > cause the asynchronous thread is entered, it will do nothing and will
> exit
> > > immediately.
> > >
> > > Peeking at the other version of the patch you do:
> > >
> > > cyapa_remove()
> > > {
> > > ...
> > > cyapa->removed = true;
> > > ...
> > > kfree(cyapa);
> > > }
> > >
> > > and
> > >
> > > void cyapa_detect_async(void *data, async_cookie_t cookie)
> > > {
> > > ...
> > > if (cyapa->removed) {
> > > mutex_unlock(&cyapa->state_sync_lock);
> > > return;
> > > }
> > > ...
> > > }
> > >
> > > That's not going to work.
> > >
> > > Getting asynchronous behavior by individual driver is hard and I believe
> > > Luis Rordiguez is working on allowing to do that is the driver core. So
> > > I would recommend to switch driver to do synchronous probing, at least
> > > for now, and figure out other kinks first.
> > >
> > > Thanks.
> > >
> >
> > The consideration of the thread asynchronous issue is that:
> > The detection thread has been scheduled, but not run yet, and at the time,
> > the driver itself has been removed, so when later the detection thread is
> > triggered to run, the memory of cyapa driver has been freed, and cause error,
> right?
>
> Right.
>
> >
> > I have an idea as below, please help review.
> > Add variable cyapa->detect_thread_actived to trace the number of the threads
> were scheduled,
> > and when the driver module is in removing, it will wait all scheduled
> threads has been run.
> > That is when a thread is scheduled, cyapa->detect_thread_actived will be
> increased,
> > when the thread is executed and finished, the cyapa->detect_thread_actived
> will be decrreased,
> > so, when the value of cyapa->detect_thread_actived is 0, it must be no
> thread scheduled for running.
> > So when at this time, if the driver module is marked for removed, the
> remove() thread that waiting
> > on the cyapa->detect_thread_completed will be issue, and then driver is safe
> to be removed.
> > The code is similar as below.
> >
> > For each time, before the cyapa_detect_async() function is called or
> scheduled,
> > ... {
> > ...
> > atomic_inc(&cyapa->detect_thread_actived);
> > async_schedule(cyapa_detect_async, cyapa);
> > ...
> > }
> >
> > void cyapa_detect_async(void *data, async_cookie_t cookie)
> > {
> > ...
> > if (cyapa->removed) {
> > mutex_unlock(&cyapa->state_sync_lock);
> > return;
> > }
> > ...
> > free_irq(cyapa->irq, cyapa);
> >
> > /* Wait until all scheduled thread exited. */
> > if (atomic_read(&cyapa->detect_thread_actived)
> > wait_for_completion_timeout(&cyapa->detect_thread_completed,
> > msecs_to_jiffies(4000));
> > ...
> > }
> >
> > void cyapa_detect_async(void *data, async_cookie_t cookie)
> > {
> > struct cyapa *cyapa = (struct cyapa *)data;
> >
> > mutex_lock(&cyapa->state_sync_lock);
> > if (cyapa->removed) {
> > mutex_unlock(&cyapa->state_sync_lock);
> > if (atomic_dec_and_test(&cyapa->detect_thread_actived)) {
> > /* Now, no thread runing, safe to remove this driver. */
> > complete(&cyapa->detect_thread_completed);
> > }
> > goto return;
> > }
> >
> > /* Keep synchronized with sys interface process threads. */
> > cyapa_detect(cyapa);
> >
> > mutex_unlock(&cyapa->state_sync_lock);
> > atomic_dec(&cyapa->detect_thread_actived);
> > }
>
> It can be made to work, but as I mentioned we are working on bringing
> asynchronous probing into device core in one form or another so I'd
> rather not be doing it in the driver at all at this time.
Understood.
But what should I do for this driver, right now?
Though synchronous probing could be working anyway, but I do not suggest to use this method in this driver.
For this driver and devices, I have below concerns when using asynchronous probing:
1) Slow down the system boot time.
At system start up, it will take much long time for probing, so system start up will
be slowed down.
2) Polling mode for these device would be lower efficient and unreliable in detecting.
If using asynchronous detecting when error detected in irq handler, then must use polling mode.
But use polling mode will cause device easier to get into unstable status,
Fast polling will consume too much firmware CPU time, and may make it unstable.
Low polling requires much more time for device detecting.
And also the data/command read/write in polling mode is not well synchronized between HOST and device,
it may cause the device data become unreliable.
Thanks.
>
> Thanks.
>
> --
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox