Hi Laurent, On Tue, Nov 21, 2023 at 10:50:56AM +0200, Laurent Pinchart wrote: > Hi Sakari, > > On Tue, Nov 21, 2023 at 08:41:34AM +0000, Sakari Ailus wrote: > > On Mon, Nov 20, 2023 at 11:47:43AM +0200, Laurent Pinchart wrote: > > > On Mon, Nov 20, 2023 at 09:27:51AM +0000, Sakari Ailus wrote: > > > > On Sat, Nov 18, 2023 at 11:30:31PM +0200, Laurent Pinchart wrote: > > > > > On Sat, Nov 18, 2023 at 10:20:46PM +0100, Rafael J. Wysocki wrote: > > > > > > On Sat, Nov 18, 2023 at 6:49 PM Laurent Pinchart wrote: > > > > > > > On Fri, Nov 17, 2023 at 01:14:28PM +0200, Sakari Ailus wrote: > > > > > > > > Add pm_runtime_put_mark_busy_autosusp() helper function for users that > > > > > > > > wish to set the last_busy timestamp to current time and put the > > > > > > > > usage_count of the device and set the autosuspend timer. > > > > > > > > > > > > > > > > Essentially calling pm_runtime_suspend_mark_busy_autosusp() equal to > > > > > > > > calling first pm_runtime_mark_last_busy() and then > > > > > > > > pm_runtime_put_autosuspend(). > > > > > > > > > > > > > > The vast majority if the pm_runtime_put_autosuspend() users call > > > > > > > pm_runtime_mark_last_busy() right before. Let's make the > > > > > > > pm_runtime_put_autosuspend() function do that by default, and add a > > > > > > > __pm_runtime_put_autosuspend() (name to be bikshedded) for the minority > > > > > > > of cases where updating the last busy timestamp isn't desired. We want > > > > > > > to simplify the API, not make it more complex. > > > > > > > > > > > > I would also prefer it to be done this way if not too problematic. > > > > > > > > > > I'm glad you agree :-) The change will probably be a bit painful, but I > > > > > think it's for the best. Sakari, please let me know if I can help. > > > > > > > > I actually do prefer this approach, too. > > > > > > > > There about 350 drivers using pm_runtime_autosuspend() currently. Around > > > > 150 uses pm_runtime_autosuspend() which is not preceded by > > > > pm_runtime_mark_last_busy(). Call-wise the numbers are ~ 1050 and ~ 330. > > > > > > > > I checked some of what's left: most do still call both, but in a way that > > > > evades Coccinelle matching. Some omissions seem to remain. > > > > > > > > Given that there are way more users that do also call > > > > pm_runtime_mark_last_busy(), I think I'll try to introduce > > > > __pm_runtime_put_autosuspend() and pm_runtime_put_autosuspend() > > > > documentation change first and then rename the callers that don't use > > > > pm_runtime_mark_last_busy(). > > > > > > And also drop pm_runtime_mark_last_busy() from the drivers that call > > > pm_runtime_put_autosuspend(), right ? > > > > That should be done but as it doesn't affect the functionality, it can (and > > may only) be done later on --- the current users need to be converted to > > use the to-be-added __pm_runtime_put_autosuspend() first. > > True. If you're going to send a series that change things tree-wide I > was thinking that it would be best to address multiple tree-wide changes > at the same time, that would be less churn, especially if it can be > mostly scripted with Coccinelle. That would be my preference (especially > because the issue will then be solved and we'll be able to move to > something else, instead of leaving old code lingering on for a long > time), but it's up to you. This will mean around 1000 changed lines in 350 files. Given the number of changes and how they're scattered around, I'd expect to merge first the Runtime PM API change, then later on the driver specific changes via their own trees. Doing it differently risks a large number of conflicts. Hopefully faster than changing the I²C driver probe function though. We also need to have some time before all users of pm_runtime_put_autosuspend() have been converted, including new ones merged meanwhile. -- Regards, Sakari Ailus
