From: Sean Paul <seanpaul@xxxxxxxxxxxx>
The gmu driver is initialized and cleaned up with calls from the gpu driver. As
such, the platform device stays valid after a6xx_gmu_remove is called and the
device managed resources are not freed. In the case of gpu probe failures or
unbind, these resources will remain managed.
If the gpu bind is run again (eg: if there's a probe defer somewhere in msm),
these resources will be initialized again for the same device, creating multiple
references. In the case of irqs, this causes failures since the irqs are
not shared (nor should they be).
This patch removes all devm_* calls and manually cleans things up in
gmu_remove.
Changes in v2:
- Add iounmap and free_irq to gmu_probe error paths
Cc: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx>
Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 33 ++++++++++++++++++---------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 7465423e9b71..f7240c9e11fb 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -505,9 +505,9 @@ static void a6xx_gmu_rpmh_init(struct a6xx_gmu *gmu)
err:
if (!IS_ERR_OR_NULL(pdcptr))
- devm_iounmap(gmu->dev, pdcptr);
+ iounmap(pdcptr);
if (!IS_ERR_OR_NULL(seqptr))
- devm_iounmap(gmu->dev, seqptr);
+ iounmap(seqptr);
}
/*
@@ -1197,7 +1197,7 @@ static void __iomem *a6xx_gmu_get_mmio(struct platform_device *pdev,
return ERR_PTR(-EINVAL);
}
- ret = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ ret = ioremap(res->start, resource_size(res));
if (!ret) {
DRM_DEV_ERROR(&pdev->dev, "Unable to map the %s registers\n", name);
return ERR_PTR(-EINVAL);
@@ -1213,10 +1213,10 @@ static int a6xx_gmu_get_irq(struct a6xx_gmu *gmu, struct platform_device *pdev,
irq = platform_get_irq_byname(pdev, name);
- ret = devm_request_irq(&pdev->dev, irq, handler, IRQF_TRIGGER_HIGH,
- name, gmu);
+ ret = request_irq(irq, handler, IRQF_TRIGGER_HIGH, name, gmu);
if (ret) {
- DRM_DEV_ERROR(&pdev->dev, "Unable to get interrupt %s\n", name);
+ DRM_DEV_ERROR(&pdev->dev, "Unable to get interrupt %s %d\n",
+ name, ret);
return ret;
}
@@ -1241,12 +1241,18 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
dev_pm_domain_detach(gmu->gxpd, false);
}
+ iounmap(gmu->mmio);
+ gmu->mmio = NULL;
+
a6xx_gmu_memory_free(gmu, gmu->hfi);
iommu_detach_device(gmu->domain, gmu->dev);
iommu_domain_free(gmu->domain);
+ free_irq(gmu->gmu_irq, gmu);
+ free_irq(gmu->hfi_irq, gmu);
+
gmu->initialized = false;
}
@@ -1281,24 +1287,24 @@ int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
/* Allocate memory for for the HFI queues */
gmu->hfi = a6xx_gmu_memory_alloc(gmu, SZ_16K);
if (IS_ERR(gmu->hfi))
- goto err;
+ goto err_memory;
/* Allocate memory for the GMU debug region */
gmu->debug = a6xx_gmu_memory_alloc(gmu, SZ_16K);
if (IS_ERR(gmu->debug))
- goto err;
+ goto err_memory;
/* Map the GMU registers */
gmu->mmio = a6xx_gmu_get_mmio(pdev, "gmu");
if (IS_ERR(gmu->mmio))
- goto err;
+ goto err_memory;
/* Get the HFI and GMU interrupts */
gmu->hfi_irq = a6xx_gmu_get_irq(gmu, pdev, "hfi", a6xx_hfi_irq);
gmu->gmu_irq = a6xx_gmu_get_irq(gmu, pdev, "gmu", a6xx_gmu_irq);
if (gmu->hfi_irq < 0 || gmu->gmu_irq < 0)
- goto err;
+ goto err_mmio;
/*
* Get a link to the GX power domain to reset the GPU in case of GMU
@@ -1315,7 +1321,12 @@ int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
gmu->initialized = true;
return 0;
-err:
+
+err_mmio:
+ iounmap(gmu->mmio);
+ free_irq(gmu->gmu_irq, gmu);
+ free_irq(gmu->hfi_irq, gmu);
+err_memory:
a6xx_gmu_memory_free(gmu, gmu->hfi);
if (gmu->domain) {
--
Sean Paul, Software Engineer, Google / Chromium OS
