details, details ...

[Arnd Bergmann]

On Wednesday 13 August 2008, Kevin Diggs wrote:
> In cpu exit function of a cpufreq driver:
> 
>          while (test_bit(cf750gxmChangingPllBit, &cf750gxvStateBits))
>                  msleep(1);
> 
> This bit will get cleared by a notifier callback.
> 
> In module_exit function of a related module:
> 
>          while (test_bit(PLL_LOCK_BIT, (unsigned long *)&boot_ratio)) {
>                  msleep(1);
>          }
> 
> This bit will get cleared by a timer. It will also fire the notifiers 
> needed above.
> 
> I don't think these are in interrupt context. The sleeps ok here?

Technically ok, but not nice. Besides the coding style issues,
it's still a busy loop that should be replaced by wait_for_completion().

> Also, from checkpatch:
> 
> ERROR: do not initialise externals to 0 or NULL
> #2483: FILE: arch/powerpc/kernel/cpu/pll_if.c:486:
> +int rval = 0;
> 
> ERROR: do not initialise statics to 0 or NULL
> #2058: FILE: arch/powerpc/kernel/cpu/pll_if.c:61:
> +static unsigned int override_bus_clock = 0;
> 
> Huh? What? Anyone care to teach an old dog a new trick???

Don't bother. Old gcc variants would put these variables into .data
instead of .bss, but with a new (less than 5 years or so) gcc, both
will result in .bss storage that is initialized to zero at boot time.

	Arnd <><