How to represent active low ARM GIC interrupts, enabled by external inverter?

How to represent active low ARM GIC interrupts, enabled by external inverter?

[Stephen Warren]

I'd like guidance on how to model one aspect of Tegra's interrupt
structure.

Tegra has an interrupt input pin for use by a PMU chip.

The PMC HW module within Tegra can optionally invert this signal, but
otherwise has no control over it; no interrupt status bits, no masking,
etc.

The (potentially inverted) signal is then fed into the ARM GIC, which
supports level high or rising edge interrupts only.

So my question is: How to model this.

I assume the best thing to do is have the PMC be an explicit irq_chip,
and have its .set_type op configure the inversion based on whether
its interrupt source requests low or high, and allow either level or
edge.

However, the irq_chip for the PMC couldn't implement any mask or shutdown
operation. Is it acceptable to simple provide dummy/no-op functions in
that case? I see that kernel/irq/dummychip.c does exactly that, but I'm
not sure what use-cases that file is supposed to cover.

That'd result in something like this for device tree:

/ {
    intc: interrupt-controller at 50041000 {
        compatible = "arm,cortex-a9-gic";
        /* reg and other properties omitted for brevity throughout */
        interrupt-controller;
        /* 
         * cell 0, 1: interrupt type/ID
         * cell 2: Linux IRQ flags
         */
        #interrupt-cells = <3>;
    };
    interrupt-parent = <&intc>;

    pmc: interrupt-controller at 7000e400 {
        compatible = "nvidia,tegra20-pmc";
        interrupt-controller;
        /* 
         * cell 0: interrupt ID (always 0; should we omit this field?)
         * cell 1: 0=no invert, 1=invert
         */
        #interrupt-cells = <2>;
        /*
         * Always high. Level or edge of the PMU interrupt output must be
         * configured here. Or, should it somehow be passed through from
         * the PMU's client's interrupt specifier?
         */
        interrupts = <0 88 0x04>;
    };

    i2c at 7000c000 {
        compatible = "nvidia,tegra20-i2c";

        pmu at xxx {
            interrupt-parent = <&pmc>;
            interrupts = <0 1>;
        }
    };
};

Does that look reasonable?

Other alternatives:

1) Don't hook the PMC driver and binding into the interrupt tree, but
simply have a property that controls the inversion of the signal.
The disadvantage is that the PMU's interrupt specifier would need to
always specify active high even if it was really active low with inversion
activated in the PMC.

2) Modify the ARM GIC's driver to allow active low interrupts, and call
some plugin code to request any required inversion. Most platforms
wouldn't provide such a plugin, and hence would still disallow such
requests. The disadvantage here is that it complicates the cross-SoC
GIC driver with something that may only be useful for Tegra.

Thanks.

-- 
nvpublic

How to represent active low ARM GIC interrupts, enabled by external inverter?

[Russell King - ARM Linux]

On Mon, Jan 23, 2012 at 01:18:50PM -0800, Stephen Warren wrote:
> I'd like guidance on how to model one aspect of Tegra's interrupt
> structure.
> 
> Tegra has an interrupt input pin for use by a PMU chip.
> 
> The PMC HW module within Tegra can optionally invert this signal, but
> otherwise has no control over it; no interrupt status bits, no masking,
> etc.
> 
> The (potentially inverted) signal is then fed into the ARM GIC, which
> supports level high or rising edge interrupts only.

When I originally added the IRQ trigger types to request_irq(), the
original intention was for these to be specified by the requesting
driver according to what the chip required, and there to be some kind
of hook to deal with the inversion which happens on some boards.

However, that was never implemented because there was a far better
solution: pass this information to the driver via the device resource
structures, which already have this information defined for PNP
devices.  The driver would then find the IRQ resource which would
tell it both the number and the IRQ trigger flags to be used.

Some drivers have done this, to cope with differing wiring on boards.
I don't know if DT supports this though, but I'd suggest that it's
a solution worth investigating.

How to represent active low ARM GIC interrupts, enabled by external inverter?

[Rob Herring]

On 01/23/2012 04:33 PM, Russell King - ARM Linux wrote:
> On Mon, Jan 23, 2012 at 01:18:50PM -0800, Stephen Warren wrote:
>> I'd like guidance on how to model one aspect of Tegra's interrupt
>> structure.
>>
>> Tegra has an interrupt input pin for use by a PMU chip.
>>
>> The PMC HW module within Tegra can optionally invert this signal, but
>> otherwise has no control over it; no interrupt status bits, no masking,
>> etc.
>>
>> The (potentially inverted) signal is then fed into the ARM GIC, which
>> supports level high or rising edge interrupts only.

Stephen, isn't this the same question you posted before except where the
inverter is is different?

http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-December/009880.html

> 
> When I originally added the IRQ trigger types to request_irq(), the
> original intention was for these to be specified by the requesting
> driver according to what the chip required, and there to be some kind
> of hook to deal with the inversion which happens on some boards.
> 
> However, that was never implemented because there was a far better
> solution: pass this information to the driver via the device resource
> structures, which already have this information defined for PNP
> devices.  The driver would then find the IRQ resource which would
> tell it both the number and the IRQ trigger flags to be used.
> 
> Some drivers have done this, to cope with differing wiring on boards.
> I don't know if DT supports this though, but I'd suggest that it's
> a solution worth investigating.
> 

As my suggestion was the same approach as Russell's.

Rob

How to represent active low ARM GIC interrupts, enabled by external inverter?

[Stephen Warren]

Russell King wrote at Monday, January 23, 2012 3:33 PM:
> On Mon, Jan 23, 2012 at 01:18:50PM -0800, Stephen Warren wrote:
> > I'd like guidance on how to model one aspect of Tegra's interrupt
> > structure.
> >
> > Tegra has an interrupt input pin for use by a PMU chip.
> >
> > The PMC HW module within Tegra can optionally invert this signal, but
> > otherwise has no control over it; no interrupt status bits, no masking,
> > etc.
> >
> > The (potentially inverted) signal is then fed into the ARM GIC, which
> > supports level high or rising edge interrupts only.
> 
> When I originally added the IRQ trigger types to request_irq(), the
> original intention was for these to be specified by the requesting
> driver according to what the chip required, and there to be some kind
> of hook to deal with the inversion which happens on some boards.
> 
> However, that was never implemented because there was a far better
> solution: pass this information to the driver via the device resource
> structures, which already have this information defined for PNP
> devices.  The driver would then find the IRQ resource which would
> tell it both the number and the IRQ trigger flags to be used.
> 
> Some drivers have done this, to cope with differing wiring on boards.
> I don't know if DT supports this though, but I'd suggest that it's
> a solution worth investigating.

Yes, that's basically exactly what the primary proposal was in my original
email. The issues are:

* Whether it's possible for the Tegra PMC to be represented as an irq_chip
since the only control it has is inversion; no status register or masking.

* The interrupt specifiers in the PMU device and PMC device both have to
match w.r.t. edge or level selection, which might be confusing.

-- 
nvpublic

How to represent active low ARM GIC interrupts, enabled by external inverter?

[Stephen Warren]

Rob Herring wrote at Monday, January 23, 2012 4:13 PM:
> On 01/23/2012 04:33 PM, Russell King - ARM Linux wrote:
> > On Mon, Jan 23, 2012 at 01:18:50PM -0800, Stephen Warren wrote:
> >> I'd like guidance on how to model one aspect of Tegra's interrupt
> >> structure.
> >>
> >> Tegra has an interrupt input pin for use by a PMU chip.
> >>
> >> The PMC HW module within Tegra can optionally invert this signal, but
> >> otherwise has no control over it; no interrupt status bits, no masking,
> >> etc.
> >>
> >> The (potentially inverted) signal is then fed into the ARM GIC, which
> >> supports level high or rising edge interrupts only.
> 
> Stephen, isn't this the same question you posted before except where the
> inverter is is different?
> 
> http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-December/009880.html

That is somewhat similar, but I don't think quite the same:

* That thread deals with a single interrupt source and a single sink, which
need to negotiate to use one of n possible interrupt types. Whereas this
thread is about a sink connected to a source+sink connected to a sink.

* That thread dealt with negotiating one of n supported types, whereas
this thread is about a restricted sink (GIC only supporting high types),
connecting through an intermediary to a source, and I have no idea whether
that source is flexible and can support both high or low types; I assume
it can only support low types.

I suppose if the auto-negotiation logic were enhanced to jump across n
levels instead of just 1 link, it might be applicable. That sounds complex
though.

-- 
nvpublic

How to represent active low ARM GIC interrupts, enabled by external inverter?

[Rob Herring]

On 01/23/2012 03:18 PM, Stephen Warren wrote:
> I'd like guidance on how to model one aspect of Tegra's interrupt
> structure.
> 
> Tegra has an interrupt input pin for use by a PMU chip.
> 
> The PMC HW module within Tegra can optionally invert this signal, but
> otherwise has no control over it; no interrupt status bits, no masking,
> etc.
> 
> The (potentially inverted) signal is then fed into the ARM GIC, which
> supports level high or rising edge interrupts only.
> 
> So my question is: How to model this.
> 
> I assume the best thing to do is have the PMC be an explicit irq_chip,
> and have its .set_type op configure the inversion based on whether
> its interrupt source requests low or high, and allow either level or
> edge.
> 
> However, the irq_chip for the PMC couldn't implement any mask or shutdown
> operation. Is it acceptable to simple provide dummy/no-op functions in
> that case? I see that kernel/irq/dummychip.c does exactly that, but I'm
> not sure what use-cases that file is supposed to cover.
> 
> That'd result in something like this for device tree:
> 
> / {
>     intc: interrupt-controller at 50041000 {
>         compatible = "arm,cortex-a9-gic";
>         /* reg and other properties omitted for brevity throughout */
>         interrupt-controller;
>         /* 
>          * cell 0, 1: interrupt type/ID
>          * cell 2: Linux IRQ flags
>          */
>         #interrupt-cells = <3>;
>     };
>     interrupt-parent = <&intc>;
> 
>     pmc: interrupt-controller at 7000e400 {
>         compatible = "nvidia,tegra20-pmc";
>         interrupt-controller;
>         /* 
>          * cell 0: interrupt ID (always 0; should we omit this field?)
>          * cell 1: 0=no invert, 1=invert
>          */
>         #interrupt-cells = <2>;
>         /*
>          * Always high. Level or edge of the PMU interrupt output must be
>          * configured here. Or, should it somehow be passed through from
>          * the PMU's client's interrupt specifier?
>          */
>         interrupts = <0 88 0x04>;
>     };
> 
>     i2c at 7000c000 {
>         compatible = "nvidia,tegra20-i2c";
> 
>         pmu at xxx {
>             interrupt-parent = <&pmc>;
>             interrupts = <0 1>;
>         }
>     };
> };
> 
> Does that look reasonable?
> 
> Other alternatives:
> 
> 1) Don't hook the PMC driver and binding into the interrupt tree, but
> simply have a property that controls the inversion of the signal.
> The disadvantage is that the PMU's interrupt specifier would need to
> always specify active high even if it was really active low with inversion
> activated in the PMC.

I think i would go this route considering it's only a single interrupt
line and probably just a couple of lines of code to set a bit if a
property is present.

Rob

> 2) Modify the ARM GIC's driver to allow active low interrupts, and call
> some plugin code to request any required inversion. Most platforms
> wouldn't provide such a plugin, and hence would still disallow such
> requests. The disadvantage here is that it complicates the cross-SoC
> GIC driver with something that may only be useful for Tegra.
> 
> Thanks.
> 

How to represent active low ARM GIC interrupts, enabled by external inverter?

[Stephen Warren]

Rob Herring wrote at Monday, January 23, 2012 4:48 PM:
> On 01/23/2012 03:18 PM, Stephen Warren wrote:
> > I'd like guidance on how to model one aspect of Tegra's interrupt
> > structure.
> >
> > Tegra has an interrupt input pin for use by a PMU chip.
> >
> > The PMC HW module within Tegra can optionally invert this signal, but
> > otherwise has no control over it; no interrupt status bits, no masking,
> > etc.
> >
> > The (potentially inverted) signal is then fed into the ARM GIC, which
> > supports level high or rising edge interrupts only.
> >
> > So my question is: How to model this.
> >
> > I assume the best thing to do is have the PMC be an explicit irq_chip,
> > and have its .set_type op configure the inversion based on whether
> > its interrupt source requests low or high, and allow either level or
> > edge.
> >
> > However, the irq_chip for the PMC couldn't implement any mask or shutdown
> > operation. Is it acceptable to simple provide dummy/no-op functions in
> > that case? I see that kernel/irq/dummychip.c does exactly that, but I'm
> > not sure what use-cases that file is supposed to cover.
> >
> > That'd result in something like this for device tree:
...
> > Other alternatives:
> >
> > 1) Don't hook the PMC driver and binding into the interrupt tree, but
> > simply have a property that controls the inversion of the signal.
> > The disadvantage is that the PMU's interrupt specifier would need to
> > always specify active high even if it was really active low with inversion
> > activated in the PMC.
> 
> I think i would go this route considering it's only a single interrupt
> line and probably just a couple of lines of code to set a bit if a
> property is present.

True, that's very easy to implement.

One other issue I see with this approach: the PMC driver must initialize
before the PMU driver, so that it has reprogrammed the interrupt signal
inverter before the PMU driver requests and enables the interrupt. If not,
then the interrupt will scream as soon as the PMU driver enables it. If
PMC were an explicit interrupt controller, that'd feed into the deferred
device probing feature pretty easily to solve this. Otherwise, how would
we solve this? I suppose we could do something similar to the explicit
GIC initialization and force the PMC device to be instantiated before
instantiating anything else from device tree. Does that seem reasonable?

--
nvpublic

How to represent active low ARM GIC interrupts, enabled by external inverter?

[Grant Likely]

On Wed, Jan 25, 2012 at 10:34 AM, Stephen Warren <swarren@nvidia.com> wrote:
> Rob Herring wrote at Monday, January 23, 2012 4:48 PM:
>> On 01/23/2012 03:18 PM, Stephen Warren wrote:
>> > I'd like guidance on how to model one aspect of Tegra's interrupt
>> > structure.
>> >
>> > Tegra has an interrupt input pin for use by a PMU chip.
>> >
>> > The PMC HW module within Tegra can optionally invert this signal, but
>> > otherwise has no control over it; no interrupt status bits, no masking,
>> > etc.
>> >
>> > The (potentially inverted) signal is then fed into the ARM GIC, which
>> > supports level high or rising edge interrupts only.
>> >
>> > So my question is: How to model this.
>> >
>> > I assume the best thing to do is have the PMC be an explicit irq_chip,
>> > and have its .set_type op configure the inversion based on whether
>> > its interrupt source requests low or high, and allow either level or
>> > edge.
>> >
>> > However, the irq_chip for the PMC couldn't implement any mask or shutdown
>> > operation. Is it acceptable to simple provide dummy/no-op functions in
>> > that case? I see that kernel/irq/dummychip.c does exactly that, but I'm
>> > not sure what use-cases that file is supposed to cover.
>> >
>> > That'd result in something like this for device tree:
> ...
>> > Other alternatives:
>> >
>> > 1) Don't hook the PMC driver and binding into the interrupt tree, but
>> > simply have a property that controls the inversion of the signal.
>> > The disadvantage is that the PMU's interrupt specifier would need to
>> > always specify active high even if it was really active low with inversion
>> > activated in the PMC.
>>
>> I think i would go this route considering it's only a single interrupt
>> line and probably just a couple of lines of code to set a bit if a
>> property is present.
>
> True, that's very easy to implement.
>
> One other issue I see with this approach: the PMC driver must initialize
> before the PMU driver, so that it has reprogrammed the interrupt signal
> inverter before the PMU driver requests and enables the interrupt. If not,
> then the interrupt will scream as soon as the PMU driver enables it. If
> PMC were an explicit interrupt controller, that'd feed into the deferred
> device probing feature pretty easily to solve this. Otherwise, how would
> we solve this? I suppose we could do something similar to the explicit
> GIC initialization and force the PMC device to be instantiated before
> instantiating anything else from device tree. Does that seem reasonable?

This probably makes the most sense.  There are always going to be core
SoC setup that just have to be part of the early init setup before
starting the generic driver initialization. Those things don't fit
well into the device model. This may just be one where the right thing
to do is make sure it is set up at or before of_irq_init() gets
called.

g.