Re: [PATCH v2 7/7] gpio: brcmstb: implement suspend/resume/shutdown

[Florian Fainelli]

On 10/24/2017 12:54 PM, Doug Berger wrote:
> This commit corrects problems with the previous wake implementation
> by implementing suspend and resume power management operations and
> the driver shutdown operation.
> 
> Wake masks are used to keep track of which GPIO should wake the
> device.  On suspend the GPIO state is saved and the possible wakeup
> sources are explicitly unmasked in the hardware. Non-wakeup sources
> are explicitly masked so IRQCHIP_MASK_ON_SUSPEND is no longer
> necessary.  The saved state of the GPIO is restored upon resume.
> It is important not to write to the GPIO status register since this
> has the effect of clearing bits.  The status register is explicitly
> removed from the register save and restore to ensure this.
> 
> The shutdown operation allows the hardware to be put into the same
> quiesced state as the suspend operation and removes the need for
> the reboot notifier.
> 
> Unfortunately, there appears to be some confusion about whether
> a pending disabled wake interrupt should wake the system. If a wake
> capable interrupt is disabled using the default "lazy disable"
> behavior and it is triggered before the suspend_device_irq call
> the interrupt hardware will be acknowledged by mask_ack_irq and the
> IRQS_PENDING flag is added to its state. However, the IRQS_PENDING
> flag of wake interrupts is not checked to prevent the transition to
> suspend and the hardware has been acked which prevents its wakeup.
> If the lazy disabled interrupt is triggered after the call to
> suspend_device_irqs then the wakeup logic will abort the suspend.
> The irq_disable method is defined by this GPIO driver to prevent
> lazy disable so that the pending hardware state remains asserted
> allowing the hardware to wake and providing a consistent behavior.
> 
> In addition, the IRQ_DISABLE_UNLAZY flag is set for the non-wake
> parent interrupt as a convenience to prevent the need to add code
> to the brcmstb_gpio_irq_handler to support "lazy disable" of the
> non-wake parent interrupt when it is disabled during suspend and
> resume. Chained interrupt parents are not normally disabled, but
> these GPIO devices have different parent interrupts for wake and
> non-wake handling. It is convenient to mask the non-wake parent
> when suspending to preserve the hardware state for proper wakeup
> accounting when the driver is resumed.
> 
> Signed-off-by: Doug Berger <opendmb@xxxxxxxxx>

Reviewed-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
-- 
-- 
Florian