[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADtm3G6vOqTbcxKWhbuWXrTsz-85cdfHwnM4KwS3Y9iPFPjQBg@mail.gmail.com>
Date: Thu, 26 Oct 2017 16:51:55 -0700
From: Gregory Fong <gregory.0xf0@...il.com>
To: Doug Berger <opendmb@...il.com>
Cc: Linus Walleij <linus.walleij@...aro.org>,
Brian Norris <computersforpeace@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
bcm-kernel-feedback-list <bcm-kernel-feedback-list@...adcom.com>,
linux-gpio@...r.kernel.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2 7/7] gpio: brcmstb: implement suspend/resume/shutdown
On Tue, Oct 24, 2017 at 12:54 PM, Doug Berger <opendmb@...il.com> 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@...il.com>
Acked-by: Gregory Fong <gregory.0xf0@...il.com>
Powered by blists - more mailing lists