Hi Simon, Le Thu, 12 Feb 2015 22:06:51 -0700, Simon Glass <s...@chromium.org> a écrit :
> Hi Albert, > > On 12 February 2015 at 10:37, Albert ARIBAUD (3ADEV) > <albert.arib...@3adev.fr> wrote: > > This driver only supports Driver Model, not legacy model. > > > > Signed-off-by: Albert ARIBAUD (3ADEV) <albert.arib...@3adev.fr> > > --- > > > > Changes in v2: > > - move from legacy to Driver Model support > > > > arch/arm/cpu/arm926ejs/lpc32xx/devices.c | 5 + > > arch/arm/include/asm/arch-lpc32xx/gpio.h | 43 +++++ > > drivers/gpio/Makefile | 1 + > > drivers/gpio/lpc32xx_gpio.c | 268 > > +++++++++++++++++++++++++++++++ > > 4 files changed, 317 insertions(+) > > create mode 100644 arch/arm/include/asm/arch-lpc32xx/gpio.h > > create mode 100644 drivers/gpio/lpc32xx_gpio.c > > > > diff --git a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c > > b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c > > index 81b53ea..a407098 100644 > > --- a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c > > +++ b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c > > @@ -9,6 +9,7 @@ > > #include <asm/arch/clk.h> > > #include <asm/arch/uart.h> > > #include <asm/io.h> > > +#include <dm.h> > > > > static struct clk_pm_regs *clk = (struct clk_pm_regs *)CLK_PM_BASE; > > static struct uart_ctrl_regs *ctrl = (struct uart_ctrl_regs > > *)UART_CTRL_BASE; > > @@ -61,3 +62,7 @@ void lpc32xx_i2c_init(unsigned int devnum) > > ctrl |= CLK_I2C2_ENABLE; > > writel(ctrl, &clk->i2cclk_ctrl); > > } > > + > > +U_BOOT_DEVICE(lpc32xx_gpios) = { > > + .name = "gpio_lpc32xx" > > +}; > > diff --git a/arch/arm/include/asm/arch-lpc32xx/gpio.h > > b/arch/arm/include/asm/arch-lpc32xx/gpio.h > > new file mode 100644 > > index 0000000..3bd94e3 > > --- /dev/null > > +++ b/arch/arm/include/asm/arch-lpc32xx/gpio.h > > @@ -0,0 +1,43 @@ > > +/* > > + * LPC32xx GPIO interface > > + * > > + * (C) Copyright 2014 DENX Software Engineering GmbH > > + * Written-by: Albert ARIBAUD <albert.arib...@3adev.fr> > > + * > > + * SPDX-License-Identifier: GPL-2.0+ > > + */ > > + > > +/** > > + * GPIO Register map for LPC32xx > > + */ > > + > > +struct gpio_regs { > > + u32 p3_inp_state; > > + u32 p3_outp_set; > > + u32 p3_outp_clr; > > + u32 p3_outp_state; > > + /* Watch out! the following are shared between p2 and p3 */ > > + u32 p2_p3_dir_set; > > + u32 p2_p3_dir_clr; > > + u32 p2_p3_dir_state; > > + /* Now back to 'one register for one port' */ > > + u32 p2_inp_state; > > + u32 p2_outp_set; > > + u32 p2_outp_clr; > > + u32 reserved1[6]; > > + u32 p0_inp_state; > > + u32 p0_outp_set; > > + u32 p0_outp_clr; > > + u32 p0_outp_state; > > + u32 p0_dir_set; > > + u32 p0_dir_clr; > > + u32 p0_dir_state; > > + u32 reserved2; > > + u32 p1_inp_state; > > + u32 p1_outp_set; > > + u32 p1_outp_clr; > > + u32 p1_outp_state; > > + u32 p1_dir_set; > > + u32 p1_dir_clr; > > + u32 p1_dir_state; > > +}; > > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > > index aa11f15..559894a 100644 > > --- a/drivers/gpio/Makefile > > +++ b/drivers/gpio/Makefile > > @@ -37,3 +37,4 @@ obj-$(CONFIG_ADI_GPIO2) += adi_gpio2.o > > obj-$(CONFIG_TCA642X) += tca642x.o > > oby-$(CONFIG_SX151X) += sx151x.o > > obj-$(CONFIG_SUNXI_GPIO) += sunxi_gpio.o > > +obj-$(CONFIG_LPC32XX_GPIO) += lpc32xx_gpio.o > > diff --git a/drivers/gpio/lpc32xx_gpio.c b/drivers/gpio/lpc32xx_gpio.c > > new file mode 100644 > > index 0000000..861975e > > --- /dev/null > > +++ b/drivers/gpio/lpc32xx_gpio.c > > @@ -0,0 +1,268 @@ > > +/* > > + * LPC32xxGPIO driver > > + * > > + * (C) Copyright 2014 DENX Software Engineering GmbH > > + * Written-by: Albert ARIBAUD <albert.arib...@3adev.fr> > > + * > > + * SPDX-License-Identifier: GPL-2.0+ > > + */ > > + > > +/** > > + * We only support driver model > > + */ > > +#ifndef CONFIG_DM_GPIO > > +#error Please enable Driver Model GPIO in your target configuration. > > +#endif > > Minor note - if you base this on dm/master you can use Kconfig > 'depends on DM' in this driver's Kconfig bit. > > (pull request to mainline coming soon) I will rebase then for v3. > > +#include <asm/io.h> > > +#include <asm/arch-lpc32xx/cpu.h> > > +#include <asm/arch-lpc32xx/gpio.h> > > +#include <asm-generic/gpio.h> > > +#include <dm.h> > > +#include <malloc.h> > > + > > +/** > > + * LPC32xx GPIOs work in banks but are non-homogeneous: > > + * - each bank holds a different number of GPIOs > > + * - some GPIOs are input/ouput, some input only, some output only; > > + * - some GPIOs have different meanings as an input and as an output; > > + * - some GPIOs are controlled on a given port and bit index, but > > + * read on another one. > > +* > > + * In order to keep this code simple, GPIOS are considered here as > > + * homogeneous and linear, from 0 to 127. > > + * > > + * ** WARNING ** > > + * > > + * Client code is responsible for properly using valid GPIO numbers, > > + * including cases where a single physical GPIO has differing numbers > > + * for setting its direction, reading it and/or writing to it. > > + */ > > + > > +#define LPC32XX_GPIOS 128 > > + > > +static struct gpio_regs *regs = (struct gpio_regs *)GPIO_BASE; > > Normally this would go in a > > struct lpc32xx_priv > > in the driver. Will change this. > > + > > +/** > > + * We have 4 GPIO ports of 32 bits each > > + */ > > + > > +#define MAX_GPIO 128 > > + > > +#define GPIO_TO_PORT(gpio) ((gpio / 32) & 3) > > +#define GPIO_TO_RANK(gpio) (gpio % 32) > > +#define GPIO_TO_MASK(gpio) (1 << (gpio % 32)) > > + > > +/** > > + * Array of current GPIO functions. Allocated as unsigned chars to > > + * limit memory consumption. > > + */ > > + > > +static signed char lpc32xx_function[LPC32XX_GPIOS]; > > ...along with this. Ditto. > > + > > +/** > > + * Configure a GPIO number 'offset' as input > > + */ > > + > > +static int lpc32xx_gpio_direction_input(struct udevice *dev, unsigned > > offset) > > +{ > > + int port, mask; > > + > > + port = GPIO_TO_PORT(offset); > > + mask = GPIO_TO_MASK(offset); > > + > > + switch (port) { > > + case 0: > > + writel(mask, ®s->p0_dir_clr); > > + break; > > + case 1: > > + writel(mask, ®s->p1_dir_clr); > > + break; > > + case 2: > > + /* ports 2 and 3 share a common direction */ > > + case 3: > > + writel(mask, ®s->p2_p3_dir_clr); > > + break; > > + default: > > + return -1; > > + } > > + > > + lpc32xx_function[offset] = GPIOF_INPUT; > > Another way of doing this is to read the status from the hardware. > This might allow you to support GPIOF_FUNCTION - i.e. the GPIO is > currently used by a function. Even under the simplified model I used for LPC32XX GPIOs, finding out the state of an individual gpio would be quite complicated and would incur much work for a feature which is actually only useful for the 'gpio' command -- a commodity but not a necessity as far a the actual board user is concerned. > > + > > + return 0; > > +} > > + > > +/** > > + * Get the value of a GPIO > > + */ > > + > > +static int lpc32xx_gpio_get_value(struct udevice *dev, unsigned offset) > > +{ > > + int port, rank, mask, value; > > + > > + port = GPIO_TO_PORT(offset); > > + > > + switch (port) { > > + case 0: > > + value = readl(®s->p0_inp_state); > > + break; > > + case 1: > > + value = readl(®s->p1_inp_state); > > + break; > > + case 2: > > + value = readl(®s->p2_inp_state); > > + break; > > + case 3: > > + value = readl(®s->p3_inp_state); > > + break; > > + default: > > + return -1; > > + } > > + > > + rank = GPIO_TO_RANK(offset); > > + mask = GPIO_TO_MASK(offset); > > + > > + return (value & mask) >> rank; > > +} > > + > > +/** > > + * Set a GPIO > > + */ > > + > > +static int gpio_set(unsigned gpio) > > +{ > > + int port, mask; > > + > > + port = GPIO_TO_PORT(gpio); > > + mask = GPIO_TO_MASK(gpio); > > + > > + switch (port) { > > + case 0: > > + writel(mask, ®s->p0_outp_set); > > + break; > > + case 1: > > + writel(mask, ®s->p1_outp_set); > > + break; > > + case 2: > > + writel(mask, ®s->p2_outp_set); > > + break; > > + case 3: > > + writel(mask, ®s->p3_outp_set); > > + break; > > + default: > > + return -1; > > + } > > + return 0; > > +} > > + > > +/** > > + * Clear a GPIO > > + */ > > + > > +static int gpio_clr(unsigned gpio) > > +{ > > + int port, mask; > > + > > + port = GPIO_TO_PORT(gpio); > > + mask = GPIO_TO_MASK(gpio); > > + > > + switch (port) { > > + case 0: > > + writel(mask, ®s->p0_outp_clr); > > + break; > > + case 1: > > + writel(mask, ®s->p1_outp_clr); > > + break; > > + case 2: > > + writel(mask, ®s->p2_outp_clr); > > + break; > > + case 3: > > + writel(mask, ®s->p3_outp_clr); > > + break; > > + default: > > + return -1; > > + } > > + return 0; > > +} > > + > > +/** > > + * Set the value of a GPIO > > + */ > > + > > +static int lpc32xx_gpio_set_value(struct udevice *dev, unsigned offset, > > + int value) > > +{ > > + if (value) > > + return gpio_set(offset); > > + else > > + return gpio_clr(offset); > > +} > > + > > +/** > > + * Configure a GPIO number 'offset' as output with given initial value. > > + */ > > + > > +static int lpc32xx_gpio_direction_output(struct udevice *dev, unsigned > > offset, > > + int value) > > +{ > > + int port, mask; > > + > > + port = GPIO_TO_PORT(offset); > > + mask = GPIO_TO_MASK(offset); > > + > > + switch (port) { > > + case 0: > > + writel(mask, ®s->p0_dir_set); > > + break; > > + case 1: > > + writel(mask, ®s->p1_dir_set); > > + break; > > + case 2: > > + /* ports 2 and 3 share a common direction */ > > + case 3: > > + writel(mask, ®s->p2_p3_dir_set); > > + break; > > + default: > > + return -1; > > + } > > + > > + lpc32xx_function[offset] = GPIOF_OUTPUT; > > + > > + return lpc32xx_gpio_set_value(dev, offset, value); > > +} > > + > > +static int lpc32xx_gpio_get_function(struct udevice *dev, unsigned offset) > > +{ > > + return lpc32xx_function[offset]; > > +} > > + > > +static const struct dm_gpio_ops gpio_lpc32xx_ops = { > > + .direction_input = lpc32xx_gpio_direction_input, > > + .direction_output = lpc32xx_gpio_direction_output, > > + .get_value = lpc32xx_gpio_get_value, > > + .set_value = lpc32xx_gpio_set_value, > > + .get_function = lpc32xx_gpio_get_function, > > +}; > > + > > +static int lpc32xx_gpio_probe(struct udevice *dev) > > +{ > > + struct gpio_dev_priv *uc_priv = dev->uclass_priv; > > + > > + if (dev->of_offset == -1) { > > + /* Tell the uclass how many GPIOs we have */ > > + uc_priv->gpio_count = LPC32XX_GPIOS; > > + } > > + > > + /* all GPIO functions are unknown until requested */ > > + memset(lpc32xx_function, GPIOF_UNKNOWN, LPC32XX_GPIOS); > > + > > + return 0; > > +} > > + > > +U_BOOT_DRIVER(gpio_lpc32xx) = { > > + .name = "gpio_lpc32xx", > > + .id = UCLASS_GPIO, > > + .ops = &gpio_lpc32xx_ops, > > + .probe = lpc32xx_gpio_probe, > > + .priv_auto_alloc_size = 0, > > You can use sizeof(struct lpc32xx_priv) here and it will automatically > allocate your structure when the device is probed. Yep, that'll get in along with moving the base address and intenal gpio state array. > > +}; > > -- > > 2.1.0 > > > > Regards, > Simon Thanks for your review! Cordialement, Albert ARIBAUD 3ADEV _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot