Re: [PATCH v4 2/2] media: uvcvideo: Do power management granularly

From: Sergey Senozhatsky
Date: Mon Dec 05 2022 - 20:51:56 EST


On (22/12/02 18:19), Ricardo Ribalda wrote:
> /* ------------------------------------------------------------------------
> * UVC ioctls
> */
> @@ -249,6 +289,9 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
> * developers test their webcams with the Linux driver as well as with
> * the Windows driver).
> */
> + ret = uvc_pm_get(stream);
> + if (ret)
> + return ret;
> mutex_lock(&stream->mutex);

So here we acquire stream->dev->lock outside of stream->mutex scope

> if (stream->dev->quirks & UVC_QUIRK_PROBE_EXTRAFIELDS)
> probe->dwMaxVideoFrameSize =
> @@ -257,6 +300,7 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
> /* Probe the device. */
> ret = uvc_probe_video(stream, probe);
> mutex_unlock(&stream->mutex);
> + uvc_pm_put(stream);
> if (ret < 0)
> return ret;

[..]

> @@ -468,7 +512,13 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream,
> }
>
> /* Probe the device with the new settings. */
> + ret = uvc_pm_get(stream);
> + if (ret) {
> + mutex_unlock(&stream->mutex);
> + return ret;
> + }
> ret = uvc_probe_video(stream, &probe);
> + uvc_pm_put(stream);
> if (ret < 0) {
> mutex_unlock(&stream->mutex);
> return ret;

But here we nest and grab stream->dev->lock under stream->mutex.
Is this nesting intentional?