Subject: Re: [PATCH 01/11] [POWERPC] Implement support for the GPIO LIB API

From: David Brownell <[email-masked]>
Date: Sun, 3 Feb 2008 13:17:43 -0800

On Sunday 03 February 2008, Anton Vorontsov wrote:
> This patch implements support for the GPIO LIB API. Two calls
> unimplemented though: irq_to_gpio and gpio_to_irq.

Also gpio_cansleep().

> +Example of two SOC GPIO banks defined as gpio-controller nodes:
> +
> +	qe_pio_a: gpio-controller@1400 {
> +		#gpio-cells = <2>;
> +		compatible = "fsl,qe-pario-bank";
> +		reg = <0x1400 0x18>;
> +		gpio-controller;
> +	};
> +
> +	qe_pio_e: gpio-controller@1460 {
> +		#gpio-cells = <2>;
> +		compatible = "fsl,qe-pario-bank";
> +		reg = <0x1460 0x18>;
> +		gpio-controller;
> +	};

Let me suggest another example to provide:   an I2C GPIO expander
such as a pcf8574 or pca9354 (both eight bit expanders).  The SOC
case is probably the easiest to cover.

> +#define ARCH_OF_GPIOS_PER_CHIP 32

Well, ARCH_OF_* is clearly not the now-removed ARCH_GPIOS_PER_CHIP,
but I still suggest moving away from that concept.  

> +static inline int gpio_get_value(unsigned int gpio)
> +{
> +	return __gpio_get_value(gpio);
> +}
> +
> +static inline void gpio_set_value(unsigned int gpio, int value)
> +{
> +	__gpio_set_value(gpio, value);
> +}

static inline int gpio_cansleep(unsigned int gpio)
{
	return __gpio_cansleep(gpio);
}

> +static inline int irq_to_gpio(unsigned int irq)
> +{
> +	return -ENOSYS;

Minor nit:  "-EINVAL" would be better here, since the argument
is constrained to have been returned from gpio_to_irq(), and
as you wrote that call there can be no such values.