| Message ID | 20241004094123.113691-1-sakari.ailus@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | treewide: Switch to __pm_runtime_put_autosuspend() | expand |
Am Fri, 4 Oct 2024 12:41:23 +0300 schrieb Sakari Ailus <sakari.ailus@linux.intel.com>: > pm_runtime_put_autosuspend() will soon be changed to include a call to > pm_runtime_mark_last_busy(). This patch switches the current users to > __pm_runtime_put_autosuspend() which will continue to have the > functionality of old pm_runtime_put_autosuspend(). > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > drivers/input/keyboard/omap4-keypad.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/input/keyboard/omap4-keypad.c > b/drivers/input/keyboard/omap4-keypad.c index > 040b340995d8..cc8d77601fc7 100644 --- > a/drivers/input/keyboard/omap4-keypad.c +++ > b/drivers/input/keyboard/omap4-keypad.c @@ -196,7 +196,7 @@ static > irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id) > kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)); > pm_runtime_mark_last_busy(dev); > - pm_runtime_put_autosuspend(dev); > + __pm_runtime_put_autosuspend(dev); > > return IRQ_HANDLED; > } > @@ -234,7 +234,7 @@ static int omap4_keypad_open(struct input_dev > *input) > out: > pm_runtime_mark_last_busy(dev); > - pm_runtime_put_autosuspend(dev); > + __pm_runtime_put_autosuspend(dev); > hmm, if pm_runtime_put_autosuspend() will include the call to pm_runtime_mark_last_busy(), then why is this change needed? After the change, the mark_last_busy could be removed. Regards, Andreas
Hi Andreas, On Fri, Oct 04, 2024 at 11:55:11AM +0200, Andreas Kemnade wrote: > Am Fri, 4 Oct 2024 12:41:23 +0300 > schrieb Sakari Ailus <sakari.ailus@linux.intel.com>: > > > pm_runtime_put_autosuspend() will soon be changed to include a call to > > pm_runtime_mark_last_busy(). This patch switches the current users to > > __pm_runtime_put_autosuspend() which will continue to have the > > functionality of old pm_runtime_put_autosuspend(). > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > drivers/input/keyboard/omap4-keypad.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/input/keyboard/omap4-keypad.c > > b/drivers/input/keyboard/omap4-keypad.c index > > 040b340995d8..cc8d77601fc7 100644 --- > > a/drivers/input/keyboard/omap4-keypad.c +++ > > b/drivers/input/keyboard/omap4-keypad.c @@ -196,7 +196,7 @@ static > > irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id) > > kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)); > > pm_runtime_mark_last_busy(dev); > > - pm_runtime_put_autosuspend(dev); > > + __pm_runtime_put_autosuspend(dev); > > > > return IRQ_HANDLED; > > } > > @@ -234,7 +234,7 @@ static int omap4_keypad_open(struct input_dev > > *input) > > out: > > pm_runtime_mark_last_busy(dev); > > - pm_runtime_put_autosuspend(dev); > > + __pm_runtime_put_autosuspend(dev); > > > hmm, if pm_runtime_put_autosuspend() will include the call to > pm_runtime_mark_last_busy(), then why is this change needed? > After the change, the mark_last_busy could be removed. It could, in which case we'd be living with pm_runtime_mark_last_busy() called twice, for some time, and for some code, possibly even in a release kernel, depending on how well the patches "stick". It likely wouldn't be harmful. Still, to avoid making functional changes to more than 350 files I know little about, I've instead posted this set to prepare for pm_runtime_put_autosuspend() functionality change while keeping the functionality of the code exactly the same. Also added Ilpo who had a similar question.
Am Fri, 4 Oct 2024 10:26:42 +0000 schrieb Sakari Ailus <sakari.ailus@linux.intel.com>: > Hi Andreas, > > On Fri, Oct 04, 2024 at 11:55:11AM +0200, Andreas Kemnade wrote: > > Am Fri, 4 Oct 2024 12:41:23 +0300 > > schrieb Sakari Ailus <sakari.ailus@linux.intel.com>: > > > > > pm_runtime_put_autosuspend() will soon be changed to include a > > > call to pm_runtime_mark_last_busy(). This patch switches the > > > current users to __pm_runtime_put_autosuspend() which will > > > continue to have the functionality of old > > > pm_runtime_put_autosuspend(). > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > --- > > > drivers/input/keyboard/omap4-keypad.c | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/input/keyboard/omap4-keypad.c > > > b/drivers/input/keyboard/omap4-keypad.c index > > > 040b340995d8..cc8d77601fc7 100644 --- > > > a/drivers/input/keyboard/omap4-keypad.c +++ > > > b/drivers/input/keyboard/omap4-keypad.c @@ -196,7 +196,7 @@ static > > > irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id) > > > kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)); > > > pm_runtime_mark_last_busy(dev); > > > - pm_runtime_put_autosuspend(dev); > > > + __pm_runtime_put_autosuspend(dev); > > > > > > return IRQ_HANDLED; > > > } > > > @@ -234,7 +234,7 @@ static int omap4_keypad_open(struct input_dev > > > *input) > > > out: > > > pm_runtime_mark_last_busy(dev); > > > - pm_runtime_put_autosuspend(dev); > > > + __pm_runtime_put_autosuspend(dev); > > > > > hmm, if pm_runtime_put_autosuspend() will include the call to > > pm_runtime_mark_last_busy(), then why is this change needed? > > After the change, the mark_last_busy could be removed. > > It could, in which case we'd be living with > pm_runtime_mark_last_busy() called twice, for some time, and for some > code, possibly even in a release kernel, depending on how well the > patches "stick". It likely wouldn't be harmful. Still, to avoid > making functional changes to more than 350 files I know little about, > I've instead posted this set to prepare for > pm_runtime_put_autosuspend() functionality change while keeping the > functionality of the code exactly the same. > > Also added Ilpo who had a similar question. > Ah, ok, I did not get the cover letter. > 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() with that all this makes sense to me. So: Reviewed-by: Andreas Kemnade <andreas@kemnade.info>
Hi Sakari, On Fri, Oct 04, 2024 at 10:26:42AM +0000, Sakari Ailus wrote: > Hi Andreas, > > On Fri, Oct 04, 2024 at 11:55:11AM +0200, Andreas Kemnade wrote: > > Am Fri, 4 Oct 2024 12:41:23 +0300 > > schrieb Sakari Ailus <sakari.ailus@linux.intel.com>: > > > > > pm_runtime_put_autosuspend() will soon be changed to include a call to > > > pm_runtime_mark_last_busy(). This patch switches the current users to > > > __pm_runtime_put_autosuspend() which will continue to have the > > > functionality of old pm_runtime_put_autosuspend(). > > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > --- > > > drivers/input/keyboard/omap4-keypad.c | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/input/keyboard/omap4-keypad.c > > > b/drivers/input/keyboard/omap4-keypad.c index > > > 040b340995d8..cc8d77601fc7 100644 --- > > > a/drivers/input/keyboard/omap4-keypad.c +++ > > > b/drivers/input/keyboard/omap4-keypad.c @@ -196,7 +196,7 @@ static > > > irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id) > > > kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)); > > > pm_runtime_mark_last_busy(dev); > > > - pm_runtime_put_autosuspend(dev); > > > + __pm_runtime_put_autosuspend(dev); > > > > > > return IRQ_HANDLED; > > > } > > > @@ -234,7 +234,7 @@ static int omap4_keypad_open(struct input_dev > > > *input) > > > out: > > > pm_runtime_mark_last_busy(dev); > > > - pm_runtime_put_autosuspend(dev); > > > + __pm_runtime_put_autosuspend(dev); > > > > > hmm, if pm_runtime_put_autosuspend() will include the call to > > pm_runtime_mark_last_busy(), then why is this change needed? > > After the change, the mark_last_busy could be removed. > > It could, in which case we'd be living with pm_runtime_mark_last_busy() > called twice, for some time, and for some code, possibly even in a release > kernel, depending on how well the patches "stick". It likely wouldn't be > harmful. Still, to avoid making functional changes to more than 350 files I > know little about, I've instead posted this set to prepare for > pm_runtime_put_autosuspend() functionality change while keeping the > functionality of the code exactly the same. pm_runtime_mark_last_busy() is pretty cheap (it fetches current time and stores it in the device power-related structure), so doing it twice back-to-back should not be a problem. I would prefer what Andreas suggested in the first place: remove call to pm_runtime_mark_last_busy() once pm_runtime_put_autosuspend() does it for us. I'd be OK with such change being merged through the same tree that changes to pm_runtime_put_autosuspend() will be merged through. Thanks.
diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c index 040b340995d8..cc8d77601fc7 100644 --- a/drivers/input/keyboard/omap4-keypad.c +++ b/drivers/input/keyboard/omap4-keypad.c @@ -196,7 +196,7 @@ static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id) kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)); pm_runtime_mark_last_busy(dev); - pm_runtime_put_autosuspend(dev); + __pm_runtime_put_autosuspend(dev); return IRQ_HANDLED; } @@ -234,7 +234,7 @@ static int omap4_keypad_open(struct input_dev *input) out: pm_runtime_mark_last_busy(dev); - pm_runtime_put_autosuspend(dev); + __pm_runtime_put_autosuspend(dev); return error; } @@ -268,7 +268,7 @@ static void omap4_keypad_close(struct input_dev *input) clk_disable_unprepare(keypad_data->fck); pm_runtime_mark_last_busy(dev); - pm_runtime_put_autosuspend(dev); + __pm_runtime_put_autosuspend(dev); } static int omap4_keypad_parse_dt(struct device *dev, @@ -407,7 +407,7 @@ static int omap4_keypad_probe(struct platform_device *pdev) } pm_runtime_mark_last_busy(dev); - pm_runtime_put_autosuspend(dev); + __pm_runtime_put_autosuspend(dev); if (error) return error;
pm_runtime_put_autosuspend() will soon be changed to include a call to pm_runtime_mark_last_busy(). This patch switches the current users to __pm_runtime_put_autosuspend() which will continue to have the functionality of old pm_runtime_put_autosuspend(). Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/input/keyboard/omap4-keypad.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)