• Douglas Anderson's avatar
    pinctrl: qcom: Don't clear pending interrupts when enabling · cf9d052a
    Douglas Anderson authored
    In Linux, if a driver does disable_irq() and later does enable_irq()
    on its interrupt, I believe it's expecting these properties:
    * If an interrupt was pending when the driver disabled then it will
      still be pending after the driver re-enables.
    * If an edge-triggered interrupt comes in while an interrupt is
      disabled it should assert when the interrupt is re-enabled.
    
    If you think that the above sounds a lot like the disable_irq() and
    enable_irq() are supposed to be masking/unmasking the interrupt
    instead of disabling/enabling it then you've made an astute
    observation.  Specifically when talking about interrupts, "mask"
    usually means to stop posting interrupts but keep tracking them and
    "disable" means to fully shut off interrupt detection.  It's
    unfortunate that this is so confusing, but presumably this is all the
    way it is for historical reasons.
    
    Perhaps more confusing than the above is that, even though clients of
    IRQs themselves don't have a way to request mask/unmask
    vs. disable/enable calls, IRQ chips themselves can implement both.
    ...and yet more confusing is that if an IRQ chip implements
    disable/enable then they will be called when a client driver calls
    disable_irq() / enable_irq().
    
    It does feel like some of the above could be cleared up.  However,
    without any other core interrupt changes it should be clear that when
    an IRQ chip gets a request to "disable" an IRQ that it has to treat it
    like a mask of that IRQ.
    
    In any case, after that long interlude you can see that the "unmask
    and clear" can break things.  Maulik tried to fix it so that we no
    longer did "unmask and clear" in commit 71266d9d ("pinctrl: qcom:
    Move clearing pending IRQ to .irq_request_resources callback"), but it
    only handled the PDC case and it had problems (it caused
    sc7180-trogdor devices to fail to suspend).  Let's fix.
    
    >From my understanding the source of the phantom interrupt in the
    were these two things:
    1. One that could have been introduced in msm_gpio_irq_set_type()
       (only for the non-PDC case).
    2. Edges could have been detected when a GPIO was muxed away.
    
    Fixing case #1 is easy.  We can just add a clear in
    msm_gpio_irq_set_type().
    
    Fixing case #2 is harder.  Let's use a concrete example.  In
    sc7180-trogdor.dtsi we configure the uart3 to have two pinctrl states,
    sleep and default, and mux between the two during runtime PM and
    system suspend (see geni_se_resources_{on,off}() for more
    details). The difference between the sleep and default state is that
    the RX pin is muxed to a GPIO during sleep and muxed to the UART
    otherwise.
    
    As per Qualcomm, when we mux the pin over to the UART function the PDC
    (or the non-PDC interrupt detection logic) is still watching it /
    latching edges.  These edges don't cause interrupts because the
    current code masks the interrupt unless we're entering suspend.
    However, as soon as we enter suspend we unmask the interrupt and it's
    counted as a wakeup.
    
    Let's deal with the problem like this:
    * When we mux away, we'll mask our interrupt.  This isn't necessary in
      the above case since the client already masked us, but it's a good
      idea in general.
    * When we mux back will clear any interrupts and unmask.
    
    Fixes: 4b7618fd ("pinctrl: qcom: Add irq_enable callback for msm gpio")
    Fixes: 71266d9d ("pinctrl: qcom: Move clearing pending IRQ to .irq_request_resources callback")
    Signed-off-by: default avatarDouglas Anderson <dianders@chromium.org>
    Reviewed-by: default avatarMaulik Shah <mkshah@codeaurora.org>
    Tested-by: default avatarMaulik Shah <mkshah@codeaurora.org>
    Reviewed-by: default avatarStephen Boyd <swboyd@chromium.org>
    Link: https://lore.kernel.org/r/20210114191601.v7.4.I7cf3019783720feb57b958c95c2b684940264cd1@changeidSigned-off-by: default avatarLinus Walleij <linus.walleij@linaro.org>
    cf9d052a
pinctrl-msm.c 38.5 KB