diff mbox

[U-Boot,1/3] mpc85xx: Add inline GPIO acessor functions

Message ID 1318981284-4357-2-git-send-email-Kyle.D.Moffett@boeing.com
State Changes Requested
Headers show

Commit Message

Kyle Moffett Oct. 18, 2011, 11:41 p.m. UTC
To ease the implementation of other MPC85xx board ports, several common
GPIO helpers are added to <asm/mpc85xx_gpio.h>.

Since each of these compiles to no more than 4-5 instructions it would
be very inefficient to call them out of line, therefore we put them
entirely in the header file.

The HWW-1U-1A board port which these were written for strongly prefers
to set multiple GPIOs as a single batch operation, so the API is
designed around that basis.

To assist other board ports, a small set of wrappers are used which
provides a standard gpio_request() interface around the MPC85xx-specific
functions.  This can be enabled with CONFIG_MPC85XX_GENERIC_GPIO

Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
Cc: Andy Fleming <afleming@gmail.com>
Cc: Kumar Gala <kumar.gala@freescale.com>
Cc: Peter Tyser <ptyser@xes-inc.com>

--

Changelog:
v2: Moved the inline functions to a non-board-specific header
v3: Added generic Linux-standard GPIO wrappers
v4: Improved comments and fixed minor bugs in the wrapper functions
v5: No changes
v6: Rebased onto the 'next' branch of git://git.denx.de/u-boot-mpc85xx.git
v7: No changes
v8: Rebased onto latest HEAD and add README entry for the config option
---
 README                                  |    5 ++
 arch/powerpc/include/asm/mpc85xx_gpio.h |  120 +++++++++++++++++++++++++++++++
 2 files changed, 125 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/include/asm/mpc85xx_gpio.h

Comments

Mike Frysinger Oct. 19, 2011, 3:19 a.m. UTC | #1
On Tuesday 18 October 2011 19:41:22 Kyle Moffett wrote:
> --- a/README
> +++ b/README
> 
>  - 85xx CPU Options:
> +		CONFIG_MPC85XX_GENERIC_GPIO
> +
> +		Provide a generic gpio_request() interface to the onboard
> +		processor GPIOs in the MPC85XX-series chips.

do you really need a CONFIG knob for this ?  defining this adds no overhead 
that i can see.

> --- /dev/null
> +++ b/arch/powerpc/include/asm/mpc85xx_gpio.h

missing #ifdef protection against multiple inclusion
-mike
Kumar Gala Oct. 19, 2011, 1:51 p.m. UTC | #2
On Oct 18, 2011, at 10:19 PM, Mike Frysinger wrote:

> On Tuesday 18 October 2011 19:41:22 Kyle Moffett wrote:
>> --- a/README
>> +++ b/README
>> 
>> - 85xx CPU Options:
>> +		CONFIG_MPC85XX_GENERIC_GPIO
>> +
>> +		Provide a generic gpio_request() interface to the onboard
>> +		processor GPIOs in the MPC85XX-series chips.
> 
> do you really need a CONFIG knob for this ?  defining this adds no overhead 
> that i can see.

I agree, wasn't paying attention that this is just in a .h, so lets remove the CONFIG_ option bits (both in README and in .h)

>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/mpc85xx_gpio.h
> 
> missing #ifdef protection against multiple inclusion

please fix.

- k
Kyle Moffett Oct. 19, 2011, 6:21 p.m. UTC | #3
On Oct 19, 2011, at 09:51, Kumar Gala wrote:
> On Oct 18, 2011, at 10:19 PM, Mike Frysinger wrote:
>> On Tuesday 18 October 2011 19:41:22 Kyle Moffett wrote:
>>> --- a/README
>>> +++ b/README
>>> 
>>> - 85xx CPU Options:
>>> +		CONFIG_MPC85XX_GENERIC_GPIO
>>> +
>>> +		Provide a generic gpio_request() interface to the onboard
>>> +		processor GPIOs in the MPC85XX-series chips.
>> 
>> do you really need a CONFIG knob for this ?  defining this adds no overhead 
>> that i can see.
> 
> I agree, wasn't paying attention that this is just in a .h, so lets remove the CONFIG_ option bits (both in README and in .h)

Well, I was concerned that this API could conflict with another provider
of the generic GPIO interface (IE: An I2C GPIO chip or whatever).

Looking further it appears that only 3-4 drivers implement the generic
GPIO interface, and all of those are CPU-specific, so it should not be a
problem in practice.

The reason I originally made this configurable was because I did not
want or need the generic GPIO interface; I needed an API that let me set
a large group of GPIOs simultaneously.  During an early review it was
asked why I didn't use the standard interface; so I implemented the
simple wrapper functions.

I've removed the CONFIG_* option and will be resubmitting that patch shortly.

>>> --- /dev/null
>>> +++ b/arch/powerpc/include/asm/mpc85xx_gpio.h
>> 
>> missing #ifdef protection against multiple inclusion
> 
> please fix.

Really?  Oops!  I have no idea how I missed that.  Fixed.

Cheers,
Kyle Moffett

--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
diff mbox

Patch

diff --git a/README b/README
index 7e032a9..91412f2 100644
--- a/README
+++ b/README
@@ -359,6 +359,11 @@  The following options need to be configured:
 		ICache only when Code runs from RAM.
 
 - 85xx CPU Options:
+		CONFIG_MPC85XX_GENERIC_GPIO
+
+		Provide a generic gpio_request() interface to the onboard
+		processor GPIOs in the MPC85XX-series chips.
+
 		CONFIG_SYS_FSL_TBCLK_DIV
 
 		Defines the core time base clock divider ratio compared to the
diff --git a/arch/powerpc/include/asm/mpc85xx_gpio.h b/arch/powerpc/include/asm/mpc85xx_gpio.h
new file mode 100644
index 0000000..ad54b4e
--- /dev/null
+++ b/arch/powerpc/include/asm/mpc85xx_gpio.h
@@ -0,0 +1,120 @@ 
+/*
+ * Copyright 2010 eXMeritus, A Boeing Company
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <asm/immap_85xx.h>
+
+/*
+ * The following internal functions are an MPC85XX-specific GPIO API which
+ * allows setting and querying multiple GPIOs in a single operation.
+ *
+ * All of these look relatively large, but the arguments are almost always
+ * constants, so they compile down to just a few instructions and a
+ * memory-mapped IO operation or two.
+ */
+static inline void mpc85xx_gpio_set(unsigned int mask,
+		unsigned int dir, unsigned int val)
+{
+	ccsr_gpio_t *gpio = (void *)(CONFIG_SYS_MPC85xx_GPIO_ADDR + 0xc00);
+
+	/* First mask off the unwanted parts of "dir" and "val" */
+	dir &= mask;
+	val &= mask;
+
+	/* Now read in the values we're supposed to preserve */
+	dir |= (in_be32(&gpio->gpdir) & ~mask);
+	val |= (in_be32(&gpio->gpdat) & ~mask);
+
+	/*
+	 * Poke the new output values first, then set the direction.  This
+	 * helps to avoid transient data when switching from input to output
+	 * and vice versa.
+	 */
+	out_be32(&gpio->gpdat, val);
+	out_be32(&gpio->gpdir, dir);
+}
+
+static inline void mpc85xx_gpio_set_in(unsigned int gpios)
+{
+	mpc85xx_gpio_set(gpios, 0x00000000, 0x00000000);
+}
+
+static inline void mpc85xx_gpio_set_low(unsigned int gpios)
+{
+	mpc85xx_gpio_set(gpios, 0xFFFFFFFF, 0x00000000);
+}
+
+static inline void mpc85xx_gpio_set_high(unsigned int gpios)
+{
+	mpc85xx_gpio_set(gpios, 0xFFFFFFFF, 0xFFFFFFFF);
+}
+
+static inline unsigned int mpc85xx_gpio_get(unsigned int mask)
+{
+	ccsr_gpio_t *gpio = (void *)(CONFIG_SYS_MPC85xx_GPIO_ADDR + 0xc00);
+
+	/* Read the requested values */
+	return in_be32(&gpio->gpdat) & mask;
+}
+
+/*
+ * These implement the generic Linux GPIO API on top of the other functions
+ * in this header.
+ */
+#ifdef CONFIG_MPC85XX_GENERIC_GPIO
+static inline int gpio_request(unsigned gpio, const char *label)
+{
+	/* Compatibility shim */
+	return 0;
+}
+
+static inline void gpio_free(unsigned gpio)
+{
+	/* Compatibility shim */
+}
+
+static inline int gpio_direction_input(unsigned gpio)
+{
+	mpc85xx_gpio_set_in(1U << gpio);
+	return 0;
+}
+
+static inline int gpio_direction_output(unsigned gpio, int value)
+{
+	mpc85xx_gpio_set_low(1U << gpio);
+	return 0;
+}
+
+static inline int gpio_get_value(unsigned gpio)
+{
+	return !!mpc85xx_gpio_get(1U << gpio)
+}
+
+static inline void gpio_set_value(unsigned gpio, int value)
+{
+	if (value)
+		mpc85xx_gpio_set_high(1U << gpio);
+	else
+		mpc85xx_gpio_set_low(1U << gpio);
+}
+
+static inline int gpio_is_valid(int gpio)
+{
+	return (gpio >= 0) && (gpio < 32);
+}
+#endif /* CONFIG_MPC85XX_GENERIC_GPIO */