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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] media: verisilicon: ABI breakage by removing deprecated DT binding
- [Low] media: verisilicon: orphaned code and macros left after variant removal
--

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

media: verisilicon: Remove deprecated variant

The "nxp,imx8mq-vpu" compatible string has been deprecated since commit
f3116f2e52e6 split the G1 and G2 nodes. This patch removes the driver
reference to this old binding 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..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: High]
Does removing this binding break backward compatibility with older Device Tree
Blobs (DTBs)? Device trees typically act as a stable ABI in Linux. Removing
this entry prevents the driver from binding to older DTBs that have not split
the G1 and G2 nodes, resulting in a loss of hardware video acceleration on
those systems. Even if deprecated, should the binding remain supported to ensure
older firmware continues to boot successfully?

>  	{ .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..b091d807ae4e2 100644
> --- a/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> +++ b/drivers/media/platform/verisilicon/imx8m_vpu_hw.c
> @@ -25,55 +25,6 @@
>  #define CTRL_G1_PP_FUSE		0x0c
>  #define CTRL_G2_DEC_FUSE	0x10
>  
> -static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
> -{

[Severity: Low]
With the removal of imx8mq_runtime_resume() and imx8m_soft_reset() in this
patch, several hardware control macros are left completely unused.

Could these unused macros also be removed from the top of the file?
- CTRL_SOFT_RESET, RESET_G1, RESET_G2
- CTRL_CLOCK_ENABLE, CLOCK_G1, CLOCK_G2
- CTRL_G1_DEC_FUSE, CTRL_G1_PP_FUSE, CTRL_G2_DEC_FUSE

Additionally, since imx8mq_vpu_variant was the only variant using
the runtime_resume function pointer in struct hantro_variant, should
runtime_resume be removed from the struct entirely in hantro.h?
hantro_runtime_resume() in hantro_drv.c is now effectively a no-op for all
platforms.

Finally, could the dangling extern const struct hantro_variant
imx8mq_vpu_variant declaration in hantro_hw.h be dropped too?

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