From: Florian Fainelli <firstname.lastname@example.org> To: Ulf Hansson <email@example.com> Cc: Linux PM <firstname.lastname@example.org>, "Rafael J. Wysocki" <email@example.com>, Al Cooper <firstname.lastname@example.org>, Linux Kernel Mailing List <email@example.com>, Adrian Hunter <firstname.lastname@example.org>, BCM Kernel Feedback <email@example.com>, DTML <firstname.lastname@example.org>, Linux ARM <email@example.com>, linux-mmc <firstname.lastname@example.org>, Nicolas Saenz Julienne <email@example.com>, Ray Jui <firstname.lastname@example.org>, Rob Herring <email@example.com>, Scott Branden <firstname.lastname@example.org> Subject: Re: [PATCH 2/2] mmc: sdhci-iproc: Add support for the legacy sdhci controller on the BCM7211 Date: Tue, 15 Jun 2021 08:51:26 -0700 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <CAPDyKFq92mp4CXj8-QHw=DEQ8bcAjtrmLyowrGKSJL2Fch1cJQ@mail.gmail.com> On 6/15/2021 8:30 AM, Ulf Hansson wrote: > [...] > >>> >>>> >>>> In all honesty, I am a bit surprised that the Linux device driver model >>>> does not try to default the absence of a ->shutdown() to a ->suspend() >>>> call since in most cases they are functionally equivalent, or should be, >>>> in that they need to save power and quiesce the hardware, or leave >>>> enough running to support a wake-up event. >>> >>> Well, the generall assumption is that the platform is going to be >>> entirely powered off, thus moving things into a low power state would >>> just be a waste of execution cycles. Of course, that's not the case >>> for your platform. >> >> That assumption may hold true for ACPI-enabled machines but power off is >> offered as a general function towards other more flexible and snowflaky >> systems (read embedded) as well. >> >>> >>> As I have stated earlier, to me it looks a bit questionable to use the >>> kernel_power_off() path to support the use case you describe. On the >>> other hand, we may not have a better option at this point. >> >> Correct, there is not really anything better and I am not sure what the >> semantics of something better could be anyway. >> >>> >>> Just a few things, from the top of my head, that we certainly are >>> missing to support your use case through kernel_power_off() path >>> (there are certainly more): >>> 1. In general, subsystems/drivers don't care about moving things into >>> lower power modes from their ->shutdown() callbacks. >>> 2. System wakeups and devices being affected in the wakeup path, needs >>> to be respected properly. Additionally, userspace should be able to >>> decide if system wakeups should be enabled or not. >>> 3. PM domains don't have ->shutdown() callbacks, thus it's likely that >>> they remain powered on. >>> 4. Etc... >> >> For the particular eMMC driver being discussed here this is a no-brainer > > because it is not a wake-up source, therefore there is no reason not to >> power if off if we can. It also seems proper to have it done by the >> kernel as opposed to firmware. > > Okay, I have applied the $subject patch onto my next branch, along > with patch 1/2 (the DT doc change). > > However, I still think we should look for a proper long term solution, > because the kernel_power_off() path does not currently support your > use case, with system wakeups etc. Not really, it does work fine, some drivers like gpio-keys.c or gpio-brcmstb.c will ensure that the GPIOs that are enabled as wake-up interrupts are configured that way during kernel_power_off() and the various interrupt controllers like irq-brcmstb-l2.c will make sure they don't mask wake-up interrupts. > > I guess it could be a topic that is easier to bring up at the Linux > Plumbers Conf, for example. OK, not sure if I will be able to attend, but would definitively try to. -- Florian
next prev parent reply other threads:[~2021-06-15 15:58 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-02 19:27 [PATCH 1/2] dt-bindings: mmc: sdhci-iproc: Add brcm,bcm7211a0-sdhci Al Cooper 2021-06-02 19:27 ` [PATCH 2/2] mmc: sdhci-iproc: Add support for the legacy sdhci controller on the BCM7211 Al Cooper 2021-06-08 12:40 ` Ulf Hansson 2021-06-09 3:07 ` Florian Fainelli 2021-06-09 9:22 ` Ulf Hansson 2021-06-09 23:59 ` Florian Fainelli 2021-06-10 8:49 ` Ulf Hansson 2021-06-10 15:59 ` Florian Fainelli 2021-06-11 10:23 ` Ulf Hansson 2021-06-11 16:54 ` Florian Fainelli 2021-06-14 13:19 ` Ulf Hansson 2021-06-14 19:29 ` Florian Fainelli 2021-06-15 15:30 ` Ulf Hansson 2021-06-15 15:51 ` Florian Fainelli [this message] 2021-06-15 23:46 ` [PATCH 1/2] dt-bindings: mmc: sdhci-iproc: Add brcm,bcm7211a0-sdhci Rob Herring
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH 2/2] mmc: sdhci-iproc: Add support for the legacy sdhci controller on the BCM7211' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).