Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register

[Lina Iyer]

On Tue, Nov 05 2019 at 17:53 -0700, Stephen Boyd wrote:
> Quoting Lina Iyer (2019-11-05 12:58:32)
> > On Tue, Oct 15 2019 at 00:27 -0600, Stephen Boyd wrote:
> > >
> > >I had another idea the other day. Maybe a better approach would be to
> > >make the mailbox or SCM code an interrupt controller with the
> > >appropriate functions to poke the bits necessary to make the interrupts
> > >work. Then we can make it a chip in the hierarchy between the GIC and
> > >PDC and make the interrupts call through from PDC to GIC. The locking
> > >could be handled in each respective driver if necessary, and otherwise
> > >we don't have to use a regmap or remap the same registers (except we may
> > >need to describe if the parent is the mailbox node or the scm fimware
> > >node).
> > >
> > Wouldn't that be a stretch to image the SCM register write  or a random
> > register write as an interrupt controller? But I agree that it solves
> > the issue of determining whether we want to use SCM or regmap.
>
> As far as I can tell it's similar to PDC which is basically a gate on
> the line from a dedicated chip pad or a GPIO pad that lets the interrupt
> flow through to the GIC or not. Isn't this yet another hardware block on
> those paths that control the edge type or something?
>
> > But, we would still need to add syscon to the mailbox and then regmap
> > the registers for the interrupt contoller.
>
> I'm saying that we can make the mailbox driver an interrupt controller
> driver too. Or if that doesn't work, we can map the region twice in each
> driver with ioremap and cross fingers that they don't touch the same
> register at the same time. It sounds like that is the case. We won't be
> able to fancily reserve the register region and map it in one function
> call, but maybe that can be fixed by limiting the size or offset that is
> reserved for each driver manually based on the same register property
> that's described in DT. Basically, one node in DT
>
> mailbox@f00 {
>   reg = <0xf00 0x1000>;
> };
>
> And then each driver will ioremap() the whole register region that's
> parsed from DT but each driver will mark sub-regions as reserved for the
> respective driver. That way we don't have to worry about using a regmap
> here and we'll still know what drivers are using what regions of IO in
> /proc/iomem.

Marc: What do you think of Stephen's idea? Summarizing my understanding
below -

We need to set an addition register for GPIOs that are routed to PDC and
the register may need to be written using a SCM call (SDM845) or written
from Linux (SDM855). The idea proposed here is -
Create multiple irqchips, one for each type of register access and then
put them in hierarchy based on the target.

SDM845:
TLMM  --> PDC  --> PDC-SCM-IF  --> GIC

SDM855:
TLMM  --> PDC  --> PDC-LNX-IF  --> GIC

The hierarchy would be explicit from the DT. So we would not have to
worry about figuring out using a property in DT or resource name. (May
be we can use a compatible instead?). The use of reserved_resource(),
suggested by Stephen, would help avoid other drivers writing to this
register which is part of a generic dump area for one-off registers.

--Lina