Subject: Re: powermac: More powermac backlight fixes

From: Michael Hanselmann <[email-masked]>
Date: Tue, 25 Jul 2006 22:06:50 +0200

On Tue, Jul 25, 2006 at 08:44:06PM +0200, Michael Hanselmann wrote:
> On Mon, Jul 24, 2006 at 08:03:15PM -0700, Andrew Morton wrote:
> > I see schedule_work()s in there, but no flush_scheduled_work()s or anything
> > like that.  Generally, this means there are races against rmmod, close(),
> > etc.

After discussing it with Aristeu, we came to the conclusion that it's
not dangerous. The generic backlight code can not be compiled as a
module. If a framebuffer driver unloads, it'll set pmac_backlight to
NULL. The workers check for it and don't do anything if true. So, if the
worker is called after unloading the module, it won't do anything.

Here's the other patch which fixes a little error in the patch you just
added to your tree. Without the patch, pbbuttonsd and other applications
changing the backlight will fail.

---
diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc2-git4.orig/arch/powerpc/platforms/powermac/backlight.c linux-2.6.18-rc2-git4/arch/powerpc/platforms/powermac/backlight.c
--- linux-2.6.18-rc2-git4.orig/arch/powerpc/platforms/powermac/backlight.c	2006-07-25 20:51:05.000000000 +0200
+++ linux-2.6.18-rc2-git4/arch/powerpc/platforms/powermac/backlight.c	2006-07-25 20:52:02.000000000 +0200
@@ -140,9 +140,6 @@ static int __pmac_backlight_set_legacy_b
 {
 	int error = -ENXIO;
 
-	if (atomic_read(&kernel_backlight_disabled))
-		return -EBUSY;
-
 	mutex_lock(&pmac_backlight_mutex);
 	if (pmac_backlight) {
 		struct backlight_properties *props;
@@ -170,11 +167,17 @@ static int __pmac_backlight_set_legacy_b
 
 static void pmac_backlight_set_legacy_worker(void *data)
 {
+	if (atomic_read(&kernel_backlight_disabled))
+		return;
+
 	__pmac_backlight_set_legacy_brightness(pmac_backlight_set_legacy_queued);
 }
 
 /* This function is called in interrupt context */
 void pmac_backlight_set_legacy_brightness_pmu(int brightness) {
+	if (atomic_read(&kernel_backlight_disabled))
+		return;
+
 	pmac_backlight_set_legacy_queued = brightness;
 	schedule_work(&pmac_backlight_set_legacy_work);
 }