Patchwork [BUGFIX] video: mxsfb: fix crash when unblanking the display

login
register
mail settings
Submitter Lothar Waßmann
Date Nov. 22, 2012, 12:49 p.m.
Message ID <1353588555-13168-1-git-send-email-LW@KARO-electronics.de>
Download mbox | patch
Permalink /patch/201042/
State New
Headers show

Comments

Lothar Waßmann - Nov. 22, 2012, 12:49 p.m.
The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
The write in mxsfb_disable_controller() sets the data_cnt for the LCD
DMA to 0 which obviously means the max. count for the LCD DMA and
leads to overwriting arbitrary memory when the display is unblanked.

Note: I already sent this patch in December 2011 but noone picked it up obviously.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/video/mxsfb.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Lothar Waßmann - Nov. 29, 2012, 11:55 a.m.
Hi,

adding Juergen Beisert as the driver's author to the recipient list.

Lothar Waßmann writes:
> The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
> The write in mxsfb_disable_controller() sets the data_cnt for the LCD
> DMA to 0 which obviously means the max. count for the LCD DMA and
> leads to overwriting arbitrary memory when the display is unblanked.
> 
> Note: I already sent this patch in December 2011 but noone picked it up obviously.
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>  drivers/video/mxsfb.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> index 49619b4..f2a49ef 100644
> --- a/drivers/video/mxsfb.c
> +++ b/drivers/video/mxsfb.c
> @@ -369,7 +369,8 @@ static void mxsfb_disable_controller(struct fb_info *fb_info)
>  		loop--;
>  	}
>  
> -	writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
> +	reg = readl(host->base + LCDC_VDCTRL4);
> +	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
>  
>  	clk_disable_unprepare(host->clk);
>  
> -- 
> 1.7.2.5
> 
ping?


Lothar Waßmann
Juergen Beisert - Nov. 29, 2012, 1:50 p.m.
Hi Lothar,

Lothar Waßmann wrote:
> > The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
> > The write in mxsfb_disable_controller() sets the data_cnt for the LCD
> > DMA to 0 which obviously means the max. count for the LCD DMA and
> > leads to overwriting arbitrary memory when the display is unblanked.

Overwriting? It should be a reading DMA unit, shouldn't it?

> > Note: I already sent this patch in December 2011 but noone picked it up
> > obviously.
> >
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > ---
> >  drivers/video/mxsfb.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> > index 49619b4..f2a49ef 100644
> > --- a/drivers/video/mxsfb.c
> > +++ b/drivers/video/mxsfb.c
> > @@ -369,7 +369,8 @@ static void mxsfb_disable_controller(struct fb_info
> > *fb_info) loop--;
> >  	}
> >
> > -	writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
> > +	reg = readl(host->base + LCDC_VDCTRL4);
> > +	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
> >
> >  	clk_disable_unprepare(host->clk);
> >
> > --
> > 1.7.2.5
>
> ping? 

I'm not sure if the missing SET/CLR feature for this register is a typo in the 
i.MX23/i.MX28 data sheets or if it is intended. Lets assume its the truth 
what they have written:

Acked-by: Juergen Beisert <jbe@pengutronix.de>

Regards,
Juergen
Lauri Hintsala - Jan. 4, 2013, 7:08 a.m.
On 11/29/2012 01:55 PM, Lothar Waßmann wrote:
> Hi,
>
> adding Juergen Beisert as the driver's author to the recipient list.
>
> Lothar Waßmann writes:
>> The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
>> The write in mxsfb_disable_controller() sets the data_cnt for the LCD
>> DMA to 0 which obviously means the max. count for the LCD DMA and
>> leads to overwriting arbitrary memory when the display is unblanked.
>>
>> Note: I already sent this patch in December 2011 but noone picked it up obviously.
>>
>> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>

Tested-by: Lauri Hintsala <lauri.hintsala@bluegiga.net>

This fixes the CPU freezing. I really recommend to apply this patch into 
mainline soon because unblanking freezes the whole system!!!

Default value of consoleblank parameter is 600 seconds and the whole 
system crashes every time when framebuffer console is activated after 
that timeout. I wonder why this issue is still open in mainline kernel.

Should this patch be added into stable versions too?

Best Regards,
Lauri Hintsala
Shawn Guo - Jan. 4, 2013, 7:42 a.m.
On Fri, Jan 04, 2013 at 09:08:12AM +0200, Lauri Hintsala wrote:
> On 11/29/2012 01:55 PM, Lothar Waßmann wrote:
> >Hi,
> >
> >adding Juergen Beisert as the driver's author to the recipient list.
> >
> >Lothar Waßmann writes:
> >>The VDCTRL4 register does not provide the MXS SET/CLR/TOGGLE feature.
> >>The write in mxsfb_disable_controller() sets the data_cnt for the LCD
> >>DMA to 0 which obviously means the max. count for the LCD DMA and
> >>leads to overwriting arbitrary memory when the display is unblanked.
> >>
> >>Note: I already sent this patch in December 2011 but noone picked it up obviously.
> >>
> >>Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> 
> Tested-by: Lauri Hintsala <lauri.hintsala@bluegiga.net>
> 
> This fixes the CPU freezing. I really recommend to apply this patch
> into mainline soon because unblanking freezes the whole system!!!
> 
> Default value of consoleblank parameter is 600 seconds and the whole
> system crashes every time when framebuffer console is activated
> after that timeout. I wonder why this issue is still open in
> mainline kernel.

Part of the reason is the FB maintainer's absence.  Let me try to send
it through arm-soc tree.

> 
> Should this patch be added into stable versions too?
> 
Yes, will do.

Shawn

Patch

diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
index 49619b4..f2a49ef 100644
--- a/drivers/video/mxsfb.c
+++ b/drivers/video/mxsfb.c
@@ -369,7 +369,8 @@  static void mxsfb_disable_controller(struct fb_info *fb_info)
 		loop--;
 	}
 
-	writel(VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4 + REG_CLR);
+	reg = readl(host->base + LCDC_VDCTRL4);
+	writel(reg & ~VDCTRL4_SYNC_SIGNALS_ON, host->base + LCDC_VDCTRL4);
 
 	clk_disable_unprepare(host->clk);