diff mbox series

[04/28] media: ov2680: Use select VIDEO_V4L2_SUBDEV_API

Message ID 20230607164712.63579-5-hdegoede@redhat.com (mailing list archive)
State Superseded
Headers
Series media: ov2680: Bugfixes + ACPI + selection(crop-tgt) API support |

Commit Message

Hans de Goede June 7, 2023, 4:46 p.m. UTC 
  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(-)

Comments

Dan Scally June 12, 2023, 8:20 a.m. UTC | #1 
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;
>
Hans de Goede June 12, 2023, 8:57 a.m. UTC | #2 
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 mbox series

Patch

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;