Hi Andreas, Thank you very much for your review.
Your concerns are fixed in the v2 patch. > -----Original Message----- > From: Andreas Bießmann [mailto:[email protected]] > Sent: 2015年11月1日 5:54 > To: Yang, Wenyou; U-Boot Mailing List > Subject: Re: [PATCH] gpio: atmel: Add the PIO4 driver support > > Hi Wenyou, > > sorry for long delay ... > > On 08.09.15 08:33, Wenyou Yang wrote: > > The PIO4 is introduced from SAMA5D2, as a new version for Atmel PIO > > controller. > > > > Signed-off-by: Wenyou Yang <[email protected]> > > --- > > > > arch/arm/mach-at91/include/mach/atmel_pio4.h | 48 +++++ > > drivers/gpio/Kconfig | 11 + > > drivers/gpio/Makefile | 1 + > > drivers/gpio/atmel_pio4.c | 278 > > ++++++++++++++++++++++++++ > > 4 files changed, 338 insertions(+) > > create mode 100644 arch/arm/mach-at91/include/mach/atmel_pio4.h > > create mode 100644 drivers/gpio/atmel_pio4.c > > > > diff --git a/arch/arm/mach-at91/include/mach/atmel_pio4.h > > b/arch/arm/mach-at91/include/mach/atmel_pio4.h > > new file mode 100644 > > index 0000000..5748da6 > > --- /dev/null > > +++ b/arch/arm/mach-at91/include/mach/atmel_pio4.h > > @@ -0,0 +1,48 @@ > > +/* > > + * Copyright (C) 2015 Atmel Corporation. > > + * Wenyou Yang <[email protected]> > > + * > > + * SPDX-License-Identifier: GPL-2.0+ > > + */ > > + > > +#ifndef __ATMEL_PIO4_H > > +#define __ATMEL_PIO4_H > > + > > +#ifndef __ASSEMBLY__ > > + > > +struct atmel_pio4_port { > > + u32 mskr; /* 0x00 PIO Mask Register */ > > + u32 cfgr; /* 0x04 PIO Configuration Register */ > > + u32 pdsr; /* 0x08 PIO Pin Data Status Register */ > > + u32 locksr; /* 0x0C PIO Lock Status Register */ > > + u32 sodr; /* 0x10 PIO Set Output Data Register */ > > + u32 codr; /* 0x14 PIO Clear Output Data Register */ > > + u32 odsr; /* 0x18 PIO Output Data Status Register */ > > + u32 reserved0; > > + u32 ier; /* 0x20 PIO Interrupt Enable Register */ > > + u32 idr; /* 0x24 PIO Interrupt Disable Register */ > > + u32 imr; /* 0x28 PIO Interrupt Mask Register */ > > + u32 isr; /* 0x2C PIO Interrupt Status Register */ > > + u32 reserved1[3]; > > + u32 iofr; /* 0x3C PIO I/O Freeze Register */ > > +}; > > + > > +#endif > > + > > +#define AT91_PIO_PORTA 0x0 > > +#define AT91_PIO_PORTB 0x1 > > +#define AT91_PIO_PORTC 0x2 > > +#define AT91_PIO_PORTD 0x3 > > + > > +int atmel_pio4_set_gpio(u32 port, u32 pin, u32 use_pullup); int > > +atmel_pio4_set_a_periph(u32 port, u32 pin, u32 use_pullup); int > > +atmel_pio4_set_b_periph(u32 port, u32 pin, u32 use_pullup); int > > +atmel_pio4_set_c_periph(u32 port, u32 pin, u32 use_pullup); int > > +atmel_pio4_set_d_periph(u32 port, u32 pin, u32 use_pullup); int > > +atmel_pio4_set_e_periph(u32 port, u32 pin, u32 use_pullup); int > > +atmel_pio4_set_f_periph(u32 port, u32 pin, u32 use_pullup); int > > +atmel_pio4_set_g_periph(u32 port, u32 pin, u32 use_pullup); void > > +atmel_pio4_set_pio_output(u32 port, u32 pin, u32 value); > > +u32 atmel_pio4_get_pio_input(u32 port, u32 pin); > > + > > +#endif > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index > > ef57a89..ddc47c6 100644 > > --- a/drivers/gpio/Kconfig > > +++ b/drivers/gpio/Kconfig > > @@ -21,6 +21,17 @@ config DWAPB_GPIO > > help > > Support for the Designware APB GPIO driver. > > > > +config ATMEL_PIO4 > > + bool "ATMEL PIO4 driver" > > + depends on DM > > + default n > > + help > > + Say yes here to support the Atmel PIO4 driver. > > + The PIO4 is new version of Atmel PIO controller, which manages > > + up to 128 fully programmable input/output lines. Each I/O line > > + may be dedicated as a general purpose I/O or be assigned to > > + a function of an embedded peripheral. > > + > > config LPC32XX_GPIO > > bool "LPC32XX GPIO driver" > > depends on DM > > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index > > c58aa4d..fb4fd25 100644 > > --- a/drivers/gpio/Makefile > > +++ b/drivers/gpio/Makefile > > @@ -12,6 +12,7 @@ endif > > obj-$(CONFIG_DM_GPIO) += gpio-uclass.o > > > > obj-$(CONFIG_AT91_GPIO) += at91_gpio.o > > +obj-$(CONFIG_ATMEL_PIO4) += atmel_pio4.o > > obj-$(CONFIG_INTEL_ICH6_GPIO) += intel_ich6_gpio.o > > obj-$(CONFIG_KIRKWOOD_GPIO) += kw_gpio.o > > obj-$(CONFIG_KONA_GPIO) += kona_gpio.o > > diff --git a/drivers/gpio/atmel_pio4.c b/drivers/gpio/atmel_pio4.c new > > file mode 100644 index 0000000..21095ce > > --- /dev/null > > +++ b/drivers/gpio/atmel_pio4.c > > @@ -0,0 +1,278 @@ > > +/* > > + * Atmel PIO4 device driver > > + * > > + * Copyright (C) 2015 Atmel Corporation > > + * Wenyou.Yang <[email protected]> > > + * > > + * SPDX-License-Identifier: GPL-2.0+ > > + */ > > +#include <common.h> > > +#include <dm.h> > > +#include <asm/arch/hardware.h> > > +#include <mach/gpio.h> > > +#include <mach/atmel_pio4.h> > > + > > +#define ATMEL_PIO4_PINS_PER_BANK 32 > > + > > +/* > > + * Register Field Definitions > > + */ > > +#define ATMEL_PIO4_CFGR_FUNC (0x07 << 0) > > +#define ATMEL_PIO4_CFGR_FUNC_GPIO (0x0 << 0) > > +#define ATMEL_PIO4_CFGR_FUNC_PERIPH_A (0x1 << 0) > > +#define ATMEL_PIO4_CFGR_FUNC_PERIPH_B (0x2 << 0) > > +#define ATMEL_PIO4_CFGR_FUNC_PERIPH_C (0x3 << 0) > > +#define ATMEL_PIO4_CFGR_FUNC_PERIPH_D (0x4 << 0) > > +#define ATMEL_PIO4_CFGR_FUNC_PERIPH_E (0x5 << 0) > > +#define ATMEL_PIO4_CFGR_FUNC_PERIPH_F (0x6 << 0) > > +#define ATMEL_PIO4_CFGR_FUNC_PERIPH_G (0x7 << 0) > > +#define ATMEL_PIO4_CFGR_DIR (0x01 << 8) > > +#define ATMEL_PIO4_CFGR_PUEN (0x01 << 9) > > +#define ATMEL_PIO4_CFGR_PDEN (0x01 << 10) > > +#define ATMEL_PIO4_CFGR_IFEN (0x01 << 12) > > +#define ATMEL_PIO4_CFGR_IFSCEN (0x01 << 13) > > +#define ATMEL_PIO4_CFGR_OPD (0x01 << 14) > > +#define ATMEL_PIO4_CFGR_SCHMITT (0x01 << 15) > > +#define ATMEL_PIO4_CFGR_DRVSTR (0x03 << 16) > > +#define ATMEL_PIO4_CFGR_DRVSTR_HIGH (0x00 << 16) > > +#define ATMEL_PIO4_CFGR_DRVSTR_MEDIUM (0x01 << > 16) > > +#define ATMEL_PIO4_CFGR_DRVSTR_LOW (0x02 << 16) > > current datasheet have this set to 0/1 LO, 2 MED, 3 HI > > > +#define ATMEL_PIO4_CFGR_EVTSEL (0x07 << 24) > > +#define ATMEL_PIO4_CFGR_EVTSEL_FALLING (0x00 << > 24) > > +#define ATMEL_PIO4_CFGR_EVTSEL_RISING (0x01 << > 24) > > +#define ATMEL_PIO4_CFGR_EVTSEL_BOTH (0x02 << 24) > > +#define ATMEL_PIO4_CFGR_EVTSEL_LOW (0x03 << 24) > > +#define ATMEL_PIO4_CFGR_EVTSEL_HIGH (0x04 << 24) > > +#define ATMEL_PIO4_CFGR_PCFS (0x01 << 29) > > +#define ATMEL_PIO4_CFGR_ICFS (0x01 << 30) > > +#define ATMEL_PIO4_CFGR_TAMPEN (0x01 << 31) > > current datasheet have no TAMPEN at bit 31. > > > + > > +static struct atmel_pio4_port *atmel_pio4_port_base(u32 port) { > > + struct atmel_pio4_port *base = NULL; > > + > > + switch (port) { > > + case AT91_PIO_PORTA: > > + base = (struct atmel_pio4_port *)ATMEL_BASE_PIOA; > > + break; > > + case AT91_PIO_PORTB: > > + base = (struct atmel_pio4_port *)ATMEL_BASE_PIOB; > > + break; > > + case AT91_PIO_PORTC: > > + base = (struct atmel_pio4_port *)ATMEL_BASE_PIOC; > > + break; > > + case AT91_PIO_PORTD: > > + base = (struct atmel_pio4_port *)ATMEL_BASE_PIOD; > > + break; > > + default: > > + printf("Error: Atmel PIO4: Failed to get PIO base!\n"); > > The input would be of interest here ... could you add 'port' to printf? > > > + break; > > + } > > + > > + return base; > > +} > > + > > +static int atmel_pio4_config_io_func(u32 port, u32 pin, > > + u32 func, u32 use_pullup) > > +{ > > + struct atmel_pio4_port *port_base; > > + u32 reg, mask; > > + > > + if (pin >= ATMEL_PIO4_PINS_PER_BANK) > > + return -1; > > -ENODEV > > > + > > + port_base = atmel_pio4_port_base(port); > > port_base may be NULL, please check and return -ENODEV > > > + mask = 1 << pin; > > + reg = func; > > + reg |= use_pullup ? ATMEL_PIO4_CFGR_PUEN : 0; > > + > > + writel(mask, &port_base->mskr); > > + writel(reg, &port_base->cfgr); > > + > > + return 0; > > +} > > + > > +int atmel_pio4_set_gpio(u32 port, u32 pin, u32 use_pullup) { > > + return atmel_pio4_config_io_func(port, pin, > > + ATMEL_PIO4_CFGR_FUNC_GPIO, > > + use_pullup); > > +} > > + > > +int atmel_pio4_set_a_periph(u32 port, u32 pin, u32 use_pullup) { > > + return atmel_pio4_config_io_func(port, pin, > > + ATMEL_PIO4_CFGR_FUNC_PERIPH_A, > > + use_pullup); > > +} > > + > > +int atmel_pio4_set_b_periph(u32 port, u32 pin, u32 use_pullup) { > > + return atmel_pio4_config_io_func(port, pin, > > + ATMEL_PIO4_CFGR_FUNC_PERIPH_B, > > + use_pullup); > > +} > > + > > +int atmel_pio4_set_c_periph(u32 port, u32 pin, u32 use_pullup) { > > + return atmel_pio4_config_io_func(port, pin, > > + ATMEL_PIO4_CFGR_FUNC_PERIPH_C, > > + use_pullup); > > +} > > + > > +int atmel_pio4_set_d_periph(u32 port, u32 pin, u32 use_pullup) { > > + return atmel_pio4_config_io_func(port, pin, > > + ATMEL_PIO4_CFGR_FUNC_PERIPH_D, > > + use_pullup); > > +} > > + > > +int atmel_pio4_set_e_periph(u32 port, u32 pin, u32 use_pullup) { > > + return atmel_pio4_config_io_func(port, pin, > > + ATMEL_PIO4_CFGR_FUNC_PERIPH_E, > > + use_pullup); > > +} > > + > > +int atmel_pio4_set_f_periph(u32 port, u32 pin, u32 use_pullup) { > > + return atmel_pio4_config_io_func(port, pin, > > + ATMEL_PIO4_CFGR_FUNC_PERIPH_F, > > + use_pullup); > > +} > > + > > +int atmel_pio4_set_g_periph(u32 port, u32 pin, u32 use_pullup) { > > + return atmel_pio4_config_io_func(port, pin, > > + ATMEL_PIO4_CFGR_FUNC_PERIPH_G, > > + use_pullup); > > +} > > + > > +void atmel_pio4_set_pio_output(u32 port, u32 pin, u32 value) { > > + struct atmel_pio4_port *port_base; > > + u32 reg, mask; > > + > > + port_base = atmel_pio4_port_base(port); > > + mask = 0x01 << pin; > > pin and port may be out of range, please add checks (as in > atmel_pio4_config_io_func()) > > Or even better, re-use atmel_pio4_config_io_func() here ... > > > + reg = ATMEL_PIO4_CFGR_FUNC_GPIO | ATMEL_PIO4_CFGR_DIR; > > + > > + writel(mask, &port_base->mskr); > > + writel(reg, &port_base->cfgr); > > + > > + if (value) > > + writel(mask, &port_base->sodr); > > + else > > + writel(mask, &port_base->codr); > > +} > > + > > +u32 atmel_pio4_get_pio_input(u32 port, u32 pin) { > > + struct atmel_pio4_port *port_base; > > + u32 reg, mask; > > + > > + port_base = atmel_pio4_port_base(port); > > + mask = 0x01 << pin; > > same as atmel_pio4_set_pio_output() ... > > > + reg = ATMEL_PIO4_CFGR_FUNC_GPIO; > > + > > + writel(mask, &port_base->mskr); > > + writel(reg, &port_base->cfgr); > > + > > + return (readl(&port_base->pdsr) & mask) ? 1 : 0; } > > + > > +#ifdef CONFIG_DM_GPIO > > +static int atmel_pio4_direction_input(struct udevice *dev, unsigned > > +offset) { > > + struct at91_port_platdata *plat = dev_get_platdata(dev); > > + struct atmel_pio4_port *port_base = (atmel_pio4_port *)plat->base_addr; > > + u32 mask = 0x01 << offset; > > + u32 reg = ATMEL_PIO4_CFGR_FUNC_GPIO; > > + > > + writel(mask, &port_base->mskr); > > + writel(reg, &port_base->cfgr); > > + > > + return 0; > > +} > > + > > +static int atmel_pio4_direction_output(struct udevice *dev, > > + unsigned offset, int value) { > > + struct at91_port_platdata *plat = dev_get_platdata(dev); > > + struct atmel_pio4_port *port_base = (atmel_pio4_port *)plat->base_addr; > > + u32 mask = 0x01 << offset; > > + u32 reg = ATMEL_PIO4_CFGR_FUNC_GPIO | ATMEL_PIO4_CFGR_DIR; > > + > > + writel(mask, &port_base->mskr); > > + writel(reg, &port_base->cfgr); > > + > > + if (value) > > + writel(mask, &port_base->sodr); > > + else > > + writel(mask, &port_base->codr); > > + > > + return 0; > > +} > > + > > +static int atmel_pio4_get_value(struct udevice *dev, unsigned offset) > > +{ > > + struct at91_port_platdata *plat = dev_get_platdata(dev); > > + struct atmel_pio4_port *port_base = (atmel_pio4_port *)plat->base_addr; > > + u32 mask = 0x01 << offset; > > + > > + return (readl(&port_base->pdsr) & mask) ? 1 : 0; } > > + > > +static int atmel_pio4_set_value(struct udevice *dev, > > + unsigned offset, int value) > > +{ > > + struct at91_port_platdata *plat = dev_get_platdata(dev); > > + struct atmel_pio4_port *port_base = (atmel_pio4_port *)plat->base_addr; > > + u32 mask = 0x01 << offset; > > + > > + if (value) > > + writel(mask, &port_base->sodr); > > + else > > + writel(mask, &port_base->codr); > > + > > + return 0; > > +} > > + > > +static int atmel_pio4_get_function(struct udevice *dev, unsigned > > +offset) { > > + struct at91_port_platdata *plat = dev_get_platdata(dev); > > + struct atmel_pio4_port *port_base = (atmel_pio4_port *)plat->base_addr; > > + u32 mask = 0x01 << offset; > > + > > + writel(mask, &port_base->mskr); > > + > > + return (readl(&port_base->cfgr) & > > + ATMEL_PIO4_CFGR_DIR) ? GPIOF_OUTPUT : GPIOF_INPUT; } > > + > > +static const struct dm_gpio_ops atmel_pio4_ops = { > > + .direction_input = atmel_pio4_direction_input, > > + .direction_output = atmel_pio4_direction_output, > > + .get_value = atmel_pio4_get_value, > > + .set_value = atmel_pio4_set_value, > > + .get_function = atmel_pio4_get_function, > > +}; > > + > > +static int atmel_pio4_probe(struct udevice *dev) { > > + struct at91_port_platdata *plat = dev_get_platdata(dev); > > + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); > > + > > + uc_priv->bank_name = plat->bank_name; > > + uc_priv->gpio_count = ATMEL_PIO4_PINS_PER_BANK; > > + > > + return 0; > > +} > > + > > +U_BOOT_DRIVER(gpio_atmel_pio4) = { > > + .name = "gpio_atmel_pio4", > > + .id = UCLASS_GPIO, > > + .ops = &atmel_pio4_ops, > > + .probe = atmel_pio4_probe, > > +}; > > +#endif > > > > Andreas Thank again. Best Regards, Wenyou Yang _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
