* [PATCH v3 0/3] Add device tree support to the cyttsp driver
@ 2016-01-10 17:36 Oreste Salerno
2016-01-10 17:36 ` [PATCH v3 1/3] Input: cyttsp - remove unused irq_gpio from platform_data Oreste Salerno
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Oreste Salerno @ 2016-01-10 17:36 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA, fery-+wT8y+m8/X5BDgjK7y7TUQ,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA
This set of patches adds support to the cyttsp touchscreen driver
to retrieve platform-specific details from the device tree.
Additionally, they define a default init function that performs a hard
reset of the chip on boot.
---
Changes in v3:
- Rework binding names and descriptions
Changes in v2:
- Fix err_cast.cocci warning in patch 2/3
Oreste Salerno (3):
Input: cyttsp - remove unused irq_gpio from platform_data
Input: cyttsp - add device tree bindings
Input: cyttsp - add default init function
.../bindings/input/touchscreen/cyttsp.txt | 82 ++++++++++++
drivers/input/touchscreen/cyttsp_core.c | 146 ++++++++++++++++++++-
include/linux/input/cyttsp.h | 4 +-
3 files changed, 224 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
--
1.9.1
--
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 [flat|nested] 11+ messages in thread
* [PATCH v3 1/3] Input: cyttsp - remove unused irq_gpio from platform_data
2016-01-10 17:36 [PATCH v3 0/3] Add device tree support to the cyttsp driver Oreste Salerno
@ 2016-01-10 17:36 ` Oreste Salerno
[not found] ` <cover.1452447124.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
2016-01-10 17:36 ` [PATCH v3 3/3] Input: cyttsp - add default init function Oreste Salerno
2 siblings, 0 replies; 11+ messages in thread
From: Oreste Salerno @ 2016-01-10 17:36 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, fery, dmitry.torokhov, robh+dt, pawel.moll,
mark.rutland, ijc+devicetree, galak, devicetree, oreste.salerno
This field is not being used and the irq is anyway passed
via the 'irq' parameter to cyttsp_probe.
Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
---
include/linux/input/cyttsp.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/linux/input/cyttsp.h b/include/linux/input/cyttsp.h
index 5af7c66..d7c2520 100644
--- a/include/linux/input/cyttsp.h
+++ b/include/linux/input/cyttsp.h
@@ -51,7 +51,6 @@ struct cyttsp_platform_data {
int (*init)(void);
void (*exit)(void);
char *name;
- s16 irq_gpio;
u8 *bl_keys;
};
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/3] Input: cyttsp - add device tree bindings
[not found] ` <cover.1452447124.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
@ 2016-01-10 17:36 ` Oreste Salerno
2016-01-11 3:01 ` Rob Herring
[not found] ` <dd329fe3420fba4049c353a9a092ff9c45c5a252.1452447124.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
0 siblings, 2 replies; 11+ messages in thread
From: Oreste Salerno @ 2016-01-10 17:36 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA, fery-+wT8y+m8/X5BDgjK7y7TUQ,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA
Signed-off-by: Oreste Salerno <oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
---
.../bindings/input/touchscreen/cyttsp.txt | 82 +++++++++++++
drivers/input/touchscreen/cyttsp_core.c | 134 +++++++++++++++++++--
include/linux/input/cyttsp.h | 3 +
3 files changed, 212 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
new file mode 100644
index 0000000..2dc5c65
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
@@ -0,0 +1,82 @@
+* Cypress cyttsp touchscreen controller
+
+Required properties:
+- compatible : must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
+- reg : Device I2C address or SPI chip select number
+- spi-max-frequency : Maximum SPI clocking speed of the device (for cyttsp-spi)
+- interrupt-parent : the phandle for the gpio controller
+ (see interrupt binding[0]).
+- interrupts : (gpio) interrupt to which the chip is connected
+ (see interrupt binding[0]).
+- reset-gpios : the reset gpio the chip is connected to
+ (see GPIO binding[1] for more details).
+- touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
+- touchscreen-size-y : vertical resolution of touchscreen (in pixels)
+- bootloader-key : the 8-byte bootloader key that is required to switch
+ the chip from bootloader mode (default mode) to
+ application mode.
+ This property has to be specified as an array of 8
+ '/bits/ 8' values.
+
+Optional properties:
+- active-distance : the distance in pixels beyond which a touch must move
+ before movement is detected and reported by the device.
+ Valid values: 0-15.
+- active-interval-ms : the minimum period in ms between consecutive
+ scanning/processing cycles when the chip is in active mode.
+ Valid values: 0-255.
+- lowpower-interval-ms : the minimum period in ms between consecutive
+ scanning/processing cycles when the chip is in low-power mode.
+ Valid values: 0-2550
+- touch-timeout-ms : minimum time in ms spent in the active power state while no
+ touches are detected before entering low-power mode.
+ Valid values: 0-2550
+
+[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+[1]: Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+ &i2c1 {
+ /* ... */
+ cyttsp@a {
+ compatible = "cypress,cyttsp-i2c";
+ reg = <0xa>;
+ interrupt-parent = <&msm_gpio>;
+ interrupts = <13 0x2008>;
+ reset-gpios = <&msm_gpio 12 0x00>;
+
+ touchscreen-size-x = <800>;
+ touchscreen-size-y = <480>;
+ bootloader-key = /bits/ 8 <0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08>;
+
+ active-distance = <8>;
+ active-interval-ms = <0>;
+ lowpower-interval-ms = <200>;
+ touch-timeout-ms = <100>;
+ };
+
+ /* ... */
+ };
+
+ &mcspi1 {
+ /* ... */
+ cyttsp@0 {
+ compatible = "cypress,cyttsp-spi";
+ spi-max-frequency = <6000000>;
+ reg = <0>;
+ interrupt-parent = <&msm_gpio>;
+ interrupts = <13 0x2008>;
+ reset-gpios = <&msm_gpio 12 0x00>;
+
+ touchscreen-size-x = <800>;
+ touchscreen-size-y = <480>;
+ bootloader-key = /bits/ 8 <0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08>;
+
+ active-distance = <8>;
+ active-interval-ms = <0>;
+ lowpower-interval-ms = <200>;
+ touch-timeout-ms = <100>;
+ };
+
+ /* ... */
+ };
diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
index 5b74e8b..5dc6bf6 100644
--- a/drivers/input/touchscreen/cyttsp_core.c
+++ b/drivers/input/touchscreen/cyttsp_core.c
@@ -33,6 +33,8 @@
#include <linux/gpio.h>
#include <linux/interrupt.h>
#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/gpio/consumer.h>
#include "cyttsp_core.h"
@@ -57,6 +59,7 @@
#define CY_DELAY_DFLT 20 /* ms */
#define CY_DELAY_MAX 500
#define CY_ACT_DIST_DFLT 0xF8
+#define CY_ACT_DIST_MASK 0x0F
#define CY_HNDSHK_BIT 0x80
/* device mode bits */
#define CY_OPERATE_MODE 0x00
@@ -528,18 +531,136 @@ static void cyttsp_close(struct input_dev *dev)
cyttsp_disable(ts);
}
+#ifdef CONFIG_OF
+static const struct cyttsp_platform_data *cyttsp_parse_dt(struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ struct cyttsp_platform_data *pdata;
+ u32 dt_value;
+ int ret;
+ static const char err_msg[] =
+ "property not provided in the device tree";
+
+ if (!np)
+ return ERR_PTR(-ENOENT);
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return ERR_PTR(-ENOMEM);
+
+ pdata->bl_keys = devm_kzalloc(dev, CY_NUM_BL_KEYS, GFP_KERNEL);
+ if (!pdata->bl_keys)
+ return ERR_PTR(-ENOMEM);
+
+ /* Set some default values */
+ pdata->act_dist = CY_ACT_DIST_DFLT;
+ pdata->act_intrvl = CY_ACT_INTRVL_DFLT;
+ pdata->tch_tmout = CY_TCH_TMOUT_DFLT;
+ pdata->lp_intrvl = CY_LP_INTRVL_DFLT;
+ pdata->name = "cyttsp";
+
+ ret = of_property_read_u32(np, "touchscreen-size-x", &pdata->maxx);
+ if (ret) {
+ dev_err(dev, "touchscreen-size-x %s\n", err_msg);
+ return ERR_PTR(ret);
+ }
+
+ ret = of_property_read_u32(np, "touchscreen-size-y", &pdata->maxy);
+ if (ret) {
+ dev_err(dev, "touchscreen-size-y %s\n", err_msg);
+ return ERR_PTR(ret);
+ }
+
+ pdata->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(pdata->reset_gpio)) {
+ ret = PTR_ERR(pdata->reset_gpio);
+ dev_err(dev, "error acquiring reset gpio: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+
+ ret = of_property_read_u8_array(np, "bootloader-key",
+ pdata->bl_keys, CY_NUM_BL_KEYS);
+ if (ret) {
+ dev_err(dev, "bootloader-key %s\n", err_msg);
+ return ERR_PTR(ret);
+ }
+
+ if (!of_property_read_u32(np, "active-distance", &dt_value)) {
+ if (dt_value > 15) {
+ dev_err(dev, "active-distance (%u) must be [0-15]\n",
+ dt_value);
+ return ERR_PTR(-EINVAL);
+ }
+ pdata->act_dist &= ~CY_ACT_DIST_MASK;
+ pdata->act_dist |= (u8)dt_value;
+ }
+
+ if (!of_property_read_u32(np, "active-interval-ms", &dt_value)) {
+ if (dt_value > 255) {
+ dev_err(dev, "active-interval-ms (%u) must be [0-255]\n",
+ dt_value);
+ return ERR_PTR(-EINVAL);
+ }
+ pdata->act_intrvl = (u8)dt_value;
+ }
+
+ if (!of_property_read_u32(np, "lowpower-interval-ms", &dt_value)) {
+ if (dt_value > 2550) {
+ dev_err(dev, "lowpower-interval-ms (%u) must be [0-2550]\n",
+ dt_value);
+ return ERR_PTR(-EINVAL);
+ }
+ /* Register value is expressed in 0.01s / bit */
+ pdata->lp_intrvl = (u8)(dt_value/10);
+ }
+
+ if (!of_property_read_u32(np, "touch-timeout-ms", &dt_value)) {
+ if (dt_value > 2550) {
+ dev_err(dev, "touch-timeout-ms (%u) must be [0-2550]\n",
+ dt_value);
+ return ERR_PTR(-EINVAL);
+ }
+ /* Register value is expressed in 0.01s / bit */
+ pdata->tch_tmout = (u8)(dt_value/10);
+ }
+
+ return pdata;
+}
+#else
+static const struct cyttsp_platform_data *cyttsp_parse_dt(struct device *dev)
+{
+ return ERR_PTR(-ENOENT);
+}
+#endif
+
+static const struct cyttsp_platform_data *
+cyttsp_get_platform_data(struct device *dev)
+{
+ const struct cyttsp_platform_data *pdata;
+
+ pdata = dev_get_platdata(dev);
+ if (pdata)
+ return pdata;
+
+ pdata = cyttsp_parse_dt(dev);
+ if (!IS_ERR(pdata) || PTR_ERR(pdata) != -ENOENT)
+ return pdata;
+
+ dev_err(dev, "No platform data specified\n");
+ return ERR_PTR(-EINVAL);
+}
+
struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
struct device *dev, int irq, size_t xfer_buf_size)
{
- const struct cyttsp_platform_data *pdata = dev_get_platdata(dev);
+ const struct cyttsp_platform_data *pdata;
struct cyttsp *ts;
struct input_dev *input_dev;
int error;
- if (!pdata || !pdata->name || irq <= 0) {
- error = -EINVAL;
- goto err_out;
- }
+ pdata = cyttsp_get_platform_data(dev);
+ if (IS_ERR(pdata))
+ return ERR_CAST(pdata);
ts = kzalloc(sizeof(*ts) + xfer_buf_size, GFP_KERNEL);
input_dev = input_allocate_device();
@@ -550,7 +671,7 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
ts->dev = dev;
ts->input = input_dev;
- ts->pdata = dev_get_platdata(dev);
+ ts->pdata = pdata;
ts->bus_ops = bus_ops;
ts->irq = irq;
@@ -618,7 +739,6 @@ err_platform_exit:
err_free_mem:
input_free_device(input_dev);
kfree(ts);
-err_out:
return ERR_PTR(error);
}
EXPORT_SYMBOL_GPL(cyttsp_probe);
diff --git a/include/linux/input/cyttsp.h b/include/linux/input/cyttsp.h
index d7c2520..92a9d52 100644
--- a/include/linux/input/cyttsp.h
+++ b/include/linux/input/cyttsp.h
@@ -29,6 +29,8 @@
#ifndef _CYTTSP_H_
#define _CYTTSP_H_
+#include <linux/gpio/consumer.h>
+
#define CY_SPI_NAME "cyttsp-spi"
#define CY_I2C_NAME "cyttsp-i2c"
/* Active Power state scanning/processing refresh interval */
@@ -51,6 +53,7 @@ struct cyttsp_platform_data {
int (*init)(void);
void (*exit)(void);
char *name;
+ struct gpio_desc *reset_gpio;
u8 *bl_keys;
};
--
1.9.1
--
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 related [flat|nested] 11+ messages in thread
* [PATCH v3 3/3] Input: cyttsp - add default init function
2016-01-10 17:36 [PATCH v3 0/3] Add device tree support to the cyttsp driver Oreste Salerno
2016-01-10 17:36 ` [PATCH v3 1/3] Input: cyttsp - remove unused irq_gpio from platform_data Oreste Salerno
[not found] ` <cover.1452447124.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
@ 2016-01-10 17:36 ` Oreste Salerno
[not found] ` <01aa2661875444ecc771dfce1f466b6846ac5eea.1452447124.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
2 siblings, 1 reply; 11+ messages in thread
From: Oreste Salerno @ 2016-01-10 17:36 UTC (permalink / raw)
To: linux-kernel
Cc: linux-input, fery, dmitry.torokhov, robh+dt, pawel.moll,
mark.rutland, ijc+devicetree, galak, devicetree, oreste.salerno
When using device trees, it's not possible to assign an init()
function in the platform data.
In case the init function has not been assigned, attempt to
perform a hard reset of the chip, using reset gpio defined in the
platform data / device tree.
Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
---
drivers/input/touchscreen/cyttsp_core.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
index 5dc6bf6..baf0fd5 100644
--- a/drivers/input/touchscreen/cyttsp_core.c
+++ b/drivers/input/touchscreen/cyttsp_core.c
@@ -239,6 +239,16 @@ static int cyttsp_set_sysinfo_regs(struct cyttsp *ts)
return retval;
}
+static void cyttsp_hard_reset(struct cyttsp *ts)
+{
+ if (ts->pdata->reset_gpio) {
+ gpiod_set_value_cansleep(ts->pdata->reset_gpio, 1);
+ msleep(CY_DELAY_DFLT);
+ gpiod_set_value_cansleep(ts->pdata->reset_gpio, 0);
+ msleep(CY_DELAY_DFLT);
+ }
+}
+
static int cyttsp_soft_reset(struct cyttsp *ts)
{
unsigned long timeout;
@@ -685,6 +695,8 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
error);
goto err_free_mem;
}
+ } else {
+ cyttsp_hard_reset(ts);
}
input_dev->name = pdata->name;
--
1.9.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings
2016-01-10 17:36 ` [PATCH v3 2/3] Input: cyttsp - add device tree bindings Oreste Salerno
@ 2016-01-11 3:01 ` Rob Herring
2016-01-11 17:54 ` Oreste Salerno
[not found] ` <dd329fe3420fba4049c353a9a092ff9c45c5a252.1452447124.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
1 sibling, 1 reply; 11+ messages in thread
From: Rob Herring @ 2016-01-11 3:01 UTC (permalink / raw)
To: Oreste Salerno
Cc: linux-kernel, linux-input, fery, dmitry.torokhov, pawel.moll,
mark.rutland, ijc+devicetree, galak, devicetree
On Sun, Jan 10, 2016 at 06:36:08PM +0100, Oreste Salerno wrote:
> Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
> ---
> .../bindings/input/touchscreen/cyttsp.txt | 82 +++++++++++++
> drivers/input/touchscreen/cyttsp_core.c | 134 +++++++++++++++++++--
> include/linux/input/cyttsp.h | 3 +
> 3 files changed, 212 insertions(+), 7 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> new file mode 100644
> index 0000000..2dc5c65
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> @@ -0,0 +1,82 @@
> +* Cypress cyttsp touchscreen controller
> +
> +Required properties:
> +- compatible : must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
> +- reg : Device I2C address or SPI chip select number
> +- spi-max-frequency : Maximum SPI clocking speed of the device (for cyttsp-spi)
> +- interrupt-parent : the phandle for the gpio controller
> + (see interrupt binding[0]).
> +- interrupts : (gpio) interrupt to which the chip is connected
> + (see interrupt binding[0]).
> +- reset-gpios : the reset gpio the chip is connected to
> + (see GPIO binding[1] for more details).
> +- touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
> +- touchscreen-size-y : vertical resolution of touchscreen (in pixels)
> +- bootloader-key : the 8-byte bootloader key that is required to switch
> + the chip from bootloader mode (default mode) to
> + application mode.
> + This property has to be specified as an array of 8
> + '/bits/ 8' values.
> +
> +Optional properties:
> +- active-distance : the distance in pixels beyond which a touch must move
> + before movement is detected and reported by the device.
> + Valid values: 0-15.
Wouldn't this be the same as touchscreen-fuzz-(x|y) which is the
replacement for the deprecated moving-threshold?
> +- active-interval-ms : the minimum period in ms between consecutive
> + scanning/processing cycles when the chip is in active mode.
> + Valid values: 0-255.
> +- lowpower-interval-ms : the minimum period in ms between consecutive
> + scanning/processing cycles when the chip is in low-power mode.
> + Valid values: 0-2550
> +- touch-timeout-ms : minimum time in ms spent in the active power state while no
> + touches are detected before entering low-power mode.
> + Valid values: 0-2550
> +
Otherwise, looks fine.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings
2016-01-11 3:01 ` Rob Herring
@ 2016-01-11 17:54 ` Oreste Salerno
2016-01-11 18:45 ` Dmitry Torokhov
2016-01-11 23:51 ` Rob Herring
0 siblings, 2 replies; 11+ messages in thread
From: Oreste Salerno @ 2016-01-11 17:54 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel, linux-input, fery, dmitry.torokhov, pawel.moll,
mark.rutland, ijc+devicetree, galak, devicetree
On Sun, Jan 10, 2016 at 09:01:56PM -0600, Rob Herring wrote:
> On Sun, Jan 10, 2016 at 06:36:08PM +0100, Oreste Salerno wrote:
> > Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
> > ---
> > .../bindings/input/touchscreen/cyttsp.txt | 82 +++++++++++++
> > drivers/input/touchscreen/cyttsp_core.c | 134 +++++++++++++++++++--
> > include/linux/input/cyttsp.h | 3 +
> > 3 files changed, 212 insertions(+), 7 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > new file mode 100644
> > index 0000000..2dc5c65
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > @@ -0,0 +1,82 @@
> > +* Cypress cyttsp touchscreen controller
> > +
> > +Required properties:
> > +- compatible : must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
> > +- reg : Device I2C address or SPI chip select number
> > +- spi-max-frequency : Maximum SPI clocking speed of the device (for cyttsp-spi)
> > +- interrupt-parent : the phandle for the gpio controller
> > + (see interrupt binding[0]).
> > +- interrupts : (gpio) interrupt to which the chip is connected
> > + (see interrupt binding[0]).
> > +- reset-gpios : the reset gpio the chip is connected to
> > + (see GPIO binding[1] for more details).
> > +- touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
> > +- touchscreen-size-y : vertical resolution of touchscreen (in pixels)
> > +- bootloader-key : the 8-byte bootloader key that is required to switch
> > + the chip from bootloader mode (default mode) to
> > + application mode.
> > + This property has to be specified as an array of 8
> > + '/bits/ 8' values.
> > +
> > +Optional properties:
> > +- active-distance : the distance in pixels beyond which a touch must move
> > + before movement is detected and reported by the device.
> > + Valid values: 0-15.
>
> Wouldn't this be the same as touchscreen-fuzz-(x|y) which is the
> replacement for the deprecated moving-threshold?
>
It is somewhat related. But I understand that the touchscreen-fuzz-(x|y) properties
are meant to be used as parameters passed to input_set_abs_params(), which are used
for a SW-based jitter filtering.
The active-distance value instead is used to set a HW register in the chip which
effectively performs the filtering in the HW.
The driver is currently passing 0 as fuzz parameter to input_set_abs_params().
> > +- active-interval-ms : the minimum period in ms between consecutive
> > + scanning/processing cycles when the chip is in active mode.
> > + Valid values: 0-255.
> > +- lowpower-interval-ms : the minimum period in ms between consecutive
> > + scanning/processing cycles when the chip is in low-power mode.
> > + Valid values: 0-2550
> > +- touch-timeout-ms : minimum time in ms spent in the active power state while no
> > + touches are detected before entering low-power mode.
> > + Valid values: 0-2550
> > +
>
> Otherwise, looks fine.
I am still waiting for a review from the driver maintainer (Ferruh Yigit), but I see
that he hasn't been actively reviewing patches for the cyttsp* drivers for a while.
I wonder what would be the process in this case?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings
[not found] ` <dd329fe3420fba4049c353a9a092ff9c45c5a252.1452447124.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
@ 2016-01-11 18:30 ` Dmitry Torokhov
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2016-01-11 18:30 UTC (permalink / raw)
To: Oreste Salerno
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, fery-+wT8y+m8/X5BDgjK7y7TUQ,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA
Hi Oreste,
On Sun, Jan 10, 2016 at 06:36:08PM +0100, Oreste Salerno wrote:
> Signed-off-by: Oreste Salerno <oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
> ---
> .../bindings/input/touchscreen/cyttsp.txt | 82 +++++++++++++
> drivers/input/touchscreen/cyttsp_core.c | 134 +++++++++++++++++++--
> include/linux/input/cyttsp.h | 3 +
> 3 files changed, 212 insertions(+), 7 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> new file mode 100644
> index 0000000..2dc5c65
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> @@ -0,0 +1,82 @@
> +* Cypress cyttsp touchscreen controller
> +
> +Required properties:
> +- compatible : must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
> +- reg : Device I2C address or SPI chip select number
> +- spi-max-frequency : Maximum SPI clocking speed of the device (for cyttsp-spi)
Stray space before tab after spi-max-frequency.
> +- interrupt-parent : the phandle for the gpio controller
> + (see interrupt binding[0]).
> +- interrupts : (gpio) interrupt to which the chip is connected
> + (see interrupt binding[0]).
> +- reset-gpios : the reset gpio the chip is connected to
> + (see GPIO binding[1] for more details).
> +- touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
> +- touchscreen-size-y : vertical resolution of touchscreen (in pixels)
Let's direct users to common touchscreen properties.
> +- bootloader-key : the 8-byte bootloader key that is required to switch
> + the chip from bootloader mode (default mode) to
> + application mode.
> + This property has to be specified as an array of 8
> + '/bits/ 8' values.
> +
> +Optional properties:
> +- active-distance : the distance in pixels beyond which a touch must move
> + before movement is detected and reported by the device.
> + Valid values: 0-15.
> +- active-interval-ms : the minimum period in ms between consecutive
> + scanning/processing cycles when the chip is in active mode.
> + Valid values: 0-255.
> +- lowpower-interval-ms : the minimum period in ms between consecutive
> + scanning/processing cycles when the chip is in low-power mode.
> + Valid values: 0-2550
> +- touch-timeout-ms : minimum time in ms spent in the active power state while no
> + touches are detected before entering low-power mode.
> + Valid values: 0-2550
> +
> +[0]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> +[1]: Documentation/devicetree/bindings/gpio/gpio.txt
> +
> +Example:
> + &i2c1 {
> + /* ... */
> + cyttsp@a {
> + compatible = "cypress,cyttsp-i2c";
> + reg = <0xa>;
> + interrupt-parent = <&msm_gpio>;
> + interrupts = <13 0x2008>;
> + reset-gpios = <&msm_gpio 12 0x00>;
> +
> + touchscreen-size-x = <800>;
> + touchscreen-size-y = <480>;
> + bootloader-key = /bits/ 8 <0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08>;
> +
> + active-distance = <8>;
> + active-interval-ms = <0>;
> + lowpower-interval-ms = <200>;
> + touch-timeout-ms = <100>;
> + };
> +
> + /* ... */
> + };
> +
> + &mcspi1 {
> + /* ... */
> + cyttsp@0 {
> + compatible = "cypress,cyttsp-spi";
> + spi-max-frequency = <6000000>;
> + reg = <0>;
> + interrupt-parent = <&msm_gpio>;
> + interrupts = <13 0x2008>;
> + reset-gpios = <&msm_gpio 12 0x00>;
> +
> + touchscreen-size-x = <800>;
> + touchscreen-size-y = <480>;
> + bootloader-key = /bits/ 8 <0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08>;
> +
> + active-distance = <8>;
> + active-interval-ms = <0>;
> + lowpower-interval-ms = <200>;
> + touch-timeout-ms = <100>;
> + };
> +
> + /* ... */
> + };
> diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
> index 5b74e8b..5dc6bf6 100644
> --- a/drivers/input/touchscreen/cyttsp_core.c
> +++ b/drivers/input/touchscreen/cyttsp_core.c
> @@ -33,6 +33,8 @@
> #include <linux/gpio.h>
> #include <linux/interrupt.h>
> #include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/gpio/consumer.h>
>
> #include "cyttsp_core.h"
>
> @@ -57,6 +59,7 @@
> #define CY_DELAY_DFLT 20 /* ms */
> #define CY_DELAY_MAX 500
> #define CY_ACT_DIST_DFLT 0xF8
> +#define CY_ACT_DIST_MASK 0x0F
> #define CY_HNDSHK_BIT 0x80
> /* device mode bits */
> #define CY_OPERATE_MODE 0x00
> @@ -528,18 +531,136 @@ static void cyttsp_close(struct input_dev *dev)
> cyttsp_disable(ts);
> }
>
> +#ifdef CONFIG_OF
> +static const struct cyttsp_platform_data *cyttsp_parse_dt(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct cyttsp_platform_data *pdata;
> + u32 dt_value;
> + int ret;
> + static const char err_msg[] =
> + "property not provided in the device tree";
> +
> + if (!np)
> + return ERR_PTR(-ENOENT);
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return ERR_PTR(-ENOMEM);
> +
> + pdata->bl_keys = devm_kzalloc(dev, CY_NUM_BL_KEYS, GFP_KERNEL);
> + if (!pdata->bl_keys)
> + return ERR_PTR(-ENOMEM);
> +
> + /* Set some default values */
> + pdata->act_dist = CY_ACT_DIST_DFLT;
> + pdata->act_intrvl = CY_ACT_INTRVL_DFLT;
> + pdata->tch_tmout = CY_TCH_TMOUT_DFLT;
> + pdata->lp_intrvl = CY_LP_INTRVL_DFLT;
> + pdata->name = "cyttsp";
> +
> + ret = of_property_read_u32(np, "touchscreen-size-x", &pdata->maxx);
> + if (ret) {
> + dev_err(dev, "touchscreen-size-x %s\n", err_msg);
> + return ERR_PTR(ret);
> + }
> +
> + ret = of_property_read_u32(np, "touchscreen-size-y", &pdata->maxy);
> + if (ret) {
> + dev_err(dev, "touchscreen-size-y %s\n", err_msg);
> + return ERR_PTR(ret);
> + }
I'd rather we called touchscreen_parse_properties() in common code when
setting up the input device. It should work for variety of platform
providers (OF, ACPI, etc).
> +
> + pdata->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(pdata->reset_gpio)) {
> + ret = PTR_ERR(pdata->reset_gpio);
> + dev_err(dev, "error acquiring reset gpio: %d\n", ret);
> + return ERR_PTR(ret);
> + }
Please call it directly in probe(). Again, it should work fine not only
on OF, but on all other platforms, so
> +
> + ret = of_property_read_u8_array(np, "bootloader-key",
> + pdata->bl_keys, CY_NUM_BL_KEYS);
> + if (ret) {
> + dev_err(dev, "bootloader-key %s\n", err_msg);
> + return ERR_PTR(ret);
> + }
> +
> + if (!of_property_read_u32(np, "active-distance", &dt_value)) {
> + if (dt_value > 15) {
> + dev_err(dev, "active-distance (%u) must be [0-15]\n",
> + dt_value);
> + return ERR_PTR(-EINVAL);
> + }
> + pdata->act_dist &= ~CY_ACT_DIST_MASK;
> + pdata->act_dist |= (u8)dt_value;
I do not think you need to cast.
> + }
> +
> + if (!of_property_read_u32(np, "active-interval-ms", &dt_value)) {
> + if (dt_value > 255) {
> + dev_err(dev, "active-interval-ms (%u) must be [0-255]\n",
> + dt_value);
> + return ERR_PTR(-EINVAL);
> + }
> + pdata->act_intrvl = (u8)dt_value;
I do not think you need to cast.
> + }
> +
> + if (!of_property_read_u32(np, "lowpower-interval-ms", &dt_value)) {
> + if (dt_value > 2550) {
> + dev_err(dev, "lowpower-interval-ms (%u) must be [0-2550]\n",
> + dt_value);
> + return ERR_PTR(-EINVAL);
> + }
> + /* Register value is expressed in 0.01s / bit */
> + pdata->lp_intrvl = (u8)(dt_value/10);
Spaces around operations please. Do we need explicit cast here?
> + }
> +
> + if (!of_property_read_u32(np, "touch-timeout-ms", &dt_value)) {
> + if (dt_value > 2550) {
> + dev_err(dev, "touch-timeout-ms (%u) must be [0-2550]\n",
> + dt_value);
> + return ERR_PTR(-EINVAL);
> + }
> + /* Register value is expressed in 0.01s / bit */
> + pdata->tch_tmout = (u8)(dt_value/10);
Spaces around operations please. Do we need explicit cast here?
> + }
> +
> + return pdata;
> +}
> +#else
> +static const struct cyttsp_platform_data *cyttsp_parse_dt(struct device *dev)
> +{
> + return ERR_PTR(-ENOENT);
> +}
> +#endif
> +
> +static const struct cyttsp_platform_data *
> +cyttsp_get_platform_data(struct device *dev)
> +{
> + const struct cyttsp_platform_data *pdata;
> +
> + pdata = dev_get_platdata(dev);
> + if (pdata)
> + return pdata;
> +
> + pdata = cyttsp_parse_dt(dev);
> + if (!IS_ERR(pdata) || PTR_ERR(pdata) != -ENOENT)
> + return pdata;
> +
> + dev_err(dev, "No platform data specified\n");
> + return ERR_PTR(-EINVAL);
> +}
> +
> struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
> struct device *dev, int irq, size_t xfer_buf_size)
> {
> - const struct cyttsp_platform_data *pdata = dev_get_platdata(dev);
> + const struct cyttsp_platform_data *pdata;
> struct cyttsp *ts;
> struct input_dev *input_dev;
> int error;
>
> - if (!pdata || !pdata->name || irq <= 0) {
> - error = -EINVAL;
> - goto err_out;
> - }
> + pdata = cyttsp_get_platform_data(dev);
> + if (IS_ERR(pdata))
> + return ERR_CAST(pdata);
>
> ts = kzalloc(sizeof(*ts) + xfer_buf_size, GFP_KERNEL);
> input_dev = input_allocate_device();
> @@ -550,7 +671,7 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
>
> ts->dev = dev;
> ts->input = input_dev;
> - ts->pdata = dev_get_platdata(dev);
> + ts->pdata = pdata;
> ts->bus_ops = bus_ops;
> ts->irq = irq;
>
> @@ -618,7 +739,6 @@ err_platform_exit:
> err_free_mem:
> input_free_device(input_dev);
> kfree(ts);
> -err_out:
> return ERR_PTR(error);
> }
> EXPORT_SYMBOL_GPL(cyttsp_probe);
> diff --git a/include/linux/input/cyttsp.h b/include/linux/input/cyttsp.h
> index d7c2520..92a9d52 100644
> --- a/include/linux/input/cyttsp.h
> +++ b/include/linux/input/cyttsp.h
> @@ -29,6 +29,8 @@
> #ifndef _CYTTSP_H_
> #define _CYTTSP_H_
>
> +#include <linux/gpio/consumer.h>
> +
> #define CY_SPI_NAME "cyttsp-spi"
> #define CY_I2C_NAME "cyttsp-i2c"
> /* Active Power state scanning/processing refresh interval */
> @@ -51,6 +53,7 @@ struct cyttsp_platform_data {
> int (*init)(void);
> void (*exit)(void);
> char *name;
> + struct gpio_desc *reset_gpio;
No, we shoudl not be passing gpio descriptor in platform data. For
non-of non-ACPI system gpiod_* framework allows boards to specify lookup
tables so they can use that. Just drop it form here.
> u8 *bl_keys;
> };
>
> --
> 1.9.1
>
Thanks.
--
Dmitry
--
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 [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings
2016-01-11 17:54 ` Oreste Salerno
@ 2016-01-11 18:45 ` Dmitry Torokhov
2016-01-11 19:01 ` Oreste Salerno
2016-01-11 23:51 ` Rob Herring
1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Torokhov @ 2016-01-11 18:45 UTC (permalink / raw)
To: Oreste Salerno
Cc: Rob Herring, linux-kernel, linux-input, fery, pawel.moll,
mark.rutland, ijc+devicetree, galak, devicetree
On Mon, Jan 11, 2016 at 06:54:23PM +0100, Oreste Salerno wrote:
> On Sun, Jan 10, 2016 at 09:01:56PM -0600, Rob Herring wrote:
> > On Sun, Jan 10, 2016 at 06:36:08PM +0100, Oreste Salerno wrote:
> > > Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
> > > ---
> > > .../bindings/input/touchscreen/cyttsp.txt | 82 +++++++++++++
> > > drivers/input/touchscreen/cyttsp_core.c | 134 +++++++++++++++++++--
> > > include/linux/input/cyttsp.h | 3 +
> > > 3 files changed, 212 insertions(+), 7 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > > new file mode 100644
> > > index 0000000..2dc5c65
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > > @@ -0,0 +1,82 @@
> > > +* Cypress cyttsp touchscreen controller
> > > +
> > > +Required properties:
> > > +- compatible : must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
> > > +- reg : Device I2C address or SPI chip select number
> > > +- spi-max-frequency : Maximum SPI clocking speed of the device (for cyttsp-spi)
> > > +- interrupt-parent : the phandle for the gpio controller
> > > + (see interrupt binding[0]).
> > > +- interrupts : (gpio) interrupt to which the chip is connected
> > > + (see interrupt binding[0]).
> > > +- reset-gpios : the reset gpio the chip is connected to
> > > + (see GPIO binding[1] for more details).
> > > +- touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
> > > +- touchscreen-size-y : vertical resolution of touchscreen (in pixels)
> > > +- bootloader-key : the 8-byte bootloader key that is required to switch
> > > + the chip from bootloader mode (default mode) to
> > > + application mode.
> > > + This property has to be specified as an array of 8
> > > + '/bits/ 8' values.
> > > +
> > > +Optional properties:
> > > +- active-distance : the distance in pixels beyond which a touch must move
> > > + before movement is detected and reported by the device.
> > > + Valid values: 0-15.
> >
> > Wouldn't this be the same as touchscreen-fuzz-(x|y) which is the
> > replacement for the deprecated moving-threshold?
> >
>
> It is somewhat related. But I understand that the touchscreen-fuzz-(x|y) properties
> are meant to be used as parameters passed to input_set_abs_params(), which are used
> for a SW-based jitter filtering.
> The active-distance value instead is used to set a HW register in the chip which
> effectively performs the filtering in the HW.
> The driver is currently passing 0 as fuzz parameter to input_set_abs_params().
It doe snot have to and it will not if you use
touchscreen_parse_properties(). But you are right, fuzz is reserved for
software de-jittering, and it can be changed at any time from userspace
at input device level, but will not be communicated to the hardware. I
think hardware-based filtering is hardware-specific and I think
having a separate property is fine for it. We can still standardize on
name, units, etc though.
>
> > > +- active-interval-ms : the minimum period in ms between consecutive
> > > + scanning/processing cycles when the chip is in active mode.
> > > + Valid values: 0-255.
> > > +- lowpower-interval-ms : the minimum period in ms between consecutive
> > > + scanning/processing cycles when the chip is in low-power mode.
> > > + Valid values: 0-2550
> > > +- touch-timeout-ms : minimum time in ms spent in the active power state while no
> > > + touches are detected before entering low-power mode.
> > > + Valid values: 0-2550
> > > +
> >
> > Otherwise, looks fine.
>
> I am still waiting for a review from the driver maintainer (Ferruh Yigit), but I see
> that he hasn't been actively reviewing patches for the cyttsp* drivers for a while.
> I wonder what would be the process in this case?
Please address the items from my other email and I should be able to
pick it up.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings
2016-01-11 18:45 ` Dmitry Torokhov
@ 2016-01-11 19:01 ` Oreste Salerno
0 siblings, 0 replies; 11+ messages in thread
From: Oreste Salerno @ 2016-01-11 19:01 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, linux-kernel, linux-input, fery, pawel.moll,
mark.rutland, ijc+devicetree, galak, devicetree
On Mon, Jan 11, 2016 at 10:45:50AM -0800, Dmitry Torokhov wrote:
> On Mon, Jan 11, 2016 at 06:54:23PM +0100, Oreste Salerno wrote:
> > On Sun, Jan 10, 2016 at 09:01:56PM -0600, Rob Herring wrote:
> > > On Sun, Jan 10, 2016 at 06:36:08PM +0100, Oreste Salerno wrote:
> > > > Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
> > > > ---
> > > > .../bindings/input/touchscreen/cyttsp.txt | 82 +++++++++++++
> > > > drivers/input/touchscreen/cyttsp_core.c | 134 +++++++++++++++++++--
> > > > include/linux/input/cyttsp.h | 3 +
> > > > 3 files changed, 212 insertions(+), 7 deletions(-)
> > > > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > > > new file mode 100644
> > > > index 0000000..2dc5c65
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
> > > > @@ -0,0 +1,82 @@
> > > > +* Cypress cyttsp touchscreen controller
> > > > +
> > > > +Required properties:
> > > > +- compatible : must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
> > > > +- reg : Device I2C address or SPI chip select number
> > > > +- spi-max-frequency : Maximum SPI clocking speed of the device (for cyttsp-spi)
> > > > +- interrupt-parent : the phandle for the gpio controller
> > > > + (see interrupt binding[0]).
> > > > +- interrupts : (gpio) interrupt to which the chip is connected
> > > > + (see interrupt binding[0]).
> > > > +- reset-gpios : the reset gpio the chip is connected to
> > > > + (see GPIO binding[1] for more details).
> > > > +- touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
> > > > +- touchscreen-size-y : vertical resolution of touchscreen (in pixels)
> > > > +- bootloader-key : the 8-byte bootloader key that is required to switch
> > > > + the chip from bootloader mode (default mode) to
> > > > + application mode.
> > > > + This property has to be specified as an array of 8
> > > > + '/bits/ 8' values.
> > > > +
> > > > +Optional properties:
> > > > +- active-distance : the distance in pixels beyond which a touch must move
> > > > + before movement is detected and reported by the device.
> > > > + Valid values: 0-15.
> > >
> > > Wouldn't this be the same as touchscreen-fuzz-(x|y) which is the
> > > replacement for the deprecated moving-threshold?
> > >
> >
> > It is somewhat related. But I understand that the touchscreen-fuzz-(x|y) properties
> > are meant to be used as parameters passed to input_set_abs_params(), which are used
> > for a SW-based jitter filtering.
> > The active-distance value instead is used to set a HW register in the chip which
> > effectively performs the filtering in the HW.
> > The driver is currently passing 0 as fuzz parameter to input_set_abs_params().
>
> It doe snot have to and it will not if you use
> touchscreen_parse_properties(). But you are right, fuzz is reserved for
> software de-jittering, and it can be changed at any time from userspace
> at input device level, but will not be communicated to the hardware. I
> think hardware-based filtering is hardware-specific and I think
> having a separate property is fine for it. We can still standardize on
> name, units, etc though.
>
Yes, I looked at touchscreen_parse_properties and I was planning to use it as part
of a future improvement patch. But it makes sense to introduce it in this patch already.
> >
> > > > +- active-interval-ms : the minimum period in ms between consecutive
> > > > + scanning/processing cycles when the chip is in active mode.
> > > > + Valid values: 0-255.
> > > > +- lowpower-interval-ms : the minimum period in ms between consecutive
> > > > + scanning/processing cycles when the chip is in low-power mode.
> > > > + Valid values: 0-2550
> > > > +- touch-timeout-ms : minimum time in ms spent in the active power state while no
> > > > + touches are detected before entering low-power mode.
> > > > + Valid values: 0-2550
> > > > +
> > >
> > > Otherwise, looks fine.
> >
> > I am still waiting for a review from the driver maintainer (Ferruh Yigit), but I see
> > that he hasn't been actively reviewing patches for the cyttsp* drivers for a while.
> > I wonder what would be the process in this case?
>
> Please address the items from my other email and I should be able to
> pick it up.
>
OK, I'll try to rework the patch soon and post a new version.
Thanks a lot for your review!
> Thanks.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/3] Input: cyttsp - add default init function
[not found] ` <01aa2661875444ecc771dfce1f466b6846ac5eea.1452447124.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
@ 2016-01-11 20:02 ` Dmitry Torokhov
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Torokhov @ 2016-01-11 20:02 UTC (permalink / raw)
To: Oreste Salerno
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, fery-+wT8y+m8/X5BDgjK7y7TUQ,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA
On Sun, Jan 10, 2016 at 06:36:09PM +0100, Oreste Salerno wrote:
> When using device trees, it's not possible to assign an init()
> function in the platform data.
> In case the init function has not been assigned, attempt to
> perform a hard reset of the chip, using reset gpio defined in the
> platform data / device tree.
Hmm, I do not see any users of cyttsp platform data in mainline, so
let's kill init/exit and just do GPIO toggle for everyone. We might need
to add regulator support as well, but that's when someone will need it.
Thanks.
>
> Signed-off-by: Oreste Salerno <oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/input/touchscreen/cyttsp_core.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/input/touchscreen/cyttsp_core.c b/drivers/input/touchscreen/cyttsp_core.c
> index 5dc6bf6..baf0fd5 100644
> --- a/drivers/input/touchscreen/cyttsp_core.c
> +++ b/drivers/input/touchscreen/cyttsp_core.c
> @@ -239,6 +239,16 @@ static int cyttsp_set_sysinfo_regs(struct cyttsp *ts)
> return retval;
> }
>
> +static void cyttsp_hard_reset(struct cyttsp *ts)
> +{
> + if (ts->pdata->reset_gpio) {
> + gpiod_set_value_cansleep(ts->pdata->reset_gpio, 1);
> + msleep(CY_DELAY_DFLT);
> + gpiod_set_value_cansleep(ts->pdata->reset_gpio, 0);
> + msleep(CY_DELAY_DFLT);
> + }
> +}
> +
> static int cyttsp_soft_reset(struct cyttsp *ts)
> {
> unsigned long timeout;
> @@ -685,6 +695,8 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
> error);
> goto err_free_mem;
> }
> + } else {
> + cyttsp_hard_reset(ts);
> }
>
> input_dev->name = pdata->name;
> --
> 1.9.1
>
--
Dmitry
--
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 [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/3] Input: cyttsp - add device tree bindings
2016-01-11 17:54 ` Oreste Salerno
2016-01-11 18:45 ` Dmitry Torokhov
@ 2016-01-11 23:51 ` Rob Herring
1 sibling, 0 replies; 11+ messages in thread
From: Rob Herring @ 2016-01-11 23:51 UTC (permalink / raw)
To: Oreste Salerno
Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, fery,
Dmitry Torokhov, Pawel Moll, Mark Rutland, Ian Campbell,
Kumar Gala, devicetree@vger.kernel.org
On Mon, Jan 11, 2016 at 11:54 AM, Oreste Salerno <oreste.salerno@tomtom.com> wrote:
> On Sun, Jan 10, 2016 at 09:01:56PM -0600, Rob Herring wrote:
>> On Sun, Jan 10, 2016 at 06:36:08PM +0100, Oreste Salerno wrote:
>> > Signed-off-by: Oreste Salerno <oreste.salerno@tomtom.com>
>> > ---
>> > .../bindings/input/touchscreen/cyttsp.txt | 82 +++++++++++++
>> > drivers/input/touchscreen/cyttsp_core.c | 134 +++++++++++++++++++--
>> > include/linux/input/cyttsp.h | 3 +
>> > 3 files changed, 212 insertions(+), 7 deletions(-)
>> > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
>> > new file mode 100644
>> > index 0000000..2dc5c65
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/input/touchscreen/cyttsp.txt
>> > @@ -0,0 +1,82 @@
>> > +* Cypress cyttsp touchscreen controller
>> > +
>> > +Required properties:
>> > +- compatible : must be "cypress,cyttsp-i2c" or "cypress,cyttsp-spi"
>> > +- reg : Device I2C address or SPI chip select number
>> > +- spi-max-frequency : Maximum SPI clocking speed of the device (for cyttsp-spi)
>> > +- interrupt-parent : the phandle for the gpio controller
>> > + (see interrupt binding[0]).
>> > +- interrupts : (gpio) interrupt to which the chip is connected
>> > + (see interrupt binding[0]).
>> > +- reset-gpios : the reset gpio the chip is connected to
>> > + (see GPIO binding[1] for more details).
>> > +- touchscreen-size-x : horizontal resolution of touchscreen (in pixels)
>> > +- touchscreen-size-y : vertical resolution of touchscreen (in pixels)
>> > +- bootloader-key : the 8-byte bootloader key that is required to switch
>> > + the chip from bootloader mode (default mode) to
>> > + application mode.
>> > + This property has to be specified as an array of 8
>> > + '/bits/ 8' values.
>> > +
>> > +Optional properties:
>> > +- active-distance : the distance in pixels beyond which a touch must move
>> > + before movement is detected and reported by the device.
>> > + Valid values: 0-15.
>>
>> Wouldn't this be the same as touchscreen-fuzz-(x|y) which is the
>> replacement for the deprecated moving-threshold?
>>
>
> It is somewhat related. But I understand that the touchscreen-fuzz-(x|y) properties
> are meant to be used as parameters passed to input_set_abs_params(), which are used
> for a SW-based jitter filtering.
> The active-distance value instead is used to set a HW register in the chip which
> effectively performs the filtering in the HW.
> The driver is currently passing 0 as fuzz parameter to input_set_abs_params().
>
>> > +- active-interval-ms : the minimum period in ms between consecutive
>> > + scanning/processing cycles when the chip is in active mode.
>> > + Valid values: 0-255.
>> > +- lowpower-interval-ms : the minimum period in ms between consecutive
>> > + scanning/processing cycles when the chip is in low-power mode.
>> > + Valid values: 0-2550
>> > +- touch-timeout-ms : minimum time in ms spent in the active power state while no
>> > + touches are detected before entering low-power mode.
>> > + Valid values: 0-2550
>> > +
>>
>> Otherwise, looks fine.
>
> I am still waiting for a review from the driver maintainer (Ferruh Yigit), but I see
> that he hasn't been actively reviewing patches for the cyttsp* drivers for a while.
> I wonder what would be the process in this case?
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-01-11 23:51 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-10 17:36 [PATCH v3 0/3] Add device tree support to the cyttsp driver Oreste Salerno
2016-01-10 17:36 ` [PATCH v3 1/3] Input: cyttsp - remove unused irq_gpio from platform_data Oreste Salerno
[not found] ` <cover.1452447124.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
2016-01-10 17:36 ` [PATCH v3 2/3] Input: cyttsp - add device tree bindings Oreste Salerno
2016-01-11 3:01 ` Rob Herring
2016-01-11 17:54 ` Oreste Salerno
2016-01-11 18:45 ` Dmitry Torokhov
2016-01-11 19:01 ` Oreste Salerno
2016-01-11 23:51 ` Rob Herring
[not found] ` <dd329fe3420fba4049c353a9a092ff9c45c5a252.1452447124.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
2016-01-11 18:30 ` Dmitry Torokhov
2016-01-10 17:36 ` [PATCH v3 3/3] Input: cyttsp - add default init function Oreste Salerno
[not found] ` <01aa2661875444ecc771dfce1f466b6846ac5eea.1452447124.git.oreste.salerno-Jdzig1fPfSTQT0dZR+AlfA@public.gmane.org>
2016-01-11 20:02 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).