2018-04-16 19:55 GMT+09:00 Sakari Ailus <[email protected]>: > Hi Akinobu, > > As the driver now offers a V4L2 sub-device uAPI, it needs to serialise > access to its internal data structures. This appears to be fine, but there > are additional requirements; for instance ov772x_select_params() should > likely fail if you're streaming.
OK. I can find many subdev drivers that have 'streaming' flag in the private data to keep track of the streaming state and make set_fmt() return -EBUSY if streaming is on. I'll prepare another patch that does the same thing. > On Mon, Apr 16, 2018 at 11:51:51AM +0900, Akinobu Mita wrote: >> The set_fmt() in subdev pad ops, the s_ctrl() for subdev control handler, >> and the s_frame_interval() in subdev video ops could be called when the >> device is under power saving mode. These callbacks for ov772x driver >> cause updating H/W registers that will fail under power saving mode. >> >> This avoids it by not apply any changes to H/W if the device is not powered >> up. Instead the changes will be restored right after power-up. >> >> Cc: Jacopo Mondi <[email protected]> >> Cc: Laurent Pinchart <[email protected]> >> Cc: Hans Verkuil <[email protected]> >> Cc: Sakari Ailus <[email protected]> >> Cc: Mauro Carvalho Chehab <[email protected]> >> Signed-off-by: Akinobu Mita <[email protected]> >> --- >> * v2 >> - New patch >> >> drivers/media/i2c/ov772x.c | 77 >> +++++++++++++++++++++++++++++++++++++--------- >> 1 file changed, 62 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c >> index 1297a21..c44728f 100644 >> --- a/drivers/media/i2c/ov772x.c >> +++ b/drivers/media/i2c/ov772x.c >> @@ -741,19 +741,29 @@ static int ov772x_s_frame_interval(struct v4l2_subdev >> *sd, >> struct ov772x_priv *priv = to_ov772x(sd); >> struct v4l2_fract *tpf = &ival->interval; >> unsigned int fps; >> - int ret; >> + int ret = 0; >> >> fps = ov772x_select_fps(priv, tpf); >> >> - ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win); >> - if (ret) >> - return ret; >> + mutex_lock(&priv->power_lock); >> + /* >> + * If the device is not powered up by the host driver do >> + * not apply any changes to H/W at this time. Instead >> + * the frame rate will be restored right after power-up. >> + */ >> + if (priv->power_count > 0) { >> + ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win); >> + if (ret) >> + goto error; >> + } >> >> tpf->numerator = 1; >> tpf->denominator = fps; >> priv->fps = fps; > > Newline before a label would be nice. I see.
