On Monday, August 28, 2017 4:24:51 PM CEST Ulf Hansson wrote: > On 28 August 2017 at 15:40, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: > > On Monday, August 28, 2017 2:54:40 PM CEST Ulf Hansson wrote: > >> On 28 August 2017 at 14:39, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: > >> > On Monday, August 28, 2017 10:31:44 AM CEST Ulf Hansson wrote: > >> >> On 28 August 2017 at 03:30, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: > >> >> > On Friday, August 25, 2017 3:42:35 PM CEST Rafael J. Wysocki wrote: > >> >> >> On Thursday, August 24, 2017 11:50:40 PM CEST Rafael J. Wysocki wrote: > >> >> >> > On Thursday, August 24, 2017 6:35:49 PM CEST Rafael J. Wysocki wrote: > >> >> >> > > On Thursday, August 24, 2017 11:15:26 AM CEST Ulf Hansson wrote: > > > > [cut] > > > >> >> > > >> >> > Well, not really, because if the device remains runtime suspended, > >> >> > ->runtime_suspend() will not be called by pm_runtime_force_suspend() and > >> >> > acpi_dev_suspend_late() should not be called then. > >> >> > > >> >> > So more changes in the ACPI PM domain are needed after all. > >> >> > >> >> Yes, that's what I thought as well. > >> >> > >> >> Anyway, let me cook a new version of the series - trying to address > >> >> the first bits you have pointed out. Then we can continue with > >> >> fine-tuning on top, addressing further optimizations of the ACPI PM > >> >> domain. > >> > > >> > Actually, please hold on and let me show you what I would like to do > >> > first. > >> > >> Hmm. > >> > >> I think I have almost done the work for the ACPI PM domain already. > >> It's just a matter of minor tweaks to the changes in patch 6 and 7 > >> (and of course to get them into a shape that you prefer) and then > >> dropping patch 5 altogether. > >> > >> Wouldn't it be better if you build upon my changes? > >> > >> Anyway, if you have strong opinion of driving this, I am fine stepping aside. > > > > OK, so see below. :-) > > > > If what you want is to make the i2c designware driver use _force_suspend() and > > _force_resume(), then to me this is only tangentially related to direct_complete > > and can be done without messing up with that one. > > > > So the problem is not when direct_complete is set (becasue the driver's and > > PM domain's callbacks will not be invoked then even), but when it is not set. > > For i2c designware driver case - then you are right! Although, that's > because the i2c designware driver has nothing else to do than to call > pm_runtime_force_suspend|resume() from the late suspend and early > resume callbacks. > > However, for other drivers this isn't the case. A driver may have some > additional things to cope with during system sleep, besides making > sure to call pm_runtime_force_suspend|resume(). Sure enough, but I'm seeing that as a separate issue. > Then as I stated earlier, it would be of great value if we could > remain having runtime PM enabled during the entire device_suspend() > phase. I am not sure how you intend to address that, or perhaps you > did in some of those earlier patches you posted. I did, but I'd prefer to get back to that later. If the issues occurring when direct_complete is not set are addressed first, disabling direct_complete for the devices that cannot use it should be straightforward. > In my re-spinned series (not posted yet), I am still addressing both > issues above, but also not preventing the direct_complete path for > parent/suppliers when the runtime PM centric path is used. > > > If direct_complete is not set, the ACPI PM domain resumes the device in > > acpi_subsys_suspend(), because it doesn't know two things: (a) why direct_complete > > is not set and (b) whether or not the drivers PM callbacks can cope with a > > runtime suspended device. These two things are separate, so they need to > > be addressed separately. > > Yes. > > > > > For (b) I'd like to use the SAFE_SUSPEND flag from my previous patch. > > Seems like a reasonable approach. > > > > > As far as (a) is concerned, there are two possible reasons for not setting > > direct_complete. First, it may be necessary to change the power state of the > > device and in that case the device *should* be resumed in acpi_subsys_suspend(). > > Second, direct_complete may not be set for the device's children and in that > > case acpi_subsys_suspend() may not care as long as SAFE_SUSPEND is set. > > Okay! Good. :-) Let me split the patch into a more manageable series, then, clean it up a bit and add the LPSS coverage to the ACPI part. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
