linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: PM regression with commit 5de85b9d57ab PM runtime re-init in v4.5-rc1
Date: Tue, 2 Feb 2016 13:03:46 -0800	[thread overview]
Message-ID: <20160202210345.GZ19432@atomide.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1602021402250.1653-100000@iolanthe.rowland.org>

* Alan Stern <stern@rowland.harvard.edu> [160202 11:17]:
> On Tue, 2 Feb 2016, Tony Lindgren wrote:
> > I'd like to have pm_runtime_put_sync() disable the hardware after
> > the initial failed probe. Currently that does not happen unless
> > pm_runtime_dont_use_autosuspend() is called before pm_runtime_put_sync().
> 
> pm_runtime_put_sync() doesn't do anything to the hardware if the usage
> count was > 1, because after the decrement it's still nonzero.  Where
> is the particular call of pm_runtime_put_sync() that you're interested
> in, and what is the usage count when it runs?  It's not at all unusual
> for the usage count to be > 1 during a probe.

The usage count is 0 at that point, it seems the be the RPM_AUTO
causing the issues that we set at the end of rpm_idle().

> Also, what is autosuspend_delay set to for your device?  And is 
> runtime_auto set?

It's 100 at that point, see the commented snippet below from
omap_hsmmc_probe():

	pm_runtime_enable(host->dev);
	pm_runtime_get_sync(host->dev);
	pm_runtime_set_autosuspend_delay(host->dev, MMC_AUTOSUSPEND_DELAY);
	/* NOTE: pm_runtime_dont_use_autosuspend(host->dev) needed here? */
	pm_runtime_use_autosuspend(host->dev);
	...
	/* gets -EPROBE_DEFER */
err_irq:
	...
	pm_runtime_put_sync(host->dev);
        pm_runtime_disable(host->dev);
	/* NOTE: suspend callback never gets called unless
	 * pm_runtime_dont_use_autosuspend() is called
	 * before pm_runtime_put_sync() above.
	 */
	 ...

> > > Does pm_runtime_use_autosuspend() get called by the probe routine?  If 
> > > it does, then perhaps you can get what you want by having the probe 
> > > routine call pm_runtime_dont_use_autosuspend() whenever it's about to 
> > > return an error -- particularly -EDEFER.
> > 
> > Yes so far that's the only fix that seems to work like I posted
> > earlier. But is that the right fix though?
> 
> No, not really.  Ideally you would leave autosuspend turned on.  The 
> delay would be long enough to that after -EDEFER, another probe would 
> start before the delay expired.  But shortly after the last probe 
> attempt, the delay would expire and the device would then be put in low 
> power.

But then what about the new reinit function? To me it seems that
we should not attempt to maintain a state from the earlier failed
probe. Or are you thinking we just skip the reinit if autosuspend
is set?

> > If we wanted to have some generic fix, it seems we would have to pass
> > a new flag in pm_runtime_put_sync() to ignore any autosuspend
> > configuration. But I don't know if that's what we want to or should
> > do though?
> 
> I don't think so.

So should we just establish a policy that pm_runtime_use_autosuspend()
needs to be paired with pm_runtime_dont_use_autosuspend() for
pm_runtime_put_sync() to work?

Regards,

Tony

  reply	other threads:[~2016-02-02 21:03 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-26 22:48 PM regression with commit 5de85b9d57ab PM runtime re-init in v4.5-rc1 Tony Lindgren
2016-01-26 22:50 ` Tony Lindgren
2016-01-26 23:14 ` Rafael J. Wysocki
2016-01-26 23:22   ` Tony Lindgren
2016-01-26 23:37     ` Rafael J. Wysocki
2016-01-26 23:52       ` Tony Lindgren
2016-01-27  7:54         ` Rafael J. Wysocki
2016-01-27  8:17           ` Ulf Hansson
2016-01-27 15:19             ` Tony Lindgren
2016-01-27 22:51             ` Rafael J. Wysocki
2016-01-28 14:29         ` Ulf Hansson
2016-01-28 16:58           ` Tony Lindgren
2016-02-01 16:44             ` Ulf Hansson
2016-02-01 18:11               ` Tony Lindgren
2016-02-01 22:06                 ` Tony Lindgren
2016-02-01 22:17                   ` Rafael J. Wysocki
2016-02-01 22:29                     ` Tony Lindgren
2016-02-01 23:10                       ` Rafael J. Wysocki
2016-02-01 23:28                         ` Tony Lindgren
2016-02-01 23:44                           ` Tony Lindgren
2016-02-01 23:49                           ` Alan Stern
2016-02-02  3:05                             ` Tony Lindgren
2016-02-02 10:07                               ` Ulf Hansson
2016-02-02 10:42                                 ` Ulf Hansson
2016-02-02 16:23                                   ` Alan Stern
2016-02-02 16:35                                   ` Tony Lindgren
2016-02-02 20:47                                     ` Ulf Hansson
2016-02-02 23:41                                       ` Tony Lindgren
2016-02-03 10:23                                         ` Ulf Hansson
2016-02-03 10:25                                           ` Ulf Hansson
2016-02-03 12:18                                             ` Rafael J. Wysocki
2016-02-03 14:58                                               ` Ulf Hansson
2016-02-03 15:45                                                 ` Alan Stern
2016-02-03 16:09                                                   ` Tony Lindgren
2016-02-03 16:24                                                     ` Ulf Hansson
2016-02-03 17:01                                                       ` Tony Lindgren
2016-02-03 17:16                                                       ` Rafael J. Wysocki
2016-02-03 16:27                                           ` Tony Lindgren
2016-02-03 18:02                                             ` Ulf Hansson
2016-02-03 18:28                                               ` Tony Lindgren
2016-02-03 18:37                                                 ` Ulf Hansson
2016-02-03 18:45                                                   ` Tony Lindgren
2016-02-03 21:51                                                     ` Tony Lindgren
2016-02-02 16:15                                 ` Alan Stern
2016-02-02 16:49                                   ` Tony Lindgren
2016-02-02 18:05                                     ` Tony Lindgren
2016-02-02 18:43                                       ` Alan Stern
2016-02-02 18:54                                         ` Tony Lindgren
2016-02-02 19:16                                           ` Alan Stern
2016-02-02 21:03                                             ` Tony Lindgren [this message]
2016-02-02 21:45                                               ` Alan Stern
2016-02-02 23:46                                                 ` Tony Lindgren
2016-02-03 13:06                                                   ` Rafael J. Wysocki
2016-02-03 16:36                                                     ` Tony Lindgren
2016-02-03 15:48                                                   ` Alan Stern
2016-02-03 16:37                                                     ` Tony Lindgren
2016-02-03 17:18                                                   ` Rafael J. Wysocki
2016-02-03 17:22                                                     ` Tony Lindgren
2016-02-03 17:27                                                       ` Rafael J. Wysocki
2016-02-04 10:20                                                     ` Ulf Hansson
2016-02-04 16:04                                                       ` Alan Stern
2016-02-04 17:20                                                         ` Tony Lindgren
2016-02-04 21:11                                                         ` Ulf Hansson
2016-02-04 22:09                                                           ` Alan Stern
2016-02-04 22:34                                                             ` Ulf Hansson
2016-02-05  1:08                                                               ` Tony Lindgren
2016-02-05  6:54                                                                 ` Ulf Hansson
2016-02-05 19:10                                                                   ` Tony Lindgren
2016-02-02 18:47                                       ` Tony Lindgren
2016-02-02 20:24                                   ` Ulf Hansson
2016-02-02 21:24                                     ` Alan Stern
2016-02-02 21:39                                     ` Tony Lindgren
2016-02-03 13:03                                       ` Rafael J. Wysocki
2016-02-03 16:49                                         ` Tony Lindgren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160202210345.GZ19432@atomide.com \
    --to=tony@atomide.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).