Patch 1 and 3 are fixes.
Patch 2 is just a kind of clean-up noticed while patching this driver.
If patch 2 is considered as useless, patch 3 can be applied on top of
patch 1, it does nor depend on the 2nd patch.
Christophe JAILLET (3):
drm/meson: Fix an un-handled error path in 'meson_drv_bind_master()'
drm/meson: Use drm_dev_put() instead of drm_dev_unref()
drm/meson: Fix some error handling paths in 'meson_drv_bind_master()'
drivers/gpu/drm/meson/meson_drv.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)
--
2.14.1
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
According to 'drivers/gpu/drm/drm_drv.c', 'drm_dev_unref()' is just a
compatibility alias for 'drm_dev_put()'. So use the latter instead.
Signed-off-by: Christophe JAILLET <[email protected]>
---
drivers/gpu/drm/meson/meson_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 02b0886debc0..6ee3dd7fa450 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -284,7 +284,7 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
return 0;
free_drm:
- drm_dev_unref(drm);
+ drm_dev_put(drm);
return ret;
}
@@ -303,7 +303,7 @@ static void meson_drv_unbind(struct device *dev)
drm_kms_helper_poll_fini(drm);
drm_fbdev_cma_fini(priv->fbdev);
drm_mode_config_cleanup(drm);
- drm_dev_unref(drm);
+ drm_dev_put(drm);
}
--
2.14.1
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
If one of these functions fail, we whould free 'drm', as alreadry done in
the other error handling paths, below and above.
Fixes: bbbe775ec5b5 ("drm: Add support for Amlogic Meson Graphic Controller")
Signed-off-by: Christophe JAILLET <[email protected]>
---
drivers/gpu/drm/meson/meson_drv.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 6ee3dd7fa450..3baceb744beb 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -189,35 +189,43 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vpu");
regs = devm_ioremap_resource(dev, res);
- if (IS_ERR(regs))
- return PTR_ERR(regs);
+ if (IS_ERR(regs)) {
+ ret = PTR_ERR(regs);
+ goto free_drm;
+ }
priv->io_base = regs;
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hhi");
/* Simply ioremap since it may be a shared register zone */
regs = devm_ioremap(dev, res->start, resource_size(res));
- if (!regs)
- return -EADDRNOTAVAIL;
+ if (!regs) {
+ ret = -EADDRNOTAVAIL;
+ goto free_drm;
+ }
priv->hhi = devm_regmap_init_mmio(dev, regs,
&meson_regmap_config);
if (IS_ERR(priv->hhi)) {
dev_err(&pdev->dev, "Couldn't create the HHI regmap\n");
- return PTR_ERR(priv->hhi);
+ ret = PTR_ERR(priv->hhi);
+ goto free_drm;
}
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc");
/* Simply ioremap since it may be a shared register zone */
regs = devm_ioremap(dev, res->start, resource_size(res));
- if (!regs)
- return -EADDRNOTAVAIL;
+ if (!regs) {
+ ret = -EADDRNOTAVAIL;
+ goto free_drm;
+ }
priv->dmc = devm_regmap_init_mmio(dev, regs,
&meson_regmap_config);
if (IS_ERR(priv->dmc)) {
dev_err(&pdev->dev, "Couldn't create the DMC regmap\n");
- return PTR_ERR(priv->dmc);
+ ret = PTR_ERR(priv->dmc);
+ goto free_drm;
}
priv->vsync_irq = platform_get_irq(pdev, 0);
--
2.14.1
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
'drm_vblank_init()' can fail. So handle this (unlikely) error.
Fixes: bbbe775ec5b5 ("drm: Add support for Amlogic Meson Graphic Controller")
Signed-off-by: Christophe JAILLET <[email protected]>
---
drivers/gpu/drm/meson/meson_drv.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index f9ad0e960263..02b0886debc0 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -222,7 +222,10 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
priv->vsync_irq = platform_get_irq(pdev, 0);
- drm_vblank_init(drm, 1);
+ ret = drm_vblank_init(drm, 1);
+ if (ret)
+ goto free_drm;
+
drm_mode_config_init(drm);
drm->mode_config.max_width = 3840;
drm->mode_config.max_height = 2160;
--
2.14.1
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
On 12/03/2018 21:15, Christophe JAILLET wrote:
> According to 'drivers/gpu/drm/drm_drv.c', 'drm_dev_unref()' is just a
> compatibility alias for 'drm_dev_put()'. So use the latter instead.
>
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> drivers/gpu/drm/meson/meson_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 02b0886debc0..6ee3dd7fa450 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -284,7 +284,7 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
> return 0;
>
> free_drm:
> - drm_dev_unref(drm);
> + drm_dev_put(drm);
>
> return ret;
> }
> @@ -303,7 +303,7 @@ static void meson_drv_unbind(struct device *dev)
> drm_kms_helper_poll_fini(drm);
> drm_fbdev_cma_fini(priv->fbdev);
> drm_mode_config_cleanup(drm);
> - drm_dev_unref(drm);
> + drm_dev_put(drm);
>
> }
>
>
It's not useless, there is a clear statement :
* This is a compatibility alias for drm_dev_put() and should not be used by new
* code.
Acked-by: Neil Armstrong <[email protected]>
Thanks,
Neil
On 12/03/2018 21:15, Christophe JAILLET wrote:
> If one of these functions fail, we whould free 'drm', as alreadry done in
> the other error handling paths, below and above.
>
> Fixes: bbbe775ec5b5 ("drm: Add support for Amlogic Meson Graphic Controller")
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> drivers/gpu/drm/meson/meson_drv.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 6ee3dd7fa450..3baceb744beb 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -189,35 +189,43 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vpu");
> regs = devm_ioremap_resource(dev, res);
> - if (IS_ERR(regs))
> - return PTR_ERR(regs);
> + if (IS_ERR(regs)) {
> + ret = PTR_ERR(regs);
> + goto free_drm;
> + }
>
> priv->io_base = regs;
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hhi");
> /* Simply ioremap since it may be a shared register zone */
> regs = devm_ioremap(dev, res->start, resource_size(res));
> - if (!regs)
> - return -EADDRNOTAVAIL;
> + if (!regs) {
> + ret = -EADDRNOTAVAIL;
> + goto free_drm;
> + }
>
> priv->hhi = devm_regmap_init_mmio(dev, regs,
> &meson_regmap_config);
> if (IS_ERR(priv->hhi)) {
> dev_err(&pdev->dev, "Couldn't create the HHI regmap\n");
> - return PTR_ERR(priv->hhi);
> + ret = PTR_ERR(priv->hhi);
> + goto free_drm;
> }
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dmc");
> /* Simply ioremap since it may be a shared register zone */
> regs = devm_ioremap(dev, res->start, resource_size(res));
> - if (!regs)
> - return -EADDRNOTAVAIL;
> + if (!regs) {
> + ret = -EADDRNOTAVAIL;
> + goto free_drm;
> + }
>
> priv->dmc = devm_regmap_init_mmio(dev, regs,
> &meson_regmap_config);
> if (IS_ERR(priv->dmc)) {
> dev_err(&pdev->dev, "Couldn't create the DMC regmap\n");
> - return PTR_ERR(priv->dmc);
> + ret = PTR_ERR(priv->dmc);
> + goto free_drm;
> }
>
> priv->vsync_irq = platform_get_irq(pdev, 0);
>
Acked-by: Neil Armstrong <[email protected]>
Thanks,
Neil
On 12/03/2018 21:15, Christophe JAILLET wrote:
> 'drm_vblank_init()' can fail. So handle this (unlikely) error.
>
> Fixes: bbbe775ec5b5 ("drm: Add support for Amlogic Meson Graphic Controller")
> Signed-off-by: Christophe JAILLET <[email protected]>
> ---
> drivers/gpu/drm/meson/meson_drv.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index f9ad0e960263..02b0886debc0 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -222,7 +222,10 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>
> priv->vsync_irq = platform_get_irq(pdev, 0);
>
> - drm_vblank_init(drm, 1);
> + ret = drm_vblank_init(drm, 1);
> + if (ret)
> + goto free_drm;
> +
> drm_mode_config_init(drm);
> drm->mode_config.max_width = 3840;
> drm->mode_config.max_height = 2160;
>
Acked-by: Neil Armstrong <[email protected]>
Thanks,
Neil
On 12/03/2018 21:15, Christophe JAILLET wrote:
> Patch 1 and 3 are fixes.
> Patch 2 is just a kind of clean-up noticed while patching this driver.
>
> If patch 2 is considered as useless, patch 3 can be applied on top of
> patch 1, it does nor depend on the 2nd patch.
>
> Christophe JAILLET (3):
> drm/meson: Fix an un-handled error path in 'meson_drv_bind_master()'
> drm/meson: Use drm_dev_put() instead of drm_dev_unref()
> drm/meson: Fix some error handling paths in 'meson_drv_bind_master()'
>
> drivers/gpu/drm/meson/meson_drv.c | 33 ++++++++++++++++++++++-----------
> 1 file changed, 22 insertions(+), 11 deletions(-)
>
Hi,
Thanks for the fixes, since they are not critical, I pushed them on drm-misc-next, they will be backported onto stable tree by the stable team later on using the Fixes tag.
Neil