[PATCH v1] gpio: octeon_gpio: Add GPIO controller driver for Octeon
Stefan Roese
sr at denx.de
Tue May 26 12:59:41 CEST 2020
Hi Simon,
On 25.05.20 19:03, Simon Glass wrote:
> On Thu, 14 May 2020 at 08:26, Stefan Roese <sr at denx.de> wrote:
>>
>> From: Suneel Garapati <sgarapati at marvell.com>
>>
>> Add support for GPIO controllers found on Octeon II/III and Octeon TX
>> TX2 SoC platforms.
>>
>> Signed-off-by: Aaron Williams <awilliams at marvell.com>
>> Signed-off-by: Suneel Garapati <sgarapati at marvell.com>
>> Cc: Simon Glass <sjg at chromium.org>
>> Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
>> Cc: Aaron Williams <awilliams at marvell.com>
>> Cc: Chandrakala Chavva <cchavva at marvell.com>
>> ---
>> RFC -> v1 (Stefan)
>> - Separated this patch from the OcteonTX/TX2 RFC patch series into a
>> single patch. This is useful, as the upcoming MIPS Octeon support will
>> use this GPIO driver.
>> - Added MIPS Octeon II/III support (big endian). Rename driver and its
>> function names from "octeontx" to "octeon" to better match all Octeon
>> platforms.
>> - Moved from union to defines / bitmasks. This makes the driver usage
>> on little- and big-endian platforms much easier.
>> - Used clrbits_64() instead of clrbits_le64() and friends to support
>> usage on little- and big-endian systems
>> - Removed dev->req_seq assignment
>> - Enhanced Kconfig text
>> - Rewrote GPIO_BIT macro
>> - Dropped many macros to calculate the registers offsets and implemented
>> simple functions for this (easier to read)
>> - Used GENMASK_ULL and FIELD_GET helpers
>> - Minor cosmetic changes (dropped brackets etc)
>> - Reword commit text and subject
>>
>> drivers/gpio/Kconfig | 10 ++
>> drivers/gpio/Makefile | 1 +
>> drivers/gpio/octeon_gpio.c | 241 +++++++++++++++++++++++++++++++++++++
>> 3 files changed, 252 insertions(+)
>> create mode 100644 drivers/gpio/octeon_gpio.c
>>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index 2081520f42..9c72930268 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -316,6 +316,16 @@ config PIC32_GPIO
>> help
>> Say yes here to support Microchip PIC32 GPIOs.
>>
>> +config OCTEON_GPIO
>> + bool "Octeon II/III/TX/TX2 GPIO driver"
>> + depends on DM_GPIO && (ARCH_OCTEON || ARCH_OCTEONTX || ARCH_OCTEONTX2)
>> + default y
>> + help
>> + Add support for the Marvell Octeon GPIO driver. This is used with
>> + various Octeon parts such as Octeon II/III and OcteonTX/TX2.
>> + Octeon II/III has 32 GPIOs (count defined via DT) and OcteonTX/TX2
>> + has 64 GPIOs (count defined via internal register).
>> +
>> config STM32_GPIO
>> bool "ST STM32 GPIO driver"
>> depends on DM_GPIO && (STM32 || ARCH_STM32MP)
>> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
>> index 7638259007..eb6364adb4 100644
>> --- a/drivers/gpio/Makefile
>> +++ b/drivers/gpio/Makefile
>> @@ -58,6 +58,7 @@ obj-$(CONFIG_HIKEY_GPIO) += hi6220_gpio.o
>> obj-$(CONFIG_HSDK_CREG_GPIO) += hsdk-creg-gpio.o
>> obj-$(CONFIG_IMX_RGPIO2P) += imx_rgpio2p.o
>> obj-$(CONFIG_PIC32_GPIO) += pic32_gpio.o
>> +obj-$(CONFIG_OCTEON_GPIO) += octeon_gpio.o
>> obj-$(CONFIG_MVEBU_GPIO) += mvebu_gpio.o
>> obj-$(CONFIG_MSM_GPIO) += msm_gpio.o
>> obj-$(CONFIG_$(SPL_)PCF8575_GPIO) += pcf8575_gpio.o
>> diff --git a/drivers/gpio/octeon_gpio.c b/drivers/gpio/octeon_gpio.c
>> new file mode 100644
>> index 0000000000..8079f6c5d9
>> --- /dev/null
>> +++ b/drivers/gpio/octeon_gpio.c
>> @@ -0,0 +1,241 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Marvell International Ltd.
>> + *
>> + * (C) Copyright 2011
>> + * eInfochips Ltd. <www.einfochips.com>
>> + * Written-by: Ajay Bhargav <ajay.bhargav at einfochips.com>
>> + *
>> + * (C) Copyright 2010
>> + * Marvell Semiconductor <www.marvell.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <errno.h>
>> +#include <fdtdec.h>
>> +#include <pci_ids.h>
>> +#include <asm/bitops.h>
>> +#include <asm/gpio.h>
>> +#include <asm/io.h>
>> +#include <linux/bitfield.h>
>> +#include <dt-bindings/gpio/gpio.h>
>> +
>> +/* Returns the bit value to write or read based on the offset */
>> +#define GPIO_BIT(x) BIT_ULL((x) & 0x3f)
>> +
>> +/* Some offsets which are different on Octeon & OcteonTX/TX2 */
>> +#if defined(CONFIG_ARCH_OCTEON)
>> +#define REG_OFFS 0x80
>> +#define GPIO_BIT_CFG_OFFS 0x100
>> +#else
>> +#define REG_OFFS 0x00
>> +#define GPIO_BIT_CFG_OFFS 0x400
>> +#endif
>> +
>> +#define GPIO_RX_DAT (REG_OFFS + 0x00)
>> +#define GPIO_TX_SET (REG_OFFS + 0x08)
>> +#define GPIO_TX_CLR (REG_OFFS + 0x10)
>> +#define GPIO_CONST (REG_OFFS + 0x90) /* OcteonTX only */
>> +
>> +/* Offset to register-set for 2nd GPIOs (> 63), OcteonTX only */
>> +#define GPIO1_OFFSET 0x1400
>> +
>> +/* GPIO_CONST register bits */
>> +#define GPIO_CONST_GPIOS_MASK GENMASK_ULL(7, 0)
>> +
>> +/* GPIO_BIT_CFG register bits */
>> +#define GPIO_BIT_CFG_TX_OE BIT_ULL(0)
>> +#define GPIO_BIT_CFG_PIN_XOR BIT_ULL(1)
>> +#define GPIO_BIT_CFG_INT_EN BIT_ULL(2)
>> +#define GPIO_BIT_CFG_PIN_SEL_MASK GENMASK_ULL(26, 16)
>> +
>> +struct octeon_gpio {
>> + void __iomem *base;
>> +};
>> +
>> +/* Returns the offset to the output register based on the offset and value */
>> +static u32 gpio_tx_reg(int offset, int value)
>> +{
>> + u32 ret;
>> +
>> + ret = value ? GPIO_TX_SET : GPIO_TX_CLR;
>> + if (offset > 63)
>> + ret += GPIO1_OFFSET;
>> +
>> + return ret;
>> +}
>> +
>> +/* Returns the offset to the input data register based on the offset */
>> +static u32 gpio_rx_dat_reg(int offset)
>> +{
>> + u32 ret;
>> +
>> + ret = GPIO_RX_DAT;
>> + if (offset > 63)
>> + ret += GPIO1_OFFSET;
>> +
>> + return ret;
>> +}
>> +
>> +/* Returns the bit configuration register based on the offset */
>> +static u32 gpio_bit_cfg_reg(int offset)
>> +{
>> + return GPIO_BIT_CFG_OFFS + 8 * offset;
>> +}
>> +
>> +static int octeon_gpio_dir_input(struct udevice *dev, unsigned int offset)
>> +{
>> + struct octeon_gpio *gpio = dev_get_priv(dev);
>> +
>> + debug("%s(%s, %u)\n", __func__, dev->name, offset);
>> + clrbits_64(gpio->base + gpio_bit_cfg_reg(offset),
>> + GPIO_BIT_CFG_TX_OE | GPIO_BIT_CFG_PIN_XOR |
>> + GPIO_BIT_CFG_INT_EN | GPIO_BIT_CFG_PIN_SEL_MASK);
>> +
>> + return 0;
>> +}
>> +
>> +static int octeon_gpio_dir_output(struct udevice *dev, unsigned int offset,
>> + int value)
>> +{
>> + struct octeon_gpio *gpio = dev_get_priv(dev);
>> +
>> + debug("%s(%s, %u, %d)\n", __func__, dev->name, offset, value);
>> + writeq(GPIO_BIT(offset), gpio->base + gpio_tx_reg(offset, value));
>> +
>> + clrsetbits_64(gpio->base + gpio_bit_cfg_reg(offset),
>> + GPIO_BIT_CFG_PIN_SEL_MASK | GPIO_BIT_CFG_INT_EN,
>> + GPIO_BIT_CFG_TX_OE);
>> +
>> + return 0;
>> +}
>> +
>> +static int octeon_gpio_get_value(struct udevice *dev, unsigned int offset)
>> +{
>> + struct octeon_gpio *gpio = dev_get_priv(dev);
>> + u64 reg = readq(gpio->base + gpio_rx_dat_reg(offset));
>> +
>> + debug("%s(%s, %u): value: %d\n", __func__, dev->name, offset,
>> + !!(reg & GPIO_BIT(offset)));
>> +
>> + return !!(reg & GPIO_BIT(offset));
>> +}
>> +
>> +static int octeon_gpio_set_value(struct udevice *dev,
>> + unsigned int offset, int value)
>> +{
>> + struct octeon_gpio *gpio = dev_get_priv(dev);
>> +
>> + debug("%s(%s, %u, %d)\n", __func__, dev->name, offset, value);
>> + writeq(GPIO_BIT(offset), gpio->base + gpio_tx_reg(offset, value));
>> +
>> + return 0;
>> +}
>> +
>> +static int octeon_gpio_get_function(struct udevice *dev, unsigned int offset)
>> +{
>> + struct octeon_gpio *gpio = dev_get_priv(dev);
>> + u64 val = readq(gpio->base + gpio_bit_cfg_reg(offset));
>> + int pin_sel;
>> +
>> + debug("%s(%s, %u): GPIO_BIT_CFG: 0x%llx\n", __func__, dev->name,
>> + offset, val);
>> + pin_sel = FIELD_GET(GPIO_BIT_CFG_PIN_SEL_MASK, val);
>> + if (pin_sel)
>> + return GPIOF_FUNC;
>> + else if (val & GPIO_BIT_CFG_TX_OE)
>> + return GPIOF_OUTPUT;
>> + else
>> + return GPIOF_INPUT;
>> +}
>> +
>> +static int octeon_gpio_xlate(struct udevice *dev, struct gpio_desc *desc,
>> + struct ofnode_phandle_args *args)
>> +{
>> + if (args->args_count < 1)
>> + return -EINVAL;
>> +
>> + desc->offset = args->args[0];
>> + desc->flags = 0;
>> + if (args->args_count > 1) {
>> + if (args->args[1] & GPIO_ACTIVE_LOW)
>> + desc->flags |= GPIOD_ACTIVE_LOW;
>> + /* In the future add tri-state flag support */
>> + }
>> + return 0;
>> +}
>> +
>> +static const struct dm_gpio_ops octeon_gpio_ops = {
>> + .direction_input = octeon_gpio_dir_input,
>> + .direction_output = octeon_gpio_dir_output,
>> + .get_value = octeon_gpio_get_value,
>> + .set_value = octeon_gpio_set_value,
>> + .get_function = octeon_gpio_get_function,
>> + .xlate = octeon_gpio_xlate,
>> +};
>> +
>> +static int octeon_gpio_probe(struct udevice *dev)
>> +{
>> + struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>> + struct octeon_gpio *priv = dev_get_priv(dev);
>> + char *end;
>> +#if !defined(CONFIG_ARCH_OCTEON)
>> + pci_dev_t bdf = dm_pci_get_bdf(dev);
>> +#endif
>> +
>> +#if !defined(CONFIG_ARCH_OCTEON)
>> + priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0,
>> + PCI_REGION_MEM);
>> +#else
>> + priv->base = dev_remap_addr(dev);
>> +#endif
>> + if (!priv->base) {
>> + debug("%s(%s): Could not get base address\n",
>> + __func__, dev->name);
>> + return -1;
>> + }
>> +
>> +#if !defined(CONFIG_ARCH_OCTEON)
>
> Should not have #ifdefs in drivers. Perhaps use the compatible string?
Yes, will try to remove this.
>> + uc_priv->gpio_count = readq(priv->base + GPIO_CONST) &
>> + GPIO_CONST_GPIOS_MASK;
>> +#else
>> + uc_priv->gpio_count = ofnode_read_u32_default(dev->node,
>> + "nr-gpios", 32);
>> +#endif
>> +
>> + uc_priv->bank_name = strdup(dev->name);
>> + end = strchr(uc_priv->bank_name, '@');
>> + end[0] = 'A' + dev->seq;
>> + end[1] = '\0';
>> +
>> + debug("%s(%s): base address: %p, pin count: %d\n",
>> + __func__, dev->name, priv->base, uc_priv->gpio_count);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct udevice_id octeon_gpio_ids[] = {
>> + { .compatible = "cavium,thunder-8890-gpio" },
>> + { .compatible = "cavium,octeon-7890-gpio" },
>> + { }
>> +};
>> +
>> +U_BOOT_DRIVER(octeon_gpio) = {
>> + .name = "octeon_gpio",
>> + .id = UCLASS_GPIO,
>> + .of_match = of_match_ptr(octeon_gpio_ids),
>> + .probe = octeon_gpio_probe,
>> + .priv_auto_alloc_size = sizeof(struct octeon_gpio),
>> + .ops = &octeon_gpio_ops,
>> + .flags = DM_FLAG_PRE_RELOC,
>> +};
>> +
>> +#if !defined(CONFIG_ARCH_OCTEON)
>
> Is it bad to always enable this and remove the #ifdef?
It didn't work with the base Octeon port, which didn't enable DM_PCI.
But I'll add DM_PCI and then this will work as well.
Thanks,
Stefan
More information about the U-Boot
mailing list