[PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Olof Johansson]

This seems to have been broken since 2010, so obviously noone actually
cares about the driver:

make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of function 'irq_to_gpio' [-Werror=implicit-function-declaration]

irq_to_gpio isn't available on most platforms today, so the driver
will need some rework by someone who has hardware access and can test
(to make sure that, for example, switching to level interrupts and just
keep taking them while there's more to process works).

I guess it could just be scheduled for removal, but let's start with
marking it CONFIG_BROKEN.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 drivers/input/touchscreen/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 2a21419..b8f153e 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -213,7 +213,7 @@ config TOUCHSCREEN_HAMPSHIRE
 
 config TOUCHSCREEN_EETI
 	tristate "EETI touchscreen panel support"
-	depends on I2C
+	depends on I2C && BROKEN
 	help
 	  Say Y here to enable support for I2C connected EETI touch panels.
 
-- 
1.7.9.2.359.gebfc2

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Dmitry Torokhov]

Hi Olof,

On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
> This seems to have been broken since 2010, so obviously noone actually
> cares about the driver:
> 
> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of function 'irq_to_gpio' [-Werror=implicit-function-declaration]
> 
> irq_to_gpio isn't available on most platforms today, so the driver
> will need some rework by someone who has hardware access and can test
> (to make sure that, for example, switching to level interrupts and just
> keep taking them while there's more to process works).
> 
> I guess it could just be scheduled for removal, but let's start with
> marking it CONFIG_BROKEN.

Well, it probably works quite well on arches that do have irq_to_gpio(),
let's ask Daniel and Sven if they still have this hardware and if they
can try the patch below that implements what you suggested.

Thanks.

-- 
Dmitry


Input: eeti_ts - do not use irq_to_gpio

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

irq_to_gpio is not available on many platforms. Let's switch to using
one-shot level interrupts with threaded IRQ handlers that should stay
active as long as there is data.

Reported-by: Olof Johansson <olof@lixom.net>
Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/touchscreen/eeti_ts.c |  166 ++++++++++++-----------------------
 1 files changed, 56 insertions(+), 110 deletions(-)


diff --git a/drivers/input/touchscreen/eeti_ts.c b/drivers/input/touchscreen/eeti_ts.c
index 503c709..4d875e5 100644
--- a/drivers/input/touchscreen/eeti_ts.c
+++ b/drivers/input/touchscreen/eeti_ts.c
@@ -46,9 +46,6 @@ MODULE_PARM_DESC(flip_y, "flip y coordinate");
 struct eeti_ts_priv {
 	struct i2c_client *client;
 	struct input_dev *input;
-	struct work_struct work;
-	struct mutex mutex;
-	int irq, irq_active_high;
 };
 
 #define EETI_TS_BITDEPTH	(11)
@@ -60,26 +57,18 @@ struct eeti_ts_priv {
 #define REPORT_BIT_HAS_PRESSURE	(1 << 6)
 #define REPORT_RES_BITS(v)	(((v) >> 1) + EETI_TS_BITDEPTH)
 
-static inline int eeti_ts_irq_active(struct eeti_ts_priv *priv)
-{
-	return gpio_get_value(irq_to_gpio(priv->irq)) == priv->irq_active_high;
-}
-
-static void eeti_ts_read(struct work_struct *work)
+static irqreturn_t eeti_ts_isr(int irq, void *dev_id)
 {
+	struct eeti_ts_priv *priv = dev_id;
+	struct i2c_client *client = priv->client;
+	struct input_dev *input  = priv->input;
+	unsigned int x, y, res, pressed;
+	int rc;
 	char buf[6];
-	unsigned int x, y, res, pressed, to = 100;
-	struct eeti_ts_priv *priv =
-		container_of(work, struct eeti_ts_priv, work);
-
-	mutex_lock(&priv->mutex);
-
-	while (eeti_ts_irq_active(priv) && --to)
-		i2c_master_recv(priv->client, buf, sizeof(buf));
 
-	if (!to) {
-		dev_err(&priv->client->dev,
-			"unable to clear IRQ - line stuck?\n");
+	rc = i2c_master_recv(client, buf, sizeof(buf));
+	if (rc < 0) {
+		dev_err(&client->dev, "i2c_master_recv failed, error: %d", rc);
 		goto out;
 	}
 
@@ -103,46 +92,22 @@ static void eeti_ts_read(struct work_struct *work)
 		y = EETI_MAXVAL - y;
 
 	if (buf[0] & REPORT_BIT_HAS_PRESSURE)
-		input_report_abs(priv->input, ABS_PRESSURE, buf[5]);
+		input_report_abs(input, ABS_PRESSURE, buf[5]);
 
-	input_report_abs(priv->input, ABS_X, x);
-	input_report_abs(priv->input, ABS_Y, y);
-	input_report_key(priv->input, BTN_TOUCH, !!pressed);
-	input_sync(priv->input);
+	input_report_abs(input, ABS_X, x);
+	input_report_abs(input, ABS_Y, y);
+	input_report_key(input, BTN_TOUCH, pressed);
+	input_sync(input);
 
 out:
-	mutex_unlock(&priv->mutex);
-}
-
-static irqreturn_t eeti_ts_isr(int irq, void *dev_id)
-{
-	struct eeti_ts_priv *priv = dev_id;
-
-	 /* postpone I2C transactions as we are atomic */
-	schedule_work(&priv->work);
-
 	return IRQ_HANDLED;
 }
 
-static void eeti_ts_start(struct eeti_ts_priv *priv)
-{
-	enable_irq(priv->irq);
-
-	/* Read the events once to arm the IRQ */
-	eeti_ts_read(&priv->work);
-}
-
-static void eeti_ts_stop(struct eeti_ts_priv *priv)
-{
-	disable_irq(priv->irq);
-	cancel_work_sync(&priv->work);
-}
-
 static int eeti_ts_open(struct input_dev *dev)
 {
 	struct eeti_ts_priv *priv = input_get_drvdata(dev);
 
-	eeti_ts_start(priv);
+	enable_irq(priv->client->irq);
 
 	return 0;
 }
@@ -151,17 +116,17 @@ static void eeti_ts_close(struct input_dev *dev)
 {
 	struct eeti_ts_priv *priv = input_get_drvdata(dev);
 
-	eeti_ts_stop(priv);
+	disable_irq(priv->client->irq);
 }
 
 static int __devinit eeti_ts_probe(struct i2c_client *client,
 				   const struct i2c_device_id *idp)
 {
-	struct eeti_ts_platform_data *pdata;
+	const struct eeti_ts_platform_data *pdata = client->dev.platform_data;
 	struct eeti_ts_priv *priv;
 	struct input_dev *input;
 	unsigned int irq_flags;
-	int err = -ENOMEM;
+	int err;
 
 	/*
 	 * In contrast to what's described in the datasheet, there seems
@@ -171,25 +136,15 @@ static int __devinit eeti_ts_probe(struct i2c_client *client,
 	 */
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv) {
-		dev_err(&client->dev, "failed to allocate driver data\n");
-		goto err0;
-	}
-
-	mutex_init(&priv->mutex);
 	input = input_allocate_device();
-
-	if (!input) {
-		dev_err(&client->dev, "Failed to allocate input device.\n");
-		goto err1;
+	if (!priv || !input) {
+		dev_err(&client->dev, "failed to memory\n");
+		err = -ENOMEM;
+		goto err_free_mem;
 	}
 
-	input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
-	input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
-
-	input_set_abs_params(input, ABS_X, 0, EETI_MAXVAL, 0, 0);
-	input_set_abs_params(input, ABS_Y, 0, EETI_MAXVAL, 0, 0);
-	input_set_abs_params(input, ABS_PRESSURE, 0, 0xff, 0, 0);
+	priv->client = client;
+	priv->input = input;
 
 	input->name = client->name;
 	input->id.bustype = BUS_I2C;
@@ -197,49 +152,47 @@ static int __devinit eeti_ts_probe(struct i2c_client *client,
 	input->open = eeti_ts_open;
 	input->close = eeti_ts_close;
 
-	priv->client = client;
-	priv->input = input;
-	priv->irq = client->irq;
-
-	pdata = client->dev.platform_data;
-
-	if (pdata)
-		priv->irq_active_high = pdata->irq_active_high;
+	input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
+	input->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
 
-	irq_flags = priv->irq_active_high ?
-		IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
+	input_set_abs_params(input, ABS_X, 0, EETI_MAXVAL, 0, 0);
+	input_set_abs_params(input, ABS_Y, 0, EETI_MAXVAL, 0, 0);
+	input_set_abs_params(input, ABS_PRESSURE, 0, 0xff, 0, 0);
 
-	INIT_WORK(&priv->work, eeti_ts_read);
-	i2c_set_clientdata(client, priv);
 	input_set_drvdata(input, priv);
 
-	err = input_register_device(input);
-	if (err)
-		goto err1;
+	irq_flags = pdata && pdata->irq_active_high ?
+			IRQF_TRIGGER_HIGH : IRQF_TRIGGER_LOW;
+	irq_flags |= IRQF_ONESHOT;
 
-	err = request_irq(priv->irq, eeti_ts_isr, irq_flags,
-			  client->name, priv);
-	if (err) {
+	err = request_threaded_irq(client->irq, NULL, eeti_ts_isr,
+				   irq_flags, client->name, priv);
+	if (err < 0) {
 		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
-		goto err2;
+		goto err_free_mem;
 	}
 
 	/*
-	 * Disable the device for now. It will be enabled once the
+	 * Disable the IRQ for now. It will be enabled once the
 	 * input device is opened.
 	 */
-	eeti_ts_stop(priv);
+	disable_irq(client->irq);
 
+	err = input_register_device(input);
+	if (err)
+		goto err_free_irq;
+
+	i2c_set_clientdata(client, priv);
 	device_init_wakeup(&client->dev, 0);
+
 	return 0;
 
-err2:
-	input_unregister_device(input);
-	input = NULL; /* so we dont try to free it below */
-err1:
+err_free_irq:
+	free_irq(client->irq, priv);
+err_free_mem:
 	input_free_device(input);
 	kfree(priv);
-err0:
+
 	return err;
 }
 
@@ -247,20 +200,14 @@ static int __devexit eeti_ts_remove(struct i2c_client *client)
 {
 	struct eeti_ts_priv *priv = i2c_get_clientdata(client);
 
-	free_irq(priv->irq, priv);
-	/*
-	 * eeti_ts_stop() leaves IRQ disabled. We need to re-enable it
-	 * so that device still works if we reload the driver.
-	 */
-	enable_irq(priv->irq);
-
+	free_irq(client->irq, priv);
 	input_unregister_device(priv->input);
 	kfree(priv);
 
 	return 0;
 }
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 static int eeti_ts_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
@@ -270,12 +217,12 @@ static int eeti_ts_suspend(struct device *dev)
 	mutex_lock(&input_dev->mutex);
 
 	if (input_dev->users)
-		eeti_ts_stop(priv);
+		disable_irq(client->irq);
 
 	mutex_unlock(&input_dev->mutex);
 
 	if (device_may_wakeup(&client->dev))
-		enable_irq_wake(priv->irq);
+		enable_irq_wake(client->irq);
 
 	return 0;
 }
@@ -287,20 +234,20 @@ static int eeti_ts_resume(struct device *dev)
 	struct input_dev *input_dev = priv->input;
 
 	if (device_may_wakeup(&client->dev))
-		disable_irq_wake(priv->irq);
+		disable_irq_wake(client->irq);
 
 	mutex_lock(&input_dev->mutex);
 
 	if (input_dev->users)
-		eeti_ts_start(priv);
+		enable_irq(client->irq);
 
 	mutex_unlock(&input_dev->mutex);
 
 	return 0;
 }
+#endif
 
 static SIMPLE_DEV_PM_OPS(eeti_ts_pm, eeti_ts_suspend, eeti_ts_resume);
-#endif
 
 static const struct i2c_device_id eeti_ts_id[] = {
 	{ "eeti_ts", 0 },
@@ -311,9 +258,8 @@ MODULE_DEVICE_TABLE(i2c, eeti_ts_id);
 static struct i2c_driver eeti_ts_driver = {
 	.driver = {
 		.name = "eeti_ts",
-#ifdef CONFIG_PM
+		.owner = THIS_MODULE,
 		.pm = &eeti_ts_pm,
-#endif
 	},
 	.probe = eeti_ts_probe,
 	.remove = __devexit_p(eeti_ts_remove),

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Sven Neumann]

Hi,

On 07.04.12 09:02, Dmitry Torokhov wrote:
> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
>> This seems to have been broken since 2010, so obviously noone actually
>> cares about the driver:
>>
>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of function 'irq_to_gpio' [-Werror=implicit-function-declaration]
>>
>> irq_to_gpio isn't available on most platforms today, so the driver
>> will need some rework by someone who has hardware access and can test
>> (to make sure that, for example, switching to level interrupts and just
>> keep taking them while there's more to process works).
>>
>> I guess it could just be scheduled for removal, but let's start with
>> marking it CONFIG_BROKEN.
>
> Well, it probably works quite well on arches that do have irq_to_gpio(),
> let's ask Daniel and Sven if they still have this hardware and if they
> can try the patch below that implements what you suggested.

This hardware is still in use and we also still follow kernel 
development and try to update our customer devices to recent kernel 
versions regularly. Currently we are at 3.1.10 and the touchscreen works 
well with that. I'll try to update to a more recent kernel next week and 
will try your patch.


Regards, Sven

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Olof Johansson]

On Sat, Apr 7, 2012 at 11:38 AM, Sven Neumann <s.neumann@raumfeld.com> wrote:
> Hi,
>
>
> On 07.04.12 09:02, Dmitry Torokhov wrote:
>>
>> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
>>>
>>> This seems to have been broken since 2010, so obviously noone actually
>>> cares about the driver:
>>>
>>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
>>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
>>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of
>>> function 'irq_to_gpio' [-Werror=implicit-function-declaration]
>>>
>>> irq_to_gpio isn't available on most platforms today, so the driver
>>> will need some rework by someone who has hardware access and can test
>>> (to make sure that, for example, switching to level interrupts and just
>>> keep taking them while there's more to process works).
>>>
>>> I guess it could just be scheduled for removal, but let's start with
>>> marking it CONFIG_BROKEN.
>>
>>
>> Well, it probably works quite well on arches that do have irq_to_gpio(),
>> let's ask Daniel and Sven if they still have this hardware and if they
>> can try the patch below that implements what you suggested.
>
>
> This hardware is still in use and we also still follow kernel development
> and try to update our customer devices to recent kernel versions regularly.
> Currently we are at 3.1.10 and the touchscreen works well with that. I'll
> try to update to a more recent kernel next week and will try your patch.

Ah, you're right, and this was my bad. Looks like this change was
introduced in 3.2 and broke this and one more driver (ezx-pcap):

Author:     Haojian Zhuang <haojian.zhuang@marvell.com>
AuthorDate: Mon Oct 10 16:03:51 2011 +0800
Commit:     Haojian Zhuang <haojian.zhuang@marvell.com>
CommitDate: Mon Nov 14 21:07:59 2011 +0800

    ARM: pxa: rename gpio_to_irq and irq_to_gpio

    Avoid to define gpio_to_irq() and irq_to_gpio() for potential name
    confliction since multiple architecture will be built together.

    Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>


Haojian, I think it was probably premature to do the multiplatform
change like that, since it means that a PXA-only kernel has no mapping
from irq_to_gpio to pxa_irq_to_gpio. Can you please address this as a
fix for 3.4?


Thanks,

-Olof

[PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Olof Johansson]

On Sat, Apr 7, 2012 at 11:38 AM, Sven Neumann <s.neumann@raumfeld.com> wrote:
> Hi,
>
>
> On 07.04.12 09:02, Dmitry Torokhov wrote:
>>
>> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
>>>
>>> This seems to have been broken since 2010, so obviously noone actually
>>> cares about the driver:
>>>
>>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
>>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
>>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of
>>> function 'irq_to_gpio' [-Werror=implicit-function-declaration]
>>>
>>> irq_to_gpio isn't available on most platforms today, so the driver
>>> will need some rework by someone who has hardware access and can test
>>> (to make sure that, for example, switching to level interrupts and just
>>> keep taking them while there's more to process works).
>>>
>>> I guess it could just be scheduled for removal, but let's start with
>>> marking it CONFIG_BROKEN.
>>
>>
>> Well, it probably works quite well on arches that do have irq_to_gpio(),
>> let's ask Daniel and Sven if they still have this hardware and if they
>> can try the patch below that implements what you suggested.
>
>
> This hardware is still in use and we also still follow kernel development
> and try to update our customer devices to recent kernel versions regularly.
> Currently we are at 3.1.10 and the touchscreen works well with that. I'll
> try to update to a more recent kernel next week and will try your patch.

Ah, you're right, and this was my bad. Looks like this change was
introduced in 3.2 and broke this and one more driver (ezx-pcap):

Author:     Haojian Zhuang <haojian.zhuang@marvell.com>
AuthorDate: Mon Oct 10 16:03:51 2011 +0800
Commit:     Haojian Zhuang <haojian.zhuang@marvell.com>
CommitDate: Mon Nov 14 21:07:59 2011 +0800

    ARM: pxa: rename gpio_to_irq and irq_to_gpio

    Avoid to define gpio_to_irq() and irq_to_gpio() for potential name
    confliction since multiple architecture will be built together.

    Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>


Haojian, I think it was probably premature to do the multiplatform
change like that, since it means that a PXA-only kernel has no mapping
from irq_to_gpio to pxa_irq_to_gpio. Can you please address this as a
fix for 3.4?


Thanks,

-Olof

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Joachim Eastwood]

On Sat, Apr 7, 2012 at 10:32 PM, Olof Johansson <olof@lixom.net> wrote:
> On Sat, Apr 7, 2012 at 11:38 AM, Sven Neumann <s.neumann@raumfeld.com> wrote:
>> Hi,
>>
>>
>> On 07.04.12 09:02, Dmitry Torokhov wrote:
>>>
>>> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
>>>>
>>>> This seems to have been broken since 2010, so obviously noone actually
>>>> cares about the driver:
>>>>
>>>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
>>>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
>>>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of
>>>> function 'irq_to_gpio' [-Werror=implicit-function-declaration]
>>>>
>>>> irq_to_gpio isn't available on most platforms today, so the driver
>>>> will need some rework by someone who has hardware access and can test
>>>> (to make sure that, for example, switching to level interrupts and just
>>>> keep taking them while there's more to process works).
>>>>
>>>> I guess it could just be scheduled for removal, but let's start with
>>>> marking it CONFIG_BROKEN.
>>>
>>>
>>> Well, it probably works quite well on arches that do have irq_to_gpio(),
>>> let's ask Daniel and Sven if they still have this hardware and if they
>>> can try the patch below that implements what you suggested.
>>
>>
>> This hardware is still in use and we also still follow kernel development
>> and try to update our customer devices to recent kernel versions regularly.
>> Currently we are at 3.1.10 and the touchscreen works well with that. I'll
>> try to update to a more recent kernel next week and will try your patch.
>
> Ah, you're right, and this was my bad. Looks like this change was
> introduced in 3.2 and broke this and one more driver (ezx-pcap):
>
> Author:     Haojian Zhuang <haojian.zhuang@marvell.com>
> AuthorDate: Mon Oct 10 16:03:51 2011 +0800
> Commit:     Haojian Zhuang <haojian.zhuang@marvell.com>
> CommitDate: Mon Nov 14 21:07:59 2011 +0800
>
>    ARM: pxa: rename gpio_to_irq and irq_to_gpio
>
>    Avoid to define gpio_to_irq() and irq_to_gpio() for potential name
>    confliction since multiple architecture will be built together.
>
>    Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
>
>
> Haojian, I think it was probably premature to do the multiplatform
> change like that, since it means that a PXA-only kernel has no mapping
> from irq_to_gpio to pxa_irq_to_gpio. Can you please address this as a
> fix for 3.4?
>
>
> Thanks,
>
> -Olof

It's the same story with AT91. Here irq_to_gpio was also removed some
time ago. I believe it's the same for a lot of other ARM architectures
also.
Currently there 7 drivers that uses irq_to_gpio, these are obviously
broken on architectures that not provide the function.

I think it would be better to fix the drivers rather than resurrect
irq_to_gpio on the individual architectures.

Users of irq_to_gpio in drivers:
pata_rb532_cf, eeti_ts, egalax_ts, ezx-pcap, db1xxx_ss, tosa_battery, lis3l02dq

regards
Joachim Eastwood

> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Joachim Eastwood]

On Sat, Apr 7, 2012 at 10:32 PM, Olof Johansson <olof@lixom.net> wrote:
> On Sat, Apr 7, 2012 at 11:38 AM, Sven Neumann <s.neumann@raumfeld.com> wrote:
>> Hi,
>>
>>
>> On 07.04.12 09:02, Dmitry Torokhov wrote:
>>>
>>> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
>>>>
>>>> This seems to have been broken since 2010, so obviously noone actually
>>>> cares about the driver:
>>>>
>>>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
>>>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
>>>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of
>>>> function 'irq_to_gpio' [-Werror=implicit-function-declaration]
>>>>
>>>> irq_to_gpio isn't available on most platforms today, so the driver
>>>> will need some rework by someone who has hardware access and can test
>>>> (to make sure that, for example, switching to level interrupts and just
>>>> keep taking them while there's more to process works).
>>>>
>>>> I guess it could just be scheduled for removal, but let's start with
>>>> marking it CONFIG_BROKEN.
>>>
>>>
>>> Well, it probably works quite well on arches that do have irq_to_gpio(),
>>> let's ask Daniel and Sven if they still have this hardware and if they
>>> can try the patch below that implements what you suggested.
>>
>>
>> This hardware is still in use and we also still follow kernel development
>> and try to update our customer devices to recent kernel versions regularly.
>> Currently we are at 3.1.10 and the touchscreen works well with that. I'll
>> try to update to a more recent kernel next week and will try your patch.
>
> Ah, you're right, and this was my bad. Looks like this change was
> introduced in 3.2 and broke this and one more driver (ezx-pcap):
>
> Author: ? ? Haojian Zhuang <haojian.zhuang@marvell.com>
> AuthorDate: Mon Oct 10 16:03:51 2011 +0800
> Commit: ? ? Haojian Zhuang <haojian.zhuang@marvell.com>
> CommitDate: Mon Nov 14 21:07:59 2011 +0800
>
> ? ?ARM: pxa: rename gpio_to_irq and irq_to_gpio
>
> ? ?Avoid to define gpio_to_irq() and irq_to_gpio() for potential name
> ? ?confliction since multiple architecture will be built together.
>
> ? ?Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
>
>
> Haojian, I think it was probably premature to do the multiplatform
> change like that, since it means that a PXA-only kernel has no mapping
> from irq_to_gpio to pxa_irq_to_gpio. Can you please address this as a
> fix for 3.4?
>
>
> Thanks,
>
> -Olof

It's the same story with AT91. Here irq_to_gpio was also removed some
time ago. I believe it's the same for a lot of other ARM architectures
also.
Currently there 7 drivers that uses irq_to_gpio, these are obviously
broken on architectures that not provide the function.

I think it would be better to fix the drivers rather than resurrect
irq_to_gpio on the individual architectures.

Users of irq_to_gpio in drivers:
pata_rb532_cf, eeti_ts, egalax_ts, ezx-pcap, db1xxx_ss, tosa_battery, lis3l02dq

regards
Joachim Eastwood

> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Joachim Eastwood]

On Sat, Apr 7, 2012 at 10:32 PM, Olof Johansson <olof@lixom.net> wrote:
> On Sat, Apr 7, 2012 at 11:38 AM, Sven Neumann <s.neumann@raumfeld.com> wrote:
>> Hi,
>>
>>
>> On 07.04.12 09:02, Dmitry Torokhov wrote:
>>>
>>> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
>>>>
>>>> This seems to have been broken since 2010, so obviously noone actually
>>>> cares about the driver:
>>>>
>>>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
>>>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
>>>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of
>>>> function 'irq_to_gpio' [-Werror=implicit-function-declaration]
>>>>
>>>> irq_to_gpio isn't available on most platforms today, so the driver
>>>> will need some rework by someone who has hardware access and can test
>>>> (to make sure that, for example, switching to level interrupts and just
>>>> keep taking them while there's more to process works).
>>>>
>>>> I guess it could just be scheduled for removal, but let's start with
>>>> marking it CONFIG_BROKEN.
>>>
>>>
>>> Well, it probably works quite well on arches that do have irq_to_gpio(),
>>> let's ask Daniel and Sven if they still have this hardware and if they
>>> can try the patch below that implements what you suggested.
>>
>>
>> This hardware is still in use and we also still follow kernel development
>> and try to update our customer devices to recent kernel versions regularly.
>> Currently we are at 3.1.10 and the touchscreen works well with that. I'll
>> try to update to a more recent kernel next week and will try your patch.
>
> Ah, you're right, and this was my bad. Looks like this change was
> introduced in 3.2 and broke this and one more driver (ezx-pcap):
>
> Author:     Haojian Zhuang <haojian.zhuang@marvell.com>
> AuthorDate: Mon Oct 10 16:03:51 2011 +0800
> Commit:     Haojian Zhuang <haojian.zhuang@marvell.com>
> CommitDate: Mon Nov 14 21:07:59 2011 +0800
>
>    ARM: pxa: rename gpio_to_irq and irq_to_gpio
>
>    Avoid to define gpio_to_irq() and irq_to_gpio() for potential name
>    confliction since multiple architecture will be built together.
>
>    Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
>
>
> Haojian, I think it was probably premature to do the multiplatform
> change like that, since it means that a PXA-only kernel has no mapping
> from irq_to_gpio to pxa_irq_to_gpio. Can you please address this as a
> fix for 3.4?
>
>
> Thanks,
>
> -Olof

It's the same story with AT91. Here irq_to_gpio was also removed some
time ago. I believe it's the same for a lot of other ARM architectures
also.
Currently there 7 drivers that uses irq_to_gpio, these are obviously
broken on architectures that not provide the function.

I think it would be better to fix the drivers rather than resurrect
irq_to_gpio on the individual architectures.

Users of irq_to_gpio in drivers:
pata_rb532_cf, eeti_ts, egalax_ts, ezx-pcap, db1xxx_ss, tosa_battery, lis3l02dq

regards
Joachim Eastwood

> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Haojian Zhuang]

On Sun, Apr 8, 2012 at 4:32 AM, Olof Johansson <olof@lixom.net> wrote:
> On Sat, Apr 7, 2012 at 11:38 AM, Sven Neumann <s.neumann@raumfeld.com> wrote:
>> Hi,
>>
>>
>> On 07.04.12 09:02, Dmitry Torokhov wrote:
>>>
>>> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
>>>>
>>>> This seems to have been broken since 2010, so obviously noone actually
>>>> cares about the driver:
>>>>
>>>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
>>>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
>>>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of
>>>> function 'irq_to_gpio' [-Werror=implicit-function-declaration]
>>>>
>>>> irq_to_gpio isn't available on most platforms today, so the driver
>>>> will need some rework by someone who has hardware access and can test
>>>> (to make sure that, for example, switching to level interrupts and just
>>>> keep taking them while there's more to process works).
>>>>
>>>> I guess it could just be scheduled for removal, but let's start with
>>>> marking it CONFIG_BROKEN.
>>>
>>>
>>> Well, it probably works quite well on arches that do have irq_to_gpio(),
>>> let's ask Daniel and Sven if they still have this hardware and if they
>>> can try the patch below that implements what you suggested.
>>
>>
>> This hardware is still in use and we also still follow kernel development
>> and try to update our customer devices to recent kernel versions regularly.
>> Currently we are at 3.1.10 and the touchscreen works well with that. I'll
>> try to update to a more recent kernel next week and will try your patch.
>
> Ah, you're right, and this was my bad. Looks like this change was
> introduced in 3.2 and broke this and one more driver (ezx-pcap):
>
> Author:     Haojian Zhuang <haojian.zhuang@marvell.com>
> AuthorDate: Mon Oct 10 16:03:51 2011 +0800
> Commit:     Haojian Zhuang <haojian.zhuang@marvell.com>
> CommitDate: Mon Nov 14 21:07:59 2011 +0800
>
>    ARM: pxa: rename gpio_to_irq and irq_to_gpio
>
>    Avoid to define gpio_to_irq() and irq_to_gpio() for potential name
>    confliction since multiple architecture will be built together.
>
>    Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
>
>
> Haojian, I think it was probably premature to do the multiplatform
> change like that, since it means that a PXA-only kernel has no mapping
> from irq_to_gpio to pxa_irq_to_gpio. Can you please address this as a
> fix for 3.4?
>
>
I tried to fix ezx-pcap before. Since there's some codebase confliction, the
patch wasn't merged. I'll format patch again. And I'll try to fix other patches
that are covered in arch-pxa if they exist.

Best Regards
Haojian

[PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Haojian Zhuang]

On Sun, Apr 8, 2012 at 4:32 AM, Olof Johansson <olof@lixom.net> wrote:
> On Sat, Apr 7, 2012 at 11:38 AM, Sven Neumann <s.neumann@raumfeld.com> wrote:
>> Hi,
>>
>>
>> On 07.04.12 09:02, Dmitry Torokhov wrote:
>>>
>>> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
>>>>
>>>> This seems to have been broken since 2010, so obviously noone actually
>>>> cares about the driver:
>>>>
>>>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
>>>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
>>>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of
>>>> function 'irq_to_gpio' [-Werror=implicit-function-declaration]
>>>>
>>>> irq_to_gpio isn't available on most platforms today, so the driver
>>>> will need some rework by someone who has hardware access and can test
>>>> (to make sure that, for example, switching to level interrupts and just
>>>> keep taking them while there's more to process works).
>>>>
>>>> I guess it could just be scheduled for removal, but let's start with
>>>> marking it CONFIG_BROKEN.
>>>
>>>
>>> Well, it probably works quite well on arches that do have irq_to_gpio(),
>>> let's ask Daniel and Sven if they still have this hardware and if they
>>> can try the patch below that implements what you suggested.
>>
>>
>> This hardware is still in use and we also still follow kernel development
>> and try to update our customer devices to recent kernel versions regularly.
>> Currently we are at 3.1.10 and the touchscreen works well with that. I'll
>> try to update to a more recent kernel next week and will try your patch.
>
> Ah, you're right, and this was my bad. Looks like this change was
> introduced in 3.2 and broke this and one more driver (ezx-pcap):
>
> Author: ? ? Haojian Zhuang <haojian.zhuang@marvell.com>
> AuthorDate: Mon Oct 10 16:03:51 2011 +0800
> Commit: ? ? Haojian Zhuang <haojian.zhuang@marvell.com>
> CommitDate: Mon Nov 14 21:07:59 2011 +0800
>
> ? ?ARM: pxa: rename gpio_to_irq and irq_to_gpio
>
> ? ?Avoid to define gpio_to_irq() and irq_to_gpio() for potential name
> ? ?confliction since multiple architecture will be built together.
>
> ? ?Signed-off-by: Haojian Zhuang <haojian.zhuang@marvell.com>
>
>
> Haojian, I think it was probably premature to do the multiplatform
> change like that, since it means that a PXA-only kernel has no mapping
> from irq_to_gpio to pxa_irq_to_gpio. Can you please address this as a
> fix for 3.4?
>
>
I tried to fix ezx-pcap before. Since there's some codebase confliction, the
patch wasn't merged. I'll format patch again. And I'll try to fix other patches
that are covered in arch-pxa if they exist.

Best Regards
Haojian

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Russell King - ARM Linux]

On Sat, Apr 07, 2012 at 01:32:31PM -0700, Olof Johansson wrote:
> Haojian, I think it was probably premature to do the multiplatform
> change like that, since it means that a PXA-only kernel has no mapping
> from irq_to_gpio to pxa_irq_to_gpio. Can you please address this as a
> fix for 3.4?

Please fix ezx-pcap instead - it's broken as it currently stands by using
irq_to_gpio(), and it's one reason why my randconfig builds on OMAP fail.

The big problem is - what does this do:

        do {
...
        } while (gpio_get_value(irq_to_gpio(pcap->spi->irq)));

if pcap->spi->irq has no GPIO associated with the interrupt?  irq_to_gpio()
probably returns some random number which might be some other GPIO in the
system, and gpio_get_value() could oops if irq_to_gpio returns a negative
or large positive number.  To put it another way, according to the above
code, irq_to_gpio() must always return a valid gpio for the IRQ even if
the IRQ doesn't have a GPIO associated with it.

This is a fine illustration of why irq_to_gpio() is just plain broken in
its design.

[PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Russell King - ARM Linux]

On Sat, Apr 07, 2012 at 01:32:31PM -0700, Olof Johansson wrote:
> Haojian, I think it was probably premature to do the multiplatform
> change like that, since it means that a PXA-only kernel has no mapping
> from irq_to_gpio to pxa_irq_to_gpio. Can you please address this as a
> fix for 3.4?

Please fix ezx-pcap instead - it's broken as it currently stands by using
irq_to_gpio(), and it's one reason why my randconfig builds on OMAP fail.

The big problem is - what does this do:

        do {
...
        } while (gpio_get_value(irq_to_gpio(pcap->spi->irq)));

if pcap->spi->irq has no GPIO associated with the interrupt?  irq_to_gpio()
probably returns some random number which might be some other GPIO in the
system, and gpio_get_value() could oops if irq_to_gpio returns a negative
or large positive number.  To put it another way, according to the above
code, irq_to_gpio() must always return a valid gpio for the IRQ even if
the IRQ doesn't have a GPIO associated with it.

This is a fine illustration of why irq_to_gpio() is just plain broken in
its design.

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Dmitry Torokhov]

On Tue, Apr 10, 2012 at 11:10:47AM +0100, Russell King - ARM Linux wrote:
> On Sat, Apr 07, 2012 at 01:32:31PM -0700, Olof Johansson wrote:
> > Haojian, I think it was probably premature to do the multiplatform
> > change like that, since it means that a PXA-only kernel has no mapping
> > from irq_to_gpio to pxa_irq_to_gpio. Can you please address this as a
> > fix for 3.4?
> 
> Please fix ezx-pcap instead - it's broken as it currently stands by using
> irq_to_gpio(), and it's one reason why my randconfig builds on OMAP fail.
> 
> The big problem is - what does this do:
> 
>         do {
> ...
>         } while (gpio_get_value(irq_to_gpio(pcap->spi->irq)));
> 
> if pcap->spi->irq has no GPIO associated with the interrupt?  irq_to_gpio()
> probably returns some random number which might be some other GPIO in the
> system, and gpio_get_value() could oops if irq_to_gpio returns a negative
> or large positive number.  To put it another way, according to the above
> code, irq_to_gpio() must always return a valid gpio for the IRQ even if
> the IRQ doesn't have a GPIO associated with it.
> 
> This is a fine illustration of why irq_to_gpio() is just plain broken in
> its design.

OK, so we can fix eeti_ts and wacom_i2c; but what do we do with
egalax_ts.c? It reconfigures gpio as output and sens a pulse to wake the
controller from sleep. I guess we'll need platform data attached to
it...

Thanks.

-- 
Dmitry

[PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Dmitry Torokhov]

On Tue, Apr 10, 2012 at 11:10:47AM +0100, Russell King - ARM Linux wrote:
> On Sat, Apr 07, 2012 at 01:32:31PM -0700, Olof Johansson wrote:
> > Haojian, I think it was probably premature to do the multiplatform
> > change like that, since it means that a PXA-only kernel has no mapping
> > from irq_to_gpio to pxa_irq_to_gpio. Can you please address this as a
> > fix for 3.4?
> 
> Please fix ezx-pcap instead - it's broken as it currently stands by using
> irq_to_gpio(), and it's one reason why my randconfig builds on OMAP fail.
> 
> The big problem is - what does this do:
> 
>         do {
> ...
>         } while (gpio_get_value(irq_to_gpio(pcap->spi->irq)));
> 
> if pcap->spi->irq has no GPIO associated with the interrupt?  irq_to_gpio()
> probably returns some random number which might be some other GPIO in the
> system, and gpio_get_value() could oops if irq_to_gpio returns a negative
> or large positive number.  To put it another way, according to the above
> code, irq_to_gpio() must always return a valid gpio for the IRQ even if
> the IRQ doesn't have a GPIO associated with it.
> 
> This is a fine illustration of why irq_to_gpio() is just plain broken in
> its design.

OK, so we can fix eeti_ts and wacom_i2c; but what do we do with
egalax_ts.c? It reconfigures gpio as output and sens a pulse to wake the
controller from sleep. I guess we'll need platform data attached to
it...

Thanks.

-- 
Dmitry

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Dmitry Torokhov]

Hi Sven,

On Sat, Apr 07, 2012 at 08:38:33PM +0200, Sven Neumann wrote:
> Hi,
> 
> On 07.04.12 09:02, Dmitry Torokhov wrote:
> >On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
> >>This seems to have been broken since 2010, so obviously noone actually
> >>cares about the driver:
> >>
> >>make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
> >>drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
> >>drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of function 'irq_to_gpio' [-Werror=implicit-function-declaration]
> >>
> >>irq_to_gpio isn't available on most platforms today, so the driver
> >>will need some rework by someone who has hardware access and can test
> >>(to make sure that, for example, switching to level interrupts and just
> >>keep taking them while there's more to process works).
> >>
> >>I guess it could just be scheduled for removal, but let's start with
> >>marking it CONFIG_BROKEN.
> >
> >Well, it probably works quite well on arches that do have irq_to_gpio(),
> >let's ask Daniel and Sven if they still have this hardware and if they
> >can try the patch below that implements what you suggested.
> 
> This hardware is still in use and we also still follow kernel
> development and try to update our customer devices to recent kernel
> versions regularly. Currently we are at 3.1.10 and the touchscreen
> works well with that. I'll try to update to a more recent kernel
> next week and will try your patch.
> 

Did you have a chance to test the patch?

Thanks.

-- 
Dmitry

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Dmitry Torokhov]

On Wed, May 02, 2012 at 09:36:51PM -0700, Dmitry Torokhov wrote:
> Hi Sven,
> 
> On Sat, Apr 07, 2012 at 08:38:33PM +0200, Sven Neumann wrote:
> > Hi,
> > 
> > On 07.04.12 09:02, Dmitry Torokhov wrote:
> > >On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
> > >>This seems to have been broken since 2010, so obviously noone actually
> > >>cares about the driver:
> > >>
> > >>make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
> > >>drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
> > >>drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of function 'irq_to_gpio' [-Werror=implicit-function-declaration]
> > >>
> > >>irq_to_gpio isn't available on most platforms today, so the driver
> > >>will need some rework by someone who has hardware access and can test
> > >>(to make sure that, for example, switching to level interrupts and just
> > >>keep taking them while there's more to process works).
> > >>
> > >>I guess it could just be scheduled for removal, but let's start with
> > >>marking it CONFIG_BROKEN.
> > >
> > >Well, it probably works quite well on arches that do have irq_to_gpio(),
> > >let's ask Daniel and Sven if they still have this hardware and if they
> > >can try the patch below that implements what you suggested.
> > 
> > This hardware is still in use and we also still follow kernel
> > development and try to update our customer devices to recent kernel
> > versions regularly. Currently we are at 3.1.10 and the touchscreen
> > works well with that. I'll try to update to a more recent kernel
> > next week and will try your patch.
> > 
> 
> Did you have a chance to test the patch?

*ping*

It would be nice to get driver in mainline compile [and work] again...

Thanks.

-- 
Dmitry

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Daniel Mack]

On 13.07.2012 09:01, Dmitry Torokhov wrote:
> On Wed, May 02, 2012 at 09:36:51PM -0700, Dmitry Torokhov wrote:
>> Hi Sven,
>>
>> On Sat, Apr 07, 2012 at 08:38:33PM +0200, Sven Neumann wrote:
>>> Hi,
>>>
>>> On 07.04.12 09:02, Dmitry Torokhov wrote:
>>>> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
>>>>> This seems to have been broken since 2010, so obviously noone actually
>>>>> cares about the driver:
>>>>>
>>>>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
>>>>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
>>>>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of function 'irq_to_gpio' [-Werror=implicit-function-declaration]
>>>>>
>>>>> irq_to_gpio isn't available on most platforms today, so the driver
>>>>> will need some rework by someone who has hardware access and can test
>>>>> (to make sure that, for example, switching to level interrupts and just
>>>>> keep taking them while there's more to process works).
>>>>>
>>>>> I guess it could just be scheduled for removal, but let's start with
>>>>> marking it CONFIG_BROKEN.
>>>>
>>>> Well, it probably works quite well on arches that do have irq_to_gpio(),
>>>> let's ask Daniel and Sven if they still have this hardware and if they
>>>> can try the patch below that implements what you suggested.
>>>
>>> This hardware is still in use and we also still follow kernel
>>> development and try to update our customer devices to recent kernel
>>> versions regularly. Currently we are at 3.1.10 and the touchscreen
>>> works well with that. I'll try to update to a more recent kernel
>>> next week and will try your patch.
>>>
>>
>> Did you have a chance to test the patch?
> 
> *ping*
> 
> It would be nice to get driver in mainline compile [and work] again...

Sorry, I got too much stuff to do right now. Please give me a week or so
and I'll be able to test this.


Thanks,
Daniel

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Daniel Mack]

On 13.07.2012 09:01, Dmitry Torokhov wrote:
> On Wed, May 02, 2012 at 09:36:51PM -0700, Dmitry Torokhov wrote:
>> Hi Sven,
>>
>> On Sat, Apr 07, 2012 at 08:38:33PM +0200, Sven Neumann wrote:
>>> Hi,
>>>
>>> On 07.04.12 09:02, Dmitry Torokhov wrote:
>>>> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
>>>>> This seems to have been broken since 2010, so obviously noone actually
>>>>> cares about the driver:
>>>>>
>>>>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
>>>>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
>>>>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of function 'irq_to_gpio' [-Werror=implicit-function-declaration]
>>>>>
>>>>> irq_to_gpio isn't available on most platforms today, so the driver
>>>>> will need some rework by someone who has hardware access and can test
>>>>> (to make sure that, for example, switching to level interrupts and just
>>>>> keep taking them while there's more to process works).
>>>>>
>>>>> I guess it could just be scheduled for removal, but let's start with
>>>>> marking it CONFIG_BROKEN.
>>>>
>>>> Well, it probably works quite well on arches that do have irq_to_gpio(),
>>>> let's ask Daniel and Sven if they still have this hardware and if they
>>>> can try the patch below that implements what you suggested.
>>>
>>> This hardware is still in use and we also still follow kernel
>>> development and try to update our customer devices to recent kernel
>>> versions regularly. Currently we are at 3.1.10 and the touchscreen
>>> works well with that. I'll try to update to a more recent kernel
>>> next week and will try your patch.
>>>
>>
>> Did you have a chance to test the patch?
> 
> *ping*
> 
> It would be nice to get driver in mainline compile [and work] again...

We gave that patch a quick try today and it doesn't seem to work. We
don't get any events from the touch screen anymore. We need to debug
this further, hopefully by the end of this week.

If there's anything obvious in the setup of the threaded IRQ handler,
please let us know. Otherwise, I'll get back once I have a fixed version
of the patch.


Thanks,
Daniel

> 
> Thanks.
> 

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Daniel Mack]

On 17.07.2012 14:59, Daniel Mack wrote:
> On 13.07.2012 09:01, Dmitry Torokhov wrote:
>> On Wed, May 02, 2012 at 09:36:51PM -0700, Dmitry Torokhov wrote:
>>> Hi Sven,
>>>
>>> On Sat, Apr 07, 2012 at 08:38:33PM +0200, Sven Neumann wrote:
>>>> Hi,
>>>>
>>>> On 07.04.12 09:02, Dmitry Torokhov wrote:
>>>>> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
>>>>>> This seems to have been broken since 2010, so obviously noone actually
>>>>>> cares about the driver:
>>>>>>
>>>>>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
>>>>>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
>>>>>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of function 'irq_to_gpio' [-Werror=implicit-function-declaration]
>>>>>>
>>>>>> irq_to_gpio isn't available on most platforms today, so the driver
>>>>>> will need some rework by someone who has hardware access and can test
>>>>>> (to make sure that, for example, switching to level interrupts and just
>>>>>> keep taking them while there's more to process works).
>>>>>>
>>>>>> I guess it could just be scheduled for removal, but let's start with
>>>>>> marking it CONFIG_BROKEN.
>>>>>
>>>>> Well, it probably works quite well on arches that do have irq_to_gpio(),
>>>>> let's ask Daniel and Sven if they still have this hardware and if they
>>>>> can try the patch below that implements what you suggested.
>>>>
>>>> This hardware is still in use and we also still follow kernel
>>>> development and try to update our customer devices to recent kernel
>>>> versions regularly. Currently we are at 3.1.10 and the touchscreen
>>>> works well with that. I'll try to update to a more recent kernel
>>>> next week and will try your patch.
>>>>
>>>
>>> Did you have a chance to test the patch?
>>
>> *ping*
>>
>> It would be nice to get driver in mainline compile [and work] again...
> 
> We gave that patch a quick try today and it doesn't seem to work. We
> don't get any events from the touch screen anymore. We need to debug
> this further, hopefully by the end of this week.
> 
> If there's anything obvious in the setup of the threaded IRQ handler,
> please let us know. Otherwise, I'll get back once I have a fixed version
> of the patch.

Ok, finally I found some time. In general, the patch works fine. The
only detail I had to amend was the irqflags, which were changed from
IRQF_TRIGGER_RISING/IRQF_TRIGGER_FALLING to
IRQF_TRIGGER_HIGH/IRQF_TRIGGER_LOW, which doesn't work as the PXA can't
deal with level-based IRQs. Changing this back to RISING/FALLING makes
the driver work again.

With that correction, feel free to add my Acked-by:/Tested-by:

Thanks and sorry again for the slow response.

Daniel

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Dmitry Torokhov]

On Thu, Jul 19, 2012 at 05:36:12PM +0200, Daniel Mack wrote:
> On 17.07.2012 14:59, Daniel Mack wrote:
> > On 13.07.2012 09:01, Dmitry Torokhov wrote:
> >> On Wed, May 02, 2012 at 09:36:51PM -0700, Dmitry Torokhov wrote:
> >>> Hi Sven,
> >>>
> >>> On Sat, Apr 07, 2012 at 08:38:33PM +0200, Sven Neumann wrote:
> >>>> Hi,
> >>>>
> >>>> On 07.04.12 09:02, Dmitry Torokhov wrote:
> >>>>> On Fri, Apr 06, 2012 at 10:40:07PM -0700, Olof Johansson wrote:
> >>>>>> This seems to have been broken since 2010, so obviously noone actually
> >>>>>> cares about the driver:
> >>>>>>
> >>>>>> make[4]: *** [drivers/input/touchscreen/eeti_ts.o] Error 1
> >>>>>> drivers/input/touchscreen/eeti_ts.c: In function 'eeti_ts_irq_active':
> >>>>>> drivers/input/touchscreen/eeti_ts.c:65:2: error: implicit declaration of function 'irq_to_gpio' [-Werror=implicit-function-declaration]
> >>>>>>
> >>>>>> irq_to_gpio isn't available on most platforms today, so the driver
> >>>>>> will need some rework by someone who has hardware access and can test
> >>>>>> (to make sure that, for example, switching to level interrupts and just
> >>>>>> keep taking them while there's more to process works).
> >>>>>>
> >>>>>> I guess it could just be scheduled for removal, but let's start with
> >>>>>> marking it CONFIG_BROKEN.
> >>>>>
> >>>>> Well, it probably works quite well on arches that do have irq_to_gpio(),
> >>>>> let's ask Daniel and Sven if they still have this hardware and if they
> >>>>> can try the patch below that implements what you suggested.
> >>>>
> >>>> This hardware is still in use and we also still follow kernel
> >>>> development and try to update our customer devices to recent kernel
> >>>> versions regularly. Currently we are at 3.1.10 and the touchscreen
> >>>> works well with that. I'll try to update to a more recent kernel
> >>>> next week and will try your patch.
> >>>>
> >>>
> >>> Did you have a chance to test the patch?
> >>
> >> *ping*
> >>
> >> It would be nice to get driver in mainline compile [and work] again...
> > 
> > We gave that patch a quick try today and it doesn't seem to work. We
> > don't get any events from the touch screen anymore. We need to debug
> > this further, hopefully by the end of this week.
> > 
> > If there's anything obvious in the setup of the threaded IRQ handler,
> > please let us know. Otherwise, I'll get back once I have a fixed version
> > of the patch.
> 
> Ok, finally I found some time. In general, the patch works fine. The
> only detail I had to amend was the irqflags, which were changed from
> IRQF_TRIGGER_RISING/IRQF_TRIGGER_FALLING to
> IRQF_TRIGGER_HIGH/IRQF_TRIGGER_LOW, which doesn't work as the PXA can't
> deal with level-based IRQs. Changing this back to RISING/FALLING makes
> the driver work again.

Hmm, but that would mean we need to restore reading the data in open()
to make sure we re-arm IRQ in case somebody touched the screen before it
was opened by userspace...

Thanks.

-- 
Dmitry

Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[Daniel Mack]

On 23.07.2012 18:51, Dmitry Torokhov wrote:
> On Thu, Jul 19, 2012 at 05:36:12PM +0200, Daniel Mack wrote:

>> Ok, finally I found some time. In general, the patch works fine. The
>> only detail I had to amend was the irqflags, which were changed from
>> IRQF_TRIGGER_RISING/IRQF_TRIGGER_FALLING to
>> IRQF_TRIGGER_HIGH/IRQF_TRIGGER_LOW, which doesn't work as the PXA can't
>> deal with level-based IRQs. Changing this back to RISING/FALLING makes
>> the driver work again.
> 
> Hmm, but that would mean we need to restore reading the data in open()
> to make sure we re-arm IRQ in case somebody touched the screen before it
> was opened by userspace...

Hmm, right, that was the reason why I put it there in the first place.
Thanks for the heads-up. Would you do it? Want me to?


Daniel

Emulating level IRQs (was: Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN)

[Daniel Mack]

On 23.07.2012 18:51, Dmitry Torokhov wrote:
> On Thu, Jul 19, 2012 at 05:36:12PM +0200, Daniel Mack wrote:

>> Ok, finally I found some time. In general, the patch works fine. The
>> only detail I had to amend was the irqflags, which were changed from
>> IRQF_TRIGGER_RISING/IRQF_TRIGGER_FALLING to
>> IRQF_TRIGGER_HIGH/IRQF_TRIGGER_LOW, which doesn't work as the PXA can't
>> deal with level-based IRQs. Changing this back to RISING/FALLING makes
>> the driver work again.
> 
> Hmm, but that would mean we need to restore reading the data in open()
> to make sure we re-arm IRQ in case somebody touched the screen before it
> was opened by userspace...

I had another look at this and don't really know what to do here. We
definitely need level interrupts for this device as the interrupt line's
level is the only that tells us when we can stop reading from the
device. So it's not just the start condition that bites us here.

I copied some people that might help find a solution.

To summarize the problem: The EETI touchscreen is a device that asserts
a GPIO line when it has events to deliver and waits for I2C commands to
empty its buffers. When there are no more buffered events, it will
de-assert the line.

This device is connected to a PXA GPIO that is only able to deliver edge
IRQs, and the old implemenation was to wait for an interrupt and then
read data as long as the IRQ's corresponding GPIO was asserted. However,
expecting that an IRQ is mappable to a GPIO is not something we should
do, so the only clean solution is to teach the PXA GPIO controller level
IRQs.

So it boils down to the question: Is there any easy and generic way to
emulate level irq on chips that don't support that natively?


Daniel

Re: Emulating level IRQs (was: Re: [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN)

[Mark Brown]

On Tue, Jul 24, 2012 at 08:01:56PM +0200, Daniel Mack wrote:
> On 23.07.2012 18:51, Dmitry Torokhov wrote:

> > Hmm, but that would mean we need to restore reading the data in open()
> > to make sure we re-arm IRQ in case somebody touched the screen before it
> > was opened by userspace...

> I had another look at this and don't really know what to do here. We
> definitely need level interrupts for this device as the interrupt line's
> level is the only that tells us when we can stop reading from the
> device. So it's not just the start condition that bites us here.

> I copied some people that might help find a solution.

I've raised the same issue myself, it's fairly common.

> So it boils down to the question: Is there any easy and generic way to
> emulate level irq on chips that don't support that natively?

Nothing in core.  The nearest thing is to poll until you run out of work
(which is rude but survivable for threaded IRQs that don't assert too
much), ideally just reusing the level IRQ if it can report IRQ_NONE.

Re: Emulating level IRQs

[Daniel Mack]

On 24.07.2012 20:01, Daniel Mack wrote:
> On 23.07.2012 18:51, Dmitry Torokhov wrote:
>> On Thu, Jul 19, 2012 at 05:36:12PM +0200, Daniel Mack wrote:
> 
>>> Ok, finally I found some time. In general, the patch works fine. The
>>> only detail I had to amend was the irqflags, which were changed from
>>> IRQF_TRIGGER_RISING/IRQF_TRIGGER_FALLING to
>>> IRQF_TRIGGER_HIGH/IRQF_TRIGGER_LOW, which doesn't work as the PXA can't
>>> deal with level-based IRQs. Changing this back to RISING/FALLING makes
>>> the driver work again.
>>
>> Hmm, but that would mean we need to restore reading the data in open()
>> to make sure we re-arm IRQ in case somebody touched the screen before it
>> was opened by userspace...
> 
> I had another look at this and don't really know what to do here. We
> definitely need level interrupts for this device as the interrupt line's
> level is the only that tells us when we can stop reading from the
> device. So it's not just the start condition that bites us here.
> 
> I copied some people that might help find a solution.
> 
> To summarize the problem: The EETI touchscreen is a device that asserts
> a GPIO line when it has events to deliver and waits for I2C commands to
> empty its buffers. When there are no more buffered events, it will
> de-assert the line.
> 
> This device is connected to a PXA GPIO that is only able to deliver edge
> IRQs, and the old implemenation was to wait for an interrupt and then
> read data as long as the IRQ's corresponding GPIO was asserted. However,
> expecting that an IRQ is mappable to a GPIO is not something we should
> do, so the only clean solution is to teach the PXA GPIO controller level
> IRQs.
> 
> So it boils down to the question: Is there any easy and generic way to
> emulate level irq on chips that don't support that natively?

Otherwise, we would need some sort of generic irq_to_gpio() again, and
the interrupt line the driver listens to must have support for that sort
of mapping.

Any opinion on this, anyone?



Thanks,
Daniel

Re: Emulating level IRQs

[Haojian Zhuang]

On Mon, Aug 6, 2012 at 12:22 AM, Daniel Mack <zonque@gmail.com> wrote:
> On 24.07.2012 20:01, Daniel Mack wrote:
>> On 23.07.2012 18:51, Dmitry Torokhov wrote:
>>> On Thu, Jul 19, 2012 at 05:36:12PM +0200, Daniel Mack wrote:
>>
>>>> Ok, finally I found some time. In general, the patch works fine. The
>>>> only detail I had to amend was the irqflags, which were changed from
>>>> IRQF_TRIGGER_RISING/IRQF_TRIGGER_FALLING to
>>>> IRQF_TRIGGER_HIGH/IRQF_TRIGGER_LOW, which doesn't work as the PXA can't
>>>> deal with level-based IRQs. Changing this back to RISING/FALLING makes
>>>> the driver work again.
>>>
>>> Hmm, but that would mean we need to restore reading the data in open()
>>> to make sure we re-arm IRQ in case somebody touched the screen before it
>>> was opened by userspace...
>>
>> I had another look at this and don't really know what to do here. We
>> definitely need level interrupts for this device as the interrupt line's
>> level is the only that tells us when we can stop reading from the
>> device. So it's not just the start condition that bites us here.
>>
>> I copied some people that might help find a solution.
>>
>> To summarize the problem: The EETI touchscreen is a device that asserts
>> a GPIO line when it has events to deliver and waits for I2C commands to
>> empty its buffers. When there are no more buffered events, it will
>> de-assert the line.
>>
>> This device is connected to a PXA GPIO that is only able to deliver edge
>> IRQs, and the old implemenation was to wait for an interrupt and then
>> read data as long as the IRQ's corresponding GPIO was asserted. However,
>> expecting that an IRQ is mappable to a GPIO is not something we should
>> do, so the only clean solution is to teach the PXA GPIO controller level
>> IRQs.
>>
>> So it boils down to the question: Is there any easy and generic way to
>> emulate level irq on chips that don't support that natively?
>
> Otherwise, we would need some sort of generic irq_to_gpio() again, and
> the interrupt line the driver listens to must have support for that sort
> of mapping.
>
> Any opinion on this, anyone?
>
Since you're using gpio as input, you need to call gpio_request() and set it
as input direction. And you could also transfer the gpio number into touch
driver via platform data. Is it OK for you?

Regards
Haojian

Re: Emulating level IRQs

[Daniel Mack]

On 05.08.2012 18:56, Haojian Zhuang wrote:
> On Mon, Aug 6, 2012 at 12:22 AM, Daniel Mack <zonque@gmail.com> wrote:
>> On 24.07.2012 20:01, Daniel Mack wrote:
>>> On 23.07.2012 18:51, Dmitry Torokhov wrote:
>>>> On Thu, Jul 19, 2012 at 05:36:12PM +0200, Daniel Mack wrote:
>>>
>>>>> Ok, finally I found some time. In general, the patch works fine. The
>>>>> only detail I had to amend was the irqflags, which were changed from
>>>>> IRQF_TRIGGER_RISING/IRQF_TRIGGER_FALLING to
>>>>> IRQF_TRIGGER_HIGH/IRQF_TRIGGER_LOW, which doesn't work as the PXA can't
>>>>> deal with level-based IRQs. Changing this back to RISING/FALLING makes
>>>>> the driver work again.
>>>>
>>>> Hmm, but that would mean we need to restore reading the data in open()
>>>> to make sure we re-arm IRQ in case somebody touched the screen before it
>>>> was opened by userspace...
>>>
>>> I had another look at this and don't really know what to do here. We
>>> definitely need level interrupts for this device as the interrupt line's
>>> level is the only that tells us when we can stop reading from the
>>> device. So it's not just the start condition that bites us here.
>>>
>>> I copied some people that might help find a solution.
>>>
>>> To summarize the problem: The EETI touchscreen is a device that asserts
>>> a GPIO line when it has events to deliver and waits for I2C commands to
>>> empty its buffers. When there are no more buffered events, it will
>>> de-assert the line.
>>>
>>> This device is connected to a PXA GPIO that is only able to deliver edge
>>> IRQs, and the old implemenation was to wait for an interrupt and then
>>> read data as long as the IRQ's corresponding GPIO was asserted. However,
>>> expecting that an IRQ is mappable to a GPIO is not something we should
>>> do, so the only clean solution is to teach the PXA GPIO controller level
>>> IRQs.
>>>
>>> So it boils down to the question: Is there any easy and generic way to
>>> emulate level irq on chips that don't support that natively?
>>
>> Otherwise, we would need some sort of generic irq_to_gpio() again, and
>> the interrupt line the driver listens to must have support for that sort
>> of mapping.
>>
>> Any opinion on this, anyone?
>>
> Since you're using gpio as input, you need to call gpio_request() and set it
> as input direction. And you could also transfer the gpio number into touch
> driver via platform data. Is it OK for you?

No, that's not the point. What we get via the i2c runtime data is an
interrupt number. The driver is driven by that interrupt and doesn't
poll on a GPIO line, which is how it should be.

However, in order to know when to stop reading from the device, we need
to monitor the GPIO line after the interrupt has arrived, and read as
long as the line is asserted. Then we stop reading and wait for the next
interrupt to arrive.

Hence, what we need here is either a GPIO/IRQ controller that is able to
handle level-IRQs (which the PXA can't do), or we need to have a generic
way to map IRQ lines back to GPIOs.

Of course, I could pass the GPIO in the platform data and the IRQ in the
I2C data and leave it to the user of the driver to keep both values in
sync, but I wanted to avoid that.


Daniel

Re: Emulating level IRQs

[Eric Miao]

On Mon, Aug 6, 2012 at 1:56 AM, Daniel Mack <zonque@gmail.com> wrote:
> On 05.08.2012 18:56, Haojian Zhuang wrote:
>> On Mon, Aug 6, 2012 at 12:22 AM, Daniel Mack <zonque@gmail.com> wrote:
>>> On 24.07.2012 20:01, Daniel Mack wrote:
>>>> On 23.07.2012 18:51, Dmitry Torokhov wrote:
>>>>> On Thu, Jul 19, 2012 at 05:36:12PM +0200, Daniel Mack wrote:
>>>>
>>>>>> Ok, finally I found some time. In general, the patch works fine. The
>>>>>> only detail I had to amend was the irqflags, which were changed from
>>>>>> IRQF_TRIGGER_RISING/IRQF_TRIGGER_FALLING to
>>>>>> IRQF_TRIGGER_HIGH/IRQF_TRIGGER_LOW, which doesn't work as the PXA can't
>>>>>> deal with level-based IRQs. Changing this back to RISING/FALLING makes
>>>>>> the driver work again.
>>>>>
>>>>> Hmm, but that would mean we need to restore reading the data in open()
>>>>> to make sure we re-arm IRQ in case somebody touched the screen before it
>>>>> was opened by userspace...
>>>>
>>>> I had another look at this and don't really know what to do here. We
>>>> definitely need level interrupts for this device as the interrupt line's
>>>> level is the only that tells us when we can stop reading from the
>>>> device. So it's not just the start condition that bites us here.
>>>>
>>>> I copied some people that might help find a solution.
>>>>
>>>> To summarize the problem: The EETI touchscreen is a device that asserts
>>>> a GPIO line when it has events to deliver and waits for I2C commands to
>>>> empty its buffers. When there are no more buffered events, it will
>>>> de-assert the line.
>>>>
>>>> This device is connected to a PXA GPIO that is only able to deliver edge
>>>> IRQs, and the old implemenation was to wait for an interrupt and then
>>>> read data as long as the IRQ's corresponding GPIO was asserted. However,
>>>> expecting that an IRQ is mappable to a GPIO is not something we should
>>>> do, so the only clean solution is to teach the PXA GPIO controller level
>>>> IRQs.
>>>>
>>>> So it boils down to the question: Is there any easy and generic way to
>>>> emulate level irq on chips that don't support that natively?
>>>
>>> Otherwise, we would need some sort of generic irq_to_gpio() again, and
>>> the interrupt line the driver listens to must have support for that sort
>>> of mapping.
>>>
>>> Any opinion on this, anyone?
>>>
>> Since you're using gpio as input, you need to call gpio_request() and set it
>> as input direction. And you could also transfer the gpio number into touch
>> driver via platform data. Is it OK for you?
>
> No, that's not the point. What we get via the i2c runtime data is an
> interrupt number. The driver is driven by that interrupt and doesn't
> poll on a GPIO line, which is how it should be.
>
> However, in order to know when to stop reading from the device, we need
> to monitor the GPIO line after the interrupt has arrived, and read as
> long as the line is asserted. Then we stop reading and wait for the next
> interrupt to arrive.
>
> Hence, what we need here is either a GPIO/IRQ controller that is able to
> handle level-IRQs (which the PXA can't do), or we need to have a generic
> way to map IRQ lines back to GPIOs.
>
> Of course, I could pass the GPIO in the platform data and the IRQ in the
> I2C data and leave it to the user of the driver to keep both values in
> sync, but I wanted to avoid that.

I see no better way except to encode the GPIO line into the platform data.
In order to solve the sync issue, I personally think mapping the GPIO to IRQ
would be better here, and ignore the irq value from the I2C data. A forward
mapping of gpio_to_irq() will be less problematic here, and for those platforms
where gpio_to_irq() returns invalid, those platforms are probably not desirable
for this chip.

So my understanding, if it's correct, that we can treat the EETI chip as having
two separate inputs: one IRQ line (for the event notification) and one GPIO line
(for a condition where data are emptied), we could naturally have two numbers
in the driver, but unfortunately they end up being in sync as they are
physically
one pin.

And Daniel, I haven't looked into the driver myself, I guess you might need to
change the pin role to GPIO with GPIO API explicitly at run-time, e.g.
gpio_direction_input() followed by gpio_get_value(), but I believe you should
have already done that good enough as always :-)

Re: Emulating level IRQs

[Mark Brown]

On Mon, Aug 06, 2012 at 09:45:13AM +0800, Eric Miao wrote:

> So my understanding, if it's correct, that we can treat the EETI chip as having
> two separate inputs: one IRQ line (for the event notification) and one GPIO line
> (for a condition where data are emptied), we could naturally have two numbers
> in the driver, but unfortunately they end up being in sync as they are
> physically
> one pin.

...unless the interrupt controller supports level IRQs at which point we
don't need the GPIO, of course :/ .

Re: Emulating level IRQs

[Daniel Mack]

[-- Attachment #1: Type: text/plain, Size: 900 bytes --]

On 06.08.2012 18:36, Mark Brown wrote:
> On Mon, Aug 06, 2012 at 09:45:13AM +0800, Eric Miao wrote:
> 
>> So my understanding, if it's correct, that we can treat the EETI chip as having
>> two separate inputs: one IRQ line (for the event notification) and one GPIO line
>> (for a condition where data are emptied), we could naturally have two numbers
>> in the driver, but unfortunately they end up being in sync as they are
>> physically
>> one pin.
> 
> ...unless the interrupt controller supports level IRQs at which point we
> don't need the GPIO, of course :/ .

Exactly. My question was rather - like the subject says ;) - if there is
a way to emulate level IRQs on controllers that can't do that natively.

But ok, I followed Eric's suggestion now and implemented it that way.
This also works fine for me.

Dmitry, are you fine with that patch?


Many thanks for the input, everyone!
Daniel



[-- Attachment #2: 0001-Input-eeti_ts-pass-gpio-value-instead-of-IRQ.patch --]
[-- Type: text/x-patch, Size: 3730 bytes --]

>From bfb14c1a0417435ebcf5bdebbb94ae6812cb4aee Mon Sep 17 00:00:00 2001
From: Daniel Mack <zonque@gmail.com>
Date: Tue, 7 Aug 2012 17:02:59 +0200
Subject: [PATCH] Input: eeti_ts: pass gpio value instead of IRQ

The EETI touchscreen asserts its IRQ line as soon as it has data in its
internal buffers. The line is automatically deasserted once all data has
been read via I2C. Hence, the driver has to monitor the GPIO line and
cannot simply rely on the interrupt handler reception.

In the current implementation of the driver, irq_to_gpio() is used to
determine the GPIO number from the i2c_client's IRQ value.

As irq_to_gpio() is not available on all platforms, this patch changes
this and makes the driver ignore the passed in IRQ. Instead, a GPIO is
added to the platform_data struct and gpio_to_irq is used to derive the
IRQ from that GPIO. If this fails, bail out. The driver is only able to
work in environments where the touchscreen GPIO can be mapped to an
IRQ.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/input/touchscreen/eeti_ts.c | 21 +++++++++++++--------
 include/linux/input/eeti_ts.h       |  1 +
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/input/touchscreen/eeti_ts.c b/drivers/input/touchscreen/eeti_ts.c
index 503c709..908407e 100644
--- a/drivers/input/touchscreen/eeti_ts.c
+++ b/drivers/input/touchscreen/eeti_ts.c
@@ -48,7 +48,7 @@ struct eeti_ts_priv {
 	struct input_dev *input;
 	struct work_struct work;
 	struct mutex mutex;
-	int irq, irq_active_high;
+	int irq_gpio, irq, irq_active_high;
 };
 
 #define EETI_TS_BITDEPTH	(11)
@@ -62,7 +62,7 @@ struct eeti_ts_priv {
 
 static inline int eeti_ts_irq_active(struct eeti_ts_priv *priv)
 {
-	return gpio_get_value(irq_to_gpio(priv->irq)) == priv->irq_active_high;
+	return gpio_get_value(priv->irq_gpio) == priv->irq_active_high;
 }
 
 static void eeti_ts_read(struct work_struct *work)
@@ -157,7 +157,7 @@ static void eeti_ts_close(struct input_dev *dev)
 static int __devinit eeti_ts_probe(struct i2c_client *client,
 				   const struct i2c_device_id *idp)
 {
-	struct eeti_ts_platform_data *pdata;
+	struct eeti_ts_platform_data *pdata = client->dev.platform_data;
 	struct eeti_ts_priv *priv;
 	struct input_dev *input;
 	unsigned int irq_flags;
@@ -199,9 +199,12 @@ static int __devinit eeti_ts_probe(struct i2c_client *client,
 
 	priv->client = client;
 	priv->input = input;
-	priv->irq = client->irq;
+	priv->irq_gpio = pdata->irq_gpio;
+	priv->irq = gpio_to_irq(pdata->irq_gpio);
 
-	pdata = client->dev.platform_data;
+	err = gpio_request_one(pdata->irq_gpio, GPIOF_IN, client->name);
+	if (err < 0)
+		goto err1;
 
 	if (pdata)
 		priv->irq_active_high = pdata->irq_active_high;
@@ -215,13 +218,13 @@ static int __devinit eeti_ts_probe(struct i2c_client *client,
 
 	err = input_register_device(input);
 	if (err)
-		goto err1;
+		goto err2;
 
 	err = request_irq(priv->irq, eeti_ts_isr, irq_flags,
 			  client->name, priv);
 	if (err) {
 		dev_err(&client->dev, "Unable to request touchscreen IRQ.\n");
-		goto err2;
+		goto err3;
 	}
 
 	/*
@@ -233,9 +236,11 @@ static int __devinit eeti_ts_probe(struct i2c_client *client,
 	device_init_wakeup(&client->dev, 0);
 	return 0;
 
-err2:
+err3:
 	input_unregister_device(input);
 	input = NULL; /* so we dont try to free it below */
+err2:
+	gpio_free(pdata->irq_gpio);
 err1:
 	input_free_device(input);
 	kfree(priv);
diff --git a/include/linux/input/eeti_ts.h b/include/linux/input/eeti_ts.h
index f875b31..16625d7 100644
--- a/include/linux/input/eeti_ts.h
+++ b/include/linux/input/eeti_ts.h
@@ -2,6 +2,7 @@
 #define LINUX_INPUT_EETI_TS_H
 
 struct eeti_ts_platform_data {
+	int irq_gpio;
 	unsigned int irq_active_high;
 };
 
-- 
1.7.11.2