* [PATCH v2 0/2] gpio: Add a driver for Cadence GPIO controller @ 2018-12-17 15:36 Jan Kotas 2018-12-17 15:36 ` [PATCH v2 1/2] dt-bindings: gpio: Add bindings for Cadence GPIO Jan Kotas 2018-12-17 15:36 ` [PATCH v2 2/2] gpio: Add Cadence GPIO driver Jan Kotas 0 siblings, 2 replies; 11+ messages in thread From: Jan Kotas @ 2018-12-17 15:36 UTC (permalink / raw) To: linus.walleij, bgolaszewski, robh+dt, mark.rutland Cc: linux-gpio, devicetree, linux-kernel, Jan Kotas This patchset adds a driver support for Cadence GPIO controller. Number of supported lines is configurable. The driver initializes all pins as inputs in probe(). Currently it only supports level-sensitive interrupts. The interrupts controller was tested with I2C and SPI IPs acting as interrupt sources. Changes since V1: Switched to generic GPIO infrastructure. Redesigned interrupts, now uses chained irqchip. Drop support for edge-sensitive interrupts. Restore bypass settings at unexport/remove. Jan Kotas (2): dt-bindings: gpio: Add bindings for Cadence GPIO gpio: Add Cadence GPIO driver .../devicetree/bindings/gpio/cdns,gpio.txt | 44 ++++ drivers/gpio/Kconfig | 8 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-cadence.c | 275 +++++++++++++++++++++ 4 files changed, 328 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/cdns,gpio.txt create mode 100644 drivers/gpio/gpio-cadence.c -- 2.15.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] dt-bindings: gpio: Add bindings for Cadence GPIO 2018-12-17 15:36 [PATCH v2 0/2] gpio: Add a driver for Cadence GPIO controller Jan Kotas @ 2018-12-17 15:36 ` Jan Kotas 2018-12-18 17:33 ` Rob Herring 2018-12-17 15:36 ` [PATCH v2 2/2] gpio: Add Cadence GPIO driver Jan Kotas 1 sibling, 1 reply; 11+ messages in thread From: Jan Kotas @ 2018-12-17 15:36 UTC (permalink / raw) To: linus.walleij, bgolaszewski, robh+dt, mark.rutland Cc: linux-gpio, devicetree, linux-kernel, Jan Kotas This patch adds a DT binding documentation for Cadence GPIO controller. Signed-off-by: Jan Kotas <jank@cadence.com> --- .../devicetree/bindings/gpio/cdns,gpio.txt | 44 ++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/cdns,gpio.txt diff --git a/Documentation/devicetree/bindings/gpio/cdns,gpio.txt b/Documentation/devicetree/bindings/gpio/cdns,gpio.txt new file mode 100644 index 000000000..c87a47827 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/cdns,gpio.txt @@ -0,0 +1,44 @@ +Cadence GPIO controller bindings + +Required properties: +- compatible: should be "cdns,gpio-r1p02". +- reg: the register base address and size. +- #gpio-cells: should be 2. + * first cell is the GPIO number. + * second cell specifies the GPIO flags, as defined in + <dt-bindings/gpio/gpio.h>. Only the GPIO_ACTIVE_HIGH + and GPIO_ACTIVE_LOW flags are supported. +- gpio-controller: marks the device as a GPIO controller. +- clocks: should contain one entry referencing the peripheral clock driving + the GPIO controller. + +Optional properties: +- ngpios: integer number of gpio lines supported by this controller, up to 32. +- interrupt-parent: phandle of the parent interrupt controller. +- interrupts: interrupt specifier for the controllers interrupt. +- interrupt-controller: marks the device as an interrupt controller. When + defined, interrupts, interrupt-parent and #interrupt-cells + are required. +- interrupt-cells: should be 2. + * first cell is the GPIO number you want to use as an IRQ source. + * second cell specifies the IRQ type, as defined in + <dt-bindings/interrupt-controller/irq.h>. + Currently only level sensitive IRQs are supported. + + +Example: + gpio0: gpio-controller@fd060000 { + compatible = "cdns,gpio-r1p02"; + reg =<0xfd060000 0x1000>; + + clocks = <&gpio_clk>; + + interrupt-parent = <&gic>; + interrupts = <0 5 IRQ_TYPE_LEVEL_HIGH>; + + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; -- 2.15.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] dt-bindings: gpio: Add bindings for Cadence GPIO 2018-12-17 15:36 ` [PATCH v2 1/2] dt-bindings: gpio: Add bindings for Cadence GPIO Jan Kotas @ 2018-12-18 17:33 ` Rob Herring 0 siblings, 0 replies; 11+ messages in thread From: Rob Herring @ 2018-12-18 17:33 UTC (permalink / raw) To: Jan Kotas Cc: linus.walleij, bgolaszewski, mark.rutland, linux-gpio, devicetree, linux-kernel On Mon, Dec 17, 2018 at 03:36:51PM +0000, Jan Kotas wrote: > This patch adds a DT binding documentation for > Cadence GPIO controller. > > Signed-off-by: Jan Kotas <jank@cadence.com> > --- > .../devicetree/bindings/gpio/cdns,gpio.txt | 44 ++++++++++++++++++++++ > 1 file changed, 44 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpio/cdns,gpio.txt > > diff --git a/Documentation/devicetree/bindings/gpio/cdns,gpio.txt b/Documentation/devicetree/bindings/gpio/cdns,gpio.txt > new file mode 100644 > index 000000000..c87a47827 > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/cdns,gpio.txt > @@ -0,0 +1,44 @@ > +Cadence GPIO controller bindings > + > +Required properties: > +- compatible: should be "cdns,gpio-r1p02". > +- reg: the register base address and size. > +- #gpio-cells: should be 2. > + * first cell is the GPIO number. > + * second cell specifies the GPIO flags, as defined in > + <dt-bindings/gpio/gpio.h>. Only the GPIO_ACTIVE_HIGH > + and GPIO_ACTIVE_LOW flags are supported. > +- gpio-controller: marks the device as a GPIO controller. > +- clocks: should contain one entry referencing the peripheral clock driving > + the GPIO controller. > + > +Optional properties: > +- ngpios: integer number of gpio lines supported by this controller, up to 32. > +- interrupt-parent: phandle of the parent interrupt controller. Don't document interrupt-parent as it is implied and could be in a parent. Otherwise, Reviewed-by: Rob Herring <robh@kernel.org> Rob ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] gpio: Add Cadence GPIO driver 2018-12-17 15:36 [PATCH v2 0/2] gpio: Add a driver for Cadence GPIO controller Jan Kotas 2018-12-17 15:36 ` [PATCH v2 1/2] dt-bindings: gpio: Add bindings for Cadence GPIO Jan Kotas @ 2018-12-17 15:36 ` Jan Kotas 2018-12-17 15:51 ` Bartosz Golaszewski 2018-12-21 10:34 ` Linus Walleij 1 sibling, 2 replies; 11+ messages in thread From: Jan Kotas @ 2018-12-17 15:36 UTC (permalink / raw) To: linus.walleij, bgolaszewski, robh+dt, mark.rutland Cc: linux-gpio, devicetree, linux-kernel, Jan Kotas This patch adds a driver for Cadence GPIO controller. It can be enabled with GPIO_CADENCE Kconfig option. It uses generic GPIO infrastructure and works as an interrupt controller. At the moment it only supports level sensitive irqs. Signed-off-by: Jan Kotas <jank@cadence.com> --- drivers/gpio/Kconfig | 8 ++ drivers/gpio/Makefile | 1 + drivers/gpio/gpio-cadence.c | 275 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 284 insertions(+) create mode 100644 drivers/gpio/gpio-cadence.c diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 833a1b51c..8259f8f25 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -160,6 +160,14 @@ config GPIO_BRCMSTB help Say yes here to enable GPIO support for Broadcom STB (BCM7XXX) SoCs. +config GPIO_CADENCE + tristate "Cadence GPIO support" + depends on OF_GPIO + select GPIO_GENERIC + select GPIOLIB_IRQCHIP + help + Say yes here to enable support for Cadence GPIO controller. + config GPIO_CLPS711X tristate "CLPS711X GPIO support" depends on ARCH_CLPS711X || COMPILE_TEST diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index 671c4477c..5fce8634a 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -37,6 +37,7 @@ obj-$(CONFIG_GPIO_BCM_KONA) += gpio-bcm-kona.o obj-$(CONFIG_GPIO_BD9571MWV) += gpio-bd9571mwv.o obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o obj-$(CONFIG_GPIO_BT8XX) += gpio-bt8xx.o +obj-$(CONFIG_GPIO_CADENCE) += gpio-cadence.o obj-$(CONFIG_GPIO_CLPS711X) += gpio-clps711x.o obj-$(CONFIG_GPIO_CS5535) += gpio-cs5535.o obj-$(CONFIG_GPIO_CRYSTAL_COVE) += gpio-crystalcove.o diff --git a/drivers/gpio/gpio-cadence.c b/drivers/gpio/gpio-cadence.c new file mode 100644 index 000000000..16cc9f50f --- /dev/null +++ b/drivers/gpio/gpio-cadence.c @@ -0,0 +1,275 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Copyright 2017-2018 Cadence + * + * Authors: + * Jan Kotas <jank@cadence.com> + * Boris Brezillon <boris.brezillon@free-electrons.com> + */ + +#include <linux/gpio/driver.h> +#include <linux/clk.h> +#include <linux/interrupt.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/slab.h> + +#define CDNS_GPIO_BYPASS_MODE 0x00 +#define CDNS_GPIO_DIRECTION_MODE 0x04 +#define CDNS_GPIO_OUTPUT_EN 0x08 +#define CDNS_GPIO_OUTPUT_VALUE 0x0c +#define CDNS_GPIO_INPUT_VALUE 0x10 +#define CDNS_GPIO_IRQ_MASK 0x14 +#define CDNS_GPIO_IRQ_EN 0x18 +#define CDNS_GPIO_IRQ_DIS 0x1c +#define CDNS_GPIO_IRQ_STATUS 0x20 +#define CDNS_GPIO_IRQ_TYPE 0x24 +#define CDNS_GPIO_IRQ_VALUE 0x28 +#define CDNS_GPIO_IRQ_ANY_EDGE 0x2c + +struct cdns_gpio_chip { + struct gpio_chip gc; + struct clk *pclk; + void __iomem *regs; + u32 bypass_orig; +}; + +static int cdns_gpio_request(struct gpio_chip *chip, unsigned int offset) +{ + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); + + iowrite32(ioread32(cgpio->regs + CDNS_GPIO_BYPASS_MODE) & ~BIT(offset), + cgpio->regs + CDNS_GPIO_BYPASS_MODE); + + return 0; +} + +static void cdns_gpio_free(struct gpio_chip *chip, unsigned int offset) +{ + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); + + iowrite32(ioread32(cgpio->regs + CDNS_GPIO_BYPASS_MODE) | + (BIT(offset) & cgpio->bypass_orig), + cgpio->regs + CDNS_GPIO_BYPASS_MODE); +} + +static void cdns_gpio_irq_mask(struct irq_data *d) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(d); + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); + + iowrite32(BIT(d->hwirq), cgpio->regs + CDNS_GPIO_IRQ_DIS); +} + +static void cdns_gpio_irq_unmask(struct irq_data *d) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(d); + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); + + iowrite32(BIT(d->hwirq), cgpio->regs + CDNS_GPIO_IRQ_EN); +} + +static int cdns_gpio_irq_set_type(struct irq_data *d, unsigned int type) +{ + struct gpio_chip *chip = irq_data_get_irq_chip_data(d); + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); + u32 int_value = ioread32(cgpio->regs + CDNS_GPIO_IRQ_VALUE); + u32 int_type = ioread32(cgpio->regs + CDNS_GPIO_IRQ_TYPE); + u32 mask = BIT(d->hwirq); + + int_value &= ~mask; + int_type &= ~mask; + + /* + * The GPIO controller doesn't have an ACK register. + * All interrupt statuses are cleared on a status register read. + * Don't support edge interrupts for now. + */ + + if (type == IRQ_TYPE_LEVEL_HIGH) { + int_type |= mask; + int_value |= mask; + } else if (type == IRQ_TYPE_LEVEL_LOW) { + int_type |= mask; + } else { + return -EINVAL; + } + + iowrite32(int_value, cgpio->regs + CDNS_GPIO_IRQ_VALUE); + iowrite32(int_type, cgpio->regs + CDNS_GPIO_IRQ_TYPE); + + return 0; +} + +static void cdns_gpio_irq_handler(struct irq_desc *desc) +{ + struct gpio_chip *chip = irq_desc_get_handler_data(desc); + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); + struct irq_chip *irqchip = irq_desc_get_chip(desc); + unsigned long status; + int hwirq; + + chained_irq_enter(irqchip, desc); + + status = ioread32(cgpio->regs + CDNS_GPIO_IRQ_STATUS) & + ~ioread32(cgpio->regs + CDNS_GPIO_IRQ_MASK); + + for_each_set_bit(hwirq, &status, chip->ngpio) + generic_handle_irq(irq_find_mapping(chip->irq.domain, hwirq)); + + chained_irq_exit(irqchip, desc); +} + +static struct irq_chip cdns_gpio_irqchip = { + .name = "cdns-gpio", + .irq_mask = cdns_gpio_irq_mask, + .irq_unmask = cdns_gpio_irq_unmask, + .irq_set_type = cdns_gpio_irq_set_type +}; + +static int cdns_gpio_probe(struct platform_device *pdev) +{ + struct cdns_gpio_chip *cgpio; + struct resource *res; + int ret, irq; + u32 dir_prev; + u32 num_gpios = 32; + + cgpio = devm_kzalloc(&pdev->dev, sizeof(*cgpio), GFP_KERNEL); + if (!cgpio) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + cgpio->regs = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(cgpio->regs)) + return PTR_ERR(cgpio->regs); + + of_property_read_u32(pdev->dev.of_node, "ngpios", &num_gpios); + + if (num_gpios > 32) { + dev_err(&pdev->dev, "ngpios must be less or equal 32\n"); + return -EINVAL; + } + + /* + * Set all pins as inputs by default, otherwise: + * gpiochip_lock_as_irq: + * tried to flag a GPIO set as output for IRQ + * Generic GPIO driver stores the direction value internally, + * so it needs to be changed before bgpio_init() is called. + */ + dir_prev = ioread32(cgpio->regs + CDNS_GPIO_DIRECTION_MODE); + iowrite32(GENMASK(num_gpios - 1, 0), + cgpio->regs + CDNS_GPIO_DIRECTION_MODE); + + ret = bgpio_init(&cgpio->gc, &pdev->dev, 4, + cgpio->regs + CDNS_GPIO_INPUT_VALUE, + cgpio->regs + CDNS_GPIO_OUTPUT_VALUE, + NULL, + NULL, + cgpio->regs + CDNS_GPIO_DIRECTION_MODE, + BGPIOF_READ_OUTPUT_REG_SET); + if (ret) { + dev_err(&pdev->dev, "Failed to register generic gpio, %d\n", + ret); + goto err_revert_dir; + } + + cgpio->gc.label = dev_name(&pdev->dev); + cgpio->gc.ngpio = num_gpios; + cgpio->gc.parent = &pdev->dev; + cgpio->gc.base = -1; + cgpio->gc.owner = THIS_MODULE; + cgpio->gc.request = cdns_gpio_request; + cgpio->gc.free = cdns_gpio_free; + + cgpio->pclk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(cgpio->pclk)) { + ret = PTR_ERR(cgpio->pclk); + dev_err(&pdev->dev, + "Failed to retrieve peripheral clock, %d\n", ret); + goto err_revert_dir; + } + + ret = clk_prepare_enable(cgpio->pclk); + if (ret) { + dev_err(&pdev->dev, + "Failed to enable the peripheral clock, %d\n", ret); + goto err_revert_dir; + } + + ret = devm_gpiochip_add_data(&pdev->dev, &cgpio->gc, cgpio); + if (ret < 0) { + dev_err(&pdev->dev, "Could not register gpiochip, %d\n", ret); + goto err_disable_clk; + } + + /* + * irq_chip support + */ + irq = platform_get_irq(pdev, 0); + if (irq >= 0) { + ret = gpiochip_irqchip_add(&cgpio->gc, &cdns_gpio_irqchip, + 0, handle_level_irq, + IRQ_TYPE_NONE); + if (ret) { + dev_err(&pdev->dev, "Could not add irqchip, %d\n", + ret); + goto err_disable_clk; + } + gpiochip_set_chained_irqchip(&cgpio->gc, &cdns_gpio_irqchip, + irq, cdns_gpio_irq_handler); + } + + cgpio->bypass_orig = ioread32(cgpio->regs + CDNS_GPIO_BYPASS_MODE); + + /* + * Enable gpio outputs, ignored for input direction + */ + iowrite32(GENMASK(num_gpios - 1, 0), + cgpio->regs + CDNS_GPIO_OUTPUT_EN); + iowrite32(0, cgpio->regs + CDNS_GPIO_BYPASS_MODE); + + platform_set_drvdata(pdev, cgpio); + return 0; + +err_disable_clk: + clk_disable_unprepare(cgpio->pclk); + +err_revert_dir: + iowrite32(dir_prev, cgpio->regs + CDNS_GPIO_DIRECTION_MODE); + + return ret; +} + +static int cdns_gpio_remove(struct platform_device *pdev) +{ + struct cdns_gpio_chip *cgpio = platform_get_drvdata(pdev); + + iowrite32(cgpio->bypass_orig, cgpio->regs + CDNS_GPIO_BYPASS_MODE); + clk_disable_unprepare(cgpio->pclk); + + return 0; +} + +static const struct of_device_id cdns_of_ids[] = { + { .compatible = "cdns,gpio-r1p02" }, + { /* sentinel */ }, +}; + +static struct platform_driver cdns_gpio_driver = { + .driver = { + .name = "cdns-gpio", + .of_match_table = cdns_of_ids, + }, + .probe = cdns_gpio_probe, + .remove = cdns_gpio_remove, +}; +module_platform_driver(cdns_gpio_driver); + +MODULE_AUTHOR("Jan Kotas <jank@cadence.com>"); +MODULE_DESCRIPTION("Cadence GPIO driver"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:cdns-gpio"); -- 2.15.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] gpio: Add Cadence GPIO driver 2018-12-17 15:36 ` [PATCH v2 2/2] gpio: Add Cadence GPIO driver Jan Kotas @ 2018-12-17 15:51 ` Bartosz Golaszewski 2018-12-17 22:21 ` Linus Walleij 2018-12-21 10:34 ` Linus Walleij 1 sibling, 1 reply; 11+ messages in thread From: Bartosz Golaszewski @ 2018-12-17 15:51 UTC (permalink / raw) To: Jan Kotas Cc: Linus Walleij, Rob Herring, Mark Rutland, linux-gpio, linux-devicetree, LKML pon., 17 gru 2018 o 16:37 Jan Kotas <jank@cadence.com> napisał(a): > > This patch adds a driver for Cadence GPIO controller. > > It can be enabled with GPIO_CADENCE Kconfig option. > It uses generic GPIO infrastructure and works > as an interrupt controller. > At the moment it only supports level sensitive irqs. > > Signed-off-by: Jan Kotas <jank@cadence.com> > --- > drivers/gpio/Kconfig | 8 ++ > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-cadence.c | 275 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 284 insertions(+) > create mode 100644 drivers/gpio/gpio-cadence.c > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 833a1b51c..8259f8f25 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -160,6 +160,14 @@ config GPIO_BRCMSTB > help > Say yes here to enable GPIO support for Broadcom STB (BCM7XXX) SoCs. > > +config GPIO_CADENCE > + tristate "Cadence GPIO support" > + depends on OF_GPIO > + select GPIO_GENERIC > + select GPIOLIB_IRQCHIP > + help > + Say yes here to enable support for Cadence GPIO controller. > + > config GPIO_CLPS711X > tristate "CLPS711X GPIO support" > depends on ARCH_CLPS711X || COMPILE_TEST > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index 671c4477c..5fce8634a 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -37,6 +37,7 @@ obj-$(CONFIG_GPIO_BCM_KONA) += gpio-bcm-kona.o > obj-$(CONFIG_GPIO_BD9571MWV) += gpio-bd9571mwv.o > obj-$(CONFIG_GPIO_BRCMSTB) += gpio-brcmstb.o > obj-$(CONFIG_GPIO_BT8XX) += gpio-bt8xx.o > +obj-$(CONFIG_GPIO_CADENCE) += gpio-cadence.o > obj-$(CONFIG_GPIO_CLPS711X) += gpio-clps711x.o > obj-$(CONFIG_GPIO_CS5535) += gpio-cs5535.o > obj-$(CONFIG_GPIO_CRYSTAL_COVE) += gpio-crystalcove.o > diff --git a/drivers/gpio/gpio-cadence.c b/drivers/gpio/gpio-cadence.c > new file mode 100644 > index 000000000..16cc9f50f > --- /dev/null > +++ b/drivers/gpio/gpio-cadence.c > @@ -0,0 +1,275 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * Copyright 2017-2018 Cadence > + * > + * Authors: > + * Jan Kotas <jank@cadence.com> > + * Boris Brezillon <boris.brezillon@free-electrons.com> > + */ > + > +#include <linux/gpio/driver.h> > +#include <linux/clk.h> > +#include <linux/interrupt.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > + > +#define CDNS_GPIO_BYPASS_MODE 0x00 > +#define CDNS_GPIO_DIRECTION_MODE 0x04 > +#define CDNS_GPIO_OUTPUT_EN 0x08 > +#define CDNS_GPIO_OUTPUT_VALUE 0x0c > +#define CDNS_GPIO_INPUT_VALUE 0x10 > +#define CDNS_GPIO_IRQ_MASK 0x14 > +#define CDNS_GPIO_IRQ_EN 0x18 > +#define CDNS_GPIO_IRQ_DIS 0x1c > +#define CDNS_GPIO_IRQ_STATUS 0x20 > +#define CDNS_GPIO_IRQ_TYPE 0x24 > +#define CDNS_GPIO_IRQ_VALUE 0x28 > +#define CDNS_GPIO_IRQ_ANY_EDGE 0x2c > + > +struct cdns_gpio_chip { > + struct gpio_chip gc; > + struct clk *pclk; > + void __iomem *regs; > + u32 bypass_orig; > +}; > + > +static int cdns_gpio_request(struct gpio_chip *chip, unsigned int offset) > +{ > + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); > + > + iowrite32(ioread32(cgpio->regs + CDNS_GPIO_BYPASS_MODE) & ~BIT(offset), > + cgpio->regs + CDNS_GPIO_BYPASS_MODE); > + > + return 0; > +} > + > +static void cdns_gpio_free(struct gpio_chip *chip, unsigned int offset) > +{ > + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); > + > + iowrite32(ioread32(cgpio->regs + CDNS_GPIO_BYPASS_MODE) | > + (BIT(offset) & cgpio->bypass_orig), > + cgpio->regs + CDNS_GPIO_BYPASS_MODE); > +} > + > +static void cdns_gpio_irq_mask(struct irq_data *d) > +{ > + struct gpio_chip *chip = irq_data_get_irq_chip_data(d); > + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); > + > + iowrite32(BIT(d->hwirq), cgpio->regs + CDNS_GPIO_IRQ_DIS); > +} > + > +static void cdns_gpio_irq_unmask(struct irq_data *d) > +{ > + struct gpio_chip *chip = irq_data_get_irq_chip_data(d); > + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); > + > + iowrite32(BIT(d->hwirq), cgpio->regs + CDNS_GPIO_IRQ_EN); > +} > + > +static int cdns_gpio_irq_set_type(struct irq_data *d, unsigned int type) > +{ > + struct gpio_chip *chip = irq_data_get_irq_chip_data(d); > + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); > + u32 int_value = ioread32(cgpio->regs + CDNS_GPIO_IRQ_VALUE); > + u32 int_type = ioread32(cgpio->regs + CDNS_GPIO_IRQ_TYPE); > + u32 mask = BIT(d->hwirq); > + > + int_value &= ~mask; > + int_type &= ~mask; > + > + /* > + * The GPIO controller doesn't have an ACK register. > + * All interrupt statuses are cleared on a status register read. > + * Don't support edge interrupts for now. > + */ > + > + if (type == IRQ_TYPE_LEVEL_HIGH) { > + int_type |= mask; > + int_value |= mask; > + } else if (type == IRQ_TYPE_LEVEL_LOW) { > + int_type |= mask; > + } else { > + return -EINVAL; > + } > + > + iowrite32(int_value, cgpio->regs + CDNS_GPIO_IRQ_VALUE); > + iowrite32(int_type, cgpio->regs + CDNS_GPIO_IRQ_TYPE); > + > + return 0; > +} > + > +static void cdns_gpio_irq_handler(struct irq_desc *desc) > +{ > + struct gpio_chip *chip = irq_desc_get_handler_data(desc); > + struct cdns_gpio_chip *cgpio = gpiochip_get_data(chip); > + struct irq_chip *irqchip = irq_desc_get_chip(desc); > + unsigned long status; > + int hwirq; > + > + chained_irq_enter(irqchip, desc); > + > + status = ioread32(cgpio->regs + CDNS_GPIO_IRQ_STATUS) & > + ~ioread32(cgpio->regs + CDNS_GPIO_IRQ_MASK); > + > + for_each_set_bit(hwirq, &status, chip->ngpio) > + generic_handle_irq(irq_find_mapping(chip->irq.domain, hwirq)); > + > + chained_irq_exit(irqchip, desc); > +} > + > +static struct irq_chip cdns_gpio_irqchip = { > + .name = "cdns-gpio", > + .irq_mask = cdns_gpio_irq_mask, > + .irq_unmask = cdns_gpio_irq_unmask, > + .irq_set_type = cdns_gpio_irq_set_type > +}; > + > +static int cdns_gpio_probe(struct platform_device *pdev) > +{ > + struct cdns_gpio_chip *cgpio; > + struct resource *res; > + int ret, irq; > + u32 dir_prev; > + u32 num_gpios = 32; > + > + cgpio = devm_kzalloc(&pdev->dev, sizeof(*cgpio), GFP_KERNEL); > + if (!cgpio) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + cgpio->regs = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(cgpio->regs)) > + return PTR_ERR(cgpio->regs); > + > + of_property_read_u32(pdev->dev.of_node, "ngpios", &num_gpios); > + > + if (num_gpios > 32) { > + dev_err(&pdev->dev, "ngpios must be less or equal 32\n"); > + return -EINVAL; > + } > + > + /* > + * Set all pins as inputs by default, otherwise: > + * gpiochip_lock_as_irq: > + * tried to flag a GPIO set as output for IRQ > + * Generic GPIO driver stores the direction value internally, > + * so it needs to be changed before bgpio_init() is called. > + */ > + dir_prev = ioread32(cgpio->regs + CDNS_GPIO_DIRECTION_MODE); > + iowrite32(GENMASK(num_gpios - 1, 0), > + cgpio->regs + CDNS_GPIO_DIRECTION_MODE); > + > + ret = bgpio_init(&cgpio->gc, &pdev->dev, 4, > + cgpio->regs + CDNS_GPIO_INPUT_VALUE, > + cgpio->regs + CDNS_GPIO_OUTPUT_VALUE, > + NULL, > + NULL, > + cgpio->regs + CDNS_GPIO_DIRECTION_MODE, > + BGPIOF_READ_OUTPUT_REG_SET); > + if (ret) { > + dev_err(&pdev->dev, "Failed to register generic gpio, %d\n", > + ret); > + goto err_revert_dir; > + } > + > + cgpio->gc.label = dev_name(&pdev->dev); > + cgpio->gc.ngpio = num_gpios; > + cgpio->gc.parent = &pdev->dev; > + cgpio->gc.base = -1; > + cgpio->gc.owner = THIS_MODULE; > + cgpio->gc.request = cdns_gpio_request; > + cgpio->gc.free = cdns_gpio_free; > + > + cgpio->pclk = devm_clk_get(&pdev->dev, NULL); > + if (IS_ERR(cgpio->pclk)) { > + ret = PTR_ERR(cgpio->pclk); > + dev_err(&pdev->dev, > + "Failed to retrieve peripheral clock, %d\n", ret); > + goto err_revert_dir; > + } > + > + ret = clk_prepare_enable(cgpio->pclk); > + if (ret) { > + dev_err(&pdev->dev, > + "Failed to enable the peripheral clock, %d\n", ret); > + goto err_revert_dir; > + } > + > + ret = devm_gpiochip_add_data(&pdev->dev, &cgpio->gc, cgpio); > + if (ret < 0) { > + dev_err(&pdev->dev, "Could not register gpiochip, %d\n", ret); > + goto err_disable_clk; > + } > + > + /* > + * irq_chip support > + */ > + irq = platform_get_irq(pdev, 0); > + if (irq >= 0) { > + ret = gpiochip_irqchip_add(&cgpio->gc, &cdns_gpio_irqchip, > + 0, handle_level_irq, > + IRQ_TYPE_NONE); > + if (ret) { > + dev_err(&pdev->dev, "Could not add irqchip, %d\n", > + ret); > + goto err_disable_clk; > + } > + gpiochip_set_chained_irqchip(&cgpio->gc, &cdns_gpio_irqchip, > + irq, cdns_gpio_irq_handler); > + } > + > + cgpio->bypass_orig = ioread32(cgpio->regs + CDNS_GPIO_BYPASS_MODE); > + > + /* > + * Enable gpio outputs, ignored for input direction > + */ > + iowrite32(GENMASK(num_gpios - 1, 0), > + cgpio->regs + CDNS_GPIO_OUTPUT_EN); > + iowrite32(0, cgpio->regs + CDNS_GPIO_BYPASS_MODE); > + > + platform_set_drvdata(pdev, cgpio); > + return 0; > + > +err_disable_clk: > + clk_disable_unprepare(cgpio->pclk); > + > +err_revert_dir: > + iowrite32(dir_prev, cgpio->regs + CDNS_GPIO_DIRECTION_MODE); > + > + return ret; > +} > + > +static int cdns_gpio_remove(struct platform_device *pdev) > +{ > + struct cdns_gpio_chip *cgpio = platform_get_drvdata(pdev); > + > + iowrite32(cgpio->bypass_orig, cgpio->regs + CDNS_GPIO_BYPASS_MODE); > + clk_disable_unprepare(cgpio->pclk); > + > + return 0; > +} > + > +static const struct of_device_id cdns_of_ids[] = { > + { .compatible = "cdns,gpio-r1p02" }, > + { /* sentinel */ }, > +}; > + > +static struct platform_driver cdns_gpio_driver = { > + .driver = { > + .name = "cdns-gpio", > + .of_match_table = cdns_of_ids, > + }, > + .probe = cdns_gpio_probe, > + .remove = cdns_gpio_remove, > +}; > +module_platform_driver(cdns_gpio_driver); > + > +MODULE_AUTHOR("Jan Kotas <jank@cadence.com>"); > +MODULE_DESCRIPTION("Cadence GPIO driver"); > +MODULE_LICENSE("GPL v2"); > +MODULE_ALIAS("platform:cdns-gpio"); > -- > 2.15.0 > The driver looks good but is there any particular reason not to use regmap for register IO? Bart ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] gpio: Add Cadence GPIO driver 2018-12-17 15:51 ` Bartosz Golaszewski @ 2018-12-17 22:21 ` Linus Walleij 2018-12-18 12:50 ` Bartosz Golaszewski 0 siblings, 1 reply; 11+ messages in thread From: Linus Walleij @ 2018-12-17 22:21 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Jan Kotas, Rob Herring, Mark Rutland, linux-gpio, linux-devicetree, LKML On Mon, Dec 17, 2018 at 4:51 PM Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > The driver looks good but is there any particular reason not to use > regmap for register IO? I thought we only use regmap for MMIO when the register range is shared (as in a system controller) so that some registers are for this, some register or even bits in a register for some other driver, so they need the spinlock in the regmap to protect the register range. It is also nice for shadowing/caching of register contents I guess, wat does this driver get from regmap MMIO? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] gpio: Add Cadence GPIO driver 2018-12-17 22:21 ` Linus Walleij @ 2018-12-18 12:50 ` Bartosz Golaszewski 2018-12-18 13:54 ` Janek Kotas 0 siblings, 1 reply; 11+ messages in thread From: Bartosz Golaszewski @ 2018-12-18 12:50 UTC (permalink / raw) To: Linus Walleij Cc: Jan Kotas, Rob Herring, Mark Rutland, linux-gpio, linux-devicetree, LKML pon., 17 gru 2018 o 23:22 Linus Walleij <linus.walleij@linaro.org> napisał(a): > > On Mon, Dec 17, 2018 at 4:51 PM Bartosz Golaszewski > <bgolaszewski@baylibre.com> wrote: > > > The driver looks good but is there any particular reason not to use > > regmap for register IO? > > I thought we only use regmap for MMIO when the register range is > shared (as in a system controller) so that some registers are for this, > some register or even bits in a register for some other driver, so they > need the spinlock in the regmap to protect the register range. > This is what syscon is for. Regmap simply abstracts any register IO. For instance: there's no locking in this driver. Are we sure it's not needed? Regmap provides internal locking for you in the form of a mutex or spinlock. Also: it looks like the interrupts here are quite simple with a single bit per interrupt in the status register and the same layout in the mask register - it could probably profit from using the regmap_irq_chip and not bother with reimplementing irq_chip callbacks. > It is also nice for shadowing/caching of register contents I guess, > wat does this driver get from regmap MMIO? > Code shrinkage IMO. Note that I'm not blocking this from being merged - I just think that using modern frameworks is always a good idea. Best regards, Bartosz Golaszewski > Yours, > Linus Walleij ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] gpio: Add Cadence GPIO driver 2018-12-18 12:50 ` Bartosz Golaszewski @ 2018-12-18 13:54 ` Janek Kotas 2018-12-18 14:06 ` Bartosz Golaszewski 0 siblings, 1 reply; 11+ messages in thread From: Janek Kotas @ 2018-12-18 13:54 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Linus Walleij, Janek Kotas, Rob Herring, Mark Rutland, linux-gpio, linux-devicetree, LKML > On 18 Dec 2018, at 13:50, Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > > > pon., 17 gru 2018 o 23:22 Linus Walleij <linus.walleij@linaro.org> napisał(a): >> >> On Mon, Dec 17, 2018 at 4:51 PM Bartosz Golaszewski >> <bgolaszewski@baylibre.com> wrote: >> >>> The driver looks good but is there any particular reason not to use >>> regmap for register IO? >> >> I thought we only use regmap for MMIO when the register range is >> shared (as in a system controller) so that some registers are for this, >> some register or even bits in a register for some other driver, so they >> need the spinlock in the regmap to protect the register range. >> > > This is what syscon is for. Regmap simply abstracts any register IO. > For instance: there's no locking in this driver. Are we sure it's not > needed? Regmap provides internal locking for you in the form of a > mutex or spinlock. > > Also: it looks like the interrupts here are quite simple with a single > bit per interrupt in the status register and the same layout in the > mask register - it could probably profit from using the > regmap_irq_chip and not bother with reimplementing irq_chip callbacks. > >> It is also nice for shadowing/caching of register contents I guess, >> wat does this driver get from regmap MMIO? >> > > Code shrinkage IMO. > > Note that I'm not blocking this from being merged - I just think that > using modern frameworks is always a good idea. I can reimplement the driver using regmap, but It seems in such case I won’t be able to use the Generic GPIO Infrastructure, would I? So I would need to provide functions for setting direction, etc. I think it would make the driver code bigger. Regards, Jan > Best regards, > Bartosz Golaszewski > >> Yours, >> Linus Walleij ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] gpio: Add Cadence GPIO driver 2018-12-18 13:54 ` Janek Kotas @ 2018-12-18 14:06 ` Bartosz Golaszewski 2018-12-21 10:37 ` Linus Walleij 0 siblings, 1 reply; 11+ messages in thread From: Bartosz Golaszewski @ 2018-12-18 14:06 UTC (permalink / raw) To: Janek Kotas Cc: Linus Walleij, Rob Herring, Mark Rutland, linux-gpio, linux-devicetree, LKML wt., 18 gru 2018 o 14:54 Janek Kotas <jank@cadence.com> napisał(a): > > > > On 18 Dec 2018, at 13:50, Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > > > > > > pon., 17 gru 2018 o 23:22 Linus Walleij <linus.walleij@linaro.org> napisał(a): > >> > >> On Mon, Dec 17, 2018 at 4:51 PM Bartosz Golaszewski > >> <bgolaszewski@baylibre.com> wrote: > >> > >>> The driver looks good but is there any particular reason not to use > >>> regmap for register IO? > >> > >> I thought we only use regmap for MMIO when the register range is > >> shared (as in a system controller) so that some registers are for this, > >> some register or even bits in a register for some other driver, so they > >> need the spinlock in the regmap to protect the register range. > >> > > > > This is what syscon is for. Regmap simply abstracts any register IO. > > For instance: there's no locking in this driver. Are we sure it's not > > needed? Regmap provides internal locking for you in the form of a > > mutex or spinlock. > > > > Also: it looks like the interrupts here are quite simple with a single > > bit per interrupt in the status register and the same layout in the > > mask register - it could probably profit from using the > > regmap_irq_chip and not bother with reimplementing irq_chip callbacks. > > > >> It is also nice for shadowing/caching of register contents I guess, > >> wat does this driver get from regmap MMIO? > >> > > > > Code shrinkage IMO. > > > > Note that I'm not blocking this from being merged - I just think that > > using modern frameworks is always a good idea. > > I can reimplement the driver using regmap, but It seems in such case > I won’t be able to use the Generic GPIO Infrastructure, would I? > So I would need to provide functions for setting direction, etc. > I think it would make the driver code bigger. > Indeed. If anything: gpio-mmio would need to be converted to using regmap. So I guess nevermind my comment. Bart ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] gpio: Add Cadence GPIO driver 2018-12-18 14:06 ` Bartosz Golaszewski @ 2018-12-21 10:37 ` Linus Walleij 0 siblings, 0 replies; 11+ messages in thread From: Linus Walleij @ 2018-12-21 10:37 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Janek Kotas, Rob Herring, Mark Rutland, linux-gpio, linux-devicetree, LKML On Tue, Dec 18, 2018 at 3:06 PM Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > wt., 18 gru 2018 o 14:54 Janek Kotas <jank@cadence.com> napisał(a): > > I can reimplement the driver using regmap, but It seems in such case > > I won’t be able to use the Generic GPIO Infrastructure, would I? > > So I would need to provide functions for setting direction, etc. > > I think it would make the driver code bigger. > > Indeed. If anything: gpio-mmio would need to be converted to using regmap. Ouch, let's go with this for now. I think for gpio-mmio we have two new cases we really need to cover: - port-mapped I/O (x86) - regmap I/O Augmenting gpio-mmio for these could probably strip out quite a bunch of code from several drivers. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] gpio: Add Cadence GPIO driver 2018-12-17 15:36 ` [PATCH v2 2/2] gpio: Add Cadence GPIO driver Jan Kotas 2018-12-17 15:51 ` Bartosz Golaszewski @ 2018-12-21 10:34 ` Linus Walleij 1 sibling, 0 replies; 11+ messages in thread From: Linus Walleij @ 2018-12-21 10:34 UTC (permalink / raw) To: Jan Kotas Cc: Bartosz Golaszewski, Rob Herring, Mark Rutland, open list:GPIO SUBSYSTEM, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, linux-kernel On Mon, Dec 17, 2018 at 4:37 PM Jan Kotas <jank@cadence.com> wrote: > This patch adds a driver for Cadence GPIO controller. > > It can be enabled with GPIO_CADENCE Kconfig option. > It uses generic GPIO infrastructure and works > as an interrupt controller. > At the moment it only supports level sensitive irqs. > > Signed-off-by: Jan Kotas <jank@cadence.com> Patch applied. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-12-21 10:37 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-17 15:36 [PATCH v2 0/2] gpio: Add a driver for Cadence GPIO controller Jan Kotas 2018-12-17 15:36 ` [PATCH v2 1/2] dt-bindings: gpio: Add bindings for Cadence GPIO Jan Kotas 2018-12-18 17:33 ` Rob Herring 2018-12-17 15:36 ` [PATCH v2 2/2] gpio: Add Cadence GPIO driver Jan Kotas 2018-12-17 15:51 ` Bartosz Golaszewski 2018-12-17 22:21 ` Linus Walleij 2018-12-18 12:50 ` Bartosz Golaszewski 2018-12-18 13:54 ` Janek Kotas 2018-12-18 14:06 ` Bartosz Golaszewski 2018-12-21 10:37 ` Linus Walleij 2018-12-21 10:34 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).