Subject: Re: [PATCH/RFC] CPM1: implement GPIO API

From: Arnd Bergmann <[email-masked]>
Date: Wed, 12 Dec 2007 23:16:33 +0100

On Wednesday 12 December 2007, Jochen Friedrich wrote:

> +static spinlock_t *cpm1_port_locks;
> +static int cpm1_num_ports;

Having an array of spinlocks is rather unusual and normally not necessary.
Did you measure a significant performance impact by using a global lock
for all ports?

If not, I would recommend simplifying this.

> +EXPORT_SYMBOL_GPL(gpio_request);
> +EXPORT_SYMBOL_GPL(gpio_direction_input);
> +EXPORT_SYMBOL_GPL(gpio_direction_output);
> +EXPORT_SYMBOL_GPL(gpio_get_value);
> +EXPORT_SYMBOL_GPL(gpio_set_value);

All these function names are rather generic identifiers, but you export them
from a platform specific file. I'd say they should either have a more specific
name space (e.g. cpm1_gpio_request), or should be implemented in a more
generic way.

	Arnd <><