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
