Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1506807 > unrolled thread

Re: [PATCH] gpio: mpc8xxx: Correct irq handler function

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] gpio: mpc8xxx: Correct irq handler function Linus Walleij <linus.walleij@linaro.org> - 2016-10-24 02:30 +0200
    RE: [PATCH] gpio: mpc8xxx: Correct irq handler function Gang Liu <gang.liu@nxp.com> - 2016-10-24 06:10 +0200
      Re: [PATCH] gpio: mpc8xxx: Correct irq handler function Linus Walleij <linus.walleij@linaro.org> - 2016-10-25 11:20 +0200

#1506807 — Re: [PATCH] gpio: mpc8xxx: Correct irq handler function

On Fri, Oct 21, 2016 at 9:31 AM, Liu Gang <Gang.Liu@nxp.com> wrote:

> From the beginning of the gpio-mpc8xxx.c, the "handle_level_irq"
> has being used to handle GPIO interrupts in the PowerPC/Layerscape
> platforms. But actually, almost all PowerPC/Layerscape platforms
> assert an interrupt request upon either a high-to-low change or
> any change on the state of the signal.
>
> So the "handle_level_irq" is not reasonable for PowerPC/Layerscape
> GPIO interrupt, it should be "handle_edge_irq". Otherwise the system
> may lost some interrupts from the PIN's state changes.
>
> Signed-off-by: Liu Gang <Gang.Liu@nxp.com>

Yes and especially so since this irqchip implements .irq_ack() and
unless you use the edge handler it will never be called.

Patch applied for fixes, tell me if it also needs to go into stable.

If you want to look closer at this driver, I think it is possible to
simplify it quite a bit by using GPIOLIB_IRQCHIP like a few
other drivers, for example gpio-pl061.c. Patches accepted :)

Yours,
Linus Walleij

[toc] | [next] | [standalone]

#1506861

 
> On Fri, Oct 21, 2016 at 9:31 AM, Liu Gang <Gang.Liu@nxp.com> wrote:
> 
> > From the beginning of the gpio-mpc8xxx.c, the "handle_level_irq"
> > has being used to handle GPIO interrupts in the PowerPC/Layerscape
> > platforms. But actually, almost all PowerPC/Layerscape platforms
> > assert an interrupt request upon either a high-to-low change or any
> > change on the state of the signal.
> >
> > So the "handle_level_irq" is not reasonable for PowerPC/Layerscape
> > GPIO interrupt, it should be "handle_edge_irq". Otherwise the system
> > may lost some interrupts from the PIN's state changes.
> >
> > Signed-off-by: Liu Gang <Gang.Liu@nxp.com>
> 
> Yes and especially so since this irqchip implements .irq_ack() and unless you
> use the edge handler it will never be called.
> 
> Patch applied for fixes, tell me if it also needs to go into stable.
> 
[Liu Gang] yes, please also put it into stable. Thanks!

> If you want to look closer at this driver, I think it is possible to simplify it quite a
> bit by using GPIOLIB_IRQCHIP like a few other drivers, for example gpio-pl061.c.
> Patches accepted :)
> 
> Yours,
> Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1508106

On Mon, Oct 24, 2016 at 5:46 AM, Gang Liu <gang.liu@nxp.com> wrote:
>> On Fri, Oct 21, 2016 at 9:31 AM, Liu Gang <Gang.Liu@nxp.com> wrote:
>>
>> > From the beginning of the gpio-mpc8xxx.c, the "handle_level_irq"
>> > has being used to handle GPIO interrupts in the PowerPC/Layerscape
>> > platforms. But actually, almost all PowerPC/Layerscape platforms
>> > assert an interrupt request upon either a high-to-low change or any
>> > change on the state of the signal.
>> >
>> > So the "handle_level_irq" is not reasonable for PowerPC/Layerscape
>> > GPIO interrupt, it should be "handle_edge_irq". Otherwise the system
>> > may lost some interrupts from the PIN's state changes.
>> >
>> > Signed-off-by: Liu Gang <Gang.Liu@nxp.com>
>>
>> Yes and especially so since this irqchip implements .irq_ack() and unless you
>> use the edge handler it will never be called.
>>
>> Patch applied for fixes, tell me if it also needs to go into stable.
>>
> [Liu Gang] yes, please also put it into stable. Thanks!

Ooops too late. Will tell Greg to pick this separately.

Yours,
Linus Walleij

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web