Thanks Simon,
On 03/31/2017 09:20 PM, Simon Glass wrote:
Hi,
On 20 March 2017 at 14:48, Vikas Manocha vikas.manocha@st.com wrote:
This patch adds gpio driver supporting driver model for stm32f7 gpio.
Signed-off-by: Vikas Manocha vikas.manocha@st.com cc: Christophe KERELLO christophe.kerello@st.com
drivers/gpio/Kconfig | 9 +++ drivers/gpio/Makefile | 1 + drivers/gpio/stm32f7_gpio.c | 189 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 199 insertions(+) create mode 100644 drivers/gpio/stm32f7_gpio.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 8d9ab52..c8af398 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -151,6 +151,15 @@ config PIC32_GPIO help Say yes here to support Microchip PIC32 GPIOs.
+config STM32F7_GPIO
bool "ST STM32 GPIO driver"depends on DM_GPIOdefault yhelpDevice model driver support for STM32 GPIO controller. It should beusable on many stm32 families like stm32f4 & stm32H7.Tested on STM32F7.config MVEBU_GPIO bool "Marvell MVEBU GPIO driver" depends on DM_GPIO && ARCH_MVEBU diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 8939226..9c2a9cc 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -49,6 +49,7 @@ oby-$(CONFIG_SX151X) += sx151x.o obj-$(CONFIG_SUNXI_GPIO) += sunxi_gpio.o obj-$(CONFIG_LPC32XX_GPIO) += lpc32xx_gpio.o obj-$(CONFIG_STM32_GPIO) += stm32_gpio.o +obj-$(CONFIG_STM32F7_GPIO) += stm32f7_gpio.o obj-$(CONFIG_GPIO_UNIPHIER) += gpio-uniphier.o obj-$(CONFIG_ZYNQ_GPIO) += zynq_gpio.o obj-$(CONFIG_VYBRID_GPIO) += vybrid_gpio.o diff --git a/drivers/gpio/stm32f7_gpio.c b/drivers/gpio/stm32f7_gpio.c new file mode 100644 index 0000000..369227a --- /dev/null +++ b/drivers/gpio/stm32f7_gpio.c @@ -0,0 +1,189 @@ +/*
- (C) Copyright 2017
- Vikas Manocha, vikas.manocha@st.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <clk.h> +#include <dm.h> +#include <fdtdec.h> +#include <linux/io.h> +#include <linux/errno.h>
linux/ comes after asm/ so put those two at the bottom.
ok
+#include <asm/io.h> +#include <asm/arch/stm32.h> +#include <asm/arch/gpio.h> +#include <asm/gpio.h>
+#define STM32_GPIOS_PER_BANK 16 +#define MAX_SIZE_BANK_NAME 6 +#define MODE_BITS(gpio_pin) (gpio_pin * 2) +#define MODE_BITS_MASK 3 +#define OSPEED_MASK 3 +#define PUPD_MASK 3 +#define OTYPE_MSK 1 +#define AFR_MASK 0xF +#define IN_OUT_BIT_INDEX(gpio_pin) (1UL << (gpio_pin))
+DECLARE_GLOBAL_DATA_PTR;
+struct stm32_gpio_regs {
u32 moder; /* GPIO port mode */u32 otyper; /* GPIO port output type */u32 ospeedr; /* GPIO port output speed */u32 pupdr; /* GPIO port pull-up/pull-down */u32 idr; /* GPIO port input data */u32 odr; /* GPIO port output data */u32 bsrr; /* GPIO port bit set/reset */u32 lckr; /* GPIO port configuration lock */u32 afr[2]; /* GPIO alternate function */+};
+struct stm32_gpio_priv {
struct stm32_gpio_regs *regs;char name[MAX_SIZE_BANK_NAME];+};
+#define CHECK_CTL(x) (!x || x->af > 15 || x->mode > 3 || x->otype > 1 || \
x->pupd > 2 || x->speed > 3)You only use this once, so can you just inline it?
Yes, i will inline it in v2.
+int stm32_gpio_config(struct gpio_desc *desc, const struct stm32_gpio_ctl *ctl) +{
Where is this function called? Should this be in a pinctrl driver? You cannot call directly into a driver from outside.
Yes, it will be better to keep it in pinctrl driverl. Let me do it in v2.
struct stm32_gpio_priv *priv = dev_get_priv(desc->dev);struct stm32_gpio_regs *regs = priv->regs;u32 index;if (CHECK_CTL(ctl))return -EINVAL;index = (desc->offset & 0x07) * 4;clrsetbits_le32(®s->afr[desc->offset >> 3], AFR_MASK << index,ctl->af << index);index = desc->offset * 2;clrsetbits_le32(®s->moder, MODE_BITS_MASK << index,ctl->mode << index);clrsetbits_le32(®s->ospeedr, OSPEED_MASK << index,ctl->speed << index);clrsetbits_le32(®s->pupdr, PUPD_MASK << index, ctl->pupd << index);index = desc->offset;clrsetbits_le32(®s->otyper, OTYPE_MSK << index, ctl->otype << index);return 0;+}
+static int stm32_gpio_direction_input(struct udevice *dev, unsigned offset) +{
struct stm32_gpio_priv *priv = dev_get_priv(dev);struct stm32_gpio_regs *regs = priv->regs;int bits_index = MODE_BITS(offset);int mask = MODE_BITS_MASK << bits_index;clrsetbits_le32(®s->moder, mask, STM32_GPIO_MODE_IN << bits_index);return 0;+}
+static int stm32_gpio_direction_output(struct udevice *dev, unsigned offset,
int value)+{
struct stm32_gpio_priv *priv = dev_get_priv(dev);struct stm32_gpio_regs *regs = priv->regs;int bits_index = MODE_BITS(offset);int mask = MODE_BITS_MASK << bits_index;clrsetbits_le32(®s->moder, mask, STM32_GPIO_MODE_OUT << bits_index);mask = IN_OUT_BIT_INDEX(offset);clrsetbits_le32(®s->odr, mask, value ? mask : 0);return 0;+}
+static int stm32_gpio_get_value(struct udevice *dev, unsigned offset) +{
struct stm32_gpio_priv *priv = dev_get_priv(dev);struct stm32_gpio_regs *regs = priv->regs;return readl(®s->idr) & IN_OUT_BIT_INDEX(offset) ? 1 : 0;+}
+static int stm32_gpio_set_value(struct udevice *dev, unsigned offset, int value) +{
struct stm32_gpio_priv *priv = dev_get_priv(dev);struct stm32_gpio_regs *regs = priv->regs;int mask = IN_OUT_BIT_INDEX(offset);clrsetbits_le32(®s->odr, mask, value ? mask : 0);return 0;+}
+static const struct dm_gpio_ops gpio_stm32_ops = {
.direction_input = stm32_gpio_direction_input,.direction_output = stm32_gpio_direction_output,.get_value = stm32_gpio_get_value,.set_value = stm32_gpio_set_value,Do you want to implement get_function()?
not now, may be another patch.
+};
+static int gpio_stm32_probe(struct udevice *dev) +{
struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);struct stm32_gpio_priv *priv = dev_get_priv(dev);fdt_addr_t addr;int ret;addr = fdtdec_get_addr_size_auto_parent(gd->fdt_blob,dev->parent->of_offset,dev->of_offset, "reg", 0, NULL,true);Can you use dev_get_addr() or dev_get_addr_ptr()? Also we don't want to use of_offset directly. Use dev_of_offset().
agree, i will update it to dev_get_addr() in v2.
if (addr == FDT_ADDR_T_NONE)return -EINVAL;priv->regs = (struct stm32_gpio_regs *)addr;ret = fdtdec_get_byte_array(gd->fdt_blob, dev->of_offset,"st,bank-name", (u8 *)priv->name,sizeof(priv->name) - 1);if (ret)return ret;Can you not just make priv->name point to the property value? Why copy it?
you mean by using fdtdec_locate_byte_array()...
Cheers, Vikas
priv->name[MAX_SIZE_BANK_NAME - 1] = '\0';uc_priv->gpio_count = STM32_GPIOS_PER_BANK;uc_priv->bank_name = priv->name;debug("%s, addr = 0x%p, bank_name = %s\n", __func__, (u32 *)priv->regs,uc_priv->bank_name);+#ifdef CONFIG_CLK
struct clk clk;ret = clk_get_by_index(dev, 0, &clk);if (ret < 0)return ret;ret = clk_enable(&clk);if (ret) {dev_err(dev, "failed to enable clock\n");return ret;}debug("clock enabled for device %s\n", dev->name);+#endif
return 0;+}
+static const struct udevice_id stm32_gpio_ids[] = {
{ .compatible = "st,stm32-gpio" },{ }+};
+U_BOOT_DRIVER(gpio_stm32) = {
.name = "gpio_stm32",.id = UCLASS_GPIO,.of_match = stm32_gpio_ids,.probe = gpio_stm32_probe,.ops = &gpio_stm32_ops,.flags = DM_FLAG_PRE_RELOC | DM_UC_FLAG_SEQ_ALIAS,.priv_auto_alloc_size = sizeof(struct stm32_gpio_priv),+};
1.9.1
Regards, Simon .