From: Ulf Hansson <email@example.com> To: Florian Fainelli <firstname.lastname@example.org> Cc: Linux PM <email@example.com>, "Rafael J. Wysocki" <firstname.lastname@example.org>, Al Cooper <email@example.com>, Linux Kernel Mailing List <firstname.lastname@example.org>, Adrian Hunter <email@example.com>, BCM Kernel Feedback <firstname.lastname@example.org>, DTML <email@example.com>, Linux ARM <firstname.lastname@example.org>, linux-mmc <email@example.com>, Nicolas Saenz Julienne <firstname.lastname@example.org>, Ray Jui <email@example.com>, Rob Herring <firstname.lastname@example.org>, Scott Branden <email@example.com> Subject: Re: [PATCH 2/2] mmc: sdhci-iproc: Add support for the legacy sdhci controller on the BCM7211 Date: Tue, 15 Jun 2021 17:30:39 +0200 [thread overview] Message-ID: <CAPDyKFq92mp4CXj8-QHw=DEQ8bcAjtrmLyowrGKSJL2Fch1cJQ@mail.gmail.com> (raw) In-Reply-To: <firstname.lastname@example.org> [...] > > > >> > >> 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. I guess it could be a topic that is easier to bring up at the Linux Plumbers Conf, for example. Kind regards Uffe
next prev parent reply other threads:[~2021-06-15 15:31 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 [this message] 2021-06-15 15:51 ` Florian Fainelli 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 \ --in-reply-to='CAPDyKFq92mp4CXj8-QHw=DEQ8bcAjtrmLyowrGKSJL2Fch1cJQ@mail.gmail.com' \ --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).