public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH] omapfb: Condition mutex acquisition
@ 2009-09-29 14:14 Aguirre Rodriguez, Sergio Alberto
  2009-09-29 14:22 ` Tomi Valkeinen
  0 siblings, 1 reply; 14+ messages in thread
From: Aguirre Rodriguez, Sergio Alberto @ 2009-09-29 14:14 UTC (permalink / raw)
  To: tomi.valkeinen@nokia.com, imre.deak@nokia.com; +Cc: linux-omap@vger.kernel.org

[-- Attachment #1: Type: text/plain, Size: 2405 bytes --]

From: Sergio Aguirre <saaguirre@ti.com>

Acquiring mutex before framebuffer registration doesn't make sense,
As there's no danger of external access to the memory related fields.

NOTE: PLEASE REVIEW! I'M NOT AN EXPERT ON THIS.

Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
---
 drivers/video/omap/omapfb_main.c |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/video/omap/omapfb_main.c b/drivers/video/omap/omapfb_main.c
index 125e605..0d0c8c8 100644
--- a/drivers/video/omap/omapfb_main.c
+++ b/drivers/video/omap/omapfb_main.c
@@ -393,7 +393,7 @@ static void omapfb_sync(struct fb_info *fbi)
  * Set fb_info.fix fields and also updates fbdev.
  * When calling this fb_info.var must be set up already.
  */
-static void set_fb_fix(struct fb_info *fbi)
+static void set_fb_fix(struct fb_info *fbi, int from_init)
 {
 	struct fb_fix_screeninfo *fix = &fbi->fix;
 	struct fb_var_screeninfo *var = &fbi->var;
@@ -403,10 +403,16 @@ static void set_fb_fix(struct fb_info *fbi)
 
 	rg = &plane->fbdev->mem_desc.region[plane->idx];
 	fbi->screen_base	= rg->vaddr;
-	mutex_lock(&fbi->mm_lock);
-	fix->smem_start		= rg->paddr;
-	fix->smem_len		= rg->size;
-	mutex_unlock(&fbi->mm_lock);
+
+	if (!from_init) {
+		mutex_lock(&fbi->mm_lock);
+		fix->smem_start		= rg->paddr;
+		fix->smem_len		= rg->size;
+		mutex_unlock(&fbi->mm_lock);
+	} else {
+		fix->smem_start		= rg->paddr;
+		fix->smem_len		= rg->size;
+	}
 
 	fix->type = FB_TYPE_PACKED_PIXELS;
 	bpp = var->bits_per_pixel;
@@ -704,7 +710,7 @@ static int omapfb_set_par(struct fb_info *fbi)
 	int r = 0;
 
 	omapfb_rqueue_lock(fbdev);
-	set_fb_fix(fbi);
+	set_fb_fix(fbi, 0);
 	r = ctrl_change_mode(fbi);
 	omapfb_rqueue_unlock(fbdev);
 
@@ -904,7 +910,7 @@ static int omapfb_setup_mem(struct fb_info *fbi, struct omapfb_mem_info *mi)
 		if (old_size != size) {
 			if (size) {
 				memcpy(&fbi->var, new_var, sizeof(fbi->var));
-				set_fb_fix(fbi);
+				set_fb_fix(fbi, 0);
 			} else {
 				/*
 				 * Set these explicitly to indicate that the
@@ -1504,7 +1510,7 @@ static int fbinfo_init(struct omapfb_device *fbdev, struct fb_info *info)
 	var->bits_per_pixel = fbdev->panel->bpp;
 
 	set_fb_var(info, var);
-	set_fb_fix(info);
+	set_fb_fix(info, 1);
 
 	r = fb_alloc_cmap(&info->cmap, 16, 0);
 	if (r != 0)
-- 
1.6.0.4

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-omapfb-Condition-mutex-acquisition.patch --]
[-- Type: text/x-patch; name="0001-omapfb-Condition-mutex-acquisition.patch", Size: 2493 bytes --]

From bba2d89ea45ec0359dec6fe21c1756ad3406e347 Mon Sep 17 00:00:00 2001
From: Sergio Aguirre <saaguirre@ti.com>
Date: Tue, 29 Sep 2009 07:44:19 -0500
Subject: [PATCH] omapfb: Condition mutex acquisition

Acquiring mutex before framebuffer registration doesn't make sense,
As there's no danger of external access to the memory related fields.

NOTE: PLEASE REVIEW! I'M NOT AN EXPERT ON THIS.

Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
---
 drivers/video/omap/omapfb_main.c |   22 ++++++++++++++--------
 1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/video/omap/omapfb_main.c b/drivers/video/omap/omapfb_main.c
index 125e605..0d0c8c8 100644
--- a/drivers/video/omap/omapfb_main.c
+++ b/drivers/video/omap/omapfb_main.c
@@ -393,7 +393,7 @@ static void omapfb_sync(struct fb_info *fbi)
  * Set fb_info.fix fields and also updates fbdev.
  * When calling this fb_info.var must be set up already.
  */
-static void set_fb_fix(struct fb_info *fbi)
+static void set_fb_fix(struct fb_info *fbi, int from_init)
 {
 	struct fb_fix_screeninfo *fix = &fbi->fix;
 	struct fb_var_screeninfo *var = &fbi->var;
@@ -403,10 +403,16 @@ static void set_fb_fix(struct fb_info *fbi)
 
 	rg = &plane->fbdev->mem_desc.region[plane->idx];
 	fbi->screen_base	= rg->vaddr;
-	mutex_lock(&fbi->mm_lock);
-	fix->smem_start		= rg->paddr;
-	fix->smem_len		= rg->size;
-	mutex_unlock(&fbi->mm_lock);
+
+	if (!from_init) {
+		mutex_lock(&fbi->mm_lock);
+		fix->smem_start		= rg->paddr;
+		fix->smem_len		= rg->size;
+		mutex_unlock(&fbi->mm_lock);
+	} else {
+		fix->smem_start		= rg->paddr;
+		fix->smem_len		= rg->size;
+	}
 
 	fix->type = FB_TYPE_PACKED_PIXELS;
 	bpp = var->bits_per_pixel;
@@ -704,7 +710,7 @@ static int omapfb_set_par(struct fb_info *fbi)
 	int r = 0;
 
 	omapfb_rqueue_lock(fbdev);
-	set_fb_fix(fbi);
+	set_fb_fix(fbi, 0);
 	r = ctrl_change_mode(fbi);
 	omapfb_rqueue_unlock(fbdev);
 
@@ -904,7 +910,7 @@ static int omapfb_setup_mem(struct fb_info *fbi, struct omapfb_mem_info *mi)
 		if (old_size != size) {
 			if (size) {
 				memcpy(&fbi->var, new_var, sizeof(fbi->var));
-				set_fb_fix(fbi);
+				set_fb_fix(fbi, 0);
 			} else {
 				/*
 				 * Set these explicitly to indicate that the
@@ -1504,7 +1510,7 @@ static int fbinfo_init(struct omapfb_device *fbdev, struct fb_info *info)
 	var->bits_per_pixel = fbdev->panel->bpp;
 
 	set_fb_var(info, var);
-	set_fb_fix(info);
+	set_fb_fix(info, 1);
 
 	r = fb_alloc_cmap(&info->cmap, 16, 0);
 	if (r != 0)
-- 
1.6.0.4


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

* Re: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-29 14:14 [RFC][PATCH] omapfb: Condition mutex acquisition Aguirre Rodriguez, Sergio Alberto
@ 2009-09-29 14:22 ` Tomi Valkeinen
  2009-09-29 14:34   ` Aguirre Rodriguez, Sergio Alberto
  0 siblings, 1 reply; 14+ messages in thread
From: Tomi Valkeinen @ 2009-09-29 14:22 UTC (permalink / raw)
  To: ext Aguirre Rodriguez, Sergio Alberto
  Cc: Deak Imre (Nokia-D/Helsinki), linux-omap@vger.kernel.org

On Tue, 2009-09-29 at 16:14 +0200, ext Aguirre Rodriguez, Sergio Alberto
wrote:
> From: Sergio Aguirre <saaguirre@ti.com>
> 
> Acquiring mutex before framebuffer registration doesn't make sense,
> As there's no danger of external access to the memory related fields.

What problem does this patch solve? It makes the code more complex.

 Tomi

> 
> NOTE: PLEASE REVIEW! I'M NOT AN EXPERT ON THIS.
> 
> Signed-off-by: Sergio Aguirre <saaguirre@ti.com>
> ---
>  drivers/video/omap/omapfb_main.c |   22 ++++++++++++++--------
>  1 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/video/omap/omapfb_main.c b/drivers/video/omap/omapfb_main.c
> index 125e605..0d0c8c8 100644
> --- a/drivers/video/omap/omapfb_main.c
> +++ b/drivers/video/omap/omapfb_main.c
> @@ -393,7 +393,7 @@ static void omapfb_sync(struct fb_info *fbi)
>   * Set fb_info.fix fields and also updates fbdev.
>   * When calling this fb_info.var must be set up already.
>   */
> -static void set_fb_fix(struct fb_info *fbi)
> +static void set_fb_fix(struct fb_info *fbi, int from_init)
>  {
>  	struct fb_fix_screeninfo *fix = &fbi->fix;
>  	struct fb_var_screeninfo *var = &fbi->var;
> @@ -403,10 +403,16 @@ static void set_fb_fix(struct fb_info *fbi)
>  
>  	rg = &plane->fbdev->mem_desc.region[plane->idx];
>  	fbi->screen_base	= rg->vaddr;
> -	mutex_lock(&fbi->mm_lock);
> -	fix->smem_start		= rg->paddr;
> -	fix->smem_len		= rg->size;
> -	mutex_unlock(&fbi->mm_lock);
> +
> +	if (!from_init) {
> +		mutex_lock(&fbi->mm_lock);
> +		fix->smem_start		= rg->paddr;
> +		fix->smem_len		= rg->size;
> +		mutex_unlock(&fbi->mm_lock);
> +	} else {
> +		fix->smem_start		= rg->paddr;
> +		fix->smem_len		= rg->size;
> +	}
>  
>  	fix->type = FB_TYPE_PACKED_PIXELS;
>  	bpp = var->bits_per_pixel;
> @@ -704,7 +710,7 @@ static int omapfb_set_par(struct fb_info *fbi)
>  	int r = 0;
>  
>  	omapfb_rqueue_lock(fbdev);
> -	set_fb_fix(fbi);
> +	set_fb_fix(fbi, 0);
>  	r = ctrl_change_mode(fbi);
>  	omapfb_rqueue_unlock(fbdev);
>  
> @@ -904,7 +910,7 @@ static int omapfb_setup_mem(struct fb_info *fbi, struct omapfb_mem_info *mi)
>  		if (old_size != size) {
>  			if (size) {
>  				memcpy(&fbi->var, new_var, sizeof(fbi->var));
> -				set_fb_fix(fbi);
> +				set_fb_fix(fbi, 0);
>  			} else {
>  				/*
>  				 * Set these explicitly to indicate that the
> @@ -1504,7 +1510,7 @@ static int fbinfo_init(struct omapfb_device *fbdev, struct fb_info *info)
>  	var->bits_per_pixel = fbdev->panel->bpp;
>  
>  	set_fb_var(info, var);
> -	set_fb_fix(info);
> +	set_fb_fix(info, 1);
>  
>  	r = fb_alloc_cmap(&info->cmap, 16, 0);
>  	if (r != 0)


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

* RE: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-29 14:22 ` Tomi Valkeinen
@ 2009-09-29 14:34   ` Aguirre Rodriguez, Sergio Alberto
  2009-09-29 14:37     ` Aguirre Rodriguez, Sergio Alberto
                       ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Aguirre Rodriguez, Sergio Alberto @ 2009-09-29 14:34 UTC (permalink / raw)
  To: tomi.valkeinen@nokia.com
  Cc: Deak Imre (Nokia-D/Helsinki), linux-omap@vger.kernel.org

From: Tomi Valkeinen [tomi.valkeinen@nokia.com]
Sent: Tuesday, September 29, 2009 9:22 AM
> On Tue, 2009-09-29 at 16:14 +0200, ext Aguirre Rodriguez, Sergio Alberto
> wrote:
> > From: Sergio Aguirre <saaguirre@ti.com>
> >
> > Acquiring mutex before framebuffer registration doesn't make sense,
> > As there's no danger of external access to the memory related fields.
> 
> What problem does this patch solve? It makes the code more complex.
> 

Tomi,

Thanks for your time.

The problem was that, during platform driver registration,
this sequence was executed:

-> omapfb_probe
   -> omapfb_do_probe
      -> planes_init
         -> fbinfo_init
            -> set_fb_fix
      ...
      -> register_framebuffer

And then, inside that function, an attempt of acquiring a
mutex failed, because it wasn't initialized before trying it:

mutex_lock(&fbi->mm_lock);

It is actually initialized later in omapfb_do_probe in register_framebuffer call.

So, how is the best to solve this then?

BTW, this problem is found on current Linus tree, and in
current l-o tree aswell, and i saw it on my 3430SDP VG5.0.1.

Regards,
Sergio

>  Tomi
> 

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

* RE: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-29 14:34   ` Aguirre Rodriguez, Sergio Alberto
@ 2009-09-29 14:37     ` Aguirre Rodriguez, Sergio Alberto
  2009-09-29 14:43     ` Hunter, Jon
  2009-09-29 14:53     ` Tomi Valkeinen
  2 siblings, 0 replies; 14+ messages in thread
From: Aguirre Rodriguez, Sergio Alberto @ 2009-09-29 14:37 UTC (permalink / raw)
  To: Aguirre Rodriguez, Sergio Alberto, tomi.valkeinen@nokia.com
  Cc: Deak Imre (Nokia-D/Helsinki), linux-omap@vger.kernel.org

From: linux-omap-owner@vger.kernel.org [linux-omap-owner@vger.kernel.org] On Behalf Of Aguirre Rodriguez, Sergio Alberto [saaguirre@ti.com]
Sent: Tuesday, September 29, 2009 9:34 AM
> From: Tomi Valkeinen [tomi.valkeinen@nokia.com]
> Sent: Tuesday, September 29, 2009 9:22 AM
> > On Tue, 2009-09-29 at 16:14 +0200, ext Aguirre Rodriguez, Sergio Alberto
> > wrote:
> > > From: Sergio Aguirre <saaguirre@ti.com>
> > >
> > > Acquiring mutex before framebuffer registration doesn't make sense,
> > > As there's no danger of external access to the memory related fields.
> >
> > What problem does this patch solve? It makes the code more complex.
> >
> 
> Tomi,
> 
> Thanks for your time.
> 
> The problem was that, during platform driver registration,
> this sequence was executed:
> 
> -> omapfb_probe
>    -> omapfb_do_probe
>       -> planes_init
>          -> fbinfo_init
>             -> set_fb_fix

Forgot to mark that the attempt to use the mutex its done here in set_fb_fix function

>       ...
>       -> register_framebuffer
> 
> And then, inside that function, an attempt of acquiring a
> mutex failed, because it wasn't initialized before trying it:
> 
> mutex_lock(&fbi->mm_lock);
> 
> It is actually initialized later in omapfb_do_probe in register_framebuffer call.
> 
> So, how is the best to solve this then?
> 
> BTW, this problem is found on current Linus tree, and in
> current l-o tree aswell, and i saw it on my 3430SDP VG5.0.1.

BTW #2. This makes framebuffer work on my board. I can see Tux during bootup,
and also i was able to run some basic framebuffer tests on my board without
apparent issues.
> 
> Regards,
> Sergio
> 
> >  Tomi
> >

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

* RE: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-29 14:34   ` Aguirre Rodriguez, Sergio Alberto
  2009-09-29 14:37     ` Aguirre Rodriguez, Sergio Alberto
@ 2009-09-29 14:43     ` Hunter, Jon
  2009-09-29 14:53     ` Tomi Valkeinen
  2 siblings, 0 replies; 14+ messages in thread
From: Hunter, Jon @ 2009-09-29 14:43 UTC (permalink / raw)
  To: Aguirre Rodriguez, Sergio Alberto, tomi.valkeinen@nokia.com
  Cc: Deak Imre (Nokia-D/Helsinki), linux-omap@vger.kernel.org

Aguirre Rodriguez, Sergio Alberto wrote:
> And then, inside that function, an attempt of acquiring a
> mutex failed, because it wasn't initialized before trying it:
> 
> mutex_lock(&fbi->mm_lock);

I would like to stress that this does cause the kernel to crash during boot up because this is mutex not initialised before it is used.  

> BTW, this problem is found on current Linus tree, and in
> current l-o tree aswell, and i saw it on my 3430SDP VG5.0.1.

I am seeing this too on the beagle-board and so it appears to impact a few omap3 boards. 

Cheers
Jon

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

* RE: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-29 14:34   ` Aguirre Rodriguez, Sergio Alberto
  2009-09-29 14:37     ` Aguirre Rodriguez, Sergio Alberto
  2009-09-29 14:43     ` Hunter, Jon
@ 2009-09-29 14:53     ` Tomi Valkeinen
  2009-09-29 14:56       ` Aguirre Rodriguez, Sergio Alberto
  2009-09-30  0:53       ` Tony Lindgren
  2 siblings, 2 replies; 14+ messages in thread
From: Tomi Valkeinen @ 2009-09-29 14:53 UTC (permalink / raw)
  To: ext Aguirre Rodriguez, Sergio Alberto
  Cc: Deak Imre (Nokia-D/Helsinki), linux-omap@vger.kernel.org

On Tue, 2009-09-29 at 16:34 +0200, ext Aguirre Rodriguez, Sergio Alberto
wrote:
> From: Tomi Valkeinen [tomi.valkeinen@nokia.com]
> Sent: Tuesday, September 29, 2009 9:22 AM
> > On Tue, 2009-09-29 at 16:14 +0200, ext Aguirre Rodriguez, Sergio Alberto
> > wrote:
> > > From: Sergio Aguirre <saaguirre@ti.com>
> > >
> > > Acquiring mutex before framebuffer registration doesn't make sense,
> > > As there's no danger of external access to the memory related fields.
> > 
> > What problem does this patch solve? It makes the code more complex.
> > 
> 
> Tomi,
> 
> Thanks for your time.
> 
> The problem was that, during platform driver registration,
> this sequence was executed:
> 
> -> omapfb_probe
>    -> omapfb_do_probe
>       -> planes_init
>          -> fbinfo_init
>             -> set_fb_fix
>       ...
>       -> register_framebuffer
> 
> And then, inside that function, an attempt of acquiring a
> mutex failed, because it wasn't initialized before trying it:
> 
> mutex_lock(&fbi->mm_lock);
> 
> It is actually initialized later in omapfb_do_probe in register_framebuffer call.
> 
> So, how is the best to solve this then?

Oh, I wasn't implying that there's something wrong with the fix, I just
didn't know what it was fixing =).

Looks like a valid fix to me.

 Tomi



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

* RE: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-29 14:53     ` Tomi Valkeinen
@ 2009-09-29 14:56       ` Aguirre Rodriguez, Sergio Alberto
  2009-09-29 18:26         ` Hiremath, Vaibhav
  2009-09-30  0:53       ` Tony Lindgren
  1 sibling, 1 reply; 14+ messages in thread
From: Aguirre Rodriguez, Sergio Alberto @ 2009-09-29 14:56 UTC (permalink / raw)
  To: tomi.valkeinen@nokia.com
  Cc: Deak Imre (Nokia-D/Helsinki), linux-omap@vger.kernel.org

From: Tomi Valkeinen [tomi.valkeinen@nokia.com]
Sent: Tuesday, September 29, 2009 9:53 AM
> On Tue, 2009-09-29 at 16:34 +0200, ext Aguirre Rodriguez, Sergio Alberto
> wrote:
> > From: Tomi Valkeinen [tomi.valkeinen@nokia.com]
> > Sent: Tuesday, September 29, 2009 9:22 AM
> > > On Tue, 2009-09-29 at 16:14 +0200, ext Aguirre Rodriguez, Sergio Alberto
> > > wrote:
> > > > From: Sergio Aguirre <saaguirre@ti.com>
> > > >
> > > > Acquiring mutex before framebuffer registration doesn't make sense,
> > > > As there's no danger of external access to the memory related fields.
> > >
> > > What problem does this patch solve? It makes the code more complex.
> > >
> >
> > Tomi,
> >
> > Thanks for your time.
> >
> > The problem was that, during platform driver registration,
> > this sequence was executed:
> >
> > -> omapfb_probe
> >    -> omapfb_do_probe
> >       -> planes_init
> >          -> fbinfo_init
> >             -> set_fb_fix
> >       ...
> >       -> register_framebuffer
> >
> > And then, inside that function, an attempt of acquiring a
> > mutex failed, because it wasn't initialized before trying it:
> >
> > mutex_lock(&fbi->mm_lock);
> >
> > It is actually initialized later in omapfb_do_probe in register_framebuffer call.
> >
> > So, how is the best to solve this then?
> 
> Oh, I wasn't implying that there's something wrong with the fix, I just
> didn't know what it was fixing =).

Ok, no prob :) Should I improve the patch description?
Any suggestion coming from an expert ;) ?

> 
> Looks like a valid fix to me.

Great! :) Once you're ok with the description, I can add your "Acked-by:" and
send to linux-fb-devel ML. Would that be ok?

Regards,
Sergio

> 
>  Tomi

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

* RE: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-29 14:56       ` Aguirre Rodriguez, Sergio Alberto
@ 2009-09-29 18:26         ` Hiremath, Vaibhav
  2009-09-30  8:32           ` Tomi Valkeinen
  0 siblings, 1 reply; 14+ messages in thread
From: Hiremath, Vaibhav @ 2009-09-29 18:26 UTC (permalink / raw)
  To: Aguirre Rodriguez, Sergio Alberto, tomi.valkeinen@nokia.com
  Cc: Deak Imre (Nokia-D/Helsinki), linux-omap@vger.kernel.org


> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Aguirre Rodriguez, Sergio
> Alberto
> Sent: Tuesday, September 29, 2009 8:27 PM
> To: tomi.valkeinen@nokia.com
> Cc: Deak Imre (Nokia-D/Helsinki); linux-omap@vger.kernel.org
> Subject: RE: [RFC][PATCH] omapfb: Condition mutex acquisition
> 
> From: Tomi Valkeinen [tomi.valkeinen@nokia.com]
> Sent: Tuesday, September 29, 2009 9:53 AM
> > On Tue, 2009-09-29 at 16:34 +0200, ext Aguirre Rodriguez, Sergio
> Alberto
> > wrote:
> > > From: Tomi Valkeinen [tomi.valkeinen@nokia.com]
> > > Sent: Tuesday, September 29, 2009 9:22 AM
> > > > On Tue, 2009-09-29 at 16:14 +0200, ext Aguirre Rodriguez,
> Sergio Alberto
> > > > wrote:
> > > > > From: Sergio Aguirre <saaguirre@ti.com>
> > > > >
> > > > > Acquiring mutex before framebuffer registration doesn't make
> sense,
> > > > > As there's no danger of external access to the memory
> related fields.
> > > >
> > > > What problem does this patch solve? It makes the code more
> complex.
> > > >
> > >
> > > Tomi,
> > >
> > > Thanks for your time.
> > >
> > > The problem was that, during platform driver registration,
> > > this sequence was executed:
> > >
> > > -> omapfb_probe
> > >    -> omapfb_do_probe
> > >       -> planes_init
> > >          -> fbinfo_init
> > >             -> set_fb_fix
> > >       ...
> > >       -> register_framebuffer
> > >
> > > And then, inside that function, an attempt of acquiring a
> > > mutex failed, because it wasn't initialized before trying it:
> > >
> > > mutex_lock(&fbi->mm_lock);
> > >
> > > It is actually initialized later in omapfb_do_probe in
> register_framebuffer call.
> > >
> > > So, how is the best to solve this then?
> >
> > Oh, I wasn't implying that there's something wrong with the fix, I
> just
> > didn't know what it was fixing =).
> 
> Ok, no prob :) Should I improve the patch description?
> Any suggestion coming from an expert ;) ?
> 
> >
> > Looks like a valid fix to me.
> 
> Great! :) Once you're ok with the description, I can add your
> "Acked-by:" and
> send to linux-fb-devel ML. Would that be ok?
> 
[Hiremath, Vaibhav] Hi Sergio,

I also think this is valid fix.

Tomi,

Don't you think we should also make use of mm_lock in new OMAP2 Fbdev driver, to be more specific and safer side?

Thanks,
Vaibhav

> Regards,
> Sergio
> 
> >
> >  Tomi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

* Re: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-29 14:53     ` Tomi Valkeinen
  2009-09-29 14:56       ` Aguirre Rodriguez, Sergio Alberto
@ 2009-09-30  0:53       ` Tony Lindgren
  2009-09-30  8:35         ` Tomi Valkeinen
  2009-09-30 12:08         ` Imre Deak
  1 sibling, 2 replies; 14+ messages in thread
From: Tony Lindgren @ 2009-09-30  0:53 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: ext Aguirre Rodriguez, Sergio Alberto,
	Deak Imre (Nokia-D/Helsinki), linux-omap@vger.kernel.org

* Tomi Valkeinen <tomi.valkeinen@nokia.com> [090929 07:54]:
> On Tue, 2009-09-29 at 16:34 +0200, ext Aguirre Rodriguez, Sergio Alberto
> wrote:
> > From: Tomi Valkeinen [tomi.valkeinen@nokia.com]
> > Sent: Tuesday, September 29, 2009 9:22 AM
> > > On Tue, 2009-09-29 at 16:14 +0200, ext Aguirre Rodriguez, Sergio Alberto
> > > wrote:
> > > > From: Sergio Aguirre <saaguirre@ti.com>
> > > >
> > > > Acquiring mutex before framebuffer registration doesn't make sense,
> > > > As there's no danger of external access to the memory related fields.
> > > 
> > > What problem does this patch solve? It makes the code more complex.
> > > 
> > 
> > Tomi,
> > 
> > Thanks for your time.
> > 
> > The problem was that, during platform driver registration,
> > this sequence was executed:
> > 
> > -> omapfb_probe
> >    -> omapfb_do_probe
> >       -> planes_init
> >          -> fbinfo_init
> >             -> set_fb_fix
> >       ...
> >       -> register_framebuffer
> > 
> > And then, inside that function, an attempt of acquiring a
> > mutex failed, because it wasn't initialized before trying it:
> > 
> > mutex_lock(&fbi->mm_lock);
> > 
> > It is actually initialized later in omapfb_do_probe in register_framebuffer call.
> > 
> > So, how is the best to solve this then?
> 
> Oh, I wasn't implying that there's something wrong with the fix, I just
> didn't know what it was fixing =).
> 
> Looks like a valid fix to me.

Tomi & Imre, do you want me to add this to the omap-fixes queue,
or are you planning to send other fixes too?

If you want me to add it, please reply with your ack.

I'll added it into omap-fixes-testing branch for now so we can
get the omaps booted.

Regards,

Tony

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

* RE: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-29 18:26         ` Hiremath, Vaibhav
@ 2009-09-30  8:32           ` Tomi Valkeinen
  0 siblings, 0 replies; 14+ messages in thread
From: Tomi Valkeinen @ 2009-09-30  8:32 UTC (permalink / raw)
  To: ext Hiremath, Vaibhav
  Cc: Aguirre Rodriguez, Sergio Alberto, Deak Imre (Nokia-D/Helsinki),
	linux-omap@vger.kernel.org

On Tue, 2009-09-29 at 20:26 +0200, ext Hiremath, Vaibhav wrote:

> Tomi,
> 
> Don't you think we should also make use of mm_lock in new OMAP2 Fbdev driver, to be more specific and safer side?
> 

Hmm yes, I think we should. One more thing to the TODO list =)

 Tomi



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

* Re: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-30  0:53       ` Tony Lindgren
@ 2009-09-30  8:35         ` Tomi Valkeinen
  2009-09-30 12:08         ` Imre Deak
  1 sibling, 0 replies; 14+ messages in thread
From: Tomi Valkeinen @ 2009-09-30  8:35 UTC (permalink / raw)
  To: ext Tony Lindgren
  Cc: ext Aguirre Rodriguez, Sergio Alberto,
	Deak Imre (Nokia-D/Helsinki), linux-omap@vger.kernel.org

On Wed, 2009-09-30 at 02:53 +0200, ext Tony Lindgren wrote:
> * Tomi Valkeinen <tomi.valkeinen@nokia.com> [090929 07:54]:
> > On Tue, 2009-09-29 at 16:34 +0200, ext Aguirre Rodriguez, Sergio Alberto
> > wrote:
> > > From: Tomi Valkeinen [tomi.valkeinen@nokia.com]
> > > Sent: Tuesday, September 29, 2009 9:22 AM
> > > > On Tue, 2009-09-29 at 16:14 +0200, ext Aguirre Rodriguez, Sergio Alberto
> > > > wrote:
> > > > > From: Sergio Aguirre <saaguirre@ti.com>
> > > > >
> > > > > Acquiring mutex before framebuffer registration doesn't make sense,
> > > > > As there's no danger of external access to the memory related fields.
> > > > 
> > > > What problem does this patch solve? It makes the code more complex.
> > > > 
> > > 
> > > Tomi,
> > > 
> > > Thanks for your time.
> > > 
> > > The problem was that, during platform driver registration,
> > > this sequence was executed:
> > > 
> > > -> omapfb_probe
> > >    -> omapfb_do_probe
> > >       -> planes_init
> > >          -> fbinfo_init
> > >             -> set_fb_fix
> > >       ...
> > >       -> register_framebuffer
> > > 
> > > And then, inside that function, an attempt of acquiring a
> > > mutex failed, because it wasn't initialized before trying it:
> > > 
> > > mutex_lock(&fbi->mm_lock);
> > > 
> > > It is actually initialized later in omapfb_do_probe in register_framebuffer call.
> > > 
> > > So, how is the best to solve this then?
> > 
> > Oh, I wasn't implying that there's something wrong with the fix, I just
> > didn't know what it was fixing =).
> > 
> > Looks like a valid fix to me.
> 
> Tomi & Imre, do you want me to add this to the omap-fixes queue,
> or are you planning to send other fixes too?
> 
> If you want me to add it, please reply with your ack.
> 
> I'll added it into omap-fixes-testing branch for now so we can
> get the omaps booted.

I don't know if Imre wants to handle this (probably not =), but I would
rather concentrate on DSS2 patches. I have enough of them already, and
my progress in upstreaming them seems to be... slow... =).

So if you can take any minor old-omapfb patches to omap-fixes queue,
it'd be great.

Acked-by: Tomi Valkeinen <tomi.valkeinen@nokia.com>

 Tomi



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

* Re: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-30  0:53       ` Tony Lindgren
  2009-09-30  8:35         ` Tomi Valkeinen
@ 2009-09-30 12:08         ` Imre Deak
  2009-09-30 13:34           ` Aguirre Rodriguez, Sergio Alberto
  1 sibling, 1 reply; 14+ messages in thread
From: Imre Deak @ 2009-09-30 12:08 UTC (permalink / raw)
  To: ext Tony Lindgren
  Cc: Valkeinen Tomi (Nokia-D/Helsinki),
	ext Aguirre Rodriguez, Sergio Alberto, linux-omap@vger.kernel.org

Hi,

On Wed, Sep 30, 2009 at 02:53:10AM +0200, ext Tony Lindgren wrote:
> [...] 
> Tomi & Imre, do you want me to add this to the omap-fixes queue,
> or are you planning to send other fixes too?
> 
> If you want me to add it, please reply with your ack.
> 
> I'll added it into omap-fixes-testing branch for now so we can
> get the omaps booted.

Acked-by: Imre Deak <imre.deak@nokia.com>

There are no other patches for the DSS1 driver, so it'd be good if
you could just apply it to omap-fixes.

--Imre


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

* RE: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-30 12:08         ` Imre Deak
@ 2009-09-30 13:34           ` Aguirre Rodriguez, Sergio Alberto
  2009-09-30 15:34             ` Tony Lindgren
  0 siblings, 1 reply; 14+ messages in thread
From: Aguirre Rodriguez, Sergio Alberto @ 2009-09-30 13:34 UTC (permalink / raw)
  To: Imre Deak, ext Tony Lindgren
  Cc: Valkeinen Tomi (Nokia-D/Helsinki), linux-omap@vger.kernel.org

Imre Deak wrote:
> Hi,
> 
> On Wed, Sep 30, 2009 at 02:53:10AM +0200, ext Tony Lindgren wrote:
>> [...]
>> Tomi & Imre, do you want me to add this to the omap-fixes queue,
>> or are you planning to send other fixes too?
>> 
>> If you want me to add it, please reply with your ack.
>> 
>> I'll added it into omap-fixes-testing branch for now so we can
>> get the omaps booted.
> 
> Acked-by: Imre Deak <imre.deak@nokia.com>
> 
> There are no other patches for the DSS1 driver, so it'd be good if
> you could just apply it to omap-fixes.

Tony,

Please take v2 of patch instead, has a richer description of the problem.

Thanks in advance,
Sergio

> 
> --Imre


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

* Re: [RFC][PATCH] omapfb: Condition mutex acquisition
  2009-09-30 13:34           ` Aguirre Rodriguez, Sergio Alberto
@ 2009-09-30 15:34             ` Tony Lindgren
  0 siblings, 0 replies; 14+ messages in thread
From: Tony Lindgren @ 2009-09-30 15:34 UTC (permalink / raw)
  To: Aguirre Rodriguez, Sergio Alberto
  Cc: Imre Deak, Valkeinen Tomi (Nokia-D/Helsinki),
	linux-omap@vger.kernel.org

* Aguirre Rodriguez, Sergio Alberto <saaguirre@ti.com> [090930 06:35]:
> Imre Deak wrote:
> > Hi,
> > 
> > On Wed, Sep 30, 2009 at 02:53:10AM +0200, ext Tony Lindgren wrote:
> >> [...]
> >> Tomi & Imre, do you want me to add this to the omap-fixes queue,
> >> or are you planning to send other fixes too?
> >> 
> >> If you want me to add it, please reply with your ack.
> >> 
> >> I'll added it into omap-fixes-testing branch for now so we can
> >> get the omaps booted.
> > 
> > Acked-by: Imre Deak <imre.deak@nokia.com>
> > 
> > There are no other patches for the DSS1 driver, so it'd be good if
> > you could just apply it to omap-fixes.
> 
> Tony,
> 
> Please take v2 of patch instead, has a richer description of the problem.
> 
> Thanks in advance,
> Sergio

OK, thanks, will queue it.

Regards,

Tony

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

end of thread, other threads:[~2009-09-30 15:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-29 14:14 [RFC][PATCH] omapfb: Condition mutex acquisition Aguirre Rodriguez, Sergio Alberto
2009-09-29 14:22 ` Tomi Valkeinen
2009-09-29 14:34   ` Aguirre Rodriguez, Sergio Alberto
2009-09-29 14:37     ` Aguirre Rodriguez, Sergio Alberto
2009-09-29 14:43     ` Hunter, Jon
2009-09-29 14:53     ` Tomi Valkeinen
2009-09-29 14:56       ` Aguirre Rodriguez, Sergio Alberto
2009-09-29 18:26         ` Hiremath, Vaibhav
2009-09-30  8:32           ` Tomi Valkeinen
2009-09-30  0:53       ` Tony Lindgren
2009-09-30  8:35         ` Tomi Valkeinen
2009-09-30 12:08         ` Imre Deak
2009-09-30 13:34           ` Aguirre Rodriguez, Sergio Alberto
2009-09-30 15:34             ` Tony Lindgren

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox