
Thanks Simon,
On 04/09/2017 12:27 PM, Simon Glass wrote:
Hi Vikas,
On 4 April 2017 at 15:45, 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
Changes in v2:
- included files in correct order.
- moved the pinctrl specific routine from gpio driver to pinctrl
- used dev_get_addr() instead of fdtdec_get_addr_size_auto_parent() in gpio driver.
- pointed gpio name to bank name in device tree blob rather than copy.
arch/arm/include/asm/arch-stm32f7/gpio.h | 16 ++++ drivers/gpio/Kconfig | 9 +++ drivers/gpio/Makefile | 1 + drivers/gpio/stm32f7_gpio.c | 135 +++++++++++++++++++++++++++++++ drivers/pinctrl/pinctrl_stm32.c | 36 ++++++++- 5 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 drivers/gpio/stm32f7_gpio.c
Reviewed-by: Simon Glass sjg@chromium.org
But please see below.
diff --git a/arch/arm/include/asm/arch-stm32f7/gpio.h b/arch/arm/include/asm/arch-stm32f7/gpio.h index 2942cd9..5c0300f 100644 --- a/arch/arm/include/asm/arch-stm32f7/gpio.h +++ b/arch/arm/include/asm/arch-stm32f7/gpio.h @@ -96,6 +96,22 @@ struct stm32_gpio_ctl { enum stm32_gpio_af af; };
+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;
+};
static inline unsigned stm32_gpio_to_port(unsigned gpio) { return gpio / 16; 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_GPIO
default y
help
Device model driver support for STM32 GPIO controller. It should be
usable 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..5ab1c5c --- /dev/null +++ b/drivers/gpio/stm32f7_gpio.c @@ -0,0 +1,135 @@ +/*
- (C) Copyright 2017
- Vikas Manocha, vikas.manocha@st.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <clk.h> +#include <common.h>
common.h should always be first
ok, I will move it before clk.h in v3.
+#include <dm.h> +#include <fdtdec.h> +#include <asm/arch/gpio.h> +#include <asm/arch/stm32.h> +#include <asm/gpio.h> +#include <asm/io.h> +#include <linux/errno.h> +#include <linux/io.h>
[..]
diff --git a/drivers/pinctrl/pinctrl_stm32.c b/drivers/pinctrl/pinctrl_stm32.c index aa2c440..e5b6b3b 100644 --- a/drivers/pinctrl/pinctrl_stm32.c +++ b/drivers/pinctrl/pinctrl_stm32.c @@ -1,10 +1,44 @@ #include <common.h> -#include <asm/arch/gpio.h> #include <dm.h> #include <dm/pinctrl.h> +#include <asm/arch/gpio.h> +#include <asm/gpio.h> +#include <asm/io.h>
DECLARE_GLOBAL_DATA_PTR;
+#define MODE_BITS_MASK 3 +#define OSPEED_MASK 3 +#define PUPD_MASK 3 +#define OTYPE_MSK 1 +#define AFR_MASK 0xF
+int stm32_gpio_config(struct gpio_desc *desc, const struct stm32_gpio_ctl *ctl)
What is this function for? It does not seem to be called.
This function is being called from stm32_pinctrl_set_state_simple() in same pinctrl driver.
Also we should not be exporting functions from drivers. Instead here a pinctrl driver should be accessed via the API in pinctrl.h.
pinctrl driver is accessed via the the APIs only, the function stm32_gpio_config should be static to avoid this confusion. I will make it static in v3.
Cheers, Vikas
Regards, Simon .