crash in aty128_set_lcd_enable on PowerBook

crash in aty128_set_lcd_enable on PowerBook

[Olaf Hering]

Current Linus tree crashes in aty128_set_lcd_enable() because par->pdev
is NULL. This happens since at least a week. Call trace is:

aty128_set_lcd_enable
aty128fb_set_par
fbcon_init
visual_init
take_over_console
fbcon_takeover
notifier_call_chain
blocking_notifier_call_chain
register_framebuffer
aty128fb_probe
pci_device_probe
bus_for_each_dev
driver_attach
bus_add_driver
driver_register
__pci_register_driver
aty128fb_init
init
kernel_thread

This happens on a PowerBook pismo.

Re: crash in aty128_set_lcd_enable on PowerBook

[Olaf Hering]

 On Sun, Jul 16, Olaf Hering wrote:

> 
> Current Linus tree crashes in aty128_set_lcd_enable() because par->pdev
> is NULL. This happens since at least a week. Call trace is:
> 
> aty128_set_lcd_enable
> aty128fb_set_par
> fbcon_init
> visual_init
> take_over_console
> fbcon_takeover
> notifier_call_chain
> blocking_notifier_call_chain
> register_framebuffer
> aty128fb_probe

aty128_init calls register_framebuffer() before it assigns pdev.

   2028         if (register_framebuffer(info) < 0)
   2029                 return 0;
   2030 
   2031         par->pm_reg = pci_find_capability(pdev, PCI_CAP_ID_PM);
   2032         par->pdev = pdev;
   2033         par->asleep = 0;
   2034         par->lock_blank = 0;

Re: crash in aty128_set_lcd_enable on PowerBook

[Benjamin Herrenschmidt]

On Sun, 2006-07-16 at 18:50 +0200, Olaf Hering wrote:
>  On Sun, Jul 16, Olaf Hering wrote:
> 
> > 
> > Current Linus tree crashes in aty128_set_lcd_enable() because par->pdev
> > is NULL. This happens since at least a week. Call trace is:
> > 
> > aty128_set_lcd_enable
> > aty128fb_set_par
> > fbcon_init
> > visual_init
> > take_over_console
> > fbcon_takeover
> > notifier_call_chain
> > blocking_notifier_call_chain
> > register_framebuffer
> > aty128fb_probe
> 
> aty128_init calls register_framebuffer() before it assigns pdev.

Yeah, that looks like some serious bogosity in that code. Care to send a
patch ?

>    2028         if (register_framebuffer(info) < 0)
>    2029                 return 0;
>    2030 
>    2031         par->pm_reg = pci_find_capability(pdev, PCI_CAP_ID_PM);
>    2032         par->pdev = pdev;
>    2033         par->asleep = 0;
>    2034         par->lock_blank = 0;
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: crash in aty128_set_lcd_enable on PowerBook

[Benjamin Herrenschmidt]

> Yeah, that looks like some serious bogosity in that code. Care to send a
> patch ?

(I'm in a hotel room in Ottawa with no r128 at hand to test so I'm not
doing it myself just now :)

Ben.

Re: crash in aty128_set_lcd_enable on PowerBook

[Olaf Hering]

 On Sun, Jul 16, Benjamin Herrenschmidt wrote:

> 
> > Yeah, that looks like some serious bogosity in that code. Care to send a
> > patch ?
> 
> (I'm in a hotel room in Ottawa with no r128 at hand to test so I'm not
> doing it myself just now :)

It crashes later for different reasons. The whole init process works by
luck it seems.

Re: crash in aty128_set_lcd_enable on PowerBook

[Benjamin Herrenschmidt]

On Sun, 2006-07-16 at 21:27 +0200, Olaf Hering wrote:
>  On Sun, Jul 16, Benjamin Herrenschmidt wrote:
> 
> > 
> > > Yeah, that looks like some serious bogosity in that code. Care to send a
> > > patch ?
> > 
> > (I'm in a hotel room in Ottawa with no r128 at hand to test so I'm not
> > doing it myself just now :)
> 
> It crashes later for different reasons. The whole init process works by
> luck it seems.

I've been having weird things happening with latest linus trees and
really no time to debug ... do you have a backtrace for the "other"
crash ?

Ben.

Re: crash in aty128_set_lcd_enable on PowerBook

[Olaf Hering]

 On Sun, Jul 16, Benjamin Herrenschmidt wrote:

> > It crashes later for different reasons. The whole init process works by
> > luck it seems.
> 
> I've been having weird things happening with latest linus trees and
> really no time to debug ... do you have a backtrace for the "other"
> crash ?

It was in aty128_bl_set_power(), cant remember the trace.

[PATCH] crash in aty128_set_lcd_enable on PowerBook

[Olaf Hering]

 On Sun, Jul 16, Olaf Hering wrote:

> 
> Current Linus tree crashes in aty128_set_lcd_enable() because par->pdev
> is NULL. This happens since at least a week. Call trace is:
> 
> aty128_set_lcd_enable
> aty128fb_set_par
> fbcon_init
> visual_init
> take_over_console
> fbcon_takeover
> notifier_call_chain
> blocking_notifier_call_chain
> register_framebuffer
> aty128fb_probe
> pci_device_probe
> bus_for_each_dev
> driver_attach
> bus_add_driver
> driver_register
> __pci_register_driver
> aty128fb_init
> init
> kernel_thread
> 


- info->fix was assigned twice.
- par->vram_size is assigned in aty128_probe(), no need to redo it again in aty128_init()
- register_framebuffer() uses uninitialized struct members,
  move it past par->pdev assignment and past aty128_bl_init().


Signed-off-by: Olaf Hering <olh@suse.de>

Index: linux-2.6.18-rc3/drivers/video/aty/aty128fb.c
===================================================================
--- linux-2.6.18-rc3.orig/drivers/video/aty/aty128fb.c
+++ linux-2.6.18-rc3/drivers/video/aty/aty128fb.c
@@ -1910,9 +1910,6 @@ static int __devinit aty128_init(struct 
 	u8 chip_rev;
 	u32 dac;
 
-	if (!par->vram_size)	/* may have already been probed */
-		par->vram_size = aty_ld_le32(CONFIG_MEMSIZE) & 0x03FFFFFF;
-
 	/* Get the chip revision */
 	chip_rev = (aty_ld_le32(CONFIG_CNTL) >> 16) & 0x1F;
 
@@ -2025,9 +2022,6 @@ static int __devinit aty128_init(struct 
 
 	aty128_init_engine(par);
 
-	if (register_framebuffer(info) < 0)
-		return 0;
-
 	par->pm_reg = pci_find_capability(pdev, PCI_CAP_ID_PM);
 	par->pdev = pdev;
 	par->asleep = 0;
@@ -2037,6 +2031,9 @@ static int __devinit aty128_init(struct 
 	aty128_bl_init(par);
 #endif
 
+	if (register_framebuffer(info) < 0)
+		return 0;
+
 	printk(KERN_INFO "fb%d: %s frame buffer device on %s\n",
 	       info->node, info->fix.id, video_card);
 
@@ -2086,7 +2083,6 @@ static int __devinit aty128_probe(struct
 	par = info->par;
 
 	info->pseudo_palette = par->pseudo_palette;
-	info->fix = aty128fb_fix;
 
 	/* Virtualize mmio region */
 	info->fix.mmio_start = reg_addr;

Re: [PATCH] crash in aty128_set_lcd_enable on PowerBook

[Antonino A. Daplas]

Olaf Hering wrote:
>  On Sun, Jul 16, Olaf Hering wrote:
> 
>> Current Linus tree crashes in aty128_set_lcd_enable() because par->pdev
>> is NULL. This happens since at least a week. Call trace is:
>>
>> aty128_set_lcd_enable
>> aty128fb_set_par
>> fbcon_init
>> visual_init
>> take_over_console
>> fbcon_takeover
>> notifier_call_chain
>> blocking_notifier_call_chain
>> register_framebuffer
>> aty128fb_probe
>> pci_device_probe
>> bus_for_each_dev
>> driver_attach
>> bus_add_driver
>> driver_register
>> __pci_register_driver
>> aty128fb_init
>> init
>> kernel_thread
>>
> 
> 
> - info->fix was assigned twice.
> - par->vram_size is assigned in aty128_probe(), no need to redo it again in aty128_init()
> - register_framebuffer() uses uninitialized struct members,
>   move it past par->pdev assignment and past aty128_bl_init().
> 
> 

Looks good.

> Signed-off-by: Olaf Hering <olh@suse.de>
Acked-by: Antonino Daplas <adaplas@pol.net>