Re: [PATCH 20/51] Input: omap4-keypad: Switch to __pm_runtime_put_autosuspend()

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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.

-- 
Dmitry