public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] GEODE: update GPIO API to support setting multiple GPIOs at once
@ 2007-10-18 19:27 Andres Salomon
  2007-10-18 19:36 ` Jordan Crouse
  0 siblings, 1 reply; 2+ messages in thread
From: Andres Salomon @ 2007-10-18 19:27 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jordan Crouse, info-linux, Thomas Gleixner

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@debian.org>

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@debian.org>
  *
  * 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);
 

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: GEODE: update GPIO API to support setting multiple GPIOs at once
  2007-10-18 19:27 [PATCH] GEODE: update GPIO API to support setting multiple GPIOs at once Andres Salomon
@ 2007-10-18 19:36 ` Jordan Crouse
  0 siblings, 0 replies; 2+ messages in thread
From: Jordan Crouse @ 2007-10-18 19:36 UTC (permalink / raw)
  To: Andres Salomon; +Cc: linux-kernel, info-linux, Thomas Gleixner

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@debian.org>

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

> 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@debian.org>
>   *
>   * 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.



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2007-10-18 23:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-18 19:27 [PATCH] GEODE: update GPIO API to support setting multiple GPIOs at once Andres Salomon
2007-10-18 19:36 ` Jordan Crouse

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox