linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] simplefb: Disable and release clocks and regulators in destroy callback
@ 2016-09-07  9:09 Chen-Yu Tsai
  2016-09-07  9:31 ` Hans de Goede
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2016-09-07  9:09 UTC (permalink / raw)
  To: Hans de Goede, Jean-Christophe Plagniol-Villard, Tomi Valkeinen
  Cc: Chen-Yu Tsai, linux-fbdev, linux-arm-kernel, linux-kernel

simplefb gets unregister when a proper framebuffer driver comes in and
kicks it out. However the claimed clocks and regulators stay enabled
as they are only released in the platform device remove function, which
in theory would never get called.

Move the clock/regulator cleanup into the framebuffer destroy callback,
which gets called as part of the framebuffer unregister process.

Note this introduces asymmetry in how the resources are claimed and
released.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/video/fbdev/simplefb.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index e9cf19977285..3620d10a0d00 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -74,8 +74,14 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
 	return 0;
 }
 
+struct simplefb_par;
+static void simplefb_clocks_destroy(struct simplefb_par *par);
+static void simplefb_regulators_destroy(struct simplefb_par *par);
+
 static void simplefb_destroy(struct fb_info *info)
 {
+	simplefb_regulators_destroy(info->par);
+	simplefb_clocks_destroy(info->par);
 	if (info->screen_base)
 		iounmap(info->screen_base);
 }
@@ -487,11 +493,8 @@ error_fb_release:
 static int simplefb_remove(struct platform_device *pdev)
 {
 	struct fb_info *info = platform_get_drvdata(pdev);
-	struct simplefb_par *par = info->par;
 
 	unregister_framebuffer(info);
-	simplefb_regulators_destroy(par);
-	simplefb_clocks_destroy(par);
 	framebuffer_release(info);
 
 	return 0;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] simplefb: Disable and release clocks and regulators in destroy callback
  2016-09-07  9:09 [PATCH] simplefb: Disable and release clocks and regulators in destroy callback Chen-Yu Tsai
@ 2016-09-07  9:31 ` Hans de Goede
  2016-09-07 11:12 ` Geert Uytterhoeven
  2016-09-27  8:22 ` Tomi Valkeinen
  2 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2016-09-07  9:31 UTC (permalink / raw)
  To: Chen-Yu Tsai, Jean-Christophe Plagniol-Villard, Tomi Valkeinen
  Cc: linux-fbdev, linux-arm-kernel, linux-kernel

Hi,

On 07-09-16 11:09, Chen-Yu Tsai wrote:
> simplefb gets unregister when a proper framebuffer driver comes in and
> kicks it out. However the claimed clocks and regulators stay enabled
> as they are only released in the platform device remove function, which
> in theory would never get called.
>
> Move the clock/regulator cleanup into the framebuffer destroy callback,
> which gets called as part of the framebuffer unregister process.
>
> Note this introduces asymmetry in how the resources are claimed and
> released.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Good catch, patch LGTM:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>  drivers/video/fbdev/simplefb.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index e9cf19977285..3620d10a0d00 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -74,8 +74,14 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
>  	return 0;
>  }
>
> +struct simplefb_par;
> +static void simplefb_clocks_destroy(struct simplefb_par *par);
> +static void simplefb_regulators_destroy(struct simplefb_par *par);
> +
>  static void simplefb_destroy(struct fb_info *info)
>  {
> +	simplefb_regulators_destroy(info->par);
> +	simplefb_clocks_destroy(info->par);
>  	if (info->screen_base)
>  		iounmap(info->screen_base);
>  }
> @@ -487,11 +493,8 @@ error_fb_release:
>  static int simplefb_remove(struct platform_device *pdev)
>  {
>  	struct fb_info *info = platform_get_drvdata(pdev);
> -	struct simplefb_par *par = info->par;
>
>  	unregister_framebuffer(info);
> -	simplefb_regulators_destroy(par);
> -	simplefb_clocks_destroy(par);
>  	framebuffer_release(info);
>
>  	return 0;
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] simplefb: Disable and release clocks and regulators in destroy callback
  2016-09-07  9:09 [PATCH] simplefb: Disable and release clocks and regulators in destroy callback Chen-Yu Tsai
  2016-09-07  9:31 ` Hans de Goede
@ 2016-09-07 11:12 ` Geert Uytterhoeven
  2016-09-07 11:29   ` Hans de Goede
  2016-09-27  8:22 ` Tomi Valkeinen
  2 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2016-09-07 11:12 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Hans de Goede, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	Linux Fbdev development list, linux-arm-kernel, linux-kernel

On Wed, Sep 7, 2016 at 11:09 AM, Chen-Yu Tsai <wens@csie.org> wrote:
> simplefb gets unregister when a proper framebuffer driver comes in and
> kicks it out. However the claimed clocks and regulators stay enabled
> as they are only released in the platform device remove function, which
> in theory would never get called.
>
> Move the clock/regulator cleanup into the framebuffer destroy callback,
> which gets called as part of the framebuffer unregister process.

Is this called before or after the new proper framebuffer driver kicks in?
If before, it may cause glitches.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] simplefb: Disable and release clocks and regulators in destroy callback
  2016-09-07 11:12 ` Geert Uytterhoeven
@ 2016-09-07 11:29   ` Hans de Goede
  2016-09-07 11:31     ` Geert Uytterhoeven
  0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2016-09-07 11:29 UTC (permalink / raw)
  To: Geert Uytterhoeven, Chen-Yu Tsai
  Cc: Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	Linux Fbdev development list, linux-arm-kernel, linux-kernel

Hi,

On 07-09-16 13:12, Geert Uytterhoeven wrote:
> On Wed, Sep 7, 2016 at 11:09 AM, Chen-Yu Tsai <wens@csie.org> wrote:
>> simplefb gets unregister when a proper framebuffer driver comes in and
>> kicks it out. However the claimed clocks and regulators stay enabled
>> as they are only released in the platform device remove function, which
>> in theory would never get called.
>>
>> Move the clock/regulator cleanup into the framebuffer destroy callback,
>> which gets called as part of the framebuffer unregister process.
>
> Is this called before or after the new proper framebuffer driver kicks in?
> If before, it may cause glitches.

It is called by the new proper framebuffer driver's probe method,
so it can make sure that it has already claimed / enabled the
clocks/regulators before it calls remove_conlicting_framebuffers,
avoiding the glitch.

Regards,

Hans

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] simplefb: Disable and release clocks and regulators in destroy callback
  2016-09-07 11:29   ` Hans de Goede
@ 2016-09-07 11:31     ` Geert Uytterhoeven
  0 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2016-09-07 11:31 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Chen-Yu Tsai, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	Linux Fbdev development list, linux-arm-kernel, linux-kernel

Hi Hans,

On Wed, Sep 7, 2016 at 1:29 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> On 07-09-16 13:12, Geert Uytterhoeven wrote:
>> On Wed, Sep 7, 2016 at 11:09 AM, Chen-Yu Tsai <wens@csie.org> wrote:
>>> simplefb gets unregister when a proper framebuffer driver comes in and
>>> kicks it out. However the claimed clocks and regulators stay enabled
>>> as they are only released in the platform device remove function, which
>>> in theory would never get called.
>>>
>>> Move the clock/regulator cleanup into the framebuffer destroy callback,
>>> which gets called as part of the framebuffer unregister process.
>>
>>
>> Is this called before or after the new proper framebuffer driver kicks in?
>> If before, it may cause glitches.
>
>
> It is called by the new proper framebuffer driver's probe method,
> so it can make sure that it has already claimed / enabled the
> clocks/regulators before it calls remove_conlicting_framebuffers,
> avoiding the glitch.

OK, thx!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] simplefb: Disable and release clocks and regulators in destroy callback
  2016-09-07  9:09 [PATCH] simplefb: Disable and release clocks and regulators in destroy callback Chen-Yu Tsai
  2016-09-07  9:31 ` Hans de Goede
  2016-09-07 11:12 ` Geert Uytterhoeven
@ 2016-09-27  8:22 ` Tomi Valkeinen
  2 siblings, 0 replies; 6+ messages in thread
From: Tomi Valkeinen @ 2016-09-27  8:22 UTC (permalink / raw)
  To: Chen-Yu Tsai, Hans de Goede, Jean-Christophe Plagniol-Villard
  Cc: linux-fbdev, linux-arm-kernel, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 730 bytes --]

On 07/09/16 12:09, Chen-Yu Tsai wrote:
> simplefb gets unregister when a proper framebuffer driver comes in and
> kicks it out. However the claimed clocks and regulators stay enabled
> as they are only released in the platform device remove function, which
> in theory would never get called.
> 
> Move the clock/regulator cleanup into the framebuffer destroy callback,
> which gets called as part of the framebuffer unregister process.
> 
> Note this introduces asymmetry in how the resources are claimed and
> released.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  drivers/video/fbdev/simplefb.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Thanks, queued for 4.9.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-09-27  8:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-07  9:09 [PATCH] simplefb: Disable and release clocks and regulators in destroy callback Chen-Yu Tsai
2016-09-07  9:31 ` Hans de Goede
2016-09-07 11:12 ` Geert Uytterhoeven
2016-09-07 11:29   ` Hans de Goede
2016-09-07 11:31     ` Geert Uytterhoeven
2016-09-27  8:22 ` Tomi Valkeinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).