[U-Boot] [PATCH 1/4] gpio: Adds GPIO driver support for Armada100

Prafulla Wadaskar prafulla at marvell.com
Mon Jul 18 21:01:31 CEST 2011



> -----Original Message-----
> From: Ajay Bhargav [mailto:ajay.bhargav at einfochips.com]
> Sent: Monday, July 18, 2011 3:12 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ajay Bhargav
> Subject: [PATCH 1/4] gpio: Adds GPIO driver support for Armada100

Correct patch name as
gpio: Add GPIO driver for Marvell SoC Armada100

> 
> This patch adds generic GPIO driver framework support for Armada100
> series.
> 
> To enable GPIO driver define CONFIG_ARMADA100_GPIO and for GPIO commands
> define CONFIG_CMD_GPIO in your board configuration file.
> 
> NOTE: This driver assumes proper configuration of MFP register for the
> GPIO
> in use.
> 
> These patches depends on patch series that adds support for Marvell
> Guruplug-Display [1,2].
> 
> Signed-off-by: Ajay Bhargav <ajay.bhargav at einfochips.com>
> ---
>  arch/arm/include/asm/arch-armada100/gpio.h |  130 +++++++++++++++++++
>  drivers/gpio/Makefile                      |    1 +
>  drivers/gpio/armada100_gpio.c              |  192
> ++++++++++++++++++++++++++++
>  3 files changed, 323 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/include/asm/arch-armada100/gpio.h
>  create mode 100644 drivers/gpio/armada100_gpio.c
> 
> diff --git a/arch/arm/include/asm/arch-armada100/gpio.h
> b/arch/arm/include/asm/arch-armada100/gpio.h
> new file mode 100644
> index 0000000..9024a2e
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-armada100/gpio.h
> @@ -0,0 +1,130 @@
> +/*
> + * (C) Copyright 2010

2011???

> + * eInfochips Ltd. <www.einfochips.com>
> + * Written-by: Ajay Bhargav <ajay.bhargav at 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 _ASM_ARCH_GPIO_H
> +#define _ASM_ARCH_GPIO_H
> +
> +#ifndef __ASSEMBLY__
> +#include <asm/types.h>
> +#endif
> +
> +#if defined(CONFIG_ARMADA100_GPIO)
> +#include <asm/arch/armada100.h>
> +
> +#define GPIO_LABEL_MAX		20
> +#define ARMD_MAX_GPIO		128

Pls derive it from MAX_MFP

> +
> +#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
> +
> +/*
> + * GPIO register map
> + * Refer Datasheet Appendix A.36
> + */
> +struct armdgpio_registers {
> +	u32 gplr0;	/* Pin Level Register - 0x0000 */
> +	u32 gplr1;	/* 0x0004 */
> +	u32 gplr2;	/* 0x0008 */
> +	u32 gpdr0;	/* Pin Direction Register - 0x000C */

NAK

You should define all GPIO register in a single struct
And point them with base address offsets


> +	u32 gpdr1;	/* 0x0010 */
> +	u32 gpdr2;	/* 0x0014 */
> +	u32 gpsr0;	/* Pin Output Set Register - 0x0018 */
> +	u32 gpsr1;	/* 0x001C */
> +	u32 gpsr2;	/* 0x0020 */
> +	u32 gpcr0;	/* Pin Output Clear Register - 0x0024 */
> +	u32 gpcr1;	/* 0x0028 */
> +	u32 gpcr2;	/* 0x002C */
> +	u32 grer0;	/* Rising-Edge Detect Enable Register - 0x0030 */
> +	u32 grer1;	/* 0x0034 */
> +	u32 grer2;	/* 0x0038 */
> +	u32 gfer0;	/* Falling-Edge Detect Enable Register - 0x003C */
> +	u32 gfer1;	/* 0x0040 */
> +	u32 gfer2;	/* 0x0044 */
> +	u32 gedr0;	/* Edge Detect Status Register - 0x0048 */
> +	u32 gedr1;	/* 0x004C */
> +	u32 gedr2;	/* 0x0050 */
> +	u32 gsdr0;	/* Bitwise Set of GPIO Direction Register - 0x0054 */
> +	u32 gsdr1;	/* 0x0058 */
> +	u32 gsdr2;	/* 0x005C */
> +	u32 gcdr0;	/* Bitwise Clear of GPIO Direction Register - 0x0060 */
> +	u32 gcdr1;	/* 0x0064 */
> +	u32 gcdr2;	/* 0x0068 */
> +	u32 gsrer0;	/* Bitwise Set of Rising-Edge Detect Enable
> +			   Register - 0x006C */
> +	u32 gsrer1;	/* 0x0070 */
> +	u32 gsrer2;	/* 0x0074 */
> +	u32 gcrer0;	/* Bitwise Clear of Rising-Edge Detect Enable
> +			   Register - 0x0078 */
> +	u32 gcrer1;	/* 0x007C */
> +	u32 gcrer2;	/* 0x0080 */
> +	u32 gsfer0;	/* Bitwise Set of Falling-Edge Detect Enable
> +			   Register - 0x0084 */
> +	u32 gsfer1;	/* 0x0088 */
> +	u32 gsfer2;	/* 0x008C */
> +	u32 gcfer0;	/* Bitwise Clear of Falling-Edge Detect Enable
> +			   Register - 0x0090 */
> +	u32 gcfer1;	/* 0x0094 */
> +	u32 gcfer2;	/* 0x0098 */
> +	u32 apmask0;	/* Bitwise Mask of Edge Detect Register - 0x009C */
> +	u32 apmask1;	/* 0x00A0 */
> +	u32 apmask2;	/* 0x00A4 */
> +	u8 pad[0x0100 - 0x00A4 - 4];
> +	u32 gplr3;	/* 0x0100 */
> +	u8 pad1[0x010C - 0x0100 - 4];
> +	u32 gpdr3;	/* 0x010C */
> +	u8 pad2[0x0118 - 0x010C - 4];
> +	u32 gpsr3;	/* 0x0118 */
> +	u8 pad3[0x0124 - 0x0118 - 4];
> +	u32 gpcr3;	/* 0x0124 */
> +	u8 pad4[0x0130 - 0x0124 - 4];
> +	u32 grer3;	/* 0x0130 */
> +	u8 pad5[0x013C - 0x0130 - 4];
> +	u32 gfer3;	/* 0x013C */
> +	u8 pad6[0x0148 - 0x013C - 4];
> +	u32 gedr3;	/* 0x0148 */
> +	u8 pad7[0x0154 - 0x0148 - 4];
> +	u32 gsdr3;	/* 0x0154 */
> +	u8 pad8[0x0160 - 0x0154 - 4];
> +	u32 gcdr3;	/* 0x0160 */
> +	u8 pad9[0x016C - 0x0160 - 4];
> +	u32 gsrer3;	/* 0x016C */
> +	u8 pad10[0x0178 - 0x016C - 4];
> +	u32 gcrer3;	/* 0x0178 */
> +	u8 pad11[0x0184 - 0x0178 - 4];
> +	u32 gsfer3;	/* 0x0184 */
> +	u8 pad12[0x0190 - 0x0184 - 4];
> +	u32 gcfer3;	/* 0x0190 */
> +	u8 pad13[0x019C - 0x0190 - 4];
> +	u32 apmask3;	/* 0x019C */
> +};
> +
> +#endif /* CONFIG_ARMADA100_GPIO */
> +#endif /* _ASM_ARCH_GPIO_H */
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 1e3ae11..31b83cd 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -26,6 +26,7 @@ include $(TOPDIR)/config.mk
>  LIB 	:= $(obj)libgpio.o
> 
>  COBJS-$(CONFIG_AT91_GPIO)	+= at91_gpio.o
> +COBJS-$(CONFIG_ARMADA100_GPIO)	+= armada100_gpio.o

I suggest you to add mvgpio.c instead of armada100_gpio.c
This will be used in future by other Marvell SoCs.

>  COBJS-$(CONFIG_KIRKWOOD_GPIO)	+= kw_gpio.o
>  COBJS-$(CONFIG_MARVELL_MFP)	+= mvmfp.o
>  COBJS-$(CONFIG_MXC_GPIO)	+= mxc_gpio.o
> diff --git a/drivers/gpio/armada100_gpio.c
> b/drivers/gpio/armada100_gpio.c
> new file mode 100644
> index 0000000..578fdac
> --- /dev/null
> +++ b/drivers/gpio/armada100_gpio.c
> @@ -0,0 +1,192 @@
> +/*
> + * (C) Copyright 2010

2011??

> + * eInfochips Ltd. <www.einfochips.com>
> + * Written-by: Ajay Bhargav <ajay.bhargav at 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>
> +
> +int gpio_request(int gp, const char *label)
> +{
> +	/*
> +	 * Assumes corresponding MFP is configured peoperly
> +	 * for use as GPIO
> +	 */

NAK, you should check here, respective MFP is being configured as GPIO, if not you should return error

> +	return 0;
> +}
> +
> +void gpio_free(int gp)
> +{
> +	/* default direction for GPIO is input */
> +	gpio_direction_input(gp);
> +}
> +
> +void gpio_toggle_value(int gp)
> +{
> +	gpio_set_value(gp, !gpio_get_value(gp));
> +}
> +
> +int gpio_direction_input(int gp)
> +{
> +	struct armdgpio_registers *gpio_regs =
> +		(struct armdgpio_registers *)ARMD1_GPIO_BASE;

Consider now this is generic GPIO driver for marvell SOCs, define this macro in gpio.h

> +
> +	if (gp < ARMD_MAX_GPIO) {
> +		switch (GPIO_TO_REG(gp)) {

Some code comments are welcomed here.

> +		case 0:
> +			writel(GPIO_TO_BIT(gp), &gpio_regs->gcdr0);
> +			break;
> +		case 1:
> +			writel(GPIO_TO_BIT(gp), &gpio_regs->gcdr1);
> +			break;
> +		case 2:
> +			writel(GPIO_TO_BIT(gp), &gpio_regs->gcdr2);
> +			break;
> +		case 3:
> +			writel(GPIO_TO_BIT(gp), &gpio_regs->gcdr3);
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	} else {
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +int gpio_direction_output(int gp, int value)
> +{
> +	struct armdgpio_registers *gpio_regs =
> +		(struct armdgpio_registers *)ARMD1_GPIO_BASE;
> +
> +	if (gp < ARMD_MAX_GPIO) {
> +		switch (GPIO_TO_REG(gp)) {
> +		case 0:
> +			writel(GPIO_TO_BIT(gp), &gpio_regs->gsdr0);

Call gpio_set_value(gp, value) here which is defined below doing the same


> +			if (value)
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpsr0);
> +			else
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpcr0);
> +			break;
> +		case 1:
> +			writel(GPIO_TO_BIT(gp), &gpio_regs->gsdr1);
> +			if (value)
> +				writel(GPIO_TO_BIT(gp), &gpio_regs->gpsr1);
> +			else
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpcr1);
> +			break;
> +		case 2:
> +			writel(GPIO_TO_BIT(gp), &gpio_regs->gsdr2);
> +			if (value)
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpsr2);
> +			else
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpcr2);
> +			break;
> +		case 3:
> +			writel(GPIO_TO_BIT(gp), &gpio_regs->gsdr3);
> +			if (value)
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpsr3);
> +			else
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpcr3);
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	} else {
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +int gpio_get_value(int gp)
> +{
> +	struct armdgpio_registers *gpio_regs =
> +		(struct armdgpio_registers *)ARMD1_GPIO_BASE;
> +	u32 gp_val;
> +
> +	if (gp < ARMD_MAX_GPIO) {
> +		switch (GPIO_TO_REG(gp)) {
> +		case 0:
> +			gp_val = readl(&gpio_regs->gplr0);
> +			break;
> +		case 1:
> +			gp_val = readl(&gpio_regs->gplr1);
> +			break;
> +		case 2:
> +			gp_val = readl(&gpio_regs->gplr2);
> +			break;
> +		case 3:
> +			gp_val = readl(&gpio_regs->gplr3);
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	return GPIO_VAL(gp, gp_val);
> +}
> +
> +void gpio_set_value(int gp, int value)
> +{
> +	struct armdgpio_registers *gpio_regs =
> +		(struct armdgpio_registers *)ARMD1_GPIO_BASE;
> +
> +	if (gp < ARMD_MAX_GPIO) {
> +		switch (GPIO_TO_REG(gp)) {
> +		case 0:
> +			if (value)
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpsr0);
> +			else
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpcr0);
> +			break;
> +		case 1:
> +			if (value)
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpsr1);
> +			else
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpcr1);
> +			break;
> +		case 2:
> +			if (value)
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpsr2);
> +			else
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpcr2);
> +			break;
> +		case 3:
> +			if (value)
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpsr3);
> +			else
> +				writel(GPIO_TO_BIT(gp),	&gpio_regs->gpcr3);
> +			break;
> +		default:
> +			panic("Invalid GPIO pin %u\n", gp);
> +		}
> +	} else {
> +		panic("Invalid GPIO pin %u\n", gp);

Can you eliminate one panic line here?

Regards..
Prafulla . .


More information about the U-Boot mailing list