Re: [PATCH] extcon: gpio: Request reasonable interrupts

[Linus Walleij]

On Mon, Jun 24, 2019 at 2:08 AM Chanwoo Choi <cw00.choi@xxxxxxxxxxx> wrote:
> On 19. 6. 8. ìì 6:24, Linus Walleij wrote:
> > On Tue, Jun 4, 2019 at 3:30 AM Chanwoo Choi <cw00.choi@xxxxxxxxxxx> wrote:
> >> On 19. 5. 31. ìì 3:39, Linus Walleij wrote:
> >
> >>> +     /*
> >>> +      * It is unlikely that this is an acknowledged interrupt that goes
> >>> +      * away after handling, what we are looking for are falling edges
> >>> +      * if the signal is active low, and rising edges if the signal is
> >>> +      * active high.
> >>> +      */
> >>> +     if (gpiod_is_active_low(data->gpiod))
> >>> +             irq_flags = IRQF_TRIGGER_FALLING;
> >>
> >> If gpiod_is_active_low(data->gpiod) is true, irq_flags might be
> >> IRQF_TRIGGER_LOW instead of IRQF_TRIGGER_FALLING. How can we sure
> >> that irq_flags is always IRQF_TRIGGER_FALLING?
> >
> > OK correct me if I'm wrong, but this is an external connector and
> > the GPIO goes low/high when the connector is physically inserted.
> > If it was level trigged, it would lock up the CPU with interrupts until
> > it was unplugged again, since there is no way to acknowledge a
> > level IRQ.
> >
> > I think level IRQ on GPIOs are only used for logic peripherals
> > such as ethernet controllers etc where you can talk to the peripheral
> > and get it to deassert the line and thus acknowledge the IRQ.
> >
> > So the way I see it only edge triggering makes sense for extcon.
> >
> > Correct me if I'm wrong.
>
> Sorry for late reply because of vacation.

Don't worry I am not in a hurry. This is clean-up work :)

> Actually, I have not thought that the kind of irq_flags are fixed
> according to the category of specific h/w device. Until now, as I knew,
> the h/w device have to initialize the the kind of irq_flags
> for each peripheral device dependency. The each vendor of peripheral device
> might design the kind of the kind of irq-flags for detection.
>
> If possible, could you provide some example on mainline kernel?

I don't know exactly what kind of example you are looking
for, but in e.g. drivers/input/keyboard/gpio_keys.c
you find this code:

                isr = gpio_keys_gpio_isr;
                irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;

                switch (button->wakeup_event_action) {
                case EV_ACT_ASSERTED:
                        bdata->wakeup_trigger_type = active_low ?
                                IRQ_TYPE_EDGE_FALLING : IRQ_TYPE_EDGE_RISING;
                        break;
                case EV_ACT_DEASSERTED:
                        bdata->wakeup_trigger_type = active_low ?
                                IRQ_TYPE_EDGE_RISING : IRQ_TYPE_EDGE_FALLING;
                        break;

Is this what you're looking for?

Yours,
Linus Walleij