* [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent
[not found] <1568411962-1022-1-git-send-email-ilina@codeaurora.org>
@ 2019-09-13 21:59 ` Lina Iyer
2019-10-03 12:02 ` Linus Walleij
2019-11-08 21:29 ` Doug Anderson
2019-09-13 21:59 ` [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register Lina Iyer
1 sibling, 2 replies; 10+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
To: swboyd, evgreen, maz, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
Lina Iyer, devicetree
Some interrupt controllers in a SoC, are always powered on and have a
select interrupts routed to them, so that they can wakeup the SoC from
suspend. Add wakeup-parent DT property to refer to these interrupt
controllers.
Cc: devicetree@vger.kernel.org
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/interrupt-controller/interrupts.txt | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
index 8a3c408..c10e310 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
@@ -108,3 +108,16 @@ commonly used:
sensitivity = <7>;
};
};
+
+3) Interrupt wakeup parent
+--------------------------
+
+Some interrupt controllers in a SoC, are always powered on and have a select
+interrupts routed to them, so that they can wakeup the SoC from suspend. These
+interrupt controllers do not fall into the category of a parent interrupt
+controller and can be specified by the "wakeup-parent" property and contain a
+single phandle referring to the wakeup capable interrupt controller.
+
+ Example:
+ wakeup-parent = <&pdc_intc>;
+
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
[not found] <1568411962-1022-1-git-send-email-ilina@codeaurora.org>
2019-09-13 21:59 ` [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent Lina Iyer
@ 2019-09-13 21:59 ` Lina Iyer
2019-09-30 22:14 ` Rob Herring
2019-09-30 22:33 ` Stephen Boyd
1 sibling, 2 replies; 10+ messages in thread
From: Lina Iyer @ 2019-09-13 21:59 UTC (permalink / raw)
To: swboyd, evgreen, maz, linus.walleij
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
Lina Iyer, devicetree
In addition to configuring the PDC, additional registers that interface
the GIC have to be configured to match the GPIO type. The registers on
some QCOM SoCs are access restricted, while on other SoCs are not. They
SoCs with access restriction to these SPI registers need to be written
from the firmware using the SCM interface. Add a flag to indicate if the
register is to be written using SCM interface.
Cc: devicetree@vger.kernel.org
Signed-off-by: Lina Iyer <ilina@codeaurora.org>
---
.../devicetree/bindings/interrupt-controller/qcom,pdc.txt | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
index 8e0797c..e329f8d 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
@@ -24,6 +24,9 @@ Properties:
Usage: required
Value type: <prop-encoded-array>
Definition: Specifies the base physical address for PDC hardware.
+ Optionally, specify the PDC's GIC interface registers that
+ need to be configured for wakeup capable GPIOs routed to
+ the PDC.
- interrupt-cells:
Usage: required
@@ -50,15 +53,23 @@ Properties:
The second element is the GIC hwirq number for the PDC port.
The third element is the number of interrupts in sequence.
+- qcom,scm-spi-cfg:
+ Usage: optional
+ Value type: <bool>
+ Definition: Specifies if the SPI configuration registers have to be
+ written from the firmware. Sometimes the PDC interface
+ register to the GIC can only be written from the firmware.
+
Example:
pdc: interrupt-controller@b220000 {
compatible = "qcom,sdm845-pdc";
- reg = <0xb220000 0x30000>;
+ reg = <0 0x0b220000 0 0x30000>, <0 0x179900f0 0 0x60>;
qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
#interrupt-cells = <2>;
interrupt-parent = <&intc>;
interrupt-controller;
+ qcom,scm-spi-cfg;
};
DT binding of a device that wants to use the GIC SPI 514 as a wakeup
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
2019-09-13 21:59 ` [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register Lina Iyer
@ 2019-09-30 22:14 ` Rob Herring
2019-09-30 22:33 ` Stephen Boyd
1 sibling, 0 replies; 10+ messages in thread
From: Rob Herring @ 2019-09-30 22:14 UTC (permalink / raw)
Cc: swboyd, evgreen, maz, linus.walleij, linux-kernel, linux-arm-msm,
bjorn.andersson, mkshah, linux-gpio, Lina Iyer, devicetree
On Fri, 13 Sep 2019 15:59:14 -0600, Lina Iyer wrote:
> In addition to configuring the PDC, additional registers that interface
> the GIC have to be configured to match the GPIO type. The registers on
> some QCOM SoCs are access restricted, while on other SoCs are not. They
> SoCs with access restriction to these SPI registers need to be written
> from the firmware using the SCM interface. Add a flag to indicate if the
> register is to be written using SCM interface.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
> .../devicetree/bindings/interrupt-controller/qcom,pdc.txt | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
2019-09-13 21:59 ` [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register Lina Iyer
2019-09-30 22:14 ` Rob Herring
@ 2019-09-30 22:33 ` Stephen Boyd
[not found] ` <5da6b849.1c69fb81.a9b04.1b9f@mx.google.com>
1 sibling, 1 reply; 10+ messages in thread
From: Stephen Boyd @ 2019-09-30 22:33 UTC (permalink / raw)
To: evgreen, linus.walleij, maz
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, mkshah, linux-gpio,
Lina Iyer, devicetree
Quoting Lina Iyer (2019-09-13 14:59:14)
> In addition to configuring the PDC, additional registers that interface
> the GIC have to be configured to match the GPIO type. The registers on
> some QCOM SoCs are access restricted, while on other SoCs are not. They
> SoCs with access restriction to these SPI registers need to be written
> from the firmware using the SCM interface. Add a flag to indicate if the
> register is to be written using SCM interface.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> ---
> .../devicetree/bindings/interrupt-controller/qcom,pdc.txt | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> index 8e0797c..e329f8d 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
> @@ -24,6 +24,9 @@ Properties:
> Usage: required
> Value type: <prop-encoded-array>
> Definition: Specifies the base physical address for PDC hardware.
> + Optionally, specify the PDC's GIC interface registers that
> + need to be configured for wakeup capable GPIOs routed to
> + the PDC.
>
> - interrupt-cells:
> Usage: required
> @@ -50,15 +53,23 @@ Properties:
> The second element is the GIC hwirq number for the PDC port.
> The third element is the number of interrupts in sequence.
>
> +- qcom,scm-spi-cfg:
> + Usage: optional
> + Value type: <bool>
> + Definition: Specifies if the SPI configuration registers have to be
> + written from the firmware. Sometimes the PDC interface
> + register to the GIC can only be written from the firmware.
> +
> Example:
>
> pdc: interrupt-controller@b220000 {
> compatible = "qcom,sdm845-pdc";
> - reg = <0xb220000 0x30000>;
> + reg = <0 0x0b220000 0 0x30000>, <0 0x179900f0 0 0x60>;
> qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
> #interrupt-cells = <2>;
> interrupt-parent = <&intc>;
> interrupt-controller;
> + qcom,scm-spi-cfg;
> };
This overlaps register region with the mailbox node. That node is
actually a pile of random "CPU" registers used to ping remote processors
and apparently control how the PDC interacts with the GIC. Maybe this
can be changed to a phandle and then the driver can interogate the
phandle to determine if it's the SCM firmware or if it's the shared
mailbox register? If it's a shared mailbox then it can write to it at
the offset it knows about (because it's sdm845 compatible specific) and
if it's SCM then it can use the hardcoded address as well?
Basically I'm saying that it just needs a phandle.
qcom,spi-cfg = <&scm>;
or
qcom,spi-cfg = <&mailbox>;
and then driver knows how to use that to write into random registers.
Maybe we can have an API in regmap that finds the regmap for a given
device node? That way we don't have to funnel everything through syscon
for this.
of_get_regmap(struct device_node *np, const char *name);
Where NULL name means "first available" and then do the devres search
otherwise for a device that has the matching node pointer.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent
2019-09-13 21:59 ` [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent Lina Iyer
@ 2019-10-03 12:02 ` Linus Walleij
2019-11-08 21:29 ` Doug Anderson
1 sibling, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2019-10-03 12:02 UTC (permalink / raw)
To: Lina Iyer
Cc: Stephen Boyd, Evan Green, Marc Zyngier,
linux-kernel@vger.kernel.org, MSM, Bjorn Andersson, mkshah,
open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
On Fri, Sep 13, 2019 at 11:59 PM Lina Iyer <ilina@codeaurora.org> wrote:
> Some interrupt controllers in a SoC, are always powered on and have a
> select interrupts routed to them, so that they can wakeup the SoC from
> suspend. Add wakeup-parent DT property to refer to these interrupt
> controllers.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
[not found] ` <5da6b849.1c69fb81.a9b04.1b9f@mx.google.com>
@ 2019-11-05 20:58 ` Lina Iyer
2019-11-06 0:53 ` Stephen Boyd
0 siblings, 1 reply; 10+ messages in thread
From: Lina Iyer @ 2019-11-05 20:58 UTC (permalink / raw)
To: Stephen Boyd
Cc: evgreen, linus.walleij, maz, linux-kernel, linux-arm-msm,
bjorn.andersson, mkshah, linux-gpio, devicetree
Sorry for the late reply.
On Tue, Oct 15 2019 at 00:27 -0600, Stephen Boyd wrote:
>Quoting Stephen Boyd (2019-09-30 15:33:01)
>> Quoting Lina Iyer (2019-09-13 14:59:14)
>> > In addition to configuring the PDC, additional registers that interface
>> > the GIC have to be configured to match the GPIO type. The registers on
>> > some QCOM SoCs are access restricted, while on other SoCs are not. They
>> > SoCs with access restriction to these SPI registers need to be written
>> > from the firmware using the SCM interface. Add a flag to indicate if the
>> > register is to be written using SCM interface.
>> >
>> > Cc: devicetree@vger.kernel.org
>> > Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>> > ---
>> > .../devicetree/bindings/interrupt-controller/qcom,pdc.txt | 13 ++++++++++++-
>> > 1 file changed, 12 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>> > index 8e0797c..e329f8d 100644
>> > --- a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>> > +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt
>> > @@ -24,6 +24,9 @@ Properties:
>> > Usage: required
>> > Value type: <prop-encoded-array>
>> > Definition: Specifies the base physical address for PDC hardware.
>> > + Optionally, specify the PDC's GIC interface registers that
>> > + need to be configured for wakeup capable GPIOs routed to
>> > + the PDC.
>> >
>> > - interrupt-cells:
>> > Usage: required
>> > @@ -50,15 +53,23 @@ Properties:
>> > The second element is the GIC hwirq number for the PDC port.
>> > The third element is the number of interrupts in sequence.
>> >
>> > +- qcom,scm-spi-cfg:
>> > + Usage: optional
>> > + Value type: <bool>
>> > + Definition: Specifies if the SPI configuration registers have to be
>> > + written from the firmware. Sometimes the PDC interface
>> > + register to the GIC can only be written from the firmware.
>> > +
>> > Example:
>> >
>> > pdc: interrupt-controller@b220000 {
>> > compatible = "qcom,sdm845-pdc";
>> > - reg = <0xb220000 0x30000>;
>> > + reg = <0 0x0b220000 0 0x30000>, <0 0x179900f0 0 0x60>;
>> > qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>;
>> > #interrupt-cells = <2>;
>> > interrupt-parent = <&intc>;
>> > interrupt-controller;
>> > + qcom,scm-spi-cfg;
>> > };
>>
>> This overlaps register region with the mailbox node. That node is
>> actually a pile of random "CPU" registers used to ping remote processors
>> and apparently control how the PDC interacts with the GIC. Maybe this
>> can be changed to a phandle and then the driver can interogate the
>> phandle to determine if it's the SCM firmware or if it's the shared
>> mailbox register? If it's a shared mailbox then it can write to it at
>> the offset it knows about (because it's sdm845 compatible specific) and
>> if it's SCM then it can use the hardcoded address as well?
>>
>> Basically I'm saying that it just needs a phandle.
>>
>> qcom,spi-cfg = <&scm>;
>>
>> or
>>
>> qcom,spi-cfg = <&mailbox>;
>>
>> and then driver knows how to use that to write into random registers.
>> Maybe we can have an API in regmap that finds the regmap for a given
>> device node? That way we don't have to funnel everything through syscon
>> for this.
>>
>> of_get_regmap(struct device_node *np, const char *name);
>>
>> Where NULL name means "first available" and then do the devres search
>> otherwise for a device that has the matching node pointer.
>>
>
>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.
But, we would still need to add syscon to the mailbox and then regmap
the registers for the interrupt contoller.
Thanks,
Lina
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
2019-11-05 20:58 ` Lina Iyer
@ 2019-11-06 0:53 ` Stephen Boyd
2019-11-11 18:37 ` Lina Iyer
0 siblings, 1 reply; 10+ messages in thread
From: Stephen Boyd @ 2019-11-06 0:53 UTC (permalink / raw)
To: Lina Iyer
Cc: evgreen, linus.walleij, maz, linux-kernel, linux-arm-msm,
bjorn.andersson, mkshah, linux-gpio, devicetree
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent
2019-09-13 21:59 ` [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent Lina Iyer
2019-10-03 12:02 ` Linus Walleij
@ 2019-11-08 21:29 ` Doug Anderson
1 sibling, 0 replies; 10+ messages in thread
From: Doug Anderson @ 2019-11-08 21:29 UTC (permalink / raw)
To: Lina Iyer
Cc: Stephen Boyd, Evan Green, maz, LinusW, LKML, linux-arm-msm,
Bjorn Andersson, mkshah, open list:GPIO SUBSYSTEM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi,
On Fri, Sep 13, 2019 at 3:00 PM Lina Iyer <ilina@codeaurora.org> wrote:
>
> Some interrupt controllers in a SoC, are always powered on and have a
> select interrupts routed to them, so that they can wakeup the SoC from
> suspend. Add wakeup-parent DT property to refer to these interrupt
> controllers.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> .../devicetree/bindings/interrupt-controller/interrupts.txt | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> index 8a3c408..c10e310 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> @@ -108,3 +108,16 @@ commonly used:
> sensitivity = <7>;
> };
> };
> +
> +3) Interrupt wakeup parent
> +--------------------------
> +
> +Some interrupt controllers in a SoC, are always powered on and have a select
> +interrupts routed to them, so that they can wakeup the SoC from suspend. These
> +interrupt controllers do not fall into the category of a parent interrupt
> +controller and can be specified by the "wakeup-parent" property and contain a
> +single phandle referring to the wakeup capable interrupt controller.
> +
> + Example:
> + wakeup-parent = <&pdc_intc>;
> +
nit: git flags the above line as whitespace damage. Please remove
if/when you spin.
Thanks!
-Doug
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
2019-11-06 0:53 ` Stephen Boyd
@ 2019-11-11 18:37 ` Lina Iyer
2019-11-12 11:52 ` Marc Zyngier
0 siblings, 1 reply; 10+ messages in thread
From: Lina Iyer @ 2019-11-11 18:37 UTC (permalink / raw)
To: Stephen Boyd
Cc: evgreen, linus.walleij, maz, linux-kernel, linux-arm-msm,
bjorn.andersson, mkshah, linux-gpio, devicetree
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register
2019-11-11 18:37 ` Lina Iyer
@ 2019-11-12 11:52 ` Marc Zyngier
0 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2019-11-12 11:52 UTC (permalink / raw)
To: Lina Iyer
Cc: Stephen Boyd, evgreen, linus.walleij, linux-kernel, linux-arm-msm,
bjorn.andersson, mkshah, linux-gpio, devicetree
On 2019-11-11 19:46, Lina Iyer wrote:
> 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.
That seems sensible: the two SoCs use different implementations of
their GPIO configurations (at least apparently, I'm pretty sure it
is the same HW underneath), and it makes sense to abstract that
as separate entities.
As for the DT binding, use whatever makes sense for you (compatible
seems a reasonable choice).
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-11-12 11:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1568411962-1022-1-git-send-email-ilina@codeaurora.org>
2019-09-13 21:59 ` [PATCH RFC v2 05/14] of: irq: document properties for wakeup interrupt parent Lina Iyer
2019-10-03 12:02 ` Linus Walleij
2019-11-08 21:29 ` Doug Anderson
2019-09-13 21:59 ` [PATCH RFC v2 06/14] dt-bindings/interrupt-controller: pdc: add SPI config register Lina Iyer
2019-09-30 22:14 ` Rob Herring
2019-09-30 22:33 ` Stephen Boyd
[not found] ` <5da6b849.1c69fb81.a9b04.1b9f@mx.google.com>
2019-11-05 20:58 ` Lina Iyer
2019-11-06 0:53 ` Stephen Boyd
2019-11-11 18:37 ` Lina Iyer
2019-11-12 11:52 ` Marc Zyngier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).