[PATCH] gta02-pm-gps.patch

Werner Almesberger werner at openmoko.org
Wed Feb 20 07:19:20 CET 2008


Willie wrote:
> This is GPS driver for GTA02. It deals with power on/off GPS module.
> The GPS data can be generated through UART1.

Thanks, I'll do a merging round later today.

One coding comment: a while ago, I've suggested this structure ...

> +#ifdef CONFIG_MACH_NEO1973_GTA01
> +	if (machine_is_neo1973_gta01())
> +		s3c2410_gpio_setpin(GTA01_GPIO_GPS_PWRON, on);
> +#endif /* CONFIG_MACH_NEO1973_GTA01 */

... which you've now also adopted. Thanks for that !

Using this structure makes you code correct, but it still creates lots
of ugly #ifdefs. So we should now move one step further.

This is how we can get rid of these #ifdefs:

- always include all the header files we reference (e.g., 50606 and
  50633, even if we're only building for GTA01)

- put the #ifdef CONFIG_MACH_NEO1973_... where machine_is_neo1973_...
  is defined, like this:

	#ifdef CONFIG_MACH_NEO1973_GTA02
	... /* roughly what we have now */ ...
	#else
	#define machine_is_neo1973_gta02	0
	#endif

- in the .c file, just test like this:

	if (machine_is_neo1973_gta02()) {
		...
	}

If CONFIG_MACH_NEO1973_GTA02 is not set, this becomes

	if (0) {
		...
	}

which gcc immediately recognizes as code it can remove. Thus no code
gets actually generated for items we don't need, and the source is
much more readable.

(I didn't make all this up just now. The general concept of trusting
gcc to do basic optimizations, and to actively use them, has been
good kernel coding practice for a while.)

Since we don't have too much code that uses this structure yet, I
think the easiest approach will be if I convert all this later today
or tomorrow, and do a few test builds to make sure I didn't overlook
any quirks. We can then use that code as an example for how to avoid
this sort of armies of #ifdefs.

- Werner




More information about the openmoko-kernel mailing list