Re: [PATCH 1/4 v1] gpio: Add support for hierarchical IRQ domains

[Brian Masney <masneyb@xxxxxxxxxxxxx>]

On Mon, Jun 24, 2019 at 03:25:28PM +0200, Linus Walleij wrote:
> +static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d,
> +					       unsigned int irq,
> +					       unsigned int nr_irqs,
> +					       void *data)
> +{
> +	struct gpio_chip *gc = d->host_data;
> +	irq_hw_number_t hwirq;
> +	unsigned int type = IRQ_TYPE_NONE;
> +	struct irq_fwspec *fwspec = data;
> +	int ret;
> +	int i;
> +
> +	chip_info(gc, "called %s\n", __func__);
> +
> +	ret = gpiochip_hierarchy_irq_domain_translate(d, fwspec, &hwirq, &type);
> +	if (ret)
> +		return ret;
> +
> +	chip_info(gc, "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;
> +		unsigned int parent_hwirq;
> +		unsigned int parent_type;
> +		struct gpio_irq_chip *girq = &gc->irq;
> +
> +		ret = girq->child_to_parent_hwirq(gc, hwirq, type,
> +						  &parent_hwirq, &parent_type);
> +		if (ret) {
> +			chip_err(gc, "can't look up hwirq %lu\n", hwirq);
> +			return ret;
> +		}
> +		chip_info(gc, "found parent hwirq %u\n", 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,
> +				    gc->irq.chip,
> +				    gc,
> +				    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] = parent_hwirq;
> +		/* This parent only handles asserted level IRQs */
> +		parent_fwspec.param[1] = parent_type;
> +		chip_info(gc, "alloc_irqs_parent for %d parent hwirq %d\n",
> +			  irq + i, parent_hwirq);
> +		ret = irq_domain_alloc_irqs_parent(d, irq + i, 1,
> +						   &parent_fwspec);

I started to convert qcom's spmi-gpio over to this new API. I'm not done
yet but I noticed that this new API assumes two cells for the parent,
however spmi-gpio's parent (drivers/spmi/spmi-pmic-arb.c) expects four
cells. See pmic_gpio_domain_alloc() in
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c for more details.

What do you think about adding a function pointer to struct
gpio_irq_chip that is used to populate the parent_fwspec?

Brian