From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751446AbdJ1Q6e (ORCPT ); Sat, 28 Oct 2017 12:58:34 -0400 Received: from mail-yw0-f193.google.com ([209.85.161.193]:56499 "EHLO mail-yw0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751230AbdJ1Q6b (ORCPT ); Sat, 28 Oct 2017 12:58:31 -0400 X-Google-Smtp-Source: ABhQp+Qqt8kfNfzNBeQeorZt9KkTGdkEIDY3ogESKcnQdQt1YzXZPdPdFmAhe0/DqB0W1tQKs1sxKg== Subject: Re: [PATCH v2 7/7] gpio: brcmstb: implement suspend/resume/shutdown To: Doug Berger , Gregory Fong Cc: Linus Walleij , Brian Norris , Florian Fainelli , bcm-kernel-feedback-list@broadcom.com, linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org References: <20171024195451.30535-1-opendmb@gmail.com> <20171024195451.30535-8-opendmb@gmail.com> From: Florian Fainelli Message-ID: Date: Sat, 28 Oct 2017 09:58:27 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171024195451.30535-8-opendmb@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 Reviewed-by: Florian Fainelli -- -- Florian