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
On Mon, Aug 6, 2012 at 12:22 AM, Daniel Mack <[email protected]> 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
On 05.08.2012 18:56, Haojian Zhuang wrote:
> On Mon, Aug 6, 2012 at 12:22 AM, Daniel Mack <[email protected]> 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
On Mon, Aug 6, 2012 at 1:56 AM, Daniel Mack <[email protected]> wrote:
> On 05.08.2012 18:56, Haojian Zhuang wrote:
>> On Mon, Aug 6, 2012 at 12:22 AM, Daniel Mack <[email protected]> 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 :-)
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 :/ .
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