[PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN

[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

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

[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

[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

[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