crash in aty128_set_lcd_enable on PowerBook

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.

 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;

On Sun, 2006-07-16 at 18:50 +0200, Olaf Hering wrote:

 On Sun, Jul 16, Olaf Hering wrote:

quoted

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

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.

 On Sun, Jul 16, Benjamin Herrenschmidt wrote:

quoted

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.

On Sun, 2006-07-16 at 21:27 +0200, Olaf Hering wrote:

 On Sun, Jul 16, Benjamin Herrenschmidt wrote:

quoted

quoted

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.

 On Sun, Jul 16, Benjamin Herrenschmidt wrote:

quoted

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.

 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 <redacted>

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;

Olaf Hering wrote:

 On Sun, Jul 16, Olaf Hering wrote:

quoted

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 <redacted>

Acked-by: Antonino Daplas <redacted>