Re: [PATCH 1/2] media: verisilicon: Remove deprecated variant

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] media: verisilicon: Removing DT compatible string breaks backward compatibility
- [Low] media: verisilicon: Dead and misleading backward compatibility block in hantro_probe()
- [Low] media: verisilicon: Incomplete removal of imx8mq_vpu_variant leaves dead code
--

commit 1f4bbb2a1b7cf6366695f8a3ceae88b4f0b1e95c
Author: Benjamin Gaignard <benjamin.gaignard@collabora.com>

media: verisilicon: Remove deprecated variant
    
"nxp,imx8mq-vpu" is tagged as deprecated since:
commit f3116f2e52e6 ("media: dt-bindings: media: nxp, imx8mq-vpu: Split G1 and G2 nodes")
merged since v5.17-rc1.
There is more reference to it since that.
Remove for driver and Documentation to clean up the code.

> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index ad71c0402ef3b..70de47ddb023b 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -725,7 +725,6 @@ static const struct of_device_id of_hantro_match[] = {
>  #endif
>  #ifdef CONFIG_VIDEO_HANTRO_IMX8M
>  	{ .compatible = "nxp,imx8mm-vpu-g1", .data = &imx8mm_vpu_g1_variant, },
> -	{ .compatible = "nxp,imx8mq-vpu", .data = &imx8mq_vpu_variant, },

[Severity: High]
Does removing this compatible string break backward compatibility for older
device trees? The driver will silently fail to probe for the VPU on systems
using older Device Tree Blobs that only use the nxp,imx8mq-vpu compatible
string.

[Severity: Low]
Since nxp,imx8mq-vpu is removed from the match table, does this leave a dead
backward compatibility block in hantro_probe()?

drivers/media/platform/verisilicon/hantro_drv.c:hantro_probe() {
	...
	/*
	 * Support for nxp,imx8mq-vpu is kept for backwards compatibility
	 * but it's deprecated. Please update your DTS file to use
	 * nxp,imx8mq-vpu-g1 or nxp,imx8mq-vpu-g2 instead.
	 */
	if (of_device_is_compatible(pdev->dev.of_node, "nxp,imx8mq-vpu"))
		dev_warn(&pdev->dev, "%s compatible is deprecated\n",
			 match->compatible);
	...
}

If an older DTB with only nxp,imx8mq-vpu is used, the device won't probe and
the block is never reached. If a node happens to include both the new valid
string and the deprecated one, the driver will probe via the valid string, but
this block will execute and incorrectly print that the valid string is
deprecated.

>  	{ .compatible = "nxp,imx8mq-vpu-g1", .data = &imx8mq_vpu_g1_variant },
>  	{ .compatible = "nxp,imx8mq-vpu-g2", .data = &imx8mq_vpu_g2_variant },
>  #endif

> diff --git a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> index fa4224de4b996..3b63f626d33f2 100644
> --- a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> +++ b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
[ ... ]
> -const struct hantro_variant imx8mq_vpu_variant = {
> -	.dec_fmts = imx8m_vpu_dec_fmts,
> -	.num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
> -	.postproc_fmts = imx8m_vpu_postproc_fmts,
> -	.num_postproc_fmts = ARRAY_SIZE(imx8m_vpu_postproc_fmts),
> -	.postproc_ops = &hantro_g1_postproc_ops,
> -	.codec = HANTRO_MPEG2_DECODER | HANTRO_VP8_DECODER |
> -		 HANTRO_H264_DECODER,
> -	.codec_ops = imx8mq_vpu_codec_ops,
> -	.init = imx8mq_vpu_hw_init,
> -	.runtime_resume = imx8mq_runtime_resume,
> -	.irqs = imx8mq_irqs,
> -	.num_irqs = ARRAY_SIZE(imx8mq_irqs),
> -	.clk_names = imx8mq_clk_names,
> -	.num_clocks = ARRAY_SIZE(imx8mq_clk_names),
> -	.reg_names = imx8mq_reg_names,
> -	.num_regs = ARRAY_SIZE(imx8mq_reg_names)
> -};
> -

[Severity: Low]
Does the removal of imx8mq_vpu_variant leave behind unreferenced variables
and functions?

The imx8mq_vpu_codec_ops array, imx8m_vpu_g1_reset(), and imx8m_soft_reset()
are no longer used anywhere. Additionally, the ctrl_base member in
struct hantro_dev is never assigned anymore, and the extern declaration of
imx8mq_vpu_variant in hantro_hw.h still exists.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923090307.49959-1-benjamin.gaignard@collabora.com?part=1