On Fri, Aug 23, 2019 at 9:02 AM Sagar Shrikant Kadam <sagar.ka...@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 <wes...@sifive.com> and/or Atish Patra > <atish.pa...@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.ka...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot