[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[Ajay Bhargav]

This patch adds generic GPIO driver framework support for Marvell SoCs.

To enable GPIO driver define CONFIG_MV_GPIO and for GPIO commands
define CONFIG_CMD_GPIO in your board configuration file.

Signed-off-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
---
 drivers/gpio/Makefile |    1 +
 drivers/gpio/mvgpio.c |  125 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/mvgpio.h      |   52 ++++++++++++++++++++
 3 files changed, 178 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/mvgpio.c
 create mode 100644 include/mvgpio.h

diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 62ec97d..c6f652f 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -28,6 +28,7 @@ LIB 	:= $(obj)libgpio.o
 COBJS-$(CONFIG_AT91_GPIO)	+= at91_gpio.o
 COBJS-$(CONFIG_KIRKWOOD_GPIO)	+= kw_gpio.o
 COBJS-$(CONFIG_MARVELL_MFP)	+= mvmfp.o
+COBJS-$(CONFIG_MV_GPIO)		+= mvgpio.o
 COBJS-$(CONFIG_MXC_GPIO)	+= mxc_gpio.o
 COBJS-$(CONFIG_PCA953X)		+= pca953x.o
 COBJS-$(CONFIG_S5P)		+= s5p_gpio.o
diff --git a/drivers/gpio/mvgpio.c b/drivers/gpio/mvgpio.c
new file mode 100644
index 0000000..d64f0db
--- /dev/null
+++ b/drivers/gpio/mvgpio.c
@@ -0,0 +1,125 @@
+/*
+ * (C) Copyright 2011
+ * eInfochips Ltd. <www.einfochips.com>
+ * Written-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
+ *
+ * (C) Copyright 2010
+ * Marvell Semiconductor <www.marvell.com>
+ *
+ * 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., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/errno.h>
+#include <asm/gpio.h>
+
+char gpio_names[MV_MAX_GPIO][GPIO_LABEL_MAX];
+
+static int get_gpio_base(int bank)
+{
+	switch (bank) {
+	case 0:
+		return GPIO_BANK0_BASE;
+	case 1:
+		return GPIO_BANK1_BASE;
+	case 2:
+		return GPIO_BANK2_BASE;
+	case 3:
+		return GPIO_BANK3_BASE;
+	}
+	return 0;
+}
+
+int gpio_request(int gp, const char *label)
+{
+	if (gp >= MV_MAX_GPIO)
+		return -EINVAL;
+
+	if (strlen(gpio_names[gp]) == 0) {
+		strncpy(gpio_names[gp], label, GPIO_LABEL_MAX);
+		gpio_names[gp][GPIO_LABEL_MAX - 1] = '\0';
+	} else {
+		return -EBUSY;
+	}
+	return 0;
+}
+
+void gpio_free(int gp)
+{
+	gpio_names[gp][0] = '\0';
+}
+
+void gpio_toggle_value(int gp)
+{
+	gpio_set_value(gp, !gpio_get_value(gp));
+}
+
+int gpio_direction_input(int gp)
+{
+	struct gpio_reg *gpio_reg_bank;
+
+	if (gp >= MV_MAX_GPIO)
+		return -EINVAL;
+
+	gpio_reg_bank = (struct gpio_reg *) get_gpio_base(GPIO_TO_REG(gp));
+	writel(GPIO_TO_BIT(gp), &gpio_reg_bank->gcdr);
+	return 0;
+}
+
+int gpio_direction_output(int gp, int value)
+{
+	struct gpio_reg *gpio_reg_bank;
+
+	if (gp >= MV_MAX_GPIO)
+		return -EINVAL;
+
+	gpio_reg_bank = (struct gpio_reg *) get_gpio_base(GPIO_TO_REG(gp));
+	writel(GPIO_TO_BIT(gp), &gpio_reg_bank->gsdr);
+	gpio_set_value(gp, value);
+	return 0;
+}
+
+int gpio_get_value(int gp)
+{
+	struct gpio_reg *gpio_reg_bank;
+	u32 gp_val;
+
+	if (gp >= MV_MAX_GPIO)
+		return -EINVAL;
+
+	gpio_reg_bank = (struct gpio_reg *) get_gpio_base(GPIO_TO_REG(gp));
+	gp_val = readl(&gpio_reg_bank->gplr);
+
+	return GPIO_VAL(gp, gp_val);
+}
+
+void gpio_set_value(int gp, int value)
+{
+	struct gpio_reg *gpio_reg_bank;
+
+	if (gp >= MV_MAX_GPIO)
+		return;
+
+	gpio_reg_bank = (struct gpio_reg *) get_gpio_base(GPIO_TO_REG(gp));
+	if (value)
+		writel(GPIO_TO_BIT(gp),	&gpio_reg_bank->gpsr);
+	else
+		writel(GPIO_TO_BIT(gp),	&gpio_reg_bank->gpcr);
+}
diff --git a/include/mvgpio.h b/include/mvgpio.h
new file mode 100644
index 0000000..7d0258f
--- /dev/null
+++ b/include/mvgpio.h
@@ -0,0 +1,52 @@
+/*
+ * (C) Copyright 2011
+ * eInfochips Ltd. <www.einfochips.com>
+ * Written-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
+ *
+ * (C) Copyright 2010
+ * Marvell Semiconductor <www.marvell.com>
+ *
+ * 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., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#ifndef _MV_GPIO_H
+#define _MV_GPIO_H
+
+#include <asm/types.h>
+
+#if defined(CONFIG_MV_GPIO)
+
+#define MV_GPIO_BASE		0xD4019000
+#define GPIO_BANK0_BASE		(MV_GPIO_BASE + 0x0000)
+#define GPIO_BANK1_BASE		(MV_GPIO_BASE + 0x0004)
+#define GPIO_BANK2_BASE		(MV_GPIO_BASE + 0x0008)
+#define GPIO_BANK3_BASE		(MV_GPIO_BASE + 0x0100)
+
+#define GPIO_LABEL_MAX		20
+#define MV_MAX_GPIO		128
+
+#define GPIO_TO_REG(gp)		(gp >> 5)
+#define GPIO_TO_BIT(gp)		(1 << (gp & 0x1F))
+#define GPIO_VAL(gp, val)	((val >> (gp & 0x1F)) & 0x01)
+
+#define GPIO_SET		1
+#define GPIO_CLR		0
+
+#endif /* CONFIG_MV_GPIO */
+#endif /* _MV_GPIO_H */
-- 
1.7.0.4

[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[Lei Wen]

Hi Ajay,

On Thu, Jul 21, 2011 at 1:39 PM, Ajay Bhargav
<ajay.bhargav@einfochips.com> wrote:
> This patch adds generic GPIO driver framework support for Marvell SoCs.
>
> To enable GPIO driver define CONFIG_MV_GPIO and for GPIO commands
> define CONFIG_CMD_GPIO in your board configuration file.
>
> Signed-off-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
> ---
> ?drivers/gpio/Makefile | ? ?1 +
> ?drivers/gpio/mvgpio.c | ?125 +++++++++++++++++++++++++++++++++++++++++++++++++
> ?include/mvgpio.h ? ? ?| ? 52 ++++++++++++++++++++
> ?3 files changed, 178 insertions(+), 0 deletions(-)
> ?create mode 100644 drivers/gpio/mvgpio.c
> ?create mode 100644 include/mvgpio.h
>
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 62ec97d..c6f652f 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -28,6 +28,7 @@ LIB ? := $(obj)libgpio.o
> ?COBJS-$(CONFIG_AT91_GPIO) ? ? ?+= at91_gpio.o
> ?COBJS-$(CONFIG_KIRKWOOD_GPIO) ?+= kw_gpio.o
> ?COBJS-$(CONFIG_MARVELL_MFP) ? ?+= mvmfp.o
> +COBJS-$(CONFIG_MV_GPIO) ? ? ? ? ? ? ? ?+= mvgpio.o
> ?COBJS-$(CONFIG_MXC_GPIO) ? ? ? += mxc_gpio.o
> ?COBJS-$(CONFIG_PCA953X) ? ? ? ? ? ? ? ?+= pca953x.o
> ?COBJS-$(CONFIG_S5P) ? ? ? ? ? ?+= s5p_gpio.o
> diff --git a/drivers/gpio/mvgpio.c b/drivers/gpio/mvgpio.c
> new file mode 100644
> index 0000000..d64f0db
> --- /dev/null
> +++ b/drivers/gpio/mvgpio.c
> @@ -0,0 +1,125 @@
> +/*
> + * (C) Copyright 2011
> + * eInfochips Ltd. <www.einfochips.com>
> + * Written-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
> + *
> + * (C) Copyright 2010
> + * Marvell Semiconductor <www.marvell.com>
> + *
> + * 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., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/errno.h>
> +#include <asm/gpio.h>
> +
> +char gpio_names[MV_MAX_GPIO][GPIO_LABEL_MAX];
> +
> +static int get_gpio_base(int bank)
> +{
> + ? ? ? switch (bank) {
> + ? ? ? case 0:
> + ? ? ? ? ? ? ? return GPIO_BANK0_BASE;
> + ? ? ? case 1:
> + ? ? ? ? ? ? ? return GPIO_BANK1_BASE;
> + ? ? ? case 2:
> + ? ? ? ? ? ? ? return GPIO_BANK2_BASE;
> + ? ? ? case 3:
> + ? ? ? ? ? ? ? return GPIO_BANK3_BASE;
> + ? ? ? }
> + ? ? ? return 0;
> +}

Please put this get_gpio_base into arch's self directory, since
different soc may have
different number of banks. For example, mmp3 has 6 banks.

Best regards,
Lei

[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[Mike Frysinger]

On Thu, Jul 21, 2011 at 01:39, Ajay Bhargav wrote:
> +char gpio_names[MV_MAX_GPIO][GPIO_LABEL_MAX];

probably want to be static ... and this functionality is optional if
you want to just ignore the labels.  for Blackfin gpio, i make it a
build option (useful for debug, not so much for production).

> +static int get_gpio_base(int bank)

if this returned a "struct gpio_reg *", you wouldnt need to cast
everything else below ...
-mike

[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[Prafulla Wadaskar]

> -----Original Message-----
> From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> Sent: Thursday, July 21, 2011 11:10 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ajay Bhargav
> Subject: [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs
> 
> This patch adds generic GPIO driver framework support for Marvell SoCs.
> 
> To enable GPIO driver define CONFIG_MV_GPIO and for GPIO commands
> define CONFIG_CMD_GPIO in your board configuration file.
> 
> Signed-off-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
> ---
>  drivers/gpio/Makefile |    1 +
>  drivers/gpio/mvgpio.c |  125
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/mvgpio.h      |   52 ++++++++++++++++++++
>  3 files changed, 178 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/gpio/mvgpio.c
>  create mode 100644 include/mvgpio.h

I don't think mvgpio.h is needed here, basically header file in include/ is intended to provide interface to other subsystems.

In this case mvgpio.c is enabling support for gpio framework that will be interfaced using asm/gpio.h.

1. You can split stuff in mvgpio.h to asm/arch/gpio.h and/or mvgpio.c
2. you can move it to drivers/gpio/

I would recommend first option.
 
> 
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 62ec97d..c6f652f 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -28,6 +28,7 @@ LIB 	:= $(obj)libgpio.o
>  COBJS-$(CONFIG_AT91_GPIO)	+= at91_gpio.o
>  COBJS-$(CONFIG_KIRKWOOD_GPIO)	+= kw_gpio.o
>  COBJS-$(CONFIG_MARVELL_MFP)	+= mvmfp.o
> +COBJS-$(CONFIG_MV_GPIO)		+= mvgpio.o

Make it CONFIG_MARVELL_GPIO to sync with other configurations i.e. CONFIG_MARVELL_MFP

>  COBJS-$(CONFIG_MXC_GPIO)	+= mxc_gpio.o
>  COBJS-$(CONFIG_PCA953X)		+= pca953x.o
>  COBJS-$(CONFIG_S5P)		+= s5p_gpio.o
> diff --git a/drivers/gpio/mvgpio.c b/drivers/gpio/mvgpio.c
> new file mode 100644
> index 0000000..d64f0db
> --- /dev/null
> +++ b/drivers/gpio/mvgpio.c
> @@ -0,0 +1,125 @@
> +/*
> + * (C) Copyright 2011
> + * eInfochips Ltd. <www.einfochips.com>
> + * Written-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
> + *
> + * (C) Copyright 2010
> + * Marvell Semiconductor <www.marvell.com>
> + *
> + * 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., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/errno.h>
> +#include <asm/gpio.h>
> +
> +char gpio_names[MV_MAX_GPIO][GPIO_LABEL_MAX];

This is overhead, what is need? Just avoid it.

> +
> +static int get_gpio_base(int bank)
> +{
> +	switch (bank) {
> +	case 0:
> +		return GPIO_BANK0_BASE;
> +	case 1:
> +		return GPIO_BANK1_BASE;
> +	case 2:
> +		return GPIO_BANK2_BASE;
> +	case 3:
> +		return GPIO_BANK3_BASE;
> +	}
> +	return 0;
> +}

As discussed, this would be preferred macro in gpio.h

> +
> +int gpio_request(int gp, const char *label)
> +{
> +	if (gp >= MV_MAX_GPIO)
> +		return -EINVAL;

Minimum fulfillment, this should be okay here.

> +
> +	if (strlen(gpio_names[gp]) == 0) {
> +		strncpy(gpio_names[gp], label, GPIO_LABEL_MAX);
> +		gpio_names[gp][GPIO_LABEL_MAX - 1] = '\0';
> +	} else {
> +		return -EBUSY;

Pls remove this.

> +	}
> +	return 0;
> +}
> +
> +void gpio_free(int gp)
> +{

Just return

> +	gpio_names[gp][0] = '\0';
> +}
> +
> +void gpio_toggle_value(int gp)
> +{
> +	gpio_set_value(gp, !gpio_get_value(gp));
> +}
> +
> +int gpio_direction_input(int gp)
> +{
> +	struct gpio_reg *gpio_reg_bank;
> +
> +	if (gp >= MV_MAX_GPIO)
> +		return -EINVAL;
> +
> +	gpio_reg_bank = (struct gpio_reg *) get_gpio_base(GPIO_TO_REG(gp));
> +	writel(GPIO_TO_BIT(gp), &gpio_reg_bank->gcdr);
> +	return 0;
> +}
> +
> +int gpio_direction_output(int gp, int value)
> +{
> +	struct gpio_reg *gpio_reg_bank;
> +
> +	if (gp >= MV_MAX_GPIO)
> +		return -EINVAL;
> +
> +	gpio_reg_bank = (struct gpio_reg *) get_gpio_base(GPIO_TO_REG(gp));
> +	writel(GPIO_TO_BIT(gp), &gpio_reg_bank->gsdr);
> +	gpio_set_value(gp, value);
> +	return 0;
> +}
> +
> +int gpio_get_value(int gp)
> +{
> +	struct gpio_reg *gpio_reg_bank;
> +	u32 gp_val;
> +
> +	if (gp >= MV_MAX_GPIO)
> +		return -EINVAL;
> +
> +	gpio_reg_bank = (struct gpio_reg *) get_gpio_base(GPIO_TO_REG(gp));
> +	gp_val = readl(&gpio_reg_bank->gplr);
> +
> +	return GPIO_VAL(gp, gp_val);
> +}
> +
> +void gpio_set_value(int gp, int value)
> +{
> +	struct gpio_reg *gpio_reg_bank;
> +
> +	if (gp >= MV_MAX_GPIO)
> +		return;
> +
> +	gpio_reg_bank = (struct gpio_reg *) get_gpio_base(GPIO_TO_REG(gp));
> +	if (value)
> +		writel(GPIO_TO_BIT(gp),	&gpio_reg_bank->gpsr);
> +	else
> +		writel(GPIO_TO_BIT(gp),	&gpio_reg_bank->gpcr);
> +}
> diff --git a/include/mvgpio.h b/include/mvgpio.h
> new file mode 100644
> index 0000000..7d0258f
> --- /dev/null
> +++ b/include/mvgpio.h
> @@ -0,0 +1,52 @@
> +/*
> + * (C) Copyright 2011
> + * eInfochips Ltd. <www.einfochips.com>
> + * Written-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
> + *
> + * (C) Copyright 2010
> + * Marvell Semiconductor <www.marvell.com>
> + *
> + * 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., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#ifndef _MV_GPIO_H
> +#define _MV_GPIO_H
> +
> +#include <asm/types.h>
> +
> +#if defined(CONFIG_MV_GPIO)
> +
> +#define MV_GPIO_BASE		0xD4019000
> +#define GPIO_BANK0_BASE		(MV_GPIO_BASE + 0x0000)
> +#define GPIO_BANK1_BASE		(MV_GPIO_BASE + 0x0004)
> +#define GPIO_BANK2_BASE		(MV_GPIO_BASE + 0x0008)
> +#define GPIO_BANK3_BASE		(MV_GPIO_BASE + 0x0100)

Above BAR should go in armada100.h like-
#define ARMD1_GPIO0_BASE         0xD4019000
#define ARMD1_GPIO1_BASE         0xD4019004
#define ARMD1_GPIO2_BASE         0xD4019008
#define ARMD1_GPIO3_BASE         0xD4019100

> +

Rest stuff should go in asm/arch/gpio.h

> +#define GPIO_LABEL_MAX		20
> +#define MV_MAX_GPIO		128
> +
> +#define GPIO_TO_REG(gp)		(gp >> 5)
> +#define GPIO_TO_BIT(gp)		(1 << (gp & 0x1F))
> +#define GPIO_VAL(gp, val)	((val >> (gp & 0x1F)) & 0x01)
> +
> +#define GPIO_SET		1
> +#define GPIO_CLR		0
> +
> +#endif /* CONFIG_MV_GPIO */
> +#endif /* _MV_GPIO_H */
> --
> 1.7.0.4

Regards..
Prafulla . .

[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[Prafulla Wadaskar]

> -----Original Message-----
> From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> Sent: Thursday, July 21, 2011 11:10 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ajay Bhargav
> Subject: [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs
> 
> This patch adds generic GPIO driver framework support for Marvell SoCs.
> 
> To enable GPIO driver define CONFIG_MV_GPIO and for GPIO commands
> define CONFIG_CMD_GPIO in your board configuration file.
> 
> Signed-off-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
> ---
>  drivers/gpio/Makefile |    1 +
>  drivers/gpio/mvgpio.c |  125
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/mvgpio.h      |   52 ++++++++++++++++++++
>  3 files changed, 178 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/gpio/mvgpio.c
>  create mode 100644 include/mvgpio.h
> 
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 62ec97d..c6f652f 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -28,6 +28,7 @@ LIB 	:= $(obj)libgpio.o
>  COBJS-$(CONFIG_AT91_GPIO)	+= at91_gpio.o
>  COBJS-$(CONFIG_KIRKWOOD_GPIO)	+= kw_gpio.o
>  COBJS-$(CONFIG_MARVELL_MFP)	+= mvmfp.o
> +COBJS-$(CONFIG_MV_GPIO)		+= mvgpio.o
>  COBJS-$(CONFIG_MXC_GPIO)	+= mxc_gpio.o
>  COBJS-$(CONFIG_PCA953X)		+= pca953x.o
>  COBJS-$(CONFIG_S5P)		+= s5p_gpio.o
> diff --git a/drivers/gpio/mvgpio.c b/drivers/gpio/mvgpio.c
> new file mode 100644
> index 0000000..d64f0db
> --- /dev/null
> +++ b/drivers/gpio/mvgpio.c
> @@ -0,0 +1,125 @@
> +/*
> + * (C) Copyright 2011
> + * eInfochips Ltd. <www.einfochips.com>
> + * Written-by: Ajay Bhargav <ajay.bhargav@einfochips.com>
> + *
> + * (C) Copyright 2010
> + * Marvell Semiconductor <www.marvell.com>
> + *
> + * 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., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/errno.h>
> +#include <asm/gpio.h>
> +
> +char gpio_names[MV_MAX_GPIO][GPIO_LABEL_MAX];
> +
> +static int get_gpio_base(int bank)

Please define this function as 

static struct gpio_reg *get_gpio_base(int gp)

> +{
> +	switch (bank) {
> +	case 0:
> +		return GPIO_BANK0_BASE;
> +	case 1:
> +		return GPIO_BANK1_BASE;
> +	case 2:
> +		return GPIO_BANK2_BASE;
> +	case 3:
> +		return GPIO_BANK3_BASE;
> +	}
> +	return 0;
> +}

And make it part of gpio.h

> +
> +int gpio_request(int gp, const char *label)
> +{
> +	if (gp >= MV_MAX_GPIO)
> +		return -EINVAL;
> +
> +	if (strlen(gpio_names[gp]) == 0) {
> +		strncpy(gpio_names[gp], label, GPIO_LABEL_MAX);
> +		gpio_names[gp][GPIO_LABEL_MAX - 1] = '\0';
> +	} else {
> +		return -EBUSY;
> +	}
> +	return 0;
> +}
> +
> +void gpio_free(int gp)
> +{
> +	gpio_names[gp][0] = '\0';
> +}
> +
> +void gpio_toggle_value(int gp)
> +{
> +	gpio_set_value(gp, !gpio_get_value(gp));
> +}
> +
> +int gpio_direction_input(int gp)
> +{
> +	struct gpio_reg *gpio_reg_bank;
> +
> +	if (gp >= MV_MAX_GPIO)
> +		return -EINVAL;
> +
> +	gpio_reg_bank = (struct gpio_reg *) get_gpio_base(GPIO_TO_REG(gp));
> +	writel(GPIO_TO_BIT(gp), &gpio_reg_bank->gcdr);
> +	return 0;
> +}
> +
> +int gpio_direction_output(int gp, int value)
> +{
> +	struct gpio_reg *gpio_reg_bank;
> +
> +	if (gp >= MV_MAX_GPIO)
> +		return -EINVAL;
> +
> +	gpio_reg_bank = (struct gpio_reg *) get_gpio_base(GPIO_TO_REG(gp));
> +	writel(GPIO_TO_BIT(gp), &gpio_reg_bank->gsdr);
> +	gpio_set_value(gp, value);
> +	return 0;
> +}
> +
> +int gpio_get_value(int gp)
> +{
> +	struct gpio_reg *gpio_reg_bank;
> +	u32 gp_val;
> +
> +	if (gp >= MV_MAX_GPIO)

It is good to display some error message here.

> +		return -EINVAL;
> +
> +	gpio_reg_bank = (struct gpio_reg *) get_gpio_base(GPIO_TO_REG(gp));
> +	gp_val = readl(&gpio_reg_bank->gplr);
> +
> +	return GPIO_VAL(gp, gp_val);
> +}
> +
> +void gpio_set_value(int gp, int value)
> +{
> +	struct gpio_reg *gpio_reg_bank;
> +
> +	if (gp >= MV_MAX_GPIO)

It is good to display some error message here.

Regards..
Prafulla. . .

[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[Ajay Bhargav]

Hi Prafulla,

Thank you so much for feedback.

> In this case mvgpio.c is enabling support for gpio framework that will
> be interfaced using asm/gpio.h.
> 
> 1. You can split stuff in mvgpio.h to asm/arch/gpio.h and/or mvgpio.c
> 2. you can move it to drivers/gpio/
> 
> I would recommend first option.
>  
So you mean the first patch will have only mvgpio.c that enables the support
for gpio for Marvell SoCs and then second patch will enable support for Armada
series via gpio.h file. So if anyone want to use mvgpio.c he/she should write a
compatible version of gpio.h in their arch folder. I hope i am getting your right.

> > +static int get_gpio_base(int bank)
> > +{
> > +        switch (bank) {
> > +        case 0:
> > +                return GPIO_BANK0_BASE;
> > +        case 1:
> > +                return GPIO_BANK1_BASE;
> > +        case 2:
> > +                return GPIO_BANK2_BASE;
> > +        case 3:
> > +                return GPIO_BANK3_BASE;
> > +        }
> > +        return 0;
> > +}
> 
> As discussed, this would be preferred macro in gpio.h
> 
I am finding it difficult to fit this as a macro. so i thought to have it as
inline function in gpio.h will that be ok?

Regards,
Ajay Bhargav

[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[Prafulla Wadaskar]

> -----Original Message-----
> From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> Sent: Friday, July 22, 2011 10:08 AM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de
> Subject: Re: [PATCH 1/2] gpio: Add GPIO driver framework for Marvell
> SoCs
> 
> Hi Prafulla,
> 
> Thank you so much for feedback.
> 
> > In this case mvgpio.c is enabling support for gpio framework that will
> > be interfaced using asm/gpio.h.
> >
> > 1. You can split stuff in mvgpio.h to asm/arch/gpio.h and/or mvgpio.c
> > 2. you can move it to drivers/gpio/
> >
> > I would recommend first option.
> >
> So you mean the first patch will have only mvgpio.c that enables the
> support
> for gpio for Marvell SoCs and then second patch will enable support for
> Armada
> series via gpio.h file. So if anyone want to use mvgpio.c he/she should
> write a
> compatible version of gpio.h in their arch folder. I hope i am getting
> your right.

I don't mind if you merge these two patches, any future support will add gpio.h

> 
> > > +static int get_gpio_base(int bank)
> > > +{
> > > +        switch (bank) {
> > > +        case 0:
> > > +                return GPIO_BANK0_BASE;
> > > +        case 1:
> > > +                return GPIO_BANK1_BASE;
> > > +        case 2:
> > > +                return GPIO_BANK2_BASE;
> > > +        case 3:
> > > +                return GPIO_BANK3_BASE;
> > > +        }
> > > +        return 0;
> > > +}
> >
> > As discussed, this would be preferred macro in gpio.h
> >
> I am finding it difficult to fit this as a macro. so i thought to have
> it as
> inline function in gpio.h will that be ok?

Inline function should be okay

Regards..
Prafulla . . . 

> 
> Regards,
> Ajay Bhargav

[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[Ajay Bhargav]

Hi Prafulla,

> I don't mind if you merge these two patches, any future support will
> add gpio.h
> 
Let them be separate easy to track changes :)

I will be sending updated patches.

Regards,
Ajay Bhargav

[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[Lei Wen]

Hi Ajay,

On Thu, Jul 21, 2011 at 8:34 PM, Ajay Bhargav
<ajay.bhargav@einfochips.com> wrote:
> Hi Lei,
>
>> Please put this get_gpio_base into arch's self directory, since
>> different soc may have
>> different number of banks. For example, mmp3 has 6 banks.
>>
>
> Can I declare get_gpio_base in arch's gpio.h file as static inline?
> so other archs can have their own version in their respective gpio.h
> file.

Ok, I think that could be accepted.

Best regards,
Lei

[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[Ajay Bhargav]

Hi Lei,

> Please put this get_gpio_base into arch's self directory, since
> different soc may have
> different number of banks. For example, mmp3 has 6 banks.
> 

Can I declare get_gpio_base in arch's gpio.h file as static inline?
so other archs can have their own version in their respective gpio.h
file.

Regards,
Ajay Bhargav

[U-Boot] [PATCH 1/2] gpio: Add GPIO driver framework for Marvell SoCs

[Ajay Bhargav]

----- "Lei Wen" <adrian.wenl@gmail.com> wrote:

> Hi Ajay,
> 
> Please put this get_gpio_base into arch's self directory, since
> different soc may have
> different number of banks. For example, mmp3 has 6 banks.
> 
> Best regards,
> Lei
> 

was out of my mind... I think having a macro for this would be better
coz individual arch can have their own macros. Anymore comments?

Regards,
Ajay Bhargav