[PATCH 12/20] Drop FIQ dependency for GTA01 configuration.

Werner Almesberger werner at openmoko.org
Fri Oct 3 06:33:06 CEST 2008


Jonas Bonn wrote:
> +#ifdef CONFIG_MACH_NEO1973_GTA02
>  #include <asm/arch-s3c2410/fiq_ipc_gta02.h>
> +#endif
...
> +#ifdef CONFIG_MACH_NEO1973_GTA02
>  	if (machine_is_neo1973_gta02()) { /* use FIQ to control GPIO */
>  		fiq_ipc.vib_pwm = value; /* set it for FIQ */
>  		fiq_kick(); /* start up FIQs if not already going */
>  		return;
>  	}
> +#endif

Please don't do this. This kind of #ifdef flood is considered extremely
bad coding practice, because if makes the code hard to read and hides
subtle build issues.

The correct way of dealing with such dependencies is to make sure
they're always satisfied in the "main" code, and the #ifdef part is
done "en bloc" in a single location, preferably in a header file.

For example, if you need a GPIO_GTA02_FOO, then you'd include both the
GTA01 and the GTA02 definitions, and use machine_is_neo1973_gta02 to
decide whether to include the code or not.

include/asm-arm/mach-types.h defined machine_is_neo1973_gta02 as (0) if
CONFIG_MACH_NEO1973_GTA02 is not set, so everything that's

	if (machine_is_neo1973_gta02()) ...

is automatically removed by the compiler. All you have to worry about
is to make sure the (unused) declarations referenced by that code are
available. (As a test, you could remove the definition and see that
there's really not references in the object files.)

fiq_ipc is a bit more complicated. You could move those operations
into a set of (possibly inline) functions that are then controlled by
a large #ifdef. E.g.,

#ifdef CONFIG_MACH_NEO1973_GTA02

static inline void do_fiq_foo(...)
{
	... stuff ...
}

...

#else /* CONFIG_MACH_NEO1973_GTA02 */

static inline void do_fiq_foo(...)
{
	/* no-op */
}

#endif /* !CONFIG_MACH_NEO1973_GTA02 */

This type of construct is very commonly used in the kernel. E.g., see
how spinlocks are enabled and disabled depending on whether the kernel
is built for SMP. You certainly don't see a gazillion of

#ifdef CONFIG_SMP
	spin_lock(something);
#endif

in the code, and we're all happier for it :-)

- Werner



More information about the openmoko-kernel mailing list