[PATCH] firmware: sysfb: Fix reference count of sysfb parent device

[PATCH] firmware: sysfb: Fix reference count of sysfb parent device

[Thomas Zimmermann]

Retrieving the system framebuffer's parent device in sysfb_init()
increments the parent device's reference count. Hence release the
reference before leaving the init function.

Adding the sysfb platform device acquires and additional reference
for the parent. This keeps the parent device around while the system
framebuffer is in use.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: 9eac534db001 ("firmware/sysfb: Set firmware-framebuffer parent device")
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Sui Jingfeng <suijingfeng@loongson.cn>
Cc: <stable@vger.kernel.org> # v6.9+
---
 drivers/firmware/sysfb.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
index 880ffcb50088..dd274563deeb 100644
--- a/drivers/firmware/sysfb.c
+++ b/drivers/firmware/sysfb.c
@@ -101,8 +101,10 @@ static __init struct device *sysfb_parent_dev(const struct screen_info *si)
 	if (IS_ERR(pdev)) {
 		return ERR_CAST(pdev);
 	} else if (pdev) {
-		if (!sysfb_pci_dev_is_enabled(pdev))
+		if (!sysfb_pci_dev_is_enabled(pdev)) {
+			pci_dev_put(pdev);
 			return ERR_PTR(-ENODEV);
+		}
 		return &pdev->dev;
 	}
 
@@ -137,7 +139,7 @@ static __init int sysfb_init(void)
 	if (compatible) {
 		pd = sysfb_create_simplefb(si, &mode, parent);
 		if (!IS_ERR(pd))
-			goto unlock_mutex;
+			goto put_device;
 	}
 
 	/* if the FB is incompatible, create a legacy framebuffer device */
@@ -155,7 +157,7 @@ static __init int sysfb_init(void)
 	pd = platform_device_alloc(name, 0);
 	if (!pd) {
 		ret = -ENOMEM;
-		goto unlock_mutex;
+		goto put_device;
 	}
 
 	pd->dev.parent = parent;
@@ -170,9 +172,12 @@ static __init int sysfb_init(void)
 	if (ret)
 		goto err;
 
-	goto unlock_mutex;
+
+	goto put_device;
 err:
 	platform_device_put(pd);
+put_device:
+	put_device(parent);
 unlock_mutex:
 	mutex_unlock(&disable_lock);
 	return ret;
-- 
2.45.2

Re: [PATCH] firmware: sysfb: Fix reference count of sysfb parent device

[Javier Martinez Canillas]

Thomas Zimmermann <tzimmermann@suse.de> writes:

Hello Thomas,

> Retrieving the system framebuffer's parent device in sysfb_init()
> increments the parent device's reference count. Hence release the
> reference before leaving the init function.
>
> Adding the sysfb platform device acquires and additional reference
> for the parent. This keeps the parent device around while the system
> framebuffer is in use.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 9eac534db001 ("firmware/sysfb: Set firmware-framebuffer parent device")
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Sui Jingfeng <suijingfeng@loongson.cn>
> Cc: <stable@vger.kernel.org> # v6.9+
> ---

Looks good to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Re: [PATCH] firmware: sysfb: Fix reference count of sysfb parent device

[Marek Olšák]

Hi Thomas,

FYI, this doesn't fix the issue of lightdm not being able to start for me.

Marek


Marek

On Tue, Jun 25, 2024 at 4:18 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Retrieving the system framebuffer's parent device in sysfb_init()
> increments the parent device's reference count. Hence release the
> reference before leaving the init function.
>
> Adding the sysfb platform device acquires and additional reference
> for the parent. This keeps the parent device around while the system
> framebuffer is in use.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Fixes: 9eac534db001 ("firmware/sysfb: Set firmware-framebuffer parent device")
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Sui Jingfeng <suijingfeng@loongson.cn>
> Cc: <stable@vger.kernel.org> # v6.9+
> ---
>  drivers/firmware/sysfb.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
> index 880ffcb50088..dd274563deeb 100644
> --- a/drivers/firmware/sysfb.c
> +++ b/drivers/firmware/sysfb.c
> @@ -101,8 +101,10 @@ static __init struct device *sysfb_parent_dev(const struct screen_info *si)
>         if (IS_ERR(pdev)) {
>                 return ERR_CAST(pdev);
>         } else if (pdev) {
> -               if (!sysfb_pci_dev_is_enabled(pdev))
> +               if (!sysfb_pci_dev_is_enabled(pdev)) {
> +                       pci_dev_put(pdev);
>                         return ERR_PTR(-ENODEV);
> +               }
>                 return &pdev->dev;
>         }
>
> @@ -137,7 +139,7 @@ static __init int sysfb_init(void)
>         if (compatible) {
>                 pd = sysfb_create_simplefb(si, &mode, parent);
>                 if (!IS_ERR(pd))
> -                       goto unlock_mutex;
> +                       goto put_device;
>         }
>
>         /* if the FB is incompatible, create a legacy framebuffer device */
> @@ -155,7 +157,7 @@ static __init int sysfb_init(void)
>         pd = platform_device_alloc(name, 0);
>         if (!pd) {
>                 ret = -ENOMEM;
> -               goto unlock_mutex;
> +               goto put_device;
>         }
>
>         pd->dev.parent = parent;
> @@ -170,9 +172,12 @@ static __init int sysfb_init(void)
>         if (ret)
>                 goto err;
>
> -       goto unlock_mutex;
> +
> +       goto put_device;
>  err:
>         platform_device_put(pd);
> +put_device:
> +       put_device(parent);
>  unlock_mutex:
>         mutex_unlock(&disable_lock);
>         return ret;
> --
> 2.45.2
>

Re: [PATCH] firmware: sysfb: Fix reference count of sysfb parent device

[Thomas Zimmermann]

Hi

Am 28.06.24 um 20:54 schrieb Marek Olšák:
> Hi Thomas,
>
> FYI, this doesn't fix the issue of lightdm not being able to start for me.

Of course, that's expected. It's a different bug.

Best regards
Thomas

>
> Marek
>
>
> Marek
>
> On Tue, Jun 25, 2024 at 4:18 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Retrieving the system framebuffer's parent device in sysfb_init()
>> increments the parent device's reference count. Hence release the
>> reference before leaving the init function.
>>
>> Adding the sysfb platform device acquires and additional reference
>> for the parent. This keeps the parent device around while the system
>> framebuffer is in use.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Fixes: 9eac534db001 ("firmware/sysfb: Set firmware-framebuffer parent device")
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Javier Martinez Canillas <javierm@redhat.com>
>> Cc: Helge Deller <deller@gmx.de>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Dan Carpenter <dan.carpenter@linaro.org>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Sui Jingfeng <suijingfeng@loongson.cn>
>> Cc: <stable@vger.kernel.org> # v6.9+
>> ---
>>   drivers/firmware/sysfb.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
>> index 880ffcb50088..dd274563deeb 100644
>> --- a/drivers/firmware/sysfb.c
>> +++ b/drivers/firmware/sysfb.c
>> @@ -101,8 +101,10 @@ static __init struct device *sysfb_parent_dev(const struct screen_info *si)
>>          if (IS_ERR(pdev)) {
>>                  return ERR_CAST(pdev);
>>          } else if (pdev) {
>> -               if (!sysfb_pci_dev_is_enabled(pdev))
>> +               if (!sysfb_pci_dev_is_enabled(pdev)) {
>> +                       pci_dev_put(pdev);
>>                          return ERR_PTR(-ENODEV);
>> +               }
>>                  return &pdev->dev;
>>          }
>>
>> @@ -137,7 +139,7 @@ static __init int sysfb_init(void)
>>          if (compatible) {
>>                  pd = sysfb_create_simplefb(si, &mode, parent);
>>                  if (!IS_ERR(pd))
>> -                       goto unlock_mutex;
>> +                       goto put_device;
>>          }
>>
>>          /* if the FB is incompatible, create a legacy framebuffer device */
>> @@ -155,7 +157,7 @@ static __init int sysfb_init(void)
>>          pd = platform_device_alloc(name, 0);
>>          if (!pd) {
>>                  ret = -ENOMEM;
>> -               goto unlock_mutex;
>> +               goto put_device;
>>          }
>>
>>          pd->dev.parent = parent;
>> @@ -170,9 +172,12 @@ static __init int sysfb_init(void)
>>          if (ret)
>>                  goto err;
>>
>> -       goto unlock_mutex;
>> +
>> +       goto put_device;
>>   err:
>>          platform_device_put(pd);
>> +put_device:
>> +       put_device(parent);
>>   unlock_mutex:
>>          mutex_unlock(&disable_lock);
>>          return ret;
>> --
>> 2.45.2
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)