All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Glass <sjg@chromium.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 08/18] dm: gpio: Add driver for stm32f7 gpio controller
Date: Sun, 9 Apr 2017 13:27:26 -0600	[thread overview]
Message-ID: <CAPnjgZ2Z81_CLNxhb9=uCQd6c-1J98pwqfOm_Qx6QuDq7R6oEw@mail.gmail.com> (raw)
In-Reply-To: <1491342324-5820-9-git-send-email-vikas.manocha@st.com>

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

> +#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.

Also we should not be exporting functions from drivers. Instead here a
pinctrl driver should be accessed via the API in pinctrl.h.

Regards,
Simon

  reply	other threads:[~2017-04-09 19:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-04 21:45 [U-Boot] [PATCH v2 00/18] stm32f7: add sdram & gpio drivers Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 01/18] stm32f7: use clock driver to enable qspi controller clock Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 02/18] stm32f7: sdram: move sdram driver code to ram drivers area Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 03/18] stm32f7: dm: add driver model support for sdram Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 04/18] ARM: DT: stm32f7: add sdram pin contol node Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 05/18] stm32f7: use driver model for sdram initialization Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 06/18] stm32f7: use clock driver to enable sdram controller clock Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 07/18] stm32f7: sdram: use sdram device tree node to configure sdram controller Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 08/18] dm: gpio: Add driver for stm32f7 gpio controller Vikas Manocha
2017-04-09 19:27   ` Simon Glass [this message]
2017-04-10 16:25     ` Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 09/18] ARM: DT: stm32f7: add gpio device tree nodes Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 10/18] stm32f7: use stm32f7 gpio driver supporting driver model Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 11/18] stm32f746: to switch on user LED1 & read user button Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 12/18] stm32f7: stm32f746-disco: read memory info from device tree Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 13/18] stm32f7: enable board info read " Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 14/18] stm32f7: sdram: correct sdram configuration as per micron sdram Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 15/18] stm32f7: increase the max no of pin configuration to 70 Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 16/18] stm32f7: move board specific pin muxing to dts Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 17/18] stm32f7: add support for stm32f769 disco board Vikas Manocha
2017-04-04 21:45 ` [U-Boot] [PATCH v2 18/18] stm32f7: remove not needed configuration from board config Vikas Manocha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPnjgZ2Z81_CLNxhb9=uCQd6c-1J98pwqfOm_Qx6QuDq7R6oEw@mail.gmail.com' \
    --to=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    --subject='Re: [U-Boot] [PATCH v2 08/18] dm: gpio: Add driver for stm32f7 gpio controller' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.