On Tue, May 26, 2009 at 01:49:18PM -0700, Andrew Morton wrote:

On Tue, 26 May 2009 02:05:14 +0300
Ville Syrjala [off-list ref] wrote:

quoted

Apparently HP OmniBook 500's BIOS doesn't like the way atyfb reprograms
the hardware. The BIOS will simply hang after a reboot. Fix the problem
by restoring the hardware to it's original state on reboot.


...

@@ -3502,6 +3503,11 @@ static int __devinit atyfb_pci_probe(struct pci_dev *pdev, const struct pci_devi
 	par->mmap_map[1].prot_flag = _PAGE_E;
 #endif /* __sparc__ */
 
+	mutex_lock(&reboot_lock);
+	if (!reboot_info)
+		reboot_info = info;
+	mutex_unlock(&reboot_lock);

This looks risky to me.  We save away a pointer to a structure which
was created by framebuffer_alloc().  What guarantee is there that this
memory is still valid when the reboot happens later on?

atyfb_remove() will clear the pointer before freeing the memory. The
mutex is supposed to make sure that the structure won't be freed while 
the reboot notifier is executing.

Hmm. I suppose I might have to grab the fb_info lock there too to
protect against other fb activity happening at the same time.

I also noticed that I managed to misplace reboot_info pointer clearing
a bit. It should really be in atyfb_pci_remove() instead of
atyfb_remove() since it's set in atyfb_pci_probe().

quoted

 	return 0;
 
 err_release_io:

@@ -3613,9 +3619,14 @@ static void __devexit atyfb_remove(struct fb_info *info)
 {
 	struct atyfb_par *par = (struct atyfb_par *) info->par;
 
+	mutex_lock(&reboot_lock);
+	if (reboot_info == info)
+		reboot_info = NULL;
+	mutex_unlock(&reboot_lock);
+
 	/* restore video mode */
-	aty_set_crtc(par, &saved_crtc);
-	par->pll_ops->set_pll(info, &saved_pll);
+	aty_set_crtc(par, &par->saved_crtc);
+	par->pll_ops->set_pll(info, &par->saved_pll);
 
 	unregister_framebuffer(info);
 

@@ -3808,6 +3819,39 @@ static int __init atyfb_setup(char *options)
 }
 #endif  /*  MODULE  */
 
+static int atyfb_reboot_notify(struct notifier_block *nb,
+			       unsigned long code, void *unused)
+{
+	struct atyfb_par *par;
+
+	if (code != SYS_RESTART)
+		return NOTIFY_DONE;
+
+	mutex_lock(&reboot_lock);
+
+	if (!reboot_info)
+		goto out;
+
+	par = reboot_info->par;
+
+	/*
+	 * HP OmniBook 500's BIOS doesn't like the state of the
+	 * hardware after atyfb has been used. Restore the hardware
+	 * to the original state to allow succesful reboots.

"successful" ;)

Right.

quoted

+	 */
+	aty_set_crtc(par, &par->saved_crtc);
+	par->pll_ops->set_pll(reboot_info, &par->saved_pll);
+
+ out:
+	mutex_unlock(&reboot_lock);
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block atyfb_reboot_notifier = {
+	.notifier_call = atyfb_reboot_notify,
+};
+
 static int __init atyfb_init(void)
 {
     int err1 = 1, err2 = 1;

@@ -3826,11 +3870,18 @@ static int __init atyfb_init(void)
     err2 = atyfb_atari_probe();
 #endif
 
-    return (err1 && err2) ? -ENODEV : 0;
+    if (err1 && err2)
+	return -ENODEV;
+
+    register_reboot_notifier(&atyfb_reboot_notifier);
+
+    return 0;
 }

ick.  Please feel free to repair the indenting in atyfb_init().

The indentation is broken in other parts of the driver too. I'll make
a separate cosmetics patch to clean it all up.

quoted

 static void __exit atyfb_exit(void)
 {
+	unregister_reboot_notifier(&atyfb_reboot_notifier);
+
 #ifdef CONFIG_PCI
 	pci_unregister_driver(&atyfb_driver);
 #endif

So we do the restoration for all supported devices on all machines,
even though it's only known to be needed on one card on one machine.

Hopefully that's safe, but a more cautious approach would use a
whitelist of some form.  I don't have enough experience with these
things to be able to judge the risk.

It should be safe in theory :) If the restoration breaks on some system
then the probe error handling and remove handling are also broken since
they do the same stuff. But to be honest I didn't test it on other
systems. I could do a DMI match but I'm not sure the extra complexity
is actually warranted. I'll give it a spin on a few other systems in
any case.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/

------------------------------------------------------------------------------
Register Now for Creativity and Technology (CaT), June 3rd, NYC. CaT 
is a gathering of tech-side developers & brand creativity professionals. Meet
the minds behind Google Creative Lab, Visual Complexity, Processing, & 
iPhoneDevCamp as they present alongside digital heavyweights like Barbarian 
Group, R/GA, & Big Spaceship. http://p.sf.net/sfu/creativitycat-com