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

[Thierry Reding <thierry.reding@gmail.com>]

[-- 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 --]