diff mbox

[3/4] drm/omap: fix DMM driver (un)registration

Message ID 1396442280-6189-3-git-send-email-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomi Valkeinen April 2, 2014, 12:37 p.m. UTC
At the moment the DMM driver is never unregistered, even if it's
registered in the omapdrm module's init function. This means we'll get
errors when reloading the omapdrm module.

Fix this by unregistering the DMM driver properly, and also change the
module init to fail if DMM driver cannot be registered, simplifying the
unregister path as we don't need to keep the state whether we registered
the DMM driver or not.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

Comments

Rob Clark April 2, 2014, 2:14 p.m. UTC | #1
On Wed, Apr 2, 2014 at 8:37 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> At the moment the DMM driver is never unregistered, even if it's
> registered in the omapdrm module's init function. This means we'll get
> errors when reloading the omapdrm module.
>
> Fix this by unregistering the DMM driver properly, and also change the
> module init to fail if DMM driver cannot be registered, simplifying the
> unregister path as we don't need to keep the state whether we registered
> the DMM driver or not.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_drv.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
> index df3e66416a30..f16a07d1668d 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
> @@ -727,18 +727,33 @@ static struct platform_driver pdev = {
>
>  static int __init omap_drm_init(void)
>  {
> +       int r;
> +
>         DBG("init");
> -       if (platform_driver_register(&omap_dmm_driver)) {
> -               /* we can continue on without DMM.. so not fatal */
> -               dev_err(NULL, "DMM registration failed\n");
> +
> +       r = platform_driver_register(&omap_dmm_driver);

the one thing I wonder slightly about, this is making omap_dmm_driver
register fail fatal, whereas it wasn't before..

That said, I don't remember in which case the dmm driver registration
would fail.  I think registering the driver should succeed even (for
example) on omap3 without dmm/tiler device.  But I guess you've
probably tested on o3 just to make sure?  Assuming you have:

Reviewed-by: Rob Clark <robdclark@gmail.com>

> +       if (r) {
> +               pr_err("DMM driver registration failed\n");
> +               return r;
>         }
> -       return platform_driver_register(&pdev);
> +
> +       r = platform_driver_register(&pdev);
> +       if (r) {
> +               pr_err("omapdrm driver registration failed\n");
> +               platform_driver_unregister(&omap_dmm_driver);
> +               return r;
> +       }
> +
> +       return 0;
>  }
>
>  static void __exit omap_drm_fini(void)
>  {
>         DBG("fini");
> +
>         platform_driver_unregister(&pdev);
> +
> +       platform_driver_unregister(&omap_dmm_driver);
>  }
>
>  /* need late_initcall() so we load after dss_driver's are loaded */
> --
> 1.8.3.2
>
Tomi Valkeinen April 2, 2014, 2:18 p.m. UTC | #2
On 02/04/14 17:14, Rob Clark wrote:
> On Wed, Apr 2, 2014 at 8:37 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> At the moment the DMM driver is never unregistered, even if it's
>> registered in the omapdrm module's init function. This means we'll get
>> errors when reloading the omapdrm module.
>>
>> Fix this by unregistering the DMM driver properly, and also change the
>> module init to fail if DMM driver cannot be registered, simplifying the
>> unregister path as we don't need to keep the state whether we registered
>> the DMM driver or not.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/omap_drv.c | 23 +++++++++++++++++++----
>>  1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
>> index df3e66416a30..f16a07d1668d 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_drv.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c
>> @@ -727,18 +727,33 @@ static struct platform_driver pdev = {
>>
>>  static int __init omap_drm_init(void)
>>  {
>> +       int r;
>> +
>>         DBG("init");
>> -       if (platform_driver_register(&omap_dmm_driver)) {
>> -               /* we can continue on without DMM.. so not fatal */
>> -               dev_err(NULL, "DMM registration failed\n");
>> +
>> +       r = platform_driver_register(&omap_dmm_driver);
> 
> the one thing I wonder slightly about, this is making omap_dmm_driver
> register fail fatal, whereas it wasn't before..
> 
> That said, I don't remember in which case the dmm driver registration
> would fail.  I think registering the driver should succeed even (for
> example) on omap3 without dmm/tiler device.  But I guess you've
> probably tested on o3 just to make sure?  Assuming you have:

Yes. I think the driver registration could fail more or less only if
we're out of memory, or the driver is already register, or some other
similar situation.

 Tomi
diff mbox

Patch

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index df3e66416a30..f16a07d1668d 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -727,18 +727,33 @@  static struct platform_driver pdev = {
 
 static int __init omap_drm_init(void)
 {
+	int r;
+
 	DBG("init");
-	if (platform_driver_register(&omap_dmm_driver)) {
-		/* we can continue on without DMM.. so not fatal */
-		dev_err(NULL, "DMM registration failed\n");
+
+	r = platform_driver_register(&omap_dmm_driver);
+	if (r) {
+		pr_err("DMM driver registration failed\n");
+		return r;
 	}
-	return platform_driver_register(&pdev);
+
+	r = platform_driver_register(&pdev);
+	if (r) {
+		pr_err("omapdrm driver registration failed\n");
+		platform_driver_unregister(&omap_dmm_driver);
+		return r;
+	}
+
+	return 0;
 }
 
 static void __exit omap_drm_fini(void)
 {
 	DBG("fini");
+
 	platform_driver_unregister(&pdev);
+
+	platform_driver_unregister(&omap_dmm_driver);
 }
 
 /* need late_initcall() so we load after dss_driver's are loaded */