Re: [PATCH RESEND RFC 1/4] drivers: pinctrl: qcom: add wakeup capability to GPIO

[Lina Iyer]

On Thu, Aug 02 2018 at 01:27 -0600, Marc Zyngier wrote:
> On Thu, 02 Aug 2018 07:51:04 +0100,
> Lina Iyer <ilina@xxxxxxxxxxxxxx> wrote:
> > On Thu, Aug 02 2018 at 00:08 -0600, Marc Zyngier wrote:
> > > Hi Lina,
> > >
> > > On Wed, 01 Aug 2018 20:45:38 +0100,
> > > Lina Iyer <ilina@xxxxxxxxxxxxxx> wrote:
> > >>
> > >> Thanks for the feedback, Marc.
> > >>
> > >> On Wed, Aug 01 2018 at 00:31 -0600, Marc Zyngier wrote:
> > >> > On Wed, 01 Aug 2018 03:00:18 +0100,
> > >> > Lina Iyer <ilina@xxxxxxxxxxxxxx> wrote:
> > >> >>
> > >> >> +static irqreturn_t wake_irq_gpio_handler(int irq, void *data)
> > >> >> +{
> > >> >> +	struct irq_data *irqd = data;
> > >> >> +	struct irq_desc *desc = irq_data_to_desc(irqd);
> > >> >> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> > >> >> +	struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd);
> > >> >> +	int irq_pin = irq_find_mapping(gc->irq.domain, irqd->hwirq);
> > >> >> +
> > >> >> +	chained_irq_enter(chip, desc);
> > >> >> +	generic_handle_irq(irq_pin);
> > >> >> +	chained_irq_exit(chip, desc);
> > >> >
> > >> > That's crazy. I'm not even commenting on the irq handler vs chained
> > >> > irqchip thing, but directly calling into a completely different part
> > >> > of the irq hierarchy makes me feel nauseous,
> > >> >
> > >> I know the sentiment; I am not too happy about it myself. Explanation
> > >> below.
> > >>
> > >> > Why isn't the interrupt still pending at the pinctrl level? Looking at
> > >> > the diagram in the cover letter, I'd have hoped that the signal routed
> > >> > to the PDC would wakeup the GIC, but that by virtue of being *also*
> > >> > wired to the TLMM, the interrupt would be handled via the normal path.
> > >> >
> > >> The short answer: TLMM is not active to sense a wakeup interrupt.
> > >
> > > I get that bit. See below for the bit I don't get.
> > >
> > >> When we enter system sleep mode, the TLMM and the GIC are powered off
> > >> and the PDC is the only powered-on interrupt controller. The IRQs
> > >> configured at the PDC are the only ones capable of waking the system.
> > >> Upon sensing the interrupt, the PDC intiates a system wakeup and replays
> > >> the interrupt to the GIC. The GIC relays that to AP. Unfortunately, the
> > >> interrupts from the GPIO do not trigger the TLMM summary line. Therefore
> > >> this handler needs to figure out what GPIO caused the wakeup and notify
> > >> the corresponding driver.
> > >
> > > That's most bizarre. What causes the TLMM output line not to reflect
> > > the fact that an input is asserted?
> > There is a parallel line that is directed from the PIN directly to the
> > PDC in addition to the TLMM. This line does not go through the TLMM.
>
> I got that from the cover letter.
>
> > Active path						_____
> >                 /-------------- > [ TLMM ] --------> |     |
> >            [ Device GPIO ]	   	     summary   | GIC | ==>
> > 		\				       |     |
> > 		  ----------------> [ PDC ] ---------> |_____|
> > Wakeup path	     GPIO as IRQ		IRQ
> >
> > > I understand that it has been
> > > turned off, but surely the PDC wakes it up at the same time as the
> > > GIC, and it should realise that there is something pending...
> > >
> > PDC is always-on and when it detects any interrupt, will wakeup the GIC
> > and then replay the interrupt line to the GIC. This interrupt line is
> > not the summary line, but a separate line from GPIO/PIN to the PDC.
> >
> > Since the TLMM was also in low power mode, when the GIC was powered
> > down, the TLMM never sees the IRQ and therefore will not send the
> > summary line to GIC. The wakeup path is GPIO -> PDC -> GIC.
>
> Sure. But once woken up (GIC *and* TLMM), the gpio line (which I
> assume is level) is still high at the TLMM input. So why isn't it
> registering that state once it has been woken up?
>
> I can understand that it would be missing an edge. But that doesn't
> hold for level signalling.
Sure, yes. Sorry for not registering your point in my response.
Once woken up we should see the level interrupt in TLMM.

-- Lina

> > >> > Why isn't that the case? And if that's because the HW is broken and
> > >> > doesn't buffer edge interrupts, why can't you use the resend mechanism
> > >> > instead?
> > >> >
> > >> The PDC hardware can replay the interrupts accurately. However, it will
> > >> replay only the pin and its not the TLMM summary IRQ. The handler here,
> > >> needs to notify the driver that the wakeup interrupt happened and needs
> > >> to take action. If I could trip the summary IRQ in this handler that
> > >> would work too. Can it be done?
> > >
> > > So the TLMM has indeed noticed the interrupt and you can read the TLMM
> > > status registers to find out what fired?
> > I think that's where it is probably confusing. The TLMM will not see the
> > interrupt because it was in low power mode.
>
> See above. I can buy that for edge, but not level.
>
> Thanks,
>
> 	M.
>
> --
> Jazz is not dead, it just smell funny.