Re: [PATCH 2/3] drm/panfrost: Enable the GPU timestamp counter on MT8188

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/panfrost: missing error propagation for MFGCFG regmap prevents probe deferral
--

commit 93148a7d7b8819bdd74ff76c5e5b0fcc15b88cbf
Author: Michał Kopeć <michal@nozomi.space>

drm/panfrost: Enable the GPU timestamp counter on MT8188

This commit adds a vendor quirk to enable the GPU timestamp counter on
MediaTek MT8188 SoCs. It looks up the MFGCFG block regmap from the device
tree and sets the required enable bits on power-up to ensure timestamps
are correctly reported.

> diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> index 29ac0dce5ff89..971abc9cc222c 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
[ ... ]
> +static void panfrost_gpu_get_mtk_mfgcfg(struct panfrost_device *pfdev)
> +{
> +	struct device_node *np;
> +	struct regmap *regmap;
> +
> +	np = of_parse_phandle(pfdev->base.dev->of_node, "mediatek,mfgcfg", 0);
> +	if (!np)
> +		return;
> +
> +	regmap = device_node_to_regmap(np);
> +	of_node_put(np);
> +	if (IS_ERR(regmap)) {
> +		dev_warn(pfdev->base.dev, "failed to get MFGCFG: %pe\n", regmap);
> +		return;

[Severity: Medium]
Should this function return the error code from device_node_to_regmap()
instead of returning void?

If device_node_to_regmap() returns an error pointer (for example, due to probe
deferral or memory allocation failure), the error is swallowed and the probe
path continues without the regmap. This can cause the GPU timestamp counter to
break silently, leaving userspace queries reading zero.

> +	}
> +
> +	pfdev->mtk_mfgcfg = regmap;
> +}
[ ... ]
> @@ -519,6 +556,8 @@ int panfrost_gpu_init(struct panfrost_device *pfdev)
>  {
>  	int err;
>  
> +	panfrost_gpu_get_mtk_mfgcfg(pfdev);
> +

[Severity: Medium]
Since panfrost_gpu_get_mtk_mfgcfg() doesn't return an error, panfrost_gpu_init()
proceeds despite the failure to acquire the MFGCFG regmap.

Would it be better to propagate the error here so that the driver can properly
defer probe or fail robustly when the necessary resources aren't available?

>  	err = panfrost_gpu_soft_reset(pfdev);
>  	if (err)
>  		return err;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003200132.866240-1-michal@nozomi.space?part=2