From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Cc: linux-media@vger.kernel.org, linux-mediatek@lists.infradead.org,
imx@lists.linux.dev, linux-amlogic@lists.infradead.org,
linux-renesas-soc@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-rockchip@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
linux-arm-msm@vger.kernel.org,
"Matthew Majewski" <mattwmajewski@gmail.com>,
"Mirela Rabulea" <mirela.rabulea@nxp.com>,
"Jacek Anaszewski" <jacek.anaszewski@gmail.com>,
"Michael Tretter" <m.tretter@pengutronix.de>,
"Devarsh Thakkar" <devarsht@ti.com>,
"Uwe Kleine-König" <u.kleine-koenig@baylibre.com>,
"AngeloGioacchino Del Regno"
<angelogioacchino.delregno@collabora.com>,
"Martin Blumenstingl" <martin.blumenstingl@googlemail.com>,
"Alexandre Torgue" <alexandre.torgue@foss.st.com>,
"Philipp Zabel" <p.zabel@pengutronix.de>,
"Arnd Bergmann" <arnd@arndb.de>,
"Sylwester Nawrocki" <s.nawrocki@samsung.com>,
"Ming Qian" <ming.qian@nxp.com>,
"Pengutronix Kernel Team" <kernel@pengutronix.de>,
"Kieran Bingham" <kieran.bingham+renesas@ideasonboard.com>,
"Nathan Chancellor" <nathan@kernel.org>,
"Sascha Hauer" <s.hauer@pengutronix.de>,
"Hans Verkuil" <hverkuil@kernel.org>,
"Kevin Hilman" <khilman@baylibre.com>,
"Xavier Roumegue" <xavier.roumegue@oss.nxp.com>,
"Shuah Khan" <skhan@linuxfoundation.org>,
"Andrzej Pietrasiewicz" <andrzejtp2010@gmail.com>,
"Tiffany Lin" <tiffany.lin@mediatek.com>,
"Zhou Peng" <eagle.zhou@nxp.com>,
"Dikshita Agarwal" <quic_dikshita@quicinc.com>,
"Vikash Garodia" <quic_vgarodia@quicinc.com>,
"Yunfei Dong" <yunfei.dong@mediatek.com>,
"Ezequiel Garcia" <ezequiel@vanguardiasur.com.ar>,
"Neil Armstrong" <neil.armstrong@linaro.org>,
"Jiasheng Jiang" <jiashengjiangcool@gmail.com>,
"Shawn Guo" <shawnguo@kernel.org>,
"Łukasz Stelmach" <l.stelmach@samsung.com>,
"Abhinav Kumar" <abhinav.kumar@linux.dev>,
"Heiko Stuebner" <heiko@sntech.de>,
"Benoit Parrot" <bparrot@ti.com>,
"Andrew-CT Chen" <andrew-ct.chen@mediatek.com>,
"Geert Uytterhoeven" <geert+renesas@glider.be>,
"Magnus Damm" <magnus.damm@gmail.com>,
"Maxime Coquelin" <mcoquelin.stm32@gmail.com>,
"Jacob Chen" <jacob-chen@iotwrt.com>,
"Bin Liu" <bin.liu@mediatek.com>,
"Matthias Brugger" <matthias.bgg@gmail.com>,
"Fabio Estevam" <festevam@gmail.com>,
"Jerome Brunet" <jbrunet@baylibre.com>,
"Bryan O'Donoghue" <bryan.odonoghue@linaro.org>
Subject: Re: [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult
Date: Wed, 8 Oct 2025 22:21:29 +0300 [thread overview]
Message-ID: <20251008192129.GG16422@pendragon.ideasonboard.com> (raw)
In-Reply-To: <205478244873d09cad5b77bd887f6a836c31c7ec.camel@collabora.com>
On Wed, Oct 08, 2025 at 02:24:30PM -0400, Nicolas Dufresne wrote:
> Hi,
>
> Le mercredi 08 octobre 2025 à 20:50 +0300, Laurent Pinchart a écrit :
> > Hello,
> >
> > The v4l2_m2m_get_vq() function never returns NULL, but many mem2mem
> > drivers still check its return value and consider NULL as an error. This
> > may have originated a long time ago from valid checks when
> > v4l2_m2m_get_vq() could return NULL, with drivers then just copying the
> > checks. This series attempts to stop the cargo-cult behaviour.
> >
> > Patch 01/25 starts by explicitly stating in kerneldoc that the
> > v4l2_m2m_get_vq() function never returns NULL. All the other patches
> > drop NULL checks from drivers.
> >
> > I have carefully checked all patched locations in all drivers. They fall
> > in 3 categories:
> >
> > - Checks in the VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIOC_S_FMT handlers:
> > Those may have been added to ensure that the format type has a valid
> > value, but that is ensured by the V4L2 ioctl core before calling the
> > handlers. The checks can be dropped without a need to replace them
> > with proper type checks.
> >
> > - Checks in the VIDIOC_S_SELECTION handler: The only location where this
> > is performed has an explicit type check, so the NULL check can also be
> > dropped.
> >
> > - Checks in other locations where the type parameter to the
> > v4l2_m2m_get_vq() function is hardcoded: The hardcoded type is valid,
> > so the NULL check can't have been meant to check the type. It can also
> > be removed.
> >
> > There's no dependency between any of those patches so they can be merged
> > in any order.
> >
> > Laurent Pinchart (25):
> > media: v4l2-mem2mem: Document that v4l2_m2m_get_vq() never returns
> > NULL
> > media: allgro-dvt: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: meson-g2d: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: amphion: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: coda: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: imagination: e5010: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: m2m-deinterlace: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: mediatek: jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: mediatek: vcodec: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: dw100: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: imx-jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: imx-pxp: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: nxp: imx8-isi: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: mx2_emmaprp: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: qcom: venus: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: renesas: fdp1: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: rcar_jpu: Drop unneeded v4l2_m2m_get_vq() NULL check
>
> Why not "renesas: jpu" to match the fdp1 patch naming ?
I tried to go with the most common prefix as reported by git log. I
don't mind changing this, I'll wait for more reviews to see if a v2 is
needed, otherwise this can be updated when applying if desired.
> > media: platform: rga: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: samsung: s5p-g2d: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: samsung: s5p-jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: stm32: dma2d: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: ti: vpe: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: vicodec: Drop unneeded v4l2_m2m_get_vq() NULL check
> > media: vim2m: Drop unneeded v4l2_m2m_get_vq() NULL check
>
> I reviewed the list and it seems complete to me.
Thank you.
> > drivers/media/platform/allegro-dvt/allegro-core.c | 2 --
> > drivers/media/platform/amlogic/meson-ge2d/ge2d.c | 5 -----
> > drivers/media/platform/amphion/vdec.c | 2 --
> > drivers/media/platform/amphion/venc.c | 2 --
> > .../media/platform/chips-media/coda/coda-common.c | 4 ----
> > drivers/media/platform/imagination/e5010-jpeg-enc.c | 4 ----
> > drivers/media/platform/m2m-deinterlace.c | 7 -------
> > drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c | 7 -------
> > .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 7 -------
> > .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c | 2 --
> > .../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c | 2 --
> > .../mediatek/vcodec/encoder/mtk_vcodec_enc.c | 8 --------
> > drivers/media/platform/nxp/dw100/dw100.c | 7 -------
> > drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 4 ----
> > drivers/media/platform/nxp/imx-pxp.c | 7 -------
> > drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c | 2 --
> > drivers/media/platform/nxp/mx2_emmaprp.c | 7 -------
> > drivers/media/platform/qcom/iris/iris_vdec.c | 2 --
> > drivers/media/platform/qcom/venus/vdec.c | 2 --
> > drivers/media/platform/qcom/venus/venc.c | 2 --
> > drivers/media/platform/renesas/rcar_fdp1.c | 3 ---
> > drivers/media/platform/renesas/rcar_jpu.c | 8 --------
> > drivers/media/platform/rockchip/rga/rga.c | 4 ----
> > drivers/media/platform/samsung/s5p-g2d/g2d.c | 4 ----
> > drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c | 7 -------
> > drivers/media/platform/st/stm32/dma2d/dma2d.c | 5 -----
> > drivers/media/platform/ti/vpe/vpe.c | 7 -------
> > drivers/media/test-drivers/vicodec/vicodec-core.c | 7 -------
> > drivers/media/test-drivers/vim2m.c | 12 ------------
> > drivers/media/v4l2-core/v4l2-mem2mem.c | 12 +-----------
> > include/media/v4l2-mem2mem.h | 3 +++
> > 31 files changed, 4 insertions(+), 153 deletions(-)
> >
> >
> > base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
--
Regards,
Laurent Pinchart
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
prev parent reply other threads:[~2025-10-08 19:21 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-08 17:50 [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult Laurent Pinchart
2025-10-08 17:50 ` [PATCH 03/25] media: meson-g2d: Drop unneeded v4l2_m2m_get_vq() NULL check Laurent Pinchart
2025-10-08 18:13 ` Neil Armstrong
2025-10-08 18:24 ` [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult Nicolas Dufresne
2025-10-08 19:21 ` Laurent Pinchart [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251008192129.GG16422@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=abhinav.kumar@linux.dev \
--cc=alexandre.torgue@foss.st.com \
--cc=andrew-ct.chen@mediatek.com \
--cc=andrzejtp2010@gmail.com \
--cc=angelogioacchino.delregno@collabora.com \
--cc=arnd@arndb.de \
--cc=bin.liu@mediatek.com \
--cc=bparrot@ti.com \
--cc=bryan.odonoghue@linaro.org \
--cc=devarsht@ti.com \
--cc=eagle.zhou@nxp.com \
--cc=ezequiel@vanguardiasur.com.ar \
--cc=festevam@gmail.com \
--cc=geert+renesas@glider.be \
--cc=heiko@sntech.de \
--cc=hverkuil@kernel.org \
--cc=imx@lists.linux.dev \
--cc=jacek.anaszewski@gmail.com \
--cc=jacob-chen@iotwrt.com \
--cc=jbrunet@baylibre.com \
--cc=jiashengjiangcool@gmail.com \
--cc=kernel@pengutronix.de \
--cc=khilman@baylibre.com \
--cc=kieran.bingham+renesas@ideasonboard.com \
--cc=l.stelmach@samsung.com \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=m.tretter@pengutronix.de \
--cc=magnus.damm@gmail.com \
--cc=martin.blumenstingl@googlemail.com \
--cc=matthias.bgg@gmail.com \
--cc=mattwmajewski@gmail.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=ming.qian@nxp.com \
--cc=mirela.rabulea@nxp.com \
--cc=nathan@kernel.org \
--cc=neil.armstrong@linaro.org \
--cc=nicolas.dufresne@collabora.com \
--cc=p.zabel@pengutronix.de \
--cc=quic_dikshita@quicinc.com \
--cc=quic_vgarodia@quicinc.com \
--cc=s.hauer@pengutronix.de \
--cc=s.nawrocki@samsung.com \
--cc=shawnguo@kernel.org \
--cc=skhan@linuxfoundation.org \
--cc=tiffany.lin@mediatek.com \
--cc=u.kleine-koenig@baylibre.com \
--cc=xavier.roumegue@oss.nxp.com \
--cc=yunfei.dong@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox