| Message ID | 20230503075340.45755-1-y.oudjana@protonmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: camss: camss-video: Don't zero subdev format again after initialization | expand |
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>
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>
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);
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);
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);