lkml.org 
[lkml]   [2018]   [Feb]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] i2c: imx: Fix PM device usage count
On Sat, Feb 24, 2018 at 11:43:28PM +0100, Tobias Jordan wrote:
> pm_runtime_get_sync() increases the device's usage count even when
> reporting an error, so add a call to pm_runtime_put_noidle() in the
> related error branches.
>
> Fixes: 588eb93ea49f ("i2c: imx: add runtime pm support to improve the
> performance")
> Signed-off-by: Tobias Jordan <Tobias.Jordan@elektrobit.com>
> ---
> In i2c_imx_xfer(), one could also move the "out" label up (in front of
> the call to pm_runtime_put_autosuspend()), but I'm not sure what the
> underlying error scenario is; calling _put_noidle() seems to be the
> safer bet.
>
> This is one of a number of patches for problems found using coccinelle
> scripting in the SIL2LinuxMP project. The patch has been compile-tested;
> it's based on linux-next-20180223.
>
> For a discussion of the corresponding issue, see
> https://marc.info/?l=linux-pm&m=151904483924999&w=2

I don't get the original mail, so reply here. In reply to the question I
would have asked here, too:

> Why isn't ...get_sync() directly calling ...put_noidle() but relies on
> the driver implementation to do it? It seems unintuitive for a _get_
> function to increase the usage count although returning an error.

Rafael replied:

> Because ...get_sync() returns an error when runtime PM is disabled and
> we wanted that case to be transparent for the users of it.
>
> In the majority of cases (if not always) errors returned by
> ...get_sync()
> mean disabled runtime PM or flaky hardware and the latter is much less
> common (and generally there's not much to do about them in the kernel
> anyway).

If pm_runtime_get_sync() should be transparent for the users if PM is
disable, why not simply return success then? Or introduce a return value
convention like:

<0 (i.e. -ESOMETHIN) is error
0 success
1 PM is disabled

(Taking a quick glimpse there seem to be already some cases where 1 or
-ESOMETHING is returned, but I didn't find documentation explaining the
return values.)

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |

\
 
 \ /
  Last update: 2018-02-25 20:58    [W:0.043 / U:1.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site