[PATCHv4][ 1/4] Input: tsc2007: Add device tree support.

[PATCHv4][ 1/4] Input: tsc2007: Add device tree support.

[Denis Carikli]

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Lothar Waßmann <LW@KARO-electronics.de>
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v4:
- The x-plate-ohms property was renamed to ti,x-plate-ohms.
- multilines strings were avoided.
- shorten the message related to the lack of ti,x-plate-ohms in the dts.
- whitespace fix in tsc2007_get_pendown_state_dt(struct tsc2007 *ts)
- (ts->gpio < 0) replaced by !gpio_is_valid(ts->gpio)
- devm_kzalloc is now used instead of kzalloc, and some minor code change was
  made because of that.
---
 .../bindings/input/touchscreen/tsc2007.txt         |   44 +++++
 drivers/input/touchscreen/tsc2007.c                |  201 +++++++++++++++-----
 2 files changed, 200 insertions(+), 45 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
new file mode 100644
index 0000000..fadd3f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
@@ -0,0 +1,44 @@
+* Texas Instruments tsc2007 touchscreen controller
+
+Required properties:
+- compatible: must be "ti,tsc2007".
+- reg: I2C address of the chip.
+- pinctrl-0: Should specify pin control groups used for this controller
+  (see pinctrl bindings[0]).
+- pinctrl-names: Should contain only one value - "default"
+  (see pinctrl bindings[0]).
+- interrupt-parent: the phandle for the interrupt controller
+  (see interrupt binding[1]).
+- interrupts: interrupt to which the chip is connected
+  (see interrupt binding[1]).
+- ti,x-plate-ohms: X-plate resistance in ohms.
+
+Optional properties:
+- gpios: the interrupt gpio the chip is connected to (trough the penirq pin)
+  (see GPIO binding[2] for more details).
+- max-rt: maximum pressure.
+- fuzzy: specifies the fuzz value that is used to filter noise from the event
+  stream.
+- poll-period: how much time to wait(in millisecond) before reading again the
+  values from the tsc2007.
+
+[0]: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+[1]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+[2]: Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+	&i2c1 {
+		/* ... */
+		tsc2007@49 {
+			compatible = "ti,tsc2007";
+			reg = <0x49>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_tsc2007_1>;
+			interrupt-parent = <&gpio4>;
+			interrupts = <0x0 0x8>;
+			gpios = <&gpio4 0 0>;
+			ti,x-plate-ohms = <180>;
+		};
+
+		/* ... */
+	};
diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index 0b67ba4..3143ebc 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -26,6 +26,9 @@
 #include <linux/interrupt.h>
 #include <linux/i2c.h>
 #include <linux/i2c/tsc2007.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 
 #define TSC2007_MEASURE_TEMP0		(0x0 << 4)
 #define TSC2007_MEASURE_AUX		(0x2 << 4)
@@ -74,7 +77,10 @@ struct tsc2007 {
 	u16			max_rt;
 	unsigned long		poll_delay;
 	unsigned long		poll_period;
+	int			fuzzy;
+	char			of;
 
+	unsigned		gpio;
 	int			irq;
 
 	wait_queue_head_t	wait;
@@ -84,6 +90,14 @@ struct tsc2007 {
 	void			(*clear_penirq)(void);
 };
 
+static int tsc2007_get_pendown_state_dt(struct tsc2007 *ts)
+{
+	if (gpio_is_valid(ts->gpio))
+		return !gpio_get_value(ts->gpio);
+	else
+		return true;
+}
+
 static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
 {
 	s32 data;
@@ -158,6 +172,9 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
 	 * to fall back on the pressure reading.
 	 */
 
+	if (ts->of)
+		return tsc2007_get_pendown_state_dt(ts);
+
 	if (!ts->get_pendown_state)
 		return true;
 
@@ -175,10 +192,10 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 
 		/* pen is down, continue with the measurement */
 		tsc2007_read_values(ts, &tc);
-
 		rt = tsc2007_calculate_pressure(ts, &tc);
 
-		if (rt == 0 && !ts->get_pendown_state) {
+		if ((ts->of && rt == 0 && !gpio_is_valid(ts->gpio)) ||
+			(!ts->of && rt == 0 && !ts->get_pendown_state)) {
 			/*
 			 * If pressure reported is 0 and we don't have
 			 * callback to check pendown state, we have to
@@ -198,7 +215,6 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 			input_report_abs(input, ABS_PRESSURE, rt);
 
 			input_sync(input);
-
 		} else {
 			/*
 			 * Sample found inconsistent by debouncing or pressure is
@@ -217,7 +233,6 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 	input_report_key(input, BTN_TOUCH, 0);
 	input_report_abs(input, ABS_PRESSURE, 0);
 	input_sync(input);
-
 	if (ts->clear_penirq)
 		ts->clear_penirq();
 
@@ -228,11 +243,17 @@ static irqreturn_t tsc2007_hard_irq(int irq, void *handle)
 {
 	struct tsc2007 *ts = handle;
 
-	if (!ts->get_pendown_state || likely(ts->get_pendown_state()))
-		return IRQ_WAKE_THREAD;
+	if (!ts->of) {
+		if (!ts->get_pendown_state || likely(ts->get_pendown_state()))
+			return IRQ_WAKE_THREAD;
 
-	if (ts->clear_penirq)
-		ts->clear_penirq();
+		if (ts->clear_penirq)
+			ts->clear_penirq();
+	} else {
+		if ((!gpio_is_valid(ts->gpio)) ||
+		    likely(tsc2007_get_pendown_state_dt(ts)))
+			return IRQ_WAKE_THREAD;
+	}
 
 	return IRQ_HANDLED;
 }
@@ -273,34 +294,65 @@ static void tsc2007_close(struct input_dev *input_dev)
 	tsc2007_stop(ts);
 }
 
-static int tsc2007_probe(struct i2c_client *client,
-				   const struct i2c_device_id *id)
+#ifdef CONFIG_OF
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
+			    struct device_node *np)
 {
-	struct tsc2007 *ts;
-	struct tsc2007_platform_data *pdata = client->dev.platform_data;
-	struct input_dev *input_dev;
-	int err;
-
-	if (!pdata) {
-		dev_err(&client->dev, "platform data is required!\n");
+	int err = 0;
+	u32 val32;
+	u64 val64;
+
+	if (!of_property_read_u32(np, "max-rt", &val32))
+		ts->max_rt = val32;
+	else
+		ts->max_rt = MAX_12BIT;
+
+	if (!of_property_read_u32(np, "fuzzy", &val32))
+		ts->fuzzy = val32;
+
+	if (!of_property_read_u64(np, "poll-period", &val64))
+		ts->poll_period = val64;
+	else
+		ts->poll_period = 1;
+
+	if (!of_property_read_u32(np, "ti,x-plate-ohms", &val32)) {
+		ts->x_plate_ohms = val32;
+	} else {
+		dev_err(&client->dev,
+			"Error: lacking ti,x-plate-ohms devicetree property. (err %d).",
+			err);
 		return -EINVAL;
 	}
 
-	if (!i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_READ_WORD_DATA))
-		return -EIO;
+	ts->gpio = of_get_gpio(np, 0);
+	if (!gpio_is_valid(ts->gpio))
+		dev_err(&client->dev,
+			"GPIO not found (of_get_gpio returned %d)\n",
+			ts->gpio);
 
-	ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
-	input_dev = input_allocate_device();
-	if (!ts || !input_dev) {
-		err = -ENOMEM;
-		goto err_free_mem;
-	}
+	/* Used to detect if it is probed trough the device tree,
+	 * in order to be able to use that information in the IRQ handler.
+	 */
+	ts->of = 1;
 
-	ts->client = client;
-	ts->irq = client->irq;
-	ts->input = input_dev;
-	init_waitqueue_head(&ts->wait);
+	return 0;
+}
+#else
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
+			    struct device_node *np)
+{
+	return -ENODEV;
+}
+#endif
+
+static int tsc2007_probe_pdev(struct i2c_client *client, struct tsc2007 *ts,
+			      struct tsc2007_platform_data *pdata,
+			      const struct i2c_device_id *id)
+{
+	if (!pdata) {
+		dev_err(&client->dev, "platform data is required!\n");
+		return -EINVAL;
+	}
 
 	ts->model             = pdata->model;
 	ts->x_plate_ohms      = pdata->x_plate_ohms;
@@ -309,13 +361,57 @@ static int tsc2007_probe(struct i2c_client *client,
 	ts->poll_period       = pdata->poll_period ? : 1;
 	ts->get_pendown_state = pdata->get_pendown_state;
 	ts->clear_penirq      = pdata->clear_penirq;
+	ts->fuzzy             = pdata->fuzzy;
 
 	if (pdata->x_plate_ohms == 0) {
 		dev_err(&client->dev, "x_plate_ohms is not set up in platform data");
-		err = -EINVAL;
-		goto err_free_mem;
+		return -EINVAL;
 	}
 
+	/* Used to detect if it is probed trough the device tree,
+	 * in order to be able to use that information in the IRQ handler.
+	 */
+	ts->of = 0;
+
+	return 0;
+}
+
+static int tsc2007_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct device_node *np = client->dev.of_node;
+	struct tsc2007_platform_data *pdata = client->dev.platform_data;
+	struct tsc2007 *ts;
+	struct input_dev *input_dev;
+	int err = 0;
+
+	ts = devm_kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	if (np)
+		err = tsc2007_probe_dt(client, ts, np);
+	else
+		err = tsc2007_probe_pdev(client, ts, pdata, id);
+
+	if (err)
+		return err;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_WORD_DATA))
+		return -EIO;
+
+	input_dev = input_allocate_device();
+	if (!input_dev) {
+		err = -ENOMEM;
+		goto err_free_input;
+	};
+
+	ts->client = client;
+	ts->irq = client->irq;
+	ts->input = input_dev;
+	init_waitqueue_head(&ts->wait);
+
 	snprintf(ts->phys, sizeof(ts->phys),
 		 "%s/input0", dev_name(&client->dev));
 
@@ -331,19 +427,21 @@ static int tsc2007_probe(struct i2c_client *client,
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
 	input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
 
-	input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, pdata->fuzzx, 0);
-	input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, pdata->fuzzy, 0);
+	input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, ts->fuzzy, 0);
+	input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, ts->fuzzy, 0);
 	input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT,
-			pdata->fuzzz, 0);
+			     ts->fuzzy, 0);
 
-	if (pdata->init_platform_hw)
-		pdata->init_platform_hw();
+	if (!np) {
+		if (pdata->init_platform_hw)
+			pdata->init_platform_hw();
+	}
 
 	err = request_threaded_irq(ts->irq, tsc2007_hard_irq, tsc2007_soft_irq,
 				   IRQF_ONESHOT, client->dev.driver->name, ts);
 	if (err < 0) {
 		dev_err(&client->dev, "irq %d busy?\n", ts->irq);
-		goto err_free_mem;
+		goto err_free_input;
 	}
 
 	tsc2007_stop(ts);
@@ -358,23 +456,27 @@ static int tsc2007_probe(struct i2c_client *client,
 
  err_free_irq:
 	free_irq(ts->irq, ts);
-	if (pdata->exit_platform_hw)
-		pdata->exit_platform_hw();
- err_free_mem:
+	if (!np) {
+		if (pdata->exit_platform_hw)
+			pdata->exit_platform_hw();
+	}
+ err_free_input:
 	input_free_device(input_dev);
-	kfree(ts);
 	return err;
 }
 
 static int tsc2007_remove(struct i2c_client *client)
 {
+	struct device_node *np = client->dev.of_node;
 	struct tsc2007	*ts = i2c_get_clientdata(client);
 	struct tsc2007_platform_data *pdata = client->dev.platform_data;
 
 	free_irq(ts->irq, ts);
 
-	if (pdata->exit_platform_hw)
-		pdata->exit_platform_hw();
+	if (!np) {
+		if (pdata->exit_platform_hw)
+			pdata->exit_platform_hw();
+	}
 
 	input_unregister_device(ts->input);
 	kfree(ts);
@@ -389,10 +491,19 @@ static const struct i2c_device_id tsc2007_idtable[] = {
 
 MODULE_DEVICE_TABLE(i2c, tsc2007_idtable);
 
+#ifdef CONFIG_OF
+static const struct of_device_id tsc2007_of_match[] = {
+	{ .compatible = "ti,tsc2007" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tsc2007_of_match);
+#endif
+
 static struct i2c_driver tsc2007_driver = {
 	.driver = {
 		.owner	= THIS_MODULE,
-		.name	= "tsc2007"
+		.name	= "tsc2007",
+		.of_match_table = of_match_ptr(tsc2007_of_match),
 	},
 	.id_table	= tsc2007_idtable,
 	.probe		= tsc2007_probe,
-- 
1.7.9.5

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

[PATCHv4][ 2/4] ARM: dts: cpuimx51 Add touchscreen support.

[Denis Carikli]

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Lothar Waßmann <LW@KARO-electronics.de>
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v4:
- This patch was splitted, only the part enabling the audio was kept.
  Part of the rest has already been submited, and the remaining parts
  will be submited later, because they depend on changes touching other
  subsystems.
- The x-plate-ohms property was renamed to ti,x-plate-ohms.
- The tsc2007 label was added for consistency.
---
 arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
index 8638656..34ca8d3a 100644
--- a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
+++ b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
@@ -42,6 +42,17 @@
 		compatible = "nxp,pcf8563";
 		reg = <0x51>;
 	};
+
+	tsc2007: tsc2007@49 {
+		compatible = "ti,tsc2007";
+		reg = <0x49>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_tsc2007_1>;
+		interrupt-parent = <&gpio4>;
+		interrupts = <0x0 0x8>;
+		gpios = <&gpio4 0 0>;
+		ti,x-plate-ohms = <180>;
+	};
 };
 
 &iomuxc {
-- 
1.7.9.5

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

[PATCHv4][ 3/4] ARM: dts: cpuimx35 Add touchscreen support.

[Denis Carikli]

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Lothar Waßmann <LW@KARO-electronics.de>
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v4:
- This patch was splitted, only the part enabling the audio was kept.
  Part of the rest has already been submited, and the remaining parts
  will be submited later, because they depend on changes touching other
  subsystems.
- The x-plate-ohms property was renamed to ti,x-plate-ohms.
---
 arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
index 2c2d4cd..a22230b 100644
--- a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
+++ b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
@@ -41,6 +41,17 @@
 		compatible = "nxp,pcf8563";
 		reg = <0x51>;
 	};
+
+	tsc2007: tsc2007@48 {
+		compatible = "ti,tsc2007";
+		reg = <0x48>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_tsc2007_1>;
+		interrupt-parent = <&gpio3>;
+		interrupts = <0x2 0x8>;
+		gpios = <&gpio3 2 0>;
+		ti,x-plate-ohms = <180>;
+	};
 };
 
 &iomuxc {
-- 
1.7.9.5


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv4][ 4/4] ARM: imx_v6_v7_defconfig: Enable tsc2007 support.

[Denis Carikli]

The eukrea cpuimx35 and cpuimx51 have a tsc2007 touchscreen controller,
  so we turn it on.

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Lothar Waßmann <LW@KARO-electronics.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v2->v4:
- This patch was splitted, only the part enabling the audio was kept.
  Part of the rest has already been submited, and the remaining parts
  will be submited later, because they depend on changes touching other
  subsystems.
---
 arch/arm/configs/imx_v6_v7_defconfig |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index d8a52a0..23ba17d 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -130,6 +130,7 @@ CONFIG_MOUSE_PS2_ELANTECH=y
 CONFIG_INPUT_TOUCHSCREEN=y
 CONFIG_TOUCHSCREEN_EGALAX=y
 CONFIG_TOUCHSCREEN_MC13783=y
+CONFIG_TOUCHSCREEN_TSC2007=y
 CONFIG_INPUT_MISC=y
 CONFIG_INPUT_MMA8450=y
 CONFIG_SERIO_SERPORT=m
-- 
1.7.9.5

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

Re: [PATCHv4][ 1/4] Input: tsc2007: Add device tree support.

[Dmitry Torokhov]

Hi Denis,

On Mon, Oct 21, 2013 at 03:54:24PM +0200, Denis Carikli wrote:
>  
> +	if (ts->of)
> +		return tsc2007_get_pendown_state_dt(ts);
> +
>  	if (!ts->get_pendown_state)
>  		return true;

Instead of special casing "if (ts->of)" all over the place why don't you
set up the device structure as:

	if (<configuring_tsc2007_form_dt>)
		ts->get_pendown_state = tsc2007_get_pendown_state_dt;

and be done with it?

Thanks.

-- 
Dmitry

Re: [PATCHv4][ 1/4] Input: tsc2007: Add device tree support.

[Lothar Waßmann]

Hi,

> On Mon, Oct 21, 2013 at 03:54:24PM +0200, Denis Carikli wrote:
> >  
> > +	if (ts->of)
> > +		return tsc2007_get_pendown_state_dt(ts);
> > +
> >  	if (!ts->get_pendown_state)
> >  		return true;
> 
> Instead of special casing "if (ts->of)" all over the place why don't you
> set up the device structure as:
> 
> 	if (<configuring_tsc2007_form_dt>)
> 		ts->get_pendown_state = tsc2007_get_pendown_state_dt;
> 
> and be done with it?
>
I also thought about that, but the existing function does not have any
parameters, while the DT version of get_pendown_state() requires to get
the GPIO passed to it somehow.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

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

Re: [PATCHv4][ 1/4] Input: tsc2007: Add device tree support.

[Dmitry Torokhov]

On Tue, Oct 22, 2013 at 11:49:47AM +0200, Lothar Waßmann wrote:
> Hi,
> 
> > On Mon, Oct 21, 2013 at 03:54:24PM +0200, Denis Carikli wrote:
> > >  
> > > +	if (ts->of)
> > > +		return tsc2007_get_pendown_state_dt(ts);
> > > +
> > >  	if (!ts->get_pendown_state)
> > >  		return true;
> > 
> > Instead of special casing "if (ts->of)" all over the place why don't you
> > set up the device structure as:
> > 
> > 	if (<configuring_tsc2007_form_dt>)
> > 		ts->get_pendown_state = tsc2007_get_pendown_state_dt;
> > 
> > and be done with it?
> >
> I also thought about that, but the existing function does not have any
> parameters, while the DT version of get_pendown_state() requires to get
> the GPIO passed to it somehow.

You can always have tsc2007_get_pendown_state_platform() wrapping the
call. Or we just go and fix board code.

Thanks.

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

Re: [PATCHv4][ 1/4] Input: tsc2007: Add device tree support.

[Lothar Waßmann]

Hi,

> On Tue, Oct 22, 2013 at 11:49:47AM +0200, Lothar Waßmann wrote:
> > Hi,
> > 
> > > On Mon, Oct 21, 2013 at 03:54:24PM +0200, Denis Carikli wrote:
> > > >  
> > > > +	if (ts->of)
> > > > +		return tsc2007_get_pendown_state_dt(ts);
> > > > +
> > > >  	if (!ts->get_pendown_state)
> > > >  		return true;
> > > 
> > > Instead of special casing "if (ts->of)" all over the place why don't you
> > > set up the device structure as:
> > > 
> > > 	if (<configuring_tsc2007_form_dt>)
> > > 		ts->get_pendown_state = tsc2007_get_pendown_state_dt;
> > > 
> > > and be done with it?
> > >
> > I also thought about that, but the existing function does not have any
> > parameters, while the DT version of get_pendown_state() requires to get
> > the GPIO passed to it somehow.
> 
> You can always have tsc2007_get_pendown_state_platform() wrapping the
>
Yes, but how would you pass the GPIO number to the
get_pendown_state_dt() function? Global variables are just ugly.

> call. Or we just go and fix board code.
>
That's IMO a better option, but not easy to achieve without breaking
anything. The in-kernel platforms would be easy to fix, but there may be
external users that still use the old hook, so you can't just remove it
or change its semantics.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

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

Re: [PATCHv4][ 1/4] Input: tsc2007: Add device tree support.

[Dmitry Torokhov]

On Wed, Oct 23, 2013 at 09:25:53AM +0200, Lothar Waßmann wrote:
> Hi,
> 
> > On Tue, Oct 22, 2013 at 11:49:47AM +0200, Lothar Waßmann wrote:
> > > Hi,
> > > 
> > > > On Mon, Oct 21, 2013 at 03:54:24PM +0200, Denis Carikli wrote:
> > > > >  
> > > > > +	if (ts->of)
> > > > > +		return tsc2007_get_pendown_state_dt(ts);
> > > > > +
> > > > >  	if (!ts->get_pendown_state)
> > > > >  		return true;
> > > > 
> > > > Instead of special casing "if (ts->of)" all over the place why don't you
> > > > set up the device structure as:
> > > > 
> > > > 	if (<configuring_tsc2007_form_dt>)
> > > > 		ts->get_pendown_state = tsc2007_get_pendown_state_dt;
> > > > 
> > > > and be done with it?
> > > >
> > > I also thought about that, but the existing function does not have any
> > > parameters, while the DT version of get_pendown_state() requires to get
> > > the GPIO passed to it somehow.
> > 
> > You can always have tsc2007_get_pendown_state_platform() wrapping the
> >
> Yes, but how would you pass the GPIO number to the
> get_pendown_state_dt() function? Global variables are just ugly.

You'd have

	get_pendown_state_dt(struct tsc *)
	get_pendown_state_platform(struct tsc *)
	{
		return ts->get_platform_pendown_state();
	}

and youd'd have

struct tsc {
	...
	bool (*get_pendown_state)(struct tsc);
	bool (*get_platform_pendown_state)(void);

> 
> > call. Or we just go and fix board code.
> >
> That's IMO a better option, but not easy to achieve without breaking
> anything. The in-kernel platforms would be easy to fix, but there may be
> external users that still use the old hook, so you can't just remove it
> or change its semantics.

Sure can - they are out of tree after all.

Thanks.

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

Re: [PATCHv4][ 1/4] Input: tsc2007: Add device tree support.

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1586 bytes --]

On Tue, Oct 22, 2013 at 03:35:05PM -0700, Dmitry Torokhov wrote:
> On Tue, Oct 22, 2013 at 11:49:47AM +0200, Lothar Waßmann wrote:
> > Hi,
> > 
> > > On Mon, Oct 21, 2013 at 03:54:24PM +0200, Denis Carikli wrote:
> > > >  
> > > > +	if (ts->of)
> > > > +		return tsc2007_get_pendown_state_dt(ts);
> > > > +
> > > >  	if (!ts->get_pendown_state)
> > > >  		return true;
> > > 
> > > Instead of special casing "if (ts->of)" all over the place why don't you
> > > set up the device structure as:
> > > 
> > > 	if (<configuring_tsc2007_form_dt>)
> > > 		ts->get_pendown_state = tsc2007_get_pendown_state_dt;
> > > 
> > > and be done with it?
> > >
> > I also thought about that, but the existing function does not have any
> > parameters, while the DT version of get_pendown_state() requires to get
> > the GPIO passed to it somehow.
> 
> You can always have tsc2007_get_pendown_state_platform() wrapping the
> call. Or we just go and fix board code.

I used to have a patch which did exactly that but never got around to
submitting it. Essentially what it did was add a void * parameter to the
.get_pendown_state() and .clear_penirq() callbacks, along with a new
.callback_data field that the driver could set. At the same time there
was some code to unify code for boards that merely use a simple GPIO as
pendown.

I'm attaching what I think is the latest version. I no longer have
access to the hardware so I can't test this, but perhaps it can serve as
an example of how this could work. Sorry this isn't in the form of a
proper patch.

Thierry

[-- Attachment #1.2: tsc2007.patch --]
[-- Type: text/x-diff, Size: 10216 bytes --]

diff --git a/arch/arm/mach-imx/mach-cpuimx35.c b/arch/arm/mach-imx/mach-cpuimx35.c
index d49b0ec..0dd8381 100644
--- a/arch/arm/mach-imx/mach-cpuimx35.c
+++ b/arch/arm/mach-imx/mach-cpuimx35.c
@@ -62,6 +62,7 @@ static int tsc2007_get_pendown_state(void)
 static struct tsc2007_platform_data tsc2007_info = {
 	.model			= 2007,
 	.x_plate_ohms		= 180,
+	.pendown_gpio		= -1,
 	.get_pendown_state = tsc2007_get_pendown_state,
 };
 
diff --git a/arch/arm/mach-imx/mach-cpuimx51sd.c b/arch/arm/mach-imx/mach-cpuimx51sd.c
index b87cc49..ef2a7e6 100644
--- a/arch/arm/mach-imx/mach-cpuimx51sd.c
+++ b/arch/arm/mach-imx/mach-cpuimx51sd.c
@@ -134,6 +134,7 @@ static int tsc2007_get_pendown_state(void)
 static struct tsc2007_platform_data tsc2007_info = {
 	.model			= 2007,
 	.x_plate_ohms		= 180,
+	.pendown_gpio		= -1,
 	.get_pendown_state	= tsc2007_get_pendown_state,
 };
 
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index b85957a..69abf30 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -1199,6 +1199,7 @@ static int ts_init(void)
 static struct tsc2007_platform_data tsc2007_info = {
 	.model			= 2007,
 	.x_plate_ohms		= 180,
+	.pendown_gpio		= -1,
 	.get_pendown_state	= ts_get_pendown_state,
 	.init_platform_hw	= ts_init,
 };
diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index 64559e8a..6cfd0ef 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -517,6 +517,7 @@ static int ts_init(void)
 static struct tsc2007_platform_data tsc2007_info = {
 	.model			= 2007,
 	.x_plate_ohms		= 180,
+	.pendown_gpio		= -1,
 	.get_pendown_state	= ts_get_pendown_state,
 	.init_platform_hw	= ts_init,
 };
diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index 1473d23..c87fdac 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -20,10 +20,15 @@
  *  published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) "tsc2007: " fmt
+
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/input.h>
 #include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/of_gpio.h>
+#include <linux/of_irq.h>
 #include <linux/i2c.h>
 #include <linux/i2c/tsc2007.h>
 
@@ -75,13 +80,16 @@ struct tsc2007 {
 	unsigned long		poll_delay;
 	unsigned long		poll_period;
 
+	int			pendown_gpio;
+	int			active_low;
 	int			irq;
 
 	wait_queue_head_t	wait;
 	bool			stopped;
 
-	int			(*get_pendown_state)(void);
-	void			(*clear_penirq)(void);
+	void			*callback_data;
+	int			(*get_pendown_state)(void *data);
+	void			(*clear_penirq)(void *data);
 };
 
 static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
@@ -161,7 +169,7 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
 	if (!ts->get_pendown_state)
 		return true;
 
-	return ts->get_pendown_state();
+	return ts->get_pendown_state(ts->callback_data);
 }
 
 static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
@@ -171,6 +179,13 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 	struct ts_event tc;
 	u32 rt;
 
+	/*
+	 * With some panels we need to wait a bit otherwise the first value
+	 * is often wrong.
+	 */
+	if (ts->poll_delay > 0)
+		msleep(ts->poll_delay);
+
 	while (!ts->stopped && tsc2007_is_pen_down(ts)) {
 
 		/* pen is down, continue with the measurement */
@@ -219,7 +234,7 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 	input_sync(input);
 
 	if (ts->clear_penirq)
-		ts->clear_penirq();
+		ts->clear_penirq(ts->callback_data);
 
 	return IRQ_HANDLED;
 }
@@ -228,11 +243,11 @@ static irqreturn_t tsc2007_hard_irq(int irq, void *handle)
 {
 	struct tsc2007 *ts = handle;
 
-	if (!ts->get_pendown_state || likely(ts->get_pendown_state()))
+	if (!ts->get_pendown_state || likely(ts->get_pendown_state(ts->callback_data)))
 		return IRQ_WAKE_THREAD;
 
 	if (ts->clear_penirq)
-		ts->clear_penirq();
+		ts->clear_penirq(ts->callback_data);
 
 	return IRQ_HANDLED;
 }
@@ -273,6 +288,75 @@ static void tsc2007_close(struct input_dev *input_dev)
 	tsc2007_stop(ts);
 }
 
+static int tsc2007_get_pendown_state(void *data)
+{
+	struct tsc2007 *ts = data;
+	int ret = 0;
+
+	ret = !!gpio_get_value(ts->pendown_gpio);
+	if (ts->active_low)
+		ret = !ret;
+
+	return ret;
+}
+
+static struct tsc2007_platform_data *tsc2007_parse_dt(struct device *dev)
+{
+	struct tsc2007_platform_data *pdata;
+	enum of_gpio_flags flags;
+	u32 value, values[3];
+	int err;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	err = of_get_named_gpio_flags(dev->of_node, "pendown-gpios", 0,
+				      &flags);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->pendown_gpio = err;
+
+	if (flags & OF_GPIO_ACTIVE_LOW)
+		pdata->active_low = true;
+
+	err = of_property_read_u32(dev->of_node, "x-plate-ohms", &value);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->x_plate_ohms = value;
+
+	err = of_property_read_u32(dev->of_node, "max-rt", &value);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->max_rt = value;
+
+	err = of_property_read_u32(dev->of_node, "poll-delay", &value);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->poll_delay = value;
+
+	err = of_property_read_u32(dev->of_node, "poll-period", &value);
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->poll_period = value;
+
+	err = of_property_read_u32_array(dev->of_node, "fuzz", values,
+					 ARRAY_SIZE(values));
+	if (err < 0)
+		return ERR_PTR(err);
+
+	pdata->fuzzx = values[0];
+	pdata->fuzzy = values[1];
+	pdata->fuzzz = values[2];
+
+	return pdata;
+}
+
 static int __devinit tsc2007_probe(struct i2c_client *client,
 				   const struct i2c_device_id *id)
 {
@@ -281,18 +365,42 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 	struct input_dev *input_dev;
 	int err;
 
+	ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
+	if (!ts)
+		return -ENOMEM;
+
+	if (IS_ENABLED(CONFIG_OF) && client->dev.of_node) {
+		client->irq = irq_of_parse_and_map(client->dev.of_node, 0);
+		if (!client->irq) {
+			err = -EPROBE_DEFER;
+			goto err_free_mem;
+		}
+	}
+
 	if (!pdata) {
-		dev_err(&client->dev, "platform data is required!\n");
-		return -EINVAL;
+		if (IS_ENABLED(CONFIG_OF) && client->dev.of_node) {
+			pdata = tsc2007_parse_dt(&client->dev);
+			if (IS_ERR(pdata)) {
+				err = PTR_ERR(pdata);
+				goto err_free_mem;
+			}
+
+			pdata->callback_data = ts;
+		} else {
+			dev_err(&client->dev, "platform data is required!\n");
+			err = -EINVAL;
+			goto err_free_mem;
+		}
 	}
 
 	if (!i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_READ_WORD_DATA))
-		return -EIO;
+				     I2C_FUNC_SMBUS_READ_WORD_DATA)) {
+		err = -EIO;
+		goto err_free_mem;
+	}
 
-	ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
 	input_dev = input_allocate_device();
-	if (!ts || !input_dev) {
+	if (!input_dev) {
 		err = -ENOMEM;
 		goto err_free_mem;
 	}
@@ -307,13 +415,27 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 	ts->max_rt            = pdata->max_rt ? : MAX_12BIT;
 	ts->poll_delay        = pdata->poll_delay ? : 1;
 	ts->poll_period       = pdata->poll_period ? : 1;
+	ts->callback_data     = pdata->callback_data;
 	ts->get_pendown_state = pdata->get_pendown_state;
 	ts->clear_penirq      = pdata->clear_penirq;
 
 	if (pdata->x_plate_ohms == 0) {
 		dev_err(&client->dev, "x_plate_ohms is not set up in platform data");
 		err = -EINVAL;
-		goto err_free_mem;
+		goto err_free_dev;
+	}
+
+	if (gpio_is_valid(pdata->pendown_gpio)) {
+		err = gpio_request_one(pdata->pendown_gpio, GPIOF_IN,
+				       "tsc2007");
+		if (err < 0)
+			goto err_free_dev;
+
+		ts->get_pendown_state = tsc2007_get_pendown_state;
+		ts->pendown_gpio = pdata->pendown_gpio;
+		ts->active_low = pdata->active_low;
+	} else {
+		ts->pendown_gpio = -EINVAL;
 	}
 
 	snprintf(ts->phys, sizeof(ts->phys),
@@ -343,7 +465,7 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 				   IRQF_ONESHOT, client->dev.driver->name, ts);
 	if (err < 0) {
 		dev_err(&client->dev, "irq %d busy?\n", ts->irq);
-		goto err_free_mem;
+		goto err_free_gpio;
 	}
 
 	tsc2007_stop(ts);
@@ -360,8 +482,12 @@ static int __devinit tsc2007_probe(struct i2c_client *client,
 	free_irq(ts->irq, ts);
 	if (pdata->exit_platform_hw)
 		pdata->exit_platform_hw();
- err_free_mem:
+ err_free_gpio:
+	if (gpio_is_valid(pdata->pendown_gpio))
+		gpio_free(pdata->pendown_gpio);
+ err_free_dev:
 	input_free_device(input_dev);
+ err_free_mem:
 	kfree(ts);
 	return err;
 }
@@ -373,6 +499,9 @@ static int __devexit tsc2007_remove(struct i2c_client *client)
 
 	free_irq(ts->irq, ts);
 
+	if (gpio_is_valid(ts->pendown_gpio))
+		gpio_free(ts->pendown_gpio);
+
 	if (pdata->exit_platform_hw)
 		pdata->exit_platform_hw();
 
diff --git a/drivers/mfd/timberdale.c b/drivers/mfd/timberdale.c
index a447f4e..249d307 100644
--- a/drivers/mfd/timberdale.c
+++ b/drivers/mfd/timberdale.c
@@ -64,7 +64,8 @@ struct timberdale_device {
 
 static struct tsc2007_platform_data timberdale_tsc2007_platform_data = {
 	.model = 2003,
-	.x_plate_ohms = 100
+	.x_plate_ohms = 100,
+	.pendown_gpio = -1,
 };
 
 static struct i2c_board_info timberdale_i2c_board_info[] = {
diff --git a/include/linux/i2c/tsc2007.h b/include/linux/i2c/tsc2007.h
index 506a9f7..8d72771 100644
--- a/include/linux/i2c/tsc2007.h
+++ b/include/linux/i2c/tsc2007.h
@@ -14,8 +14,12 @@ struct tsc2007_platform_data {
 	int	fuzzy;
 	int	fuzzz;
 
-	int	(*get_pendown_state)(void);
-	void	(*clear_penirq)(void);		/* If needed, clear 2nd level
+	int	pendown_gpio;
+	bool	active_low;
+
+	void	*callback_data;
+	int	(*get_pendown_state)(void *data);
+	void	(*clear_penirq)(void *data);	/* If needed, clear 2nd level
 						   interrupt source */
 	int	(*init_platform_hw)(void);
 	void	(*exit_platform_hw)(void);

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