Re: details, details ...

[Arnd Bergmann <arnd@arndb.de>]

On Wednesday 13 August 2008, Kevin Diggs wrote:
> In cpu exit function of a cpufreq driver:
>=20
> =A0 =A0 =A0 =A0 =A0while (test_bit(cf750gxmChangingPllBit, &cf750gxvState=
Bits))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0msleep(1);
>=20
> This bit will get cleared by a notifier callback.
>=20
> In module_exit function of a related module:
>=20
> =A0 =A0 =A0 =A0 =A0while (test_bit(PLL_LOCK_BIT, (unsigned long *)&boot_r=
atio)) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0msleep(1);
> =A0 =A0 =A0 =A0 =A0}
>=20
> This bit will get cleared by a timer. It will also fire the notifiers=20
> needed above.
>=20
> 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:
>=20
> ERROR: do not initialise externals to 0 or NULL
> #2483: FILE: arch/powerpc/kernel/cpu/pll_if.c:486:
> +int rval =3D 0;
>=20
> 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 =3D 0;
>=20
> 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 <><