* [PATCH 2/6 v2] gpio: ixp4xx: Convert to hierarchical GPIOLIB_IRQCHIP
2019-08-08 12:32 [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains Linus Walleij
@ 2019-08-08 12:32 ` Linus Walleij
2019-08-08 12:32 ` [PATCH 3/6 v2] qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core Linus Walleij
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2019-08-08 12:32 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Thomas Gleixner,
Marc Zyngier, Lina Iyer, Jon Hunter, Sowjanya Komatineni,
Bitan Biswas, linux-tegra, Thierry Reding, Brian Masney
This modifies the IXP4xx driver to use the new helpers
to handle the remapping of parent to child hardware irqs
in the gpiolib core.
This pulls the majority of the code out of the driver
and use the generic code in gpiolib.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Lina Iyer <ilina@codeaurora.org>
Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: Sowjanya Komatineni <skomatineni@nvidia.com>
Cc: Bitan Biswas <bbiswas@nvidia.com>
Cc: linux-tegra@vger.kernel.org
Cc: Thierry Reding <treding@nvidia.com>
Cc: Brian Masney <masneyb@onstation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Rebased.
ChangeLog RFC->v1:
- Fixed some bugs in dereferencing the gpio_chip first
from the irqchip data, then dereference the
local state container from the gpio_chip
- Adapted to changes in the core patch like provide
as translation callback rather than a table.
- Tested on the Linksys NSLU2 and works like a charm
---
drivers/gpio/Kconfig | 2 +-
drivers/gpio/gpio-ixp4xx.c | 277 +++++++++----------------------------
2 files changed, 63 insertions(+), 216 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index bb13c266c329..b34e9b11a7ef 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -288,7 +288,7 @@ config GPIO_IXP4XX
depends on ARM # For <asm/mach-types.h>
depends on ARCH_IXP4XX
select GPIO_GENERIC
- select IRQ_DOMAIN
+ select GPIOLIB_IRQCHIP
select IRQ_DOMAIN_HIERARCHY
help
Say yes here to support the GPIO functionality of a number of Intel
diff --git a/drivers/gpio/gpio-ixp4xx.c b/drivers/gpio/gpio-ixp4xx.c
index 670c2a85a35b..8bd23e80c61f 100644
--- a/drivers/gpio/gpio-ixp4xx.c
+++ b/drivers/gpio/gpio-ixp4xx.c
@@ -47,7 +47,6 @@
* @dev: containing device for this instance
* @fwnode: the fwnode for this GPIO chip
* @gc: gpiochip for this instance
- * @domain: irqdomain for this chip instance
* @base: remapped I/O-memory base
* @irq_edge: Each bit represents an IRQ: 1: edge-triggered,
* 0: level triggered
@@ -56,48 +55,22 @@ struct ixp4xx_gpio {
struct device *dev;
struct fwnode_handle *fwnode;
struct gpio_chip gc;
- struct irq_domain *domain;
void __iomem *base;
unsigned long long irq_edge;
};
-/**
- * struct ixp4xx_gpio_map - IXP4 GPIO to parent IRQ map
- * @gpio_offset: offset of the IXP4 GPIO line
- * @parent_hwirq: hwirq on the parent IRQ controller
- */
-struct ixp4xx_gpio_map {
- int gpio_offset;
- int parent_hwirq;
-};
-
-/* GPIO lines 0..12 have corresponding IRQs, GPIOs 13..15 have no IRQs */
-const struct ixp4xx_gpio_map ixp4xx_gpiomap[] = {
- { .gpio_offset = 0, .parent_hwirq = 6 },
- { .gpio_offset = 1, .parent_hwirq = 7 },
- { .gpio_offset = 2, .parent_hwirq = 19 },
- { .gpio_offset = 3, .parent_hwirq = 20 },
- { .gpio_offset = 4, .parent_hwirq = 21 },
- { .gpio_offset = 5, .parent_hwirq = 22 },
- { .gpio_offset = 6, .parent_hwirq = 23 },
- { .gpio_offset = 7, .parent_hwirq = 24 },
- { .gpio_offset = 8, .parent_hwirq = 25 },
- { .gpio_offset = 9, .parent_hwirq = 26 },
- { .gpio_offset = 10, .parent_hwirq = 27 },
- { .gpio_offset = 11, .parent_hwirq = 28 },
- { .gpio_offset = 12, .parent_hwirq = 29 },
-};
-
static void ixp4xx_gpio_irq_ack(struct irq_data *d)
{
- struct ixp4xx_gpio *g = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct ixp4xx_gpio *g = gpiochip_get_data(gc);
__raw_writel(BIT(d->hwirq), g->base + IXP4XX_REG_GPIS);
}
static void ixp4xx_gpio_irq_unmask(struct irq_data *d)
{
- struct ixp4xx_gpio *g = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct ixp4xx_gpio *g = gpiochip_get_data(gc);
/* ACK when unmasking if not edge-triggered */
if (!(g->irq_edge & BIT(d->hwirq)))
@@ -108,7 +81,8 @@ static void ixp4xx_gpio_irq_unmask(struct irq_data *d)
static int ixp4xx_gpio_irq_set_type(struct irq_data *d, unsigned int type)
{
- struct ixp4xx_gpio *g = irq_data_get_irq_chip_data(d);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct ixp4xx_gpio *g = gpiochip_get_data(gc);
int line = d->hwirq;
unsigned long flags;
u32 int_style;
@@ -187,122 +161,31 @@ static struct irq_chip ixp4xx_gpio_irqchip = {
.irq_set_type = ixp4xx_gpio_irq_set_type,
};
-static int ixp4xx_gpio_to_irq(struct gpio_chip *gc, unsigned int offset)
-{
- struct ixp4xx_gpio *g = gpiochip_get_data(gc);
- struct irq_fwspec fwspec;
-
- fwspec.fwnode = g->fwnode;
- fwspec.param_count = 2;
- fwspec.param[0] = offset;
- fwspec.param[1] = IRQ_TYPE_NONE;
-
- return irq_create_fwspec_mapping(&fwspec);
-}
-
-static int ixp4xx_gpio_irq_domain_translate(struct irq_domain *domain,
- struct irq_fwspec *fwspec,
- unsigned long *hwirq,
- unsigned int *type)
+static int ixp4xx_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
+ unsigned int child,
+ unsigned int child_type,
+ unsigned int *parent,
+ unsigned int *parent_type)
{
- int ret;
+ /* All these interrupts are level high in the CPU */
+ *parent_type = IRQ_TYPE_LEVEL_HIGH;
- /* We support standard DT translation */
- if (is_of_node(fwspec->fwnode) && fwspec->param_count == 2) {
- return irq_domain_translate_twocell(domain, fwspec,
- hwirq, type);
+ /* GPIO lines 0..12 have dedicated IRQs */
+ if (child == 0) {
+ *parent = 6;
+ return 0;
}
-
- /* This goes away when we transition to DT */
- if (is_fwnode_irqchip(fwspec->fwnode)) {
- ret = irq_domain_translate_twocell(domain, fwspec,
- hwirq, type);
- if (ret)
- return ret;
- WARN_ON(*type == IRQ_TYPE_NONE);
+ if (child == 1) {
+ *parent = 7;
return 0;
}
- return -EINVAL;
-}
-
-static int ixp4xx_gpio_irq_domain_alloc(struct irq_domain *d,
- unsigned int irq, unsigned int nr_irqs,
- void *data)
-{
- struct ixp4xx_gpio *g = d->host_data;
- irq_hw_number_t hwirq;
- unsigned int type = IRQ_TYPE_NONE;
- struct irq_fwspec *fwspec = data;
- int ret;
- int i;
-
- ret = ixp4xx_gpio_irq_domain_translate(d, fwspec, &hwirq, &type);
- if (ret)
- return ret;
-
- dev_dbg(g->dev, "allocate IRQ %d..%d, hwirq %lu..%lu\n",
- irq, irq + nr_irqs - 1,
- hwirq, hwirq + nr_irqs - 1);
-
- for (i = 0; i < nr_irqs; i++) {
- struct irq_fwspec parent_fwspec;
- const struct ixp4xx_gpio_map *map;
- int j;
-
- /* Not all lines support IRQs */
- for (j = 0; j < ARRAY_SIZE(ixp4xx_gpiomap); j++) {
- map = &ixp4xx_gpiomap[j];
- if (map->gpio_offset == hwirq)
- break;
- }
- if (j == ARRAY_SIZE(ixp4xx_gpiomap)) {
- dev_err(g->dev, "can't look up hwirq %lu\n", hwirq);
- return -EINVAL;
- }
- dev_dbg(g->dev, "found parent hwirq %u\n", map->parent_hwirq);
-
- /*
- * We set handle_bad_irq because the .set_type() should
- * always be invoked and set the right type of handler.
- */
- irq_domain_set_info(d,
- irq + i,
- hwirq + i,
- &ixp4xx_gpio_irqchip,
- g,
- handle_bad_irq,
- NULL, NULL);
- irq_set_probe(irq + i);
-
- /*
- * Create a IRQ fwspec to send up to the parent irqdomain:
- * specify the hwirq we address on the parent and tie it
- * all together up the chain.
- */
- parent_fwspec.fwnode = d->parent->fwnode;
- parent_fwspec.param_count = 2;
- parent_fwspec.param[0] = map->parent_hwirq;
- /* This parent only handles asserted level IRQs */
- parent_fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
- dev_dbg(g->dev, "alloc_irqs_parent for %d parent hwirq %d\n",
- irq + i, map->parent_hwirq);
- ret = irq_domain_alloc_irqs_parent(d, irq + i, 1,
- &parent_fwspec);
- if (ret)
- dev_err(g->dev,
- "failed to allocate parent hwirq %d for hwirq %lu\n",
- map->parent_hwirq, hwirq);
+ if (child >= 2 && child <= 12) {
+ *parent = child + 17;
+ return 0;
}
-
- return 0;
+ return -EINVAL;
}
-static const struct irq_domain_ops ixp4xx_gpio_irqdomain_ops = {
- .translate = ixp4xx_gpio_irq_domain_translate,
- .alloc = ixp4xx_gpio_irq_domain_alloc,
- .free = irq_domain_free_irqs_common,
-};
-
static int ixp4xx_gpio_probe(struct platform_device *pdev)
{
unsigned long flags;
@@ -311,8 +194,8 @@ static int ixp4xx_gpio_probe(struct platform_device *pdev)
struct irq_domain *parent;
struct resource *res;
struct ixp4xx_gpio *g;
+ struct gpio_irq_chip *girq;
int ret;
- int i;
g = devm_kzalloc(dev, sizeof(*g), GFP_KERNEL);
if (!g)
@@ -326,6 +209,35 @@ static int ixp4xx_gpio_probe(struct platform_device *pdev)
return PTR_ERR(g->base);
}
+ /*
+ * When we convert to device tree we will simply look up the
+ * parent irqdomain using irq_find_host(parent) as parent comes
+ * from IRQCHIP_DECLARE(), then use of_node_to_fwnode() to get
+ * the fwnode. For now we need this boardfile style code.
+ */
+ if (np) {
+ struct device_node *irq_parent;
+
+ irq_parent = of_irq_find_parent(np);
+ if (!irq_parent) {
+ dev_err(dev, "no IRQ parent node\n");
+ return -ENODEV;
+ }
+ parent = irq_find_host(irq_parent);
+ if (!parent) {
+ dev_err(dev, "no IRQ parent domain\n");
+ return -ENODEV;
+ }
+ g->fwnode = of_node_to_fwnode(np);
+ } else {
+ parent = ixp4xx_get_irq_domain();
+ g->fwnode = irq_domain_alloc_fwnode(g->base);
+ if (!g->fwnode) {
+ dev_err(dev, "no domain base\n");
+ return -ENODEV;
+ }
+ }
+
/*
* Make sure GPIO 14 and 15 are NOT used as clocks but GPIO on
* specific machines.
@@ -360,7 +272,6 @@ static int ixp4xx_gpio_probe(struct platform_device *pdev)
dev_err(dev, "unable to init generic GPIO\n");
return ret;
}
- g->gc.to_irq = ixp4xx_gpio_to_irq;
g->gc.ngpio = 16;
g->gc.label = "IXP4XX_GPIO_CHIP";
/*
@@ -372,86 +283,22 @@ static int ixp4xx_gpio_probe(struct platform_device *pdev)
g->gc.parent = &pdev->dev;
g->gc.owner = THIS_MODULE;
+ girq = &g->gc.irq;
+ girq->chip = &ixp4xx_gpio_irqchip;
+ girq->fwnode = g->fwnode;
+ girq->parent_domain = parent;
+ girq->child_to_parent_hwirq = ixp4xx_gpio_child_to_parent_hwirq;
+ girq->handler = handle_bad_irq;
+ girq->default_type = IRQ_TYPE_NONE;
+
ret = devm_gpiochip_add_data(dev, &g->gc, g);
if (ret) {
dev_err(dev, "failed to add SoC gpiochip\n");
return ret;
}
- /*
- * When we convert to device tree we will simply look up the
- * parent irqdomain using irq_find_host(parent) as parent comes
- * from IRQCHIP_DECLARE(), then use of_node_to_fwnode() to get
- * the fwnode. For now we need this boardfile style code.
- */
- if (np) {
- struct device_node *irq_parent;
-
- irq_parent = of_irq_find_parent(np);
- if (!irq_parent) {
- dev_err(dev, "no IRQ parent node\n");
- return -ENODEV;
- }
- parent = irq_find_host(irq_parent);
- if (!parent) {
- dev_err(dev, "no IRQ parent domain\n");
- return -ENODEV;
- }
- g->fwnode = of_node_to_fwnode(np);
- } else {
- parent = ixp4xx_get_irq_domain();
- g->fwnode = irq_domain_alloc_fwnode(g->base);
- if (!g->fwnode) {
- dev_err(dev, "no domain base\n");
- return -ENODEV;
- }
- }
- g->domain = irq_domain_create_hierarchy(parent,
- IRQ_DOMAIN_FLAG_HIERARCHY,
- ARRAY_SIZE(ixp4xx_gpiomap),
- g->fwnode,
- &ixp4xx_gpio_irqdomain_ops,
- g);
- if (!g->domain) {
- irq_domain_free_fwnode(g->fwnode);
- dev_err(dev, "no hierarchical irq domain\n");
- return ret;
- }
-
- /*
- * After adding OF support, this is no longer needed: irqs
- * will be allocated for the respective fwnodes.
- */
- if (!np) {
- for (i = 0; i < ARRAY_SIZE(ixp4xx_gpiomap); i++) {
- const struct ixp4xx_gpio_map *map = &ixp4xx_gpiomap[i];
- struct irq_fwspec fwspec;
-
- fwspec.fwnode = g->fwnode;
- /* This is the hwirq for the GPIO line side of things */
- fwspec.param[0] = map->gpio_offset;
- fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
- fwspec.param_count = 2;
- ret = __irq_domain_alloc_irqs(g->domain,
- -1, /* just pick something */
- 1,
- NUMA_NO_NODE,
- &fwspec,
- false,
- NULL);
- if (ret < 0) {
- irq_domain_free_fwnode(g->fwnode);
- dev_err(dev,
- "can not allocate irq for GPIO line %d parent hwirq %d in hierarchy domain: %d\n",
- map->gpio_offset, map->parent_hwirq,
- ret);
- return ret;
- }
- }
- }
-
platform_set_drvdata(pdev, g);
- dev_info(dev, "IXP4 GPIO @%p registered\n", g->base);
+ dev_info(dev, "IXP4 GPIO registered\n");
return 0;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/6 v2] qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core
2019-08-08 12:32 [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains Linus Walleij
2019-08-08 12:32 ` [PATCH 2/6 v2] gpio: ixp4xx: Convert to hierarchical GPIOLIB_IRQCHIP Linus Walleij
@ 2019-08-08 12:32 ` Linus Walleij
2019-08-14 8:19 ` Linus Walleij
2019-08-08 12:32 ` [PATCH 4/6 v2] RFT: gpio: thunderx: Switch to GPIOLIB_IRQCHIP Linus Walleij
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Linus Walleij @ 2019-08-08 12:32 UTC (permalink / raw)
To: linux-gpio; +Cc: Bartosz Golaszewski, Brian Masney, Linus Walleij
From: Brian Masney <masneyb@onstation.org>
Now that the GPIO core has support for hierarchical IRQ chips, convert
Qualcomm's spmi-gpio over to use these new helpers to reduce duplicated
code across drivers.
This change was tested on a LG Nexus 5 (hammerhead) phone.
Signed-off-by: Brian Masney <masneyb@onstation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Change "child_pin_to_irq" into "child_offset_to_irq" so
we avoid confusion with pin control.
---
drivers/pinctrl/qcom/Kconfig | 1 +
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 92 +++++++-----------------
2 files changed, 26 insertions(+), 67 deletions(-)
diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index 8e14a5f2e970..fa2c87821401 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -138,6 +138,7 @@ config PINCTRL_QCOM_SPMI_PMIC
select PINMUX
select PINCONF
select GENERIC_PINCONF
+ select GPIOLIB_IRQCHIP
select IRQ_DOMAIN_HIERARCHY
help
This is the pinctrl, pinmux, pinconf and gpiolib driver for the
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index f39da87ea185..442db15e0729 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -170,8 +170,6 @@ struct pmic_gpio_state {
struct regmap *map;
struct pinctrl_dev *ctrl;
struct gpio_chip chip;
- struct fwnode_handle *fwnode;
- struct irq_domain *domain;
};
static const struct pinconf_generic_params pmic_gpio_bindings[] = {
@@ -751,23 +749,6 @@ static int pmic_gpio_of_xlate(struct gpio_chip *chip,
return gpio_desc->args[0] - PMIC_GPIO_PHYSICAL_OFFSET;
}
-static int pmic_gpio_to_irq(struct gpio_chip *chip, unsigned pin)
-{
- struct pmic_gpio_state *state = gpiochip_get_data(chip);
- struct irq_fwspec fwspec;
-
- fwspec.fwnode = state->fwnode;
- fwspec.param_count = 2;
- fwspec.param[0] = pin + PMIC_GPIO_PHYSICAL_OFFSET;
- /*
- * Set the type to a safe value temporarily. This will be overwritten
- * later with the proper value by irq_set_type.
- */
- fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
-
- return irq_create_fwspec_mapping(&fwspec);
-}
-
static void pmic_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
{
struct pmic_gpio_state *state = gpiochip_get_data(chip);
@@ -787,7 +768,6 @@ static const struct gpio_chip pmic_gpio_gpio_template = {
.request = gpiochip_generic_request,
.free = gpiochip_generic_free,
.of_xlate = pmic_gpio_of_xlate,
- .to_irq = pmic_gpio_to_irq,
.dbg_show = pmic_gpio_dbg_show,
};
@@ -964,46 +944,24 @@ static int pmic_gpio_domain_translate(struct irq_domain *domain,
return 0;
}
-static int pmic_gpio_domain_alloc(struct irq_domain *domain, unsigned int virq,
- unsigned int nr_irqs, void *data)
+static unsigned int pmic_gpio_child_offset_to_irq(struct gpio_chip *chip,
+ unsigned int offset)
{
- struct pmic_gpio_state *state = container_of(domain->host_data,
- struct pmic_gpio_state,
- chip);
- struct irq_fwspec *fwspec = data;
- struct irq_fwspec parent_fwspec;
- irq_hw_number_t hwirq;
- unsigned int type;
- int ret, i;
-
- ret = pmic_gpio_domain_translate(domain, fwspec, &hwirq, &type);
- if (ret)
- return ret;
-
- for (i = 0; i < nr_irqs; i++)
- irq_domain_set_info(domain, virq + i, hwirq + i,
- &pmic_gpio_irq_chip, state,
- handle_level_irq, NULL, NULL);
+ return offset + PMIC_GPIO_PHYSICAL_OFFSET;
+}
- parent_fwspec.fwnode = domain->parent->fwnode;
- parent_fwspec.param_count = 4;
- parent_fwspec.param[0] = 0;
- parent_fwspec.param[1] = hwirq + 0xc0;
- parent_fwspec.param[2] = 0;
- parent_fwspec.param[3] = fwspec->param[1];
+static int pmic_gpio_child_to_parent_hwirq(struct gpio_chip *chip,
+ unsigned int child_hwirq,
+ unsigned int child_type,
+ unsigned int *parent_hwirq,
+ unsigned int *parent_type)
+{
+ *parent_hwirq = child_hwirq + 0xc0;
+ *parent_type = child_type;
- return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
- &parent_fwspec);
+ return 0;
}
-static const struct irq_domain_ops pmic_gpio_domain_ops = {
- .activate = gpiochip_irq_domain_activate,
- .alloc = pmic_gpio_domain_alloc,
- .deactivate = gpiochip_irq_domain_deactivate,
- .free = irq_domain_free_irqs_common,
- .translate = pmic_gpio_domain_translate,
-};
-
static int pmic_gpio_probe(struct platform_device *pdev)
{
struct irq_domain *parent_domain;
@@ -1013,6 +971,7 @@ static int pmic_gpio_probe(struct platform_device *pdev)
struct pinctrl_desc *pctrldesc;
struct pmic_gpio_pad *pad, *pads;
struct pmic_gpio_state *state;
+ struct gpio_irq_chip *girq;
int ret, npins, i;
u32 reg;
@@ -1092,19 +1051,21 @@ static int pmic_gpio_probe(struct platform_device *pdev)
if (!parent_domain)
return -ENXIO;
- state->fwnode = of_node_to_fwnode(state->dev->of_node);
- state->domain = irq_domain_create_hierarchy(parent_domain, 0,
- state->chip.ngpio,
- state->fwnode,
- &pmic_gpio_domain_ops,
- &state->chip);
- if (!state->domain)
- return -ENODEV;
+ girq = &state->chip.irq;
+ girq->chip = &pmic_gpio_irq_chip;
+ girq->default_type = IRQ_TYPE_NONE;
+ girq->handler = handle_level_irq;
+ girq->fwnode = of_node_to_fwnode(state->dev->of_node);
+ girq->parent_domain = parent_domain;
+ girq->child_to_parent_hwirq = pmic_gpio_child_to_parent_hwirq;
+ girq->populate_parent_fwspec = gpiochip_populate_parent_fwspec_fourcell;
+ girq->child_offset_to_irq = pmic_gpio_child_offset_to_irq;
+ girq->child_irq_domain_ops.translate = pmic_gpio_domain_translate;
ret = gpiochip_add_data(&state->chip, state);
if (ret) {
dev_err(state->dev, "can't add gpio chip\n");
- goto err_chip_add_data;
+ return ret;
}
/*
@@ -1130,8 +1091,6 @@ static int pmic_gpio_probe(struct platform_device *pdev)
err_range:
gpiochip_remove(&state->chip);
-err_chip_add_data:
- irq_domain_remove(state->domain);
return ret;
}
@@ -1140,7 +1099,6 @@ static int pmic_gpio_remove(struct platform_device *pdev)
struct pmic_gpio_state *state = platform_get_drvdata(pdev);
gpiochip_remove(&state->chip);
- irq_domain_remove(state->domain);
return 0;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6 v2] qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core
2019-08-08 12:32 ` [PATCH 3/6 v2] qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core Linus Walleij
@ 2019-08-14 8:19 ` Linus Walleij
0 siblings, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2019-08-14 8:19 UTC (permalink / raw)
To: open list:GPIO SUBSYSTEM; +Cc: Bartosz Golaszewski, Brian Masney
On Thu, Aug 8, 2019 at 2:32 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> From: Brian Masney <masneyb@onstation.org>
>
> Now that the GPIO core has support for hierarchical IRQ chips, convert
> Qualcomm's spmi-gpio over to use these new helpers to reduce duplicated
> code across drivers.
>
> This change was tested on a LG Nexus 5 (hammerhead) phone.
>
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Change "child_pin_to_irq" into "child_offset_to_irq" so
> we avoid confusion with pin control.
This patch is now applied.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/6 v2] RFT: gpio: thunderx: Switch to GPIOLIB_IRQCHIP
2019-08-08 12:32 [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains Linus Walleij
2019-08-08 12:32 ` [PATCH 2/6 v2] gpio: ixp4xx: Convert to hierarchical GPIOLIB_IRQCHIP Linus Walleij
2019-08-08 12:32 ` [PATCH 3/6 v2] qcom: spmi-gpio: convert to hierarchical IRQ helpers in gpio core Linus Walleij
@ 2019-08-08 12:32 ` Linus Walleij
2019-08-14 8:21 ` Linus Walleij
2019-08-08 12:32 ` [PATCH 5/6 v2] RFT: gpio: uniphier: " Linus Walleij
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Linus Walleij @ 2019-08-08 12:32 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, David Daney, Thierry Reding,
Brian Masney
Use the new infrastructure for hierarchical irqchips in
gpiolib.
The major part of the rewrite was dues to the fact that
the driver was passing around a per-irq pointer to
struct thunderx_line * data container, and the central
handlers will assume struct gpio_chip * to be passed
to we need to use the hwirq as index to look up the
struct thunderx_line * for each IRQ.
The pushing and pop:ing of the irqdomain was confusing
because I've never seen this before, but I tried to
replicate it as best I could.
I have no chance to test or debug this so I need
help.
Cc: David Daney <david.daney@cavium.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Brian Masney <masneyb@onstation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog RFT v1-> RFT v2:
- Rebased.
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-thunderx.c | 163 ++++++++++++-----------------------
2 files changed, 57 insertions(+), 107 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b34e9b11a7ef..3125aca2db9f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -539,6 +539,7 @@ config GPIO_THUNDERX
tristate "Cavium ThunderX/OCTEON-TX GPIO"
depends on ARCH_THUNDER || (64BIT && COMPILE_TEST)
depends on PCI_MSI
+ select GPIOLIB_IRQCHIP
select IRQ_DOMAIN_HIERARCHY
select IRQ_FASTEOI_HIERARCHY_HANDLERS
help
diff --git a/drivers/gpio/gpio-thunderx.c b/drivers/gpio/gpio-thunderx.c
index 715371b5102a..ddad5c7ea617 100644
--- a/drivers/gpio/gpio-thunderx.c
+++ b/drivers/gpio/gpio-thunderx.c
@@ -53,7 +53,6 @@ struct thunderx_line {
struct thunderx_gpio {
struct gpio_chip chip;
u8 __iomem *register_base;
- struct irq_domain *irqd;
struct msix_entry *msix_entries; /* per line MSI-X */
struct thunderx_line *line_entries; /* per line irq info */
raw_spinlock_t lock;
@@ -283,54 +282,60 @@ static void thunderx_gpio_set_multiple(struct gpio_chip *chip,
}
}
-static void thunderx_gpio_irq_ack(struct irq_data *data)
+static void thunderx_gpio_irq_ack(struct irq_data *d)
{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
writeq(GPIO_INTR_INTR,
- txline->txgpio->register_base + intr_reg(txline->line));
+ txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
}
-static void thunderx_gpio_irq_mask(struct irq_data *data)
+static void thunderx_gpio_irq_mask(struct irq_data *d)
{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
writeq(GPIO_INTR_ENA_W1C,
- txline->txgpio->register_base + intr_reg(txline->line));
+ txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
}
-static void thunderx_gpio_irq_mask_ack(struct irq_data *data)
+static void thunderx_gpio_irq_mask_ack(struct irq_data *d)
{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
writeq(GPIO_INTR_ENA_W1C | GPIO_INTR_INTR,
- txline->txgpio->register_base + intr_reg(txline->line));
+ txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
}
-static void thunderx_gpio_irq_unmask(struct irq_data *data)
+static void thunderx_gpio_irq_unmask(struct irq_data *d)
{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
writeq(GPIO_INTR_ENA_W1S,
- txline->txgpio->register_base + intr_reg(txline->line));
+ txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
}
-static int thunderx_gpio_irq_set_type(struct irq_data *data,
+static int thunderx_gpio_irq_set_type(struct irq_data *d,
unsigned int flow_type)
{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
- struct thunderx_gpio *txgpio = txline->txgpio;
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
+ struct thunderx_line *txline =
+ &txgpio->line_entries[irqd_to_hwirq(d)];
u64 bit_cfg;
- irqd_set_trigger_type(data, flow_type);
+ irqd_set_trigger_type(d, flow_type);
bit_cfg = txline->fil_bits | GPIO_BIT_CFG_INT_EN;
if (flow_type & IRQ_TYPE_EDGE_BOTH) {
- irq_set_handler_locked(data, handle_fasteoi_ack_irq);
+ irq_set_handler_locked(d, handle_fasteoi_ack_irq);
bit_cfg |= GPIO_BIT_CFG_INT_TYPE;
} else {
- irq_set_handler_locked(data, handle_fasteoi_mask_irq);
+ irq_set_handler_locked(d, handle_fasteoi_mask_irq);
}
raw_spin_lock(&txgpio->lock);
@@ -359,33 +364,6 @@ static void thunderx_gpio_irq_disable(struct irq_data *data)
irq_chip_disable_parent(data);
}
-static int thunderx_gpio_irq_request_resources(struct irq_data *data)
-{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
- struct thunderx_gpio *txgpio = txline->txgpio;
- int r;
-
- r = gpiochip_lock_as_irq(&txgpio->chip, txline->line);
- if (r)
- return r;
-
- r = irq_chip_request_resources_parent(data);
- if (r)
- gpiochip_unlock_as_irq(&txgpio->chip, txline->line);
-
- return r;
-}
-
-static void thunderx_gpio_irq_release_resources(struct irq_data *data)
-{
- struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
- struct thunderx_gpio *txgpio = txline->txgpio;
-
- irq_chip_release_resources_parent(data);
-
- gpiochip_unlock_as_irq(&txgpio->chip, txline->line);
-}
-
/*
* Interrupts are chained from underlying MSI-X vectors. We have
* these irq_chip functions to be able to handle level triggering
@@ -402,48 +380,22 @@ static struct irq_chip thunderx_gpio_irq_chip = {
.irq_unmask = thunderx_gpio_irq_unmask,
.irq_eoi = irq_chip_eoi_parent,
.irq_set_affinity = irq_chip_set_affinity_parent,
- .irq_request_resources = thunderx_gpio_irq_request_resources,
- .irq_release_resources = thunderx_gpio_irq_release_resources,
.irq_set_type = thunderx_gpio_irq_set_type,
.flags = IRQCHIP_SET_TYPE_MASKED
};
-static int thunderx_gpio_irq_translate(struct irq_domain *d,
- struct irq_fwspec *fwspec,
- irq_hw_number_t *hwirq,
- unsigned int *type)
+static int thunderx_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
+ unsigned int child,
+ unsigned int child_type,
+ unsigned int *parent,
+ unsigned int *parent_type)
{
- struct thunderx_gpio *txgpio = d->host_data;
-
- if (WARN_ON(fwspec->param_count < 2))
- return -EINVAL;
- if (fwspec->param[0] >= txgpio->chip.ngpio)
- return -EINVAL;
- *hwirq = fwspec->param[0];
- *type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
- return 0;
-}
-
-static int thunderx_gpio_irq_alloc(struct irq_domain *d, unsigned int virq,
- unsigned int nr_irqs, void *arg)
-{
- struct thunderx_line *txline = arg;
+ struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
- return irq_domain_set_hwirq_and_chip(d, virq, txline->line,
- &thunderx_gpio_irq_chip, txline);
-}
-
-static const struct irq_domain_ops thunderx_gpio_irqd_ops = {
- .alloc = thunderx_gpio_irq_alloc,
- .translate = thunderx_gpio_irq_translate
-};
-
-static int thunderx_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
-{
- struct thunderx_gpio *txgpio = gpiochip_get_data(chip);
-
- return irq_find_mapping(txgpio->irqd, offset);
+ *parent = txgpio->base_msi + (2 * child);
+ *parent_type = IRQ_TYPE_LEVEL_HIGH;
+ return 0;
}
static int thunderx_gpio_probe(struct pci_dev *pdev,
@@ -453,6 +405,7 @@ static int thunderx_gpio_probe(struct pci_dev *pdev,
struct device *dev = &pdev->dev;
struct thunderx_gpio *txgpio;
struct gpio_chip *chip;
+ struct gpio_irq_chip *girq;
int ngpio, i;
int err = 0;
@@ -497,8 +450,8 @@ static int thunderx_gpio_probe(struct pci_dev *pdev,
}
txgpio->msix_entries = devm_kcalloc(dev,
- ngpio, sizeof(struct msix_entry),
- GFP_KERNEL);
+ ngpio, sizeof(struct msix_entry),
+ GFP_KERNEL);
if (!txgpio->msix_entries) {
err = -ENOMEM;
goto out;
@@ -539,27 +492,6 @@ static int thunderx_gpio_probe(struct pci_dev *pdev,
if (err < 0)
goto out;
- /*
- * Push GPIO specific irqdomain on hierarchy created as a side
- * effect of the pci_enable_msix()
- */
- txgpio->irqd = irq_domain_create_hierarchy(irq_get_irq_data(txgpio->msix_entries[0].vector)->domain,
- 0, 0, of_node_to_fwnode(dev->of_node),
- &thunderx_gpio_irqd_ops, txgpio);
- if (!txgpio->irqd) {
- err = -ENOMEM;
- goto out;
- }
-
- /* Push on irq_data and the domain for each line. */
- for (i = 0; i < ngpio; i++) {
- err = irq_domain_push_irq(txgpio->irqd,
- txgpio->msix_entries[i].vector,
- &txgpio->line_entries[i]);
- if (err < 0)
- dev_err(dev, "irq_domain_push_irq: %d\n", err);
- }
-
chip->label = KBUILD_MODNAME;
chip->parent = dev;
chip->owner = THIS_MODULE;
@@ -574,11 +506,28 @@ static int thunderx_gpio_probe(struct pci_dev *pdev,
chip->set = thunderx_gpio_set;
chip->set_multiple = thunderx_gpio_set_multiple;
chip->set_config = thunderx_gpio_set_config;
- chip->to_irq = thunderx_gpio_to_irq;
+ girq = &chip->irq;
+ girq->chip = &thunderx_gpio_irq_chip;
+ girq->fwnode = of_node_to_fwnode(dev->of_node);
+ girq->parent_domain =
+ irq_get_irq_data(txgpio->msix_entries[0].vector)->domain;
+ girq->child_to_parent_hwirq = thunderx_gpio_child_to_parent_hwirq;
+ girq->handler = handle_bad_irq;
+ girq->default_type = IRQ_TYPE_NONE;
+
err = devm_gpiochip_add_data(dev, chip, txgpio);
if (err)
goto out;
+ /* Push on irq_data and the domain for each line. */
+ for (i = 0; i < ngpio; i++) {
+ err = irq_domain_push_irq(chip->irq.domain,
+ txgpio->msix_entries[i].vector,
+ chip);
+ if (err < 0)
+ dev_err(dev, "irq_domain_push_irq: %d\n", err);
+ }
+
dev_info(dev, "ThunderX GPIO: %d lines with base %d.\n",
ngpio, chip->base);
return 0;
@@ -593,10 +542,10 @@ static void thunderx_gpio_remove(struct pci_dev *pdev)
struct thunderx_gpio *txgpio = pci_get_drvdata(pdev);
for (i = 0; i < txgpio->chip.ngpio; i++)
- irq_domain_pop_irq(txgpio->irqd,
+ irq_domain_pop_irq(txgpio->chip.irq.domain,
txgpio->msix_entries[i].vector);
- irq_domain_remove(txgpio->irqd);
+ irq_domain_remove(txgpio->chip.irq.domain);
pci_set_drvdata(pdev, NULL);
}
--
2.21.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6 v2] RFT: gpio: thunderx: Switch to GPIOLIB_IRQCHIP
2019-08-08 12:32 ` [PATCH 4/6 v2] RFT: gpio: thunderx: Switch to GPIOLIB_IRQCHIP Linus Walleij
@ 2019-08-14 8:21 ` Linus Walleij
0 siblings, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2019-08-14 8:21 UTC (permalink / raw)
To: open list:GPIO SUBSYSTEM
Cc: Bartosz Golaszewski, David Daney, Thierry Reding, Brian Masney
On Thu, Aug 8, 2019 at 2:32 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> Use the new infrastructure for hierarchical irqchips in
> gpiolib.
>
> The major part of the rewrite was dues to the fact that
> the driver was passing around a per-irq pointer to
> struct thunderx_line * data container, and the central
> handlers will assume struct gpio_chip * to be passed
> to we need to use the hwirq as index to look up the
> struct thunderx_line * for each IRQ.
>
> The pushing and pop:ing of the irqdomain was confusing
> because I've never seen this before, but I tried to
> replicate it as best I could.
>
> I have no chance to test or debug this so I need
> help.
>
> Cc: David Daney <david.daney@cavium.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Brian Masney <masneyb@onstation.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog RFT v1-> RFT v2:
> - Rebased.
As noone seems interested in reviewing or testing ThunderX
I have applied the patch, as I think there are ThunderX machines
in the test farms, things will crop out now.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/6 v2] RFT: gpio: uniphier: Switch to GPIOLIB_IRQCHIP
2019-08-08 12:32 [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains Linus Walleij
` (2 preceding siblings ...)
2019-08-08 12:32 ` [PATCH 4/6 v2] RFT: gpio: thunderx: Switch to GPIOLIB_IRQCHIP Linus Walleij
@ 2019-08-08 12:32 ` Linus Walleij
2019-08-08 12:32 ` [PATCH 6/6 v2] RFT: gpio: uniphier: Restrict valid interrupts Linus Walleij
` (2 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2019-08-08 12:32 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Masahiro Yamada,
Thierry Reding, Brian Masney
Use the new infrastructure for hierarchical irqchips in
gpiolib.
I have no chance to test or debug this so I need
help.
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Brian Masney <masneyb@onstation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog RFT v1 -> RFT v2:
- Drop noisy change of "data" parameter to "d"
- Use ->child_offset_to_irq() callback to account for the
UNIPHIER_GPIO_IRQ_OFFSET.
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-uniphier.c | 161 +++++++++--------------------------
2 files changed, 43 insertions(+), 119 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 3125aca2db9f..cc07bbe4864e 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -550,6 +550,7 @@ config GPIO_UNIPHIER
tristate "UniPhier GPIO support"
depends on ARCH_UNIPHIER || COMPILE_TEST
depends on OF_GPIO
+ select GPIOLIB_IRQCHIP
select IRQ_DOMAIN_HIERARCHY
help
Say yes here to support UniPhier GPIOs.
diff --git a/drivers/gpio/gpio-uniphier.c b/drivers/gpio/gpio-uniphier.c
index 93cdcc41e9fb..86e8446215fc 100644
--- a/drivers/gpio/gpio-uniphier.c
+++ b/drivers/gpio/gpio-uniphier.c
@@ -6,7 +6,6 @@
#include <linux/bits.h>
#include <linux/gpio/driver.h>
#include <linux/irq.h>
-#include <linux/irqdomain.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
@@ -30,7 +29,6 @@
struct uniphier_gpio_priv {
struct gpio_chip chip;
struct irq_chip irq_chip;
- struct irq_domain *domain;
void __iomem *regs;
spinlock_t lock;
u32 saved_vals[0];
@@ -162,28 +160,10 @@ static void uniphier_gpio_set_multiple(struct gpio_chip *chip,
}
}
-static int uniphier_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
-{
- struct irq_fwspec fwspec;
-
- if (offset < UNIPHIER_GPIO_IRQ_OFFSET)
- return -ENXIO;
-
- fwspec.fwnode = of_node_to_fwnode(chip->parent->of_node);
- fwspec.param_count = 2;
- fwspec.param[0] = offset - UNIPHIER_GPIO_IRQ_OFFSET;
- /*
- * IRQ_TYPE_NONE is rejected by the parent irq domain. Set LEVEL_HIGH
- * temporarily. Anyway, ->irq_set_type() will override it later.
- */
- fwspec.param[1] = IRQ_TYPE_LEVEL_HIGH;
-
- return irq_create_fwspec_mapping(&fwspec);
-}
-
static void uniphier_gpio_irq_mask(struct irq_data *data)
{
- struct uniphier_gpio_priv *priv = data->chip_data;
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+ struct uniphier_gpio_priv *priv = gpiochip_get_data(gc);
u32 mask = BIT(data->hwirq);
uniphier_gpio_reg_update(priv, UNIPHIER_GPIO_IRQ_EN, mask, 0);
@@ -193,7 +173,8 @@ static void uniphier_gpio_irq_mask(struct irq_data *data)
static void uniphier_gpio_irq_unmask(struct irq_data *data)
{
- struct uniphier_gpio_priv *priv = data->chip_data;
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+ struct uniphier_gpio_priv *priv = gpiochip_get_data(gc);
u32 mask = BIT(data->hwirq);
uniphier_gpio_reg_update(priv, UNIPHIER_GPIO_IRQ_EN, mask, mask);
@@ -203,7 +184,8 @@ static void uniphier_gpio_irq_unmask(struct irq_data *data)
static int uniphier_gpio_irq_set_type(struct irq_data *data, unsigned int type)
{
- struct uniphier_gpio_priv *priv = data->chip_data;
+ struct gpio_chip *gc = irq_data_get_irq_chip_data(data);
+ struct uniphier_gpio_priv *priv = gpiochip_get_data(gc);
u32 mask = BIT(data->hwirq);
u32 val = 0;
@@ -245,82 +227,6 @@ static int uniphier_gpio_irq_get_parent_hwirq(struct uniphier_gpio_priv *priv,
return -ENOENT;
}
-static int uniphier_gpio_irq_domain_translate(struct irq_domain *domain,
- struct irq_fwspec *fwspec,
- unsigned long *out_hwirq,
- unsigned int *out_type)
-{
- if (WARN_ON(fwspec->param_count < 2))
- return -EINVAL;
-
- *out_hwirq = fwspec->param[0];
- *out_type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
-
- return 0;
-}
-
-static int uniphier_gpio_irq_domain_alloc(struct irq_domain *domain,
- unsigned int virq,
- unsigned int nr_irqs, void *arg)
-{
- struct uniphier_gpio_priv *priv = domain->host_data;
- struct irq_fwspec parent_fwspec;
- irq_hw_number_t hwirq;
- unsigned int type;
- int ret;
-
- if (WARN_ON(nr_irqs != 1))
- return -EINVAL;
-
- ret = uniphier_gpio_irq_domain_translate(domain, arg, &hwirq, &type);
- if (ret)
- return ret;
-
- ret = uniphier_gpio_irq_get_parent_hwirq(priv, hwirq);
- if (ret < 0)
- return ret;
-
- /* parent is UniPhier AIDET */
- parent_fwspec.fwnode = domain->parent->fwnode;
- parent_fwspec.param_count = 2;
- parent_fwspec.param[0] = ret;
- parent_fwspec.param[1] = (type == IRQ_TYPE_EDGE_BOTH) ?
- IRQ_TYPE_EDGE_FALLING : type;
-
- ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
- &priv->irq_chip, priv);
- if (ret)
- return ret;
-
- return irq_domain_alloc_irqs_parent(domain, virq, 1, &parent_fwspec);
-}
-
-static int uniphier_gpio_irq_domain_activate(struct irq_domain *domain,
- struct irq_data *data, bool early)
-{
- struct uniphier_gpio_priv *priv = domain->host_data;
- struct gpio_chip *chip = &priv->chip;
-
- return gpiochip_lock_as_irq(chip, data->hwirq + UNIPHIER_GPIO_IRQ_OFFSET);
-}
-
-static void uniphier_gpio_irq_domain_deactivate(struct irq_domain *domain,
- struct irq_data *data)
-{
- struct uniphier_gpio_priv *priv = domain->host_data;
- struct gpio_chip *chip = &priv->chip;
-
- gpiochip_unlock_as_irq(chip, data->hwirq + UNIPHIER_GPIO_IRQ_OFFSET);
-}
-
-static const struct irq_domain_ops uniphier_gpio_irq_domain_ops = {
- .alloc = uniphier_gpio_irq_domain_alloc,
- .free = irq_domain_free_irqs_common,
- .activate = uniphier_gpio_irq_domain_activate,
- .deactivate = uniphier_gpio_irq_domain_deactivate,
- .translate = uniphier_gpio_irq_domain_translate,
-};
-
static void uniphier_gpio_hw_init(struct uniphier_gpio_priv *priv)
{
/*
@@ -338,6 +244,32 @@ static unsigned int uniphier_gpio_get_nbanks(unsigned int ngpio)
return DIV_ROUND_UP(ngpio, UNIPHIER_GPIO_LINES_PER_BANK);
}
+static unsigned int uniphier_gpio_child_offset_to_irq(struct gpio_chip *gc,
+ unsigned int offset)
+{
+ return offset + UNIPHIER_GPIO_IRQ_OFFSET;
+}
+
+static int uniphier_gpio_child_to_parent_hwirq(struct gpio_chip *gc,
+ unsigned int child,
+ unsigned int child_type,
+ unsigned int *parent,
+ unsigned int *parent_type)
+{
+ struct uniphier_gpio_priv *priv = gpiochip_get_data(gc);
+ int ret;
+
+ ret = uniphier_gpio_irq_get_parent_hwirq(priv, child);
+ if (ret < 0)
+ return ret;
+ *parent = ret;
+ if (child_type == IRQ_TYPE_EDGE_BOTH)
+ *parent_type = IRQ_TYPE_EDGE_FALLING;
+ else
+ *parent_type = child_type;
+ return 0;
+}
+
static int uniphier_gpio_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -345,6 +277,7 @@ static int uniphier_gpio_probe(struct platform_device *pdev)
struct irq_domain *parent_domain;
struct uniphier_gpio_priv *priv;
struct gpio_chip *chip;
+ struct gpio_irq_chip *girq;
struct irq_chip *irq_chip;
unsigned int nregs;
u32 ngpios;
@@ -386,7 +319,6 @@ static int uniphier_gpio_probe(struct platform_device *pdev)
chip->get = uniphier_gpio_get;
chip->set = uniphier_gpio_set;
chip->set_multiple = uniphier_gpio_set_multiple;
- chip->to_irq = uniphier_gpio_to_irq;
chip->base = -1;
chip->ngpio = ngpios;
@@ -398,34 +330,26 @@ static int uniphier_gpio_probe(struct platform_device *pdev)
irq_chip->irq_set_affinity = irq_chip_set_affinity_parent;
irq_chip->irq_set_type = uniphier_gpio_irq_set_type;
+ girq = &chip->irq;
+ girq->chip = irq_chip;
+ girq->fwnode = of_node_to_fwnode(dev->of_node);
+ girq->parent_domain = parent_domain;
+ girq->child_offset_to_irq = uniphier_gpio_child_offset_to_irq;
+ girq->child_to_parent_hwirq = uniphier_gpio_child_to_parent_hwirq;
+ girq->handler = handle_bad_irq;
+ girq->default_type = IRQ_TYPE_NONE;
+
uniphier_gpio_hw_init(priv);
ret = devm_gpiochip_add_data(dev, chip, priv);
if (ret)
return ret;
- priv->domain = irq_domain_create_hierarchy(
- parent_domain, 0,
- UNIPHIER_GPIO_IRQ_MAX_NUM,
- of_node_to_fwnode(dev->of_node),
- &uniphier_gpio_irq_domain_ops, priv);
- if (!priv->domain)
- return -ENOMEM;
-
platform_set_drvdata(pdev, priv);
return 0;
}
-static int uniphier_gpio_remove(struct platform_device *pdev)
-{
- struct uniphier_gpio_priv *priv = platform_get_drvdata(pdev);
-
- irq_domain_remove(priv->domain);
-
- return 0;
-}
-
static int __maybe_unused uniphier_gpio_suspend(struct device *dev)
{
struct uniphier_gpio_priv *priv = dev_get_drvdata(dev);
@@ -485,7 +409,6 @@ MODULE_DEVICE_TABLE(of, uniphier_gpio_match);
static struct platform_driver uniphier_gpio_driver = {
.probe = uniphier_gpio_probe,
- .remove = uniphier_gpio_remove,
.driver = {
.name = "uniphier-gpio",
.of_match_table = uniphier_gpio_match,
--
2.21.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 6/6 v2] RFT: gpio: uniphier: Restrict valid interrupts
2019-08-08 12:32 [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains Linus Walleij
` (3 preceding siblings ...)
2019-08-08 12:32 ` [PATCH 5/6 v2] RFT: gpio: uniphier: " Linus Walleij
@ 2019-08-08 12:32 ` Linus Walleij
2019-08-14 8:18 ` [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains Linus Walleij
2019-08-16 1:10 ` Brian Masney
6 siblings, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2019-08-08 12:32 UTC (permalink / raw)
To: linux-gpio
Cc: Bartosz Golaszewski, Linus Walleij, Masahiro Yamada,
Thierry Reding, Brian Masney
The Uniphier GPIO can only create interrupt mappings on
GPIOs offset 0..23. Set the valid_mask in the struct
gpio_irqchip and clear bits 24..ngpios indicating that
these can not be mapped to interrupts.
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Brian Masney <masneyb@onstation.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog RFT -> RTF v2:
- New patch to set the valid mask for the interrupts.
---
drivers/gpio/gpio-uniphier.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpio/gpio-uniphier.c b/drivers/gpio/gpio-uniphier.c
index 86e8446215fc..9840fcf7de57 100644
--- a/drivers/gpio/gpio-uniphier.c
+++ b/drivers/gpio/gpio-uniphier.c
@@ -338,6 +338,7 @@ static int uniphier_gpio_probe(struct platform_device *pdev)
girq->child_to_parent_hwirq = uniphier_gpio_child_to_parent_hwirq;
girq->handler = handle_bad_irq;
girq->default_type = IRQ_TYPE_NONE;
+ girq->need_valid_mask = true;
uniphier_gpio_hw_init(priv);
@@ -345,6 +346,10 @@ static int uniphier_gpio_probe(struct platform_device *pdev)
if (ret)
return ret;
+ /* Only GPIOs 0..UNIPHIER_GPIO_IRQ_MAX_NUM are valid for interrupts */
+ bitmap_clear(girq->valid_mask, UNIPHIER_GPIO_IRQ_MAX_NUM,
+ ngpios - UNIPHIER_GPIO_IRQ_MAX_NUM);
+
platform_set_drvdata(pdev, priv);
return 0;
--
2.21.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains
2019-08-08 12:32 [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains Linus Walleij
` (4 preceding siblings ...)
2019-08-08 12:32 ` [PATCH 6/6 v2] RFT: gpio: uniphier: Restrict valid interrupts Linus Walleij
@ 2019-08-14 8:18 ` Linus Walleij
2019-08-16 1:10 ` Brian Masney
6 siblings, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2019-08-14 8:18 UTC (permalink / raw)
To: open list:GPIO SUBSYSTEM
Cc: Bartosz Golaszewski, Thomas Gleixner, Marc Zyngier, Lina Iyer,
Jon Hunter, Sowjanya Komatineni, Bitan Biswas, linux-tegra,
David Daney, Masahiro Yamada, Brian Masney, Thierry Reding
On Thu, Aug 8, 2019 at 2:32 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> Hierarchical IRQ domains can be used to stack different IRQ
> controllers on top of each other.
This is now applied so we get a solid base for testing in linux-next.
Address further concerns with incremental patches, either written
by yourselves or by poking me to do them :)
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains
2019-08-08 12:32 [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains Linus Walleij
` (5 preceding siblings ...)
2019-08-14 8:18 ` [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains Linus Walleij
@ 2019-08-16 1:10 ` Brian Masney
2019-08-23 8:24 ` Linus Walleij
6 siblings, 1 reply; 17+ messages in thread
From: Brian Masney @ 2019-08-16 1:10 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-gpio, Bartosz Golaszewski, Thomas Gleixner, Marc Zyngier,
Lina Iyer, Jon Hunter, Sowjanya Komatineni, Bitan Biswas,
linux-tegra, David Daney, Masahiro Yamada, Thierry Reding
Hi Linus,
On Thu, Aug 08, 2019 at 02:32:37PM +0200, Linus Walleij wrote:
> Hierarchical IRQ domains can be used to stack different IRQ
> controllers on top of each other.
>
> Bring hierarchical IRQ domains into the GPIOLIB core with the
> following basic idea:
>
> Drivers that need their interrupts handled hierarchically
> specify a callback to translate the child hardware IRQ and
> IRQ type for each GPIO offset to a parent hardware IRQ and
> parent hardware IRQ type.
>
> Users have to pass the callback, fwnode, and parent irqdomain
> before calling gpiochip_irqchip_add().
>
> We use the new method of just filling in the struct
> gpio_irq_chip before adding the gpiochip for all hierarchical
> irqchips of this type.
>
> The code path for device tree is pretty straight-forward,
> while the code path for old boardfiles or anything else will
> be more convoluted requireing upfront allocation of the
> interrupts when adding the chip.
>
> One specific use-case where this can be useful is if a power
> management controller has top-level controls for wakeup
> interrupts. In such cases, the power management controller can
> be a parent to other interrupt controllers and program
> additional registers when an IRQ has its wake capability
> enabled or disabled.
>
> The hierarchical irqchip helper code will only be available
> when IRQ_DOMAIN_HIERARCHY is selected to GPIO chips using
> this should select or depend on that symbol. When using
> hierarchical IRQs, the parent interrupt controller must
> also be hierarchical all the way up to the top interrupt
> controller wireing directly into the CPU, so on systems
> that do not have this we can get rid of all the extra
> code for supporting hierarchical irqs.
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Lina Iyer <ilina@codeaurora.org>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Sowjanya Komatineni <skomatineni@nvidia.com>
> Cc: Bitan Biswas <bbiswas@nvidia.com>
> Cc: linux-tegra@vger.kernel.org
> Cc: David Daney <david.daney@cavium.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Brian Masney <masneyb@onstation.org>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> Co-developed-by: Brian Masney <masneyb@onstation.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
[ snip ]
> @@ -1827,10 +2099,23 @@ EXPORT_SYMBOL_GPL(gpiochip_irq_domain_deactivate);
>
> static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
^^^^^^
I started to convert ssbi-gpio over to this and pm8xxx_gpio_to_irq() has
this little snippet that's different from spmi-gpio:
[ fwspec mapping code ]
/*
* Cache the IRQ since pm8xxx_gpio_get() needs this to get determine the
* line level.
*/
pin->irq = ret;
Here's the relevant code in pm8xxx_gpio_get():
if (pin->mode == PM8XXX_GPIO_MODE_OUTPUT) {
ret = pin->output_value;
} else if (pin->irq >= 0) {
ret = irq_get_irqchip_state(pin->irq, IRQCHIP_STATE_LINE_LEVEL, &state);
...
}
What do you think about using EXPORT_SYMBOL_GPL() for gpiochip_to_irq() so
that we can call it in pm8xxx_gpio_to_irq()? Or do you have any other
suggestions for how we can get rid of that IRQ cache?
I don't see any other issues for ssbi-gpio.
Brian
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains
2019-08-16 1:10 ` Brian Masney
@ 2019-08-23 8:24 ` Linus Walleij
2019-08-23 9:12 ` Marc Zyngier
0 siblings, 1 reply; 17+ messages in thread
From: Linus Walleij @ 2019-08-23 8:24 UTC (permalink / raw)
To: Brian Masney, Marc Zyngier
Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Thomas Gleixner,
Lina Iyer, Jon Hunter, Sowjanya Komatineni, Bitan Biswas,
linux-tegra, David Daney, Masahiro Yamada, Thierry Reding
Hi Brian,
I tried to look into this ssbi-gpio problem...
Paging in Marc Z as well.
On Fri, Aug 16, 2019 at 3:10 AM Brian Masney <masneyb@onstation.org> wrote:
> I started to convert ssbi-gpio over to this and pm8xxx_gpio_to_irq() has
> this little snippet that's different from spmi-gpio:
>
> [ fwspec mapping code ]
>
> /*
> * Cache the IRQ since pm8xxx_gpio_get() needs this to get determine the
> * line level.
> */
> pin->irq = ret;
>
> Here's the relevant code in pm8xxx_gpio_get():
>
> if (pin->mode == PM8XXX_GPIO_MODE_OUTPUT) {
> ret = pin->output_value;
> } else if (pin->irq >= 0) {
> ret = irq_get_irqchip_state(pin->irq, IRQCHIP_STATE_LINE_LEVEL, &state);
> ...
> }
It's a bit annoying that this API (irq_get_irqchip_state()) is relying on
the global irq numbers and the internal function using irqdesc
__irq_get_irqchip_state() is not exported.
We should be encouraged to operate on IRQ descriptors rather
than IRQ numbers when doing this kind of deep core work,
right?
> What do you think about using EXPORT_SYMBOL_GPL() for gpiochip_to_irq() so
> that we can call it in pm8xxx_gpio_to_irq()?
I would like to avoid that because people tend to abuse every
API I expose (leasson learnt: APIs shall be narrow and deep).
> Or do you have any other
> suggestions for how we can get rid of that IRQ cache?
So the issue is that moving this to the hierarchical helpers
rids pm8xxx_gpio_to_irq() and we needed that to map this.
I would rather just add a new driver API to wrap the irqchip
API:
ret = gpiochip_get_irq_state(offset, IRQCHIP_STATE_LINE_LEVEL, &state);
Where int gpiochip_get_own_irq_state(int offset, ...) will figure
out the gpio_desc of the offset and then call gpiod_to_irq()
and then use that irq number to call irq_get_irqchip_state()
and the goal is met.
This should work I think, and expose a more precise
API for what this driver wants.
> I don't see any other issues for ssbi-gpio.
That's good news!
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains
2019-08-23 8:24 ` Linus Walleij
@ 2019-08-23 9:12 ` Marc Zyngier
2019-08-23 10:13 ` Linus Walleij
2019-08-26 16:15 ` Lina Iyer
0 siblings, 2 replies; 17+ messages in thread
From: Marc Zyngier @ 2019-08-23 9:12 UTC (permalink / raw)
To: Linus Walleij, Brian Masney
Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, Thomas Gleixner,
Lina Iyer, Jon Hunter, Sowjanya Komatineni, Bitan Biswas,
linux-tegra, David Daney, Masahiro Yamada, Thierry Reding
On 23/08/2019 09:24, Linus Walleij wrote:
> Hi Brian,
>
> I tried to look into this ssbi-gpio problem...
>
> Paging in Marc Z as well.
>
> On Fri, Aug 16, 2019 at 3:10 AM Brian Masney <masneyb@onstation.org> wrote:
>
>> I started to convert ssbi-gpio over to this and pm8xxx_gpio_to_irq() has
>> this little snippet that's different from spmi-gpio:
>>
>> [ fwspec mapping code ]
>>
>> /*
>> * Cache the IRQ since pm8xxx_gpio_get() needs this to get determine the
>> * line level.
>> */
>> pin->irq = ret;
>>
>> Here's the relevant code in pm8xxx_gpio_get():
>>
>> if (pin->mode == PM8XXX_GPIO_MODE_OUTPUT) {
>> ret = pin->output_value;
>> } else if (pin->irq >= 0) {
>> ret = irq_get_irqchip_state(pin->irq, IRQCHIP_STATE_LINE_LEVEL, &state);
>> ...
>> }
>
> It's a bit annoying that this API (irq_get_irqchip_state()) is relying on
> the global irq numbers and the internal function using irqdesc
> __irq_get_irqchip_state() is not exported.
>
> We should be encouraged to operate on IRQ descriptors rather
> than IRQ numbers when doing this kind of deep core work,
> right?
Certainly, I'd like to gradually move over from the IRQ number to an
irq_desc. In interrupt-heavy contexts, this ends up being quite time
consuming. I have an old patch series somewhere changing irq domains to
use irq_descs internally instead of IRQ numbers, which I should maybe
revive.
As for __irq_get_irqchip_state, the main issue is that it doesn't
perform any locking on its own, so you'd have to either provide your
own, or wrap it with something else.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains
2019-08-23 9:12 ` Marc Zyngier
@ 2019-08-23 10:13 ` Linus Walleij
2019-08-26 16:15 ` Lina Iyer
1 sibling, 0 replies; 17+ messages in thread
From: Linus Walleij @ 2019-08-23 10:13 UTC (permalink / raw)
To: Marc Zyngier
Cc: Brian Masney, open list:GPIO SUBSYSTEM, Bartosz Golaszewski,
Thomas Gleixner, Lina Iyer, Jon Hunter, Sowjanya Komatineni,
Bitan Biswas, linux-tegra, David Daney, Masahiro Yamada,
Thierry Reding
On Fri, Aug 23, 2019 at 11:12 AM Marc Zyngier <marc.zyngier@arm.com> wrote:
> > We should be encouraged to operate on IRQ descriptors rather
> > than IRQ numbers when doing this kind of deep core work,
> > right?
>
> Certainly, I'd like to gradually move over from the IRQ number to an
> irq_desc. In interrupt-heavy contexts, this ends up being quite time
> consuming. I have an old patch series somewhere changing irq domains to
> use irq_descs internally instead of IRQ numbers, which I should maybe
> revive.
We currently interact most heavily with the irqchip by way of
mapping GPIO lines to the corresponding IRQs, so I suppose
the existing
int gpiod_to_irq(struct gpio_desc *desc);
Need a corresponding
struct irq_desc *gpiod_to_irq_desc(struct gpio_desc *desc);
at some point, and then we can start to de-pollute the kernel
from this for all drivers using the modern GPIO descriptor
API. It's a big task but can certainly be done with some
help.
On the driver back-end we are in surprisingly good shape
thanks to all the abstractions provided for dealing with IRQ
chip drivers.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains
2019-08-23 9:12 ` Marc Zyngier
2019-08-23 10:13 ` Linus Walleij
@ 2019-08-26 16:15 ` Lina Iyer
2019-08-26 18:44 ` Marc Zyngier
1 sibling, 1 reply; 17+ messages in thread
From: Lina Iyer @ 2019-08-26 16:15 UTC (permalink / raw)
To: Marc Zyngier
Cc: Linus Walleij, Brian Masney, open list:GPIO SUBSYSTEM,
Bartosz Golaszewski, Thomas Gleixner, Jon Hunter,
Sowjanya Komatineni, Bitan Biswas, linux-tegra, David Daney,
Masahiro Yamada, Thierry Reding
On Fri, Aug 23 2019 at 03:12 -0600, Marc Zyngier wrote:
>On 23/08/2019 09:24, Linus Walleij wrote:
>> Hi Brian,
>>
>> I tried to look into this ssbi-gpio problem...
>>
>> Paging in Marc Z as well.
>>
>> On Fri, Aug 16, 2019 at 3:10 AM Brian Masney <masneyb@onstation.org> wrote:
>>
>>> I started to convert ssbi-gpio over to this and pm8xxx_gpio_to_irq() has
>>> this little snippet that's different from spmi-gpio:
>>>
>>> [ fwspec mapping code ]
>>>
>>> /*
>>> * Cache the IRQ since pm8xxx_gpio_get() needs this to get determine the
>>> * line level.
>>> */
>>> pin->irq = ret;
>>>
>>> Here's the relevant code in pm8xxx_gpio_get():
>>>
>>> if (pin->mode == PM8XXX_GPIO_MODE_OUTPUT) {
>>> ret = pin->output_value;
>>> } else if (pin->irq >= 0) {
>>> ret = irq_get_irqchip_state(pin->irq, IRQCHIP_STATE_LINE_LEVEL, &state);
>>> ...
>>> }
>>
>> It's a bit annoying that this API (irq_get_irqchip_state()) is relying on
>> the global irq numbers and the internal function using irqdesc
>> __irq_get_irqchip_state() is not exported.
>>
>> We should be encouraged to operate on IRQ descriptors rather
>> than IRQ numbers when doing this kind of deep core work,
>> right?
>
>Certainly, I'd like to gradually move over from the IRQ number to an
>irq_desc. In interrupt-heavy contexts, this ends up being quite time
>consuming. I have an old patch series somewhere changing irq domains to
>use irq_descs internally instead of IRQ numbers, which I should maybe
>revive.
>
>As for __irq_get_irqchip_state, the main issue is that it doesn't
>perform any locking on its own, so you'd have to either provide your
>own, or wrap it with something else.
>
Marc, on a related note, What are your thoughts on a
irq_set_irqchip_state? We are running into an issue where chip state
clear is required as well as clearing out that of the parent. Do you see
problems in doing that from an irqchip driver?
--Lina
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains
2019-08-26 16:15 ` Lina Iyer
@ 2019-08-26 18:44 ` Marc Zyngier
2019-08-26 20:14 ` Lina Iyer
0 siblings, 1 reply; 17+ messages in thread
From: Marc Zyngier @ 2019-08-26 18:44 UTC (permalink / raw)
To: Lina Iyer
Cc: Linus Walleij, Brian Masney, open list:GPIO SUBSYSTEM,
Bartosz Golaszewski, Thomas Gleixner, Jon Hunter,
Sowjanya Komatineni, Bitan Biswas, linux-tegra, David Daney,
Masahiro Yamada, Thierry Reding
On Mon, 26 Aug 2019 10:15:26 -0600
Lina Iyer <ilina@codeaurora.org> wrote:
> On Fri, Aug 23 2019 at 03:12 -0600, Marc Zyngier wrote:
> >On 23/08/2019 09:24, Linus Walleij wrote:
> >> Hi Brian,
> >>
> >> I tried to look into this ssbi-gpio problem...
> >>
> >> Paging in Marc Z as well.
> >>
> >> On Fri, Aug 16, 2019 at 3:10 AM Brian Masney <masneyb@onstation.org> wrote:
> >>
> >>> I started to convert ssbi-gpio over to this and pm8xxx_gpio_to_irq() has
> >>> this little snippet that's different from spmi-gpio:
> >>>
> >>> [ fwspec mapping code ]
> >>>
> >>> /*
> >>> * Cache the IRQ since pm8xxx_gpio_get() needs this to get determine the
> >>> * line level.
> >>> */
> >>> pin->irq = ret;
> >>>
> >>> Here's the relevant code in pm8xxx_gpio_get():
> >>>
> >>> if (pin->mode == PM8XXX_GPIO_MODE_OUTPUT) {
> >>> ret = pin->output_value;
> >>> } else if (pin->irq >= 0) {
> >>> ret = irq_get_irqchip_state(pin->irq, IRQCHIP_STATE_LINE_LEVEL, &state);
> >>> ...
> >>> }
> >>
> >> It's a bit annoying that this API (irq_get_irqchip_state()) is relying on
> >> the global irq numbers and the internal function using irqdesc
> >> __irq_get_irqchip_state() is not exported.
> >>
> >> We should be encouraged to operate on IRQ descriptors rather
> >> than IRQ numbers when doing this kind of deep core work,
> >> right?
> >
> >Certainly, I'd like to gradually move over from the IRQ number to an
> >irq_desc. In interrupt-heavy contexts, this ends up being quite time
> >consuming. I have an old patch series somewhere changing irq domains to
> >use irq_descs internally instead of IRQ numbers, which I should maybe
> >revive.
> >
> >As for __irq_get_irqchip_state, the main issue is that it doesn't
> >perform any locking on its own, so you'd have to either provide your
> >own, or wrap it with something else.
> >
> Marc, on a related note, What are your thoughts on a
> irq_set_irqchip_state? We are running into an issue where chip state
> clear is required as well as clearing out that of the parent. Do you see
> problems in doing that from an irqchip driver?
[desperately trying to switch to my korg address...]
I'm not sure I fully get the question: if this is whether it is worth
implementing it, it usually is a good idea if the HW supports is. But I
have the feeling that this isn't your question, so maybe explaining
your use-case would help.
Do you mean using irq_set_irqchip_state from within an irqchip driver?
That'd be quite odd, as this function is usually used when something
*outside* of the irqchip driver needs to poke at some internal state
because "it knows best" (see for example what KVM does with the active
state).
But again, stating your problem would help.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains
2019-08-26 18:44 ` Marc Zyngier
@ 2019-08-26 20:14 ` Lina Iyer
2019-08-27 9:00 ` Marc Zyngier
0 siblings, 1 reply; 17+ messages in thread
From: Lina Iyer @ 2019-08-26 20:14 UTC (permalink / raw)
To: Marc Zyngier
Cc: Linus Walleij, Brian Masney, open list:GPIO SUBSYSTEM,
Bartosz Golaszewski, Thomas Gleixner, Jon Hunter,
Sowjanya Komatineni, Bitan Biswas, linux-tegra, David Daney,
Masahiro Yamada, Thierry Reding
On Mon, Aug 26 2019 at 13:02 -0600, Marc Zyngier wrote:
>On Mon, 26 Aug 2019 10:15:26 -0600
>Lina Iyer <ilina@codeaurora.org> wrote:
>
>> On Fri, Aug 23 2019 at 03:12 -0600, Marc Zyngier wrote:
>> >On 23/08/2019 09:24, Linus Walleij wrote:
>> >> Hi Brian,
>> >>
>> >> I tried to look into this ssbi-gpio problem...
>> >>
>> >> Paging in Marc Z as well.
>> >>
>> >> On Fri, Aug 16, 2019 at 3:10 AM Brian Masney <masneyb@onstation.org> wrote:
>> >>
>> >>> I started to convert ssbi-gpio over to this and pm8xxx_gpio_to_irq() has
>> >>> this little snippet that's different from spmi-gpio:
>> >>>
>> >>> [ fwspec mapping code ]
>> >>>
>> >>> /*
>> >>> * Cache the IRQ since pm8xxx_gpio_get() needs this to get determine the
>> >>> * line level.
>> >>> */
>> >>> pin->irq = ret;
>> >>>
>> >>> Here's the relevant code in pm8xxx_gpio_get():
>> >>>
>> >>> if (pin->mode == PM8XXX_GPIO_MODE_OUTPUT) {
>> >>> ret = pin->output_value;
>> >>> } else if (pin->irq >= 0) {
>> >>> ret = irq_get_irqchip_state(pin->irq, IRQCHIP_STATE_LINE_LEVEL, &state);
>> >>> ...
>> >>> }
>> >>
>> >> It's a bit annoying that this API (irq_get_irqchip_state()) is relying on
>> >> the global irq numbers and the internal function using irqdesc
>> >> __irq_get_irqchip_state() is not exported.
>> >>
>> >> We should be encouraged to operate on IRQ descriptors rather
>> >> than IRQ numbers when doing this kind of deep core work,
>> >> right?
>> >
>> >Certainly, I'd like to gradually move over from the IRQ number to an
>> >irq_desc. In interrupt-heavy contexts, this ends up being quite time
>> >consuming. I have an old patch series somewhere changing irq domains to
>> >use irq_descs internally instead of IRQ numbers, which I should maybe
>> >revive.
>> >
>> >As for __irq_get_irqchip_state, the main issue is that it doesn't
>> >perform any locking on its own, so you'd have to either provide your
>> >own, or wrap it with something else.
>> >
>> Marc, on a related note, What are your thoughts on a
>> irq_set_irqchip_state? We are running into an issue where chip state
>> clear is required as well as clearing out that of the parent. Do you see
>> problems in doing that from an irqchip driver?
>
>[desperately trying to switch to my korg address...]
>
>I'm not sure I fully get the question: if this is whether it is worth
>implementing it, it usually is a good idea if the HW supports is. But I
>have the feeling that this isn't your question, so maybe explaining
>your use-case would help.
>
>Do you mean using irq_set_irqchip_state from within an irqchip driver?
>That'd be quite odd, as this function is usually used when something
>*outside* of the irqchip driver needs to poke at some internal state
>because "it knows best" (see for example what KVM does with the active
>state).
>
Yeah, I meant to say, when invoked on a parent irqchip from a child
irqchip.
In my case when GPIO is used as an interrupt. Even though the interrupt
is not enabled, the GPIO could cause the GIC_ISPEND to be set at GIC. We
were wondering if we could use the irq_set_irqchip_state to clear
pending state at GPIO's irqchip parent and all the way to GIC, when the
GPIO is enabled as an interrupt.
-- Lina
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains
2019-08-26 20:14 ` Lina Iyer
@ 2019-08-27 9:00 ` Marc Zyngier
0 siblings, 0 replies; 17+ messages in thread
From: Marc Zyngier @ 2019-08-27 9:00 UTC (permalink / raw)
To: Lina Iyer
Cc: Linus Walleij, Brian Masney, open list:GPIO SUBSYSTEM,
Bartosz Golaszewski, Thomas Gleixner, Jon Hunter,
Sowjanya Komatineni, Bitan Biswas, linux-tegra, David Daney,
Masahiro Yamada, Thierry Reding
On 26/08/2019 21:14, Lina Iyer wrote:
> On Mon, Aug 26 2019 at 13:02 -0600, Marc Zyngier wrote:
>> On Mon, 26 Aug 2019 10:15:26 -0600
>> Lina Iyer <ilina@codeaurora.org> wrote:
>>
>>> On Fri, Aug 23 2019 at 03:12 -0600, Marc Zyngier wrote:
>>>> On 23/08/2019 09:24, Linus Walleij wrote:
>>>>> Hi Brian,
>>>>>
>>>>> I tried to look into this ssbi-gpio problem...
>>>>>
>>>>> Paging in Marc Z as well.
>>>>>
>>>>> On Fri, Aug 16, 2019 at 3:10 AM Brian Masney <masneyb@onstation.org> wrote:
>>>>>
>>>>>> I started to convert ssbi-gpio over to this and pm8xxx_gpio_to_irq() has
>>>>>> this little snippet that's different from spmi-gpio:
>>>>>>
>>>>>> [ fwspec mapping code ]
>>>>>>
>>>>>> /*
>>>>>> * Cache the IRQ since pm8xxx_gpio_get() needs this to get determine the
>>>>>> * line level.
>>>>>> */
>>>>>> pin->irq = ret;
>>>>>>
>>>>>> Here's the relevant code in pm8xxx_gpio_get():
>>>>>>
>>>>>> if (pin->mode == PM8XXX_GPIO_MODE_OUTPUT) {
>>>>>> ret = pin->output_value;
>>>>>> } else if (pin->irq >= 0) {
>>>>>> ret = irq_get_irqchip_state(pin->irq, IRQCHIP_STATE_LINE_LEVEL, &state);
>>>>>> ...
>>>>>> }
>>>>>
>>>>> It's a bit annoying that this API (irq_get_irqchip_state()) is relying on
>>>>> the global irq numbers and the internal function using irqdesc
>>>>> __irq_get_irqchip_state() is not exported.
>>>>>
>>>>> We should be encouraged to operate on IRQ descriptors rather
>>>>> than IRQ numbers when doing this kind of deep core work,
>>>>> right?
>>>>
>>>> Certainly, I'd like to gradually move over from the IRQ number to an
>>>> irq_desc. In interrupt-heavy contexts, this ends up being quite time
>>>> consuming. I have an old patch series somewhere changing irq domains to
>>>> use irq_descs internally instead of IRQ numbers, which I should maybe
>>>> revive.
>>>>
>>>> As for __irq_get_irqchip_state, the main issue is that it doesn't
>>>> perform any locking on its own, so you'd have to either provide your
>>>> own, or wrap it with something else.
>>>>
>>> Marc, on a related note, What are your thoughts on a
>>> irq_set_irqchip_state? We are running into an issue where chip state
>>> clear is required as well as clearing out that of the parent. Do you see
>>> problems in doing that from an irqchip driver?
>>
>> [desperately trying to switch to my korg address...]
>>
>> I'm not sure I fully get the question: if this is whether it is worth
>> implementing it, it usually is a good idea if the HW supports is. But I
>> have the feeling that this isn't your question, so maybe explaining
>> your use-case would help.
>>
>> Do you mean using irq_set_irqchip_state from within an irqchip driver?
>> That'd be quite odd, as this function is usually used when something
>> *outside* of the irqchip driver needs to poke at some internal state
>> because "it knows best" (see for example what KVM does with the active
>> state).
>>
> Yeah, I meant to say, when invoked on a parent irqchip from a child
> irqchip.
>
> In my case when GPIO is used as an interrupt. Even though the interrupt
> is not enabled, the GPIO could cause the GIC_ISPEND to be set at GIC. We
> were wondering if we could use the irq_set_irqchip_state to clear
> pending state at GPIO's irqchip parent and all the way to GIC, when the
> GPIO is enabled as an interrupt.
You can of course chain the call from one level to its parent. It's a
bit odd to have to clear a pending bit in this context, but I guess
that's a HW-specific limitation.
M.
--
Jazz is not dead, it just smells funny...
^ permalink raw reply [flat|nested] 17+ messages in thread