Re: [PATCH] media: hantro: disable runtime PM before hardware teardown

Benjamin Gaignard <[email protected]>
Newsgroups org.infradead.lists.linux-rockchip,dev.linux.lists.linux-kernel-mentees,org.kernel.vger.linux-kernel,org.kernel.vger.linux-media
Message-ID <[email protected]>
Le 22/07/2026 à 18:08, Tharit Tangkijwanichakul a écrit :
> Removing the Hantro driver may trigger an SError on RK3588:
> 	SError Interrupt on CPU6, code 0x00000000be000011 -- SError
> 	...
> 	do_serror+0x5c/0x74
> 	el1h_64_error_handler+0x34/0x50
> 	el1h_64_error+0x6c/0x70
> 	regmap_mmio_read32le+0x10/0x20 (P)
> 	_regmap_bus_reg_read+0x6c/0xb4
> 	_regmap_read+0x60/0xdc
> 	regmap_read+0x4c/0x80
> 	rockchip_pd_power+0xe4/0x600
> 	rockchip_pd_power_off+0x1c/0x60
> 	_genpd_power_off+0x98/0x198
> 	genpd_power_off.part.0+0x178/0x288
> 	genpd_runtime_suspend+0x210/0x300
> 	__rpm_callback+0x48/0x1e0
> 	rpm_callback+0x74/0x80
> 	rpm_suspend+0x10c/0x580
> 	rpm_idle+0x134/0x1b8
> 	update_autosuspend+0x30/0xc4
> 	__pm_runtime_use_autosuspend+0x48/0x64
> 	hantro_remove+0x8c/0xa8 [hantro_vpu]
> 	...
>
> hantro_remove() currently asserts the device resets before disabling
> runtime PM.
>
> Calling pm_runtime_dont_use_autosuspend() causes an idle device to be
> runtime-suspended immediately. At that point the Hantro hardware has
> already been torn down, while the generic power-domain genpd code
> still performs the power-off sequence. genpd regmap_mmio_read causes panic
> subsequently because the reset lines were asserted.
>
> Disable runtime PM before asserting the reset lines so that no runtime-PM
> transition can race with hardware teardown.
>
> Apply the same ordering to the probe error paths as well.
>
> Fixes: ea71631b7129 ("media: hantro: add support for reset lines")
> Signed-off-by: Tharit Tangkijwanichakul <[email protected]>

Reviewed-by: Benjamin Gaignard <[email protected]>

> ---
>   drivers/media/platform/verisilicon/hantro_drv.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index 2e81877f640f..bbeec5ae509a 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -1212,7 +1212,7 @@ static int hantro_probe(struct platform_device *pdev)
>   	ret = clk_bulk_prepare(vpu->variant->num_clocks, vpu->clocks);
>   	if (ret) {
>   		dev_err(&pdev->dev, "Failed to prepare clocks\n");
> -		goto err_rst_assert;
> +		goto err_pm_disable_assert_reset;
>   	}
>   
>   	ret = v4l2_device_register(&pdev->dev, &vpu->v4l2_dev);
> @@ -1266,8 +1266,11 @@ static int hantro_probe(struct platform_device *pdev)
>   	v4l2_device_unregister(&vpu->v4l2_dev);
>   err_clk_unprepare:
>   	clk_bulk_unprepare(vpu->variant->num_clocks, vpu->clocks);
> -err_rst_assert:
> +err_pm_disable_assert_reset:
> +	pm_runtime_dont_use_autosuspend(vpu->dev);
> +	pm_runtime_disable(vpu->dev);
>   	reset_control_assert(vpu->resets);
> +	return ret;
>   err_pm_disable:
>   	pm_runtime_dont_use_autosuspend(vpu->dev);
>   	pm_runtime_disable(vpu->dev);
> @@ -1287,9 +1290,9 @@ static void hantro_remove(struct platform_device *pdev)
>   	v4l2_m2m_put(vpu->m2m_dev);
>   	v4l2_device_unregister(&vpu->v4l2_dev);
>   	clk_bulk_unprepare(vpu->variant->num_clocks, vpu->clocks);
> -	reset_control_assert(vpu->resets);
>   	pm_runtime_dont_use_autosuspend(vpu->dev);
>   	pm_runtime_disable(vpu->dev);
> +	reset_control_assert(vpu->resets);
>   }
>   
>   #ifdef CONFIG_PM
>
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482

_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.