Re: GEODE: update GPIO API to support setting multiple GPIOs atonce

[Jordan Crouse]

On 18/10/07 15:27 -0400, Andres Salomon wrote:
> The existing Geode GPIO API only allows for updating one GPIO at once.  There
> are instances where users want to update multiple GPIOs at once.  With the
> current API, they are given two choices; either ignore the GPIO API:
> 
>       outl(0xc000, gpio_base + GPIO_OUTPUT_VAL);
>       outl(0xc000, gpio_base + GPIO_OUTPUT_ENABLE);
> 
> Alternatively, call each GPIO update separately:
> 
>       geode_gpio_set(14, GPIO_OUTPUT_VAL);
>       geode_gpio_set(15, GPIO_OUTPUT_VAL);
>       geode_gpio_set(14, GPIO_OUTPUT_ENABLE);
>       geode_gpio_set(15, GPIO_OUTPUT_ENABLE);
> 
> Neither are desirable.  This patch changes the GPIO API to allow for setting
> of multiple GPIOs at once; rather than being passed an integer, we pass
> a bitmask and provide a translation function.  The above code would now
> look like this:
> 
>       geode_gpio_set(geode_gpio(14)|geode_gpio(15), GPIO_OUTPUT_VAL);
>       geode_gpio_set(geode_gpio(14)|geode_gpio(15), GPIO_OUTPUT_ENABLE);
> 
> Since there are no upstream users of the GPIO API yet (afaik), best to
> change this now.  This also adds a bit of sanity checking; it is no
> longer possible to use a GPIO above 28.
> 
> 
> Note the semantics of geode_gpio_isset() have changed:
> geode_gpio_isset(geode_gpio(3)|geode_gpio(4), ...)
> will only return true iff both GPIOs are set. 
> 
> Signed-off-by: Andres Salomon <dilinger@xxxxxxxxxx>

Acked-by: Jordan Crouse <jordan.crouse@xxxxxxx>

> diff --git a/arch/x86/kernel/geode_32.c b/arch/x86/kernel/geode_32.c
> index f12d8c5..9c7f7d3 100644
> --- a/arch/x86/kernel/geode_32.c
> +++ b/arch/x86/kernel/geode_32.c
> @@ -1,6 +1,7 @@
>  /*
>   * AMD Geode southbridge support code
>   * Copyright (C) 2006, Advanced Micro Devices, Inc.
> + * Copyright (C) 2007, Andres Salomon <dilinger@xxxxxxxxxx>
>   *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of version 2 of the GNU General Public License
> @@ -51,45 +52,62 @@ EXPORT_SYMBOL_GPL(geode_get_dev_base);
>  
>  /* === GPIO API === */
>  
> -void geode_gpio_set(unsigned int gpio, unsigned int reg)
> +void geode_gpio_set(u32 gpio, unsigned int reg)
>  {
>  	u32 base = geode_get_dev_base(GEODE_DEV_GPIO);
>  
>  	if (!base)
>  		return;
>  
> -	if (gpio < 16)
> -		outl(1 << gpio, base + reg);
> -	else
> -		outl(1 << (gpio - 16), base + 0x80 + reg);
> +	/* low bank register */
> +	if (gpio & 0xFFFF)
> +		outl(gpio & 0xFFFF, base + reg);
> +	/* high bank register */
> +	gpio >>= 16;
> +	if (gpio)
> +		outl(gpio, base + 0x80 + reg);
>  }
>  EXPORT_SYMBOL_GPL(geode_gpio_set);
>  
> -void geode_gpio_clear(unsigned int gpio, unsigned int reg)
> +void geode_gpio_clear(u32 gpio, unsigned int reg)
>  {
>  	u32 base = geode_get_dev_base(GEODE_DEV_GPIO);
>  
>  	if (!base)
>  		return;
>  
> -	if (gpio < 16)
> -		outl(1 << (gpio + 16), base + reg);
> -	else
> -		outl(1 << gpio, base + 0x80 + reg);
> +	/* low bank register */
> +	if (gpio & 0xFFFF)
> +		outl((gpio & 0xFFFF) << 16, base + reg);
> +	/* high bank register */
> +	gpio &= (0xFFFF << 16);
> +	if (gpio)
> +		outl(gpio, base + 0x80 + reg);
>  }
>  EXPORT_SYMBOL_GPL(geode_gpio_clear);
>  
> -int geode_gpio_isset(unsigned int gpio, unsigned int reg)
> +int geode_gpio_isset(u32 gpio, unsigned int reg)
>  {
>  	u32 base = geode_get_dev_base(GEODE_DEV_GPIO);
> +	u32 val;
>  
>  	if (!base)
>  		return 0;
>  
> -	if (gpio < 16)
> -		return (inl(base + reg) & (1 << gpio)) ? 1 : 0;
> -	else
> -		return (inl(base + 0x80 + reg) & (1 << (gpio - 16))) ? 1 : 0;
> +	/* low bank register */
> +	if (gpio & 0xFFFF) {
> +		val = inl(base + reg) & (gpio & 0xFFFF);
> +		if ((gpio & 0xFFFF) == val)
> +			return 1;
> +	}
> +	/* high bank register */
> +	gpio >>= 16;
> +	if (gpio) {
> +		val = inl(base + 0x80 + reg) & gpio;
> +		if (gpio == val)
> +			return 1;
> +	}
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(geode_gpio_isset);
>  
> diff --git a/include/asm-x86/geode.h b/include/asm-x86/geode.h
> index d948988..593d3ec 100644
> --- a/include/asm-x86/geode.h
> +++ b/include/asm-x86/geode.h
> @@ -119,9 +119,15 @@ extern int geode_get_dev_base(unsigned int dev);
>  #define GPIO_MAP_Z		0xE8
>  #define GPIO_MAP_W		0xEC
>  
> -extern void geode_gpio_set(unsigned int, unsigned int);
> -extern void geode_gpio_clear(unsigned int, unsigned int);
> -extern int geode_gpio_isset(unsigned int, unsigned int);
> +static inline u32 geode_gpio(unsigned int nr)
> +{
> +	BUG_ON(nr > 28);
> +	return 1 << nr;
> +}
> +
> +extern void geode_gpio_set(u32, unsigned int);
> +extern void geode_gpio_clear(u32, unsigned int);
> +extern int geode_gpio_isset(u32, unsigned int);
>  extern void geode_gpio_setup_event(unsigned int, int, int);
>  extern void geode_gpio_set_irq(unsigned int, unsigned int);
>  
> 
> 

-- 
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/