lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 2 Aug 2018 06:58:27 -0600 From: Lina Iyer <ilina@...eaurora.org> To: Marc Zyngier <marc.zyngier@....com> Cc: swboyd@...omium.org, evgreen@...omium.org, linus.walleij@...aro.org, bjorn.andersson@...aro.org, rplsssn@...eaurora.org, linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org, rnayak@...eaurora.org, devicetree@...r.kernel.org Subject: Re: [PATCH RESEND RFC 1/4] drivers: pinctrl: qcom: add wakeup capability to GPIO On Thu, Aug 02 2018 at 01:27 -0600, Marc Zyngier wrote: >On Thu, 02 Aug 2018 07:51:04 +0100, >Lina Iyer <ilina@...eaurora.org> 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@...eaurora.org> 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@...eaurora.org> 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.
Powered by blists - more mailing lists