[PATCH 0/4] Fix the regression for the thunderx gpio

[PATCH 0/4] Fix the regression for the thunderx gpio

[Kevin Hao]

Hi Linus,

Since the commit a7fc89f9d5fc ("gpio: thunderx: Switch to
GPIOLIB_IRQCHIP"), the thunderx gpio doesn't work anymore. I noticed
that you have submitted a patch [1] to fix the " irq_domain_push_irq: -22"
error, but the kernel would panic after applying that fix because the hwirq
passed to the msi irqdomain is still not correct. It seems that we need
more codes to make the thunderx gpio work with the GPIOLIB_IRQCHIP. So I
would prefer to revert the commit a7fc89f9d5fc first to make the thunderx
gpio to work on the 5.4.x and 5.5 at least. We can then do more test for
GPIOLIB_IRQCHIP switching (which the patch 2 ~ 4 do) before merging
them.

[1] https://patchwork.ozlabs.org/patch/1210180/

Kevin Hao (4):
  Revert "gpio: thunderx: Switch to GPIOLIB_IRQCHIP"
  gpiolib: Add support for the irqdomain which doesn't use irq_fwspec as
    arg
  gpiolib: Add the support for the msi parent domain
  gpio: thunderx: Switch to GPIOLIB_IRQCHIP

 drivers/gpio/gpio-tegra186.c             | 13 ++++++--
 drivers/gpio/gpio-thunderx.c             | 36 +++++++++++++++++++---
 drivers/gpio/gpiolib.c                   | 51 ++++++++++++++++++++++----------
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c |  2 +-
 drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c |  2 +-
 include/linux/gpio/driver.h              | 21 +++++--------
 6 files changed, 87 insertions(+), 38 deletions(-)

-- 
2.14.4

[PATCH 1/4] Revert "gpio: thunderx: Switch to GPIOLIB_IRQCHIP"

[Kevin Hao]

This reverts commit a7fc89f9d5fcc10a5474cfe555f5a9e5df8b0f1f because
there are some bugs in this commit, and we don't have a simple way to
fix these bugs. So revert this commit to make the thunderx gpio work
on the stable kernel at least. We will switch to GPIOLIB_IRQCHIP
for thunderx gpio by following patches.

Fixes: a7fc89f9d5fc ("gpio: thunderx: Switch to GPIOLIB_IRQCHIP")
Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 drivers/gpio/Kconfig         |   1 -
 drivers/gpio/gpio-thunderx.c | 163 ++++++++++++++++++++++++++++---------------
 2 files changed, 107 insertions(+), 57 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 9e99d09a64c6..ce65919dee12 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -571,7 +571,6 @@ 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 d08d86a22b1f..462770479045 100644
--- a/drivers/gpio/gpio-thunderx.c
+++ b/drivers/gpio/gpio-thunderx.c
@@ -53,6 +53,7 @@ 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;
@@ -285,60 +286,54 @@ static void thunderx_gpio_set_multiple(struct gpio_chip *chip,
 	}
 }
 
-static void thunderx_gpio_irq_ack(struct irq_data *d)
+static void thunderx_gpio_irq_ack(struct irq_data *data)
 {
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
+	struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
 
 	writeq(GPIO_INTR_INTR,
-	       txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
+	       txline->txgpio->register_base + intr_reg(txline->line));
 }
 
-static void thunderx_gpio_irq_mask(struct irq_data *d)
+static void thunderx_gpio_irq_mask(struct irq_data *data)
 {
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
+	struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
 
 	writeq(GPIO_INTR_ENA_W1C,
-	       txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
+	       txline->txgpio->register_base + intr_reg(txline->line));
 }
 
-static void thunderx_gpio_irq_mask_ack(struct irq_data *d)
+static void thunderx_gpio_irq_mask_ack(struct irq_data *data)
 {
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
+	struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
 
 	writeq(GPIO_INTR_ENA_W1C | GPIO_INTR_INTR,
-	       txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
+	       txline->txgpio->register_base + intr_reg(txline->line));
 }
 
-static void thunderx_gpio_irq_unmask(struct irq_data *d)
+static void thunderx_gpio_irq_unmask(struct irq_data *data)
 {
-	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
-	struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
+	struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
 
 	writeq(GPIO_INTR_ENA_W1S,
-	       txgpio->register_base + intr_reg(irqd_to_hwirq(d)));
+	       txline->txgpio->register_base + intr_reg(txline->line));
 }
 
-static int thunderx_gpio_irq_set_type(struct irq_data *d,
+static int thunderx_gpio_irq_set_type(struct irq_data *data,
 				      unsigned int flow_type)
 {
-	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)];
+	struct thunderx_line *txline = irq_data_get_irq_chip_data(data);
+	struct thunderx_gpio *txgpio = txline->txgpio;
 	u64 bit_cfg;
 
-	irqd_set_trigger_type(d, flow_type);
+	irqd_set_trigger_type(data, flow_type);
 
 	bit_cfg = txline->fil_bits | GPIO_BIT_CFG_INT_EN;
 
 	if (flow_type & IRQ_TYPE_EDGE_BOTH) {
-		irq_set_handler_locked(d, handle_fasteoi_ack_irq);
+		irq_set_handler_locked(data, handle_fasteoi_ack_irq);
 		bit_cfg |= GPIO_BIT_CFG_INT_TYPE;
 	} else {
-		irq_set_handler_locked(d, handle_fasteoi_mask_irq);
+		irq_set_handler_locked(data, handle_fasteoi_mask_irq);
 	}
 
 	raw_spin_lock(&txgpio->lock);
@@ -367,6 +362,33 @@ 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
@@ -383,24 +405,50 @@ 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_child_to_parent_hwirq(struct gpio_chip *gc,
-					       unsigned int child,
-					       unsigned int child_type,
-					       unsigned int *parent,
-					       unsigned int *parent_type)
+static int thunderx_gpio_irq_translate(struct irq_domain *d,
+				       struct irq_fwspec *fwspec,
+				       irq_hw_number_t *hwirq,
+				       unsigned int *type)
 {
-	struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
-
-	*parent = txgpio->base_msi + (2 * child);
-	*parent_type = IRQ_TYPE_LEVEL_HIGH;
+	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;
+
+	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);
+}
+
 static int thunderx_gpio_probe(struct pci_dev *pdev,
 			       const struct pci_device_id *id)
 {
@@ -408,7 +456,6 @@ 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;
 
@@ -453,8 +500,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;
@@ -495,6 +542,27 @@ 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;
@@ -509,28 +577,11 @@ 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;
-	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;
-
+	chip->to_irq = thunderx_gpio_to_irq;
 	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;
@@ -545,10 +596,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->chip.irq.domain,
+		irq_domain_pop_irq(txgpio->irqd,
 				   txgpio->msix_entries[i].vector);
 
-	irq_domain_remove(txgpio->chip.irq.domain);
+	irq_domain_remove(txgpio->irqd);
 
 	pci_set_drvdata(pdev, NULL);
 }
-- 
2.14.4

[PATCH 2/4] gpiolib: Add support for the irqdomain which doesn't use irq_fwspec as arg

[Kevin Hao]

Some gpio's parent irqdomain may not use the struct irq_fwspec as
argument, such as msi irqdomain. So rename the callback
populate_parent_fwspec() to populate_parent_alloc_arg() and make it
allocate and populate the specific struct which is needed by the
parent irqdomain.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 drivers/gpio/gpio-tegra186.c             | 13 ++++++---
 drivers/gpio/gpiolib.c                   | 45 ++++++++++++++++++++------------
 drivers/pinctrl/qcom/pinctrl-spmi-gpio.c |  2 +-
 drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c |  2 +-
 include/linux/gpio/driver.h              | 21 ++++++---------
 5 files changed, 49 insertions(+), 34 deletions(-)

diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
index 55b43b7ce88d..de241263d4be 100644
--- a/drivers/gpio/gpio-tegra186.c
+++ b/drivers/gpio/gpio-tegra186.c
@@ -448,17 +448,24 @@ static int tegra186_gpio_irq_domain_translate(struct irq_domain *domain,
 	return 0;
 }
 
-static void tegra186_gpio_populate_parent_fwspec(struct gpio_chip *chip,
-						 struct irq_fwspec *fwspec,
+static void *tegra186_gpio_populate_parent_fwspec(struct gpio_chip *chip,
 						 unsigned int parent_hwirq,
 						 unsigned int parent_type)
 {
 	struct tegra_gpio *gpio = gpiochip_get_data(chip);
+	struct irq_fwspec *fwspec;
 
+	fwspec = kmalloc(sizeof(*fwspec), GFP_KERNEL);
+	if (!fwspec)
+		return NULL;
+
+	fwspec->fwnode = chip->irq.parent_domain->fwnode;
 	fwspec->param_count = 3;
 	fwspec->param[0] = gpio->soc->instance;
 	fwspec->param[1] = parent_hwirq;
 	fwspec->param[2] = parent_type;
+
+	return fwspec;
 }
 
 static int tegra186_gpio_child_to_parent_hwirq(struct gpio_chip *chip,
@@ -621,7 +628,7 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
 	irq->chip = &gpio->intc;
 	irq->fwnode = of_node_to_fwnode(pdev->dev.of_node);
 	irq->child_to_parent_hwirq = tegra186_gpio_child_to_parent_hwirq;
-	irq->populate_parent_fwspec = tegra186_gpio_populate_parent_fwspec;
+	irq->populate_parent_alloc_arg = tegra186_gpio_populate_parent_fwspec;
 	irq->child_offset_to_irq = tegra186_gpio_child_offset_to_irq;
 	irq->child_irq_domain_ops.translate = tegra186_gpio_irq_domain_translate;
 	irq->handler = handle_simple_irq;
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 967371377a9d..bedf611a7c9e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1990,7 +1990,7 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 	irq_hw_number_t hwirq;
 	unsigned int type = IRQ_TYPE_NONE;
 	struct irq_fwspec *fwspec = data;
-	struct irq_fwspec parent_fwspec;
+	void *parent_arg;
 	unsigned int parent_hwirq;
 	unsigned int parent_type;
 	struct gpio_irq_chip *girq = &gc->irq;
@@ -2029,23 +2029,20 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 			    NULL, NULL);
 	irq_set_probe(irq);
 
-	/*
-	 * 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;
 	/* This parent only handles asserted level IRQs */
-	girq->populate_parent_fwspec(gc, &parent_fwspec, parent_hwirq,
-				     parent_type);
+	parent_arg = girq->populate_parent_alloc_arg(gc, parent_hwirq, parent_type);
+	if (!parent_arg)
+		return -ENOMEM;
+
 	chip_info(gc, "alloc_irqs_parent for %d parent hwirq %d\n",
 		  irq, parent_hwirq);
-	ret = irq_domain_alloc_irqs_parent(d, irq, 1, &parent_fwspec);
+	ret = irq_domain_alloc_irqs_parent(d, irq, 1, parent_arg);
 	if (ret)
 		chip_err(gc,
 			 "failed to allocate parent hwirq %d for hwirq %lu\n",
 			 parent_hwirq, hwirq);
 
+	kfree(parent_arg);
 	return ret;
 }
 
@@ -2082,8 +2079,8 @@ static int gpiochip_hierarchy_add_domain(struct gpio_chip *gc)
 	if (!gc->irq.child_offset_to_irq)
 		gc->irq.child_offset_to_irq = gpiochip_child_offset_to_irq_noop;
 
-	if (!gc->irq.populate_parent_fwspec)
-		gc->irq.populate_parent_fwspec =
+	if (!gc->irq.populate_parent_alloc_arg)
+		gc->irq.populate_parent_alloc_arg =
 			gpiochip_populate_parent_fwspec_twocell;
 
 	gpiochip_hierarchy_setup_domain_ops(&gc->irq.child_irq_domain_ops);
@@ -2109,27 +2106,43 @@ static bool gpiochip_hierarchy_is_hierarchical(struct gpio_chip *gc)
 	return !!gc->irq.parent_domain;
 }
 
-void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
-					     struct irq_fwspec *fwspec,
+void *gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
 					     unsigned int parent_hwirq,
 					     unsigned int parent_type)
 {
+	struct irq_fwspec *fwspec;
+
+	fwspec = kmalloc(sizeof(*fwspec), GFP_KERNEL);
+	if (!fwspec)
+		return NULL;
+
+	fwspec->fwnode = chip->irq.parent_domain->fwnode;
 	fwspec->param_count = 2;
 	fwspec->param[0] = parent_hwirq;
 	fwspec->param[1] = parent_type;
+
+	return fwspec;
 }
 EXPORT_SYMBOL_GPL(gpiochip_populate_parent_fwspec_twocell);
 
-void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
-					      struct irq_fwspec *fwspec,
+void *gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
 					      unsigned int parent_hwirq,
 					      unsigned int parent_type)
 {
+	struct irq_fwspec *fwspec;
+
+	fwspec = kmalloc(sizeof(*fwspec), GFP_KERNEL);
+	if (!fwspec)
+		return NULL;
+
+	fwspec->fwnode = chip->irq.parent_domain->fwnode;
 	fwspec->param_count = 4;
 	fwspec->param[0] = 0;
 	fwspec->param[1] = parent_hwirq;
 	fwspec->param[2] = 0;
 	fwspec->param[3] = parent_type;
+
+	return fwspec;
 }
 EXPORT_SYMBOL_GPL(gpiochip_populate_parent_fwspec_fourcell);
 
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index 653d1095bfea..fe0be8a6ebb7 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -1060,7 +1060,7 @@ static int pmic_gpio_probe(struct platform_device *pdev)
 	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->populate_parent_alloc_arg = 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;
 
diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
index c7912135bbfb..fba1d41d20ec 100644
--- a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
@@ -794,7 +794,7 @@ static int pm8xxx_gpio_probe(struct platform_device *pdev)
 	girq->fwnode = of_node_to_fwnode(pctrl->dev->of_node);
 	girq->parent_domain = parent_domain;
 	girq->child_to_parent_hwirq = pm8xxx_child_to_parent_hwirq;
-	girq->populate_parent_fwspec = gpiochip_populate_parent_fwspec_fourcell;
+	girq->populate_parent_alloc_arg = gpiochip_populate_parent_fwspec_fourcell;
 	girq->child_offset_to_irq = pm8xxx_child_offset_to_irq;
 	girq->child_irq_domain_ops.translate = pm8xxx_domain_translate;
 
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 4f032de10bae..ea6e615ad7fc 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -94,16 +94,15 @@ struct gpio_irq_chip {
 				     unsigned int *parent_type);
 
 	/**
-	 * @populate_parent_fwspec:
+	 * @populate_parent_alloc_arg :
 	 *
-	 * This optional callback populates the &struct irq_fwspec for the
-	 * parent's IRQ domain. If this is not specified, then
+	 * This optional callback allocates and populates the specific struct
+	 * for the parent's IRQ domain. If this is not specified, then
 	 * &gpiochip_populate_parent_fwspec_twocell will be used. A four-cell
 	 * variant named &gpiochip_populate_parent_fwspec_fourcell is also
 	 * available.
 	 */
-	void (*populate_parent_fwspec)(struct gpio_chip *chip,
-				       struct irq_fwspec *fwspec,
+	void *(*populate_parent_alloc_arg)(struct gpio_chip *chip,
 				       unsigned int parent_hwirq,
 				       unsigned int parent_type);
 
@@ -537,26 +536,22 @@ struct bgpio_pdata {
 
 #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
 
-void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
-					     struct irq_fwspec *fwspec,
+void *gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
 					     unsigned int parent_hwirq,
 					     unsigned int parent_type);
-void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
-					      struct irq_fwspec *fwspec,
+void *gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
 					      unsigned int parent_hwirq,
 					      unsigned int parent_type);
 
 #else
 
-static inline void gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
-						    struct irq_fwspec *fwspec,
+static inline void *gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
 						    unsigned int parent_hwirq,
 						    unsigned int parent_type)
 {
 }
 
-static inline void gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
-						     struct irq_fwspec *fwspec,
+static inline void *gpiochip_populate_parent_fwspec_fourcell(struct gpio_chip *chip,
 						     unsigned int parent_hwirq,
 						     unsigned int parent_type)
 {
-- 
2.14.4

[PATCH 3/4] gpiolib: Add the support for the msi parent domain

[Kevin Hao]

If the gpio's parent irqdomain is a msi irqdomain, we should ignore
the EEXIST error returned by the msi irqdomain because all the msi
interrupts have already been allocated.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 drivers/gpio/gpiolib.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index bedf611a7c9e..1cca9ab61923 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2037,6 +2037,12 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
 	chip_info(gc, "alloc_irqs_parent for %d parent hwirq %d\n",
 		  irq, parent_hwirq);
 	ret = irq_domain_alloc_irqs_parent(d, irq, 1, parent_arg);
+	/*
+	 * If the parent irqdomain is msi, the interrupts have already
+	 * been allocated, so the EEXIST is good.
+	 */
+	if (irq_domain_is_msi(d->parent) && (ret == -EEXIST))
+		ret = 0;
 	if (ret)
 		chip_err(gc,
 			 "failed to allocate parent hwirq %d for hwirq %lu\n",
-- 
2.14.4

[PATCH 4/4] gpio: thunderx: Switch to GPIOLIB_IRQCHIP

[Kevin Hao]

The main parts of this patch are from commit a7fc89f9d5fc ("gpio:
thunderx: Switch to GPIOLIB_IRQCHIP") and patch [1]. And also adjust
thunderx_gpio_child_to_parent_hwirq() and add
thunderx_gpio_populate_parent_alloc_info() to make sure that
the correct hwirq are passed to the parent msi irqdomain.

[1] https://patchwork.ozlabs.org/patch/1210180/

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 drivers/gpio/Kconfig         |   1 +
 drivers/gpio/gpio-thunderx.c | 177 +++++++++++++++++++------------------------
 2 files changed, 78 insertions(+), 100 deletions(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ce65919dee12..9e99d09a64c6 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -571,6 +571,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 462770479045..9f66deab46ea 100644
--- a/drivers/gpio/gpio-thunderx.c
+++ b/drivers/gpio/gpio-thunderx.c
@@ -15,6 +15,7 @@
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/spinlock.h>
+#include <asm-generic/msi.h>
 
 
 #define GPIO_RX_DAT	0x0
@@ -53,7 +54,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;
@@ -286,54 +286,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);
@@ -362,33 +368,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
@@ -405,48 +384,42 @@ 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;
+	struct thunderx_gpio *txgpio = gpiochip_get_data(gc);
+	struct irq_data *irqd;
+	unsigned int irq;
 
-	if (WARN_ON(fwspec->param_count < 2))
+	irq = txgpio->msix_entries[child].vector;
+	irqd = irq_domain_get_irq_data(gc->irq.parent_domain, irq);
+	if (!irqd)
 		return -EINVAL;
-	if (fwspec->param[0] >= txgpio->chip.ngpio)
-		return -EINVAL;
-	*hwirq = fwspec->param[0];
-	*type = fwspec->param[1] & IRQ_TYPE_SENSE_MASK;
+	*parent = irqd_to_hwirq(irqd);
+	*parent_type = IRQ_TYPE_LEVEL_HIGH;
 	return 0;
 }
 
-static int thunderx_gpio_irq_alloc(struct irq_domain *d, unsigned int virq,
-				   unsigned int nr_irqs, void *arg)
+static void *thunderx_gpio_populate_parent_alloc_info(struct gpio_chip *chip,
+						      unsigned int parent_hwirq,
+						      unsigned int parent_type)
 {
-	struct thunderx_line *txline = arg;
-
-	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
-};
+	msi_alloc_info_t *info;
 
-static int thunderx_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
-{
-	struct thunderx_gpio *txgpio = gpiochip_get_data(chip);
+	info = kmalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return NULL;
 
-	return irq_find_mapping(txgpio->irqd, offset);
+	info->hwirq = parent_hwirq;
+	return info;
 }
 
 static int thunderx_gpio_probe(struct pci_dev *pdev,
@@ -456,6 +429,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;
 
@@ -500,8 +474,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;
@@ -542,27 +516,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;
@@ -577,11 +530,35 @@ 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->populate_parent_alloc_arg = thunderx_gpio_populate_parent_alloc_info;
+	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++) {
+		struct irq_fwspec fwspec;
+
+		fwspec.fwnode = of_node_to_fwnode(dev->of_node);
+		fwspec.param_count = 2;
+		fwspec.param[0] = i;
+		fwspec.param[1] = IRQ_TYPE_NONE;
+		err = irq_domain_push_irq(girq->domain,
+					  txgpio->msix_entries[i].vector,
+					  &fwspec);
+		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;
@@ -596,10 +573,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.14.4

Re: [PATCH 0/4] Fix the regression for the thunderx gpio

[Linus Walleij]

On Tue, Jan 14, 2020 at 9:39 AM Kevin Hao <haokexin@gmail.com> wrote:

> Since the commit a7fc89f9d5fc ("gpio: thunderx: Switch to
> GPIOLIB_IRQCHIP"), the thunderx gpio doesn't work anymore. I noticed
> that you have submitted a patch [1] to fix the " irq_domain_push_irq: -22"
> error, but the kernel would panic after applying that fix because the hwirq
> passed to the msi irqdomain is still not correct. It seems that we need
> more codes to make the thunderx gpio work with the GPIOLIB_IRQCHIP. So I
> would prefer to revert the commit a7fc89f9d5fc first to make the thunderx
> gpio to work on the 5.4.x and 5.5 at least. We can then do more test for
> GPIOLIB_IRQCHIP switching (which the patch 2 ~ 4 do) before merging
> them.

Thanks a LOT Kevin, and I'm sorry for open coding and breaking this
driver so much :/

I have applied all four patches for fixes.

Yours,
Linus Walleij

Re: [PATCH 2/4] gpiolib: Add support for the irqdomain which doesn't use irq_fwspec as arg

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2819 bytes --]

Hi Kevin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on gpio/for-next]
[also build test WARNING on pinctrl/devel v5.5-rc6 next-20200110]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Kevin-Hao/Fix-the-regression-for-the-thunderx-gpio/20200114-164910
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: m68k-m5475evb_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/asm-generic/gpio.h:13:0,
                    from arch/m68k/include/asm/mcfgpio.h:12,
                    from arch/m68k/include/asm/gpio.h:14,
                    from include/linux/gpio.h:59,
                    from arch/m68k/coldfire/device.c:15:
   include/linux/gpio/driver.h: In function 'gpiochip_populate_parent_fwspec_twocell':
>> include/linux/gpio/driver.h:552:1: warning: no return statement in function returning non-void [-Wreturn-type]
    }
    ^
   include/linux/gpio/driver.h: In function 'gpiochip_populate_parent_fwspec_fourcell':
   include/linux/gpio/driver.h:558:1: warning: no return statement in function returning non-void [-Wreturn-type]
    }
    ^

vim +552 include/linux/gpio/driver.h

fdd61a013a24f2 Linus Walleij 2019-08-08  547  
cee429771459e7 Kevin Hao     2020-01-14  548  static inline void *gpiochip_populate_parent_fwspec_twocell(struct gpio_chip *chip,
fdd61a013a24f2 Linus Walleij 2019-08-08  549  						    unsigned int parent_hwirq,
fdd61a013a24f2 Linus Walleij 2019-08-08  550  						    unsigned int parent_type)
fdd61a013a24f2 Linus Walleij 2019-08-08  551  {
fdd61a013a24f2 Linus Walleij 2019-08-08 @552  }
fdd61a013a24f2 Linus Walleij 2019-08-08  553  

:::::: The code at line 552 was first introduced by commit
:::::: fdd61a013a24f2699aec1a446f0168682b6f9ec4 gpio: Add support for hierarchical IRQ domains

:::::: TO: Linus Walleij <linus.walleij@linaro.org>
:::::: CC: Linus Walleij <linus.walleij@linaro.org>

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corporation

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 7255 bytes --]

Re: [PATCH 0/4] Fix the regression for the thunderx gpio

[Tim Harvey]

On Wed, Jan 15, 2020 at 2:20 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Tue, Jan 14, 2020 at 9:39 AM Kevin Hao <haokexin@gmail.com> wrote:
>
> > Since the commit a7fc89f9d5fc ("gpio: thunderx: Switch to
> > GPIOLIB_IRQCHIP"), the thunderx gpio doesn't work anymore. I noticed
> > that you have submitted a patch [1] to fix the " irq_domain_push_irq: -22"
> > error, but the kernel would panic after applying that fix because the hwirq
> > passed to the msi irqdomain is still not correct. It seems that we need
> > more codes to make the thunderx gpio work with the GPIOLIB_IRQCHIP. So I
> > would prefer to revert the commit a7fc89f9d5fc first to make the thunderx
> > gpio to work on the 5.4.x and 5.5 at least. We can then do more test for
> > GPIOLIB_IRQCHIP switching (which the patch 2 ~ 4 do) before merging
> > them.
>
> Thanks a LOT Kevin, and I'm sorry for open coding and breaking this
> driver so much :/
>
> I have applied all four patches for fixes.
>

I'm running into an issue with thunderx-gpio when using a gpio as an
interrupt with an mfd driver I'm working on[1]. The breakage appeared
with 0d04d0c146786da42c6e68c7d2f09c956c5b5bd3 'gpio: thunderx: Use the
default parent apis for {request,release}_resources'[2] and occurs
when irq_chip_request_resources_parent() fails with -ENOSYS. Any ideas
what happened here... It seems perhaps parent_data got lost?

1. https://patchwork.kernel.org/patch/11401555/
2. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/gpio-thunderx.c?id=0d04d0c146786da42c6e68c7d2f09c956c5b5bd3

Best regards,

Tim

Re: [PATCH 0/4] Fix the regression for the thunderx gpio

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 2691 bytes --]

On Tue, Mar 10, 2020 at 01:40:56PM -0700, Tim Harvey wrote:
> On Wed, Jan 15, 2020 at 2:20 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > On Tue, Jan 14, 2020 at 9:39 AM Kevin Hao <haokexin@gmail.com> wrote:
> >
> > > Since the commit a7fc89f9d5fc ("gpio: thunderx: Switch to
> > > GPIOLIB_IRQCHIP"), the thunderx gpio doesn't work anymore. I noticed
> > > that you have submitted a patch [1] to fix the " irq_domain_push_irq: -22"
> > > error, but the kernel would panic after applying that fix because the hwirq
> > > passed to the msi irqdomain is still not correct. It seems that we need
> > > more codes to make the thunderx gpio work with the GPIOLIB_IRQCHIP. So I
> > > would prefer to revert the commit a7fc89f9d5fc first to make the thunderx
> > > gpio to work on the 5.4.x and 5.5 at least. We can then do more test for
> > > GPIOLIB_IRQCHIP switching (which the patch 2 ~ 4 do) before merging
> > > them.
> >
> > Thanks a LOT Kevin, and I'm sorry for open coding and breaking this
> > driver so much :/
> >
> > I have applied all four patches for fixes.
> >
> 
> I'm running into an issue with thunderx-gpio when using a gpio as an
> interrupt with an mfd driver I'm working on[1]. The breakage appeared
> with 0d04d0c146786da42c6e68c7d2f09c956c5b5bd3 'gpio: thunderx: Use the
> default parent apis for {request,release}_resources'[2] and occurs
> when irq_chip_request_resources_parent() fails with -ENOSYS. Any ideas
> what happened here... It seems perhaps parent_data got lost?

No, the parent_data doesn't get lost. The reason that -ENOSYS is returned is because
the its_msi_irq_chip (the parent irq chip of thunderx-gpio) doesn't implement the
.irq_request_resources callback. As you can see in the irq_chip_request_resources_parent() code:
    int irq_chip_request_resources_parent(struct irq_data *data)
    {
    	data = data->parent_data;
    
    	if (data->chip->irq_request_resources)
    		return data->chip->irq_request_resources(data);
    
    	return -ENOSYS;
    }

So the commit 0d04d0c14678 does change the logic of the original code and make
the thunderx_gpio_irq_request_resources() return -ENOSYS in a normal case. But
it doesn't matter now since the thunderx_gpio_irq_request_resources() has been
dropped by the patches in this patch series. So your code should work with the latest
code. Could you rebase your code and git it a try?

Thanks,
Kevin


> 
> 1. https://patchwork.kernel.org/patch/11401555/
> 2. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpio/gpio-thunderx.c?id=0d04d0c146786da42c6e68c7d2f09c956c5b5bd3
> 
> Best regards,
> 
> Tim

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]