media: camss: camss-video: Don't zero subdev format again after initialization

[PATCH] media: camss: camss-video: Don't zero subdev format again after initialization

Posted by Yassine Oudjana 2 years, 7 months ago

From: Yassine Oudjana <y.oudjana@protonmail.com>

In an earlier commit, setting the which field of the subdev format struct
in video_get_subdev_format was moved to a designated initializer that also
zeroes all other fields. However, the memset call that was zeroing the
fields earlier was left in place, causing the which field to be cleared
after being set in the initializer.

Remove the memset call from video_get_subdev_format to avoid clearing the
initialized which field.

Fixes: ecefa105cc44 ("media: Zero-initialize all structures passed to subdev pad operations")
Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
---
 drivers/media/platform/qcom/camss/camss-video.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
index 898f32177b12..8640db306026 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -353,7 +353,6 @@ static int video_get_subdev_format(struct camss_video *video,
 	if (subdev == NULL)
 		return -EPIPE;
 
-	memset(&fmt, 0, sizeof(fmt));
 	fmt.pad = pad;
 
 	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);
-- 
2.40.0

Re: [PATCH] media: camss: camss-video: Don't zero subdev format again after initialization

Posted by Laurent Pinchart 2 years, 6 months ago

Hi Yassine,

Thank you for the patch.

On Wed, May 03, 2023 at 10:53:40AM +0300, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> In an earlier commit, setting the which field of the subdev format struct
> in video_get_subdev_format was moved to a designated initializer that also
> zeroes all other fields. However, the memset call that was zeroing the
> fields earlier was left in place, causing the which field to be cleared
> after being set in the initializer.
> 
> Remove the memset call from video_get_subdev_format to avoid clearing the
> initialized which field.
> 
> Fixes: ecefa105cc44 ("media: Zero-initialize all structures passed to subdev pad operations")
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

This is a regression fix, I'll send a pull request right away.

> ---
>  drivers/media/platform/qcom/camss/camss-video.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index 898f32177b12..8640db306026 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -353,7 +353,6 @@ static int video_get_subdev_format(struct camss_video *video,
>  	if (subdev == NULL)
>  		return -EPIPE;
>  
> -	memset(&fmt, 0, sizeof(fmt));
>  	fmt.pad = pad;
>  
>  	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: camss: camss-video: Don't zero subdev format again after initialization

Posted by Laurent Pinchart 2 years, 6 months ago

On Tue, May 30, 2023 at 08:42:30PM +0300, Laurent Pinchart wrote:
> Hi Yassine,
> 
> Thank you for the patch.
> 
> On Wed, May 03, 2023 at 10:53:40AM +0300, Yassine Oudjana wrote:
> > From: Yassine Oudjana <y.oudjana@protonmail.com>
> > 
> > In an earlier commit, setting the which field of the subdev format struct
> > in video_get_subdev_format was moved to a designated initializer that also
> > zeroes all other fields. However, the memset call that was zeroing the
> > fields earlier was left in place, causing the which field to be cleared
> > after being set in the initializer.
> > 
> > Remove the memset call from video_get_subdev_format to avoid clearing the
> > initialized which field.
> > 
> > Fixes: ecefa105cc44 ("media: Zero-initialize all structures passed to subdev pad operations")
> > Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> This is a regression fix, I'll send a pull request right away.

The patch has actually been applied to the media fixes branch already
:-)

> > ---
> >  drivers/media/platform/qcom/camss/camss-video.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> > index 898f32177b12..8640db306026 100644
> > --- a/drivers/media/platform/qcom/camss/camss-video.c
> > +++ b/drivers/media/platform/qcom/camss/camss-video.c
> > @@ -353,7 +353,6 @@ static int video_get_subdev_format(struct camss_video *video,
> >  	if (subdev == NULL)
> >  		return -EPIPE;
> >  
> > -	memset(&fmt, 0, sizeof(fmt));
> >  	fmt.pad = pad;
> >  
> >  	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: camss: camss-video: Don't zero subdev format again after initialization

Posted by Andrey Konovalov 2 years, 7 months ago

On 03.05.2023 10:53, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> In an earlier commit, setting the which field of the subdev format struct
> in video_get_subdev_format was moved to a designated initializer that also
> zeroes all other fields. However, the memset call that was zeroing the
> fields earlier was left in place, causing the which field to be cleared
> after being set in the initializer.
> 
> Remove the memset call from video_get_subdev_format to avoid clearing the
> initialized which field.
> 
> Fixes: ecefa105cc44 ("media: Zero-initialize all structures passed to subdev pad operations")
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
>   drivers/media/platform/qcom/camss/camss-video.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index 898f32177b12..8640db306026 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -353,7 +353,6 @@ static int video_get_subdev_format(struct camss_video *video,
>   	if (subdev == NULL)
>   		return -EPIPE;
>   
> -	memset(&fmt, 0, sizeof(fmt));
>   	fmt.pad = pad;
>   
>   	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);

Have verified that this fix does work on RB3 and RB5 boards and is 
necessary for the capture to work.

Thanks!

Tested-by: Andrey Konovalov <andrey.konovalov@linaro.org>

Re: [PATCH] media: camss: camss-video: Don't zero subdev format again after initialization

Posted by Bryan O'Donoghue 2 years, 7 months ago

On 03/05/2023 08:53, Yassine Oudjana wrote:
> From: Yassine Oudjana <y.oudjana@protonmail.com>
> 
> In an earlier commit, setting the which field of the subdev format struct
> in video_get_subdev_format was moved to a designated initializer that also
> zeroes all other fields. However, the memset call that was zeroing the
> fields earlier was left in place, causing the which field to be cleared
> after being set in the initializer.
> 
> Remove the memset call from video_get_subdev_format to avoid clearing the
> initialized which field.
> 
> Fixes: ecefa105cc44 ("media: Zero-initialize all structures passed to subdev pad operations")
> Signed-off-by: Yassine Oudjana <y.oudjana@protonmail.com>
> ---
>   drivers/media/platform/qcom/camss/camss-video.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index 898f32177b12..8640db306026 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -353,7 +353,6 @@ static int video_get_subdev_format(struct camss_video *video,
>   	if (subdev == NULL)
>   		return -EPIPE;
>   
> -	memset(&fmt, 0, sizeof(fmt));
>   	fmt.pad = pad;
>   
>   	ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt);

Acked-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>