Re: [PATCH 1/2] spi: Add missing pm_runtime_put_noidle() after failed get

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/22/2018 11:32 AM, Mark Brown wrote:
On Mon, May 21, 2018 at 04:19:07PM -0600, Mahadevan, Girish wrote:
On 5/21/2018 2:23 PM, Andy Shevchenko wrote:
On Mon, May 21, 2018 at 6:46 PM, Mark Brown <broonie@xxxxxxxxxx> wrote:
On Fri, May 18, 2018 at 10:30:07AM -0700, Tony Lindgren wrote:
If pm_runtime_get_sync() fails we should call pm_runtime_put_noidle().
This is probably not a critical fix as we should only hit this when
things are broken elsewhere.
This feels like a bug in the runtime PM APIs to be honest - I'd really
not expect that if a function call like a get failed there'd be any
cleanup to do.  I'd expect a very high proportion of users to have the
same problem due to this.
I don't remember the full and correct explanation, but there is a
rationale behind such behaviour (I suppose it's related to sync/async
agnosticism of RPM ops)
We've seen this behavior before and have had similar code to protect
against these failures before.
The pm_runtime_get_sync() fails in certain conditions (for e.g when
called during certain portions of the system suspend/resume).
AFAIK:
Since the runtime APIs internally increment the refcount then call
rpm_resume(), in case rpm_resume() fails (for e.g in case RPM is
disabled) then the driver has to either put the refcount and not go
through with the transaction OR manually move the RPM state and call the
RPM callback APIs internally.
That's still really confusing and surprising, it seems like if the
driver wants to manually call the callbacks there should be a specific
API that it can use to get that behaviour rather than requiring every
user to know that this might happen and manually handle it.


That's a design choice made a long time a go and sorry for it turning out to be inconvenient.

However, there are pieces of code in the kernel that don't check the return value of pm_runtime_get_sync() and then call pm_runtime_put() (or equivalent) after that unconditionally. They all would need to be changed to do the latter only if the preceding pm_runtime_get_sync() was successful.


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux