[PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult

[Nicolas Dufresne]

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 ?

>   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.

Nicolas

> 
>  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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20251008/56af186b/attachment.sig>