On 08/10/2024 7:24 pm, Rafael J. Wysocki wrote:
On Tue, Oct 8, 2024 at 12:35 AM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:On Tue, 8 Oct 2024 at 00:25, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:Hi Ulf, On Tue, Oct 08, 2024 at 12:08:24AM +0200, Ulf Hansson wrote:On Mon, 7 Oct 2024 at 20:49, Laurent Pinchart wrote:On Fri, Oct 04, 2024 at 04:38:36PM +0200, Ulf Hansson wrote:On Fri, 4 Oct 2024 at 11:41, Sakari Ailus wrote:Hello everyone, This set will switch the users of pm_runtime_put_autosuspend() to __pm_runtime_put_autosuspend() while the former will soon be re-purposed to include a call to pm_runtime_mark_last_busy(). The two are almost always used together, apart from bugs which are likely common. Going forward, most new users should be using pm_runtime_put_autosuspend(). Once this conversion is done and pm_runtime_put_autosuspend() re-purposed, I'll post another set to merge the calls to __pm_runtime_put_autosuspend() and pm_runtime_mark_last_busy().That sounds like it could cause a lot of churns. Why not add a new helper function that does the pm_runtime_put_autosuspend() and the pm_runtime_mark_last_busy() things? Then we can start moving users over to this new interface, rather than having this intermediate step?I think the API would be nicer if we used the shortest and simplest function names for the most common use cases. Following pm_runtime_put_autosuspend() with pm_runtime_mark_last_busy() is that most common use case. That's why I like Sakari's approach of repurposing pm_runtime_put_autosuspend(), and introducing __pm_runtime_put_autosuspend() for the odd cases where pm_runtime_mark_last_busy() shouldn't be called.Okay, so the reason for this approach is because we couldn't find a short and descriptive name that could be used in favor of pm_runtime_put_autosuspend(). Let me throw some ideas at it and maybe you like it - or not. :-)I like the idea at least :-)I don't know what options you guys discussed, but to me the entire "autosuspend"-suffix isn't really that necessary in my opinion. There are more ways than calling pm_runtime_put_autosuspend() that triggers us to use the RPM_AUTO flag for rpm_suspend(). For example, just calling pm_runtime_put() has the similar effect.To be honest, I'm lost there. pm_runtime_put() calls __pm_runtime_idle(RPM_GET_PUT | RPM_ASYNC), while pm_runtime_put_autosuspend() calls __pm_runtime_suspend(RPM_GET_PUT | RPM_ASYNC | RPM_AUTO).__pm_runtime_idle() ends up calling rpm_idle(), which may call rpm_suspend() - if it succeeds to idle the device. In that case, it tags on the RPM_AUTO flag in the call to rpm_suspend(). Quite similar to what is happening when calling pm_runtime_put_autosuspend().Right. For almost everybody, except for a small bunch of drivers that actually have a .runtime_idle() callback, pm_runtime_put() is literally equivalent to pm_runtime_put_autosuspend(). So really the question is why anyone who doesn't provide a .runtime_idle() callback bothers with using this special pm_runtime_put_autosuspend() thing,
Because they are following the documentation? It says: "Drivers should call pm_runtime_mark_last_busy() to update this field after carrying out I/O, typically just before calling pm_runtime_put_autosuspend()." and "In order to use autosuspend, subsystems or drivers must call pm_runtime_use_autosuspend() (...), and thereafter they should use the various `*_autosuspend()` helper functions instead of the non# autosuspend counterparts" So the documentation says I should be using pm_runtime_put_autosuspend() instead of pm_runtime_put(). Seems unfair to criticise people for following the documentation. _______________________________________________ greybus-dev mailing list -- greybus-dev@xxxxxxxxxxxxxxxx To unsubscribe send an email to greybus-dev-leave@xxxxxxxxxxxxxxxx
|