| Message ID | 20230425094747.2769693-1-martin.kepplinger@puri.sm |
|---|---|
| State | Accepted |
| Commit | 04fc06f6dc1592ed5d675311ac50d8fba5db62ab |
| Headers | show |
| Series | [v2] media: hi846: fix usage of pm_runtime_get_if_in_use() | expand |
Hi Martin, On Tue, Apr 25, 2023 at 11:47:47AM +0200, Martin Kepplinger wrote: > pm_runtime_get_if_in_use() does not only return nonzero values when > the device is in use, it can return a negative errno too. > > And especially during resuming from system suspend, when runtime pm > is not yet up again, -EAGAIN is being returned, so the subsequent > pm_runtime_put() call results in a refcount underflow. > > Fix system-resume by handling -EAGAIN of pm_runtime_get_if_in_use(). > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > --- > > revision history > ---------------- > v2 (thank you Sakari and Laurent): > * drop the other patch (the streaming-state in suspend/resume needs to > be solved differently). > * Sakari pointed out that many drivers are affected by this and that > runtime-pm might need changes instead. I think this patch doesn't hurt > and could serve as a reminder to do so. I guess it's appropriate to add: Fixes: e8c0882685f9 ("media: i2c: add driver for the SK Hynix Hi-846 8M pixel camera")
Am Dienstag, dem 16.05.2023 um 16:22 +0300 schrieb Sakari Ailus: > Hi Martin, > > On Tue, Apr 25, 2023 at 11:47:47AM +0200, Martin Kepplinger wrote: > > pm_runtime_get_if_in_use() does not only return nonzero values when > > the device is in use, it can return a negative errno too. > > > > And especially during resuming from system suspend, when runtime pm > > is not yet up again, -EAGAIN is being returned, so the subsequent > > pm_runtime_put() call results in a refcount underflow. > > > > Fix system-resume by handling -EAGAIN of > > pm_runtime_get_if_in_use(). > > > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > > --- > > > > revision history > > ---------------- > > v2 (thank you Sakari and Laurent): > > * drop the other patch (the streaming-state in suspend/resume needs > > to > > be solved differently). > > * Sakari pointed out that many drivers are affected by this and > > that > > runtime-pm might need changes instead. I think this patch doesn't > > hurt > > and could serve as a reminder to do so. > > I guess it's appropriate to add: > > Fixes: e8c0882685f9 ("media: i2c: add driver for the SK Hynix Hi-846 > 8M pixel camera") > I agree. Feel free to do so when applying. martin
diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c index 306dc35e925f..f8709cdf28b3 100644 --- a/drivers/media/i2c/hi846.c +++ b/drivers/media/i2c/hi846.c @@ -1353,7 +1353,8 @@ static int hi846_set_ctrl(struct v4l2_ctrl *ctrl) exposure_max); } - if (!pm_runtime_get_if_in_use(&client->dev)) + ret = pm_runtime_get_if_in_use(&client->dev); + if (!ret || ret == -EAGAIN) return 0; switch (ctrl->id) {
pm_runtime_get_if_in_use() does not only return nonzero values when the device is in use, it can return a negative errno too. And especially during resuming from system suspend, when runtime pm is not yet up again, -EAGAIN is being returned, so the subsequent pm_runtime_put() call results in a refcount underflow. Fix system-resume by handling -EAGAIN of pm_runtime_get_if_in_use(). Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> --- revision history ---------------- v2 (thank you Sakari and Laurent): * drop the other patch (the streaming-state in suspend/resume needs to be solved differently). * Sakari pointed out that many drivers are affected by this and that runtime-pm might need changes instead. I think this patch doesn't hurt and could serve as a reminder to do so. v1: initial submission: https://lore.kernel.org/linux-media/20230405092904.1129395-1-martin.kepplinger@puri.sm/ drivers/media/i2c/hi846.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)