* [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN
2012-04-07 20:32 ` [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN Olof Johansson
@ 2012-04-07 21:04 ` Joachim Eastwood
2012-04-09 2:28 ` Haojian Zhuang
2012-04-10 10:10 ` Russell King - ARM Linux
2 siblings, 0 replies; 5+ messages in thread
From: Joachim Eastwood @ 2012-04-07 21:04 UTC (permalink / raw)
To: linux-arm-kernel
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/
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN
2012-04-07 20:32 ` [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN Olof Johansson
2012-04-07 21:04 ` Joachim Eastwood
@ 2012-04-09 2:28 ` Haojian Zhuang
2012-04-10 10:10 ` Russell King - ARM Linux
2 siblings, 0 replies; 5+ messages in thread
From: Haojian Zhuang @ 2012-04-09 2:28 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN
2012-04-07 20:32 ` [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN Olof Johansson
2012-04-07 21:04 ` Joachim Eastwood
2012-04-09 2:28 ` Haojian Zhuang
@ 2012-04-10 10:10 ` Russell King - ARM Linux
2012-04-10 16:01 ` Dmitry Torokhov
2 siblings, 1 reply; 5+ messages in thread
From: Russell King - ARM Linux @ 2012-04-10 10:10 UTC (permalink / raw)
To: linux-arm-kernel
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] Input: eeti_ts: Mark as CONFIG_BROKEN
2012-04-10 10:10 ` Russell King - ARM Linux
@ 2012-04-10 16:01 ` Dmitry Torokhov
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2012-04-10 16:01 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread