[...] > > > > The problem with $subject patch is that drivers/buses are required to > > check the runtime PM status, with pm_runtime_suspended(), > > pm_runtime_status_suspended() or pm_runtime_active() in one of its > > system suspend/resume callbacks , to synchronize it with the HW state > > for its device (turn on/off clocks for example). > > Well, I'm kind of unaware of this requirement. > > It clearly is not even followed by the code without the $subject patch. > > The real requirement is that the runtime PM status at the point when > runtime PM is re-enabled, that is in device_resume_early(), must > reflect the current actual HW state. Right. Seems like we are in agreement, just that there seems to be multiple ways to describe the similar problem. > > > Certainly, we can not rely on drivers to conform to this behaviour and > > there are plenty of cases where they really don't. For example, we > > have drivers that only implements runtime PM support or simply don't > > care about the runtime PM status during system resume, but just leaves > > the device in the state it is already in. > > Drivers that only support runtime PM are broken with respect to system > sleep ATM. They need to be made to support system sleep or they > cannot be used on systems that use system sleep. There may be a way > around this for system suspend/resume (see below), but not for > hibernation. I think calling them broken may be to take this a step too far. While I certainly agree that these drivers have room for some improvements, it looks to me that these drivers work today, but may not with $subject patch. > > > Moreover, we have the users of pm_runtime_force_suspend|resume(), > > which we also know *not* to work with DPM_FLAG_SMART_SUSPEND and thus > > $subject patch too. I am less worried about these cases though, as I > > believe we should be able to fix them, by taking into account the > > suggested "->power.set_active flag", or something along those lines. > > Yes, and that's what I'm going to do. > > > That said, it seems like we need another way forward. > > I still don't see why, sorry. > > I guess the concern is that if a device suddenly needs to be resumed > during system resume even though it was runtime-suspended before the > preceding system suspend, there is no way to tell its driver/bus > type/etc that this is the case if they all decide to leave the device > as is, but as I have said for multiple times in this thread, leaving a > device as is during system resume may not be an option unless it is a > leaf device without any subordinates. This has always been the case. > > We'll see if there is any damage resulting from the $subject change > and we'll fix it if so. > > In the future, though, I'd like to integrate system resume with > runtime PM more than it is now. Namely, it should be possible to move > the re-enabling of runtime PM to the front of device_resume_early() > and then use pm_runtime_resume() for resuming devices whose drivers > support runtime PM (I don't see any fundamental obstacles to this). > One benefit of doing this would be that pm_runtime_resume() would > automatically trigger a runtime resume for all of the "superior" > devices, so they could be left in whatever state they had been in > before the preceding system suspend regardless of what happens to > their "subordinates". It is likely that some kind of driver opt-in > will be needed for this or maybe the core can figure it out by itself. Right, I am certainly interested to discuss this topic too. It's not an easy thing and the biggest problem is that we can't really just change the behaviour without a big risk of breaking things. Some kind of opt-in behaviour is the only thing that can work, in my opinion. Anyway, I am here to review and discuss. :-) > > It can look at what callbacks are implemented etc. For example, if a > driver only implements :runtime_suspend() and :runtime_resume() and no > other PM callbacks, it is reasonable to assume that the devices > handled by it should be suspended and resumed with the help of the > runtime PM infrastructure even during system-wide suspend/resume (that > doesn't apply to hibernation, though). Maybe this can work in some opt-in/out way, but certainly not as a solution for all. For example, we have subsystems that deal with system suspend/resume quite differently, where drivers instead implement some subsystem specific callbacks, rather than the common system suspend/resume ops. Kind regards Uffe
