Subject: Re: powermac: More powermac backlight fixes

From: Andrew Morton <[email-masked]>
Date: Mon, 24 Jul 2006 20:03:15 -0700

On Sat, 15 Jul 2006 15:09:00 +0200
Michael Hanselmann <[email-masked]> wrote:

> This patch fixes several problems:
> - The legacy backlight value might be set at interrupt time. Introduced
>   a worker to prevent it from directly calling the backlight code.
> - via-pmu allows the backlight to be grabbed, in which case we need to
>   prevent other kernel code from changing the brightness.
> - Don't send PMU requests in via-pmu-backlight when the machine is about
>   to sleep or waking up.
> - More Kconfig fixes.
> 
> ...
>
>  static void pmac_backlight_key_worker(void *data);
> +static void pmac_backlight_set_legacy_worker(void *data);
> +
>  static DECLARE_WORK(pmac_backlight_key_work, pmac_backlight_key_worker, NULL);
> +static DECLARE_WORK(pmac_backlight_set_legacy_work, pmac_backlight_set_legacy_worker, NULL);

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.

>  
> +void pmac_backlight_disable()
> +{
> +	atomic_inc(&kernel_backlight_disabled);
> +}
> +
> +void pmac_backlight_enable()
> +{
> +	atomic_dec(&kernel_backlight_disabled);
> +}
> +

So if userspace calls ioctl(PMU_IOC_GRAB_BACKLIGHT) eleven times, eleven
enables are needed?  (Actually, eleven open()/close() sequences, I think).

Methinks you wanted just

	kernel_backlight_disabled = 1;
?