How about merge this into current mvmfp.c? Just add some function into it, then no need another c file.
Best regards, Lei On Tue, Jul 19, 2011 at 3:01 AM, Prafulla Wadaskar <prafu...@marvell.com> wrote: > > >> -----Original Message----- >> From: Ajay Bhargav [mailto:ajay.bhar...@einfochips.com] >> Sent: Monday, July 18, 2011 3:12 PM >> To: Prafulla Wadaskar >> Cc: u-boot@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.bhar...@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.bhar...@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.bhar...@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 . . > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
