diff mbox

[U-Boot,1/2] mpc83xx: Add a GPIO driver for the MPC83XX family

Message ID 1312929090-29375-1-git-send-email-joe.hershberger@ni.com
State Superseded, archived
Delegated to: Kim Phillips
Headers show

Commit Message

Joe Hershberger Aug. 9, 2011, 10:31 p.m. UTC
Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
Cc: Joe Hershberger <joe.hershberger@gmail.com>
Cc: Kim Phillips <kim.phillips@freescale.com>
---
 arch/powerpc/include/asm/arch-mpc83xx/gpio.h |   37 +++++
 arch/powerpc/include/asm/gpio.h              |   38 +++++
 board/freescale/mpc8313erdb/mpc8313erdb.c    |    9 +
 drivers/gpio/Makefile                        |    1 +
 drivers/gpio/mpc83xx_gpio.c                  |  202 ++++++++++++++++++++++++++
 5 files changed, 287 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/include/asm/arch-mpc83xx/gpio.h
 create mode 100644 arch/powerpc/include/asm/gpio.h
 create mode 100644 drivers/gpio/mpc83xx_gpio.c

Comments

Scott Wood Aug. 9, 2011, 10:15 p.m. UTC | #1
On 08/09/2011 05:31 PM, Joe Hershberger wrote:
> diff --git a/arch/powerpc/include/asm/arch-mpc83xx/gpio.h b/arch/powerpc/include/asm/arch-mpc83xx/gpio.h
> new file mode 100644
> index 0000000..4319d07
> --- /dev/null
> +++ b/arch/powerpc/include/asm/arch-mpc83xx/gpio.h
[snip
> +void gpio_init_f(void);
> +void gpio_init_r(void);

These are quite generic function names for 83xx to claim.  If it's to be
part of the generic gpio API, why not add it to asm/gpio.h?

> @@ -140,6 +146,9 @@ void board_init_f(ulong bootflag)
>  
>  void board_init_r(gd_t *gd, ulong dest_addr)
>  {
> +#if defined(CONFIG_MPC83XX_GPIO) && !defined(CONFIG_NAND_SPL)
> +	gpio_init_r();
> +#endif
>  	nand_boot();
>  }

This instance of board_init_r() is only for CONFIG_NAND_SPL, so this
won't do anything.

Maybe create a board_early_init_r() for this board?

-Scott
Joe Hershberger Aug. 9, 2011, 10:23 p.m. UTC | #2
On Tue, Aug 9, 2011 at 5:15 PM, Scott Wood <scottwood@freescale.com> wrote:
> On 08/09/2011 05:31 PM, Joe Hershberger wrote:
>> diff --git a/arch/powerpc/include/asm/arch-mpc83xx/gpio.h b/arch/powerpc/include/asm/arch-mpc83xx/gpio.h
>> new file mode 100644
>> index 0000000..4319d07
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/arch-mpc83xx/gpio.h
> [snip
>> +void gpio_init_f(void);
>> +void gpio_init_r(void);
>
> These are quite generic function names for 83xx to claim.  If it's to be
> part of the generic gpio API, why not add it to asm/gpio.h?

That's a good point.  I'll make them board specific since the board
init must call them (there isn't a generic place that will call them).

>> @@ -140,6 +146,9 @@ void board_init_f(ulong bootflag)
>>
>>  void board_init_r(gd_t *gd, ulong dest_addr)
>>  {
>> +#if defined(CONFIG_MPC83XX_GPIO) && !defined(CONFIG_NAND_SPL)
>> +     gpio_init_r();
>> +#endif
>>       nand_boot();
>>  }
>
> This instance of board_init_r() is only for CONFIG_NAND_SPL, so this
> won't do anything.
>
> Maybe create a board_early_init_r() for this board?

Sounds good.

-Joe
Kim Phillips Aug. 9, 2011, 10:54 p.m. UTC | #3
On Tue, 9 Aug 2011 17:31:29 -0500
Joe Hershberger <joe.hershberger@ni.com> wrote:

> diff --git a/arch/powerpc/include/asm/gpio.h b/arch/powerpc/include/asm/gpio.h
> new file mode 100644
> index 0000000..eb071d1
> --- /dev/null
> +++ b/arch/powerpc/include/asm/gpio.h
> @@ -0,0 +1,38 @@
> +/*
> + * Copyright (c) 2011, NVIDIA Corp. All rights reserved.
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * 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
> + */
> +
> +#ifndef _GPIO_H_
> +#define _GPIO_H_
> +
> +#include <asm/arch/gpio.h>
> +/*
> + * Generic GPIO API
> + */
> +
> +int gpio_request(int gp, const char *label);
> +void gpio_free(int gp);
> +void gpio_toggle_value(int gp);
> +int gpio_direction_input(int gp);
> +int gpio_direction_output(int gp, int value);
> +int gpio_get_value(int gp);
> +void gpio_set_value(int gp, int value);
> +
> +#endif	/* _GPIO_H_ */

looks like this GPIO API-definition file is starting to be copied
for each arch, and there's nothing arch-specific in it. As such, it
probably belongs in include/asm-generic/, not arch/powerpc.  Other
arches need fixing in this area, too.

> +/* set GPIO pin 'gp' as an input */
> +int gpio_direction_input(int gp)
> +{
> +	volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;

no volatile

> +	unsigned int ctrlr;
> +	unsigned int line;
> +	unsigned int lineMask;

no camelCase

> +
> +	/* 32-bits per controller */
> +	ctrlr = gp >> 5;
> +	line = gp & (0x1F);
> +
> +	/* Big endian */
> +	lineMask = 1 << (31 - line);
> +
> +	im->gpio[ctrlr].dir &= ~lineMask;

must use i/o accessors and/or clr/setbits_be32, please fix
everywhere.

> +	/* Update the local output buffer soft copy */
> +	gpio_output_value[ctrlr] =
> +		(gpio_output_value[ctrlr] & ~lineMask) | (value ? lineMask : 0);

what's the use of having a local output buffer soft copy?  I don't
see it being used anywhere.

Kim
Joe Hershberger Aug. 9, 2011, 11:03 p.m. UTC | #4
On Tue, Aug 9, 2011 at 5:54 PM, Kim Phillips <kim.phillips@freescale.com> wrote:
> On Tue, 9 Aug 2011 17:31:29 -0500 Joe Hershberger <joe.hershberger@ni.com> wrote:
>
>> +     /* Update the local output buffer soft copy */
>> +     gpio_output_value[ctrlr] =
>> +             (gpio_output_value[ctrlr] & ~lineMask) | (value ? lineMask : 0);
>
> what's the use of having a local output buffer soft copy?  I don't
> see it being used anywhere.

It's used in the line you quoted.  If other lines on the same
controller are configured for open-collector, but are externally
pulled low, then using the DAT register instead of the soft copy will
force those to be actively grounded.  To avoid this, I use a
soft-copy.

-Joe
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/arch-mpc83xx/gpio.h b/arch/powerpc/include/asm/arch-mpc83xx/gpio.h
new file mode 100644
index 0000000..4319d07
--- /dev/null
+++ b/arch/powerpc/include/asm/arch-mpc83xx/gpio.h
@@ -0,0 +1,37 @@ 
+/*
+ * 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
+ */
+
+#ifndef _MPC83XX_GPIO_H_
+#define _MPC83XX_GPIO_H_
+
+/*
+ * The MCP83xx's 1-2 GPIO controllers each with 32 bits.
+ */
+#if defined(CONFIG_MPC8313) || defined(CONFIG_MPC8308) || defined(CONFIG_MPC8315)
+	#define MPC83XX_GPIO_CTRLRS 1
+#elif defined(CONFIG_MPC834x) || defined(CONFIG_MPC837x)
+	#define MPC83XX_GPIO_CTRLRS 2
+#else
+	#define MPC83XX_GPIO_CTRLRS 0
+#endif
+
+#define MAX_NUM_GPIOS		(32 * MPC83XX_GPIO_CTRLRS)
+
+void gpio_init_f(void);
+void gpio_init_r(void);
+
+#endif	/* MPC83XX_GPIO_H_ */
diff --git a/arch/powerpc/include/asm/gpio.h b/arch/powerpc/include/asm/gpio.h
new file mode 100644
index 0000000..eb071d1
--- /dev/null
+++ b/arch/powerpc/include/asm/gpio.h
@@ -0,0 +1,38 @@ 
+/*
+ * Copyright (c) 2011, NVIDIA Corp. All rights reserved.
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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
+ */
+
+#ifndef _GPIO_H_
+#define _GPIO_H_
+
+#include <asm/arch/gpio.h>
+/*
+ * Generic GPIO API
+ */
+
+int gpio_request(int gp, const char *label);
+void gpio_free(int gp);
+void gpio_toggle_value(int gp);
+int gpio_direction_input(int gp);
+int gpio_direction_output(int gp, int value);
+int gpio_get_value(int gp);
+void gpio_set_value(int gp, int value);
+
+#endif	/* _GPIO_H_ */
diff --git a/board/freescale/mpc8313erdb/mpc8313erdb.c b/board/freescale/mpc8313erdb/mpc8313erdb.c
index 08f873d..353b9f4 100644
--- a/board/freescale/mpc8313erdb/mpc8313erdb.c
+++ b/board/freescale/mpc8313erdb/mpc8313erdb.c
@@ -31,6 +31,9 @@ 
 #include <vsc7385.h>
 #include <ns16550.h>
 #include <nand.h>
+#if defined(CONFIG_MPC83XX_GPIO) && !defined(CONFIG_NAND_SPL)
+#include <asm/gpio.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -42,6 +45,9 @@  int board_early_init_f(void)
 	if (im->pmc.pmccr1 & PMCCR1_POWER_OFF)
 		gd->flags |= GD_FLG_SILENT;
 #endif
+#if defined(CONFIG_MPC83XX_GPIO) && !defined(CONFIG_NAND_SPL)
+	gpio_init_f();
+#endif
 
 	return 0;
 }
@@ -140,6 +146,9 @@  void board_init_f(ulong bootflag)
 
 void board_init_r(gd_t *gd, ulong dest_addr)
 {
+#if defined(CONFIG_MPC83XX_GPIO) && !defined(CONFIG_NAND_SPL)
+	gpio_init_r();
+#endif
 	nand_boot();
 }
 
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 62ec97d..563b8ec 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -33,6 +33,7 @@  COBJS-$(CONFIG_PCA953X)		+= pca953x.o
 COBJS-$(CONFIG_S5P)		+= s5p_gpio.o
 COBJS-$(CONFIG_TEGRA2_GPIO)	+= tegra2_gpio.o
 COBJS-$(CONFIG_DA8XX_GPIO)	+= da8xx_gpio.o
+COBJS-$(CONFIG_MPC83XX_GPIO)	+= mpc83xx_gpio.o
 
 COBJS	:= $(COBJS-y)
 SRCS 	:= $(COBJS:.o=.c)
diff --git a/drivers/gpio/mpc83xx_gpio.c b/drivers/gpio/mpc83xx_gpio.c
new file mode 100644
index 0000000..4f100b9
--- /dev/null
+++ b/drivers/gpio/mpc83xx_gpio.c
@@ -0,0 +1,202 @@ 
+/*
+ * Freescale MPC83xx GPIO handling.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 <common.h>
+#include <mpc83xx.h>
+#include <asm/gpio.h>
+
+#ifndef MPC83XX_GPIO_0_INIT_DIRECTION
+	#define MPC83XX_GPIO_0_INIT_DIRECTION 0
+#endif
+#ifndef MPC83XX_GPIO_1_INIT_DIRECTION
+	#define MPC83XX_GPIO_1_INIT_DIRECTION 0
+#endif
+#ifndef MPC83XX_GPIO_0_INIT_OPEN_DRAIN
+	#define MPC83XX_GPIO_0_INIT_OPEN_DRAIN 0
+#endif
+#ifndef MPC83XX_GPIO_1_INIT_OPEN_DRAIN
+	#define MPC83XX_GPIO_1_INIT_OPEN_DRAIN 0
+#endif
+#ifndef MPC83XX_GPIO_0_INIT_VALUE
+	#define MPC83XX_GPIO_0_INIT_VALUE 0
+#endif
+#ifndef MPC83XX_GPIO_1_INIT_VALUE
+	#define MPC83XX_GPIO_1_INIT_VALUE 0
+#endif
+
+static unsigned int gpio_output_value[MPC83XX_GPIO_CTRLRS];
+
+/*
+ * Generic_GPIO primitives.
+ */
+
+int gpio_request(int gp, const char *label)
+{
+	if (gp >= MAX_NUM_GPIOS)
+		return -1;
+
+	return 0;
+}
+
+void gpio_free(int gp)
+{
+}
+
+/* set GPIO pin 'gp' as an input */
+int gpio_direction_input(int gp)
+{
+	volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
+	unsigned int ctrlr;
+	unsigned int line;
+	unsigned int lineMask;
+
+	/* 32-bits per controller */
+	ctrlr = gp >> 5;
+	line = gp & (0x1F);
+
+	/* Big endian */
+	lineMask = 1 << (31 - line);
+
+	im->gpio[ctrlr].dir &= ~lineMask;
+
+	return 0;
+}
+
+void gpio_toggle_value(int gp)
+{
+	gpio_set_value(gp, !gpio_get_value(gp));
+}
+
+/* set GPIO pin 'gp' as an output, with polarity 'value' */
+int gpio_direction_output(int gp, int value)
+{
+	volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
+	unsigned int ctrlr;
+	unsigned int line;
+	unsigned int lineMask;
+
+	if (value != 0 && value != 1) {
+		printf("Error: Value parameter must be 0 or 1.\n");
+		return -1;
+	}
+
+	gpio_set_value(gp, value);
+
+	/* 32-bits per controller */
+	ctrlr = gp >> 5;
+	line = gp & (0x1F);
+
+	/* Big endian */
+	lineMask = 1 << (31 - line);
+
+	/* Make the line output */
+	im->gpio[ctrlr].dir |= lineMask;
+
+	return 0;
+}
+
+/* read GPIO IN value of pin 'gp' */
+int gpio_get_value(int gp)
+{
+	volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
+	unsigned int ctrlr;
+	unsigned int line;
+	unsigned int lineMask;
+
+	/* 32-bits per controller */
+	ctrlr = gp >> 5;
+	line = gp & (0x1F);
+
+	/* Big endian */
+	lineMask = 1 << (31 - line);
+
+	/* Read the value and mask off the bit */
+	return (im->gpio[ctrlr].dat & lineMask) != 0;
+}
+
+/* write GPIO OUT value to pin 'gp' */
+void gpio_set_value(int gp, int value)
+{
+	volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
+	unsigned int ctrlr;
+	unsigned int line;
+	unsigned int lineMask;
+
+	if (value != 0 && value != 1) {
+		printf("Error: Value parameter must be 0 or 1.\n");
+		return;
+	}
+
+	/* 32-bits per controller */
+	ctrlr = gp >> 5;
+	line = gp & (0x1F);
+
+	/* Big endian */
+	lineMask = 1 << (31 - line);
+
+	/* Update the local output buffer soft copy */
+	gpio_output_value[ctrlr] =
+		(gpio_output_value[ctrlr] & ~lineMask) | (value ? lineMask : 0);
+
+	/* Write the output */
+	im->gpio[ctrlr].dat = gpio_output_value[ctrlr];
+}
+
+/* Configure GPIO registers early */
+void gpio_init_f()
+{
+	volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR;
+
+#if MPC83XX_GPIO_CTRLRS >= 1
+	gpio_output_value[0] = MPC83XX_GPIO_0_INIT_VALUE;
+
+	im->gpio[0].dir = MPC83XX_GPIO_0_INIT_DIRECTION;
+	im->gpio[0].odr = MPC83XX_GPIO_0_INIT_OPEN_DRAIN;
+	im->gpio[0].dat = MPC83XX_GPIO_0_INIT_VALUE;
+	im->gpio[0].ier = 0xFFFFFFFF; /* Clear all events */
+	im->gpio[0].imr = 0;
+	im->gpio[0].icr = 0;
+#endif
+
+#if MPC83XX_GPIO_CTRLRS >= 2
+	gpio_output_value[1] = MPC83XX_GPIO_1_INIT_VALUE;
+
+	im->gpio[1].dir = MPC83XX_GPIO_1_INIT_DIRECTION;
+	im->gpio[1].odr = MPC83XX_GPIO_1_INIT_OPEN_DRAIN;
+	im->gpio[1].dat = MPC83XX_GPIO_1_INIT_VALUE;
+	im->gpio[1].ier = 0xFFFFFFFF; /* Clear all events */
+	im->gpio[1].imr = 0;
+	im->gpio[1].icr = 0;
+#endif
+}
+
+/* Initialize GPIO soft-copies */
+void gpio_init_r()
+{
+#if MPC83XX_GPIO_CTRLRS >= 1
+	gpio_output_value[0] = MPC83XX_GPIO_0_INIT_VALUE;
+#endif
+
+#if MPC83XX_GPIO_CTRLRS >= 2
+	gpio_output_value[1] = MPC83XX_GPIO_1_INIT_VALUE;
+#endif
+}