Multi-parent IRQ domains

Multi-parent IRQ domains

[Thierry Reding]

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

Hi everyone,

I've been trying to implement a feature on recent Tegra chips that's
called "wake events". These wake events are implemented as part of the
power management controller (PMC) and they control which events can be
used to wake the system from suspend. Events include things like an
alarm interrupt from an RTC, or the GPIO interrupt hooked up to a
power button for example.

We already have a driver for a couple of other things that the PMC does,
so I thought it'd be natural to implement this functionality as an IRQ
chip in the PMC driver. The way that this would work is that for all the
devices that are wakeup sources, we'd end up specifying the PMC as the
interrupt parent and the PMC would then itself have the main GIC as its
parent. That way, the hierarchical IRQ domain infrastructure would take
care of pretty much everything.

Unfortunately I've run into some issues here. One problem that I'm
facing is that PMC could have more than one parent. For example, the RTC
alarm interrupt goes straight to the GIC, but the power button GPIO goes
through the GPIO controller first and then to the GIC. This doesn't seem
to be something that's currently possible.

The way I imagine this to work from a DT perspective is as follows:

	rtc: rtc@c2a0000 {
		compatible = "nvidia,tegra194-rtc", "nvidia,tegra20-rtc";
		reg = <0x0c2a0000 0x10000>;
		interrupt-parent = <&pmc>;
		interrupts = <73 &gic GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
		status = "disabled";
	};

	pmc: pmc@c360000 {
		compatible = "nvidia,tegra194-pmc";
		reg = <0x0c360000 0x10000>,
		      <0x0c370000 0x10000>,
		      <0x0c380000 0x10000>,
		      <0x0c390000 0x10000>,
		      <0x0c3a0000 0x10000>;
		reg-names = "pmc", "wake", "aotag", "scratch", "misc";

		#interrupt-cells = <5>;
		interrupt-controller;
	};

	gpio-keys {
		compatible = "gpio-keys";

		power {
			label = "Power";
			gpios = <&gpio_aon TEGRA194_AON_GPIO(EE, 4)
					   GPIO_ACTIVE_LOW>;
			linux,input-type = <EV_KEY>;
			linux,code = <KEY_POWER>;
			debounce-interval = <10>;
			interrupt-parent = <&pmc>;
			interrupts = <29 &gpio_aon TEGRA194_AON_GPIO(EE, 4)
						   IRQ_TYPE_LEVEL_LOW 0>;
			wakeup-source;
		};
	};

The above is somewhat brittle because the PMC needs to be able to cover
both GIC and GPIO interrupt specifiers, so it gets 5 interrupt cells
(one for the wake event ID, one for a phandle to the parent controller
and two or three for the parent specifier). It'd be nicer if we could
actually have specifiers that exactly match the bindings for the
respective parent controllers (so that the GPIO specifier didn't have to
contain the dummy 0 at the end), but I don't see a way how that can be
achieved, given how we have to "embed" the specifier in another. We
could probably make it work in code, but DTC would still flag these as
errors.

One thing I briefly looked into was using interrupt-map to implement
this, which would give us a nice way of mapping arbitrary parent
interrupts to wake events. However, since we need to program some of the
PMC registers at ->irq_set_wake() time, using simply an interrupt-map
wouldn't do the trick, since we wouldn't be implementing an IRQ chip and
hence wouldn't be able to install any callbacks.

Perhaps a simple way to implement this would be to just blindly enable
all wake events, but that's not really what we want. We really only want
to enable them for devices that can be wakeup sources. Otherwise we may
get wakeups for completely unrelated GPIO activity.

I tried implementing support for multiple parents within the PMC driver
by looking up the parent domain based on the phandle from the specifier
at IRQ domain ->alloc() and ->translate() time, but I run into deadlock
when I do that, so before I go and open up a can of worms I wanted to
get your input on what the best way forward would be.

Does anyone have any ideas on how to solve this? Perhaps there's a
better solution to this that doesn't involve IRQ infrastructure that I'm
not aware of?

Thanks,
Thierry

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

Re: Multi-parent IRQ domains

[Florian Fainelli]

On 09/13/18 08:59, Thierry Reding wrote:
> Hi everyone,
> 
> I've been trying to implement a feature on recent Tegra chips that's
> called "wake events". These wake events are implemented as part of the
> power management controller (PMC) and they control which events can be
> used to wake the system from suspend. Events include things like an
> alarm interrupt from an RTC, or the GPIO interrupt hooked up to a
> power button for example.
> 
> We already have a driver for a couple of other things that the PMC does,
> so I thought it'd be natural to implement this functionality as an IRQ
> chip in the PMC driver. The way that this would work is that for all the
> devices that are wakeup sources, we'd end up specifying the PMC as the
> interrupt parent and the PMC would then itself have the main GIC as its
> parent. That way, the hierarchical IRQ domain infrastructure would take
> care of pretty much everything.
> 
> Unfortunately I've run into some issues here. One problem that I'm
> facing is that PMC could have more than one parent. For example, the RTC
> alarm interrupt goes straight to the GIC, but the power button GPIO goes
> through the GPIO controller first and then to the GIC. This doesn't seem
> to be something that's currently possible.
> 
> The way I imagine this to work from a DT perspective is as follows:

There is an interrupts-extended property which as a first cell takes a
phandle to the parent interrupt controller, e.g:

interrupts-extended = <&gic 73 GIPC_SPI IRQ_TYPE_LEVEL_HIGH>,
			<&pmc 10>;

(provided that I understood your problem and example correctly). We have
a similar use case on our STB chips where we have "main" interrupts
wired to the ARM GIC and wake-up interrupts wired to a specific wake-up
interrupt controller in an always-on domain. The two controllers have a
different #interrupt-cells specifier.

The interrupt infrastructure in Linux already knows how to deal with
that and whether you use of_get_irq() or platform_get_irq() you should
obtain the correct virtual interrupt numbers that you expected, parented
to the appropriate interrupt controller.

Hope this helps.
-- 
Florian

Re: Multi-parent IRQ domains

[Thierry Reding]

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

On Thu, Sep 13, 2018 at 02:50:21PM -0700, Florian Fainelli wrote:
> 
> 
> On 09/13/18 08:59, Thierry Reding wrote:
> > Hi everyone,
> > 
> > I've been trying to implement a feature on recent Tegra chips that's
> > called "wake events". These wake events are implemented as part of the
> > power management controller (PMC) and they control which events can be
> > used to wake the system from suspend. Events include things like an
> > alarm interrupt from an RTC, or the GPIO interrupt hooked up to a
> > power button for example.
> > 
> > We already have a driver for a couple of other things that the PMC does,
> > so I thought it'd be natural to implement this functionality as an IRQ
> > chip in the PMC driver. The way that this would work is that for all the
> > devices that are wakeup sources, we'd end up specifying the PMC as the
> > interrupt parent and the PMC would then itself have the main GIC as its
> > parent. That way, the hierarchical IRQ domain infrastructure would take
> > care of pretty much everything.
> > 
> > Unfortunately I've run into some issues here. One problem that I'm
> > facing is that PMC could have more than one parent. For example, the RTC
> > alarm interrupt goes straight to the GIC, but the power button GPIO goes
> > through the GPIO controller first and then to the GIC. This doesn't seem
> > to be something that's currently possible.
> > 
> > The way I imagine this to work from a DT perspective is as follows:
> 
> There is an interrupts-extended property which as a first cell takes a
> phandle to the parent interrupt controller, e.g:
> 
> interrupts-extended = <&gic 73 GIPC_SPI IRQ_TYPE_LEVEL_HIGH>,
> 			<&pmc 10>;
> 
> (provided that I understood your problem and example correctly). We have
> a similar use case on our STB chips where we have "main" interrupts
> wired to the ARM GIC and wake-up interrupts wired to a specific wake-up
> interrupt controller in an always-on domain. The two controllers have a
> different #interrupt-cells specifier.
> 
> The interrupt infrastructure in Linux already knows how to deal with
> that and whether you use of_get_irq() or platform_get_irq() you should
> obtain the correct virtual interrupt numbers that you expected, parented
> to the appropriate interrupt controller.

Yeah, I had looked at the interrupts-extended property as well, but
while I think it might work, one issue with that is that we'd be
modelling the regular IRQ and wakeup IRQ as two different IRQs. On
Tegra, however, the wakeup interrupt is the same as the regular
interrupt, except that the bits to enable the wake logic is in a
different hardware block. So the PMC acts more like an additional
mux for a subset of the interrupts in the system.

In addition to that, if I were to model the interrupts separately, I'd
also have to modify existing drivers for all the wakeup-able devices to
deal with two separate interrupts, which is rather undesirable.

Thierry

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

Re: Multi-parent IRQ domains

[Thomas Gleixner]

On Thu, 13 Sep 2018, Thierry Reding wrote:
> I've been trying to implement a feature on recent Tegra chips that's
> called "wake events". These wake events are implemented as part of the
> power management controller (PMC) and they control which events can be
> used to wake the system from suspend. Events include things like an
> alarm interrupt from an RTC, or the GPIO interrupt hooked up to a
> power button for example.
> 
> We already have a driver for a couple of other things that the PMC does,
> so I thought it'd be natural to implement this functionality as an IRQ
> chip in the PMC driver. The way that this would work is that for all the
> devices that are wakeup sources, we'd end up specifying the PMC as the
> interrupt parent and the PMC would then itself have the main GIC as its
> parent. That way, the hierarchical IRQ domain infrastructure would take
> care of pretty much everything.
> 
> Unfortunately I've run into some issues here. One problem that I'm
> facing is that PMC could have more than one parent. For example, the RTC
> alarm interrupt goes straight to the GIC, but the power button GPIO goes
> through the GPIO controller first and then to the GIC. This doesn't seem
> to be something that's currently possible.

So what you are saying is that the RTC is directly wired up to the GIC and
the GPIO interrupts are demultiplxed by a single GIC interrupt first, but
at the same time have the requirement to talk to the PMC.

Lets look at the current implementation without taking PMC into account.

- RTC gets its interrupt directly from the GIC domain

- GPIO gets its interrupt from the GPIO domain. The GPIO domain does not
  have a parent domain. It is stand alone because it sets up a demultiplex
  interrupt from the GIC domain.

  So there is absolutely no irqdomain relationship between a single GPIO
  and the GIC. The indirection through the demultiplex interrupt does not
  create one, really.

Now, you need the PMC for both, the GPIOs and the RTC. What you can do here
is to provide two irq domains in PMC. One which has GIC as its parent and
one which has no parent. Surely they need to share some resources, but
that should be a solvable problem.

Now you connect RTC to the GIC parented one and let the PMC irq chip just
hand through, except for the set_wake() functionality.

The GPIO domain is connected to the parentless PMC domain and the irq chip
merily provides the set_wake() stuff and everything else is a noop.

So GPIO ends up as a hierarchy which is still not connected to the GIC
directly.

Hmm?

Thanks,

	tglx

Re: Multi-parent IRQ domains

[Sudeep Holla]

On Thu, Sep 13, 2018 at 05:59:45PM +0200, Thierry Reding wrote:
> Hi everyone,
>
> I've been trying to implement a feature on recent Tegra chips that's
> called "wake events".

By recent, is it same to assume PSCI based system ? And the GIC is power
managed by PSCI ?

--
Regards,
Sudeep

Re: Multi-parent IRQ domains

[Thierry Reding]

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

On Fri, Sep 14, 2018 at 12:31:18PM +0200, Thomas Gleixner wrote:
> On Thu, 13 Sep 2018, Thierry Reding wrote:
> > I've been trying to implement a feature on recent Tegra chips that's
> > called "wake events". These wake events are implemented as part of the
> > power management controller (PMC) and they control which events can be
> > used to wake the system from suspend. Events include things like an
> > alarm interrupt from an RTC, or the GPIO interrupt hooked up to a
> > power button for example.
> > 
> > We already have a driver for a couple of other things that the PMC does,
> > so I thought it'd be natural to implement this functionality as an IRQ
> > chip in the PMC driver. The way that this would work is that for all the
> > devices that are wakeup sources, we'd end up specifying the PMC as the
> > interrupt parent and the PMC would then itself have the main GIC as its
> > parent. That way, the hierarchical IRQ domain infrastructure would take
> > care of pretty much everything.
> > 
> > Unfortunately I've run into some issues here. One problem that I'm
> > facing is that PMC could have more than one parent. For example, the RTC
> > alarm interrupt goes straight to the GIC, but the power button GPIO goes
> > through the GPIO controller first and then to the GIC. This doesn't seem
> > to be something that's currently possible.
> 
> So what you are saying is that the RTC is directly wired up to the GIC and
> the GPIO interrupts are demultiplxed by a single GIC interrupt first, but
> at the same time have the requirement to talk to the PMC.

Yes, that's correct. Technically the GPIO controllers can have multiple
parent interrupts at the GIC, but I don't think that's relevant to the
discussion. Just mentioning it for completeness.

> Lets look at the current implementation without taking PMC into account.
> 
> - RTC gets its interrupt directly from the GIC domain
> 
> - GPIO gets its interrupt from the GPIO domain. The GPIO domain does not
>   have a parent domain. It is stand alone because it sets up a demultiplex
>   interrupt from the GIC domain.
> 
>   So there is absolutely no irqdomain relationship between a single GPIO
>   and the GIC. The indirection through the demultiplex interrupt does not
>   create one, really.
> 
> Now, you need the PMC for both, the GPIOs and the RTC. What you can do here
> is to provide two irq domains in PMC. One which has GIC as its parent and
> one which has no parent. Surely they need to share some resources, but
> that should be a solvable problem.

I think I have this working to some degree, finally. GPIO is still
proving difficult, but RTC seems to be working fine. I've currently
solved this by making the PMC an interrupt controller and then have
an interrupt-map property in its device tree node that lists those
wake events that we're interested in. It looks something like this:

	pmc: pmc@c360000 {
		compatible = "nvidia,tegra194-pmc";
		reg = <0x0c360000 0x10000>,
		      <0x0c370000 0x10000>,
		      <0x0c380000 0x10000>,
		      <0x0c390000 0x10000>,
		      <0x0c3a0000 0x10000>;
		reg-names = "pmc", "wake", "aotag", "scratch", "misc";

		#interrupt-cells = <1>;
		interrupt-controller;

		interrupt-map = /*<29 &gpio_aon TEGRA194_AON_GPIO(EE, 4)
					      IRQ_TYPE_LEVEL_HIGH>,*/
				<73 &gic GIC_SPI 10
					 IRQ_TYPE_LEVEL_HIGH>;
	};

Note that I've commented out the GPIO wake event (this is for the power
button) because that currently crashes in the GPIO driver, probably
because I misunderstood how to properly implement this.

> Now you connect RTC to the GIC parented one and let the PMC irq chip just
> hand through, except for the set_wake() functionality.

I've solved this in the following way:

	for (i = 0; i < pmc->num_wake_parents; i++) {
		struct tegra_wake_parent *parent;
		struct irq_chip *chip;

		parent = &pmc->wake_parents[i];

		chip = &parent->chip;
		chip->name = dev_name(pmc->dev);
		chip->irq_eoi = irq_chip_eoi_parent;
		chip->irq_mask = irq_chip_mask_parent;
		chip->irq_unmask = irq_chip_unmask_parent;
		chip->irq_retrigger = irq_chip_retrigger_hierarchy;
		chip->irq_set_wake = tegra_pmc_irq_set_wake;
		chip->irq_set_type = irq_chip_set_type_parent;
		chip->irq_set_affinity = irq_chip_set_affinity_parent;

		parent->domain = irq_domain_add_hierarchy(parent->domain, 0,
							  parent->num_events,
							  pmc->dev->of_node,
							  &tegra_pmc_irq_domain_ops,
							  pmc);
		if (!parent->domain) {
			dev_err(pmc->dev, "failed to allocate domain\n");
			return -ENOMEM;
		}
	}

The pmc->num_wake_parents is the number of unique wake parents that we
have in the interrupt-map property.

I think this is what you were suggesting, we override ->irq_set_wake()
and pass through everything else.

> The GPIO domain is connected to the parentless PMC domain and the irq chip
> merily provides the set_wake() stuff and everything else is a noop.
> 
> So GPIO ends up as a hierarchy which is still not connected to the GIC
> directly.

When I use the above code on the PMC/GPIO domain, then I see crashes
because the GPIO controller doesn't implement things like the ->alloc()
callback for its IRQ domain. But perhaps this is what I misunderstand.
Are you saying that for the case of GPIO I can just *not* pass through
all other operations and just let them be NULL? So that the only
callback will be ->irq_set_wake()? What I don't quite understand is how
the IRQ code would know how to properly set up the GPIO interrupt in
that case. For example, if I have this:

	gpio_aon: gpio@c2f0000 {
		...
	};

	pmc: pmc@c360000 {
		...
	};

	gpio-keys {
		...

		power {
			...
			gpios = <&gpio_aon TEGRA194_AON_GPIO(EE, 4)
					   GPIO_ACTIVE_LOW>;
			interrupt-parent = <&pmc>;
			interrupts = <29>;
			...
		};
	};

In the above, gpio-keys will only set up a handler for the PMC interrupt
and will therefore not process any interrupts from the GPIO line. If we
don't set up a parent/child relationship between PMC and GPIO, how does
the IRQ code know how to translate the PMC/29 interrupt to the one from
the GPIO controller? The PMC controller itself doesn't generate any
interrupts for wake events, so there's no interrupt handler from which
we could do any demultiplexing.

Thierry

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

Re: Multi-parent IRQ domains

[Thomas Gleixner]

On Mon, 17 Sep 2018, Thierry Reding wrote:
> On Fri, Sep 14, 2018 at 12:31:18PM +0200, Thomas Gleixner wrote:
> > Now, you need the PMC for both, the GPIOs and the RTC. What you can do here
> > is to provide two irq domains in PMC. One which has GIC as its parent and
> > one which has no parent. Surely they need to share some resources, but
> > that should be a solvable problem.
> 
> I think I have this working to some degree, finally. GPIO is still
> proving difficult, but RTC seems to be working fine. I've currently
> solved this by making the PMC an interrupt controller and then have
> an interrupt-map property in its device tree node that lists those
> wake events that we're interested in. It looks something like this:
> 
> 	pmc: pmc@c360000 {
> 		compatible = "nvidia,tegra194-pmc";
> 		reg = <0x0c360000 0x10000>,
> 		      <0x0c370000 0x10000>,
> 		      <0x0c380000 0x10000>,
> 		      <0x0c390000 0x10000>,
> 		      <0x0c3a0000 0x10000>;
> 		reg-names = "pmc", "wake", "aotag", "scratch", "misc";
> 
> 		#interrupt-cells = <1>;
> 		interrupt-controller;
> 
> 		interrupt-map = /*<29 &gpio_aon TEGRA194_AON_GPIO(EE, 4)
> 					      IRQ_TYPE_LEVEL_HIGH>,*/
> 				<73 &gic GIC_SPI 10
> 					 IRQ_TYPE_LEVEL_HIGH>;
> 	};
> 
> Note that I've commented out the GPIO wake event (this is for the power
> button) because that currently crashes in the GPIO driver, probably
> because I misunderstood how to properly implement this.

I'm not a DT wizerd, but the GPIO cannot be linked there I think.

    RTC ---------------------------> [ PMC domain 1] -----> GIC

    Button --> [GPIO domain] ------> [ PMC domain 2]

The RTC is connected to PMC domain 1 and that allocates the GIC irq.

The button is conntected to the GPIO which connect to the PMC domain
2. That PMC domain has no connection to anything. It ends there.

> When I use the above code on the PMC/GPIO domain, then I see crashes
> because the GPIO controller doesn't implement things like the ->alloc()
> callback for its IRQ domain. But perhaps this is what I misunderstand.
> Are you saying that for the case of GPIO I can just *not* pass through
> all other operations and just let them be NULL? So that the only
> callback will be ->irq_set_wake()? What I don't quite understand is how
> the IRQ code would know how to properly set up the GPIO interrupt in
> that case.

Let's look at the PMC level first:

  The PMC has a fixed number of interrupts which are avail

  domain 1:

       That domain is used for interrupts which have a dedicated GIC
       interrupt line, e.g. the RTC

       The interrupt domain needs at least:

       alloc		= pmc_domain1_alloc

       The interrupt chip has:

       irq_mask		= irq_chip_mask_parent
       irq_unmask	= irq_chip_unmask_parent
       irq_eoi		= irq_chip_eoi_parent
       irq_set_affinity	= irq_chip_set_affinity_parent
       irq_set_type	= irq_chip_set_type_parent
       irq_set_wake	= pmc_set_wake

  domain 2:

       That domain is used for interrupts which are not related to the GIC
       directly, e.g. GPIO

       The interrupt domain needs at least:

       alloc		= pmc_domain2_alloc
       
       The interrupt chip has:

       irq_set_wake	= pmc_set_wake


Now the GPIO domain builds on top of PMC domain2

    	i.e. the parent of the GPIO domain is PMC domain2, which means that
    	it's part of a hierarchy and therefore needs an alloc function in
    	the domain ops.

	The GPIO irq chip gains one extra callback:

       	irq_set_wake	= irq_chip_set_wake_parent,


So, I don't know how GPIOs are mapped into the PMC when they are a wakeup
source. It might be all of them have the ability so there is some 1:1
relation ship or if the whole GPIO -> PMC connection can be built at run
time, but that's just an implementation detail.

Thanks,

	tglx

Re: Multi-parent IRQ domains

[Thierry Reding]

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

On Mon, Sep 17, 2018 at 02:28:01PM +0200, Thomas Gleixner wrote:
> On Mon, 17 Sep 2018, Thierry Reding wrote:
> > On Fri, Sep 14, 2018 at 12:31:18PM +0200, Thomas Gleixner wrote:
> > > Now, you need the PMC for both, the GPIOs and the RTC. What you can do here
> > > is to provide two irq domains in PMC. One which has GIC as its parent and
> > > one which has no parent. Surely they need to share some resources, but
> > > that should be a solvable problem.
> > 
> > I think I have this working to some degree, finally. GPIO is still
> > proving difficult, but RTC seems to be working fine. I've currently
> > solved this by making the PMC an interrupt controller and then have
> > an interrupt-map property in its device tree node that lists those
> > wake events that we're interested in. It looks something like this:
> > 
> > 	pmc: pmc@c360000 {
> > 		compatible = "nvidia,tegra194-pmc";
> > 		reg = <0x0c360000 0x10000>,
> > 		      <0x0c370000 0x10000>,
> > 		      <0x0c380000 0x10000>,
> > 		      <0x0c390000 0x10000>,
> > 		      <0x0c3a0000 0x10000>;
> > 		reg-names = "pmc", "wake", "aotag", "scratch", "misc";
> > 
> > 		#interrupt-cells = <1>;
> > 		interrupt-controller;
> > 
> > 		interrupt-map = /*<29 &gpio_aon TEGRA194_AON_GPIO(EE, 4)
> > 					      IRQ_TYPE_LEVEL_HIGH>,*/
> > 				<73 &gic GIC_SPI 10
> > 					 IRQ_TYPE_LEVEL_HIGH>;
> > 	};
> > 
> > Note that I've commented out the GPIO wake event (this is for the power
> > button) because that currently crashes in the GPIO driver, probably
> > because I misunderstood how to properly implement this.
> 
> I'm not a DT wizerd, but the GPIO cannot be linked there I think.
> 
>     RTC ---------------------------> [ PMC domain 1] -----> GIC
> 
>     Button --> [GPIO domain] ------> [ PMC domain 2]
> 
> The RTC is connected to PMC domain 1 and that allocates the GIC irq.
> 
> The button is conntected to the GPIO which connect to the PMC domain
> 2. That PMC domain has no connection to anything. It ends there.

Interesting, I was assuming the following:

    Button --> [PMC domain 2] --> [GPIO domain]

based on the hardware documentation that maps the wake events to
specific signals on the chip.

> > When I use the above code on the PMC/GPIO domain, then I see crashes
> > because the GPIO controller doesn't implement things like the ->alloc()
> > callback for its IRQ domain. But perhaps this is what I misunderstand.
> > Are you saying that for the case of GPIO I can just *not* pass through
> > all other operations and just let them be NULL? So that the only
> > callback will be ->irq_set_wake()? What I don't quite understand is how
> > the IRQ code would know how to properly set up the GPIO interrupt in
> > that case.
> 
> Let's look at the PMC level first:
> 
>   The PMC has a fixed number of interrupts which are avail
> 
>   domain 1:
> 
>        That domain is used for interrupts which have a dedicated GIC
>        interrupt line, e.g. the RTC
> 
>        The interrupt domain needs at least:
> 
>        alloc		= pmc_domain1_alloc
> 
>        The interrupt chip has:
> 
>        irq_mask		= irq_chip_mask_parent
>        irq_unmask	= irq_chip_unmask_parent
>        irq_eoi		= irq_chip_eoi_parent
>        irq_set_affinity	= irq_chip_set_affinity_parent
>        irq_set_type	= irq_chip_set_type_parent
>        irq_set_wake	= pmc_set_wake
> 
>   domain 2:
> 
>        That domain is used for interrupts which are not related to the GIC
>        directly, e.g. GPIO
> 
>        The interrupt domain needs at least:
> 
>        alloc		= pmc_domain2_alloc
>        
>        The interrupt chip has:
> 
>        irq_set_wake	= pmc_set_wake
> 
> 
> Now the GPIO domain builds on top of PMC domain2
> 
>     	i.e. the parent of the GPIO domain is PMC domain2, which means that
>     	it's part of a hierarchy and therefore needs an alloc function in
>     	the domain ops.
> 
> 	The GPIO irq chip gains one extra callback:
> 
>        	irq_set_wake	= irq_chip_set_wake_parent,

This looks interesting. I'm going to give that a try, though it may take
me a little to rework everything.

> So, I don't know how GPIOs are mapped into the PMC when they are a wakeup
> source. It might be all of them have the ability so there is some 1:1
> relation ship or if the whole GPIO -> PMC connection can be built at run
> time, but that's just an implementation detail.

There's a fixed mapping for which signals go to which wake event. Some
of those events are mapped to GPIOs, others to different signals, such
as the RTC alarm. I think I should be able to build a GPIO -> PMC map
at runtime using a static table.

Thanks,
Thierry

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

Re: Multi-parent IRQ domains

[Thomas Gleixner]

On Mon, 17 Sep 2018, Thierry Reding wrote:
> On Mon, Sep 17, 2018 at 02:28:01PM +0200, Thomas Gleixner wrote:
> > I'm not a DT wizerd, but the GPIO cannot be linked there I think.
> > 
> >     RTC ---------------------------> [ PMC domain 1] -----> GIC
> > 
> >     Button --> [GPIO domain] ------> [ PMC domain 2]
> > 
> > The RTC is connected to PMC domain 1 and that allocates the GIC irq.
> > 
> > The button is conntected to the GPIO which connect to the PMC domain
> > 2. That PMC domain has no connection to anything. It ends there.
> 
> Interesting, I was assuming the following:
> 
>     Button --> [PMC domain 2] --> [GPIO domain]
> 
> based on the hardware documentation that maps the wake events to
> specific signals on the chip.

That might be, but there is no reason why we can't interpret that
creatively :)

> > So, I don't know how GPIOs are mapped into the PMC when they are a wakeup
> > source. It might be all of them have the ability so there is some 1:1
> > relation ship or if the whole GPIO -> PMC connection can be built at run
> > time, but that's just an implementation detail.
> 
> There's a fixed mapping for which signals go to which wake event. Some
> of those events are mapped to GPIOs, others to different signals, such
> as the RTC alarm. I think I should be able to build a GPIO -> PMC map
> at runtime using a static table.

Ok, so for the RTC and the like (which are directly GIC backed) it's easy
as you just say: RTC interrupt = PMC irq #N.

And for GPIO you can just have the GPIO domains all parented by the PMC
domain 2 and then have the allocation function of the PMC domain do the
lookup of the GPIOs which actually have a PMC connection.

If it's there then you set the hwirq in the domains irq_data to the real
PMC hwirq number and for those which are not connected you set it to some
invalid value. In the set_type() callback you can then error out when it's
called for a GPIO which is not connected to the PMC by checking the hwirq
value.

That way the GPIO code does not have any requirement to know about the PMC
connections and only the PMC domain 2 needs to know. So if in the next chip
the GPIO/PMC association changes, you don't have to worry about it at the
GPIO (driver) level, you just hand a different GPIO/PMC map to the PMC
driver via DT.

Thanks,

	tglx