Re: [PATCH RFC 0/4] irqchip: qcom: add support for PDC interrupt controller

[Lina Iyer]

On Thu, Jan 25 2018 at 16:39 +0000, Sudeep Holla wrote:
> On 25/01/18 15:54, Lina Iyer wrote:
> > On Wed, Jan 24 2018 at 17:54 +0000, Sudeep Holla wrote:
> > > On 24/01/18 17:43, Lina Iyer wrote:
> > > > On Wed, Jan 24 2018 at 10:10 +0000, Sudeep Holla wrote:
> > > > > On 23/01/18 18:44, Lina Iyer wrote:
> > > > > > On Tue, Jan 23 2018 at 18:15 +0000, Sudeep Holla wrote:

> > Platform drivers leave their interrupts enabled at the GIC. Only
> > interrupts that are wakeup capable are of interest when the processor is
> > powered down. The remote processor (or f/w) will need to know the set of
> > wakeup capable interrupts and then configure the PDC by copying from
> > GIC. As mentioned earlier, it is simplified by letting Linux write to
> > the PDC reqisters directly.
>
> OK looks like I have not properly conveyed what I wanted to :(
> So lets take a peripheral A which has GIC interrupt X and a
> corresponding PDC interrupt Y.
>
> IIUC you want to configure Y from Linux while X is still enabled.
>
> 1. GICv3 masks all the interrupts(~X) that are not wakeup sources.
>   So when you say "platform drivers leave their interrupts enabled at
>   the GIC", it's not completely correct.
During idle path, the interrupts remain enabled. The GIC configuration
is retained, but the controller cannot sense interrupts because the
GIC logic is powered off. The PDC takes over for GIC at this time.

> 2. GIC CPU interface is disabled in firmware, so it's better to copy the
>   wakeup source to PDC just before that in the firmware.
>
> 3. Remote f/w must just know the mapping to PDC(X) for all the enabled
>   interrupts(Y) at the GIC and enable them accordingly at PDC. Is that
>   not what you have in the array in patch 4 ?
>
> I find above approach simpler instead of getting those wakeup
> interrupts defined per peripheral in DT. Further if there are any secure
> wakeup interrupts the firmware can also deal with that.
You assume that the remote processor is capable of doing all that. It is
better to de-centralize all this and have individual processors do the
work of configuring their wake up sources. We used to do that in earlier
SoCs but with SDM845, we moved to de-centralized model to reduce latency
and take the load off from time-critical idle path at the remote
processor. Dumping all this work into the firmware/PSCI is not desirable
either.

> > > > Yes. There is a partition and protected. So only permitted ELs can write
> > > > to the registers. This is done by the firmware at boot.
> > >
> > > Just for myself to understand better, so you have multiple partitions in
> > > PDC and one of them is given to EL1 or it just has one partition and
> > > that can be configured so that only permitted ELx is allowed to access
> > > it(in your case it's EL1)
> > Yes.
>
> Yes for the former or the latter :) ?
The latter.

Thanks,
Lina