diff mbox series

[5/6] omap3isp: fix sparse warning

Message ID 20190207091338.55705-6-hverkuil-cisco@xs4all.nl (mailing list archive)
State New, archived
Headers show
Series sparse/smatch fixes | expand

Commit Message

Hans Verkuil Feb. 7, 2019, 9:13 a.m. UTC
drivers/media/platform/omap3isp/ispvideo.c:1013:15: warning: unknown expression (4 0)

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/platform/omap3isp/ispvideo.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Hans Verkuil Feb. 7, 2019, 9:18 a.m. UTC | #1
On 2/7/19 10:13 AM, Hans Verkuil wrote:
> drivers/media/platform/omap3isp/ispvideo.c:1013:15: warning: unknown expression (4 0)

I should add this text to the commit log:

The combination of the v4l2_subdev_call and media_entity_to_v4l2_subdev macros
became too complex for sparse. So first assign the result of
media_entity_to_v4l2_subdev to a struct v4l2_subdev *sd variable, then
use that in v4l2_subdev_call.

Regards,

	Hans

> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/platform/omap3isp/ispvideo.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
> index 078d64114b24..911dfad90d32 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -974,6 +974,7 @@ static int isp_video_check_external_subdevs(struct isp_video *video,
>  	struct media_entity *source = NULL;
>  	struct media_entity *sink;
>  	struct v4l2_subdev_format fmt;
> +	struct v4l2_subdev *sd;
>  	struct v4l2_ext_controls ctrls;
>  	struct v4l2_ext_control ctrl;
>  	unsigned int i;
> @@ -1010,8 +1011,8 @@ static int isp_video_check_external_subdevs(struct isp_video *video,
>  
>  	fmt.pad = source_pad->index;
>  	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> -	ret = v4l2_subdev_call(media_entity_to_v4l2_subdev(sink),
> -			       pad, get_fmt, NULL, &fmt);
> +	sd = media_entity_to_v4l2_subdev(sink);
> +	ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
>  	if (unlikely(ret < 0)) {
>  		dev_warn(isp->dev, "get_fmt returned null!\n");
>  		return ret;
>
Sakari Ailus Feb. 7, 2019, 2:23 p.m. UTC | #2
On Thu, Feb 07, 2019 at 10:13:37AM +0100, Hans Verkuil wrote:
> drivers/media/platform/omap3isp/ispvideo.c:1013:15: warning: unknown expression (4 0)
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Laurent Pinchart Feb. 7, 2019, 3:45 p.m. UTC | #3
Hi Hans,

Thank you for the patch.

On Thu, Feb 07, 2019 at 10:13:37AM +0100, Hans Verkuil wrote:
> drivers/media/platform/omap3isp/ispvideo.c:1013:15: warning: unknown expression (4 0)
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/platform/omap3isp/ispvideo.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
> index 078d64114b24..911dfad90d32 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -974,6 +974,7 @@ static int isp_video_check_external_subdevs(struct isp_video *video,
>  	struct media_entity *source = NULL;
>  	struct media_entity *sink;
>  	struct v4l2_subdev_format fmt;
> +	struct v4l2_subdev *sd;
>  	struct v4l2_ext_controls ctrls;
>  	struct v4l2_ext_control ctrl;
>  	unsigned int i;
> @@ -1010,8 +1011,8 @@ static int isp_video_check_external_subdevs(struct isp_video *video,
>  
>  	fmt.pad = source_pad->index;
>  	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> -	ret = v4l2_subdev_call(media_entity_to_v4l2_subdev(sink),
> -			       pad, get_fmt, NULL, &fmt);
> +	sd = media_entity_to_v4l2_subdev(sink);
> +	ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);

This is really a workaround for an issue in smath, and I don't like it.
Where will we draw the line if we accept it ? Will we start rejecting
all nested function calls or macros because we have a tooling issue ?
This one really needs to be fixed in smatch, the code is totally fine.

>  	if (unlikely(ret < 0)) {
>  		dev_warn(isp->dev, "get_fmt returned null!\n");
>  		return ret;
diff mbox series

Patch

diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
index 078d64114b24..911dfad90d32 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -974,6 +974,7 @@  static int isp_video_check_external_subdevs(struct isp_video *video,
 	struct media_entity *source = NULL;
 	struct media_entity *sink;
 	struct v4l2_subdev_format fmt;
+	struct v4l2_subdev *sd;
 	struct v4l2_ext_controls ctrls;
 	struct v4l2_ext_control ctrl;
 	unsigned int i;
@@ -1010,8 +1011,8 @@  static int isp_video_check_external_subdevs(struct isp_video *video,
 
 	fmt.pad = source_pad->index;
 	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-	ret = v4l2_subdev_call(media_entity_to_v4l2_subdev(sink),
-			       pad, get_fmt, NULL, &fmt);
+	sd = media_entity_to_v4l2_subdev(sink);
+	ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
 	if (unlikely(ret < 0)) {
 		dev_warn(isp->dev, "get_fmt returned null!\n");
 		return ret;