From: Lewis Hanly <[email protected]>
Add a driver to support the Polarfire SoC gpio controller.
Tested with latest 5.19 kernel.
MPFS gpio interrupts are connected to IOMUX configured by system
register GPIO_INTERRUPT_FAB_CR(31:0). Interrupt connection for
some GPIO's to the PLIC (Platform Level Interrupt Controller) can be
shared (not directly connected) or direct if connection is available.
previously upstreamed dt-bindings: gpio: microchip,mpfs-gpio.yaml
Changes in v6
Fixed typo causing compile issue with kernel 6.0-rc1
Changes in v5
Added the const for the irqchip, removed in error from v2.
Changes in v4
Changed the interrupt handling from Hierarchical flow to chained interrupt
flow. The reason for the change was with hierarchical flow we requried
a interrupt number mapping array to work with our HW and this was not acceptable.
On reviewing the architecture the chained interrupt flow works better
for our hardware and configurations which are not fixed in Silicon.
Added support to read the optional DT property ngpios.
Changes in v3:
Changed order in kconfig.
Removed blank lines in driver header/source file.
Removed BYTE_BOUNDARY variable and use macro to do *4.
mpfs_gpio_assign_bit parameter uses macro instead of (i * BYTE_BOUNDARY).
Add correct definitions for direction.
Change order of variables in mpfs_gpio_irq_set_type function.
Return dev_err_probe instead of dev_err.
Remove noise of dev_inf.
Avoid using of_match_ptr.
use devm_gpiochip_add_data(..)
Update mpfs_gpio_remove.
Changes in v2:
Use raw_spinlock.
Use __assign_bit() to assign bit, added a bool variable for value.
Remove unnecessary checking gpio_index.
Remove default from switch statement.
Use const for irq_chip, name updated and use mask/unmask.
Use latest kernel api irq set_chip.
Implemented hierarchical interrupt chip support, although
suggested to use chained interrupt flow I believe this fits better.
Lewis Hanly (1):
gpio: mpfs: add polarfire soc gpio support
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-mpfs.c | 317 +++++++++++++++++++++++++++++++++++++++
3 files changed, 325 insertions(+)
create mode 100644 drivers/gpio/gpio-mpfs.c
--
2.25.1
From: Lewis Hanly <[email protected]>
Add a driver to support the Polarfire SoC gpio controller
Signed-off-by: Lewis Hanly <[email protected]>
---
drivers/gpio/Kconfig | 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-mpfs.c | 317 +++++++++++++++++++++++++++++++++++++++
3 files changed, 325 insertions(+)
create mode 100644 drivers/gpio/gpio-mpfs.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 0642f579196f..303ea7f6f7bc 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -490,6 +490,13 @@ config GPIO_PMIC_EIC_SPRD
help
Say yes here to support Spreadtrum PMIC EIC device.
+config GPIO_POLARFIRE_SOC
+ bool "Microchip FPGA GPIO support"
+ depends on OF_GPIO
+ select GPIOLIB_IRQCHIP
+ help
+ Say yes here to support the GPIO device on Microchip FPGAs.
+
config GPIO_PXA
bool "PXA GPIO support"
depends on ARCH_PXA || ARCH_MMP || COMPILE_TEST
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index a0985d30f51b..e04061ac5a28 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -120,6 +120,7 @@ obj-$(CONFIG_GPIO_PCI_IDIO_16) += gpio-pci-idio-16.o
obj-$(CONFIG_GPIO_PISOSR) += gpio-pisosr.o
obj-$(CONFIG_GPIO_PL061) += gpio-pl061.o
obj-$(CONFIG_GPIO_PMIC_EIC_SPRD) += gpio-pmic-eic-sprd.o
+obj-$(CONFIG_GPIO_POLARFIRE_SOC) += gpio-mpfs.o
obj-$(CONFIG_GPIO_PXA) += gpio-pxa.o
obj-$(CONFIG_GPIO_RASPBERRYPI_EXP) += gpio-raspberrypi-exp.o
obj-$(CONFIG_GPIO_RC5T583) += gpio-rc5t583.o
diff --git a/drivers/gpio/gpio-mpfs.c b/drivers/gpio/gpio-mpfs.c
new file mode 100644
index 000000000000..00e68a8b891d
--- /dev/null
+++ b/drivers/gpio/gpio-mpfs.c
@@ -0,0 +1,317 @@
+// SPDX-License-Identifier: (GPL-2.0)
+/*
+ * Microchip PolarFire SoC (MPFS) GPIO controller driver
+ *
+ * Copyright (c) 2018-2022 Microchip Technology Inc. and its subsidiaries
+ *
+ * Author: Lewis Hanly <[email protected]>
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/irq.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+
+#define MPFS_GPIO_CTRL(i) (0x4 * (i))
+#define MAX_NUM_GPIO 32
+#define MPFS_GPIO_EN_INT 3
+#define MPFS_GPIO_EN_OUT_BUF BIT(2)
+#define MPFS_GPIO_EN_IN BIT(1)
+#define MPFS_GPIO_EN_OUT BIT(0)
+
+#define MPFS_GPIO_TYPE_INT_EDGE_BOTH 0x80
+#define MPFS_GPIO_TYPE_INT_EDGE_NEG 0x60
+#define MPFS_GPIO_TYPE_INT_EDGE_POS 0x40
+#define MPFS_GPIO_TYPE_INT_LEVEL_LOW 0x20
+#define MPFS_GPIO_TYPE_INT_LEVEL_HIGH 0x00
+#define MPFS_GPIO_TYPE_INT_MASK GENMASK(7, 5)
+#define MPFS_IRQ_REG 0x80
+#define MPFS_INP_REG 0x84
+#define MPFS_OUTP_REG 0x88
+
+struct mpfs_gpio_chip {
+ void __iomem *base;
+ struct clk *clk;
+ raw_spinlock_t lock;
+ struct gpio_chip gc;
+};
+
+static void mpfs_gpio_assign_bit(void __iomem *addr, unsigned int bit_offset, bool value)
+{
+ unsigned long reg = readl(addr);
+
+ __assign_bit(bit_offset, ®, value);
+ writel(reg, addr);
+}
+
+static int mpfs_gpio_direction_input(struct gpio_chip *gc, unsigned int gpio_index)
+{
+ struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+ u32 gpio_cfg;
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&mpfs_gpio->lock, flags);
+
+ gpio_cfg = readl(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
+ gpio_cfg |= MPFS_GPIO_EN_IN;
+ gpio_cfg &= ~(MPFS_GPIO_EN_OUT | MPFS_GPIO_EN_OUT_BUF);
+ writel(gpio_cfg, mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
+
+ raw_spin_unlock_irqrestore(&mpfs_gpio->lock, flags);
+
+ return 0;
+}
+
+static int mpfs_gpio_direction_output(struct gpio_chip *gc, unsigned int gpio_index, int value)
+{
+ struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+ u32 gpio_cfg;
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&mpfs_gpio->lock, flags);
+
+ gpio_cfg = readl(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
+ gpio_cfg |= MPFS_GPIO_EN_OUT | MPFS_GPIO_EN_OUT_BUF;
+ gpio_cfg &= ~MPFS_GPIO_EN_IN;
+ writel(gpio_cfg, mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
+
+ mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_OUTP_REG, gpio_index, value);
+
+ raw_spin_unlock_irqrestore(&mpfs_gpio->lock, flags);
+
+ return 0;
+}
+
+static int mpfs_gpio_get_direction(struct gpio_chip *gc,
+ unsigned int gpio_index)
+{
+ struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+ u32 gpio_cfg;
+
+ gpio_cfg = readl(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
+ if (gpio_cfg & MPFS_GPIO_EN_IN)
+ return GPIO_LINE_DIRECTION_IN;
+
+ return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int mpfs_gpio_get(struct gpio_chip *gc,
+ unsigned int gpio_index)
+{
+ struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+
+ return !!(readl(mpfs_gpio->base + MPFS_INP_REG) & BIT(gpio_index));
+}
+
+static void mpfs_gpio_set(struct gpio_chip *gc, unsigned int gpio_index, int value)
+{
+ struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&mpfs_gpio->lock, flags);
+
+ mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_OUTP_REG,
+ gpio_index, value);
+
+ raw_spin_unlock_irqrestore(&mpfs_gpio->lock, flags);
+}
+
+static int mpfs_gpio_irq_set_type(struct irq_data *data, unsigned int type)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+ struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+ int gpio_index = irqd_to_hwirq(data);
+ u32 interrupt_type;
+ u32 gpio_cfg;
+ unsigned long flags;
+
+ switch (type) {
+ case IRQ_TYPE_EDGE_BOTH:
+ interrupt_type = MPFS_GPIO_TYPE_INT_EDGE_BOTH;
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ interrupt_type = MPFS_GPIO_TYPE_INT_EDGE_NEG;
+ break;
+ case IRQ_TYPE_EDGE_RISING:
+ interrupt_type = MPFS_GPIO_TYPE_INT_EDGE_POS;
+ break;
+ case IRQ_TYPE_LEVEL_HIGH:
+ interrupt_type = MPFS_GPIO_TYPE_INT_LEVEL_HIGH;
+ break;
+ case IRQ_TYPE_LEVEL_LOW:
+ interrupt_type = MPFS_GPIO_TYPE_INT_LEVEL_LOW;
+ break;
+ }
+
+ raw_spin_lock_irqsave(&mpfs_gpio->lock, flags);
+
+ gpio_cfg = readl(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
+ gpio_cfg &= ~MPFS_GPIO_TYPE_INT_MASK;
+ gpio_cfg |= interrupt_type;
+ writel(gpio_cfg, mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
+
+ raw_spin_unlock_irqrestore(&mpfs_gpio->lock, flags);
+
+ return 0;
+}
+
+static void mpfs_gpio_irq_unmask(struct irq_data *data)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+ struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+ int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO;
+
+ mpfs_gpio_direction_input(gc, gpio_index);
+ mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1);
+ mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index),
+ MPFS_GPIO_EN_INT, 1);
+}
+
+static void mpfs_gpio_irq_mask(struct irq_data *data)
+{
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+ struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
+ int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO;
+
+ mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1);
+ mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index),
+ MPFS_GPIO_EN_INT, 0);
+}
+
+static const struct irq_chip mpfs_gpio_irqchip = {
+ .name = "mpfs",
+ .irq_set_type = mpfs_gpio_irq_set_type,
+ .irq_mask = mpfs_gpio_irq_mask,
+ .irq_unmask = mpfs_gpio_irq_unmask,
+ .flags = IRQCHIP_MASK_ON_SUSPEND,
+};
+
+static void mpfs_gpio_irq_handler(struct irq_desc *desc)
+{
+ struct irq_chip *irqchip = irq_desc_get_chip(desc);
+ struct mpfs_gpio_chip *mpfs_gpio =
+ gpiochip_get_data(irq_desc_get_handler_data(desc));
+ unsigned long status;
+ int offset;
+
+ chained_irq_enter(irqchip, desc);
+
+ status = readl(mpfs_gpio->base + MPFS_IRQ_REG);
+ for_each_set_bit(offset, &status, mpfs_gpio->gc.ngpio) {
+ mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, offset, 1);
+ generic_handle_irq(irq_find_mapping(mpfs_gpio->gc.irq.domain, offset));
+ }
+
+ chained_irq_exit(irqchip, desc);
+}
+
+static int mpfs_gpio_probe(struct platform_device *pdev)
+{
+ struct clk *clk;
+ struct device *dev = &pdev->dev;
+ struct device_node *node = pdev->dev.of_node;
+ struct mpfs_gpio_chip *mpfs_gpio;
+ struct gpio_irq_chip *girq;
+ int i, ret, ngpios, nirqs;
+
+ mpfs_gpio = devm_kzalloc(dev, sizeof(*mpfs_gpio), GFP_KERNEL);
+ if (!mpfs_gpio)
+ return -ENOMEM;
+
+ mpfs_gpio->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(mpfs_gpio->base))
+ return dev_err_probe(dev, PTR_ERR(mpfs_gpio->base), "failed to ioremap memory resource\n");
+
+ clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk), "devm_clk_get failed\n");
+
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to enable clock\n");
+
+ mpfs_gpio->clk = clk;
+
+ ngpios = MAX_NUM_GPIO;
+ device_property_read_u32(dev, "ngpios", &ngpios);
+ if (ngpios > MAX_NUM_GPIO)
+ ngpios = MAX_NUM_GPIO;
+
+ raw_spin_lock_init(&mpfs_gpio->lock);
+ mpfs_gpio->gc.direction_input = mpfs_gpio_direction_input;
+ mpfs_gpio->gc.direction_output = mpfs_gpio_direction_output;
+ mpfs_gpio->gc.get_direction = mpfs_gpio_get_direction;
+ mpfs_gpio->gc.get = mpfs_gpio_get;
+ mpfs_gpio->gc.set = mpfs_gpio_set;
+ mpfs_gpio->gc.base = -1;
+ mpfs_gpio->gc.ngpio = ngpios;
+ mpfs_gpio->gc.label = dev_name(dev);
+ mpfs_gpio->gc.parent = dev;
+ mpfs_gpio->gc.owner = THIS_MODULE;
+
+ nirqs = of_irq_count(node);
+ if (nirqs > MAX_NUM_GPIO) {
+ ret = -ENXIO;
+ goto cleanup_clock;
+ }
+ girq = &mpfs_gpio->gc.irq;
+ gpio_irq_chip_set_chip(girq, &mpfs_gpio_irqchip);
+ girq->handler = handle_simple_irq;
+ girq->parent_handler = mpfs_gpio_irq_handler;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->num_parents = nirqs;
+ girq->parents = devm_kcalloc(&pdev->dev, nirqs,
+ sizeof(*girq->parents), GFP_KERNEL);
+ if (!girq->parents) {
+ ret = -ENOMEM;
+ goto cleanup_clock;
+ }
+ for (i = 0; i < nirqs; i++)
+ girq->parents[i] = platform_get_irq(pdev, i);
+
+ ret = gpiochip_add_data(&mpfs_gpio->gc, mpfs_gpio);
+ if (ret)
+ goto cleanup_clock;
+
+ platform_set_drvdata(pdev, mpfs_gpio);
+
+ return 0;
+
+cleanup_clock:
+ clk_disable_unprepare(mpfs_gpio->clk);
+ return ret;
+}
+
+static int mpfs_gpio_remove(struct platform_device *pdev)
+{
+ struct mpfs_gpio_chip *mpfs_gpio = platform_get_drvdata(pdev);
+
+ gpiochip_remove(&mpfs_gpio->gc);
+ clk_disable_unprepare(mpfs_gpio->clk);
+
+ return 0;
+}
+
+static const struct of_device_id mpfs_of_ids[] = {
+ { .compatible = "microchip,mpfs-gpio", },
+ { /* end of list */ }
+};
+
+static struct platform_driver mpfs_gpio_driver = {
+ .probe = mpfs_gpio_probe,
+ .driver = {
+ .name = "microchip,mpfs-gpio",
+ .of_match_table = mpfs_of_ids,
+ },
+ .remove = mpfs_gpio_remove,
+};
+builtin_platform_driver(mpfs_gpio_driver);
--
2.25.1
On 2022-08-15 13:08, [email protected] wrote:
> From: Lewis Hanly <[email protected]>
>
>
> Add a driver to support the Polarfire SoC gpio controller.
> Tested with latest 5.19 kernel.
>
> MPFS gpio interrupts are connected to IOMUX configured by system
> register GPIO_INTERRUPT_FAB_CR(31:0). Interrupt connection for
> some GPIO's to the PLIC (Platform Level Interrupt Controller) can be
> shared (not directly connected) or direct if connection is available.
>
> previously upstreamed dt-bindings: gpio: microchip,mpfs-gpio.yaml
>
> Changes in v6
> Fixed typo causing compile issue with kernel 6.0-rc1
3 versions in just over 5 hours. How about taking a step back
and *wait* for people to review this code? It isn't like this
is going to be merged tomorrow anyway...
The expected rate for new versions of a patch is about once
a week (see Documentation/process/submitting-patches.rst).
M.
--
Jazz is not dead. It just smells funny...
Lewis, Marc,
Top posting this bit as it's important context & a quest that I don't
have a specific place to ask.
So I have two concerns/questions about this patch, of which both
relate to interrupt handling. From the cover:
> Changed the interrupt handling from Hierarchical flow to chained
> interrupt flow. The reason for the change was with hierarchical flow we
> requried a interrupt number mapping array to work with our HW and this
> was not acceptable. On reviewing the architecture the chained interrupt
> flow works better for our hardware and configurations which are not
> fixed in Silicon.
Previously we discussed the configurations of the hardware:
https://lore.kernel.org/linux-riscv/[email protected]/
To recap, the SoC has 96 GPIOs but fewer interrupts. To provide
access to all 96 GPIOs (32 per block) there is a muxed interrupt
per block too. Whichever GPIOs do not have a direct connection
to the plic and lumped into the muxed interrupt. Which interupts
are muxed or direct is set by a single register, not part of
any of the 3 GPIO blocks. On a per block level, there is no set
configuration for which interrupts are direct or muxed - all or
no GPIO interrupts may be muxed.
My first question is how are individual GPIOs & their interrupts
related? As the interrupt mapping is sparse, how do we know that
interrupt 26 specifically corresponds to GPIO 14 on block 0, but
all 31 other GPIOs are using interrupt 51? Maybe this is never something
that matters or I have missed something.
Second question is inline @ mpfs_gpio_irq_handler().
On 15/08/2022 13:08, [email protected] wrote:
> From: Lewis Hanly <[email protected]>
>
> Add a driver to support the Polarfire SoC gpio controller
>
> Signed-off-by: Lewis Hanly <[email protected]>
> ---
> drivers/gpio/Kconfig | 7 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-mpfs.c | 317 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 325 insertions(+)
> create mode 100644 drivers/gpio/gpio-mpfs.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 0642f579196f..303ea7f6f7bc 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -490,6 +490,13 @@ config GPIO_PMIC_EIC_SPRD
> help
> Say yes here to support Spreadtrum PMIC EIC device.
>
> +config GPIO_POLARFIRE_SOC
> + bool "Microchip FPGA GPIO support"
> + depends on OF_GPIO
> + select GPIOLIB_IRQCHIP
> + help
> + Say yes here to support the GPIO device on Microchip FPGAs.
> +
> config GPIO_PXA
> bool "PXA GPIO support"
> depends on ARCH_PXA || ARCH_MMP || COMPILE_TEST
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index a0985d30f51b..e04061ac5a28 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -120,6 +120,7 @@ obj-$(CONFIG_GPIO_PCI_IDIO_16) += gpio-pci-idio-16.o
> obj-$(CONFIG_GPIO_PISOSR) += gpio-pisosr.o
> obj-$(CONFIG_GPIO_PL061) += gpio-pl061.o
> obj-$(CONFIG_GPIO_PMIC_EIC_SPRD) += gpio-pmic-eic-sprd.o
> +obj-$(CONFIG_GPIO_POLARFIRE_SOC) += gpio-mpfs.o
> obj-$(CONFIG_GPIO_PXA) += gpio-pxa.o
> obj-$(CONFIG_GPIO_RASPBERRYPI_EXP) += gpio-raspberrypi-exp.o
> obj-$(CONFIG_GPIO_RC5T583) += gpio-rc5t583.o
> diff --git a/drivers/gpio/gpio-mpfs.c b/drivers/gpio/gpio-mpfs.c
> new file mode 100644
> index 000000000000..00e68a8b891d
> --- /dev/null
> +++ b/drivers/gpio/gpio-mpfs.c
> @@ -0,0 +1,317 @@
> +// SPDX-License-Identifier: (GPL-2.0)
> +/*
> + * Microchip PolarFire SoC (MPFS) GPIO controller driver
> + *
> + * Copyright (c) 2018-2022 Microchip Technology Inc. and its subsidiaries
> + *
> + * Author: Lewis Hanly <[email protected]>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/init.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +
> +#define MPFS_GPIO_CTRL(i) (0x4 * (i))
> +#define MAX_NUM_GPIO 32
> +#define MPFS_GPIO_EN_INT 3
> +#define MPFS_GPIO_EN_OUT_BUF BIT(2)
> +#define MPFS_GPIO_EN_IN BIT(1)
> +#define MPFS_GPIO_EN_OUT BIT(0)
> +
> +#define MPFS_GPIO_TYPE_INT_EDGE_BOTH 0x80
> +#define MPFS_GPIO_TYPE_INT_EDGE_NEG 0x60
> +#define MPFS_GPIO_TYPE_INT_EDGE_POS 0x40
> +#define MPFS_GPIO_TYPE_INT_LEVEL_LOW 0x20
> +#define MPFS_GPIO_TYPE_INT_LEVEL_HIGH 0x00
> +#define MPFS_GPIO_TYPE_INT_MASK GENMASK(7, 5)
> +#define MPFS_IRQ_REG 0x80
> +#define MPFS_INP_REG 0x84
> +#define MPFS_OUTP_REG 0x88
> +
> +struct mpfs_gpio_chip {
> + void __iomem *base;
> + struct clk *clk;
> + raw_spinlock_t lock;
> + struct gpio_chip gc;
> +};
> +
> +static void mpfs_gpio_assign_bit(void __iomem *addr, unsigned int bit_offset, bool value)
> +{
> + unsigned long reg = readl(addr);
> +
> + __assign_bit(bit_offset, ®, value);
> + writel(reg, addr);
> +}
> +
> +static int mpfs_gpio_direction_input(struct gpio_chip *gc, unsigned int gpio_index)
> +{
> + struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> + u32 gpio_cfg;
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&mpfs_gpio->lock, flags);
> +
> + gpio_cfg = readl(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
> + gpio_cfg |= MPFS_GPIO_EN_IN;
> + gpio_cfg &= ~(MPFS_GPIO_EN_OUT | MPFS_GPIO_EN_OUT_BUF);
> + writel(gpio_cfg, mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
> +
> + raw_spin_unlock_irqrestore(&mpfs_gpio->lock, flags);
> +
> + return 0;
> +}
> +
> +static int mpfs_gpio_direction_output(struct gpio_chip *gc, unsigned int gpio_index, int value)
> +{
> + struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> + u32 gpio_cfg;
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&mpfs_gpio->lock, flags);
> +
> + gpio_cfg = readl(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
> + gpio_cfg |= MPFS_GPIO_EN_OUT | MPFS_GPIO_EN_OUT_BUF;
> + gpio_cfg &= ~MPFS_GPIO_EN_IN;
> + writel(gpio_cfg, mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
> +
> + mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_OUTP_REG, gpio_index, value);
> +
> + raw_spin_unlock_irqrestore(&mpfs_gpio->lock, flags);
> +
> + return 0;
> +}
> +
> +static int mpfs_gpio_get_direction(struct gpio_chip *gc,
> + unsigned int gpio_index)
> +{
> + struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> + u32 gpio_cfg;
> +
> + gpio_cfg = readl(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
> + if (gpio_cfg & MPFS_GPIO_EN_IN)
> + return GPIO_LINE_DIRECTION_IN;
> +
> + return GPIO_LINE_DIRECTION_OUT;
> +}
> +
> +static int mpfs_gpio_get(struct gpio_chip *gc,
> + unsigned int gpio_index)
> +{
> + struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> +
> + return !!(readl(mpfs_gpio->base + MPFS_INP_REG) & BIT(gpio_index));
> +}
> +
> +static void mpfs_gpio_set(struct gpio_chip *gc, unsigned int gpio_index, int value)
> +{
> + struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> + unsigned long flags;
> +
> + raw_spin_lock_irqsave(&mpfs_gpio->lock, flags);
> +
> + mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_OUTP_REG,
> + gpio_index, value);
> +
> + raw_spin_unlock_irqrestore(&mpfs_gpio->lock, flags);
> +}
> +
> +static int mpfs_gpio_irq_set_type(struct irq_data *data, unsigned int type)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
> + struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> + int gpio_index = irqd_to_hwirq(data);
> + u32 interrupt_type;
> + u32 gpio_cfg;
> + unsigned long flags;
> +
> + switch (type) {
> + case IRQ_TYPE_EDGE_BOTH:
> + interrupt_type = MPFS_GPIO_TYPE_INT_EDGE_BOTH;
> + break;
> + case IRQ_TYPE_EDGE_FALLING:
> + interrupt_type = MPFS_GPIO_TYPE_INT_EDGE_NEG;
> + break;
> + case IRQ_TYPE_EDGE_RISING:
> + interrupt_type = MPFS_GPIO_TYPE_INT_EDGE_POS;
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + interrupt_type = MPFS_GPIO_TYPE_INT_LEVEL_HIGH;
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + interrupt_type = MPFS_GPIO_TYPE_INT_LEVEL_LOW;
> + break;
> + }
> +
> + raw_spin_lock_irqsave(&mpfs_gpio->lock, flags);
> +
> + gpio_cfg = readl(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
> + gpio_cfg &= ~MPFS_GPIO_TYPE_INT_MASK;
> + gpio_cfg |= interrupt_type;
> + writel(gpio_cfg, mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index));
> +
> + raw_spin_unlock_irqrestore(&mpfs_gpio->lock, flags);
> +
> + return 0;
> +}
> +
> +static void mpfs_gpio_irq_unmask(struct irq_data *data)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
> + struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> + int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO;
> +
> + mpfs_gpio_direction_input(gc, gpio_index);
> + mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1);
> + mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index),
> + MPFS_GPIO_EN_INT, 1);
> +}
> +
> +static void mpfs_gpio_irq_mask(struct irq_data *data)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
> + struct mpfs_gpio_chip *mpfs_gpio = gpiochip_get_data(gc);
> + int gpio_index = irqd_to_hwirq(data) % MAX_NUM_GPIO;
> +
> + mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, gpio_index, 1);
> + mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_GPIO_CTRL(gpio_index),
> + MPFS_GPIO_EN_INT, 0);
> +}
> +
> +static const struct irq_chip mpfs_gpio_irqchip = {
> + .name = "mpfs",
> + .irq_set_type = mpfs_gpio_irq_set_type,
> + .irq_mask = mpfs_gpio_irq_mask,
> + .irq_unmask = mpfs_gpio_irq_unmask,
> + .flags = IRQCHIP_MASK_ON_SUSPEND,
> +};
> +
> +static void mpfs_gpio_irq_handler(struct irq_desc *desc)
> +{
> + struct irq_chip *irqchip = irq_desc_get_chip(desc);
> + struct mpfs_gpio_chip *mpfs_gpio =
> + gpiochip_get_data(irq_desc_get_handler_data(desc));
> + unsigned long status;
> + int offset;
> +
> + chained_irq_enter(irqchip, desc);
> +
> + status = readl(mpfs_gpio->base + MPFS_IRQ_REG);
> + for_each_set_bit(offset, &status, mpfs_gpio->gc.ngpio) {
> + mpfs_gpio_assign_bit(mpfs_gpio->base + MPFS_IRQ_REG, offset, 1);
> + generic_handle_irq(irq_find_mapping(mpfs_gpio->gc.irq.domain, offset));
> + }
> +
My concern here is for a potential race or missed interrupts.
As this interrupt handler is going to be used for [1..32] different
interrupts at the same time, any time it is hit it will clear all of
the GPIOs pending interrupt bits - even for GPIOs that are not using
the interrupt that triggered the handler. Again, maybe I have missed
something that ensures that cannot be a problem, but I would like to
be convinced.
Thanks,
Conor.
> + chained_irq_exit(irqchip, desc);
> +}
> +
> +static int mpfs_gpio_probe(struct platform_device *pdev)
> +{
> + struct clk *clk;
> + struct device *dev = &pdev->dev;
> + struct device_node *node = pdev->dev.of_node;
> + struct mpfs_gpio_chip *mpfs_gpio;
> + struct gpio_irq_chip *girq;
> + int i, ret, ngpios, nirqs;
> +
> + mpfs_gpio = devm_kzalloc(dev, sizeof(*mpfs_gpio), GFP_KERNEL);
> + if (!mpfs_gpio)
> + return -ENOMEM;
> +
> + mpfs_gpio->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(mpfs_gpio->base))
> + return dev_err_probe(dev, PTR_ERR(mpfs_gpio->base), "failed to ioremap memory resource\n");
> +
> + clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(clk))
> + return dev_err_probe(dev, PTR_ERR(clk), "devm_clk_get failed\n");
> +
> + ret = clk_prepare_enable(clk);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to enable clock\n");
> +
> + mpfs_gpio->clk = clk;
> +
> + ngpios = MAX_NUM_GPIO;
> + device_property_read_u32(dev, "ngpios", &ngpios);
> + if (ngpios > MAX_NUM_GPIO)
> + ngpios = MAX_NUM_GPIO;
> +
> + raw_spin_lock_init(&mpfs_gpio->lock);
> + mpfs_gpio->gc.direction_input = mpfs_gpio_direction_input;
> + mpfs_gpio->gc.direction_output = mpfs_gpio_direction_output;
> + mpfs_gpio->gc.get_direction = mpfs_gpio_get_direction;
> + mpfs_gpio->gc.get = mpfs_gpio_get;
> + mpfs_gpio->gc.set = mpfs_gpio_set;
> + mpfs_gpio->gc.base = -1;
> + mpfs_gpio->gc.ngpio = ngpios;
> + mpfs_gpio->gc.label = dev_name(dev);
> + mpfs_gpio->gc.parent = dev;
> + mpfs_gpio->gc.owner = THIS_MODULE;
> +
> + nirqs = of_irq_count(node);
> + if (nirqs > MAX_NUM_GPIO) {
> + ret = -ENXIO;
> + goto cleanup_clock;
> + }
> + girq = &mpfs_gpio->gc.irq;
> + gpio_irq_chip_set_chip(girq, &mpfs_gpio_irqchip);
> + girq->handler = handle_simple_irq;
> + girq->parent_handler = mpfs_gpio_irq_handler;
> + girq->default_type = IRQ_TYPE_NONE;
> + girq->num_parents = nirqs;
> + girq->parents = devm_kcalloc(&pdev->dev, nirqs,
> + sizeof(*girq->parents), GFP_KERNEL);
> + if (!girq->parents) {
> + ret = -ENOMEM;
> + goto cleanup_clock;
> + }
> + for (i = 0; i < nirqs; i++)
> + girq->parents[i] = platform_get_irq(pdev, i);
> +
> + ret = gpiochip_add_data(&mpfs_gpio->gc, mpfs_gpio);
> + if (ret)
> + goto cleanup_clock;
> +
> + platform_set_drvdata(pdev, mpfs_gpio);
> +
> + return 0;
> +
> +cleanup_clock:
> + clk_disable_unprepare(mpfs_gpio->clk);
> + return ret;
> +}
> +
> +static int mpfs_gpio_remove(struct platform_device *pdev)
> +{
> + struct mpfs_gpio_chip *mpfs_gpio = platform_get_drvdata(pdev);
> +
> + gpiochip_remove(&mpfs_gpio->gc);
> + clk_disable_unprepare(mpfs_gpio->clk);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mpfs_of_ids[] = {
> + { .compatible = "microchip,mpfs-gpio", },
> + { /* end of list */ }
> +};
> +
> +static struct platform_driver mpfs_gpio_driver = {
> + .probe = mpfs_gpio_probe,
> + .driver = {
> + .name = "microchip,mpfs-gpio",
> + .of_match_table = mpfs_of_ids,
> + },
> + .remove = mpfs_gpio_remove,
> +};
> +builtin_platform_driver(mpfs_gpio_driver);
On Mon, Aug 15, 2022 at 2:08 PM <[email protected]> wrote:
> From: Lewis Hanly <[email protected]>
>
> Add a driver to support the Polarfire SoC gpio controller
>
> Signed-off-by: Lewis Hanly <[email protected]>
This looks like textbook example of a driver that can use
CONFIG_GPIO_GENERIC
> +config GPIO_POLARFIRE_SOC
> + bool "Microchip FPGA GPIO support"
> + depends on OF_GPIO
> + select GPIOLIB_IRQCHIP
select GPIO_GENERIC
See e.g. drivers/gpio/gpio-ftgpio010.c for an example
of how to use bgpio_init() to set up the helper library to handle
the GPIO side of things and combine it with an irqchip.
You get get/set_multiple() for free with this approach.
Also see documentation for bgpio_init() in
drivers/gpio/gpio-mmio.c.
Yours,
Linus Walleij
On Mon, 2022-08-22 at 13:28 +0200, Linus Walleij wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Mon, Aug 15, 2022 at 2:08 PM <[email protected]> wrote:
>
> > From: Lewis Hanly <[email protected]>
> >
> > Add a driver to support the Polarfire SoC gpio controller
> >
> > Signed-off-by: Lewis Hanly <[email protected]>
>
> This looks like textbook example of a driver that can use
> CONFIG_GPIO_GENERIC
>
> > +config GPIO_POLARFIRE_SOC
> > + bool "Microchip FPGA GPIO support"
> > + depends on OF_GPIO
> > + select GPIOLIB_IRQCHIP
>
> select GPIO_GENERIC
>
> See e.g. drivers/gpio/gpio-ftgpio010.c for an example
> of how to use bgpio_init() to set up the helper library to handle
> the GPIO side of things and combine it with an irqchip.
> You get get/set_multiple() for free with this approach.
> Also see documentation for bgpio_init() in
> drivers/gpio/gpio-mmio.c.
>
> Yours,
> Linus Walleij
Thank you for the feedback.
We had looked at the bpgpio_init, our controller is not fully memory
mapped to support the bgpio_init() and get all routines for free.
While we have in/out and intr (interrupt state) 32-bit registers, we
would not get as much free as other generic memory mapped controllers.
For details on the gpio controller:
https://onlinedocs.microchip.com/pr/GUID-0E320577-28E6-4365-9BB8-9E1416A0A6E4-en-US-3/index.html?GUID-A0214F3C-C68A-45A9-B81C-4695A58C4A46#GUID-A0214F3C-C68A-45A9-B81C-4695A58C4A46__ID-00000AB0
On Tue, Aug 30, 2022 at 6:51 AM <[email protected]> wrote:
> We had looked at the bpgpio_init, our controller is not fully memory
> mapped to support the bgpio_init() and get all routines for free.
> While we have in/out and intr (interrupt state) 32-bit registers, we
> would not get as much free as other generic memory mapped controllers.
You're not really saying what the problem is?
Is it that some registers are not one-bit-indexed from 0 per GPIO?
Yours,
Linus Walleij
On Mon, Aug 22, 2022 at 2:30 PM Linus Walleij <[email protected]> wrote:
> On Mon, Aug 15, 2022 at 2:08 PM <[email protected]> wrote:
...
> This looks like textbook example of a driver that can use
> CONFIG_GPIO_GENERIC
> See e.g. drivers/gpio/gpio-ftgpio010.c for an example
> of how to use bgpio_init() to set up the helper library to handle
> the GPIO side of things and combine it with an irqchip.
> You get get/set_multiple() for free with this approach.
> Also see documentation for bgpio_init() in
> drivers/gpio/gpio-mmio.c.
I would also suggest looking at gpio-regmap.c.
--
With Best Regards,
Andy Shevchenko
On Wed, 2022-08-31 at 15:19 +0200, Linus Walleij wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Tue, Aug 30, 2022 at 6:51 AM <[email protected]> wrote:
>
> > We had looked at the bpgpio_init, our controller is not fully
> > memory
> > mapped to support the bgpio_init() and get all routines for free.
> > While we have in/out and intr (interrupt state) 32-bit registers,
> > we
> > would not get as much free as other generic memory mapped
> > controllers.
>
> You're not really saying what the problem is?
>
> Is it that some registers are not one-bit-indexed from 0 per GPIO?
Yes some of the registers are not one-bit-indexed per GPIO and for this
reason we had not implemented bgpio_init().
>
> Yours,
> Linus Walleij
On Thu, 2022-09-01 at 00:04 +0300, Andy Shevchenko wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Mon, Aug 22, 2022 at 2:30 PM Linus Walleij <
> [email protected]> wrote:
> > On Mon, Aug 15, 2022 at 2:08 PM <[email protected]> wrote:
>
> ...
>
> > This looks like textbook example of a driver that can use
> > CONFIG_GPIO_GENERIC
> > See e.g. drivers/gpio/gpio-ftgpio010.c for an example
> > of how to use bgpio_init() to set up the helper library to handle
> > the GPIO side of things and combine it with an irqchip.
> > You get get/set_multiple() for free with this approach.
> > Also see documentation for bgpio_init() in
> > drivers/gpio/gpio-mmio.c.
>
> I would also suggest looking at gpio-regmap.c.
Thanks Andy I will review the regmap.c file
>
> --
> With Best Regards,
> Andy Shevchenko
On Mon, Sep 5, 2022 at 12:45 PM <[email protected]> wrote:
> On Wed, 2022-08-31 at 15:19 +0200, Linus Walleij wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
> > On Tue, Aug 30, 2022 at 6:51 AM <[email protected]> wrote:
> >
> > > We had looked at the bpgpio_init, our controller is not fully
> > > memory
> > > mapped to support the bgpio_init() and get all routines for free.
> > > While we have in/out and intr (interrupt state) 32-bit registers,
> > > we
> > > would not get as much free as other generic memory mapped
> > > controllers.
> >
> > You're not really saying what the problem is?
> >
> > Is it that some registers are not one-bit-indexed from 0 per GPIO?
> Yes some of the registers are not one-bit-indexed per GPIO and for this
> reason we had not implemented bgpio_init().
OK that's a valid reason not to use that. Thanks!
The regmap may help though, have a look!
Yours,
Linus Walleij