| Message ID | 20230502103547.150918-3-dan.scally@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ST MIPID02 bridge format propagation fixes | expand |
Hi Daniel, Thank you for your patch. On 5/2/23 12:35, Daniel Scally wrote: > When setting formats on the sink pad, propagate the adjusted format > over to the subdev's source pad. Use the MIPID02_SOURCE macro to fetch the pad's > try format rather than relying on the pad field of the format to facilitate > this - the function is specific to the source pad anyway. > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> > --- > drivers/media/i2c/st-mipid02.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c > index f20f87562bf1..04112f26bc9d 100644 > --- a/drivers/media/i2c/st-mipid02.c > +++ b/drivers/media/i2c/st-mipid02.c > @@ -750,7 +750,7 @@ static void mipid02_set_fmt_source(struct v4l2_subdev *sd, > if (format->which != V4L2_SUBDEV_FORMAT_TRY) > return; > > - *v4l2_subdev_get_try_format(sd, sd_state, format->pad) = format->format; > + *v4l2_subdev_get_try_format(sd, sd_state, MIPID02_SOURCE) = format->format; > } > > static void mipid02_set_fmt_sink(struct v4l2_subdev *sd, > @@ -768,6 +768,9 @@ static void mipid02_set_fmt_sink(struct v4l2_subdev *sd, > fmt = &bridge->fmt; > > *fmt = format->format; > + > + /* Propagate the format change to the source pad */ > + mipid02_set_fmt_source(sd, sd_state, format); > } > > static int mipid02_set_fmt(struct v4l2_subdev *sd, By running checkpatch, I got 2 warnings : $ ./scripts/checkpatch.pl --strict --max-line-length=80 WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #7: over to the subdev's source pad. Use the MIPID02_SOURCE macro to fetch the pad's WARNING: line length of 83 exceeds 80 columns #25: FILE: drivers/media/i2c/st-mipid02.c:753: + *v4l2_subdev_get_try_format(sd, sd_state, MIPID02_SOURCE) = format->format; Could you fix these in version 2 ? st-mipid02.c has other styling issues but I'd like not to add new ones ;) Other than that, the code looks fine for me. Thank you.
Hello Benjamin On 10/05/2023 12:22, Benjamin Mugnier wrote: > Hi Daniel, > > Thank you for your patch. > > On 5/2/23 12:35, Daniel Scally wrote: >> When setting formats on the sink pad, propagate the adjusted format >> over to the subdev's source pad. Use the MIPID02_SOURCE macro to fetch the pad's >> try format rather than relying on the pad field of the format to facilitate >> this - the function is specific to the source pad anyway. >> >> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> >> --- >> drivers/media/i2c/st-mipid02.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c >> index f20f87562bf1..04112f26bc9d 100644 >> --- a/drivers/media/i2c/st-mipid02.c >> +++ b/drivers/media/i2c/st-mipid02.c >> @@ -750,7 +750,7 @@ static void mipid02_set_fmt_source(struct v4l2_subdev *sd, >> if (format->which != V4L2_SUBDEV_FORMAT_TRY) >> return; >> >> - *v4l2_subdev_get_try_format(sd, sd_state, format->pad) = format->format; >> + *v4l2_subdev_get_try_format(sd, sd_state, MIPID02_SOURCE) = format->format; >> } >> >> static void mipid02_set_fmt_sink(struct v4l2_subdev *sd, >> @@ -768,6 +768,9 @@ static void mipid02_set_fmt_sink(struct v4l2_subdev *sd, >> fmt = &bridge->fmt; >> >> *fmt = format->format; >> + >> + /* Propagate the format change to the source pad */ >> + mipid02_set_fmt_source(sd, sd_state, format); >> } >> >> static int mipid02_set_fmt(struct v4l2_subdev *sd, > By running checkpatch, I got 2 warnings : > > $ ./scripts/checkpatch.pl --strict --max-line-length=80 > WARNING: Possible unwrapped commit description (prefer a maximum 75 > chars per line) > #7: > over to the subdev's source pad. Use the MIPID02_SOURCE macro to fetch > the pad's > > WARNING: line length of 83 exceeds 80 columns > #25: FILE: drivers/media/i2c/st-mipid02.c:753: > + *v4l2_subdev_get_try_format(sd, sd_state, MIPID02_SOURCE) = > format->format; > > Could you fix these in version 2 ? st-mipid02.c has other styling issues > but I'd like not to add new ones ;) Sure thing - thanks for the review > > Other than that, the code looks fine for me. > > > Thank you. >
diff --git a/drivers/media/i2c/st-mipid02.c b/drivers/media/i2c/st-mipid02.c index f20f87562bf1..04112f26bc9d 100644 --- a/drivers/media/i2c/st-mipid02.c +++ b/drivers/media/i2c/st-mipid02.c @@ -750,7 +750,7 @@ static void mipid02_set_fmt_source(struct v4l2_subdev *sd, if (format->which != V4L2_SUBDEV_FORMAT_TRY) return; - *v4l2_subdev_get_try_format(sd, sd_state, format->pad) = format->format; + *v4l2_subdev_get_try_format(sd, sd_state, MIPID02_SOURCE) = format->format; } static void mipid02_set_fmt_sink(struct v4l2_subdev *sd, @@ -768,6 +768,9 @@ static void mipid02_set_fmt_sink(struct v4l2_subdev *sd, fmt = &bridge->fmt; *fmt = format->format; + + /* Propagate the format change to the source pad */ + mipid02_set_fmt_source(sd, sd_state, format); } static int mipid02_set_fmt(struct v4l2_subdev *sd,
When setting formats on the sink pad, propagate the adjusted format over to the subdev's source pad. Use the MIPID02_SOURCE macro to fetch the pad's try format rather than relying on the pad field of the format to facilitate this - the function is specific to the source pad anyway. Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> --- drivers/media/i2c/st-mipid02.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)