| Message ID | 20230607164712.63579-5-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: ov2680: Bugfixes + ACPI + selection(crop-tgt) API support | expand |
On 07/06/2023 17:46, Hans de Goede wrote: > Select VIDEO_V4L2_SUBDEV_API in Kconfig and drop the > ifdef CONFIG_VIDEO_V4L2_SUBDEV_API checks, like other callers > of v4l2_subdev_get_try_format() do. > > This is a preparation patch for fixing ov2680_set_fmt() > which == V4L2_SUBDEV_FORMAT_TRY calls not properly filling in > the passed in v4l2_mbus_framefmt struct. > > Fixes: 3ee47cad3e69 ("media: ov2680: Add Omnivision OV2680 sensor driver") Not sure about Fixes on this one as I don't think it was necessarily broken before, but either way Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/media/i2c/Kconfig | 1 + > drivers/media/i2c/ov2680.c | 16 ++-------------- > 2 files changed, 3 insertions(+), 14 deletions(-) > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 8f55155afe67..791473fcbad3 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -433,6 +433,7 @@ config VIDEO_OV2680 > tristate "OmniVision OV2680 sensor support" > depends on VIDEO_DEV && I2C > select MEDIA_CONTROLLER > + select VIDEO_V4L2_SUBDEV_API > select V4L2_FWNODE > help > This is a Video4Linux2 sensor driver for the OmniVision > diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c > index c1b23c5b7818..d90bbca6e913 100644 > --- a/drivers/media/i2c/ov2680.c > +++ b/drivers/media/i2c/ov2680.c > @@ -559,7 +559,6 @@ static int ov2680_get_fmt(struct v4l2_subdev *sd, > { > struct ov2680_dev *sensor = to_ov2680_dev(sd); > struct v4l2_mbus_framefmt *fmt = NULL; > - int ret = 0; > > if (format->pad != 0) > return -EINVAL; > @@ -567,22 +566,17 @@ static int ov2680_get_fmt(struct v4l2_subdev *sd, > mutex_lock(&sensor->lock); > > if (format->which == V4L2_SUBDEV_FORMAT_TRY) { > -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API > fmt = v4l2_subdev_get_try_format(&sensor->sd, sd_state, > format->pad); > -#else > - ret = -EINVAL; > -#endif > } else { > fmt = &sensor->fmt; > } > > - if (fmt) > - format->format = *fmt; > + format->format = *fmt; > > mutex_unlock(&sensor->lock); > > - return ret; > + return 0; > } > > static int ov2680_set_fmt(struct v4l2_subdev *sd, > @@ -591,9 +585,7 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd, > { > struct ov2680_dev *sensor = to_ov2680_dev(sd); > struct v4l2_mbus_framefmt *fmt = &format->format; > -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API > struct v4l2_mbus_framefmt *try_fmt; > -#endif > const struct ov2680_mode_info *mode; > int ret = 0; > > @@ -616,10 +608,8 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd, > } > > if (format->which == V4L2_SUBDEV_FORMAT_TRY) { > -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API > try_fmt = v4l2_subdev_get_try_format(sd, sd_state, 0); > format->format = *try_fmt; > -#endif > goto unlock; > } > > @@ -777,9 +767,7 @@ static int ov2680_v4l2_register(struct ov2680_dev *sensor) > v4l2_i2c_subdev_init(&sensor->sd, sensor->i2c_client, > &ov2680_subdev_ops); > > -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API > sensor->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE; > -#endif > sensor->pad.flags = MEDIA_PAD_FL_SOURCE; > sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; >
Hi, On 6/12/23 10:20, Dan Scally wrote: > > On 07/06/2023 17:46, Hans de Goede wrote: >> Select VIDEO_V4L2_SUBDEV_API in Kconfig and drop the >> ifdef CONFIG_VIDEO_V4L2_SUBDEV_API checks, like other callers >> of v4l2_subdev_get_try_format() do. >> >> This is a preparation patch for fixing ov2680_set_fmt() >> which == V4L2_SUBDEV_FORMAT_TRY calls not properly filling in >> the passed in v4l2_mbus_framefmt struct. >> >> Fixes: 3ee47cad3e69 ("media: ov2680: Add Omnivision OV2680 sensor driver") > Not sure about Fixes on this one as I don't think it was necessarily broken before, but either way Ah right, I should have added a note after a cut line that the Fixes tag is there because this is a pre-requisite for further fixes in the series. > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> Thank you, Regards, Hans >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/media/i2c/Kconfig | 1 + >> drivers/media/i2c/ov2680.c | 16 ++-------------- >> 2 files changed, 3 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig >> index 8f55155afe67..791473fcbad3 100644 >> --- a/drivers/media/i2c/Kconfig >> +++ b/drivers/media/i2c/Kconfig >> @@ -433,6 +433,7 @@ config VIDEO_OV2680 >> tristate "OmniVision OV2680 sensor support" >> depends on VIDEO_DEV && I2C >> select MEDIA_CONTROLLER >> + select VIDEO_V4L2_SUBDEV_API >> select V4L2_FWNODE >> help >> This is a Video4Linux2 sensor driver for the OmniVision >> diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c >> index c1b23c5b7818..d90bbca6e913 100644 >> --- a/drivers/media/i2c/ov2680.c >> +++ b/drivers/media/i2c/ov2680.c >> @@ -559,7 +559,6 @@ static int ov2680_get_fmt(struct v4l2_subdev *sd, >> { >> struct ov2680_dev *sensor = to_ov2680_dev(sd); >> struct v4l2_mbus_framefmt *fmt = NULL; >> - int ret = 0; >> if (format->pad != 0) >> return -EINVAL; >> @@ -567,22 +566,17 @@ static int ov2680_get_fmt(struct v4l2_subdev *sd, >> mutex_lock(&sensor->lock); >> if (format->which == V4L2_SUBDEV_FORMAT_TRY) { >> -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API >> fmt = v4l2_subdev_get_try_format(&sensor->sd, sd_state, >> format->pad); >> -#else >> - ret = -EINVAL; >> -#endif >> } else { >> fmt = &sensor->fmt; >> } >> - if (fmt) >> - format->format = *fmt; >> + format->format = *fmt; >> mutex_unlock(&sensor->lock); >> - return ret; >> + return 0; >> } >> static int ov2680_set_fmt(struct v4l2_subdev *sd, >> @@ -591,9 +585,7 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd, >> { >> struct ov2680_dev *sensor = to_ov2680_dev(sd); >> struct v4l2_mbus_framefmt *fmt = &format->format; >> -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API >> struct v4l2_mbus_framefmt *try_fmt; >> -#endif >> const struct ov2680_mode_info *mode; >> int ret = 0; >> @@ -616,10 +608,8 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd, >> } >> if (format->which == V4L2_SUBDEV_FORMAT_TRY) { >> -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API >> try_fmt = v4l2_subdev_get_try_format(sd, sd_state, 0); >> format->format = *try_fmt; >> -#endif >> goto unlock; >> } >> @@ -777,9 +767,7 @@ static int ov2680_v4l2_register(struct ov2680_dev *sensor) >> v4l2_i2c_subdev_init(&sensor->sd, sensor->i2c_client, >> &ov2680_subdev_ops); >> -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API >> sensor->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE; >> -#endif >> sensor->pad.flags = MEDIA_PAD_FL_SOURCE; >> sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR; >> >
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index 8f55155afe67..791473fcbad3 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -433,6 +433,7 @@ config VIDEO_OV2680 tristate "OmniVision OV2680 sensor support" depends on VIDEO_DEV && I2C select MEDIA_CONTROLLER + select VIDEO_V4L2_SUBDEV_API select V4L2_FWNODE help This is a Video4Linux2 sensor driver for the OmniVision diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c index c1b23c5b7818..d90bbca6e913 100644 --- a/drivers/media/i2c/ov2680.c +++ b/drivers/media/i2c/ov2680.c @@ -559,7 +559,6 @@ static int ov2680_get_fmt(struct v4l2_subdev *sd, { struct ov2680_dev *sensor = to_ov2680_dev(sd); struct v4l2_mbus_framefmt *fmt = NULL; - int ret = 0; if (format->pad != 0) return -EINVAL; @@ -567,22 +566,17 @@ static int ov2680_get_fmt(struct v4l2_subdev *sd, mutex_lock(&sensor->lock); if (format->which == V4L2_SUBDEV_FORMAT_TRY) { -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API fmt = v4l2_subdev_get_try_format(&sensor->sd, sd_state, format->pad); -#else - ret = -EINVAL; -#endif } else { fmt = &sensor->fmt; } - if (fmt) - format->format = *fmt; + format->format = *fmt; mutex_unlock(&sensor->lock); - return ret; + return 0; } static int ov2680_set_fmt(struct v4l2_subdev *sd, @@ -591,9 +585,7 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd, { struct ov2680_dev *sensor = to_ov2680_dev(sd); struct v4l2_mbus_framefmt *fmt = &format->format; -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API struct v4l2_mbus_framefmt *try_fmt; -#endif const struct ov2680_mode_info *mode; int ret = 0; @@ -616,10 +608,8 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd, } if (format->which == V4L2_SUBDEV_FORMAT_TRY) { -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API try_fmt = v4l2_subdev_get_try_format(sd, sd_state, 0); format->format = *try_fmt; -#endif goto unlock; } @@ -777,9 +767,7 @@ static int ov2680_v4l2_register(struct ov2680_dev *sensor) v4l2_i2c_subdev_init(&sensor->sd, sensor->i2c_client, &ov2680_subdev_ops); -#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API sensor->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE; -#endif sensor->pad.flags = MEDIA_PAD_FL_SOURCE; sensor->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
Select VIDEO_V4L2_SUBDEV_API in Kconfig and drop the ifdef CONFIG_VIDEO_V4L2_SUBDEV_API checks, like other callers of v4l2_subdev_get_try_format() do. This is a preparation patch for fixing ov2680_set_fmt() which == V4L2_SUBDEV_FORMAT_TRY calls not properly filling in the passed in v4l2_mbus_framefmt struct. Fixes: 3ee47cad3e69 ("media: ov2680: Add Omnivision OV2680 sensor driver") Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/media/i2c/Kconfig | 1 + drivers/media/i2c/ov2680.c | 16 ++-------------- 2 files changed, 3 insertions(+), 14 deletions(-)