Hi Philippe,
On Thu, 2017-07-20 at 14:05 +0200, Philippe CORNU wrote:
Use devm_reset_control_get_exclusive to avoid resource leakage (based on patch "Convert drivers to explicit reset API" from Philipp Zabel).
Also use platform_get_resource, which is more usual and consistent with platform_get_irq called later.
Signed-off-by: Fabien Dessenne fabien.dessenne@st.com Signed-off-by: Philippe CORNU philippe.cornu@st.com Reviewed-by: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Philipp Zabel p.zabel@pengutronix.de
Looking at the usage below, this driver only seems to care about the reset deassertion before register use, so this could use the shared API. Further, it seems that this reset is optional.
drivers/gpu/drm/stm/ltdc.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 92e58ba..d826045 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -874,7 +874,7 @@ int ltdc_load(struct drm_device *ddev) struct drm_panel *panel; struct drm_crtc *crtc; struct reset_control *rstc;
- struct resource res;
struct resource *res; int irq, ret, i;
DRM_DEBUG_DRIVER("\n");
@@ -883,7 +883,7 @@ int ltdc_load(struct drm_device *ddev) if (ret) return ret;
- rstc = of_reset_control_get(np, NULL);
- rstc = devm_reset_control_get_exclusive(dev, NULL);
I would suggest to change this to
- rstc = of_reset_control_get(np, NULL); + rstc = devm_reset_control_get_optional_shared(dev, NULL); + if (IS_ERR(rstc)) + return PTR_ERR(rstc);
mutex_init(&ldev->err_lock);
@@ -898,13 +898,14 @@ int ltdc_load(struct drm_device *ddev) return -ENODEV; }
- if (of_address_to_resource(np, 0, &res)) {
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) { DRM_ERROR("Unable to get resource\n"); ret = -ENODEV; goto err; }
- ldev->regs = devm_ioremap_resource(dev, &res);
- ldev->regs = devm_ioremap_resource(dev, res); if (IS_ERR(ldev->regs)) { DRM_ERROR("Unable to get ltdc registers\n"); ret = PTR_ERR(ldev->regs);
then below you can change:
- if (!IS_ERR(rstc)) - reset_control_deassert(rstc); + reset_control_deassert(rstc);
regards Philipp