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

[Brian Masney <masneyb@xxxxxxxxxxxxx>]

Hi Linus,

On Mon, Jun 24, 2019 at 03:25:28PM +0200, Linus Walleij wrote:
>  static int gpiochip_to_irq(struct gpio_chip *chip, unsigned offset)
>  {
> +	struct irq_domain *domain = chip->irq.domain;
> +
>  	if (!gpiochip_irqchip_irq_valid(chip, offset))
>  		return -ENXIO;
>  
> -	return irq_create_mapping(chip->irq.domain, offset);
> +	if (irq_domain_is_hierarchy(domain)) {
> +		struct irq_fwspec spec;
> +
> +		spec.fwnode = domain->fwnode;
> +		spec.param_count = 2;
> +		spec.param[0] = offset;
> +		spec.param[1] = IRQ_TYPE_NONE;
> +
> +		return irq_create_fwspec_mapping(&spec);
> +	}

spmi-gpio's to_irq() needs to add one to the 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);
	}

ssbi-gpio will have the same problem as well.

What do you think about adding a new field to the struct gpio_irq_chip
inside the CONFIG_IRQ_DOMAIN_HIERARCHY ifdef called something like
to_irq_offset? (I'm bad at naming things.)

Also, instead of hardcoding IRQ_TYPE_NONE, what do you think about using
the default_type field that's available?

Brian