* [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