Re: [PATCH v2 3/3] media: verisilicon: Remove useless internal fields

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] media: hantro: break backward compatibility by removing DT compatible string
- [Low] media: hantro: dead code and uninitialized pointer ctrl_base left behind
--

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

media: verisilicon: Remove useless internal fields
    
No more variants are using multiple registers regions.
Remove the useless fields and simplify driver code.

> diff --git a/drivers/media/platform/verisilicon/hantro.h b/drivers/media/platform/verisilicon/hantro.h
> index 0353de154a1ec..20c3afb3b8176 100644
> --- a/drivers/media/platform/verisilicon/hantro.h
> +++ b/drivers/media/platform/verisilicon/hantro.h
[ ... ]
> @@ -204,7 +200,7 @@ struct hantro_dev {
>  	struct device *dev;
>  	struct clk_bulk_data *clocks;
>  	struct reset_control *resets;
> -	void __iomem **reg_bases;
> +	void __iomem *reg_base;
>  	void __iomem *enc_base;
>  	void __iomem *dec_base;
>  	void __iomem *ctrl_base;

[Severity: Low]
Does this struct still need the ctrl_base pointer?

The initialization of vpu->ctrl_base was removed when the multi-base mapping
was dropped, leaving this field uninitialized.

Additionally, imx8mq_vpu_codec_ops in imx8m_vpu_hw.c and its associated reset
function imx8m_soft_reset() still unconditionally dereference this pointer.
Since imx8mq_vpu_variant was removed, they appear to be dead code.

Could these orphaned structures and the ctrl_base field be removed?

> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index 70de47ddb023b..f418b04279856 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -1080,7 +1080,6 @@ static int hantro_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *match;
>  	struct hantro_dev *vpu;
> -	int num_bases;

[Severity: High]
Does the removal of the nxp,imx8mq-vpu compatible string in this series break
backwards compatibility for older Device Trees?

Because the driver no longer matches that string, it will completely fail to
probe on systems booting with older DTBs, rendering the hardware unusable.

Additionally, since the string is missing from of_hantro_match, the driver
will not probe at all, which makes the existing fallback warning inside
hantro_probe() unreachable dead code:

    if (of_device_is_compatible(pdev->dev.of_node, "nxp,imx8mq-vpu"))
            dev_warn(&pdev->dev, "%s compatible is deprecated\n",
                     match->compatible);

Since kernel policy strictly maintains DT ABI compatibility, should this
deprecated string be restored to the match array so existing systems continue
to boot and the warning mechanism works as intended?

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