Re: [PATCH v5 1/3] media: verisilicon: Remove deprecated nxp,imx8mp-vpu variant

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] media: verisilicon: ABI breakage via removal of nxp,imx8mq-vpu
- [Low] media: verisilicon: incomplete cleanup of runtime_resume
--

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

media: verisilicon: Remove deprecated nxp,imx8mp-vpu variant

This commit removes the deprecated monolithic nxp,imx8mq-vpu compatible
string and its associated variant data from the hantro driver. This cleans
up the code since the split G1 and G2 nodes have been preferred since
v5.17-rc1.

> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index ad71c0402ef3b..12c804dcc6694 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: Medium]
Since Device Tree bindings are considered a stable ABI, could removing this
compatible string cause a regression?

Old DTBs with the monolithic node might fail to probe hardware video decoding
on new kernels, which breaks backward compatibility.

>  	{ .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..0d6a71ae5f454 100644
> --- a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> +++ b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
[ ... ]
> @@ -319,30 +223,9 @@ static const struct hantro_irq imx8mq_g2_irqs[] = {
>  	{ "g2", hantro_g2_irq },
>  };
>  
> -static const char * const imx8mq_clk_names[] = { "g1", "g2", "bus" };
> -static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" };
>  static const char * const imx8mq_g1_clk_names[] = { "g1" };
>  static const char * const imx8mq_g2_clk_names[] = { "g2" };
>  
> -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,

[Severity: Low]
With the removal of imx8mq_runtime_resume() and imx8mq_vpu_variant, there
are no remaining users of the runtime_resume callback in struct hantro_variant.

Should the runtime_resume function pointer in struct hantro_variant and its
wrapper hantro_runtime_resume() in hantro_drv.c also be removed to clean up
the resulting dead code?

> -	.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)
> -};

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