Re: [PATCH 1/6 v2] gpio: Add support for hierarchical IRQ domains

[Brian Masney <masneyb@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx>
> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
> Cc: Lina Iyer <ilina@xxxxxxxxxxxxxx>
> Cc: Jon Hunter <jonathanh@xxxxxxxxxx>
> Cc: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
> Cc: Bitan Biswas <bbiswas@xxxxxxxxxx>
> Cc: linux-tegra@xxxxxxxxxxxxxxx
> Cc: David Daney <david.daney@xxxxxxxxxx>
> Cc: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> Cc: Brian Masney <masneyb@xxxxxxxxxxxxx>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> Signed-off-by: Brian Masney <masneyb@xxxxxxxxxxxxx>
> Co-developed-by: Brian Masney <masneyb@xxxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---

[ 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