On (20/11/17 17:07), Sergey Senozhatsky wrote:
> > +static int ov9734_set_stream(struct v4l2_subdev *sd, int enable)
> > +{
> > + struct ov9734 *ov9734 = to_ov9734(sd);
> > + struct i2c_client *client = v4l2_get_subdevdata(sd);
> > + int ret = 0;
> > +
> > + if (ov9734->streaming == enable)
> > + return 0;
> > +
> > + mutex_lock(&ov9734->mutex);
> > + if (enable) {
> > + ret = pm_runtime_get_sync(&client->dev);
> > + if (ret < 0) {
> > + pm_runtime_put_noidle(&client->dev);
> > + mutex_unlock(&ov9734->mutex);
> > + return ret;
> > + }
> > +
> > + ret = ov9734_start_streaming(ov9734);
> > + if (ret) {
> > + enable = 0;
> > + ov9734_stop_streaming(ov9734);
> > + pm_runtime_put(&client->dev);
> > + }
> > + } else {
> > + ov9734_stop_streaming(ov9734);
> > + pm_runtime_put(&client->dev);
> > + }
>
> I assume that we will never see erroneous ->s_stream(0) calls or
> ->s_stream(0) after unsuccessful ->s_stream(1). Otherwise we will
> do extra pm_runtime_put(), not matched by pm_runtime_get().
Oh, no. There is (unprotected) if (ov9734->streaming) check
at the very top, so we are probably fine.
-ss
