| Message ID | 1449849893-14865-4-git-send-email-ulrich.hecht+renesas@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hello. On 12/11/2015 07:04 PM, Ulrich Hecht wrote: > Without this, g_crop will always return the boot-time state. > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > --- > drivers/media/i2c/adv7604.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c > index 1bfa9f3..d7d0bb7 100644 > --- a/drivers/media/i2c/adv7604.c > +++ b/drivers/media/i2c/adv7604.c > @@ -1975,6 +1975,15 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32 status, bool *handled) > > v4l2_subdev_notify_event(sd, &adv76xx_ev_fmt); > > + /* update timings */ > + if (adv76xx_query_dv_timings(sd, &state->timings) > + == -ENOLINK) { Please don't put the binary operators on the continuation line, leave them at the end of he broken up line. [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulrich, Thank you for the patch. On Friday 11 December 2015 17:04:53 Ulrich Hecht wrote: > Without this, g_crop will always return the boot-time state. > > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> > --- > drivers/media/i2c/adv7604.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c > index 1bfa9f3..d7d0bb7 100644 > --- a/drivers/media/i2c/adv7604.c > +++ b/drivers/media/i2c/adv7604.c > @@ -1975,6 +1975,15 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32 > status, bool *handled) > > v4l2_subdev_notify_event(sd, &adv76xx_ev_fmt); > > + /* update timings */ > + if (adv76xx_query_dv_timings(sd, &state->timings) > + == -ENOLINK) { Nitpicking, I would write this as ret = adv76xx_query_dv_timings(sd, &state->timings); if (ret == -ENOLINK) { to make it more explicit that the function has side effects. Functions called inside an if () statement are often assumed (at least by me) to perform checks only and not modify their parameters. > + /* no signal, fall back to default timings */ > + const struct v4l2_dv_timings cea640x480 = > + V4L2_DV_BT_CEA_640X480P59_94; > + state->timings = cea640x480; You can write this as state->timings = (struct v4l2_dv_timings) V4L2_DV_BT_CEA_640X480P59_94; without using a local variable. (And now that I mention that I wonder whether the definition of V4L2_DV_BT_CEA_640X480P59_94 should be updated to include the (struct v4l2_dv_timings)) > + } > + > if (handled) > *handled = true; > }
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index 1bfa9f3..d7d0bb7 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -1975,6 +1975,15 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32 status, bool *handled) v4l2_subdev_notify_event(sd, &adv76xx_ev_fmt); + /* update timings */ + if (adv76xx_query_dv_timings(sd, &state->timings) + == -ENOLINK) { + /* no signal, fall back to default timings */ + const struct v4l2_dv_timings cea640x480 = + V4L2_DV_BT_CEA_640X480P59_94; + state->timings = cea640x480; + } + if (handled) *handled = true; }
Without this, g_crop will always return the boot-time state. Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com> --- drivers/media/i2c/adv7604.c | 9 +++++++++ 1 file changed, 9 insertions(+)