[U-Boot] [U-BOOT PATCH] gpio: fu540: add support for DM based gpio driver for FU540 SoC

Bin Meng bmeng.cn at gmail.com
Fri Aug 23 03:12:44 UTC 2019


On Fri, Aug 23, 2019 at 9:02 AM Sagar Shrikant Kadam
<sagar.kadam at sifive.com> wrote:
>
> This patch adds a DM based driver model for gpio controller present in
> FU540-C000 SoC on HiFive Unleashed A00 board. This SoC has one GPIO
> bank and 16 GPIO lines in total, out of which GPIO0 to GPIO9 and
> GPIO15 are routed to the J1 header on the board.
>
> This implementation is ported from linux based gpio driver submitted
> for review by Wesley W. Terpstra <wesley at sifive.com> and/or Atish Patra
> <atish.patra at wdc.com>. The linux driver can be referred
> here [1]
>
> [1]: https://lkml.org/lkml/2018/10/9/1103
>
> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com>
> ---
>  arch/riscv/include/asm/arch-generic/gpio.h |  35 +++++++
>  arch/riscv/include/asm/gpio.h              |   6 ++
>  board/sifive/fu540/Kconfig                 |   3 +
>  drivers/gpio/Kconfig                       |   8 ++
>  drivers/gpio/Makefile                      |   1 +
>  drivers/gpio/fu540-gpio.c                  | 145 +++++++++++++++++++++++++++++
>  6 files changed, 198 insertions(+)
>  create mode 100644 arch/riscv/include/asm/arch-generic/gpio.h
>  create mode 100644 arch/riscv/include/asm/gpio.h
>  create mode 100644 drivers/gpio/fu540-gpio.c

Pleas split this into two patch:

patch [1/2]: add the gpio driver
patch [2/2]: enable the gpio driver in sifive/fu540

Looks DTS change is missing??

>
> diff --git a/arch/riscv/include/asm/arch-generic/gpio.h b/arch/riscv/include/asm/arch-generic/gpio.h
> new file mode 100644
> index 0000000..bedb8d8
> --- /dev/null
> +++ b/arch/riscv/include/asm/arch-generic/gpio.h
> @@ -0,0 +1,35 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2019 SiFive, Inc.
> + */
> +
> +#ifndef _GPIO_FU540_H
> +#define _GPIO_FU540_H
> +
> +#define GPIO_INPUT_VAL 0x00
> +#define GPIO_INPUT_EN  0x04
> +#define GPIO_OUTPUT_EN 0x08
> +#define GPIO_OUTPUT_VAL        0x0C
> +#define GPIO_RISE_IE   0x18
> +#define GPIO_RISE_IP   0x1C
> +#define GPIO_FALL_IE   0x20
> +#define GPIO_FALL_IP   0x24
> +#define GPIO_HIGH_IE   0x28
> +#define GPIO_HIGH_IP   0x2C
> +#define GPIO_LOW_IE    0x30
> +#define GPIO_LOW_IP    0x34
> +#define GPIO_OUTPUT_XOR        0x40
> +
> +#define NR_GPIOS       16
> +
> +enum gpio_state {
> +       LOW,
> +       HIGH
> +};
> +
> +/* Details about a GPIO bank */
> +struct fu540_gpio_platdata {
> +       u8 *base;     /* address of registers in physical memory */

Shouldn't it be u32?

> +};
> +
> +#endif /* _GPIO_FU540_H */
> diff --git a/arch/riscv/include/asm/gpio.h b/arch/riscv/include/asm/gpio.h
> new file mode 100644
> index 0000000..008d756
> --- /dev/null
> +++ b/arch/riscv/include/asm/gpio.h
> @@ -0,0 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2018 SiFive, Inc.
> + */
> +
> +#include <asm-generic/gpio.h>
> diff --git a/board/sifive/fu540/Kconfig b/board/sifive/fu540/Kconfig
> index 5d65080..f939ed2 100644
> --- a/board/sifive/fu540/Kconfig
> +++ b/board/sifive/fu540/Kconfig
> @@ -44,6 +44,9 @@ config BOARD_SPECIFIC_OPTIONS # dummy
>         imply MMC_SPI
>         imply MMC_BROKEN_CD
>         imply CMD_MMC
> +       imply DM_GPIO
> +       imply FU540_GPIO
> +       imply CMD_GPIO
>         imply SMP
>
>  endif
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 7d9c97f..b93092a 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -280,6 +280,14 @@ config STM32_GPIO
>           usable on many stm32 families like stm32f4/f7/h7 and stm32mp1.
>           Tested on STM32F7.
>
> +config FU540_GPIO

rename this to SIFIVE_GPIO

> +       bool "FU540 GPIO Driver"
> +       depends on DM_GPIO
> +       help
> +         Device model driver for GPIO controller present in FU540 SoC. This
> +         driver enables GPIO interface on HiFive Unleashed A00 board a board
> +         from SiFive Inc. having FU540-C000 SoC.
> +
>  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 4a8aa0f..238ad17 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -61,3 +61,4 @@ obj-$(CONFIG_$(SPL_)PCF8575_GPIO)     += pcf8575_gpio.o
>  obj-$(CONFIG_PM8916_GPIO)      += pm8916_gpio.o
>  obj-$(CONFIG_MT7621_GPIO)      += mt7621_gpio.o
>  obj-$(CONFIG_MSCC_SGPIO)       += mscc_sgpio.o
> +obj-$(CONFIG_FU540_GPIO)       += fu540-gpio.o
> diff --git a/drivers/gpio/fu540-gpio.c b/drivers/gpio/fu540-gpio.c

I think the name should be: sifive_gpio.c

> new file mode 100644
> index 0000000..7761689
> --- /dev/null
> +++ b/drivers/gpio/fu540-gpio.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * SiFive GPIO driver
> + *
> + * Copyright (C) 2019 SiFive, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.

No need this because there is already SPDX in the first line.

> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <asm/arch/gpio.h>
> +#include <asm/io.h>
> +#include <errno.h>
> +#include <asm-generic/gpio.h>

I think you need include <asm/gpio.h>?

> +
> +static int fu540_gpio_probe(struct udevice *dev)
> +{
> +       struct fu540_gpio_platdata *plat = dev_get_platdata(dev);
> +       struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> +       char name[18], *str;
> +
> +       sprintf(name, "gpio@%4p", plat->base);
> +       str = strdup(name);
> +       if (!str)
> +               return -ENOMEM;
> +       uc_priv->bank_name = str;

Shouldn't we directly use dev->name as the bank_name?

> +       uc_priv->gpio_count = NR_GPIOS;
> +
> +       return 0;
> +}
> +
> +static void fu540_update_gpio_reg(u8 *bptr, u32 offset, bool value)

Why bool value?

Linux driver uses int value, and why not you just reuse the same Linux
function name (sifive_assign_bit) and parameters?

> +{
> +       void __iomem *ptr = (void __iomem *)bptr;
> +
> +       u32 bit = BIT(offset), old = readl(ptr);

nits: don't do this in a single line

> +
> +       if (value)
> +               writel(old | bit, ptr);
> +       else
> +               writel(old & ~bit, ptr);
> +}
> +
> +static int fu540_gpio_direction_input(struct udevice *dev, u32 offset)
> +{
> +       struct fu540_gpio_platdata *plat = dev_get_platdata(dev);
> +
> +       if (offset > NR_GPIOS)
> +               return -EINVAL;
> +
> +       /* Configure GPIO direction as input. */

nits: remove the ending period

> +       fu540_update_gpio_reg(plat->base + GPIO_INPUT_EN,  offset, true);
> +       fu540_update_gpio_reg(plat->base + GPIO_OUTPUT_EN, offset, false);
> +
> +       return 0;
> +}
> +
> +static int fu540_gpio_direction_output(struct udevice *dev, u32 offset,
> +                                      int value)
> +{
> +       struct fu540_gpio_platdata *plat = dev_get_platdata(dev);
> +
> +       if (offset > NR_GPIOS)
> +               return -EINVAL;
> +
> +       /* Configure GPIO direction as output. */

nits: remove the ending period

> +       fu540_update_gpio_reg(plat->base + GPIO_OUTPUT_EN, offset, true);
> +       fu540_update_gpio_reg(plat->base + GPIO_INPUT_EN,  offset, false);
> +
> +       /* Set the Output state of the PIN */
> +       fu540_update_gpio_reg(plat->base + GPIO_OUTPUT_VAL, offset, value);
> +
> +       return 0;
> +}
> +
> +static int fu540_gpio_get_value(struct udevice *dev, u32 offset)
> +{
> +       struct fu540_gpio_platdata *plat = dev_get_platdata(dev);
> +       int val;
> +       int dir;
> +
> +       if (offset > NR_GPIOS)
> +               return -EINVAL;
> +
> +       /* Get direction of the pin OUTPUT=0 INPUT=1 */
> +       dir = !(readl(plat->base + GPIO_OUTPUT_EN) & BIT(offset));
> +
> +       if (dir)
> +               val = readl(plat->base + GPIO_INPUT_VAL) & BIT(offset);
> +       else
> +               val = readl(plat->base + GPIO_OUTPUT_VAL) & BIT(offset);
> +
> +       return val ? HIGH : LOW;
> +}
> +
> +static int fu540_gpio_set_value(struct udevice *dev, u32 offset, int value)
> +{
> +       struct fu540_gpio_platdata *plat = dev_get_platdata(dev);
> +
> +       if (offset > NR_GPIOS)
> +               return -EINVAL;
> +
> +       fu540_update_gpio_reg(plat->base + GPIO_OUTPUT_VAL, offset, value);
> +
> +       return 0;
> +}
> +
> +static const struct udevice_id fu540_gpio_match[] = {
> +       { .compatible = "sifive,gpio0" },
> +       { }
> +};
> +
> +static const struct dm_gpio_ops gpio_sifive_ops = {
> +       .direction_input        = fu540_gpio_direction_input,
> +       .direction_output       = fu540_gpio_direction_output,
> +       .get_value              = fu540_gpio_get_value,
> +       .set_value              = fu540_gpio_set_value,
> +};
> +
> +static int fu540_gpio_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct fu540_gpio_platdata *plat = dev_get_platdata(dev);
> +       fdt_addr_t addr;
> +
> +       addr = devfdt_get_addr(dev);
> +       if (addr == FDT_ADDR_T_NONE)
> +               return -EINVAL;
> +
> +       plat->base = (u8 *)addr;
> +       return 0;
> +}
> +
> +U_BOOT_DRIVER(gpio_sifive) = {
> +       .name   = "gpio_sifive",
> +       .id     = UCLASS_GPIO,
> +       .of_match = fu540_gpio_match,
> +       .ofdata_to_platdata = of_match_ptr(fu540_gpio_ofdata_to_platdata),
> +       .platdata_auto_alloc_size = sizeof(struct fu540_gpio_platdata),
> +       .ops    = &gpio_sifive_ops,
> +       .probe  = fu540_gpio_probe,
> +       .priv_auto_alloc_size   = sizeof(struct fu540_gpio_platdata),

This priv_auto_alloc_size is not used anywhere in this driver.

Regards,
Bin


More information about the U-Boot mailing list