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@xxxxxxxxxxxxxxxx>
Thank you,
Regards,
Hans
>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>> ---
>> 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;
>>
>
