* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
From: Mylene Josserand @ 2018-01-23 9:10 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Mark Rutland, Russell King - ARM Linux,
Maxime Ripard, Chen-Yu Tsai, linux-arm-kernel,
linux-input@vger.kernel.org, devicetree, lkml, Thomas Petazzoni,
Quentin Schulz
In-Reply-To: <CAKdAkRQme1Y+t4qxLGOFojEji1L6z3L0XS4+W0FdQ36-2hxW=A@mail.gmail.com>
Hello Dimitry,
Thank you for the review!
Le Mon, 22 Jan 2018 09:42:08 -0800,
Dmitry Torokhov <dmitry.torokhov@gmail.com> a écrit :
> Hi Mylène,
>
> On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
> <mylene.josserand@free-electrons.com> wrote:
> > Add the support of regulator to use it as VCC source.
> >
> > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > ---
> > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> > drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> > 2 files changed, 34 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > index 025cf8c9324a..48e975b9c1aa 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > @@ -30,6 +30,7 @@ Required properties:
> > Optional properties:
> > - reset-gpios: GPIO specification for the RESET input
> > - wake-gpios: GPIO specification for the WAKE input
> > + - vcc-supply: Regulator that supplies the touchscreen
> >
> > - pinctrl-names: should be "default"
> > - pinctrl-0: a phandle pointing to the pin settings for the
> > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > index c53a3d7239e7..5ee14a25a382 100644
> > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > @@ -39,6 +39,7 @@
> > #include <linux/input/mt.h>
> > #include <linux/input/touchscreen.h>
> > #include <linux/of_device.h>
> > +#include <linux/regulator/consumer.h>
> >
> > #define WORK_REGISTER_THRESHOLD 0x00
> > #define WORK_REGISTER_REPORT_RATE 0x08
> > @@ -91,6 +92,7 @@ struct edt_ft5x06_ts_data {
> > struct touchscreen_properties prop;
> > u16 num_x;
> > u16 num_y;
> > + struct regulator *vcc;
> >
> > struct gpio_desc *reset_gpio;
> > struct gpio_desc *wake_gpio;
> > @@ -993,6 +995,23 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
> >
> > tsdata->max_support_points = chip_data->max_support_points;
> >
> > + tsdata->vcc = devm_regulator_get(&client->dev, "vcc");
> > + if (IS_ERR(tsdata->vcc)) {
> > + error = PTR_ERR(tsdata->vcc);
> > + dev_err(&client->dev, "failed to request regulator: %d\n",
> > + error);
> > + return error;
> > + };
>
> As 0-day pounted out, this semicolon is not needed.
Yes, thanks, I will fix that in next version.
>
> > +
> > + if (tsdata->vcc) {
>
> You do not need to check for non-NULL here, devm_regulator_get() wil
> lnever give you a NULL. If regulator is not defined in DT/board
> mappings, then dummy regulator will be provided. You can call
> regulator_enable() and regulator_disable() and other regulator APIs
> with dummy regulator.
Okay, thanks for the explanation, I will remove that.
>
> > + error = regulator_enable(tsdata->vcc);
> > + if (error < 0) {
> > + dev_err(&client->dev, "failed to enable vcc: %d\n",
> > + error);
> > + return error;
> > + }
> > + }
> > +
> > tsdata->reset_gpio = devm_gpiod_get_optional(&client->dev,
> > "reset", GPIOD_OUT_HIGH);
> > if (IS_ERR(tsdata->reset_gpio)) {
> > @@ -1122,20 +1141,34 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
> > static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev)
> > {
> > struct i2c_client *client = to_i2c_client(dev);
> > + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
> >
> > if (device_may_wakeup(dev))
> > enable_irq_wake(client->irq);
> >
> > + if (tsdata->vcc)
>
> Same here.
yep
>
> > + regulator_disable(tsdata->vcc);
> > +
> > return 0;
> > }
> >
> > static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev)
> > {
> > struct i2c_client *client = to_i2c_client(dev);
> > + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
> > + int ret;
> >
> > if (device_may_wakeup(dev))
> > disable_irq_wake(client->irq);
> >
> > + if (tsdata->vcc) {
>
> And here.
yep
>
> > + ret = regulator_enable(tsdata->vcc);
> > + if (ret < 0) {
> > + dev_err(dev, "failed to enable vcc: %d\n", ret);
> > + return ret;
> > + }
>
> Since power to the device may have been cut, I think you need to
> restore the register settings to whatever it was (factory vs work
> mode, threshold, gain and offset registers, etc, etc).
Okay. Could you tell me how can I do that?
>
> > + }
> > +
> > return 0;
> > }
> >
> > --
> > 2.11.0
> >
>
> Thanks.
>
About your V2's review, you suggested to add support for wake/reset in
suspend/resume (that I forgot in this version). I wanted to add it but
with my board, I can't test suspend/resume. What should I do about
that?
If I send a V3 in next few days, do you think you will have time to
merge it for v4.16?
Thank you in advance,
Best regards,
Mylène
--
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
From: Mylene Josserand @ 2018-01-23 9:13 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Dmitry Torokhov, Mark Rutland, devicetree,
Russell King - ARM Linux, lkml, Quentin Schulz, Chen-Yu Tsai,
Rob Herring, linux-input@vger.kernel.org, Maxime Ripard,
Thomas Petazzoni, linux-arm-kernel
In-Reply-To: <20180123090414.47bcf118@karo-electronics.de>
Hello Lothar,
Le Tue, 23 Jan 2018 09:04:14 +0100,
Lothar Waßmann <LW@KARO-electronics.de> a écrit :
> Hi,
>
> On Mon, 22 Jan 2018 09:42:08 -0800 Dmitry Torokhov wrote:
> > Hi Mylène,
> >
> > On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
> > <mylene.josserand@free-electrons.com> wrote:
> > > Add the support of regulator to use it as VCC source.
> > >
> > > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > > ---
> > > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> > > drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> > > 2 files changed, 34 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > > index 025cf8c9324a..48e975b9c1aa 100644
> > > --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > > @@ -30,6 +30,7 @@ Required properties:
> > > Optional properties:
> > > - reset-gpios: GPIO specification for the RESET input
> > > - wake-gpios: GPIO specification for the WAKE input
> > > + - vcc-supply: Regulator that supplies the touchscreen
> > >
> > > - pinctrl-names: should be "default"
> > > - pinctrl-0: a phandle pointing to the pin settings for the
> > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > > index c53a3d7239e7..5ee14a25a382 100644
> > > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > > @@ -39,6 +39,7 @@
> > > #include <linux/input/mt.h>
> > > #include <linux/input/touchscreen.h>
> > > #include <linux/of_device.h>
> > > +#include <linux/regulator/consumer.h>
> > >
> > > #define WORK_REGISTER_THRESHOLD 0x00
> > > #define WORK_REGISTER_REPORT_RATE 0x08
> > > @@ -91,6 +92,7 @@ struct edt_ft5x06_ts_data {
> > > struct touchscreen_properties prop;
> > > u16 num_x;
> > > u16 num_y;
> > > + struct regulator *vcc;
> > >
> > > struct gpio_desc *reset_gpio;
> > > struct gpio_desc *wake_gpio;
> > > @@ -993,6 +995,23 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
> > >
> > > tsdata->max_support_points = chip_data->max_support_points;
> > >
> > > + tsdata->vcc = devm_regulator_get(&client->dev, "vcc");
> > > + if (IS_ERR(tsdata->vcc)) {
> > > + error = PTR_ERR(tsdata->vcc);
> > > + dev_err(&client->dev, "failed to request regulator: %d\n",
> > > + error);
> >
> I would check for -EPROBE_DEFER here and omit the error message in this
> case.
>
>
> Lothar Waßmann
Sure, I will add this case, thank you for the review.
Best regards,
--
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen
From: Mylene Josserand @ 2018-01-23 9:33 UTC (permalink / raw)
To: Marcus Folkesson
Cc: dmitry.torokhov, robh+dt, mark.rutland, linux-kernel, linux-input,
devicetree, thomas.petazzoni, maxime.ripard
In-Reply-To: <20180122201109.GA651@gmail.com>
Hello Marcus,
Thank you for the review.
Le Mon, 22 Jan 2018 21:11:09 +0100,
Marcus Folkesson <marcus.folkesson@gmail.com> a écrit :
> Mylene,
>
> On Fri, Dec 01, 2017 at 04:39:56PM +0100, Mylène Josserand wrote:
> > +++ b/drivers/input/touchscreen/cyttsp5.c
> > @@ -0,0 +1,1110 @@
> > +/*
> > + * Parade TrueTouch(TM) Standard Product V5 Module.
> > + * For use with Parade touchscreen controllers.
> > + *
> > + * Copyright (C) 2015 Parade Technologies
> > + * Copyright (C) 2012-2015 Cypress Semiconductor
> > + * Copyright (C) 2017 Free Electrons
> > + *
> > + * Author: Mylène Josserand <mylene.josserand@free-electrons.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * version 2, and only version 2, as published by the
> > + * Free Software Foundation.
>
> Please use SPDX license Identifier to describe the license.
>
> E.g.
> SPDX-License-Identifier: GPL-2.0
>
> Also, the MODULE_LICENSE means GPL 2.0 or later per module.h and this does
> not match the license description.
>
> Could you make sure they are in sync?
You are right, I will check that, thanks.
>
>
> > + *
> > + * 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 <asm/unaligned.h>
> > +#include <linux/crc-itu-t.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/gpio.h>
> > +#include <linux/input/mt.h>
> > +#include <linux/input/touchscreen.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/regmap.h>
>
> #include <linux/bitops.h>
>
> since you are using BIT()
okay
>
>
> [snip]
>
>
> > +static int cyttsp5_setup_input_device(struct device *dev)
> > +{
> > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > + int max_x, max_y, max_p;
> > + int max_x_tmp, max_y_tmp;
> > + int rc;
> > +
> > + __set_bit(EV_ABS, ts->input->evbit);
> > + __set_bit(EV_REL, ts->input->evbit);
> > + __set_bit(EV_KEY, ts->input->evbit);
> > +
> > + max_x_tmp = si->sensing_conf_data.res_x;
> > + max_y_tmp = si->sensing_conf_data.res_y;
> > + max_x = max_y_tmp - 1;
> > + max_y = max_x_tmp - 1;
> > + max_p = si->sensing_conf_data.max_z;
> > +
> > + input_mt_init_slots(ts->input, si->tch_abs[CY_TCH_T].max, 0);
> > +
> > + __set_bit(ABS_MT_POSITION_X, ts->input->absbit);
> > + __set_bit(ABS_MT_POSITION_Y, ts->input->absbit);
> > + __set_bit(ABS_MT_PRESSURE, ts->input->absbit);
>
> Not needed, input_set_abs_params() will set the bits for you below.
Yes, I will remove it.
>
>
> > +
> > + input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, max_x, 0, 0);
> > + input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, max_y, 0, 0);
> > + input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, max_p, 0, 0);
> > +
> > + input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
> > + input_set_abs_params(ts->input, ABS_MT_TOUCH_MINOR, 0, MAX_AREA, 0, 0);
> > +
> > + rc = input_register_device(ts->input);
> > + if (rc < 0)
> > + dev_err(dev, "Error, failed register input device r=%d\n", rc);
> > +
> > + return rc;
> > +}
> > +
> > +
>
> [snip]
>
> > +static int cyttsp5_get_hid_descriptor(struct cyttsp5 *ts,
> > + struct cyttsp5_hid_desc *desc)
> > +{
> > + struct device *dev = ts->dev;
> > + __le16 hid_desc_register = HID_DESC_REG;
> > + int rc;
> > + u8 cmd[2];
> > +
> > + /* Read HID descriptor length and version */
> > + mutex_lock(&ts->system_lock);
> > + ts->hid_cmd_state = HID_CMD_BUSY;
> > + mutex_unlock(&ts->system_lock);
> > +
> > + /* Set HID descriptor register */
> > + memcpy(cmd, &hid_desc_register, sizeof(hid_desc_register));
> > +
> > + rc = cyttsp5_write(ts, HID_DESC_REG, NULL, 0);
> > + if (rc < 0) {
> > + dev_err(dev, "Failed to get HID descriptor, rc=%d\n", rc);
> > + goto error;
> > + }
> > +
> > + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> > + msecs_to_jiffies(CY_HID_GET_HID_DESCRIPTOR_TIMEOUT));
> > + if (!rc) {
> > + dev_err(ts->dev, "HID get descriptor timed out\n");
> > + rc = -ETIME;
> > + goto error;
> > + }
> > +
> > + memcpy(desc, ts->response_buf, sizeof(struct cyttsp5_hid_desc));
> > +
> > + /* Check HID descriptor length and version */
> > + if (le16_to_cpu(desc->hid_desc_len) != sizeof(*desc) ||
> > + le16_to_cpu(desc->bcd_version) != HID_VERSION) {
> > + dev_err(dev, "Unsupported HID version\n");
> > + return -ENODEV;
>
> Maybe it is supposed to return here, but all other faults use "goto
> error".
Yep, I will use 'goto' instead of 'return' here.
>
> > + }
> > +
> > + goto exit;
> > +
> > +error:
> > + mutex_lock(&ts->system_lock);
> > + ts->hid_cmd_state = HID_CMD_DONE;
> > + mutex_unlock(&ts->system_lock);
> > +exit:
> > + return rc;
> > +}
> > +
>
> [snip]
>
> > +static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> > + const char *name)
> > +{
> > + struct cyttsp5 *ts;
> > + struct cyttsp5_sysinfo *si;
> > + int rc = 0, i;
> > +
> > + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> > + if (!ts)
> > + return -ENOMEM;
> > +
> > + /* Initialize device info */
> > + ts->regmap = regmap;
> > + ts->dev = dev;
> > + si = &ts->sysinfo;
> > + dev_set_drvdata(dev, ts);
> > +
> > + /* Initialize mutexes and spinlocks */
>
> No spinlocks here :-)
I forgot to remove the comment from vendor's driver, thanks :)
>
> > + mutex_init(&ts->system_lock);
> > +
> > + /* Initialize wait queue */
> > + init_waitqueue_head(&ts->wait_q);
> > +
> > + /* Reset the gpio to be in a reset state */
> > + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> > + if (IS_ERR(ts->reset_gpio)) {
> > + rc = PTR_ERR(ts->reset_gpio);
> > + dev_err(dev, "Failed to request reset gpio, error %d\n", rc);
> > + return rc;
> > + }
> > + gpiod_set_value(ts->reset_gpio, 1);
> > +
>
> [snip]
>
> > +static struct i2c_driver cyttsp5_i2c_driver = {
> > + .driver = {
> > + .name = CYTTSP5_NAME,
> > + .owner = THIS_MODULE,
> > + .of_match_table = of_match_ptr(cyttsp5_of_match),
> > + },
> > + .probe = cyttsp5_i2c_probe,
> > + .remove = cyttsp5_i2c_remove,
> > + .id_table = cyttsp5_i2c_id,
> > +};
> > +
> > +module_i2c_driver(cyttsp5_i2c_driver);
> > +
> > +MODULE_LICENSE("GPL");
>
> From linux/module.h:
>
> /*
> * The following license idents are currently accepted as indicating free
> * software modules
> *
> * "GPL" [GNU Public License v2 or later]
> * "GPL v2" [GNU Public License v2]
>
Okay, noted.
>
>
> > +MODULE_DESCRIPTION("Touchscreen driver for Cypress TrueTouch Gen 5 Product");
> > +MODULE_AUTHOR("Mylène Josserand <mylene.josserand@free-electrons.com>");
> > --
> > 2.11.0
> >
>
> Best regards
> Marcus Folkesson
Best regards,
--
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen
From: Mylene Josserand @ 2018-01-23 10:44 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: robh+dt, mark.rutland, linux-kernel, linux-input, devicetree,
thomas.petazzoni, maxime.ripard
In-Reply-To: <20180123041503.cezgsi7h2h4twu7n@dtor-ws>
Hi Dmitry,
Thank you for the review.
Le Mon, 22 Jan 2018 20:15:03 -0800,
Dmitry Torokhov <dmitry.torokhov@gmail.com> a écrit :
> Hi Mylène,
>
> On Fri, Dec 01, 2017 at 04:39:56PM +0100, Mylène Josserand wrote:
> > This is the basic driver for the Cypress TrueTouch Gen5 touchscreen
> > controllers. This driver supports only the I2C bus but it uses regmap
> > so SPI support could be added later.
> > The touchscreen can retrieve some defined zone that are handled as
> > buttons (according to the hardware). That is why it handles
> > button and multitouch events.
> >
> > Reviewed-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > ---
> > drivers/input/touchscreen/Kconfig | 16 +
> > drivers/input/touchscreen/Makefile | 1 +
> > drivers/input/touchscreen/cyttsp5.c | 1110 +++++++++++++++++++++++++++++++++++
> > 3 files changed, 1127 insertions(+)
> > create mode 100644 drivers/input/touchscreen/cyttsp5.c
> >
> > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> > index 0cfdb7cb610e..28eea6d5f1bb 100644
> > --- a/drivers/input/touchscreen/Kconfig
> > +++ b/drivers/input/touchscreen/Kconfig
> > @@ -238,6 +238,22 @@ config TOUCHSCREEN_CYTTSP4_SPI
> > To compile this driver as a module, choose M here: the
> > module will be called cyttsp4_spi.
> >
> > +config TOUCHSCREEN_CYTTSP5
> > + tristate "Cypress TrueTouch Gen5 Touchscreen Driver"
> > + depends on OF
>
> Does it have to be? Please use generic device properties
> (device_property_* API) and it can be used with ACPI and static board
> files, of needed.
Okay, I will try with that.
>
> > + select REGMAP_I2C
> > + select CRC_ITU_T
> > + help
> > + Driver for Parade TrueTouch Standard Product
> > + Generation 5 touchscreen controllers.
> > + I2C bus interface support only.
> > +
> > + Say Y here if you have a Cypress Gen5 touchscreen.
> > + If unsure, say N.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called cyttsp5.
> > +
> > config TOUCHSCREEN_DA9034
> > tristate "Touchscreen support for Dialog Semiconductor DA9034"
> > depends on PMIC_DA903X
> > diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> > index d2a2b3b7af27..e7d124901dd9 100644
> > --- a/drivers/input/touchscreen/Makefile
> > +++ b/drivers/input/touchscreen/Makefile
> > @@ -26,6 +26,7 @@ obj-$(CONFIG_TOUCHSCREEN_CYTTSP_SPI) += cyttsp_spi.o
> > obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_CORE) += cyttsp4_core.o
> > obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_I2C) += cyttsp4_i2c.o cyttsp_i2c_common.o
> > obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_SPI) += cyttsp4_spi.o
> > +obj-$(CONFIG_TOUCHSCREEN_CYTTSP5) += cyttsp5.o
> > obj-$(CONFIG_TOUCHSCREEN_DA9034) += da9034-ts.o
> > obj-$(CONFIG_TOUCHSCREEN_DA9052) += da9052_tsi.o
> > obj-$(CONFIG_TOUCHSCREEN_DYNAPRO) += dynapro.o
> > diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
> > new file mode 100644
> > index 000000000000..a41feea2cd5a
> > --- /dev/null
> > +++ b/drivers/input/touchscreen/cyttsp5.c
> > @@ -0,0 +1,1110 @@
> > +/*
> > + * Parade TrueTouch(TM) Standard Product V5 Module.
> > + * For use with Parade touchscreen controllers.
> > + *
> > + * Copyright (C) 2015 Parade Technologies
> > + * Copyright (C) 2012-2015 Cypress Semiconductor
> > + * Copyright (C) 2017 Free Electrons
> > + *
> > + * Author: Mylène Josserand <mylene.josserand@free-electrons.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * version 2, and only version 2, as published by the
> > + * Free Software Foundation.
> > + *
> > + * 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 <asm/unaligned.h>
> > +#include <linux/crc-itu-t.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/gpio.h>
> > +#include <linux/input/mt.h>
> > +#include <linux/input/touchscreen.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/regmap.h>
> > +
> > +#define CYTTSP5_NAME "cyttsp5"
> > +#define CY_I2C_DATA_SIZE (2 * 256)
> > +#define HID_VERSION 0x0100
> > +#define CY_MAX_INPUT 512
> > +#define CYTTSP5_PREALLOCATED_CMD_BUFFER 32
> > +#define CY_BITS_PER_BTN 1
> > +#define CY_NUM_BTN_EVENT_ID ((1 << CY_BITS_PER_BTN) - 1)
> > +
> > +#define MAX_AREA 255
> > +#define HID_OUTPUT_BL_SOP 0x1
> > +#define HID_OUTPUT_BL_EOP 0x17
> > +#define HID_OUTPUT_BL_LAUNCH_APP 0x3B
> > +#define HID_OUTPUT_BL_LAUNCH_APP_SIZE 11
> > +#define HID_OUTPUT_GET_SYSINFO 0x2
> > +#define HID_OUTPUT_GET_SYSINFO_SIZE 5
> > +
> > +#define HID_DESC_REG 0x1
> > +#define HID_INPUT_REG 0x3
> > +#define HID_OUTPUT_REG 0x4
> > +
> > +#define REPORT_ID_TOUCH 0x1
> > +#define REPORT_ID_BTN 0x3
> > +#define REPORT_SIZE_5 5
> > +#define REPORT_SIZE_8 8
> > +#define REPORT_SIZE_16 16
> > +
> > +/* Touch reports offsets */
> > +/* Header offsets */
> > +#define TOUCH_REPORT_DESC_HDR_CONTACTCOUNT 16
> > +/* Record offsets */
> > +#define TOUCH_REPORT_DESC_CONTACTID 8
> > +#define TOUCH_REPORT_DESC_X 16
> > +#define TOUCH_REPORT_DESC_Y 32
> > +#define TOUCH_REPORT_DESC_P 48
> > +#define TOUCH_REPORT_DESC_MAJ 56
> > +#define TOUCH_REPORT_DESC_MIN 64
> > +
> > +/* HID */
> > +#define HID_TOUCH_REPORT_ID 0x1
> > +#define HID_BTN_REPORT_ID 0x3
> > +#define HID_APP_RESPONSE_REPORT_ID 0x1F
> > +#define HID_APP_OUTPUT_REPORT_ID 0x2F
> > +#define HID_BL_RESPONSE_REPORT_ID 0x30
> > +#define HID_BL_OUTPUT_REPORT_ID 0x40
> > +
> > +#define HID_OUTPUT_RESPONSE_REPORT_OFFSET 2
> > +#define HID_OUTPUT_RESPONSE_CMD_OFFSET 4
> > +#define HID_OUTPUT_RESPONSE_CMD_MASK 0x7F
> > +
> > +#define HID_SYSINFO_SENSING_OFFSET 33
> > +#define HID_SYSINFO_BTN_OFFSET 48
> > +#define HID_SYSINFO_BTN_MASK 0xFF
> > +#define HID_SYSINFO_MAX_BTN 8
> > +
> > +/* Timeout in ms */
> > +#define CY_HID_OUTPUT_TIMEOUT 200
> > +#define CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT 3000
> > +#define CY_HID_GET_HID_DESCRIPTOR_TIMEOUT 4000
> > +
> > +/* maximum number of concurrent tracks */
> > +#define TOUCH_REPORT_SIZE 10
> > +#define TOUCH_INPUT_HEADER_SIZE 7
> > +#define BTN_REPORT_SIZE 9
> > +#define BTN_INPUT_HEADER_SIZE 5
> > +
> > +/* All usage pages for Touch Report */
> > +#define TOUCH_REPORT_USAGE_PG_X 0x00010030
> > +#define TOUCH_REPORT_USAGE_PG_Y 0x00010031
> > +#define TOUCH_REPORT_USAGE_PG_P 0x000D0030
> > +#define TOUCH_REPORT_USAGE_PG_CONTACTID 0x000D0051
> > +#define TOUCH_REPORT_USAGE_PG_CONTACTCOUNT 0x000D0054
> > +#define TOUCH_REPORT_USAGE_PG_MAJ 0xFF010062
> > +#define TOUCH_REPORT_USAGE_PG_MIN 0xFF010063
> > +#define TOUCH_COL_USAGE_PG 0x000D0022
> > +
> > +/* helpers */
> > +#define HI_BYTE(x) (u8)(((x) >> 8) & 0xFF)
> > +#define LOW_BYTE(x) (u8)((x) & 0xFF)
> > +
> > +/* System Information interface definitions */
> > +struct cyttsp5_sensing_conf_data_dev {
> > + u8 electrodes_x;
> > + u8 electrodes_y;
> > + __le16 len_x;
> > + __le16 len_y;
> > + __le16 res_x;
> > + __le16 res_y;
> > + __le16 max_z;
> > + u8 origin_x;
> > + u8 origin_y;
> > + u8 btn;
> > + u8 scan_mode;
> > + u8 max_num_of_tch_per_refresh_cycle;
> > +} __packed;
> > +
> > +struct cyttsp5_sensing_conf_data {
> > + u16 res_x;
> > + u16 res_y;
> > + u16 max_z;
> > + u16 len_x;
> > + u16 len_y;
> > + u8 origin_x;
> > + u8 origin_y;
> > + u8 max_tch;
> > +};
> > +
> > +enum { HID_CMD_DONE, HID_CMD_BUSY } hid_cmd_state;
> > +
> > +enum cyttsp5_tch_abs { /* for ordering within the extracted touch data array */
> > + CY_TCH_X, /* X */
> > + CY_TCH_Y, /* Y */
> > + CY_TCH_P, /* P (Z) */
> > + CY_TCH_T, /* TOUCH ID */
> > + CY_TCH_MAJ, /* TOUCH_MAJOR */
> > + CY_TCH_MIN, /* TOUCH_MINOR */
> > + CY_TCH_NUM_ABS,
> > +};
> > +
> > +struct cyttsp5_tch_abs_params {
> > + size_t ofs; /* abs byte offset */
> > + size_t size; /* size in bits */
> > + size_t min; /* min value */
> > + size_t max; /* max value */
> > + size_t bofs; /* bit offset */
> > +};
> > +
> > +struct cyttsp5_touch {
> > + int hdr;
> > + int abs[CY_TCH_NUM_ABS];
> > +};
> > +
> > +struct cyttsp5_sysinfo {
> > + struct cyttsp5_sensing_conf_data sensing_conf_data;
> > + int num_btns;
> > + struct cyttsp5_tch_abs_params tch_hdr;
> > + struct cyttsp5_tch_abs_params tch_abs[CY_TCH_NUM_ABS];
> > + u32 key_code[HID_SYSINFO_MAX_BTN];
> > + u8 *xy_mode;
> > + u8 *xy_data;
> > +};
> > +
> > +struct cyttsp5_hid_desc {
> > + __le16 hid_desc_len;
> > + u8 packet_id;
> > + u8 reserved_byte;
> > + __le16 bcd_version;
> > + __le16 report_desc_len;
> > + __le16 report_desc_register;
> > + __le16 input_register;
> > + __le16 max_input_len;
> > + __le16 output_register;
> > + __le16 max_output_len;
> > + __le16 command_register;
> > + __le16 data_register;
> > + __le16 vendor_id;
> > + __le16 product_id;
> > + __le16 version_id;
> > + u8 reserved[4];
> > +} __packed;
> > +
> > +struct cyttsp5 {
> > + struct device *dev;
> > + struct mutex system_lock;
> > + wait_queue_head_t wait_q;
> > + struct cyttsp5_sysinfo sysinfo;
> > + int hid_cmd_state;
> > + struct cyttsp5_hid_desc hid_desc;
> > + u8 cmd_buf[CYTTSP5_PREALLOCATED_CMD_BUFFER];
> > + u8 input_buf[CY_MAX_INPUT];
> > + u8 response_buf[CY_MAX_INPUT];
> > + struct gpio_desc *reset_gpio;
> > + struct input_dev *input;
> > + char phys[NAME_MAX];
> > + int num_prv_rec;
> > + struct regmap *regmap;
> > +};
> > +
> > +/*
> > + * For what understood in the datasheet, the register does not
> > + * matter. For consistency, used the Input Register address
> > + * but it does mean anything to the device. The important data
> > + * to send is the I2C address
> > + */
> > +static int cyttsp5_read(struct cyttsp5 *ts, u8 *buf, u32 max)
> > +{
> > + int rc;
> > + u32 size;
> > + u8 temp[2];
> > +
> > + if (!buf)
> > + return -EINVAL;
>
> Is it really possible? This is not a publicly facing API but private
> driver data, do you ever happen to call it with NULL?
hm, I do not think so but I will check that.
>
> > +
> > + /* Read the frame to retrieve the size */
> > + rc = regmap_bulk_read(ts->regmap, HID_INPUT_REG, temp, 2);
> > + if (rc < 0)
> > + return rc;
>
> Can we call this kind of variables "error" pelase?
Sure.
>
> > +
> > + size = get_unaligned_le16(temp);
> > + if (!size || size == 2)
> > + return 0;
> > +
> > + if (size > max)
> > + return -EINVAL;
> > +
> > + /* Get the real value */
> > + return regmap_bulk_read(ts->regmap, HID_INPUT_REG, buf, size);
> > +}
> > +
> > +static int cyttsp5_write(struct cyttsp5 *ts, unsigned int reg, u8 *data,
> > + size_t size)
> > +{
> > + u8 cmd[size + 1];
> > +
> > + /* High bytes of register address needed as first byte of cmd */
> > + cmd[0] = HI_BYTE(reg);
> > +
> > + /* Copy the rest of the data */
> > + if (data)
> > + memcpy(&cmd[1], data, size);
> > +
> > + /* The hardware wants to receive a frame with the address register
>
> The multiline comment block should start with "/*" on a separate line
> please.
I will change that.
>
> > + * contains in the first two bytes. As the regmap_write function
> > + * add the register adresse in the frame, we use the LOW_BYTE as
> > + * first frame byte for the address register and the first
> > + * data byte is the high register + left of the cmd to send
> > + */
> > + return regmap_bulk_write(ts->regmap, LOW_BYTE(reg), cmd, size + 1);
> > +}
> > +
> > +static void cyttsp5_final_sync(struct input_dev *input, int max_slots,
> > + unsigned long *ids)
> > +{
> > + int t;
> > +
> > + for (t = 0; t < max_slots; t++) {
>
> for_each_set_bit() {
> }
I did not know this macro, I will try that.
>
> > + if (test_bit(t, ids))
> > + continue;
> > + input_mt_slot(input, t);
> > + input_mt_report_slot_state(input, MT_TOOL_FINGER, false);
> > + }
> > +
> > + input_sync(input);
> > +}
> > +
> > +static void cyttsp5_report_slot_liftoff(struct cyttsp5 *ts, int max_slots)
> > +{
> > + int t;
> > +
> > + if (ts->num_prv_rec == 0)
> > + return;
>
> Do you need this check? The caller checks the same condition.
True, I will remove it.
>
> > +
> > + for (t = 0; t < max_slots; t++) {
> > + input_mt_slot(ts->input, t);
> > + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, false);
> > + }
> > +}
> > +
> > +static void cyttsp5_mt_lift_all(struct cyttsp5 *ts)
> > +{
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > + int max = si->tch_abs[CY_TCH_T].max;
> > +
> > + if (ts->num_prv_rec != 0) {
> > + cyttsp5_report_slot_liftoff(ts, max);
> > + input_sync(ts->input);
> > + ts->num_prv_rec = 0;
> > + }
> > +}
> > +
> > +static void cyttsp5_get_touch_axis(int *axis, int size, int max, u8 *xy_data,
> > + int bofs)
> > +{
> > + int nbyte;
> > + int next;
> > +
> > + for (nbyte = 0, *axis = 0, next = 0; nbyte < size; nbyte++)
> > + *axis = *axis + ((xy_data[nbyte] >> bofs) << (nbyte * 8));
> > +
> > + *axis &= max - 1;
> > +}
> > +
> > +static void cyttsp5_get_touch_record(struct cyttsp5 *ts,
> > + struct cyttsp5_touch *touch, u8 *xy_data)
> > +{
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > + enum cyttsp5_tch_abs abs;
> > +
> > + for (abs = CY_TCH_X; abs < CY_TCH_NUM_ABS; abs++) {
> > + cyttsp5_get_touch_axis(&touch->abs[abs],
> > + si->tch_abs[abs].size,
> > + si->tch_abs[abs].max,
> > + xy_data + si->tch_abs[abs].ofs,
> > + si->tch_abs[abs].bofs);
> > + }
> > +}
> > +
> > +static void cyttsp5_get_mt_touches(struct cyttsp5 *ts,
> > + struct cyttsp5_touch *tch, int num_cur_tch)
> > +{
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > + int i, t = 0;
> > + DECLARE_BITMAP(ids, si->tch_abs[CY_TCH_T].max);
> > + u8 *tch_addr;
> > + int tmp;
> > +
> > + bitmap_zero(ids, si->tch_abs[CY_TCH_T].max);
> > + memset(tch->abs, 0, sizeof(tch->abs));
> > +
> > + for (i = 0; i < num_cur_tch; i++) {
> > + tch_addr = si->xy_data + (i * TOUCH_REPORT_SIZE);
> > + cyttsp5_get_touch_record(ts, tch, tch_addr);
> > +
> > + /* Convert MAJOR/MINOR from mm to resolution */
> > + tmp = tch->abs[CY_TCH_MAJ] * 100 * si->sensing_conf_data.res_x;
> > + tch->abs[CY_TCH_MAJ] = tmp / si->sensing_conf_data.len_x;
> > + tmp = tch->abs[CY_TCH_MIN] * 100 * si->sensing_conf_data.res_x;
> > + tch->abs[CY_TCH_MIN] = tmp / si->sensing_conf_data.len_x;
> > +
> > + t = tch->abs[CY_TCH_T];
> > + input_mt_slot(ts->input, t);
> > + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, true);
> > + __set_bit(t, ids);
> > +
> > + /* position and pressure fields */
> > + input_report_abs(ts->input, ABS_MT_POSITION_X,
> > + tch->abs[CY_TCH_X]);
> > + input_report_abs(ts->input, ABS_MT_POSITION_Y,
> > + tch->abs[CY_TCH_Y]);
> > + input_report_abs(ts->input, ABS_MT_PRESSURE,
> > + tch->abs[CY_TCH_P]);
> > +
> > + /* Get the extended touch fields */
> > + input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR,
> > + tch->abs[CY_TCH_MAJ]);
> > + input_report_abs(ts->input, ABS_MT_TOUCH_MINOR,
> > + tch->abs[CY_TCH_MIN]);
> > + }
> > +
> > + cyttsp5_final_sync(ts->input, si->tch_abs[CY_TCH_T].max, ids);
> > +
> > + ts->num_prv_rec = num_cur_tch;
> > +}
> > +
> > +/* read xy_data for all current touches */
> > +static int cyttsp5_xy_worker(struct cyttsp5 *ts)
> > +{
> > + struct device *dev = ts->dev;
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > + int max_tch = si->sensing_conf_data.max_tch;
> > + struct cyttsp5_touch tch;
> > + u8 num_cur_tch;
> > +
> > + cyttsp5_get_touch_axis(&tch.hdr, si->tch_hdr.size,
> > + si->tch_hdr.max,
> > + si->xy_mode + 3 + si->tch_hdr.ofs,
> > + si->tch_hdr.bofs);
> > +
> > + num_cur_tch = tch.hdr;
> > + if (num_cur_tch > max_tch) {
> > + dev_err(dev, "Num touch err detected (n=%d)\n", num_cur_tch);
> > + num_cur_tch = max_tch;
> > + }
> > +
> > + if (num_cur_tch == 0 && ts->num_prv_rec == 0)
> > + return 0;
> > +
> > + /* extract xy_data for all currently reported touches */
> > + if (num_cur_tch)
> > + cyttsp5_get_mt_touches(ts, &tch, num_cur_tch);
> > + else
> > + cyttsp5_mt_lift_all(ts);
> > +
> > + return 0;
> > +}
> > +
> > +static int cyttsp5_mt_attention(struct device *dev)
> > +{
> > + struct cyttsp5 *ts = dev_get_drvdata(dev);
>
> Why don't you pass ts into cyttsp5_mt_attention?
Yep, you are right, I will update it.
>
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > + int rc;
> > +
> > + if (si->xy_mode[2] != HID_TOUCH_REPORT_ID)
> > + return 0;
> > +
> > + /* core handles handshake */
> > + rc = cyttsp5_xy_worker(ts);
> > + if (rc < 0)
> > + dev_err(dev, "xy_worker error r=%d\n", rc);
> > +
> > + return rc;
> > +}
> > +
> > +static int cyttsp5_setup_input_device(struct device *dev)
> > +{
> > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > + int max_x, max_y, max_p;
> > + int max_x_tmp, max_y_tmp;
> > + int rc;
> > +
> > + __set_bit(EV_ABS, ts->input->evbit);
> > + __set_bit(EV_REL, ts->input->evbit);
> > + __set_bit(EV_KEY, ts->input->evbit);
> > +
> > + max_x_tmp = si->sensing_conf_data.res_x;
> > + max_y_tmp = si->sensing_conf_data.res_y;
> > + max_x = max_y_tmp - 1;
> > + max_y = max_x_tmp - 1;
> > + max_p = si->sensing_conf_data.max_z;
> > +
> > + input_mt_init_slots(ts->input, si->tch_abs[CY_TCH_T].max, 0);
>
> Error handling.
Thanks.
>
> > +
> > + __set_bit(ABS_MT_POSITION_X, ts->input->absbit);
> > + __set_bit(ABS_MT_POSITION_Y, ts->input->absbit);
> > + __set_bit(ABS_MT_PRESSURE, ts->input->absbit);
> > +
> > + input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, max_x, 0, 0);
> > + input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, max_y, 0, 0);
> > + input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, max_p, 0, 0);
> > +
> > + input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
> > + input_set_abs_params(ts->input, ABS_MT_TOUCH_MINOR, 0, MAX_AREA, 0, 0);
>
> You might want to use the standard DT touchscreen properties and
> reporting functions to support axis inversion and swapping of axes.
Hm, I am not sure to see what you are referring to.
Could you explain me what you think of?
>
> > +
> > + rc = input_register_device(ts->input);
> > + if (rc < 0)
> > + dev_err(dev, "Error, failed register input device r=%d\n", rc);
> > +
> > + return rc;
> > +}
> > +
> > +#ifdef CONFIG_OF
> > +static int cyttsp5_parse_dt_key_code(struct device *dev)
> > +{
> > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > + struct device_node *np;
> > + int i;
> > +
> > + np = dev->of_node;
> > + if (!np)
> > + return -EINVAL;
> > +
> > + if (!si->num_btns)
> > + return 0;
> > +
> > + /* Initialize the button to RESERVED */
> > + for (i = 0; i < si->num_btns; i++)
> > + si->key_code[i] = KEY_RESERVED;
> > +
> > + return of_property_read_u32_array(np, "linux,keycodes",
> > + si->key_code, si->num_btns);
> > +}
> > +#else
> > +static int cyttsp5_parse_dt_key_code(struct device *dev)
> > +{
> > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > + int i;
> > +
> > + if (!si->num_btns)
> > + return 0;
> > +
> > + /* Initialize the button to RESERVED */
> > + for (i = 0; i < si->num_btns; i++)
> > + si->key_code[i] = KEY_RESERVED;
> > +
> > + return 0;
> > +}
> > +#endif
> > +
> > +static int cyttsp5_btn_attention(struct device *dev)
> > +{
> > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > + int cur_btn;
> > + int cur_btn_state;
> > +
> > + if (si->xy_mode[2] != HID_BTN_REPORT_ID || !si->num_btns)
> > + return 0;
> > +
> > + /* extract button press/release touch information */
> > + for (cur_btn = 0; cur_btn < si->num_btns; cur_btn++) {
> > + /* Get current button state */
> > + cur_btn_state = (si->xy_data[0] >> (cur_btn * CY_BITS_PER_BTN))
> > + & CY_NUM_BTN_EVENT_ID;
> > +
> > + input_report_key(ts->input, si->key_code[cur_btn],
> > + cur_btn_state);
> > + input_sync(ts->input);
>
> You do not need to sen sync after each button, send it once after all
> buttons sent.
Oh, yes, I will do that!
>
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static u16 cyttsp5_compute_crc(u8 *buf, u32 size)
> > +{
> > + u16 remainder = 0xFFFF;
> > + u16 xor_mask = 0x0000;
> > + u32 index;
> > + u32 byte_value;
> > + u32 table_index;
> > + u32 crc_bit_width = sizeof(u16) * 8;
> > +
> > + /* Divide the message by polynomial, via the table. */
> > + for (index = 0; index < size; index++) {
> > + byte_value = buf[index];
> > + table_index = ((byte_value >> 4) & 0x0F)
> > + ^ (remainder >> (crc_bit_width - 4));
> > + remainder = crc_itu_t_table[table_index]
> > + ^ (remainder << 4);
> > + table_index = (byte_value & 0x0F)
> > + ^ (remainder >> (crc_bit_width - 4));
> > + remainder = crc_itu_t_table[table_index]
> > + ^ (remainder << 4);
> > + }
> > +
> > + /* Perform the final remainder CRC. */
> > + return remainder ^ xor_mask;
> > +}
> > +
> > +static int cyttsp5_validate_cmd_response(struct cyttsp5 *ts, u8 code)
> > +{
> > + u16 size, crc;
> > + u8 status, offset;
> > + int command_code;
> > +
> > + size = get_unaligned_le16(&ts->response_buf[0]);
> > +
> > + if (!size)
> > + return 0;
> > +
> > + offset = ts->response_buf[HID_OUTPUT_RESPONSE_REPORT_OFFSET];
> > +
> > + if (offset == HID_BL_RESPONSE_REPORT_ID) {
> > + if (ts->response_buf[4] != HID_OUTPUT_BL_SOP) {
> > + dev_err(ts->dev, "HID output response, wrong SOP\n");
> > + return -EPROTO;
> > + }
> > +
> > + if (ts->response_buf[size - 1] != HID_OUTPUT_BL_EOP) {
> > + dev_err(ts->dev, "HID output response, wrong EOP\n");
> > + return -EPROTO;
> > + }
> > +
> > + crc = cyttsp5_compute_crc(&ts->response_buf[4], size - 7);
> > + if (ts->response_buf[size - 3] != LOW_BYTE(crc) ||
> > + ts->response_buf[size - 2] != HI_BYTE(crc)) {
> > + dev_err(ts->dev, "HID output response, wrong CRC 0x%X\n",
> > + crc);
> > + return -EPROTO;
> > + }
> > +
> > + status = ts->response_buf[5];
> > + if (status) {
> > + dev_err(ts->dev, "HID output response, ERROR:%d\n",
> > + status);
> > + return -EPROTO;
> > + }
> > + }
> > +
> > + if (offset == HID_APP_RESPONSE_REPORT_ID) {
> > + command_code = ts->response_buf[HID_OUTPUT_RESPONSE_CMD_OFFSET]
> > + & HID_OUTPUT_RESPONSE_CMD_MASK;
> > + if (command_code != code) {
> > + dev_err(ts->dev,
> > + "HID output response, wrong command_code:%X\n",
> > + command_code);
> > + return -EPROTO;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void cyttsp5_si_get_btn_data(struct cyttsp5 *ts)
> > +{
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > + int i;
> > + unsigned int btns = ts->response_buf[HID_SYSINFO_BTN_OFFSET]
> > + & HID_SYSINFO_BTN_MASK;
> > +
> > + si->num_btns = 0;
> > + for (i = 0; i < HID_SYSINFO_MAX_BTN; i++) {
> > + if (btns & BIT(i))
> > + si->num_btns++;
> > + }
>
> si->num_btns = hweight8(ts->response_buf[HID_SYSINFO_BTN_OFFSET]);
Great, I will try that!
>
> > +}
> > +
> > +static int cyttsp5_get_sysinfo_regs(struct cyttsp5 *ts)
> > +{
> > + struct cyttsp5_sensing_conf_data *scd = &ts->sysinfo.sensing_conf_data;
> > + struct cyttsp5_sensing_conf_data_dev *scd_dev =
> > + (struct cyttsp5_sensing_conf_data_dev *)
> > + &ts->response_buf[HID_SYSINFO_SENSING_OFFSET];
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > +
> > + cyttsp5_si_get_btn_data(ts);
> > +
> > + scd->max_tch = scd_dev->max_num_of_tch_per_refresh_cycle;
> > + scd->res_x = get_unaligned_le16(&scd_dev->res_x);
> > + scd->res_y = get_unaligned_le16(&scd_dev->res_y);
> > + scd->max_z = get_unaligned_le16(&scd_dev->max_z);
> > + scd->len_x = get_unaligned_le16(&scd_dev->len_x);
> > + scd->len_y = get_unaligned_le16(&scd_dev->len_y);
> > +
> > + si->xy_data = devm_kzalloc(ts->dev, scd->max_tch * TOUCH_REPORT_SIZE,
> > + GFP_KERNEL);
> > + if (!si->xy_data)
> > + return -ENOMEM;
> > +
> > + si->xy_mode = devm_kzalloc(ts->dev, TOUCH_INPUT_HEADER_SIZE,
> > + GFP_KERNEL);
> > + if (!si->xy_mode)
> > + return -ENOMEM;
>
> Why do we need these 2 allocated separately form the driver data?
xy_mode and particularly xy_data are used to retrieve buttons or touch
data that we received from the input buffer (see move_button_data()
and move_touch_data()). I guess that I can try to remove them, if you
want.
>
> > +
> > + return 0;
> > +}
> > +
> > +static int cyttsp5_hid_output_get_sysinfo(struct cyttsp5 *ts)
> > +{
> > + int rc;
> > + u8 cmd[HID_OUTPUT_GET_SYSINFO_SIZE];
> > +
> > + ts->hid_cmd_state = HID_CMD_BUSY;
> > +
> > + /* HI bytes of Output register address */
> > + cmd[0] = LOW_BYTE(HID_OUTPUT_GET_SYSINFO_SIZE);
> > + cmd[1] = HI_BYTE(HID_OUTPUT_GET_SYSINFO_SIZE);
> > + cmd[2] = HID_APP_OUTPUT_REPORT_ID;
> > + cmd[3] = 0x0; /* Reserved */
> > + cmd[4] = HID_OUTPUT_GET_SYSINFO;
> > +
> > + rc = cyttsp5_write(ts, HID_OUTPUT_REG, cmd,
> > + HID_OUTPUT_GET_SYSINFO_SIZE);
> > + if (rc) {
> > + dev_err(ts->dev, "Failed to write command %d", rc);
> > + goto error;
> > + }
> > +
> > + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> > + msecs_to_jiffies(CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT));
> > + if (!rc) {
> > + dev_err(ts->dev, "HID output cmd execution timed out\n");
> > + rc = -ETIME;
> > + goto error;
> > + }
> > +
> > + rc = cyttsp5_validate_cmd_response(ts, HID_OUTPUT_GET_SYSINFO);
> > + if (rc) {
> > + dev_err(ts->dev, "Validation of the response failed\n");
> > + goto error;
> > + }
> > +
> > + return cyttsp5_get_sysinfo_regs(ts);
> > +
> > +error:
> > + mutex_lock(&ts->system_lock);
> > + ts->hid_cmd_state = HID_CMD_DONE;
> > + mutex_unlock(&ts->system_lock);
> > + return rc;
> > +}
> > +
> > +static int cyttsp5_hid_output_bl_launch_app(struct cyttsp5 *ts)
> > +{
> > + int rc;
> > + u8 cmd[HID_OUTPUT_BL_LAUNCH_APP];
> > + u16 crc;
> > +
> > + mutex_lock(&ts->system_lock);
> > + ts->hid_cmd_state = HID_CMD_BUSY;
> > + mutex_unlock(&ts->system_lock);
> > +
> > + cmd[0] = LOW_BYTE(HID_OUTPUT_BL_LAUNCH_APP_SIZE);
> > + cmd[1] = HI_BYTE(HID_OUTPUT_BL_LAUNCH_APP_SIZE);
> > + cmd[2] = HID_BL_OUTPUT_REPORT_ID;
> > + cmd[3] = 0x0; /* Reserved */
> > + cmd[4] = HID_OUTPUT_BL_SOP;
> > + cmd[5] = HID_OUTPUT_BL_LAUNCH_APP;
> > + cmd[6] = 0x0; /* Low bytes of data */
> > + cmd[7] = 0x0; /* Hi bytes of data */
> > + crc = cyttsp5_compute_crc(&cmd[4], 4);
> > + cmd[8] = LOW_BYTE(crc);
> > + cmd[9] = HI_BYTE(crc);
> > + cmd[10] = HID_OUTPUT_BL_EOP;
> > +
> > + rc = cyttsp5_write(ts, HID_OUTPUT_REG, cmd,
> > + HID_OUTPUT_BL_LAUNCH_APP_SIZE);
> > + if (rc) {
> > + dev_err(ts->dev, "Failed to write command %d", rc);
> > + goto error;
> > + }
> > +
> > + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> > + msecs_to_jiffies(CY_HID_OUTPUT_TIMEOUT));
> > + if (!rc) {
> > + dev_err(ts->dev, "HID output cmd execution timed out\n");
> > + rc = -ETIME;
> > + goto error;
> > + }
> > +
> > + rc = cyttsp5_validate_cmd_response(ts, HID_OUTPUT_BL_LAUNCH_APP);
> > + if (rc) {
> > + dev_err(ts->dev, "Validation of the response failed\n");
> > + goto error;
> > + }
> > +
> > + return rc;
> > +
> > +error:
> > + mutex_lock(&ts->system_lock);
> > + ts->hid_cmd_state = HID_CMD_DONE;
> > + mutex_unlock(&ts->system_lock);
> > +
> > + return rc;
> > +}
> > +
> > +static int cyttsp5_get_hid_descriptor(struct cyttsp5 *ts,
> > + struct cyttsp5_hid_desc *desc)
> > +{
> > + struct device *dev = ts->dev;
> > + __le16 hid_desc_register = HID_DESC_REG;
> > + int rc;
> > + u8 cmd[2];
> > +
> > + /* Read HID descriptor length and version */
> > + mutex_lock(&ts->system_lock);
> > + ts->hid_cmd_state = HID_CMD_BUSY;
> > + mutex_unlock(&ts->system_lock);
> > +
> > + /* Set HID descriptor register */
> > + memcpy(cmd, &hid_desc_register, sizeof(hid_desc_register));
> > +
> > + rc = cyttsp5_write(ts, HID_DESC_REG, NULL, 0);
> > + if (rc < 0) {
> > + dev_err(dev, "Failed to get HID descriptor, rc=%d\n", rc);
> > + goto error;
> > + }
> > +
> > + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> > + msecs_to_jiffies(CY_HID_GET_HID_DESCRIPTOR_TIMEOUT));
> > + if (!rc) {
> > + dev_err(ts->dev, "HID get descriptor timed out\n");
> > + rc = -ETIME;
> > + goto error;
> > + }
> > +
> > + memcpy(desc, ts->response_buf, sizeof(struct cyttsp5_hid_desc));
> > +
> > + /* Check HID descriptor length and version */
> > + if (le16_to_cpu(desc->hid_desc_len) != sizeof(*desc) ||
> > + le16_to_cpu(desc->bcd_version) != HID_VERSION) {
> > + dev_err(dev, "Unsupported HID version\n");
> > + return -ENODEV;
> > + }
> > +
> > + goto exit;
> > +
> > +error:
> > + mutex_lock(&ts->system_lock);
> > + ts->hid_cmd_state = HID_CMD_DONE;
> > + mutex_unlock(&ts->system_lock);
> > +exit:
> > + return rc;
> > +}
> > +
> > +static int fill_tch_abs(struct cyttsp5_tch_abs_params *tch_abs, int report_size,
> > + int offset)
> > +{
> > + tch_abs->ofs = offset / 8;
> > + tch_abs->size = report_size / 8;
> > + if (report_size % 8)
> > + tch_abs->size += 1;
> > + tch_abs->min = 0;
> > + tch_abs->max = 1 << report_size;
> > + tch_abs->bofs = offset - (tch_abs->ofs << 3);
> > +
> > + return 0;
> > +}
> > +
> > +static int move_button_data(struct cyttsp5 *ts, struct cyttsp5_sysinfo *si)
> > +{
> > + memcpy(si->xy_mode, ts->input_buf, BTN_INPUT_HEADER_SIZE);
> > + memcpy(si->xy_data, &ts->input_buf[BTN_INPUT_HEADER_SIZE],
> > + BTN_REPORT_SIZE);
> > +
> > + return 0;
> > +}
> > +
> > +static int move_touch_data(struct cyttsp5 *ts, struct cyttsp5_sysinfo *si)
> > +{
> > + int max_tch = si->sensing_conf_data.max_tch;
> > + int num_cur_tch;
> > + int length;
> > + struct cyttsp5_tch_abs_params *tch = &si->tch_hdr;
> > +
> > + memcpy(si->xy_mode, ts->input_buf, TOUCH_INPUT_HEADER_SIZE);
> > +
> > + cyttsp5_get_touch_axis(&num_cur_tch, tch->size,
> > + tch->max, si->xy_mode + 3 + tch->ofs, tch->bofs);
> > + if (unlikely(num_cur_tch > max_tch))
> > + num_cur_tch = max_tch;
> > +
> > + length = num_cur_tch * TOUCH_REPORT_SIZE;
> > +
> > + memcpy(si->xy_data, &ts->input_buf[TOUCH_INPUT_HEADER_SIZE], length);
> > +
> > + return 0;
> > +}
> > +
> > +static irqreturn_t cyttsp5_handle_irq(int irq, void *handle)
> > +{
> > + struct cyttsp5 *ts = handle;
> > + int report_id;
> > + int size;
> > + int rc;
> > +
> > + rc = cyttsp5_read(ts, ts->input_buf, CY_MAX_INPUT);
> > + if (rc)
> > + return IRQ_HANDLED;
> > +
> > + size = get_unaligned_le16(&ts->input_buf[0]);
> > +
> > + /* check reset */
> > + if (size == 0) {
> > + memcpy(ts->response_buf, ts->input_buf, 2);
> > +
> > + mutex_lock(&ts->system_lock);
> > + ts->hid_cmd_state = HID_CMD_DONE;
> > + mutex_unlock(&ts->system_lock);
> > + wake_up(&ts->wait_q);
>
> I'd use spinlock in wait queue and wake_up_locked() instead of a
> separate mutex.
Hum, okay, I will try that.
>
> > + return IRQ_HANDLED;
> > + }
> > +
> > + report_id = ts->input_buf[2];
> > +
>
> switch(report_id) {
> }
Yep, it will be easier to understand.
>
> > + if (report_id == HID_TOUCH_REPORT_ID) {
> > + move_touch_data(ts, &ts->sysinfo);
> > + cyttsp5_mt_attention(ts->dev);
> > + } else if (report_id == HID_BTN_REPORT_ID) {
> > + move_button_data(ts, &ts->sysinfo);
> > + cyttsp5_btn_attention(ts->dev);
> > + } else {
> > + /* It is not an input but a command response */
> > + memcpy(ts->response_buf, ts->input_buf, size);
> > +
> > + mutex_lock(&ts->system_lock);
> > + ts->hid_cmd_state = HID_CMD_DONE;
> > + mutex_unlock(&ts->system_lock);
> > + wake_up(&ts->wait_q);
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int cyttsp5_deassert_int(struct cyttsp5 *ts)
> > +{
> > + u16 size;
> > + u8 buf[2];
> > + int rc;
> > +
> > + rc = regmap_bulk_read(ts->regmap, HID_INPUT_REG, buf, 2);
> > + if (rc < 0)
> > + return rc;
> > +
> > + size = get_unaligned_le16(&buf[0]);
> > + if (size == 2 || size == 0)
> > + return 0;
> > +
> > + return -EINVAL;
> > +}
> > +
> > +static int cyttsp5_fill_all_touch(struct cyttsp5 *ts)
> > +{
> > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > +
> > + fill_tch_abs(&si->tch_abs[CY_TCH_X], REPORT_SIZE_16,
> > + TOUCH_REPORT_DESC_X);
> > + fill_tch_abs(&si->tch_abs[CY_TCH_Y], REPORT_SIZE_16,
> > + TOUCH_REPORT_DESC_Y);
> > + fill_tch_abs(&si->tch_abs[CY_TCH_P], REPORT_SIZE_8,
> > + TOUCH_REPORT_DESC_P);
> > + fill_tch_abs(&si->tch_abs[CY_TCH_T], REPORT_SIZE_5,
> > + TOUCH_REPORT_DESC_CONTACTID);
> > + fill_tch_abs(&si->tch_hdr, REPORT_SIZE_5,
> > + TOUCH_REPORT_DESC_HDR_CONTACTCOUNT);
> > + fill_tch_abs(&si->tch_abs[CY_TCH_MAJ], REPORT_SIZE_8,
> > + TOUCH_REPORT_DESC_MAJ);
> > + fill_tch_abs(&si->tch_abs[CY_TCH_MIN], REPORT_SIZE_8,
> > + TOUCH_REPORT_DESC_MIN);
> > +
> > + return 0;
> > +}
> > +
> > +static int cyttsp5_startup(struct cyttsp5 *ts)
> > +{
> > + int rc;
> > +
> > + rc = cyttsp5_deassert_int(ts);
> > + if (rc) {
> > + dev_err(ts->dev, "Error on deassert int r=%d\n", rc);
> > + return -ENODEV;
> > + }
> > +
> > + /*
> > + * Launch the application as the device starts in bootloader mode
> > + * because of a power-on-reset
> > + */
> > + rc = cyttsp5_hid_output_bl_launch_app(ts);
> > + if (rc < 0) {
> > + dev_err(ts->dev, "Error on launch app r=%d\n", rc);
> > + return rc;
> > + }
> > +
> > + rc = cyttsp5_get_hid_descriptor(ts, &ts->hid_desc);
> > + if (rc < 0) {
> > + dev_err(ts->dev, "Error on getting HID descriptor r=%d\n", rc);
> > + return rc;
> > + }
> > +
> > + rc = cyttsp5_fill_all_touch(ts);
> > + if (rc < 0) {
> > + dev_err(ts->dev, "Error on report descriptor r=%d\n", rc);
> > + return rc;
> > + }
> > +
> > + rc = cyttsp5_hid_output_get_sysinfo(ts);
> > + if (rc) {
> > + dev_err(ts->dev, "Error on getting sysinfo r=%d\n", rc);
> > + return rc;
> > + }
> > +
> > + return rc;
> > +}
> > +
> > +#ifdef CONFIG_OF
> > +static const struct of_device_id cyttsp5_of_match[] = {
> > + { .compatible = "cypress,cyttsp5", },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(of, cyttsp5_of_match);
> > +#endif
> > +
> > +static const struct i2c_device_id cyttsp5_i2c_id[] = {
> > + { CYTTSP5_NAME, 0, },
> > + { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, cyttsp5_i2c_id);
> > +
> > +static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> > + const char *name)
> > +{
> > + struct cyttsp5 *ts;
> > + struct cyttsp5_sysinfo *si;
> > + int rc = 0, i;
> > +
> > + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> > + if (!ts)
> > + return -ENOMEM;
> > +
> > + /* Initialize device info */
> > + ts->regmap = regmap;
> > + ts->dev = dev;
> > + si = &ts->sysinfo;
> > + dev_set_drvdata(dev, ts);
> > +
> > + /* Initialize mutexes and spinlocks */
> > + mutex_init(&ts->system_lock);
> > +
> > + /* Initialize wait queue */
> > + init_waitqueue_head(&ts->wait_q);
> > +
> > + /* Reset the gpio to be in a reset state */
> > + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> > + if (IS_ERR(ts->reset_gpio)) {
> > + rc = PTR_ERR(ts->reset_gpio);
> > + dev_err(dev, "Failed to request reset gpio, error %d\n", rc);
> > + return rc;
> > + }
> > + gpiod_set_value(ts->reset_gpio, 1);
>
> Why not request it as GPIOD_OUT_HIGH if you going to activate it
> immediately? However it sounds as if you have a reset line that is
> active low and you want to driver it low to reset, and then release. In
> this case you need to do
>
> ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> <delay> - chip usually needs to be some time in reset state
> gpiod_set_value(ts->reset_gpio, 0);
>
> and make sure that correct polarity is specified in device tree.
Okay, I will update it too.
>
> > +
> > + /* Need a delay to have device up */
> > + msleep(20);
> > +
> > + rc = devm_request_threaded_irq(dev, irq, NULL, cyttsp5_handle_irq,
> > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>
> Just use IRQF_ONESHOT and make sure DTS specifies interrupt properly.
> BTW,I'd recommend using level, not edge interrupts. It is so easy to
> miss an edge.
Okay, thank you for the advice.
>
> > + name, ts);
> > + if (rc) {
> > + dev_err(dev, "unable to request IRQ\n");
> > + return rc;
> > + }
> > +
> > + rc = cyttsp5_startup(ts);
> > + if (rc) {
> > + dev_err(ts->dev, "Fail initial startup r=%d\n", rc);
> > + return rc;
> > + }
> > +
> > + rc = cyttsp5_parse_dt_key_code(dev);
> > + if (rc < 0) {
> > + dev_err(ts->dev, "Error while parsing dts %d\n", rc);
> > + return rc;
> > + }
> > +
> > + ts->input = devm_input_allocate_device(dev);
> > + if (!ts->input) {
> > + dev_err(dev, "Error, failed to allocate input device\n");
> > + return -ENODEV;
> > + }
> > +
> > + ts->input->name = "cyttsp5";
> > + scnprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
> > + ts->input->phys = ts->phys;
> > + ts->input->dev.parent = ts->dev;
> > + input_set_drvdata(ts->input, ts);
> > +
> > + touchscreen_parse_properties(ts->input, true, NULL);
> > +
> > + __set_bit(EV_KEY, ts->input->evbit);
> > + for (i = 0; i < si->num_btns; i++)
> > + __set_bit(si->key_code[i], ts->input->keybit);
> > +
> > + return cyttsp5_setup_input_device(dev);
> > +}
> > +
> > +static int cyttsp5_i2c_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct regmap *regmap;
> > + static const struct regmap_config config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > + };
> > +
> > + regmap = devm_regmap_init_i2c(client, &config);
> > + if (IS_ERR(regmap)) {
> > + dev_err(&client->dev, "regmap allocation failed: %ld\n",
> > + PTR_ERR(regmap));
> > + return PTR_ERR(regmap);
> > + }
> > +
> > + return cyttsp5_probe(&client->dev, regmap, client->irq, client->name);
> > +}
> > +
> > +static int cyttsp5_remove(struct device *dev)
> > +{
> > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> > +
> > + input_unregister_device(ts->input);
>
> Not needed: you are using managed input device.
You are right, thanks.
>
> > +
> > + return 0;
> > +}
> > +
> > +static int cyttsp5_i2c_remove(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > +
> > + return cyttsp5_remove(dev);
>
> Not needed if you remove cyttsp5_remove().
Sure.
>
> > +}
> > +
> > +static struct i2c_driver cyttsp5_i2c_driver = {
> > + .driver = {
> > + .name = CYTTSP5_NAME,
> > + .owner = THIS_MODULE,
> > + .of_match_table = of_match_ptr(cyttsp5_of_match),
> > + },
> > + .probe = cyttsp5_i2c_probe,
> > + .remove = cyttsp5_i2c_remove,
> > + .id_table = cyttsp5_i2c_id,
> > +};
> > +
> > +module_i2c_driver(cyttsp5_i2c_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_DESCRIPTION("Touchscreen driver for Cypress TrueTouch Gen 5 Product");
> > +MODULE_AUTHOR("Mylène Josserand <mylene.josserand@free-electrons.com>");
> > --
> > 2.11.0
> >
>
> Thanks.
>
Best regards,
--
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen
From: landyn.lawrence-jHc5sQ61u836K7/ahGyk6A @ 2018-01-23 10:50 UTC (permalink / raw)
To: Mylene Josserand, Dmitry Torokhov
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
In-Reply-To: <20180123114459.07cc3404-K8i4uRanGBt8XcdJbWeDu7NAH6kLmebB@public.gmane.org>
Hey,
thanks for the great work on this one!
Best regards
On 01/23/2018 11:44 AM, Mylene Josserand wrote:
> Hi Dmitry,
>
> Thank you for the review.
>
> Le Mon, 22 Jan 2018 20:15:03 -0800,
> Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> a écrit :
>
>> Hi Mylène,
>>
>> On Fri, Dec 01, 2017 at 04:39:56PM +0100, Mylène Josserand wrote:
>>> This is the basic driver for the Cypress TrueTouch Gen5 touchscreen
>>> controllers. This driver supports only the I2C bus but it uses regmap
>>> so SPI support could be added later.
>>> The touchscreen can retrieve some defined zone that are handled as
>>> buttons (according to the hardware). That is why it handles
>>> button and multitouch events.
>>>
>>> Reviewed-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>> Signed-off-by: Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>>> ---
>>> drivers/input/touchscreen/Kconfig | 16 +
>>> drivers/input/touchscreen/Makefile | 1 +
>>> drivers/input/touchscreen/cyttsp5.c | 1110 +++++++++++++++++++++++++++++++++++
>>> 3 files changed, 1127 insertions(+)
>>> create mode 100644 drivers/input/touchscreen/cyttsp5.c
>>>
>>> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
>>> index 0cfdb7cb610e..28eea6d5f1bb 100644
>>> --- a/drivers/input/touchscreen/Kconfig
>>> +++ b/drivers/input/touchscreen/Kconfig
>>> @@ -238,6 +238,22 @@ config TOUCHSCREEN_CYTTSP4_SPI
>>> To compile this driver as a module, choose M here: the
>>> module will be called cyttsp4_spi.
>>>
>>> +config TOUCHSCREEN_CYTTSP5
>>> + tristate "Cypress TrueTouch Gen5 Touchscreen Driver"
>>> + depends on OF
>>
>> Does it have to be? Please use generic device properties
>> (device_property_* API) and it can be used with ACPI and static board
>> files, of needed.
>
> Okay, I will try with that.
>
>>
>>> + select REGMAP_I2C
>>> + select CRC_ITU_T
>>> + help
>>> + Driver for Parade TrueTouch Standard Product
>>> + Generation 5 touchscreen controllers.
>>> + I2C bus interface support only.
>>> +
>>> + Say Y here if you have a Cypress Gen5 touchscreen.
>>> + If unsure, say N.
>>> +
>>> + To compile this driver as a module, choose M here: the
>>> + module will be called cyttsp5.
>>> +
>>> config TOUCHSCREEN_DA9034
>>> tristate "Touchscreen support for Dialog Semiconductor DA9034"
>>> depends on PMIC_DA903X
>>> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
>>> index d2a2b3b7af27..e7d124901dd9 100644
>>> --- a/drivers/input/touchscreen/Makefile
>>> +++ b/drivers/input/touchscreen/Makefile
>>> @@ -26,6 +26,7 @@ obj-$(CONFIG_TOUCHSCREEN_CYTTSP_SPI) += cyttsp_spi.o
>>> obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_CORE) += cyttsp4_core.o
>>> obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_I2C) += cyttsp4_i2c.o cyttsp_i2c_common.o
>>> obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_SPI) += cyttsp4_spi.o
>>> +obj-$(CONFIG_TOUCHSCREEN_CYTTSP5) += cyttsp5.o
>>> obj-$(CONFIG_TOUCHSCREEN_DA9034) += da9034-ts.o
>>> obj-$(CONFIG_TOUCHSCREEN_DA9052) += da9052_tsi.o
>>> obj-$(CONFIG_TOUCHSCREEN_DYNAPRO) += dynapro.o
>>> diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
>>> new file mode 100644
>>> index 000000000000..a41feea2cd5a
>>> --- /dev/null
>>> +++ b/drivers/input/touchscreen/cyttsp5.c
>>> @@ -0,0 +1,1110 @@
>>> +/*
>>> + * Parade TrueTouch(TM) Standard Product V5 Module.
>>> + * For use with Parade touchscreen controllers.
>>> + *
>>> + * Copyright (C) 2015 Parade Technologies
>>> + * Copyright (C) 2012-2015 Cypress Semiconductor
>>> + * Copyright (C) 2017 Free Electrons
>>> + *
>>> + * Author: Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.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, and only version 2, as published by the
>>> + * Free Software Foundation.
>>> + *
>>> + * 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 <asm/unaligned.h>
>>> +#include <linux/crc-itu-t.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/device.h>
>>> +#include <linux/gpio.h>
>>> +#include <linux/input/mt.h>
>>> +#include <linux/input/touchscreen.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/i2c.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/regmap.h>
>>> +
>>> +#define CYTTSP5_NAME "cyttsp5"
>>> +#define CY_I2C_DATA_SIZE (2 * 256)
>>> +#define HID_VERSION 0x0100
>>> +#define CY_MAX_INPUT 512
>>> +#define CYTTSP5_PREALLOCATED_CMD_BUFFER 32
>>> +#define CY_BITS_PER_BTN 1
>>> +#define CY_NUM_BTN_EVENT_ID ((1 << CY_BITS_PER_BTN) - 1)
>>> +
>>> +#define MAX_AREA 255
>>> +#define HID_OUTPUT_BL_SOP 0x1
>>> +#define HID_OUTPUT_BL_EOP 0x17
>>> +#define HID_OUTPUT_BL_LAUNCH_APP 0x3B
>>> +#define HID_OUTPUT_BL_LAUNCH_APP_SIZE 11
>>> +#define HID_OUTPUT_GET_SYSINFO 0x2
>>> +#define HID_OUTPUT_GET_SYSINFO_SIZE 5
>>> +
>>> +#define HID_DESC_REG 0x1
>>> +#define HID_INPUT_REG 0x3
>>> +#define HID_OUTPUT_REG 0x4
>>> +
>>> +#define REPORT_ID_TOUCH 0x1
>>> +#define REPORT_ID_BTN 0x3
>>> +#define REPORT_SIZE_5 5
>>> +#define REPORT_SIZE_8 8
>>> +#define REPORT_SIZE_16 16
>>> +
>>> +/* Touch reports offsets */
>>> +/* Header offsets */
>>> +#define TOUCH_REPORT_DESC_HDR_CONTACTCOUNT 16
>>> +/* Record offsets */
>>> +#define TOUCH_REPORT_DESC_CONTACTID 8
>>> +#define TOUCH_REPORT_DESC_X 16
>>> +#define TOUCH_REPORT_DESC_Y 32
>>> +#define TOUCH_REPORT_DESC_P 48
>>> +#define TOUCH_REPORT_DESC_MAJ 56
>>> +#define TOUCH_REPORT_DESC_MIN 64
>>> +
>>> +/* HID */
>>> +#define HID_TOUCH_REPORT_ID 0x1
>>> +#define HID_BTN_REPORT_ID 0x3
>>> +#define HID_APP_RESPONSE_REPORT_ID 0x1F
>>> +#define HID_APP_OUTPUT_REPORT_ID 0x2F
>>> +#define HID_BL_RESPONSE_REPORT_ID 0x30
>>> +#define HID_BL_OUTPUT_REPORT_ID 0x40
>>> +
>>> +#define HID_OUTPUT_RESPONSE_REPORT_OFFSET 2
>>> +#define HID_OUTPUT_RESPONSE_CMD_OFFSET 4
>>> +#define HID_OUTPUT_RESPONSE_CMD_MASK 0x7F
>>> +
>>> +#define HID_SYSINFO_SENSING_OFFSET 33
>>> +#define HID_SYSINFO_BTN_OFFSET 48
>>> +#define HID_SYSINFO_BTN_MASK 0xFF
>>> +#define HID_SYSINFO_MAX_BTN 8
>>> +
>>> +/* Timeout in ms */
>>> +#define CY_HID_OUTPUT_TIMEOUT 200
>>> +#define CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT 3000
>>> +#define CY_HID_GET_HID_DESCRIPTOR_TIMEOUT 4000
>>> +
>>> +/* maximum number of concurrent tracks */
>>> +#define TOUCH_REPORT_SIZE 10
>>> +#define TOUCH_INPUT_HEADER_SIZE 7
>>> +#define BTN_REPORT_SIZE 9
>>> +#define BTN_INPUT_HEADER_SIZE 5
>>> +
>>> +/* All usage pages for Touch Report */
>>> +#define TOUCH_REPORT_USAGE_PG_X 0x00010030
>>> +#define TOUCH_REPORT_USAGE_PG_Y 0x00010031
>>> +#define TOUCH_REPORT_USAGE_PG_P 0x000D0030
>>> +#define TOUCH_REPORT_USAGE_PG_CONTACTID 0x000D0051
>>> +#define TOUCH_REPORT_USAGE_PG_CONTACTCOUNT 0x000D0054
>>> +#define TOUCH_REPORT_USAGE_PG_MAJ 0xFF010062
>>> +#define TOUCH_REPORT_USAGE_PG_MIN 0xFF010063
>>> +#define TOUCH_COL_USAGE_PG 0x000D0022
>>> +
>>> +/* helpers */
>>> +#define HI_BYTE(x) (u8)(((x) >> 8) & 0xFF)
>>> +#define LOW_BYTE(x) (u8)((x) & 0xFF)
>>> +
>>> +/* System Information interface definitions */
>>> +struct cyttsp5_sensing_conf_data_dev {
>>> + u8 electrodes_x;
>>> + u8 electrodes_y;
>>> + __le16 len_x;
>>> + __le16 len_y;
>>> + __le16 res_x;
>>> + __le16 res_y;
>>> + __le16 max_z;
>>> + u8 origin_x;
>>> + u8 origin_y;
>>> + u8 btn;
>>> + u8 scan_mode;
>>> + u8 max_num_of_tch_per_refresh_cycle;
>>> +} __packed;
>>> +
>>> +struct cyttsp5_sensing_conf_data {
>>> + u16 res_x;
>>> + u16 res_y;
>>> + u16 max_z;
>>> + u16 len_x;
>>> + u16 len_y;
>>> + u8 origin_x;
>>> + u8 origin_y;
>>> + u8 max_tch;
>>> +};
>>> +
>>> +enum { HID_CMD_DONE, HID_CMD_BUSY } hid_cmd_state;
>>> +
>>> +enum cyttsp5_tch_abs { /* for ordering within the extracted touch data array */
>>> + CY_TCH_X, /* X */
>>> + CY_TCH_Y, /* Y */
>>> + CY_TCH_P, /* P (Z) */
>>> + CY_TCH_T, /* TOUCH ID */
>>> + CY_TCH_MAJ, /* TOUCH_MAJOR */
>>> + CY_TCH_MIN, /* TOUCH_MINOR */
>>> + CY_TCH_NUM_ABS,
>>> +};
>>> +
>>> +struct cyttsp5_tch_abs_params {
>>> + size_t ofs; /* abs byte offset */
>>> + size_t size; /* size in bits */
>>> + size_t min; /* min value */
>>> + size_t max; /* max value */
>>> + size_t bofs; /* bit offset */
>>> +};
>>> +
>>> +struct cyttsp5_touch {
>>> + int hdr;
>>> + int abs[CY_TCH_NUM_ABS];
>>> +};
>>> +
>>> +struct cyttsp5_sysinfo {
>>> + struct cyttsp5_sensing_conf_data sensing_conf_data;
>>> + int num_btns;
>>> + struct cyttsp5_tch_abs_params tch_hdr;
>>> + struct cyttsp5_tch_abs_params tch_abs[CY_TCH_NUM_ABS];
>>> + u32 key_code[HID_SYSINFO_MAX_BTN];
>>> + u8 *xy_mode;
>>> + u8 *xy_data;
>>> +};
>>> +
>>> +struct cyttsp5_hid_desc {
>>> + __le16 hid_desc_len;
>>> + u8 packet_id;
>>> + u8 reserved_byte;
>>> + __le16 bcd_version;
>>> + __le16 report_desc_len;
>>> + __le16 report_desc_register;
>>> + __le16 input_register;
>>> + __le16 max_input_len;
>>> + __le16 output_register;
>>> + __le16 max_output_len;
>>> + __le16 command_register;
>>> + __le16 data_register;
>>> + __le16 vendor_id;
>>> + __le16 product_id;
>>> + __le16 version_id;
>>> + u8 reserved[4];
>>> +} __packed;
>>> +
>>> +struct cyttsp5 {
>>> + struct device *dev;
>>> + struct mutex system_lock;
>>> + wait_queue_head_t wait_q;
>>> + struct cyttsp5_sysinfo sysinfo;
>>> + int hid_cmd_state;
>>> + struct cyttsp5_hid_desc hid_desc;
>>> + u8 cmd_buf[CYTTSP5_PREALLOCATED_CMD_BUFFER];
>>> + u8 input_buf[CY_MAX_INPUT];
>>> + u8 response_buf[CY_MAX_INPUT];
>>> + struct gpio_desc *reset_gpio;
>>> + struct input_dev *input;
>>> + char phys[NAME_MAX];
>>> + int num_prv_rec;
>>> + struct regmap *regmap;
>>> +};
>>> +
>>> +/*
>>> + * For what understood in the datasheet, the register does not
>>> + * matter. For consistency, used the Input Register address
>>> + * but it does mean anything to the device. The important data
>>> + * to send is the I2C address
>>> + */
>>> +static int cyttsp5_read(struct cyttsp5 *ts, u8 *buf, u32 max)
>>> +{
>>> + int rc;
>>> + u32 size;
>>> + u8 temp[2];
>>> +
>>> + if (!buf)
>>> + return -EINVAL;
>>
>> Is it really possible? This is not a publicly facing API but private
>> driver data, do you ever happen to call it with NULL?
>
> hm, I do not think so but I will check that.
>
>>
>>> +
>>> + /* Read the frame to retrieve the size */
>>> + rc = regmap_bulk_read(ts->regmap, HID_INPUT_REG, temp, 2);
>>> + if (rc < 0)
>>> + return rc;
>>
>> Can we call this kind of variables "error" pelase?
>
> Sure.
>
>>
>>> +
>>> + size = get_unaligned_le16(temp);
>>> + if (!size || size == 2)
>>> + return 0;
>>> +
>>> + if (size > max)
>>> + return -EINVAL;
>>> +
>>> + /* Get the real value */
>>> + return regmap_bulk_read(ts->regmap, HID_INPUT_REG, buf, size);
>>> +}
>>> +
>>> +static int cyttsp5_write(struct cyttsp5 *ts, unsigned int reg, u8 *data,
>>> + size_t size)
>>> +{
>>> + u8 cmd[size + 1];
>>> +
>>> + /* High bytes of register address needed as first byte of cmd */
>>> + cmd[0] = HI_BYTE(reg);
>>> +
>>> + /* Copy the rest of the data */
>>> + if (data)
>>> + memcpy(&cmd[1], data, size);
>>> +
>>> + /* The hardware wants to receive a frame with the address register
>>
>> The multiline comment block should start with "/*" on a separate line
>> please.
>
> I will change that.
>
>>
>>> + * contains in the first two bytes. As the regmap_write function
>>> + * add the register adresse in the frame, we use the LOW_BYTE as
>>> + * first frame byte for the address register and the first
>>> + * data byte is the high register + left of the cmd to send
>>> + */
>>> + return regmap_bulk_write(ts->regmap, LOW_BYTE(reg), cmd, size + 1);
>>> +}
>>> +
>>> +static void cyttsp5_final_sync(struct input_dev *input, int max_slots,
>>> + unsigned long *ids)
>>> +{
>>> + int t;
>>> +
>>> + for (t = 0; t < max_slots; t++) {
>>
>> for_each_set_bit() {
>> }
>
> I did not know this macro, I will try that.
>
>>
>>> + if (test_bit(t, ids))
>>> + continue;
>>> + input_mt_slot(input, t);
>>> + input_mt_report_slot_state(input, MT_TOOL_FINGER, false);
>>> + }
>>> +
>>> + input_sync(input);
>>> +}
>>> +
>>> +static void cyttsp5_report_slot_liftoff(struct cyttsp5 *ts, int max_slots)
>>> +{
>>> + int t;
>>> +
>>> + if (ts->num_prv_rec == 0)
>>> + return;
>>
>> Do you need this check? The caller checks the same condition.
>
> True, I will remove it.
>
>>
>>> +
>>> + for (t = 0; t < max_slots; t++) {
>>> + input_mt_slot(ts->input, t);
>>> + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, false);
>>> + }
>>> +}
>>> +
>>> +static void cyttsp5_mt_lift_all(struct cyttsp5 *ts)
>>> +{
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> + int max = si->tch_abs[CY_TCH_T].max;
>>> +
>>> + if (ts->num_prv_rec != 0) {
>>> + cyttsp5_report_slot_liftoff(ts, max);
>>> + input_sync(ts->input);
>>> + ts->num_prv_rec = 0;
>>> + }
>>> +}
>>> +
>>> +static void cyttsp5_get_touch_axis(int *axis, int size, int max, u8 *xy_data,
>>> + int bofs)
>>> +{
>>> + int nbyte;
>>> + int next;
>>> +
>>> + for (nbyte = 0, *axis = 0, next = 0; nbyte < size; nbyte++)
>>> + *axis = *axis + ((xy_data[nbyte] >> bofs) << (nbyte * 8));
>>> +
>>> + *axis &= max - 1;
>>> +}
>>> +
>>> +static void cyttsp5_get_touch_record(struct cyttsp5 *ts,
>>> + struct cyttsp5_touch *touch, u8 *xy_data)
>>> +{
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> + enum cyttsp5_tch_abs abs;
>>> +
>>> + for (abs = CY_TCH_X; abs < CY_TCH_NUM_ABS; abs++) {
>>> + cyttsp5_get_touch_axis(&touch->abs[abs],
>>> + si->tch_abs[abs].size,
>>> + si->tch_abs[abs].max,
>>> + xy_data + si->tch_abs[abs].ofs,
>>> + si->tch_abs[abs].bofs);
>>> + }
>>> +}
>>> +
>>> +static void cyttsp5_get_mt_touches(struct cyttsp5 *ts,
>>> + struct cyttsp5_touch *tch, int num_cur_tch)
>>> +{
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> + int i, t = 0;
>>> + DECLARE_BITMAP(ids, si->tch_abs[CY_TCH_T].max);
>>> + u8 *tch_addr;
>>> + int tmp;
>>> +
>>> + bitmap_zero(ids, si->tch_abs[CY_TCH_T].max);
>>> + memset(tch->abs, 0, sizeof(tch->abs));
>>> +
>>> + for (i = 0; i < num_cur_tch; i++) {
>>> + tch_addr = si->xy_data + (i * TOUCH_REPORT_SIZE);
>>> + cyttsp5_get_touch_record(ts, tch, tch_addr);
>>> +
>>> + /* Convert MAJOR/MINOR from mm to resolution */
>>> + tmp = tch->abs[CY_TCH_MAJ] * 100 * si->sensing_conf_data.res_x;
>>> + tch->abs[CY_TCH_MAJ] = tmp / si->sensing_conf_data.len_x;
>>> + tmp = tch->abs[CY_TCH_MIN] * 100 * si->sensing_conf_data.res_x;
>>> + tch->abs[CY_TCH_MIN] = tmp / si->sensing_conf_data.len_x;
>>> +
>>> + t = tch->abs[CY_TCH_T];
>>> + input_mt_slot(ts->input, t);
>>> + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, true);
>>> + __set_bit(t, ids);
>>> +
>>> + /* position and pressure fields */
>>> + input_report_abs(ts->input, ABS_MT_POSITION_X,
>>> + tch->abs[CY_TCH_X]);
>>> + input_report_abs(ts->input, ABS_MT_POSITION_Y,
>>> + tch->abs[CY_TCH_Y]);
>>> + input_report_abs(ts->input, ABS_MT_PRESSURE,
>>> + tch->abs[CY_TCH_P]);
>>> +
>>> + /* Get the extended touch fields */
>>> + input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR,
>>> + tch->abs[CY_TCH_MAJ]);
>>> + input_report_abs(ts->input, ABS_MT_TOUCH_MINOR,
>>> + tch->abs[CY_TCH_MIN]);
>>> + }
>>> +
>>> + cyttsp5_final_sync(ts->input, si->tch_abs[CY_TCH_T].max, ids);
>>> +
>>> + ts->num_prv_rec = num_cur_tch;
>>> +}
>>> +
>>> +/* read xy_data for all current touches */
>>> +static int cyttsp5_xy_worker(struct cyttsp5 *ts)
>>> +{
>>> + struct device *dev = ts->dev;
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> + int max_tch = si->sensing_conf_data.max_tch;
>>> + struct cyttsp5_touch tch;
>>> + u8 num_cur_tch;
>>> +
>>> + cyttsp5_get_touch_axis(&tch.hdr, si->tch_hdr.size,
>>> + si->tch_hdr.max,
>>> + si->xy_mode + 3 + si->tch_hdr.ofs,
>>> + si->tch_hdr.bofs);
>>> +
>>> + num_cur_tch = tch.hdr;
>>> + if (num_cur_tch > max_tch) {
>>> + dev_err(dev, "Num touch err detected (n=%d)\n", num_cur_tch);
>>> + num_cur_tch = max_tch;
>>> + }
>>> +
>>> + if (num_cur_tch == 0 && ts->num_prv_rec == 0)
>>> + return 0;
>>> +
>>> + /* extract xy_data for all currently reported touches */
>>> + if (num_cur_tch)
>>> + cyttsp5_get_mt_touches(ts, &tch, num_cur_tch);
>>> + else
>>> + cyttsp5_mt_lift_all(ts);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int cyttsp5_mt_attention(struct device *dev)
>>> +{
>>> + struct cyttsp5 *ts = dev_get_drvdata(dev);
>>
>> Why don't you pass ts into cyttsp5_mt_attention?
>
> Yep, you are right, I will update it.
>
>>
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> + int rc;
>>> +
>>> + if (si->xy_mode[2] != HID_TOUCH_REPORT_ID)
>>> + return 0;
>>> +
>>> + /* core handles handshake */
>>> + rc = cyttsp5_xy_worker(ts);
>>> + if (rc < 0)
>>> + dev_err(dev, "xy_worker error r=%d\n", rc);
>>> +
>>> + return rc;
>>> +}
>>> +
>>> +static int cyttsp5_setup_input_device(struct device *dev)
>>> +{
>>> + struct cyttsp5 *ts = dev_get_drvdata(dev);
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> + int max_x, max_y, max_p;
>>> + int max_x_tmp, max_y_tmp;
>>> + int rc;
>>> +
>>> + __set_bit(EV_ABS, ts->input->evbit);
>>> + __set_bit(EV_REL, ts->input->evbit);
>>> + __set_bit(EV_KEY, ts->input->evbit);
>>> +
>>> + max_x_tmp = si->sensing_conf_data.res_x;
>>> + max_y_tmp = si->sensing_conf_data.res_y;
>>> + max_x = max_y_tmp - 1;
>>> + max_y = max_x_tmp - 1;
>>> + max_p = si->sensing_conf_data.max_z;
>>> +
>>> + input_mt_init_slots(ts->input, si->tch_abs[CY_TCH_T].max, 0);
>>
>> Error handling.
>
> Thanks.
>
>>
>>> +
>>> + __set_bit(ABS_MT_POSITION_X, ts->input->absbit);
>>> + __set_bit(ABS_MT_POSITION_Y, ts->input->absbit);
>>> + __set_bit(ABS_MT_PRESSURE, ts->input->absbit);
>>> +
>>> + input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, max_x, 0, 0);
>>> + input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, max_y, 0, 0);
>>> + input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, max_p, 0, 0);
>>> +
>>> + input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
>>> + input_set_abs_params(ts->input, ABS_MT_TOUCH_MINOR, 0, MAX_AREA, 0, 0);
>>
>> You might want to use the standard DT touchscreen properties and
>> reporting functions to support axis inversion and swapping of axes.
>
> Hm, I am not sure to see what you are referring to.
> Could you explain me what you think of?
>
>>
>>> +
>>> + rc = input_register_device(ts->input);
>>> + if (rc < 0)
>>> + dev_err(dev, "Error, failed register input device r=%d\n", rc);
>>> +
>>> + return rc;
>>> +}
>>> +
>>> +#ifdef CONFIG_OF
>>> +static int cyttsp5_parse_dt_key_code(struct device *dev)
>>> +{
>>> + struct cyttsp5 *ts = dev_get_drvdata(dev);
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> + struct device_node *np;
>>> + int i;
>>> +
>>> + np = dev->of_node;
>>> + if (!np)
>>> + return -EINVAL;
>>> +
>>> + if (!si->num_btns)
>>> + return 0;
>>> +
>>> + /* Initialize the button to RESERVED */
>>> + for (i = 0; i < si->num_btns; i++)
>>> + si->key_code[i] = KEY_RESERVED;
>>> +
>>> + return of_property_read_u32_array(np, "linux,keycodes",
>>> + si->key_code, si->num_btns);
>>> +}
>>> +#else
>>> +static int cyttsp5_parse_dt_key_code(struct device *dev)
>>> +{
>>> + struct cyttsp5 *ts = dev_get_drvdata(dev);
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> + int i;
>>> +
>>> + if (!si->num_btns)
>>> + return 0;
>>> +
>>> + /* Initialize the button to RESERVED */
>>> + for (i = 0; i < si->num_btns; i++)
>>> + si->key_code[i] = KEY_RESERVED;
>>> +
>>> + return 0;
>>> +}
>>> +#endif
>>> +
>>> +static int cyttsp5_btn_attention(struct device *dev)
>>> +{
>>> + struct cyttsp5 *ts = dev_get_drvdata(dev);
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> + int cur_btn;
>>> + int cur_btn_state;
>>> +
>>> + if (si->xy_mode[2] != HID_BTN_REPORT_ID || !si->num_btns)
>>> + return 0;
>>> +
>>> + /* extract button press/release touch information */
>>> + for (cur_btn = 0; cur_btn < si->num_btns; cur_btn++) {
>>> + /* Get current button state */
>>> + cur_btn_state = (si->xy_data[0] >> (cur_btn * CY_BITS_PER_BTN))
>>> + & CY_NUM_BTN_EVENT_ID;
>>> +
>>> + input_report_key(ts->input, si->key_code[cur_btn],
>>> + cur_btn_state);
>>> + input_sync(ts->input);
>>
>> You do not need to sen sync after each button, send it once after all
>> buttons sent.
>
> Oh, yes, I will do that!
>
>>
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static u16 cyttsp5_compute_crc(u8 *buf, u32 size)
>>> +{
>>> + u16 remainder = 0xFFFF;
>>> + u16 xor_mask = 0x0000;
>>> + u32 index;
>>> + u32 byte_value;
>>> + u32 table_index;
>>> + u32 crc_bit_width = sizeof(u16) * 8;
>>> +
>>> + /* Divide the message by polynomial, via the table. */
>>> + for (index = 0; index < size; index++) {
>>> + byte_value = buf[index];
>>> + table_index = ((byte_value >> 4) & 0x0F)
>>> + ^ (remainder >> (crc_bit_width - 4));
>>> + remainder = crc_itu_t_table[table_index]
>>> + ^ (remainder << 4);
>>> + table_index = (byte_value & 0x0F)
>>> + ^ (remainder >> (crc_bit_width - 4));
>>> + remainder = crc_itu_t_table[table_index]
>>> + ^ (remainder << 4);
>>> + }
>>> +
>>> + /* Perform the final remainder CRC. */
>>> + return remainder ^ xor_mask;
>>> +}
>>> +
>>> +static int cyttsp5_validate_cmd_response(struct cyttsp5 *ts, u8 code)
>>> +{
>>> + u16 size, crc;
>>> + u8 status, offset;
>>> + int command_code;
>>> +
>>> + size = get_unaligned_le16(&ts->response_buf[0]);
>>> +
>>> + if (!size)
>>> + return 0;
>>> +
>>> + offset = ts->response_buf[HID_OUTPUT_RESPONSE_REPORT_OFFSET];
>>> +
>>> + if (offset == HID_BL_RESPONSE_REPORT_ID) {
>>> + if (ts->response_buf[4] != HID_OUTPUT_BL_SOP) {
>>> + dev_err(ts->dev, "HID output response, wrong SOP\n");
>>> + return -EPROTO;
>>> + }
>>> +
>>> + if (ts->response_buf[size - 1] != HID_OUTPUT_BL_EOP) {
>>> + dev_err(ts->dev, "HID output response, wrong EOP\n");
>>> + return -EPROTO;
>>> + }
>>> +
>>> + crc = cyttsp5_compute_crc(&ts->response_buf[4], size - 7);
>>> + if (ts->response_buf[size - 3] != LOW_BYTE(crc) ||
>>> + ts->response_buf[size - 2] != HI_BYTE(crc)) {
>>> + dev_err(ts->dev, "HID output response, wrong CRC 0x%X\n",
>>> + crc);
>>> + return -EPROTO;
>>> + }
>>> +
>>> + status = ts->response_buf[5];
>>> + if (status) {
>>> + dev_err(ts->dev, "HID output response, ERROR:%d\n",
>>> + status);
>>> + return -EPROTO;
>>> + }
>>> + }
>>> +
>>> + if (offset == HID_APP_RESPONSE_REPORT_ID) {
>>> + command_code = ts->response_buf[HID_OUTPUT_RESPONSE_CMD_OFFSET]
>>> + & HID_OUTPUT_RESPONSE_CMD_MASK;
>>> + if (command_code != code) {
>>> + dev_err(ts->dev,
>>> + "HID output response, wrong command_code:%X\n",
>>> + command_code);
>>> + return -EPROTO;
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void cyttsp5_si_get_btn_data(struct cyttsp5 *ts)
>>> +{
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> + int i;
>>> + unsigned int btns = ts->response_buf[HID_SYSINFO_BTN_OFFSET]
>>> + & HID_SYSINFO_BTN_MASK;
>>> +
>>> + si->num_btns = 0;
>>> + for (i = 0; i < HID_SYSINFO_MAX_BTN; i++) {
>>> + if (btns & BIT(i))
>>> + si->num_btns++;
>>> + }
>>
>> si->num_btns = hweight8(ts->response_buf[HID_SYSINFO_BTN_OFFSET]);
>
> Great, I will try that!
>
>>
>>> +}
>>> +
>>> +static int cyttsp5_get_sysinfo_regs(struct cyttsp5 *ts)
>>> +{
>>> + struct cyttsp5_sensing_conf_data *scd = &ts->sysinfo.sensing_conf_data;
>>> + struct cyttsp5_sensing_conf_data_dev *scd_dev =
>>> + (struct cyttsp5_sensing_conf_data_dev *)
>>> + &ts->response_buf[HID_SYSINFO_SENSING_OFFSET];
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> +
>>> + cyttsp5_si_get_btn_data(ts);
>>> +
>>> + scd->max_tch = scd_dev->max_num_of_tch_per_refresh_cycle;
>>> + scd->res_x = get_unaligned_le16(&scd_dev->res_x);
>>> + scd->res_y = get_unaligned_le16(&scd_dev->res_y);
>>> + scd->max_z = get_unaligned_le16(&scd_dev->max_z);
>>> + scd->len_x = get_unaligned_le16(&scd_dev->len_x);
>>> + scd->len_y = get_unaligned_le16(&scd_dev->len_y);
>>> +
>>> + si->xy_data = devm_kzalloc(ts->dev, scd->max_tch * TOUCH_REPORT_SIZE,
>>> + GFP_KERNEL);
>>> + if (!si->xy_data)
>>> + return -ENOMEM;
>>> +
>>> + si->xy_mode = devm_kzalloc(ts->dev, TOUCH_INPUT_HEADER_SIZE,
>>> + GFP_KERNEL);
>>> + if (!si->xy_mode)
>>> + return -ENOMEM;
>>
>> Why do we need these 2 allocated separately form the driver data?
>
> xy_mode and particularly xy_data are used to retrieve buttons or touch
> data that we received from the input buffer (see move_button_data()
> and move_touch_data()). I guess that I can try to remove them, if you
> want.
>
>>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int cyttsp5_hid_output_get_sysinfo(struct cyttsp5 *ts)
>>> +{
>>> + int rc;
>>> + u8 cmd[HID_OUTPUT_GET_SYSINFO_SIZE];
>>> +
>>> + ts->hid_cmd_state = HID_CMD_BUSY;
>>> +
>>> + /* HI bytes of Output register address */
>>> + cmd[0] = LOW_BYTE(HID_OUTPUT_GET_SYSINFO_SIZE);
>>> + cmd[1] = HI_BYTE(HID_OUTPUT_GET_SYSINFO_SIZE);
>>> + cmd[2] = HID_APP_OUTPUT_REPORT_ID;
>>> + cmd[3] = 0x0; /* Reserved */
>>> + cmd[4] = HID_OUTPUT_GET_SYSINFO;
>>> +
>>> + rc = cyttsp5_write(ts, HID_OUTPUT_REG, cmd,
>>> + HID_OUTPUT_GET_SYSINFO_SIZE);
>>> + if (rc) {
>>> + dev_err(ts->dev, "Failed to write command %d", rc);
>>> + goto error;
>>> + }
>>> +
>>> + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
>>> + msecs_to_jiffies(CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT));
>>> + if (!rc) {
>>> + dev_err(ts->dev, "HID output cmd execution timed out\n");
>>> + rc = -ETIME;
>>> + goto error;
>>> + }
>>> +
>>> + rc = cyttsp5_validate_cmd_response(ts, HID_OUTPUT_GET_SYSINFO);
>>> + if (rc) {
>>> + dev_err(ts->dev, "Validation of the response failed\n");
>>> + goto error;
>>> + }
>>> +
>>> + return cyttsp5_get_sysinfo_regs(ts);
>>> +
>>> +error:
>>> + mutex_lock(&ts->system_lock);
>>> + ts->hid_cmd_state = HID_CMD_DONE;
>>> + mutex_unlock(&ts->system_lock);
>>> + return rc;
>>> +}
>>> +
>>> +static int cyttsp5_hid_output_bl_launch_app(struct cyttsp5 *ts)
>>> +{
>>> + int rc;
>>> + u8 cmd[HID_OUTPUT_BL_LAUNCH_APP];
>>> + u16 crc;
>>> +
>>> + mutex_lock(&ts->system_lock);
>>> + ts->hid_cmd_state = HID_CMD_BUSY;
>>> + mutex_unlock(&ts->system_lock);
>>> +
>>> + cmd[0] = LOW_BYTE(HID_OUTPUT_BL_LAUNCH_APP_SIZE);
>>> + cmd[1] = HI_BYTE(HID_OUTPUT_BL_LAUNCH_APP_SIZE);
>>> + cmd[2] = HID_BL_OUTPUT_REPORT_ID;
>>> + cmd[3] = 0x0; /* Reserved */
>>> + cmd[4] = HID_OUTPUT_BL_SOP;
>>> + cmd[5] = HID_OUTPUT_BL_LAUNCH_APP;
>>> + cmd[6] = 0x0; /* Low bytes of data */
>>> + cmd[7] = 0x0; /* Hi bytes of data */
>>> + crc = cyttsp5_compute_crc(&cmd[4], 4);
>>> + cmd[8] = LOW_BYTE(crc);
>>> + cmd[9] = HI_BYTE(crc);
>>> + cmd[10] = HID_OUTPUT_BL_EOP;
>>> +
>>> + rc = cyttsp5_write(ts, HID_OUTPUT_REG, cmd,
>>> + HID_OUTPUT_BL_LAUNCH_APP_SIZE);
>>> + if (rc) {
>>> + dev_err(ts->dev, "Failed to write command %d", rc);
>>> + goto error;
>>> + }
>>> +
>>> + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
>>> + msecs_to_jiffies(CY_HID_OUTPUT_TIMEOUT));
>>> + if (!rc) {
>>> + dev_err(ts->dev, "HID output cmd execution timed out\n");
>>> + rc = -ETIME;
>>> + goto error;
>>> + }
>>> +
>>> + rc = cyttsp5_validate_cmd_response(ts, HID_OUTPUT_BL_LAUNCH_APP);
>>> + if (rc) {
>>> + dev_err(ts->dev, "Validation of the response failed\n");
>>> + goto error;
>>> + }
>>> +
>>> + return rc;
>>> +
>>> +error:
>>> + mutex_lock(&ts->system_lock);
>>> + ts->hid_cmd_state = HID_CMD_DONE;
>>> + mutex_unlock(&ts->system_lock);
>>> +
>>> + return rc;
>>> +}
>>> +
>>> +static int cyttsp5_get_hid_descriptor(struct cyttsp5 *ts,
>>> + struct cyttsp5_hid_desc *desc)
>>> +{
>>> + struct device *dev = ts->dev;
>>> + __le16 hid_desc_register = HID_DESC_REG;
>>> + int rc;
>>> + u8 cmd[2];
>>> +
>>> + /* Read HID descriptor length and version */
>>> + mutex_lock(&ts->system_lock);
>>> + ts->hid_cmd_state = HID_CMD_BUSY;
>>> + mutex_unlock(&ts->system_lock);
>>> +
>>> + /* Set HID descriptor register */
>>> + memcpy(cmd, &hid_desc_register, sizeof(hid_desc_register));
>>> +
>>> + rc = cyttsp5_write(ts, HID_DESC_REG, NULL, 0);
>>> + if (rc < 0) {
>>> + dev_err(dev, "Failed to get HID descriptor, rc=%d\n", rc);
>>> + goto error;
>>> + }
>>> +
>>> + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
>>> + msecs_to_jiffies(CY_HID_GET_HID_DESCRIPTOR_TIMEOUT));
>>> + if (!rc) {
>>> + dev_err(ts->dev, "HID get descriptor timed out\n");
>>> + rc = -ETIME;
>>> + goto error;
>>> + }
>>> +
>>> + memcpy(desc, ts->response_buf, sizeof(struct cyttsp5_hid_desc));
>>> +
>>> + /* Check HID descriptor length and version */
>>> + if (le16_to_cpu(desc->hid_desc_len) != sizeof(*desc) ||
>>> + le16_to_cpu(desc->bcd_version) != HID_VERSION) {
>>> + dev_err(dev, "Unsupported HID version\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + goto exit;
>>> +
>>> +error:
>>> + mutex_lock(&ts->system_lock);
>>> + ts->hid_cmd_state = HID_CMD_DONE;
>>> + mutex_unlock(&ts->system_lock);
>>> +exit:
>>> + return rc;
>>> +}
>>> +
>>> +static int fill_tch_abs(struct cyttsp5_tch_abs_params *tch_abs, int report_size,
>>> + int offset)
>>> +{
>>> + tch_abs->ofs = offset / 8;
>>> + tch_abs->size = report_size / 8;
>>> + if (report_size % 8)
>>> + tch_abs->size += 1;
>>> + tch_abs->min = 0;
>>> + tch_abs->max = 1 << report_size;
>>> + tch_abs->bofs = offset - (tch_abs->ofs << 3);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int move_button_data(struct cyttsp5 *ts, struct cyttsp5_sysinfo *si)
>>> +{
>>> + memcpy(si->xy_mode, ts->input_buf, BTN_INPUT_HEADER_SIZE);
>>> + memcpy(si->xy_data, &ts->input_buf[BTN_INPUT_HEADER_SIZE],
>>> + BTN_REPORT_SIZE);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int move_touch_data(struct cyttsp5 *ts, struct cyttsp5_sysinfo *si)
>>> +{
>>> + int max_tch = si->sensing_conf_data.max_tch;
>>> + int num_cur_tch;
>>> + int length;
>>> + struct cyttsp5_tch_abs_params *tch = &si->tch_hdr;
>>> +
>>> + memcpy(si->xy_mode, ts->input_buf, TOUCH_INPUT_HEADER_SIZE);
>>> +
>>> + cyttsp5_get_touch_axis(&num_cur_tch, tch->size,
>>> + tch->max, si->xy_mode + 3 + tch->ofs, tch->bofs);
>>> + if (unlikely(num_cur_tch > max_tch))
>>> + num_cur_tch = max_tch;
>>> +
>>> + length = num_cur_tch * TOUCH_REPORT_SIZE;
>>> +
>>> + memcpy(si->xy_data, &ts->input_buf[TOUCH_INPUT_HEADER_SIZE], length);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static irqreturn_t cyttsp5_handle_irq(int irq, void *handle)
>>> +{
>>> + struct cyttsp5 *ts = handle;
>>> + int report_id;
>>> + int size;
>>> + int rc;
>>> +
>>> + rc = cyttsp5_read(ts, ts->input_buf, CY_MAX_INPUT);
>>> + if (rc)
>>> + return IRQ_HANDLED;
>>> +
>>> + size = get_unaligned_le16(&ts->input_buf[0]);
>>> +
>>> + /* check reset */
>>> + if (size == 0) {
>>> + memcpy(ts->response_buf, ts->input_buf, 2);
>>> +
>>> + mutex_lock(&ts->system_lock);
>>> + ts->hid_cmd_state = HID_CMD_DONE;
>>> + mutex_unlock(&ts->system_lock);
>>> + wake_up(&ts->wait_q);
>>
>> I'd use spinlock in wait queue and wake_up_locked() instead of a
>> separate mutex.
>
> Hum, okay, I will try that.
>
>>
>>> + return IRQ_HANDLED;
>>> + }
>>> +
>>> + report_id = ts->input_buf[2];
>>> +
>>
>> switch(report_id) {
>> }
>
> Yep, it will be easier to understand.
>
>>
>>> + if (report_id == HID_TOUCH_REPORT_ID) {
>>> + move_touch_data(ts, &ts->sysinfo);
>>> + cyttsp5_mt_attention(ts->dev);
>>> + } else if (report_id == HID_BTN_REPORT_ID) {
>>> + move_button_data(ts, &ts->sysinfo);
>>> + cyttsp5_btn_attention(ts->dev);
>>> + } else {
>>> + /* It is not an input but a command response */
>>> + memcpy(ts->response_buf, ts->input_buf, size);
>>> +
>>> + mutex_lock(&ts->system_lock);
>>> + ts->hid_cmd_state = HID_CMD_DONE;
>>> + mutex_unlock(&ts->system_lock);
>>> + wake_up(&ts->wait_q);
>>> + }
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int cyttsp5_deassert_int(struct cyttsp5 *ts)
>>> +{
>>> + u16 size;
>>> + u8 buf[2];
>>> + int rc;
>>> +
>>> + rc = regmap_bulk_read(ts->regmap, HID_INPUT_REG, buf, 2);
>>> + if (rc < 0)
>>> + return rc;
>>> +
>>> + size = get_unaligned_le16(&buf[0]);
>>> + if (size == 2 || size == 0)
>>> + return 0;
>>> +
>>> + return -EINVAL;
>>> +}
>>> +
>>> +static int cyttsp5_fill_all_touch(struct cyttsp5 *ts)
>>> +{
>>> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
>>> +
>>> + fill_tch_abs(&si->tch_abs[CY_TCH_X], REPORT_SIZE_16,
>>> + TOUCH_REPORT_DESC_X);
>>> + fill_tch_abs(&si->tch_abs[CY_TCH_Y], REPORT_SIZE_16,
>>> + TOUCH_REPORT_DESC_Y);
>>> + fill_tch_abs(&si->tch_abs[CY_TCH_P], REPORT_SIZE_8,
>>> + TOUCH_REPORT_DESC_P);
>>> + fill_tch_abs(&si->tch_abs[CY_TCH_T], REPORT_SIZE_5,
>>> + TOUCH_REPORT_DESC_CONTACTID);
>>> + fill_tch_abs(&si->tch_hdr, REPORT_SIZE_5,
>>> + TOUCH_REPORT_DESC_HDR_CONTACTCOUNT);
>>> + fill_tch_abs(&si->tch_abs[CY_TCH_MAJ], REPORT_SIZE_8,
>>> + TOUCH_REPORT_DESC_MAJ);
>>> + fill_tch_abs(&si->tch_abs[CY_TCH_MIN], REPORT_SIZE_8,
>>> + TOUCH_REPORT_DESC_MIN);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int cyttsp5_startup(struct cyttsp5 *ts)
>>> +{
>>> + int rc;
>>> +
>>> + rc = cyttsp5_deassert_int(ts);
>>> + if (rc) {
>>> + dev_err(ts->dev, "Error on deassert int r=%d\n", rc);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + /*
>>> + * Launch the application as the device starts in bootloader mode
>>> + * because of a power-on-reset
>>> + */
>>> + rc = cyttsp5_hid_output_bl_launch_app(ts);
>>> + if (rc < 0) {
>>> + dev_err(ts->dev, "Error on launch app r=%d\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + rc = cyttsp5_get_hid_descriptor(ts, &ts->hid_desc);
>>> + if (rc < 0) {
>>> + dev_err(ts->dev, "Error on getting HID descriptor r=%d\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + rc = cyttsp5_fill_all_touch(ts);
>>> + if (rc < 0) {
>>> + dev_err(ts->dev, "Error on report descriptor r=%d\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + rc = cyttsp5_hid_output_get_sysinfo(ts);
>>> + if (rc) {
>>> + dev_err(ts->dev, "Error on getting sysinfo r=%d\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + return rc;
>>> +}
>>> +
>>> +#ifdef CONFIG_OF
>>> +static const struct of_device_id cyttsp5_of_match[] = {
>>> + { .compatible = "cypress,cyttsp5", },
>>> + { }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, cyttsp5_of_match);
>>> +#endif
>>> +
>>> +static const struct i2c_device_id cyttsp5_i2c_id[] = {
>>> + { CYTTSP5_NAME, 0, },
>>> + { }
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, cyttsp5_i2c_id);
>>> +
>>> +static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
>>> + const char *name)
>>> +{
>>> + struct cyttsp5 *ts;
>>> + struct cyttsp5_sysinfo *si;
>>> + int rc = 0, i;
>>> +
>>> + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
>>> + if (!ts)
>>> + return -ENOMEM;
>>> +
>>> + /* Initialize device info */
>>> + ts->regmap = regmap;
>>> + ts->dev = dev;
>>> + si = &ts->sysinfo;
>>> + dev_set_drvdata(dev, ts);
>>> +
>>> + /* Initialize mutexes and spinlocks */
>>> + mutex_init(&ts->system_lock);
>>> +
>>> + /* Initialize wait queue */
>>> + init_waitqueue_head(&ts->wait_q);
>>> +
>>> + /* Reset the gpio to be in a reset state */
>>> + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
>>> + if (IS_ERR(ts->reset_gpio)) {
>>> + rc = PTR_ERR(ts->reset_gpio);
>>> + dev_err(dev, "Failed to request reset gpio, error %d\n", rc);
>>> + return rc;
>>> + }
>>> + gpiod_set_value(ts->reset_gpio, 1);
>>
>> Why not request it as GPIOD_OUT_HIGH if you going to activate it
>> immediately? However it sounds as if you have a reset line that is
>> active low and you want to driver it low to reset, and then release. In
>> this case you need to do
>>
>> ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
>> <delay> - chip usually needs to be some time in reset state
>> gpiod_set_value(ts->reset_gpio, 0);
>>
>> and make sure that correct polarity is specified in device tree.
>
> Okay, I will update it too.
>
>>
>>> +
>>> + /* Need a delay to have device up */
>>> + msleep(20);
>>> +
>>> + rc = devm_request_threaded_irq(dev, irq, NULL, cyttsp5_handle_irq,
>>> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
>>
>> Just use IRQF_ONESHOT and make sure DTS specifies interrupt properly.
>> BTW,I'd recommend using level, not edge interrupts. It is so easy to
>> miss an edge.
>
> Okay, thank you for the advice.
>
>>
>>> + name, ts);
>>> + if (rc) {
>>> + dev_err(dev, "unable to request IRQ\n");
>>> + return rc;
>>> + }
>>> +
>>> + rc = cyttsp5_startup(ts);
>>> + if (rc) {
>>> + dev_err(ts->dev, "Fail initial startup r=%d\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + rc = cyttsp5_parse_dt_key_code(dev);
>>> + if (rc < 0) {
>>> + dev_err(ts->dev, "Error while parsing dts %d\n", rc);
>>> + return rc;
>>> + }
>>> +
>>> + ts->input = devm_input_allocate_device(dev);
>>> + if (!ts->input) {
>>> + dev_err(dev, "Error, failed to allocate input device\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + ts->input->name = "cyttsp5";
>>> + scnprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
>>> + ts->input->phys = ts->phys;
>>> + ts->input->dev.parent = ts->dev;
>>> + input_set_drvdata(ts->input, ts);
>>> +
>>> + touchscreen_parse_properties(ts->input, true, NULL);
>>> +
>>> + __set_bit(EV_KEY, ts->input->evbit);
>>> + for (i = 0; i < si->num_btns; i++)
>>> + __set_bit(si->key_code[i], ts->input->keybit);
>>> +
>>> + return cyttsp5_setup_input_device(dev);
>>> +}
>>> +
>>> +static int cyttsp5_i2c_probe(struct i2c_client *client,
>>> + const struct i2c_device_id *id)
>>> +{
>>> + struct regmap *regmap;
>>> + static const struct regmap_config config = {
>>> + .reg_bits = 8,
>>> + .val_bits = 8,
>>> + };
>>> +
>>> + regmap = devm_regmap_init_i2c(client, &config);
>>> + if (IS_ERR(regmap)) {
>>> + dev_err(&client->dev, "regmap allocation failed: %ld\n",
>>> + PTR_ERR(regmap));
>>> + return PTR_ERR(regmap);
>>> + }
>>> +
>>> + return cyttsp5_probe(&client->dev, regmap, client->irq, client->name);
>>> +}
>>> +
>>> +static int cyttsp5_remove(struct device *dev)
>>> +{
>>> + struct cyttsp5 *ts = dev_get_drvdata(dev);
>>> +
>>> + input_unregister_device(ts->input);
>>
>> Not needed: you are using managed input device.
>
> You are right, thanks.
>
>>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int cyttsp5_i2c_remove(struct i2c_client *client)
>>> +{
>>> + struct device *dev = &client->dev;
>>> +
>>> + return cyttsp5_remove(dev);
>>
>> Not needed if you remove cyttsp5_remove().
>
> Sure.
>
>>
>>> +}
>>> +
>>> +static struct i2c_driver cyttsp5_i2c_driver = {
>>> + .driver = {
>>> + .name = CYTTSP5_NAME,
>>> + .owner = THIS_MODULE,
>>> + .of_match_table = of_match_ptr(cyttsp5_of_match),
>>> + },
>>> + .probe = cyttsp5_i2c_probe,
>>> + .remove = cyttsp5_i2c_remove,
>>> + .id_table = cyttsp5_i2c_id,
>>> +};
>>> +
>>> +module_i2c_driver(cyttsp5_i2c_driver);
>>> +
>>> +MODULE_LICENSE("GPL");
>>> +MODULE_DESCRIPTION("Touchscreen driver for Cypress TrueTouch Gen 5 Product");
>>> +MODULE_AUTHOR("Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
>>> --
>>> 2.11.0
>>>
>>
>> Thanks.
>>
>
> Best regards,
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] Input: mms114 - drop platform data and use generic APIs
From: Andi Shyti @ 2018-01-23 11:15 UTC (permalink / raw)
To: Simon Shields
Cc: Dmitry Torokhov, linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring
In-Reply-To: <20180116085206.GA18232-WP75azK+jQYgsBAKwltoeQ@public.gmane.org>
Hi Simon,
sorry for the late answer,
On Tue, Jan 16, 2018 at 07:52:06PM +1100, Simon Shields wrote:
> Hi Andi,
>
> Thanks for the review!
>
> On Tue, Jan 16, 2018 at 04:56:11PM +0900, Andi Shyti wrote:
> > Hi Simon,
> >
> > On Sat, Jan 13, 2018 at 01:04:56PM +1100, Simon Shields wrote:
> > > The MMS114 platform data has no in-tree users, so drop it,
> > > and make the driver depend on CONFIG_OF.
> > >
> > > Switch to using the standard touchscreen properties via
> > > touchscreen_parse_properties(), and move the old DT parsing code
> > > to use device_property_*() APIs.
> > >
> > > Finally, use touchscreen_report_pos to report x/y coordinates
> > > and drop the custom x/y inversion code.
> > >
> > > Signed-off-by: Simon Shields <simon-WP75azK+jQYgsBAKwltoeQ@public.gmane.org>
> > > ---
> > > .../bindings/input/touchscreen/mms114.txt | 29 ++--
> > > drivers/input/touchscreen/Kconfig | 1 +
> > > drivers/input/touchscreen/mms114.c | 152 +++++++++------------
> > > include/linux/platform_data/mms114.h | 24 ----
> > > 4 files changed, 83 insertions(+), 123 deletions(-)
> > > delete mode 100644 include/linux/platform_data/mms114.h
> > >
> >
> > The patch looks good, but you would also need to update the dtsi
> > files in this same patch:
> >
> > ./arch/arm/boot/dts/exynos4412-trats2.dts
> > ./arch/arm/boot/dts/exynos4210-trats.dts
> >
> > and Cc the Samsung-soc mailing list.
> >
> > For now it's a nack because the touchscreen would not work
> > anymore with the trats boards.
>
> This patch keeps support for the old bindings. I've verified that both
> the old and new bindings work on a GT-I9300 (trats2 with a different
> bootloader/partition layout).
Oh, yes, I read the patch partially, I didn't see that you
actually kept the back compatibility by doing this:
> + if (mms114_parse_dt(data) < 0) {
> + /* No valid legacy binding found, try the common one */
> + touchscreen_parse_properties(input_dev, true, &data->props);
[...]
Sorry for the confusion, please add:
Reviewed-by: Andi Shyti <andi.shyti-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Tested-by: Andi Shyti <andi.shyti-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
And for reference to Dmitry, there is also Rob's ack.
Andi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] Add touchscreen platform data for the Teclast X3 Plus tablet.
From: Alberto Ponces @ 2018-01-23 11:18 UTC (permalink / raw)
To: linux-input
Cc: ponces26, Hans de Goede, Darren Hart, Andy Shevchenko,
platform-driver-x86, linux-kernel
Add touchscreen platform data for the Teclast X3 Plus tablet.
Signed-off-by: Alberto Ponces <ponces26@gmail.com>
---
drivers/platform/x86/silead_dmi.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/platform/x86/silead_dmi.c b/drivers/platform/x86/silead_dmi.c
index 266535c..d4c1190 100644
--- a/drivers/platform/x86/silead_dmi.c
+++ b/drivers/platform/x86/silead_dmi.c
@@ -171,6 +171,20 @@ static const struct silead_ts_dmi_data digma_citi_e200_data = {
.properties = digma_citi_e200_props,
};
+static const struct property_entry teclast_x3_plus_props[] = {
+ PROPERTY_ENTRY_U32("touchscreen-size-x", 1980),
+ PROPERTY_ENTRY_U32("touchscreen-size-y", 1500),
+ PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-teclast-x3-plus.fw"),
+ PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
+ { }
+};
+
+static const struct silead_ts_dmi_data teclast_x3_plus_data = {
+ .acpi_name = "MSSL1680:00",
+ .properties = teclast_x3_plus_props,
+};
+
static const struct dmi_system_id silead_ts_dmi_table[] = {
{
/* CUBE iwork8 Air */
@@ -271,6 +285,15 @@ static const struct dmi_system_id silead_ts_dmi_table[] = {
DMI_MATCH(DMI_BOARD_NAME, "Cherry Trail CR"),
},
},
+ {
+ /* Teclast X3 Plus */
+ .driver_data = (void *)&teclast_x3_plus_data,
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "TECLAST"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "X3 Plus"),
+ DMI_MATCH(DMI_BOARD_NAME, "X3 Plus"),
+ },
+ },
{ },
};
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] HID: rmi: Support the Fujitsu R726 Pad dock using hid-rmi
From: Jiri Kosina @ 2018-01-23 14:20 UTC (permalink / raw)
To: Andrew Duggan; +Cc: linux-input, linux-kernel, Benjamin Tissoires
In-Reply-To: <1510968090-24764-1-git-send-email-aduggan@synaptics.com>
On Fri, 17 Nov 2017, Andrew Duggan wrote:
> The Fujitsu R726 Pad has an optional USB keyboard dock which contains
> a Synaptics touchpad. The dock identifies itself as a
> Primax Rezel Tablet Keyboard.
>
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: wacom: EKR: ensure devres groups at higher indexes are released
From: Jiri Kosina @ 2018-01-23 14:30 UTC (permalink / raw)
To: Aaron Armstrong Skomra
Cc: linux-input, pinglinux, killertofu, benjamin.tissoires,
stable #4 . 9, Aaron Armstrong Skomra
In-Reply-To: <1512678716-31779-1-git-send-email-skomra@gmail.com>
On Thu, 7 Dec 2017, Aaron Armstrong Skomra wrote:
> Background: ExpressKey Remotes communicate their events via usb dongle.
> Each dongle can hold up to 5 pairings at one time and one EKR (identified
> by its serial number) can unfortunately be paired with its dongle
> more than once. The pairing takes place in a round-robin fashion.
>
> Input devices are only created once per EKR, when a new serial number
> is seen in the list of pairings. However, if a device is created for
> a "higher" paring index and subsequently a second pairing occurs at a
> lower pairing index, unpairing the remote with that serial number from
> any pairing index will currently cause a driver crash. This occurs
> infrequently, as two remotes are necessary to trigger this bug and most
> users have only one remote.
>
> As an illustration, to trigger the bug you need to have two remotes,
> and pair them in this order:
>
> 1. slot 0 -> remote 1 (input device created for remote 1)
> 2. slot 1 -> remote 1 (duplicate pairing - no device created)
> 3. slot 2 -> remote 1 (duplicate pairing - no device created)
> 4. slot 3 -> remote 1 (duplicate pairing - no device created)
> 5. slot 4 -> remote 2 (input device created for remote 2)
>
> 6. slot 0 -> remote 2 (1 destroyed and recreated at slot 1)
> 7. slot 1 -> remote 2 (1 destroyed and recreated at slot 2)
> 8. slot 2 -> remote 2 (1 destroyed and recreated at slot 3)
> 9. slot 3 -> remote 2 (1 destroyed and not recreated)
> 10. slot 4 -> remote 2 (2 was already in this slot so no changes)
>
> 11. slot 0 -> remote 1 (The current code sees remote 2 was paired over in
> one of the dongle slots it occupied and attempts
> to remove all information about remote 2 [1]. It
> calls wacom_remote_destroy_one for remote 2, but
> the destroy function assumes the lowest index is
> where the remote's input device was created. The
> code "cleans up" the other remote 2 pairings
> including the one which the input device was based
> on, assuming they were were just duplicate
> pairings. However, the cleanup doesn't call the
> devres release function for the input device that
> was created in slot 4).
>
> This issue is fixed by this commit.
>
> [1] Remote 2 should subsequently be re-created on the next packet from the
> EKR at the lowest numbered slot that it occupies (here slot 1).
>
> Fixes: f9036bd43602 ("HID: wacom: EKR: use devres groups to manage resources")
> Cc: stable <stable@vger.kernel.org> #4.9
> Signed-off-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v1] HID: sony: Print reversed MAC address via %pMR
From: Jiri Kosina @ 2018-01-23 14:38 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Benjamin Tissoires, linux-input, Roderick Colenbrander
In-Reply-To: <20171211130612.34715-1-andriy.shevchenko@linux.intel.com>
On Mon, 11 Dec 2017, Andy Shevchenko wrote:
> Reversed MAC addresses can be printed directly using %pMR specifier.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Applied, thanks (and sorry for the delay caused by spectral meltdown).
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: sony: Report DS4 version info through sysfs
From: Jiri Kosina @ 2018-01-23 14:39 UTC (permalink / raw)
To: Roderick Colenbrander
Cc: linux-input, Benjamin Tissoires, Roderick Colenbrander
In-Reply-To: <20171219190443.52950-1-roderick@gaikai.com>
On Tue, 19 Dec 2017, Roderick Colenbrander wrote:
> From: Roderick Colenbrander <roderick.colenbrander@sony.com>
>
> Report DS4 firmware and hardware version through sysfs for both
> USB and Bluetooth. This information is important for userspace
> in particular for device specific quirks (e.g. in Bluetooth stacks).
>
> Signed-off-by: Roderick Colenbrander <roderick.colenbrander@sony.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: elecom: rewrite report fixup for EX-G and future mice
From: Jiri Kosina @ 2018-01-23 14:40 UTC (permalink / raw)
To: Tomasz Kramkowski
Cc: Benjamin Tissoires, Yuxuan Shui, Diego Elio Pettenò,
Alex Manoussakis, linux-input, linux-kernel
In-Reply-To: <20171219204436.27737-1-tk@the-tk.com>
On Tue, 19 Dec 2017, Tomasz Kramkowski wrote:
> This patch rewrites the mouse report fixup used for the DEFT and HUGE
> elecom trackballs in order to make it generic enough to fix other
> elecom mice with similar issues. This patch also uses this new report
> fixup function to fix the Elecom EX-G trackball which has 6 physical
> buttons and a similar issue to the other two mice.
>
> Elecom's track record has so far shown that they like to re-use the
> same report descriptor for multiple different mice regardless of the
> number of buttons the mouse has. This means that the missing buttons
> on multiple mice can be fixed in one function without introducing
> phantom buttons which would in turn cause the number of mouse buttons
> to be misreported to userspace.
>
> This patch drops the very verbose report descriptor "diff" comment for
> a more abridged yet hopefully just as informative generic version.
>
> Signed-off-by: Tomasz Kramkowski <tk@the-tk.com>
> ---
> v2 changes:
> * pass rsize directly to mouse_button_fixup
> * add support for wireless EX-G variant
> v1: https://marc.info/?i=20171204205550.2621-1-tk@the-tk.com
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: intel-ish-hid: Enable Cannon Lake and Coffee Lake laptop/desktop
From: Jiri Kosina @ 2018-01-23 14:41 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: linux-input, linux-kernel
In-Reply-To: <20171220192410.87748-1-srinivas.pandruvada@linux.intel.com>
On Wed, 20 Dec 2017, Srinivas Pandruvada wrote:
> Added PCI ID for Cannon Lake and Coffee Lake laptop/desktop skews.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Applied to for-4.16/hid-quirks-cleanup/ish. Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Fix reporting of touch toggle (WACOM_HID_WD_MUTE_DEVICE) events
From: Jiri Kosina @ 2018-01-23 14:42 UTC (permalink / raw)
To: Jason Gerecke
Cc: linux-input, Benjamin Tissoires, Ping Cheng,
Aaron Armstrong Skomra, stable, Jason Gerecke
In-Reply-To: <20171226225355.27785-1-killertofu@gmail.com>
On Tue, 26 Dec 2017, Jason Gerecke wrote:
> Touch toggle softkeys send a '1' while pressed and a '0' while released,
> requring the kernel to keep track of wether touch should be enabled or
> disabled. The code does not handle the state transitions properly,
> however. If the key is pressed repeatedly, the following four states
> of states are cycled through (assuming touch starts out enabled):
>
> Press: shared->is_touch_on => 0, SW_MUTE_DEVICE => 1
> Release: shared->is_touch_on => 0, SW_MUTE_DEVICE => 1
> Press: shared->is_touch_on => 1, SW_MUTE_DEVICE => 0
> Release: shared->is_touch_on => 1, SW_MUTE_DEVICE => 1
>
> The hardware always properly enables/disables touch when the key is
> pressed but applications that listen for SW_MUTE_DEVICE events to provide
> feedback about the state will only ever show touch as being enabled while
> the key is held, and only every-other time. This sequence occurs because
> the fallthrough WACOM_HID_WD_TOUCHONOFF case is always handled, and it
> uses the value of the *local* is_touch_on variable as the value to
> report to userspace. The local value is equal to the shared value when
> the button is pressed, but equal to zero when the button is released.
>
> Reporting the shared value to userspace fixes this problem, but the
> fallthrough case needs to update the shared value in an incompatible
> way (which is why the local variable was introduced in the first place).
> To work around this, we just handle both cases in a single block of code
> and update the shared variable as appropriate.
>
> Fixes: d793ff8187 ("HID: wacom: generic: support touch on/off softkey")
> Cc: <stable@vger.kernel.org> # v4.12+
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> Reviewed-by: Aaron Skomra <aaron.skomra@wacom.com>
> Tested-by: Aaron Skomra <aaron.skomra@wacom.com>
Applied to for-4.16/wacom.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Add support for One by Wacom (CTL-472 / CTL-672)
From: Jiri Kosina @ 2018-01-23 14:44 UTC (permalink / raw)
To: Jason Gerecke
Cc: linux-input, Mx Jing, Ping Cheng, Aaron Skomra, Jason Gerecke
In-Reply-To: <20171226235018.5522-1-killertofu@gmail.com>
On Tue, 26 Dec 2017, Jason Gerecke wrote:
> Adds support for the second-generation "One by Wacom" tablets. These
> devices are similar to the last generation, but a slightly different size
> and reporting a higher number of pressure levels.
Please don't forget to put proper prefixes in the patch title ("HID:
wacom", as usual) next time.
Applied to for-4.16/wacom, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 1/2] HID: asus: Add touchpad max x/y and resolution info for the T200TA
From: Jiri Kosina @ 2018-01-23 14:45 UTC (permalink / raw)
To: Hans de Goede; +Cc: Benjamin Tissoires, linux-input
In-Reply-To: <20180105110919.30399-1-hdegoede@redhat.com>
On Fri, 5 Jan 2018, Hans de Goede wrote:
> The Asus T200TA uses the same USB device-id for its keyboard dock as the
> T100TA, but the touchpad has a different size and corresponding different
> max x/y values.
>
> Add a separate asus_touchpad_info struct for the T200TA and select this
> based on the DMI product-name (as we are already doing for the T100HA),
> so that we report the correct info to userspace.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Unhappily applied both to for-4.16/hid-quirks-cleanup/asus. Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: roccat: prevent an out of bounds read in kovaplus_profile_activated()
From: Jiri Kosina @ 2018-01-23 14:47 UTC (permalink / raw)
To: Dan Carpenter
Cc: Stefan Achatz, Benjamin Tissoires, linux-input, kernel-janitors
In-Reply-To: <20180110093903.yyoknoi7oc2cd5iy@mwanda>
On Wed, 10 Jan 2018, Dan Carpenter wrote:
> We get the "new_profile_index" value from the mouse device when we're
> handling raw events. Smatch taints it as untrusted data and complains
> that we need a bounds check. This seems like a reasonable warning
> otherwise there is a small read beyond the end of the array.
>
> Fixes: 0e70f97f257e ("HID: roccat: Add support for Kova[+] mouse")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: quirks: Fix keyboard + touchpad on Toshiba Click Mini not working
From: Jiri Kosina @ 2018-01-23 14:48 UTC (permalink / raw)
To: Hans de Goede; +Cc: Benjamin Tissoires, linux-input, stable
In-Reply-To: <20180117200555.22990-1-hdegoede@redhat.com>
On Wed, 17 Jan 2018, Hans de Goede wrote:
> The Toshiba Click Mini uses an i2c attached keyboard/touchpad combo
> (single i2c_hid device for both) which has a vid:pid of 04F3:0401,
> which is also used by a bunch of Elan touchpads which are handled by the
> drivers/input/mouse/elan_i2c driver, but that driver deals with pure
> touchpads and does not work for a combo device such as the one on the
> Toshiba Click Mini.
>
> The combo on the Mini has an ACPI id of ELAN0800, which is not claimed
> by the elan_i2c driver, so check for that and if it is found do not ignore
> the device. This fixes the keyboard/touchpad combo on the Mini not working
> (although with the touchpad in mouse emulation mode).
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Add touchscreen platform data for the Teclast X3 Plus tablet.
From: Hans de Goede @ 2018-01-23 14:50 UTC (permalink / raw)
To: Alberto Ponces, linux-input
Cc: Darren Hart, Andy Shevchenko, platform-driver-x86, linux-kernel
In-Reply-To: <1516706288-177-1-git-send-email-ponces26@gmail.com>
Hi,
Nitpick, please change the patch subject to:
platform/x86: silead_dmi: Add touchscreen platform data for the Teclast X3 Plus tablet
On 23-01-18 12:18, Alberto Ponces wrote:
> Add touchscreen platform data for the Teclast X3 Plus tablet. > > Signed-off-by: Alberto Ponces <ponces26@gmail.com>
Otherwise looks good to me, so with the subject fixed this is:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
But you should rebase this on top of:
http://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git/shortlog/refs/heads/testing
So that it can be applied cleanly by the platform/x86 maintainers.
Please send a rebased v2 with the fixed subject, you can add
me Reviewed-by to the v2 before sending it.
Regards,
Hans
> ---
> drivers/platform/x86/silead_dmi.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/platform/x86/silead_dmi.c b/drivers/platform/x86/silead_dmi.c
> index 266535c..d4c1190 100644
> --- a/drivers/platform/x86/silead_dmi.c
> +++ b/drivers/platform/x86/silead_dmi.c
> @@ -171,6 +171,20 @@ static const struct silead_ts_dmi_data digma_citi_e200_data = {
> .properties = digma_citi_e200_props,
> };
>
> +static const struct property_entry teclast_x3_plus_props[] = {
> + PROPERTY_ENTRY_U32("touchscreen-size-x", 1980),
> + PROPERTY_ENTRY_U32("touchscreen-size-y", 1500),
> + PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-teclast-x3-plus.fw"),
> + PROPERTY_ENTRY_U32("silead,max-fingers", 10),
> + PROPERTY_ENTRY_BOOL("silead,home-button"),
> + { }
> +};
> +
> +static const struct silead_ts_dmi_data teclast_x3_plus_data = {
> + .acpi_name = "MSSL1680:00",
> + .properties = teclast_x3_plus_props,
> +};
> +
> static const struct dmi_system_id silead_ts_dmi_table[] = {
> {
> /* CUBE iwork8 Air */
> @@ -271,6 +285,15 @@ static const struct dmi_system_id silead_ts_dmi_table[] = {
> DMI_MATCH(DMI_BOARD_NAME, "Cherry Trail CR"),
> },
> },
> + {
> + /* Teclast X3 Plus */
> + .driver_data = (void *)&teclast_x3_plus_data,
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "TECLAST"),
> + DMI_MATCH(DMI_PRODUCT_NAME, "X3 Plus"),
> + DMI_MATCH(DMI_BOARD_NAME, "X3 Plus"),
> + },
> + },
> { },
> };
>
>
^ permalink raw reply
* Re: [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen
From: Dmitry Torokhov @ 2018-01-23 17:30 UTC (permalink / raw)
To: Mylene Josserand
Cc: robh+dt, mark.rutland, linux-kernel, linux-input, devicetree,
thomas.petazzoni, maxime.ripard
In-Reply-To: <20180123114459.07cc3404@dell-desktop.home>
On Tue, Jan 23, 2018 at 11:44:59AM +0100, Mylene Josserand wrote:
> Hi Dmitry,
>
> Thank you for the review.
>
> Le Mon, 22 Jan 2018 20:15:03 -0800,
> Dmitry Torokhov <dmitry.torokhov@gmail.com> a écrit :
>
> > Hi Mylène,
> >
> > On Fri, Dec 01, 2017 at 04:39:56PM +0100, Mylène Josserand wrote:
> > > This is the basic driver for the Cypress TrueTouch Gen5 touchscreen
> > > controllers. This driver supports only the I2C bus but it uses regmap
> > > so SPI support could be added later.
> > > The touchscreen can retrieve some defined zone that are handled as
> > > buttons (according to the hardware). That is why it handles
> > > button and multitouch events.
> > >
> > > Reviewed-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > > ---
> > > drivers/input/touchscreen/Kconfig | 16 +
> > > drivers/input/touchscreen/Makefile | 1 +
> > > drivers/input/touchscreen/cyttsp5.c | 1110 +++++++++++++++++++++++++++++++++++
> > > 3 files changed, 1127 insertions(+)
> > > create mode 100644 drivers/input/touchscreen/cyttsp5.c
> > >
> > > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> > > index 0cfdb7cb610e..28eea6d5f1bb 100644
> > > --- a/drivers/input/touchscreen/Kconfig
> > > +++ b/drivers/input/touchscreen/Kconfig
> > > @@ -238,6 +238,22 @@ config TOUCHSCREEN_CYTTSP4_SPI
> > > To compile this driver as a module, choose M here: the
> > > module will be called cyttsp4_spi.
> > >
> > > +config TOUCHSCREEN_CYTTSP5
> > > + tristate "Cypress TrueTouch Gen5 Touchscreen Driver"
> > > + depends on OF
> >
> > Does it have to be? Please use generic device properties
> > (device_property_* API) and it can be used with ACPI and static board
> > files, of needed.
>
> Okay, I will try with that.
>
> >
> > > + select REGMAP_I2C
> > > + select CRC_ITU_T
> > > + help
> > > + Driver for Parade TrueTouch Standard Product
> > > + Generation 5 touchscreen controllers.
> > > + I2C bus interface support only.
> > > +
> > > + Say Y here if you have a Cypress Gen5 touchscreen.
> > > + If unsure, say N.
> > > +
> > > + To compile this driver as a module, choose M here: the
> > > + module will be called cyttsp5.
> > > +
> > > config TOUCHSCREEN_DA9034
> > > tristate "Touchscreen support for Dialog Semiconductor DA9034"
> > > depends on PMIC_DA903X
> > > diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> > > index d2a2b3b7af27..e7d124901dd9 100644
> > > --- a/drivers/input/touchscreen/Makefile
> > > +++ b/drivers/input/touchscreen/Makefile
> > > @@ -26,6 +26,7 @@ obj-$(CONFIG_TOUCHSCREEN_CYTTSP_SPI) += cyttsp_spi.o
> > > obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_CORE) += cyttsp4_core.o
> > > obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_I2C) += cyttsp4_i2c.o cyttsp_i2c_common.o
> > > obj-$(CONFIG_TOUCHSCREEN_CYTTSP4_SPI) += cyttsp4_spi.o
> > > +obj-$(CONFIG_TOUCHSCREEN_CYTTSP5) += cyttsp5.o
> > > obj-$(CONFIG_TOUCHSCREEN_DA9034) += da9034-ts.o
> > > obj-$(CONFIG_TOUCHSCREEN_DA9052) += da9052_tsi.o
> > > obj-$(CONFIG_TOUCHSCREEN_DYNAPRO) += dynapro.o
> > > diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
> > > new file mode 100644
> > > index 000000000000..a41feea2cd5a
> > > --- /dev/null
> > > +++ b/drivers/input/touchscreen/cyttsp5.c
> > > @@ -0,0 +1,1110 @@
> > > +/*
> > > + * Parade TrueTouch(TM) Standard Product V5 Module.
> > > + * For use with Parade touchscreen controllers.
> > > + *
> > > + * Copyright (C) 2015 Parade Technologies
> > > + * Copyright (C) 2012-2015 Cypress Semiconductor
> > > + * Copyright (C) 2017 Free Electrons
> > > + *
> > > + * Author: Mylène Josserand <mylene.josserand@free-electrons.com>
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU General Public License
> > > + * version 2, and only version 2, as published by the
> > > + * Free Software Foundation.
> > > + *
> > > + * 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 <asm/unaligned.h>
> > > +#include <linux/crc-itu-t.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/device.h>
> > > +#include <linux/gpio.h>
> > > +#include <linux/input/mt.h>
> > > +#include <linux/input/touchscreen.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/regmap.h>
> > > +
> > > +#define CYTTSP5_NAME "cyttsp5"
> > > +#define CY_I2C_DATA_SIZE (2 * 256)
> > > +#define HID_VERSION 0x0100
> > > +#define CY_MAX_INPUT 512
> > > +#define CYTTSP5_PREALLOCATED_CMD_BUFFER 32
> > > +#define CY_BITS_PER_BTN 1
> > > +#define CY_NUM_BTN_EVENT_ID ((1 << CY_BITS_PER_BTN) - 1)
> > > +
> > > +#define MAX_AREA 255
> > > +#define HID_OUTPUT_BL_SOP 0x1
> > > +#define HID_OUTPUT_BL_EOP 0x17
> > > +#define HID_OUTPUT_BL_LAUNCH_APP 0x3B
> > > +#define HID_OUTPUT_BL_LAUNCH_APP_SIZE 11
> > > +#define HID_OUTPUT_GET_SYSINFO 0x2
> > > +#define HID_OUTPUT_GET_SYSINFO_SIZE 5
> > > +
> > > +#define HID_DESC_REG 0x1
> > > +#define HID_INPUT_REG 0x3
> > > +#define HID_OUTPUT_REG 0x4
> > > +
> > > +#define REPORT_ID_TOUCH 0x1
> > > +#define REPORT_ID_BTN 0x3
> > > +#define REPORT_SIZE_5 5
> > > +#define REPORT_SIZE_8 8
> > > +#define REPORT_SIZE_16 16
> > > +
> > > +/* Touch reports offsets */
> > > +/* Header offsets */
> > > +#define TOUCH_REPORT_DESC_HDR_CONTACTCOUNT 16
> > > +/* Record offsets */
> > > +#define TOUCH_REPORT_DESC_CONTACTID 8
> > > +#define TOUCH_REPORT_DESC_X 16
> > > +#define TOUCH_REPORT_DESC_Y 32
> > > +#define TOUCH_REPORT_DESC_P 48
> > > +#define TOUCH_REPORT_DESC_MAJ 56
> > > +#define TOUCH_REPORT_DESC_MIN 64
> > > +
> > > +/* HID */
> > > +#define HID_TOUCH_REPORT_ID 0x1
> > > +#define HID_BTN_REPORT_ID 0x3
> > > +#define HID_APP_RESPONSE_REPORT_ID 0x1F
> > > +#define HID_APP_OUTPUT_REPORT_ID 0x2F
> > > +#define HID_BL_RESPONSE_REPORT_ID 0x30
> > > +#define HID_BL_OUTPUT_REPORT_ID 0x40
> > > +
> > > +#define HID_OUTPUT_RESPONSE_REPORT_OFFSET 2
> > > +#define HID_OUTPUT_RESPONSE_CMD_OFFSET 4
> > > +#define HID_OUTPUT_RESPONSE_CMD_MASK 0x7F
> > > +
> > > +#define HID_SYSINFO_SENSING_OFFSET 33
> > > +#define HID_SYSINFO_BTN_OFFSET 48
> > > +#define HID_SYSINFO_BTN_MASK 0xFF
> > > +#define HID_SYSINFO_MAX_BTN 8
> > > +
> > > +/* Timeout in ms */
> > > +#define CY_HID_OUTPUT_TIMEOUT 200
> > > +#define CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT 3000
> > > +#define CY_HID_GET_HID_DESCRIPTOR_TIMEOUT 4000
> > > +
> > > +/* maximum number of concurrent tracks */
> > > +#define TOUCH_REPORT_SIZE 10
> > > +#define TOUCH_INPUT_HEADER_SIZE 7
> > > +#define BTN_REPORT_SIZE 9
> > > +#define BTN_INPUT_HEADER_SIZE 5
> > > +
> > > +/* All usage pages for Touch Report */
> > > +#define TOUCH_REPORT_USAGE_PG_X 0x00010030
> > > +#define TOUCH_REPORT_USAGE_PG_Y 0x00010031
> > > +#define TOUCH_REPORT_USAGE_PG_P 0x000D0030
> > > +#define TOUCH_REPORT_USAGE_PG_CONTACTID 0x000D0051
> > > +#define TOUCH_REPORT_USAGE_PG_CONTACTCOUNT 0x000D0054
> > > +#define TOUCH_REPORT_USAGE_PG_MAJ 0xFF010062
> > > +#define TOUCH_REPORT_USAGE_PG_MIN 0xFF010063
> > > +#define TOUCH_COL_USAGE_PG 0x000D0022
> > > +
> > > +/* helpers */
> > > +#define HI_BYTE(x) (u8)(((x) >> 8) & 0xFF)
> > > +#define LOW_BYTE(x) (u8)((x) & 0xFF)
> > > +
> > > +/* System Information interface definitions */
> > > +struct cyttsp5_sensing_conf_data_dev {
> > > + u8 electrodes_x;
> > > + u8 electrodes_y;
> > > + __le16 len_x;
> > > + __le16 len_y;
> > > + __le16 res_x;
> > > + __le16 res_y;
> > > + __le16 max_z;
> > > + u8 origin_x;
> > > + u8 origin_y;
> > > + u8 btn;
> > > + u8 scan_mode;
> > > + u8 max_num_of_tch_per_refresh_cycle;
> > > +} __packed;
> > > +
> > > +struct cyttsp5_sensing_conf_data {
> > > + u16 res_x;
> > > + u16 res_y;
> > > + u16 max_z;
> > > + u16 len_x;
> > > + u16 len_y;
> > > + u8 origin_x;
> > > + u8 origin_y;
> > > + u8 max_tch;
> > > +};
> > > +
> > > +enum { HID_CMD_DONE, HID_CMD_BUSY } hid_cmd_state;
> > > +
> > > +enum cyttsp5_tch_abs { /* for ordering within the extracted touch data array */
> > > + CY_TCH_X, /* X */
> > > + CY_TCH_Y, /* Y */
> > > + CY_TCH_P, /* P (Z) */
> > > + CY_TCH_T, /* TOUCH ID */
> > > + CY_TCH_MAJ, /* TOUCH_MAJOR */
> > > + CY_TCH_MIN, /* TOUCH_MINOR */
> > > + CY_TCH_NUM_ABS,
> > > +};
> > > +
> > > +struct cyttsp5_tch_abs_params {
> > > + size_t ofs; /* abs byte offset */
> > > + size_t size; /* size in bits */
> > > + size_t min; /* min value */
> > > + size_t max; /* max value */
> > > + size_t bofs; /* bit offset */
> > > +};
> > > +
> > > +struct cyttsp5_touch {
> > > + int hdr;
> > > + int abs[CY_TCH_NUM_ABS];
> > > +};
> > > +
> > > +struct cyttsp5_sysinfo {
> > > + struct cyttsp5_sensing_conf_data sensing_conf_data;
> > > + int num_btns;
> > > + struct cyttsp5_tch_abs_params tch_hdr;
> > > + struct cyttsp5_tch_abs_params tch_abs[CY_TCH_NUM_ABS];
> > > + u32 key_code[HID_SYSINFO_MAX_BTN];
> > > + u8 *xy_mode;
> > > + u8 *xy_data;
> > > +};
> > > +
> > > +struct cyttsp5_hid_desc {
> > > + __le16 hid_desc_len;
> > > + u8 packet_id;
> > > + u8 reserved_byte;
> > > + __le16 bcd_version;
> > > + __le16 report_desc_len;
> > > + __le16 report_desc_register;
> > > + __le16 input_register;
> > > + __le16 max_input_len;
> > > + __le16 output_register;
> > > + __le16 max_output_len;
> > > + __le16 command_register;
> > > + __le16 data_register;
> > > + __le16 vendor_id;
> > > + __le16 product_id;
> > > + __le16 version_id;
> > > + u8 reserved[4];
> > > +} __packed;
> > > +
> > > +struct cyttsp5 {
> > > + struct device *dev;
> > > + struct mutex system_lock;
> > > + wait_queue_head_t wait_q;
> > > + struct cyttsp5_sysinfo sysinfo;
> > > + int hid_cmd_state;
> > > + struct cyttsp5_hid_desc hid_desc;
> > > + u8 cmd_buf[CYTTSP5_PREALLOCATED_CMD_BUFFER];
> > > + u8 input_buf[CY_MAX_INPUT];
> > > + u8 response_buf[CY_MAX_INPUT];
> > > + struct gpio_desc *reset_gpio;
> > > + struct input_dev *input;
> > > + char phys[NAME_MAX];
> > > + int num_prv_rec;
> > > + struct regmap *regmap;
> > > +};
> > > +
> > > +/*
> > > + * For what understood in the datasheet, the register does not
> > > + * matter. For consistency, used the Input Register address
> > > + * but it does mean anything to the device. The important data
> > > + * to send is the I2C address
> > > + */
> > > +static int cyttsp5_read(struct cyttsp5 *ts, u8 *buf, u32 max)
> > > +{
> > > + int rc;
> > > + u32 size;
> > > + u8 temp[2];
> > > +
> > > + if (!buf)
> > > + return -EINVAL;
> >
> > Is it really possible? This is not a publicly facing API but private
> > driver data, do you ever happen to call it with NULL?
>
> hm, I do not think so but I will check that.
>
> >
> > > +
> > > + /* Read the frame to retrieve the size */
> > > + rc = regmap_bulk_read(ts->regmap, HID_INPUT_REG, temp, 2);
> > > + if (rc < 0)
> > > + return rc;
> >
> > Can we call this kind of variables "error" pelase?
>
> Sure.
>
> >
> > > +
> > > + size = get_unaligned_le16(temp);
> > > + if (!size || size == 2)
> > > + return 0;
> > > +
> > > + if (size > max)
> > > + return -EINVAL;
> > > +
> > > + /* Get the real value */
> > > + return regmap_bulk_read(ts->regmap, HID_INPUT_REG, buf, size);
> > > +}
> > > +
> > > +static int cyttsp5_write(struct cyttsp5 *ts, unsigned int reg, u8 *data,
> > > + size_t size)
> > > +{
> > > + u8 cmd[size + 1];
> > > +
> > > + /* High bytes of register address needed as first byte of cmd */
> > > + cmd[0] = HI_BYTE(reg);
> > > +
> > > + /* Copy the rest of the data */
> > > + if (data)
> > > + memcpy(&cmd[1], data, size);
> > > +
> > > + /* The hardware wants to receive a frame with the address register
> >
> > The multiline comment block should start with "/*" on a separate line
> > please.
>
> I will change that.
>
> >
> > > + * contains in the first two bytes. As the regmap_write function
> > > + * add the register adresse in the frame, we use the LOW_BYTE as
> > > + * first frame byte for the address register and the first
> > > + * data byte is the high register + left of the cmd to send
> > > + */
> > > + return regmap_bulk_write(ts->regmap, LOW_BYTE(reg), cmd, size + 1);
> > > +}
> > > +
> > > +static void cyttsp5_final_sync(struct input_dev *input, int max_slots,
> > > + unsigned long *ids)
> > > +{
> > > + int t;
> > > +
> > > + for (t = 0; t < max_slots; t++) {
> >
> > for_each_set_bit() {
> > }
>
> I did not know this macro, I will try that.
>
> >
> > > + if (test_bit(t, ids))
> > > + continue;
> > > + input_mt_slot(input, t);
> > > + input_mt_report_slot_state(input, MT_TOOL_FINGER, false);
> > > + }
> > > +
> > > + input_sync(input);
> > > +}
> > > +
> > > +static void cyttsp5_report_slot_liftoff(struct cyttsp5 *ts, int max_slots)
> > > +{
> > > + int t;
> > > +
> > > + if (ts->num_prv_rec == 0)
> > > + return;
> >
> > Do you need this check? The caller checks the same condition.
>
> True, I will remove it.
>
> >
> > > +
> > > + for (t = 0; t < max_slots; t++) {
> > > + input_mt_slot(ts->input, t);
> > > + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, false);
> > > + }
> > > +}
> > > +
> > > +static void cyttsp5_mt_lift_all(struct cyttsp5 *ts)
> > > +{
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > + int max = si->tch_abs[CY_TCH_T].max;
> > > +
> > > + if (ts->num_prv_rec != 0) {
> > > + cyttsp5_report_slot_liftoff(ts, max);
> > > + input_sync(ts->input);
> > > + ts->num_prv_rec = 0;
> > > + }
> > > +}
> > > +
> > > +static void cyttsp5_get_touch_axis(int *axis, int size, int max, u8 *xy_data,
> > > + int bofs)
> > > +{
> > > + int nbyte;
> > > + int next;
> > > +
> > > + for (nbyte = 0, *axis = 0, next = 0; nbyte < size; nbyte++)
> > > + *axis = *axis + ((xy_data[nbyte] >> bofs) << (nbyte * 8));
> > > +
> > > + *axis &= max - 1;
> > > +}
> > > +
> > > +static void cyttsp5_get_touch_record(struct cyttsp5 *ts,
> > > + struct cyttsp5_touch *touch, u8 *xy_data)
> > > +{
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > + enum cyttsp5_tch_abs abs;
> > > +
> > > + for (abs = CY_TCH_X; abs < CY_TCH_NUM_ABS; abs++) {
> > > + cyttsp5_get_touch_axis(&touch->abs[abs],
> > > + si->tch_abs[abs].size,
> > > + si->tch_abs[abs].max,
> > > + xy_data + si->tch_abs[abs].ofs,
> > > + si->tch_abs[abs].bofs);
> > > + }
> > > +}
> > > +
> > > +static void cyttsp5_get_mt_touches(struct cyttsp5 *ts,
> > > + struct cyttsp5_touch *tch, int num_cur_tch)
> > > +{
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > + int i, t = 0;
> > > + DECLARE_BITMAP(ids, si->tch_abs[CY_TCH_T].max);
> > > + u8 *tch_addr;
> > > + int tmp;
> > > +
> > > + bitmap_zero(ids, si->tch_abs[CY_TCH_T].max);
> > > + memset(tch->abs, 0, sizeof(tch->abs));
> > > +
> > > + for (i = 0; i < num_cur_tch; i++) {
> > > + tch_addr = si->xy_data + (i * TOUCH_REPORT_SIZE);
> > > + cyttsp5_get_touch_record(ts, tch, tch_addr);
> > > +
> > > + /* Convert MAJOR/MINOR from mm to resolution */
> > > + tmp = tch->abs[CY_TCH_MAJ] * 100 * si->sensing_conf_data.res_x;
> > > + tch->abs[CY_TCH_MAJ] = tmp / si->sensing_conf_data.len_x;
> > > + tmp = tch->abs[CY_TCH_MIN] * 100 * si->sensing_conf_data.res_x;
> > > + tch->abs[CY_TCH_MIN] = tmp / si->sensing_conf_data.len_x;
> > > +
> > > + t = tch->abs[CY_TCH_T];
> > > + input_mt_slot(ts->input, t);
> > > + input_mt_report_slot_state(ts->input, MT_TOOL_FINGER, true);
> > > + __set_bit(t, ids);
> > > +
> > > + /* position and pressure fields */
> > > + input_report_abs(ts->input, ABS_MT_POSITION_X,
> > > + tch->abs[CY_TCH_X]);
> > > + input_report_abs(ts->input, ABS_MT_POSITION_Y,
> > > + tch->abs[CY_TCH_Y]);
> > > + input_report_abs(ts->input, ABS_MT_PRESSURE,
> > > + tch->abs[CY_TCH_P]);
> > > +
> > > + /* Get the extended touch fields */
> > > + input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR,
> > > + tch->abs[CY_TCH_MAJ]);
> > > + input_report_abs(ts->input, ABS_MT_TOUCH_MINOR,
> > > + tch->abs[CY_TCH_MIN]);
> > > + }
> > > +
> > > + cyttsp5_final_sync(ts->input, si->tch_abs[CY_TCH_T].max, ids);
> > > +
> > > + ts->num_prv_rec = num_cur_tch;
> > > +}
> > > +
> > > +/* read xy_data for all current touches */
> > > +static int cyttsp5_xy_worker(struct cyttsp5 *ts)
> > > +{
> > > + struct device *dev = ts->dev;
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > + int max_tch = si->sensing_conf_data.max_tch;
> > > + struct cyttsp5_touch tch;
> > > + u8 num_cur_tch;
> > > +
> > > + cyttsp5_get_touch_axis(&tch.hdr, si->tch_hdr.size,
> > > + si->tch_hdr.max,
> > > + si->xy_mode + 3 + si->tch_hdr.ofs,
> > > + si->tch_hdr.bofs);
> > > +
> > > + num_cur_tch = tch.hdr;
> > > + if (num_cur_tch > max_tch) {
> > > + dev_err(dev, "Num touch err detected (n=%d)\n", num_cur_tch);
> > > + num_cur_tch = max_tch;
> > > + }
> > > +
> > > + if (num_cur_tch == 0 && ts->num_prv_rec == 0)
> > > + return 0;
> > > +
> > > + /* extract xy_data for all currently reported touches */
> > > + if (num_cur_tch)
> > > + cyttsp5_get_mt_touches(ts, &tch, num_cur_tch);
> > > + else
> > > + cyttsp5_mt_lift_all(ts);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int cyttsp5_mt_attention(struct device *dev)
> > > +{
> > > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> >
> > Why don't you pass ts into cyttsp5_mt_attention?
>
> Yep, you are right, I will update it.
>
> >
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > + int rc;
> > > +
> > > + if (si->xy_mode[2] != HID_TOUCH_REPORT_ID)
> > > + return 0;
> > > +
> > > + /* core handles handshake */
> > > + rc = cyttsp5_xy_worker(ts);
> > > + if (rc < 0)
> > > + dev_err(dev, "xy_worker error r=%d\n", rc);
> > > +
> > > + return rc;
> > > +}
> > > +
> > > +static int cyttsp5_setup_input_device(struct device *dev)
> > > +{
> > > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > + int max_x, max_y, max_p;
> > > + int max_x_tmp, max_y_tmp;
> > > + int rc;
> > > +
> > > + __set_bit(EV_ABS, ts->input->evbit);
> > > + __set_bit(EV_REL, ts->input->evbit);
> > > + __set_bit(EV_KEY, ts->input->evbit);
> > > +
> > > + max_x_tmp = si->sensing_conf_data.res_x;
> > > + max_y_tmp = si->sensing_conf_data.res_y;
> > > + max_x = max_y_tmp - 1;
> > > + max_y = max_x_tmp - 1;
> > > + max_p = si->sensing_conf_data.max_z;
> > > +
> > > + input_mt_init_slots(ts->input, si->tch_abs[CY_TCH_T].max, 0);
> >
> > Error handling.
>
> Thanks.
>
> >
> > > +
> > > + __set_bit(ABS_MT_POSITION_X, ts->input->absbit);
> > > + __set_bit(ABS_MT_POSITION_Y, ts->input->absbit);
> > > + __set_bit(ABS_MT_PRESSURE, ts->input->absbit);
> > > +
> > > + input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, max_x, 0, 0);
> > > + input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, max_y, 0, 0);
> > > + input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, max_p, 0, 0);
> > > +
> > > + input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
> > > + input_set_abs_params(ts->input, ABS_MT_TOUCH_MINOR, 0, MAX_AREA, 0, 0);
> >
> > You might want to use the standard DT touchscreen properties and
> > reporting functions to support axis inversion and swapping of axes.
>
> Hm, I am not sure to see what you are referring to.
> Could you explain me what you think of?
Please take a look at include/linux/input/touchscreen.h and in the other
driver you are hacking on there is example of how to use this API.
Basically you need to call touchscreen_parse_properties() somewhere in
probe path, and then use touchscreen_report_pos() to report contact
coordinates within given slot.
>
> >
> > > +
> > > + rc = input_register_device(ts->input);
> > > + if (rc < 0)
> > > + dev_err(dev, "Error, failed register input device r=%d\n", rc);
> > > +
> > > + return rc;
> > > +}
> > > +
> > > +#ifdef CONFIG_OF
> > > +static int cyttsp5_parse_dt_key_code(struct device *dev)
> > > +{
> > > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > + struct device_node *np;
> > > + int i;
> > > +
> > > + np = dev->of_node;
> > > + if (!np)
> > > + return -EINVAL;
> > > +
> > > + if (!si->num_btns)
> > > + return 0;
> > > +
> > > + /* Initialize the button to RESERVED */
> > > + for (i = 0; i < si->num_btns; i++)
> > > + si->key_code[i] = KEY_RESERVED;
> > > +
> > > + return of_property_read_u32_array(np, "linux,keycodes",
> > > + si->key_code, si->num_btns);
> > > +}
> > > +#else
> > > +static int cyttsp5_parse_dt_key_code(struct device *dev)
> > > +{
> > > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > + int i;
> > > +
> > > + if (!si->num_btns)
> > > + return 0;
> > > +
> > > + /* Initialize the button to RESERVED */
> > > + for (i = 0; i < si->num_btns; i++)
> > > + si->key_code[i] = KEY_RESERVED;
> > > +
> > > + return 0;
> > > +}
> > > +#endif
> > > +
> > > +static int cyttsp5_btn_attention(struct device *dev)
> > > +{
> > > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > + int cur_btn;
> > > + int cur_btn_state;
> > > +
> > > + if (si->xy_mode[2] != HID_BTN_REPORT_ID || !si->num_btns)
> > > + return 0;
> > > +
> > > + /* extract button press/release touch information */
> > > + for (cur_btn = 0; cur_btn < si->num_btns; cur_btn++) {
> > > + /* Get current button state */
> > > + cur_btn_state = (si->xy_data[0] >> (cur_btn * CY_BITS_PER_BTN))
> > > + & CY_NUM_BTN_EVENT_ID;
> > > +
> > > + input_report_key(ts->input, si->key_code[cur_btn],
> > > + cur_btn_state);
> > > + input_sync(ts->input);
> >
> > You do not need to sen sync after each button, send it once after all
> > buttons sent.
>
> Oh, yes, I will do that!
>
> >
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static u16 cyttsp5_compute_crc(u8 *buf, u32 size)
> > > +{
> > > + u16 remainder = 0xFFFF;
> > > + u16 xor_mask = 0x0000;
> > > + u32 index;
> > > + u32 byte_value;
> > > + u32 table_index;
> > > + u32 crc_bit_width = sizeof(u16) * 8;
> > > +
> > > + /* Divide the message by polynomial, via the table. */
> > > + for (index = 0; index < size; index++) {
> > > + byte_value = buf[index];
> > > + table_index = ((byte_value >> 4) & 0x0F)
> > > + ^ (remainder >> (crc_bit_width - 4));
> > > + remainder = crc_itu_t_table[table_index]
> > > + ^ (remainder << 4);
> > > + table_index = (byte_value & 0x0F)
> > > + ^ (remainder >> (crc_bit_width - 4));
> > > + remainder = crc_itu_t_table[table_index]
> > > + ^ (remainder << 4);
> > > + }
> > > +
> > > + /* Perform the final remainder CRC. */
> > > + return remainder ^ xor_mask;
> > > +}
> > > +
> > > +static int cyttsp5_validate_cmd_response(struct cyttsp5 *ts, u8 code)
> > > +{
> > > + u16 size, crc;
> > > + u8 status, offset;
> > > + int command_code;
> > > +
> > > + size = get_unaligned_le16(&ts->response_buf[0]);
> > > +
> > > + if (!size)
> > > + return 0;
> > > +
> > > + offset = ts->response_buf[HID_OUTPUT_RESPONSE_REPORT_OFFSET];
> > > +
> > > + if (offset == HID_BL_RESPONSE_REPORT_ID) {
> > > + if (ts->response_buf[4] != HID_OUTPUT_BL_SOP) {
> > > + dev_err(ts->dev, "HID output response, wrong SOP\n");
> > > + return -EPROTO;
> > > + }
> > > +
> > > + if (ts->response_buf[size - 1] != HID_OUTPUT_BL_EOP) {
> > > + dev_err(ts->dev, "HID output response, wrong EOP\n");
> > > + return -EPROTO;
> > > + }
> > > +
> > > + crc = cyttsp5_compute_crc(&ts->response_buf[4], size - 7);
> > > + if (ts->response_buf[size - 3] != LOW_BYTE(crc) ||
> > > + ts->response_buf[size - 2] != HI_BYTE(crc)) {
> > > + dev_err(ts->dev, "HID output response, wrong CRC 0x%X\n",
> > > + crc);
> > > + return -EPROTO;
> > > + }
> > > +
> > > + status = ts->response_buf[5];
> > > + if (status) {
> > > + dev_err(ts->dev, "HID output response, ERROR:%d\n",
> > > + status);
> > > + return -EPROTO;
> > > + }
> > > + }
> > > +
> > > + if (offset == HID_APP_RESPONSE_REPORT_ID) {
> > > + command_code = ts->response_buf[HID_OUTPUT_RESPONSE_CMD_OFFSET]
> > > + & HID_OUTPUT_RESPONSE_CMD_MASK;
> > > + if (command_code != code) {
> > > + dev_err(ts->dev,
> > > + "HID output response, wrong command_code:%X\n",
> > > + command_code);
> > > + return -EPROTO;
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void cyttsp5_si_get_btn_data(struct cyttsp5 *ts)
> > > +{
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > + int i;
> > > + unsigned int btns = ts->response_buf[HID_SYSINFO_BTN_OFFSET]
> > > + & HID_SYSINFO_BTN_MASK;
> > > +
> > > + si->num_btns = 0;
> > > + for (i = 0; i < HID_SYSINFO_MAX_BTN; i++) {
> > > + if (btns & BIT(i))
> > > + si->num_btns++;
> > > + }
> >
> > si->num_btns = hweight8(ts->response_buf[HID_SYSINFO_BTN_OFFSET]);
>
> Great, I will try that!
>
> >
> > > +}
> > > +
> > > +static int cyttsp5_get_sysinfo_regs(struct cyttsp5 *ts)
> > > +{
> > > + struct cyttsp5_sensing_conf_data *scd = &ts->sysinfo.sensing_conf_data;
> > > + struct cyttsp5_sensing_conf_data_dev *scd_dev =
> > > + (struct cyttsp5_sensing_conf_data_dev *)
> > > + &ts->response_buf[HID_SYSINFO_SENSING_OFFSET];
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > +
> > > + cyttsp5_si_get_btn_data(ts);
> > > +
> > > + scd->max_tch = scd_dev->max_num_of_tch_per_refresh_cycle;
> > > + scd->res_x = get_unaligned_le16(&scd_dev->res_x);
> > > + scd->res_y = get_unaligned_le16(&scd_dev->res_y);
> > > + scd->max_z = get_unaligned_le16(&scd_dev->max_z);
> > > + scd->len_x = get_unaligned_le16(&scd_dev->len_x);
> > > + scd->len_y = get_unaligned_le16(&scd_dev->len_y);
> > > +
> > > + si->xy_data = devm_kzalloc(ts->dev, scd->max_tch * TOUCH_REPORT_SIZE,
> > > + GFP_KERNEL);
> > > + if (!si->xy_data)
> > > + return -ENOMEM;
> > > +
> > > + si->xy_mode = devm_kzalloc(ts->dev, TOUCH_INPUT_HEADER_SIZE,
> > > + GFP_KERNEL);
> > > + if (!si->xy_mode)
> > > + return -ENOMEM;
> >
> > Why do we need these 2 allocated separately form the driver data?
>
> xy_mode and particularly xy_data are used to retrieve buttons or touch
> data that we received from the input buffer (see gggggggggggggggg()
> and move_touch_data()). I guess that I can try to remove them, if you
> want.
You probably do not need them, you already have all data in input_buf,
just the right offset to cyttsp5_btn_attention() , like
...
cyttsp5_btn_attention(ts, ts->input_buf + BTN_INPUT_HEADER_SIZE);
...
>
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int cyttsp5_hid_output_get_sysinfo(struct cyttsp5 *ts)
> > > +{
> > > + int rc;
> > > + u8 cmd[HID_OUTPUT_GET_SYSINFO_SIZE];
> > > +
> > > + ts->hid_cmd_state = HID_CMD_BUSY;
> > > +
> > > + /* HI bytes of Output register address */
> > > + cmd[0] = LOW_BYTE(HID_OUTPUT_GET_SYSINFO_SIZE);
> > > + cmd[1] = HI_BYTE(HID_OUTPUT_GET_SYSINFO_SIZE);
> > > + cmd[2] = HID_APP_OUTPUT_REPORT_ID;
> > > + cmd[3] = 0x0; /* Reserved */
> > > + cmd[4] = HID_OUTPUT_GET_SYSINFO;
> > > +
> > > + rc = cyttsp5_write(ts, HID_OUTPUT_REG, cmd,
> > > + HID_OUTPUT_GET_SYSINFO_SIZE);
> > > + if (rc) {
> > > + dev_err(ts->dev, "Failed to write command %d", rc);
> > > + goto error;
> > > + }
> > > +
> > > + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> > > + msecs_to_jiffies(CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT));
> > > + if (!rc) {
> > > + dev_err(ts->dev, "HID output cmd execution timed out\n");
> > > + rc = -ETIME;
> > > + goto error;
> > > + }
> > > +
> > > + rc = cyttsp5_validate_cmd_response(ts, HID_OUTPUT_GET_SYSINFO);
> > > + if (rc) {
> > > + dev_err(ts->dev, "Validation of the response failed\n");
> > > + goto error;
> > > + }
> > > +
> > > + return cyttsp5_get_sysinfo_regs(ts);
> > > +
> > > +error:
> > > + mutex_lock(&ts->system_lock);
> > > + ts->hid_cmd_state = HID_CMD_DONE;
> > > + mutex_unlock(&ts->system_lock);
> > > + return rc;
> > > +}
> > > +
> > > +static int cyttsp5_hid_output_bl_launch_app(struct cyttsp5 *ts)
> > > +{
> > > + int rc;
> > > + u8 cmd[HID_OUTPUT_BL_LAUNCH_APP];
> > > + u16 crc;
> > > +
> > > + mutex_lock(&ts->system_lock);
> > > + ts->hid_cmd_state = HID_CMD_BUSY;
> > > + mutex_unlock(&ts->system_lock);
> > > +
> > > + cmd[0] = LOW_BYTE(HID_OUTPUT_BL_LAUNCH_APP_SIZE);
> > > + cmd[1] = HI_BYTE(HID_OUTPUT_BL_LAUNCH_APP_SIZE);
> > > + cmd[2] = HID_BL_OUTPUT_REPORT_ID;
> > > + cmd[3] = 0x0; /* Reserved */
> > > + cmd[4] = HID_OUTPUT_BL_SOP;
> > > + cmd[5] = HID_OUTPUT_BL_LAUNCH_APP;
> > > + cmd[6] = 0x0; /* Low bytes of data */
> > > + cmd[7] = 0x0; /* Hi bytes of data */
> > > + crc = cyttsp5_compute_crc(&cmd[4], 4);
> > > + cmd[8] = LOW_BYTE(crc);
> > > + cmd[9] = HI_BYTE(crc);
> > > + cmd[10] = HID_OUTPUT_BL_EOP;
> > > +
> > > + rc = cyttsp5_write(ts, HID_OUTPUT_REG, cmd,
> > > + HID_OUTPUT_BL_LAUNCH_APP_SIZE);
> > > + if (rc) {
> > > + dev_err(ts->dev, "Failed to write command %d", rc);
> > > + goto error;
> > > + }
> > > +
> > > + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> > > + msecs_to_jiffies(CY_HID_OUTPUT_TIMEOUT));
> > > + if (!rc) {
> > > + dev_err(ts->dev, "HID output cmd execution timed out\n");
> > > + rc = -ETIME;
> > > + goto error;
> > > + }
> > > +
> > > + rc = cyttsp5_validate_cmd_response(ts, HID_OUTPUT_BL_LAUNCH_APP);
> > > + if (rc) {
> > > + dev_err(ts->dev, "Validation of the response failed\n");
> > > + goto error;
> > > + }
> > > +
> > > + return rc;
> > > +
> > > +error:
> > > + mutex_lock(&ts->system_lock);
> > > + ts->hid_cmd_state = HID_CMD_DONE;
> > > + mutex_unlock(&ts->system_lock);
> > > +
> > > + return rc;
> > > +}
> > > +
> > > +static int cyttsp5_get_hid_descriptor(struct cyttsp5 *ts,
> > > + struct cyttsp5_hid_desc *desc)
> > > +{
> > > + struct device *dev = ts->dev;
> > > + __le16 hid_desc_register = HID_DESC_REG;
> > > + int rc;
> > > + u8 cmd[2];
> > > +
> > > + /* Read HID descriptor length and version */
> > > + mutex_lock(&ts->system_lock);
> > > + ts->hid_cmd_state = HID_CMD_BUSY;
> > > + mutex_unlock(&ts->system_lock);
> > > +
> > > + /* Set HID descriptor register */
> > > + memcpy(cmd, &hid_desc_register, sizeof(hid_desc_register));
> > > +
> > > + rc = cyttsp5_write(ts, HID_DESC_REG, NULL, 0);
> > > + if (rc < 0) {
> > > + dev_err(dev, "Failed to get HID descriptor, rc=%d\n", rc);
> > > + goto error;
> > > + }
> > > +
> > > + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> > > + msecs_to_jiffies(CY_HID_GET_HID_DESCRIPTOR_TIMEOUT));
> > > + if (!rc) {
> > > + dev_err(ts->dev, "HID get descriptor timed out\n");
> > > + rc = -ETIME;
> > > + goto error;
> > > + }
> > > +
> > > + memcpy(desc, ts->response_buf, sizeof(struct cyttsp5_hid_desc));
> > > +
> > > + /* Check HID descriptor length and version */
> > > + if (le16_to_cpu(desc->hid_desc_len) != sizeof(*desc) ||
> > > + le16_to_cpu(desc->bcd_version) != HID_VERSION) {
> > > + dev_err(dev, "Unsupported HID version\n");
> > > + return -ENODEV;
> > > + }
> > > +
> > > + goto exit;
> > > +
> > > +error:
> > > + mutex_lock(&ts->system_lock);
> > > + ts->hid_cmd_state = HID_CMD_DONE;
> > > + mutex_unlock(&ts->system_lock);
> > > +exit:
> > > + return rc;
> > > +}
> > > +
> > > +static int fill_tch_abs(struct cyttsp5_tch_abs_params *tch_abs, int report_size,
> > > + int offset)
> > > +{
> > > + tch_abs->ofs = offset / 8;
> > > + tch_abs->size = report_size / 8;
> > > + if (report_size % 8)
> > > + tch_abs->size += 1;
> > > + tch_abs->min = 0;
> > > + tch_abs->max = 1 << report_size;
> > > + tch_abs->bofs = offset - (tch_abs->ofs << 3);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int move_button_data(struct cyttsp5 *ts, struct cyttsp5_sysinfo *si)
> > > +{
> > > + memcpy(si->xy_mode, ts->input_buf, BTN_INPUT_HEADER_SIZE);
> > > + memcpy(si->xy_data, &ts->input_buf[BTN_INPUT_HEADER_SIZE],
> > > + BTN_REPORT_SIZE);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int move_touch_data(struct cyttsp5 *ts, struct cyttsp5_sysinfo *si)
> > > +{
> > > + int max_tch = si->sensing_conf_data.max_tch;
> > > + int num_cur_tch;
> > > + int length;
> > > + struct cyttsp5_tch_abs_params *tch = &si->tch_hdr;
> > > +
> > > + memcpy(si->xy_mode, ts->input_buf, TOUCH_INPUT_HEADER_SIZE);
> > > +
> > > + cyttsp5_get_touch_axis(&num_cur_tch, tch->size,
> > > + tch->max, si->xy_mode + 3 + tch->ofs, tch->bofs);
> > > + if (unlikely(num_cur_tch > max_tch))
> > > + num_cur_tch = max_tch;
> > > +
> > > + length = num_cur_tch * TOUCH_REPORT_SIZE;
> > > +
> > > + memcpy(si->xy_data, &ts->input_buf[TOUCH_INPUT_HEADER_SIZE], length);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static irqreturn_t cyttsp5_handle_irq(int irq, void *handle)
> > > +{
> > > + struct cyttsp5 *ts = handle;
> > > + int report_id;
> > > + int size;
> > > + int rc;
> > > +
> > > + rc = cyttsp5_read(ts, ts->input_buf, CY_MAX_INPUT);
> > > + if (rc)
> > > + return IRQ_HANDLED;
> > > +
> > > + size = get_unaligned_le16(&ts->input_buf[0]);
> > > +
> > > + /* check reset */
> > > + if (size == 0) {
> > > + memcpy(ts->response_buf, ts->input_buf, 2);
> > > +
> > > + mutex_lock(&ts->system_lock);
> > > + ts->hid_cmd_state = HID_CMD_DONE;
> > > + mutex_unlock(&ts->system_lock);
> > > + wake_up(&ts->wait_q);
> >
> > I'd use spinlock in wait queue and wake_up_locked() instead of a
> > separate mutex.
>
> Hum, okay, I will try that.
>
> >
> > > + return IRQ_HANDLED;
> > > + }
> > > +
> > > + report_id = ts->input_buf[2];
> > > +
> >
> > switch(report_id) {
> > }
>
> Yep, it will be easier to understand.
>
> >
> > > + if (report_id == HID_TOUCH_REPORT_ID) {
> > > + move_touch_data(ts, &ts->sysinfo);
> > > + cyttsp5_mt_attention(ts->dev);
> > > + } else if (report_id == HID_BTN_REPORT_ID) {
> > > + move_button_data(ts, &ts->sysinfo);
> > > + cyttsp5_btn_attention(ts->dev);
> > > + } else {
> > > + /* It is not an input but a command response */
> > > + memcpy(ts->response_buf, ts->input_buf, size);
> > > +
> > > + mutex_lock(&ts->system_lock);
> > > + ts->hid_cmd_state = HID_CMD_DONE;
> > > + mutex_unlock(&ts->system_lock);
> > > + wake_up(&ts->wait_q);
> > > + }
> > > +
> > > + return IRQ_HANDLED;
> > > +}
> > > +
> > > +static int cyttsp5_deassert_int(struct cyttsp5 *ts)
> > > +{
> > > + u16 size;
> > > + u8 buf[2];
> > > + int rc;
> > > +
> > > + rc = regmap_bulk_read(ts->regmap, HID_INPUT_REG, buf, 2);
> > > + if (rc < 0)
> > > + return rc;
> > > +
> > > + size = get_unaligned_le16(&buf[0]);
> > > + if (size == 2 || size == 0)
> > > + return 0;
> > > +
> > > + return -EINVAL;
> > > +}
> > > +
> > > +static int cyttsp5_fill_all_touch(struct cyttsp5 *ts)
> > > +{
> > > + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> > > +
> > > + fill_tch_abs(&si->tch_abs[CY_TCH_X], REPORT_SIZE_16,
> > > + TOUCH_REPORT_DESC_X);
> > > + fill_tch_abs(&si->tch_abs[CY_TCH_Y], REPORT_SIZE_16,
> > > + TOUCH_REPORT_DESC_Y);
> > > + fill_tch_abs(&si->tch_abs[CY_TCH_P], REPORT_SIZE_8,
> > > + TOUCH_REPORT_DESC_P);
> > > + fill_tch_abs(&si->tch_abs[CY_TCH_T], REPORT_SIZE_5,
> > > + TOUCH_REPORT_DESC_CONTACTID);
> > > + fill_tch_abs(&si->tch_hdr, REPORT_SIZE_5,
> > > + TOUCH_REPORT_DESC_HDR_CONTACTCOUNT);
> > > + fill_tch_abs(&si->tch_abs[CY_TCH_MAJ], REPORT_SIZE_8,
> > > + TOUCH_REPORT_DESC_MAJ);
> > > + fill_tch_abs(&si->tch_abs[CY_TCH_MIN], REPORT_SIZE_8,
> > > + TOUCH_REPORT_DESC_MIN);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int cyttsp5_startup(struct cyttsp5 *ts)
> > > +{
> > > + int rc;
> > > +
> > > + rc = cyttsp5_deassert_int(ts);
> > > + if (rc) {
> > > + dev_err(ts->dev, "Error on deassert int r=%d\n", rc);
> > > + return -ENODEV;
> > > + }
> > > +
> > > + /*
> > > + * Launch the application as the device starts in bootloader mode
> > > + * because of a power-on-reset
> > > + */
> > > + rc = cyttsp5_hid_output_bl_launch_app(ts);
> > > + if (rc < 0) {
> > > + dev_err(ts->dev, "Error on launch app r=%d\n", rc);
> > > + return rc;
> > > + }
> > > +
> > > + rc = cyttsp5_get_hid_descriptor(ts, &ts->hid_desc);
> > > + if (rc < 0) {
> > > + dev_err(ts->dev, "Error on getting HID descriptor r=%d\n", rc);
> > > + return rc;
> > > + }
> > > +
> > > + rc = cyttsp5_fill_all_touch(ts);
> > > + if (rc < 0) {
> > > + dev_err(ts->dev, "Error on report descriptor r=%d\n", rc);
> > > + return rc;
> > > + }
> > > +
> > > + rc = cyttsp5_hid_output_get_sysinfo(ts);
> > > + if (rc) {
> > > + dev_err(ts->dev, "Error on getting sysinfo r=%d\n", rc);
> > > + return rc;
> > > + }
> > > +
> > > + return rc;
> > > +}
> > > +
> > > +#ifdef CONFIG_OF
> > > +static const struct of_device_id cyttsp5_of_match[] = {
> > > + { .compatible = "cypress,cyttsp5", },
> > > + { }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, cyttsp5_of_match);
> > > +#endif
> > > +
> > > +static const struct i2c_device_id cyttsp5_i2c_id[] = {
> > > + { CYTTSP5_NAME, 0, },
> > > + { }
> > > +};
> > > +MODULE_DEVICE_TABLE(i2c, cyttsp5_i2c_id);
> > > +
> > > +static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> > > + const char *name)
> > > +{
> > > + struct cyttsp5 *ts;
> > > + struct cyttsp5_sysinfo *si;
> > > + int rc = 0, i;
> > > +
> > > + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> > > + if (!ts)
> > > + return -ENOMEM;
> > > +
> > > + /* Initialize device info */
> > > + ts->regmap = regmap;
> > > + ts->dev = dev;
> > > + si = &ts->sysinfo;
> > > + dev_set_drvdata(dev, ts);
> > > +
> > > + /* Initialize mutexes and spinlocks */
> > > + mutex_init(&ts->system_lock);
> > > +
> > > + /* Initialize wait queue */
> > > + init_waitqueue_head(&ts->wait_q);
> > > +
> > > + /* Reset the gpio to be in a reset state */
> > > + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> > > + if (IS_ERR(ts->reset_gpio)) {
> > > + rc = PTR_ERR(ts->reset_gpio);
> > > + dev_err(dev, "Failed to request reset gpio, error %d\n", rc);
> > > + return rc;
> > > + }
> > > + gpiod_set_value(ts->reset_gpio, 1);
> >
> > Why not request it as GPIOD_OUT_HIGH if you going to activate it
> > immediately? However it sounds as if you have a reset line that is
> > active low and you want to driver it low to reset, and then release. In
> > this case you need to do
> >
> > ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> > <delay> - chip usually needs to be some time in reset state
> > gpiod_set_value(ts->reset_gpio, 0);
> >
> > and make sure that correct polarity is specified in device tree.
>
> Okay, I will update it too.
>
> >
> > > +
> > > + /* Need a delay to have device up */
> > > + msleep(20);
> > > +
> > > + rc = devm_request_threaded_irq(dev, irq, NULL, cyttsp5_handle_irq,
> > > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> >
> > Just use IRQF_ONESHOT and make sure DTS specifies interrupt properly.
> > BTW,I'd recommend using level, not edge interrupts. It is so easy to
> > miss an edge.
>
> Okay, thank you for the advice.
>
> >
> > > + name, ts);
> > > + if (rc) {
> > > + dev_err(dev, "unable to request IRQ\n");
> > > + return rc;
> > > + }
> > > +
> > > + rc = cyttsp5_startup(ts);
> > > + if (rc) {
> > > + dev_err(ts->dev, "Fail initial startup r=%d\n", rc);
> > > + return rc;
> > > + }
> > > +
> > > + rc = cyttsp5_parse_dt_key_code(dev);
> > > + if (rc < 0) {
> > > + dev_err(ts->dev, "Error while parsing dts %d\n", rc);
> > > + return rc;
> > > + }
> > > +
> > > + ts->input = devm_input_allocate_device(dev);
> > > + if (!ts->input) {
> > > + dev_err(dev, "Error, failed to allocate input device\n");
> > > + return -ENODEV;
> > > + }
> > > +
> > > + ts->input->name = "cyttsp5";
> > > + scnprintf(ts->phys, sizeof(ts->phys), "%s/input0", dev_name(dev));
> > > + ts->input->phys = ts->phys;
> > > + ts->input->dev.parent = ts->dev;
> > > + input_set_drvdata(ts->input, ts);
> > > +
> > > + touchscreen_parse_properties(ts->input, true, NULL);
> > > +
> > > + __set_bit(EV_KEY, ts->input->evbit);
> > > + for (i = 0; i < si->num_btns; i++)
> > > + __set_bit(si->key_code[i], ts->input->keybit);
> > > +
> > > + return cyttsp5_setup_input_device(dev);
> > > +}
> > > +
> > > +static int cyttsp5_i2c_probe(struct i2c_client *client,
> > > + const struct i2c_device_id *id)
> > > +{
> > > + struct regmap *regmap;
> > > + static const struct regmap_config config = {
> > > + .reg_bits = 8,
> > > + .val_bits = 8,
> > > + };
> > > +
> > > + regmap = devm_regmap_init_i2c(client, &config);
> > > + if (IS_ERR(regmap)) {
> > > + dev_err(&client->dev, "regmap allocation failed: %ld\n",
> > > + PTR_ERR(regmap));
> > > + return PTR_ERR(regmap);
> > > + }
> > > +
> > > + return cyttsp5_probe(&client->dev, regmap, client->irq, client->name);
> > > +}
> > > +
> > > +static int cyttsp5_remove(struct device *dev)
> > > +{
> > > + struct cyttsp5 *ts = dev_get_drvdata(dev);
> > > +
> > > + input_unregister_device(ts->input);
> >
> > Not needed: you are using managed input device.
>
> You are right, thanks.
>
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int cyttsp5_i2c_remove(struct i2c_client *client)
> > > +{
> > > + struct device *dev = &client->dev;
> > > +
> > > + return cyttsp5_remove(dev);
> >
> > Not needed if you remove cyttsp5_remove().
>
> Sure.
>
> >
> > > +}
> > > +
> > > +static struct i2c_driver cyttsp5_i2c_driver = {
> > > + .driver = {
> > > + .name = CYTTSP5_NAME,
> > > + .owner = THIS_MODULE,
> > > + .of_match_table = of_match_ptr(cyttsp5_of_match),
> > > + },
> > > + .probe = cyttsp5_i2c_probe,
> > > + .remove = cyttsp5_i2c_remove,
> > > + .id_table = cyttsp5_i2c_id,
> > > +};
> > > +
> > > +module_i2c_driver(cyttsp5_i2c_driver);
> > > +
> > > +MODULE_LICENSE("GPL");
> > > +MODULE_DESCRIPTION("Touchscreen driver for Cypress TrueTouch Gen 5 Product");
> > > +MODULE_AUTHOR("Mylène Josserand <mylene.josserand@free-electrons.com>");
> > > --
> > > 2.11.0
> > >
> >
> > Thanks.
> >
>
> Best regards,
>
> --
> Mylène Josserand, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: edt-ft5x06 - Add support for regulator
From: Dmitry Torokhov @ 2018-01-23 17:58 UTC (permalink / raw)
To: Mylene Josserand
Cc: Rob Herring, Mark Rutland, Russell King - ARM Linux,
Maxime Ripard, Chen-Yu Tsai, linux-arm-kernel,
linux-input@vger.kernel.org, devicetree, lkml, Thomas Petazzoni,
Quentin Schulz
In-Reply-To: <20180123101055.28c4786a@dell-desktop.home>
On Tue, Jan 23, 2018 at 10:10:55AM +0100, Mylene Josserand wrote:
> Hello Dimitry,
>
> Thank you for the review!
>
> Le Mon, 22 Jan 2018 09:42:08 -0800,
> Dmitry Torokhov <dmitry.torokhov@gmail.com> a écrit :
>
> > Hi Mylène,
> >
> > On Thu, Dec 28, 2017 at 8:33 AM, Mylène Josserand
> > <mylene.josserand@free-electrons.com> wrote:
> > > Add the support of regulator to use it as VCC source.
> > >
> > > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > > ---
> > > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 +
> > > drivers/input/touchscreen/edt-ft5x06.c | 33 ++++++++++++++++++++++
> > > 2 files changed, 34 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > > index 025cf8c9324a..48e975b9c1aa 100644
> > > --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
> > > @@ -30,6 +30,7 @@ Required properties:
> > > Optional properties:
> > > - reset-gpios: GPIO specification for the RESET input
> > > - wake-gpios: GPIO specification for the WAKE input
> > > + - vcc-supply: Regulator that supplies the touchscreen
> > >
> > > - pinctrl-names: should be "default"
> > > - pinctrl-0: a phandle pointing to the pin settings for the
> > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> > > index c53a3d7239e7..5ee14a25a382 100644
> > > --- a/drivers/input/touchscreen/edt-ft5x06.c
> > > +++ b/drivers/input/touchscreen/edt-ft5x06.c
> > > @@ -39,6 +39,7 @@
> > > #include <linux/input/mt.h>
> > > #include <linux/input/touchscreen.h>
> > > #include <linux/of_device.h>
> > > +#include <linux/regulator/consumer.h>
> > >
> > > #define WORK_REGISTER_THRESHOLD 0x00
> > > #define WORK_REGISTER_REPORT_RATE 0x08
> > > @@ -91,6 +92,7 @@ struct edt_ft5x06_ts_data {
> > > struct touchscreen_properties prop;
> > > u16 num_x;
> > > u16 num_y;
> > > + struct regulator *vcc;
> > >
> > > struct gpio_desc *reset_gpio;
> > > struct gpio_desc *wake_gpio;
> > > @@ -993,6 +995,23 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
> > >
> > > tsdata->max_support_points = chip_data->max_support_points;
> > >
> > > + tsdata->vcc = devm_regulator_get(&client->dev, "vcc");
> > > + if (IS_ERR(tsdata->vcc)) {
> > > + error = PTR_ERR(tsdata->vcc);
> > > + dev_err(&client->dev, "failed to request regulator: %d\n",
> > > + error);
> > > + return error;
> > > + };
> >
> > As 0-day pounted out, this semicolon is not needed.
>
> Yes, thanks, I will fix that in next version.
>
> >
> > > +
> > > + if (tsdata->vcc) {
> >
> > You do not need to check for non-NULL here, devm_regulator_get() wil
> > lnever give you a NULL. If regulator is not defined in DT/board
> > mappings, then dummy regulator will be provided. You can call
> > regulator_enable() and regulator_disable() and other regulator APIs
> > with dummy regulator.
>
> Okay, thanks for the explanation, I will remove that.
>
> >
> > > + error = regulator_enable(tsdata->vcc);
> > > + if (error < 0) {
> > > + dev_err(&client->dev, "failed to enable vcc: %d\n",
> > > + error);
> > > + return error;
> > > + }
> > > + }
> > > +
> > > tsdata->reset_gpio = devm_gpiod_get_optional(&client->dev,
> > > "reset", GPIOD_OUT_HIGH);
> > > if (IS_ERR(tsdata->reset_gpio)) {
> > > @@ -1122,20 +1141,34 @@ static int edt_ft5x06_ts_remove(struct i2c_client *client)
> > > static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev)
> > > {
> > > struct i2c_client *client = to_i2c_client(dev);
> > > + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
> > >
> > > if (device_may_wakeup(dev))
> > > enable_irq_wake(client->irq);
> > >
> > > + if (tsdata->vcc)
> >
> > Same here.
>
> yep
>
> >
> > > + regulator_disable(tsdata->vcc);
> > > +
> > > return 0;
> > > }
> > >
> > > static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev)
> > > {
> > > struct i2c_client *client = to_i2c_client(dev);
> > > + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client);
> > > + int ret;
> > >
> > > if (device_may_wakeup(dev))
> > > disable_irq_wake(client->irq);
> > >
> > > + if (tsdata->vcc) {
> >
> > And here.
>
> yep
>
> >
> > > + ret = regulator_enable(tsdata->vcc);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to enable vcc: %d\n", ret);
> > > + return ret;
> > > + }
> >
> > Since power to the device may have been cut, I think you need to
> > restore the register settings to whatever it was (factory vs work
> > mode, threshold, gain and offset registers, etc, etc).
>
> Okay. Could you tell me how can I do that?
If you take a look at edt_ft5x06_work_mode() at the end ther eis code to
restore parameters. You want to factor it out and apply to the device
when resuming. You probably also want to see what mode (factory/normal)
the device was before suspend and restore the previous mode on resume.
>
> >
> > > + }
> > > +
> > > return 0;
> > > }
> > >
> > > --
> > > 2.11.0
> > >
> >
> > Thanks.
> >
>
> About your V2's review, you suggested to add support for wake/reset in
> suspend/resume (that I forgot in this version). I wanted to add it but
> with my board, I can't test suspend/resume. What should I do about
> that?
See if Simon Budig <simon.budig@kernelconcepts.de> can help us here.
>
> If I send a V3 in next few days, do you think you will have time to
> merge it for v4.16?
It all depends on the patch shape...
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2] platform/x86: silead_dmi: Add touchscreen platform data for the Teclast X3 Plus tablet
From: Alberto Ponces @ 2018-01-23 18:33 UTC (permalink / raw)
To: linux-input
Cc: ponces26, Hans de Goede, Darren Hart, Andy Shevchenko,
platform-driver-x86, linux-kernel
In-Reply-To: <1516706288-177-1-git-send-email-ponces26@gmail.com>
Add touchscreen platform data for the Teclast X3 Plus tablet.
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Alberto Ponces <ponces26@gmail.com>
---
drivers/platform/x86/silead_dmi.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/platform/x86/silead_dmi.c b/drivers/platform/x86/silead_dmi.c
index 048a82f..3a62409 100644
--- a/drivers/platform/x86/silead_dmi.c
+++ b/drivers/platform/x86/silead_dmi.c
@@ -263,6 +263,20 @@ static const struct silead_ts_dmi_data teclast_x98plus2_data = {
.properties = teclast_x98plus2_props,
};
+static const struct property_entry teclast_x3_plus_props[] = {
+ PROPERTY_ENTRY_U32("touchscreen-size-x", 1980),
+ PROPERTY_ENTRY_U32("touchscreen-size-y", 1500),
+ PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-teclast-x3-plus.fw"),
+ PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
+ { }
+};
+
+static const struct silead_ts_dmi_data teclast_x3_plus_data = {
+ .acpi_name = "MSSL1680:00",
+ .properties = teclast_x3_plus_props,
+};
+
static const struct dmi_system_id silead_ts_dmi_table[] = {
{
/* CUBE iwork8 Air */
@@ -423,6 +437,15 @@ static const struct dmi_system_id silead_ts_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "X98 Plus II"),
},
},
+ {
+ /* Teclast X3 Plus */
+ .driver_data = (void *)&teclast_x3_plus_data,
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "TECLAST"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "X3 Plus"),
+ DMI_MATCH(DMI_BOARD_NAME, "X3 Plus"),
+ },
+ },
{ },
};
--
2.7.4
^ permalink raw reply related
* [PATCH] Input: edt-ft5x06 - fix error handling for factory mode on non-M06
From: Dmitry Torokhov @ 2018-01-23 18:52 UTC (permalink / raw)
To: linux-input
Cc: Simon Budig, Andi Shyti, Lothar Waßmann, Mylene Josserand,
linux-kernel
When attempting enter factory mode on firmware that does not support it,
we'd error out, but leave the device with interrupts disabled, and thus
touch not working. Fix it by moving the check before we disable
interrupts/allocate memory for debug buffers.
Fixes: fd335ab04b3f ("Input: edt-ft5x06 - add support for M09 firmware version")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/touchscreen/edt-ft5x06.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index c53a3d7239e72..1e18ca0d1b4e1 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -511,6 +511,12 @@ static int edt_ft5x06_factory_mode(struct edt_ft5x06_ts_data *tsdata)
int ret;
int error;
+ if (tsdata->version != EDT_M06) {
+ dev_err(&client->dev,
+ "No factory mode support for non-M06 devices\n");
+ return -EINVAL;
+ }
+
disable_irq(client->irq);
if (!tsdata->raw_buffer) {
@@ -524,9 +530,6 @@ static int edt_ft5x06_factory_mode(struct edt_ft5x06_ts_data *tsdata)
}
/* mode register is 0x3c when in the work mode */
- if (tsdata->version != EDT_M06)
- goto m09_out;
-
error = edt_ft5x06_register_write(tsdata, WORK_REGISTER_OPMODE, 0x03);
if (error) {
dev_err(&client->dev,
@@ -559,11 +562,6 @@ static int edt_ft5x06_factory_mode(struct edt_ft5x06_ts_data *tsdata)
enable_irq(client->irq);
return error;
-
-m09_out:
- dev_err(&client->dev, "No factory mode support for M09/M12/GENERIC_FT\n");
- return -EINVAL;
-
}
static int edt_ft5x06_work_mode(struct edt_ft5x06_ts_data *tsdata)
--
2.16.0.rc1.238.g530d649a79-goog
--
Dmitry
^ permalink raw reply related
* New input driver for PIUIO input/output board
From: Devin J. Pohly @ 2018-01-23 19:20 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov
Hi all,
I've written an input driver for a custom I/O board often found in
arcade dance machines. I have maintained it out-of-tree on my GitHub
(https://github.com/djpohly/piuio) for a number of years, and it has
been running on a handful of "production" systems for just as long.
This is my first major submission to the kernel, so I would welcome any
comments or criticisms that people may have.
Thanks!
*dp
^ permalink raw reply
* [PATCH] Input: add support for PIUIO input/output board
From: Devin J. Pohly @ 2018-01-23 19:20 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Devin J. Pohly
In-Reply-To: <20180123192029.9468-1-djpohly@gmail.com>
The PIUIO is a digital input/output board most often found in arcade
dance cabinets. It uses a polling-based USB protocol to get/set the
state of inputs and outputs, with the potential for up to 48 of each
(though actual boards have fewer physical connections).
This commit provides a driver which exposes the inputs as joystick
buttons and the outputs as LEDs.
The driver also supports a smaller form of the board with 8 inputs and
outputs which uses the same protocol.
Signed-off-by: Devin J. Pohly <djpohly@gmail.com>
---
MAINTAINERS | 6 +
drivers/input/joystick/Kconfig | 11 +
drivers/input/joystick/Makefile | 1 +
drivers/input/joystick/piuio.c | 672 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 690 insertions(+)
create mode 100644 drivers/input/joystick/piuio.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2f4e462aa4a2..a39675bff62a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10691,6 +10691,12 @@ F: arch/mips/include/asm/mach-pistachio/
F: arch/mips/boot/dts/img/pistachio*
F: arch/mips/configs/pistachio*_defconfig
+PIUIO DRIVER
+M: Devin J. Pohly <djpohly@gmail.com>
+W: https://github.com/djpohly/piuio
+S: Maintained
+F: drivers/input/joystick/piuio.c
+
PKTCDVD DRIVER
S: Orphan
M: linux-block@vger.kernel.org
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index f3c2f6ea8b44..5d156e760db8 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -351,4 +351,15 @@ config JOYSTICK_PSXPAD_SPI_FF
To drive rumble motor a dedicated power supply is required.
+config JOYSTICK_PIUIO
+ tristate "PIUIO input/output interface"
+ depends on USB_ARCH_HAS_HCD
+ select USB
+ help
+ Say Y here if you want to use the PIUIO interface board for
+ digital inputs/outputs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called piuio.
+
endif
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 67651efda2e1..33c4e54fb516 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_JOYSTICK_INTERACT) += interact.o
obj-$(CONFIG_JOYSTICK_JOYDUMP) += joydump.o
obj-$(CONFIG_JOYSTICK_MAGELLAN) += magellan.o
obj-$(CONFIG_JOYSTICK_MAPLE) += maplecontrol.o
+obj-$(CONFIG_JOYSTICK_PIUIO) += piuio.o
obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
diff --git a/drivers/input/joystick/piuio.c b/drivers/input/joystick/piuio.c
new file mode 100644
index 000000000000..e1d9b34692e2
--- /dev/null
+++ b/drivers/input/joystick/piuio.c
@@ -0,0 +1,672 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Andamiro PIU IO input module, for use with the PIUIO input/output boards
+// commonly found in arcade dance cabinets.
+//
+// Copyright (C) 2012-2018 Devin J. Pohly (djpohly@gmail.com)
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/stat.h>
+#include <linux/sysfs.h>
+#include <linux/errno.h>
+#include <linux/bitops.h>
+#include <linux/leds.h>
+#include <linux/wait.h>
+#include <linux/jiffies.h>
+#include <linux/input.h>
+#include <linux/usb.h>
+#include <linux/usb/input.h>
+
+
+/*
+ * Device and protocol definitions
+ */
+#define USB_VENDOR_ID_ANCHOR 0x0547
+#define USB_PRODUCT_ID_PYTHON2 0x1002
+#define USB_VENDOR_ID_BTNBOARD 0x0d2f
+#define USB_PRODUCT_ID_BTNBOARD 0x1010
+
+/* USB message used to communicate with the device */
+#define PIUIO_MSG_REQ 0xae
+#define PIUIO_MSG_VAL 0
+#define PIUIO_MSG_IDX 0
+
+#define PIUIO_MSG_SZ 8
+#define PIUIO_MSG_LONGS (PIUIO_MSG_SZ / sizeof(unsigned long))
+
+/* Input keycode ranges */
+#define PIUIO_BTN_REG BTN_JOYSTICK
+#define PIUIO_NUM_REG (BTN_GAMEPAD - BTN_JOYSTICK)
+#define PIUIO_BTN_EXTRA BTN_TRIGGER_HAPPY
+#define PIUIO_NUM_EXTRA (KEY_MAX - BTN_TRIGGER_HAPPY)
+#define PIUIO_NUM_BTNS (PIUIO_NUM_REG + PIUIO_NUM_EXTRA)
+
+
+#ifdef CONFIG_LEDS_CLASS
+/**
+ * struct piuio_led - auxiliary struct for led devices
+ * @piu: Pointer back to the enclosing structure
+ * @dev: Actual led device
+ */
+struct piuio_led {
+ struct piuio *piu;
+ struct led_classdev dev;
+};
+
+static const char *const led_names[] = {
+ "piuio::output0",
+ "piuio::output1",
+ "piuio::output2",
+ "piuio::output3",
+ "piuio::output4",
+ "piuio::output5",
+ "piuio::output6",
+ "piuio::output7",
+ "piuio::output8",
+ "piuio::output9",
+ "piuio::output10",
+ "piuio::output11",
+ "piuio::output12",
+ "piuio::output13",
+ "piuio::output14",
+ "piuio::output15",
+ "piuio::output16",
+ "piuio::output17",
+ "piuio::output18",
+ "piuio::output19",
+ "piuio::output20",
+ "piuio::output21",
+ "piuio::output22",
+ "piuio::output23",
+ "piuio::output24",
+ "piuio::output25",
+ "piuio::output26",
+ "piuio::output27",
+ "piuio::output28",
+ "piuio::output29",
+ "piuio::output30",
+ "piuio::output31",
+ "piuio::output32",
+ "piuio::output33",
+ "piuio::output34",
+ "piuio::output35",
+ "piuio::output36",
+ "piuio::output37",
+ "piuio::output38",
+ "piuio::output39",
+ "piuio::output40",
+ "piuio::output41",
+ "piuio::output42",
+ "piuio::output43",
+ "piuio::output44",
+ "piuio::output45",
+ "piuio::output46",
+ "piuio::output47",
+};
+
+static const char *const bbled_names[] = {
+ "piuio::bboutput0",
+ "piuio::bboutput1",
+ "piuio::bboutput2",
+ "piuio::bboutput3",
+ "piuio::bboutput4",
+ "piuio::bboutput5",
+ "piuio::bboutput6",
+ "piuio::bboutput7",
+};
+#endif
+
+/**
+ * struct piuio_devtype - parameters for different types of PIUIO devices
+ * @led_names: Array of LED names, of length @outputs, to use in sysfs
+ * @inputs: Number of input pins
+ * @outputs: Number of output pins
+ * @mplex: Number of sets of inputs
+ * @mplex_bits: Number of output bits reserved for multiplexing
+ */
+struct piuio_devtype {
+#ifdef CONFIG_LEDS_CLASS
+ const char *const *led_names;
+#endif
+ int inputs;
+ int outputs;
+ int mplex;
+ int mplex_bits;
+};
+
+/**
+ * struct piuio - state of each attached PIUIO
+ * @type: Type of PIUIO device (currently either full or buttonboard)
+ * @idev: Input device associated with this PIUIO
+ * @phys: Physical path of the device. @idev's phys field points to this
+ * buffer
+ * @udev: USB device associated with this PIUIO
+ * @in: URB for requesting the current state of one set of inputs
+ * @out: URB for sending data to outputs and multiplexer
+ * @cr_in: Setup packet for @in URB
+ * @cr_out: Setup packet for @out URB
+ * @old_inputs: Previous state of input pins from the @in URB for each of the
+ * input sets. These are used to determine when a press or release
+ * has happened for a group of correlated inputs.
+ * @inputs: Buffer for the @in URB
+ * @outputs: Buffer for the @out URB
+ * @new_outputs:
+ * Staging for the @outputs buffer
+ * @set: Current set of inputs to read (0 .. @type->mplex - 1)
+ */
+struct piuio {
+ struct piuio_devtype *type;
+
+ struct input_dev *idev;
+ char phys[64];
+
+ struct usb_device *udev;
+ struct urb *in, *out;
+ struct usb_ctrlrequest cr_in, cr_out;
+ wait_queue_head_t shutdown_wait;
+
+ unsigned long (*old_inputs)[PIUIO_MSG_LONGS];
+ unsigned long inputs[PIUIO_MSG_LONGS];
+ unsigned char outputs[PIUIO_MSG_SZ];
+ unsigned char new_outputs[PIUIO_MSG_SZ];
+
+#ifdef CONFIG_LEDS_CLASS
+ struct piuio_led *led;
+#endif
+
+ int set;
+};
+
+/* Full device parameters */
+static struct piuio_devtype piuio_dev_full = {
+#ifdef CONFIG_LEDS_CLASS
+ .led_names = led_names,
+#endif
+ .inputs = (PIUIO_NUM_BTNS < 48) ? PIUIO_NUM_BTNS : 48,
+ .outputs = 48,
+ .mplex = 4,
+ .mplex_bits = 2,
+};
+
+/* Button board device parameters */
+static struct piuio_devtype piuio_dev_bb = {
+#ifdef CONFIG_LEDS_CLASS
+ .led_names = bbled_names,
+#endif
+ .inputs = (PIUIO_NUM_BTNS < 8) ? PIUIO_NUM_BTNS : 8,
+ .outputs = 8,
+ .mplex = 1,
+ .mplex_bits = 0,
+};
+
+
+/*
+ * Auxiliary functions for reporting input events
+ */
+static int keycode(unsigned int pin)
+{
+ /* Use joystick buttons first, then the extra "trigger happy" range. */
+ if (pin < PIUIO_NUM_REG)
+ return PIUIO_BTN_REG + pin;
+ pin -= PIUIO_NUM_REG;
+ return PIUIO_BTN_EXTRA + pin;
+}
+
+
+/*
+ * URB completion handlers
+ */
+static void piuio_in_completed(struct urb *urb)
+{
+ struct piuio *piu = urb->context;
+ unsigned long changed[PIUIO_MSG_LONGS];
+ unsigned long b;
+ int i, s;
+ int cur_set;
+ int ret = urb->status;
+
+ if (ret) {
+ dev_warn(&piu->udev->dev,
+ "piuio callback(in): error %d\n", ret);
+ goto resubmit;
+ }
+
+ /* Get index of the previous input set (always 0 if no multiplexer) */
+ cur_set = (piu->set + piu->type->mplex - 1) % piu->type->mplex;
+
+ /* Note what has changed in this input set, then store the inputs for
+ * next time
+ */
+ for (i = 0; i < PIUIO_MSG_LONGS; i++) {
+ changed[i] = piu->inputs[i] ^ piu->old_inputs[cur_set][i];
+ piu->old_inputs[cur_set][i] = piu->inputs[i];
+ }
+
+ /* If we are using a multiplexer, changes only count when none of the
+ * corresponding inputs in other sets are pressed. Since "pressed"
+ * reads as 0, we can use & to knock those bits out of the changes.
+ */
+ for (s = 0; s < piu->type->mplex; s++) {
+ if (s == cur_set)
+ continue;
+ for (i = 0; i < PIUIO_MSG_LONGS; i++)
+ changed[i] &= piu->old_inputs[s][i];
+ }
+
+ /* For each input which has changed state, report whether it was pressed
+ * or released based on the current value.
+ */
+ for_each_set_bit(b, changed, piu->type->inputs) {
+ input_event(piu->idev, EV_MSC, MSC_SCAN, b + 1);
+ input_report_key(piu->idev, keycode(b),
+ !test_bit(b, piu->inputs));
+ }
+
+ /* Done reporting input events */
+ input_sync(piu->idev);
+
+resubmit:
+ ret = usb_submit_urb(urb, GFP_ATOMIC);
+ if (ret == -EPERM)
+ dev_info(&piu->udev->dev, "piuio resubmit(in): shutdown\n");
+ else if (ret)
+ dev_err(&piu->udev->dev, "piuio resubmit(in): error %d\n", ret);
+
+ /* Let any waiting threads know we're done here */
+ wake_up(&piu->shutdown_wait);
+}
+
+static void piuio_out_completed(struct urb *urb)
+{
+ struct piuio *piu = urb->context;
+ int ret = urb->status;
+
+ if (ret) {
+ dev_warn(&piu->udev->dev,
+ "piuio callback(out): error %d\n", ret);
+ goto resubmit;
+ }
+
+ /* Copy in the new outputs */
+ memcpy(piu->outputs, piu->new_outputs, PIUIO_MSG_SZ);
+
+ /* If we have a multiplexer, switch to the next input set in rotation
+ * and set the appropriate output bits
+ */
+ piu->set = (piu->set + 1) % piu->type->mplex;
+
+ /* Set multiplexer bits */
+ piu->outputs[0] &= ~((1 << piu->type->mplex_bits) - 1);
+ piu->outputs[0] |= piu->set;
+ piu->outputs[2] &= ~((1 << piu->type->mplex_bits) - 1);
+ piu->outputs[2] |= piu->set;
+
+resubmit:
+ ret = usb_submit_urb(piu->out, GFP_ATOMIC);
+ if (ret == -EPERM)
+ dev_info(&piu->udev->dev, "piuio resubmit(out): shutdown\n");
+ else if (ret)
+ dev_err(&piu->udev->dev,
+ "piuio resubmit(out): error %d\n", ret);
+
+ /* Let any waiting threads know we're done here */
+ wake_up(&piu->shutdown_wait);
+}
+
+
+/*
+ * Input device events
+ */
+static int piuio_open(struct input_dev *idev)
+{
+ struct piuio *piu = input_get_drvdata(idev);
+ int ret;
+
+ /* Kick off the polling */
+ ret = usb_submit_urb(piu->out, GFP_KERNEL);
+ if (ret) {
+ dev_err(&piu->udev->dev, "piuio submit(out): error %d\n", ret);
+ return -EIO;
+ }
+
+ ret = usb_submit_urb(piu->in, GFP_KERNEL);
+ if (ret) {
+ dev_err(&piu->udev->dev, "piuio submit(in): error %d\n", ret);
+ usb_kill_urb(piu->out);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static void piuio_close(struct input_dev *idev)
+{
+ struct piuio *piu = input_get_drvdata(idev);
+ long remaining;
+
+ /* Stop polling, but wait for the last requests to complete */
+ usb_block_urb(piu->in);
+ usb_block_urb(piu->out);
+ remaining = wait_event_timeout(piu->shutdown_wait,
+ atomic_read(&piu->in->use_count) == 0 &&
+ atomic_read(&piu->out->use_count) == 0,
+ msecs_to_jiffies(5));
+ usb_unblock_urb(piu->in);
+ usb_unblock_urb(piu->out);
+
+ if (!remaining) {
+ // Timed out
+ dev_warn(&piu->udev->dev, "piuio close: urb timeout\n");
+ usb_kill_urb(piu->in);
+ usb_kill_urb(piu->out);
+ }
+
+ /* XXX Reset the outputs? */
+}
+
+
+/*
+ * Structure initialization and destruction
+ */
+static void piuio_input_init(struct piuio *piu, struct device *parent)
+{
+ struct input_dev *idev = piu->idev;
+ int i;
+
+ /* Fill in basic fields */
+ idev->name = "PIUIO input";
+ idev->phys = piu->phys;
+ usb_to_input_id(piu->udev, &idev->id);
+ idev->dev.parent = parent;
+
+ /* HACK: Buttons are sufficient to trigger a /dev/input/js* device, but
+ * for systemd (and consequently udev and Xorg) to consider us a
+ * joystick, we have to have a set of XY absolute axes.
+ */
+ set_bit(EV_KEY, idev->evbit);
+ set_bit(EV_ABS, idev->evbit);
+
+ /* Configure buttons */
+ for (i = 0; i < piu->type->inputs; i++)
+ set_bit(keycode(i), idev->keybit);
+ clear_bit(0, idev->keybit);
+
+ /* Configure fake axes */
+ set_bit(ABS_X, idev->absbit);
+ set_bit(ABS_Y, idev->absbit);
+ input_set_abs_params(idev, ABS_X, 0, 0, 0, 0);
+ input_set_abs_params(idev, ABS_Y, 0, 0, 0, 0);
+
+ /* Set device callbacks */
+ idev->open = piuio_open;
+ idev->close = piuio_close;
+
+ /* Link input device back to PIUIO */
+ input_set_drvdata(idev, piu);
+}
+
+
+#ifdef CONFIG_LEDS_CLASS
+/*
+ * Led device event
+ */
+static void piuio_led_set(struct led_classdev *dev, enum led_brightness b)
+{
+ struct piuio_led *led = container_of(dev, struct piuio_led, dev);
+ struct piuio *piu = led->piu;
+ int n;
+
+ n = led - piu->led;
+ if (n > piu->type->outputs) {
+ dev_err(&piu->udev->dev, "piuio led: bad number %d\n", n);
+ return;
+ }
+
+ /* Meh, forget atomicity, these aren't super-important */
+ if (b)
+ __set_bit(n, (unsigned long *) piu->new_outputs);
+ else
+ __clear_bit(n, (unsigned long *) piu->new_outputs);
+}
+
+int
+piu_led_register(struct piuio_led *led)
+{
+ const struct attribute_group **ag;
+ struct attribute **attr;
+ int ret;
+
+ /* Register led device */
+ ret = led_classdev_register(&led->piu->udev->dev, &led->dev);
+ if (ret)
+ return ret;
+
+ /* Relax permissions on led attributes */
+ for (ag = led->dev.dev->class->dev_groups; *ag; ag++) {
+ for (attr = (*ag)->attrs; *attr; attr++) {
+ ret = sysfs_chmod_file(&led->dev.dev->kobj, *attr,
+ 0666);
+ if (ret) {
+ led_classdev_unregister(&led->dev);
+ return ret;
+ }
+ }
+ }
+ return 0;
+}
+
+static int piuio_leds_init(struct piuio *piu)
+{
+ int i;
+ int ret;
+
+ piu->led = kcalloc(piu->type->outputs, sizeof(*piu->led), GFP_KERNEL);
+ if (!piu->led)
+ return -ENOMEM;
+
+ for (i = 0; i < piu->type->outputs; i++) {
+ /* Initialize led device and point back to piuio struct */
+ piu->led[i].dev.name = piu->type->led_names[i];
+ piu->led[i].dev.brightness_set = piuio_led_set;
+ piu->led[i].piu = piu;
+
+ ret = piu_led_register(&piu->led[i]);
+ if (ret)
+ goto out_unregister;
+ }
+
+ return 0;
+
+out_unregister:
+ for (--i; i >= 0; i--)
+ led_classdev_unregister(&piu->led[i].dev);
+ kfree(piu->led);
+ return ret;
+}
+
+static void piuio_leds_destroy(struct piuio *piu)
+{
+ int i;
+
+ for (i = 0; i < piu->type->outputs; i++)
+ led_classdev_unregister(&piu->led[i].dev);
+ kfree(piu->led);
+}
+#else
+static int piuio_leds_init(struct piuio *piu) { return 0; }
+static void piuio_leds_destroy(struct piuio *piu) {}
+#endif
+
+static int piuio_init(struct piuio *piu, struct input_dev *idev,
+ struct usb_device *udev)
+{
+ /* Note: if this function returns an error, piuio_destroy will still be
+ * called, so we don't need to clean up here
+ */
+
+ /* Allocate USB request blocks */
+ piu->in = usb_alloc_urb(0, GFP_KERNEL);
+ piu->out = usb_alloc_urb(0, GFP_KERNEL);
+ if (!piu->in || !piu->out)
+ return -ENOMEM;
+
+ /* Create dynamically allocated arrays */
+ piu->old_inputs = kcalloc(piu->type->mplex, sizeof(*piu->old_inputs),
+ GFP_KERNEL);
+ if (!piu->old_inputs)
+ return -ENOMEM;
+
+ init_waitqueue_head(&piu->shutdown_wait);
+
+ piu->idev = idev;
+ piu->udev = udev;
+ usb_make_path(udev, piu->phys, sizeof(piu->phys));
+ strlcat(piu->phys, "/input0", sizeof(piu->phys));
+
+ /* Prepare URB for multiplexer and outputs */
+ piu->cr_out.bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE;
+ piu->cr_out.bRequest = PIUIO_MSG_REQ;
+ piu->cr_out.wValue = cpu_to_le16(PIUIO_MSG_VAL);
+ piu->cr_out.wIndex = cpu_to_le16(PIUIO_MSG_IDX);
+ piu->cr_out.wLength = cpu_to_le16(PIUIO_MSG_SZ);
+ usb_fill_control_urb(piu->out, udev, usb_sndctrlpipe(udev, 0),
+ (void *) &piu->cr_out, piu->outputs, PIUIO_MSG_SZ,
+ piuio_out_completed, piu);
+
+ /* Prepare URB for inputs */
+ piu->cr_in.bRequestType = USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE;
+ piu->cr_in.bRequest = PIUIO_MSG_REQ;
+ piu->cr_in.wValue = cpu_to_le16(PIUIO_MSG_VAL);
+ piu->cr_in.wIndex = cpu_to_le16(PIUIO_MSG_IDX);
+ piu->cr_in.wLength = cpu_to_le16(PIUIO_MSG_SZ);
+ usb_fill_control_urb(piu->in, udev, usb_rcvctrlpipe(udev, 0),
+ (void *) &piu->cr_in, piu->inputs, PIUIO_MSG_SZ,
+ piuio_in_completed, piu);
+
+ return 0;
+}
+
+static void piuio_destroy(struct piuio *piu)
+{
+ /* These handle NULL gracefully, so we can call this to clean up if init
+ * fails
+ */
+ kfree(piu->old_inputs);
+ usb_free_urb(piu->out);
+ usb_free_urb(piu->in);
+}
+
+
+/*
+ * USB connect and disconnect events
+ */
+static int piuio_probe(struct usb_interface *intf,
+ const struct usb_device_id *id)
+{
+ struct piuio *piu;
+ struct usb_device *udev = interface_to_usbdev(intf);
+ struct input_dev *idev;
+ int ret = -ENOMEM;
+
+ /* Allocate PIUIO state and determine device type */
+ piu = kzalloc(sizeof(struct piuio), GFP_KERNEL);
+ if (!piu)
+ return ret;
+
+ if (id->idVendor == USB_VENDOR_ID_BTNBOARD &&
+ id->idProduct == USB_PRODUCT_ID_BTNBOARD) {
+ /* Button board card */
+ piu->type = &piuio_dev_bb;
+ } else {
+ /* Full card */
+ piu->type = &piuio_dev_full;
+ }
+
+ /* Allocate input device for generating buttonpresses */
+ idev = input_allocate_device();
+ if (!idev) {
+ kfree(piu->old_inputs);
+ kfree(piu);
+ return ret;
+ }
+
+ /* Initialize PIUIO state and input device */
+ ret = piuio_init(piu, idev, udev);
+ if (ret)
+ goto err;
+
+ piuio_input_init(piu, &intf->dev);
+
+ /* Initialize and register led devices */
+ ret = piuio_leds_init(piu);
+ if (ret)
+ goto err;
+
+ /* Register input device */
+ ret = input_register_device(piu->idev);
+ if (ret) {
+ dev_err(&intf->dev, "piuio probe: failed to register input dev\n");
+ piuio_leds_destroy(piu);
+ goto err;
+ }
+
+ /* Final USB setup */
+ usb_set_intfdata(intf, piu);
+ return 0;
+
+err:
+ piuio_destroy(piu);
+ input_free_device(idev);
+ kfree(piu);
+ return ret;
+}
+
+static void piuio_disconnect(struct usb_interface *intf)
+{
+ struct piuio *piu = usb_get_intfdata(intf);
+
+ usb_set_intfdata(intf, NULL);
+ if (!piu) {
+ dev_err(&intf->dev, "piuio disconnect: uninitialized device?\n");
+ return;
+ }
+
+ usb_kill_urb(piu->in);
+ usb_kill_urb(piu->out);
+ piuio_leds_destroy(piu);
+ input_unregister_device(piu->idev);
+ piuio_destroy(piu);
+ kfree(piu);
+}
+
+
+/*
+ * USB driver and module definitions
+ */
+static struct usb_device_id piuio_id_table[] = {
+ /* Python WDM2 Encoder used for PIUIO boards */
+ { USB_DEVICE(USB_VENDOR_ID_ANCHOR, USB_PRODUCT_ID_PYTHON2) },
+ /* Special USB ID for button board devices */
+ { USB_DEVICE(USB_VENDOR_ID_BTNBOARD, USB_PRODUCT_ID_BTNBOARD) },
+ {},
+};
+
+MODULE_DEVICE_TABLE(usb, piuio_id_table);
+
+static struct usb_driver piuio_driver = {
+ .name = "piuio",
+ .probe = piuio_probe,
+ .disconnect = piuio_disconnect,
+ .id_table = piuio_id_table,
+};
+
+MODULE_AUTHOR("Devin J. Pohly");
+MODULE_DESCRIPTION("PIUIO input/output driver");
+MODULE_VERSION("1.0");
+MODULE_LICENSE("GPL v2");
+
+module_usb_driver(piuio_driver);
--
2.16.0
^ permalink raw reply related
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