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ć <[email protected]> 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/[email protected]?part=2
