[v2,1/3] dt-bindings: arm: optee: add interrupt controller properties

Message ID 20230124105643.1737250-1-etienne.carriere@linaro.org
State New
Headers
Series [v2,1/3] dt-bindings: arm: optee: add interrupt controller properties |

Commit Message

Etienne Carriere Jan. 24, 2023, 10:56 a.m. UTC
  Adds an optional interrupt controller property to optee firmware node
in the DT bindings. Optee driver may embeds an irqchip exposing
interrupts notified by the TEE world. Optee registers up to 1 interrupt
controller and identifies each line with a line number from 0 to
UINT16_MAX.

In the example, the platform SCMI device uses optee interrupt irq 5
as async signal to trigger processing of an async incoming SCMI message,
in the scope of a CPU DVFS control. A platform can have several SCMI
channels driven this way. Optee irqs also permits small embedded devices
to share e.g. a gpio expander, a group of wakeup sources, etc... between
OP-TEE world (for sensitive services) and Linux world (for non-sensitive
services). The physical controller is driven from the TEE which exposes
some controls to Linux kernel.

Cc: Jens Wiklander <jens.wiklander@linaro.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Sumit Garg <sumit.garg@linaro.org>

Co-developed-by: Pascal Paillet <p.paillet@foss.st.com>
Signed-off-by: Pascal Paillet <p.paillet@foss.st.com>
Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes since v1:
- Added a description to #interrupt-cells property.
- Changed of example. Linux wakeup event was subject to discussion and
  i don't know much about input events in Linux. So move to SCMI.
  In the example, an SCMI server in OP-TEE world raises optee irq 5
  so that Linux scmi optee channel &scmi_cpu_dvfs pushed in the incoming
  SCMI message in the scmi device for liekly later processing in threaded
  context. The example includes all parties: optee, scmi, sram, gic.
- Obviously rephrased the commit message.
- Added Cc: tags
---
 .../arm/firmware/linaro,optee-tz.yaml         | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
  

Comments

Rob Herring Jan. 25, 2023, 8:22 p.m. UTC | #1
+Sudeep

On Tue, Jan 24, 2023 at 11:56:41AM +0100, Etienne Carriere wrote:
> Adds an optional interrupt controller property to optee firmware node
> in the DT bindings. Optee driver may embeds an irqchip exposing
> interrupts notified by the TEE world. Optee registers up to 1 interrupt
> controller and identifies each line with a line number from 0 to
> UINT16_MAX.
> 
> In the example, the platform SCMI device uses optee interrupt irq 5
> as async signal to trigger processing of an async incoming SCMI message,
> in the scope of a CPU DVFS control. A platform can have several SCMI
> channels driven this way. Optee irqs also permits small embedded devices
> to share e.g. a gpio expander, a group of wakeup sources, etc... between
> OP-TEE world (for sensitive services) and Linux world (for non-sensitive
> services). The physical controller is driven from the TEE which exposes
> some controls to Linux kernel.
> 
> Cc: Jens Wiklander <jens.wiklander@linaro.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> 
> Co-developed-by: Pascal Paillet <p.paillet@foss.st.com>
> Signed-off-by: Pascal Paillet <p.paillet@foss.st.com>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> Changes since v1:
> - Added a description to #interrupt-cells property.
> - Changed of example. Linux wakeup event was subject to discussion and
>   i don't know much about input events in Linux. So move to SCMI.
>   In the example, an SCMI server in OP-TEE world raises optee irq 5
>   so that Linux scmi optee channel &scmi_cpu_dvfs pushed in the incoming
>   SCMI message in the scmi device for liekly later processing in threaded
>   context. The example includes all parties: optee, scmi, sram, gic.
> - Obviously rephrased the commit message.
> - Added Cc: tags
> ---
>  .../arm/firmware/linaro,optee-tz.yaml         | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> index d4dc0749f9fd..9c00c27f8b2c 100644
> --- a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> +++ b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> @@ -40,6 +40,14 @@ properties:
>        HVC #0, register assignments
>        register assignments are specified in drivers/tee/optee/optee_smc.h
>  
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 1
> +    description: |
> +      OP-TEE exposes irq for irp chip controllers from OP-TEE world. Each
> +      irq is assigned a single line number identifier used as first argument.
> +
>  required:
>    - compatible
>    - method
> @@ -64,3 +72,62 @@ examples:
>              method = "hvc";
>          };
>      };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    firmware  {
> +        optee: optee {
> +            compatible = "linaro,optee-tz";
> +            method = "smc";
> +            interrupts = <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>;
> +            interrupt-controller;
> +            #interrupt-cells = <1>;
> +        };
> +
> +        scmi {
> +            compatible = "linaro,scmi-optee";
> +            linaro,optee-channel-id = <0>;
> +            interrupt-parent = <&gic>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            scmi_cpu_dvfs: protocol@13 {
> +                reg = <0x13>;
> +                linaro,optee-channel-id = <1>;
> +                shmem = <&scmi_shm_tx>, <&scmi_shm_rx>;
> +                interrupts-extended = <&optee 5>;
> +                interrupt-names = "a2p";

These properties aren't documented. Soon there will be a warning[1]. 

> +                #clock-cells = <1>;
> +            };
> +        };
> +    };
> +
> +    gic: interrupt-controller@a0021000 {
> +        compatible = "arm,cortex-a7-gic";
> +        reg = <0xa0021000 0x1000>, <0xa0022000 0x2000>;
> +        interrupt-controller;
> +        #interrupt-cells = <3>;
> +    };
> +
> +    soc {
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        sram@2ffff000 {
> +            compatible = "mmio-sram";
> +            reg = <0x2ffff000 0x1000>;
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +            ranges = <0 0x2ffff000 0x1000>;
> +
> +            scmi_shm_tx: scmi-sram@0 {
> +                compatible = "arm,scmi-shmem";
> +                reg = <0 0x80>;
> +            };
> +
> +            scmi_shm_rx: scmi-sram@100 {
> +                compatible = "arm,scmi-shmem";
> +                reg = <0x100 0x80>;
> +            };

There's no need to show providers in examples (unless the example is for 
the provider).

Rob
  
Etienne Carriere Jan. 25, 2023, 9 p.m. UTC | #2
On Wed, 25 Jan 2023 at 21:22, Rob Herring <robh@kernel.org> wrote:
>
> +Sudeep
>
> On Tue, Jan 24, 2023 at 11:56:41AM +0100, Etienne Carriere wrote:
> > Adds an optional interrupt controller property to optee firmware node
> > in the DT bindings. Optee driver may embeds an irqchip exposing
> > interrupts notified by the TEE world. Optee registers up to 1 interrupt
> > controller and identifies each line with a line number from 0 to
> > UINT16_MAX.
> >
> > In the example, the platform SCMI device uses optee interrupt irq 5
> > as async signal to trigger processing of an async incoming SCMI message,
> > in the scope of a CPU DVFS control. A platform can have several SCMI
> > channels driven this way. Optee irqs also permits small embedded devices
> > to share e.g. a gpio expander, a group of wakeup sources, etc... between
> > OP-TEE world (for sensitive services) and Linux world (for non-sensitive
> > services). The physical controller is driven from the TEE which exposes
> > some controls to Linux kernel.
> >
> > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Sumit Garg <sumit.garg@linaro.org>
> >
> > Co-developed-by: Pascal Paillet <p.paillet@foss.st.com>
> > Signed-off-by: Pascal Paillet <p.paillet@foss.st.com>
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > ---
> > Changes since v1:
> > - Added a description to #interrupt-cells property.
> > - Changed of example. Linux wakeup event was subject to discussion and
> >   i don't know much about input events in Linux. So move to SCMI.
> >   In the example, an SCMI server in OP-TEE world raises optee irq 5
> >   so that Linux scmi optee channel &scmi_cpu_dvfs pushed in the incoming
> >   SCMI message in the scmi device for liekly later processing in threaded
> >   context. The example includes all parties: optee, scmi, sram, gic.
> > - Obviously rephrased the commit message.
> > - Added Cc: tags
> > ---
> >  .../arm/firmware/linaro,optee-tz.yaml         | 67 +++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > index d4dc0749f9fd..9c00c27f8b2c 100644
> > --- a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > +++ b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > @@ -40,6 +40,14 @@ properties:
> >        HVC #0, register assignments
> >        register assignments are specified in drivers/tee/optee/optee_smc.h
> >
> > +  interrupt-controller: true
> > +
> > +  "#interrupt-cells":
> > +    const: 1
> > +    description: |
> > +      OP-TEE exposes irq for irp chip controllers from OP-TEE world. Each
> > +      irq is assigned a single line number identifier used as first argument.
> > +
> >  required:
> >    - compatible
> >    - method
> > @@ -64,3 +72,62 @@ examples:
> >              method = "hvc";
> >          };
> >      };
> > +
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    firmware  {
> > +        optee: optee {
> > +            compatible = "linaro,optee-tz";
> > +            method = "smc";
> > +            interrupts = <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>;
> > +            interrupt-controller;
> > +            #interrupt-cells = <1>;
> > +        };
> > +
> > +        scmi {
> > +            compatible = "linaro,scmi-optee";
> > +            linaro,optee-channel-id = <0>;
> > +            interrupt-parent = <&gic>;
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            scmi_cpu_dvfs: protocol@13 {
> > +                reg = <0x13>;
> > +                linaro,optee-channel-id = <1>;
> > +                shmem = <&scmi_shm_tx>, <&scmi_shm_rx>;
> > +                interrupts-extended = <&optee 5>;
> > +                interrupt-names = "a2p";
>
> These properties aren't documented. Soon there will be a warning[1].

They are.
https://github.com/torvalds/linux/blob/v6.1/Documentation/devicetree/bindings/firmware/arm%2Cscmi.yaml#L45-L53

In arm,scmi.yaml, interrupts optional property stands for interrupts
and interrupts-extended, no?


>
> > +                #clock-cells = <1>;
> > +            };
> > +        };
> > +    };
> > +
> > +    gic: interrupt-controller@a0021000 {
> > +        compatible = "arm,cortex-a7-gic";
> > +        reg = <0xa0021000 0x1000>, <0xa0022000 0x2000>;
> > +        interrupt-controller;
> > +        #interrupt-cells = <3>;
> > +    };
> > +
> > +    soc {
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +
> > +        sram@2ffff000 {
> > +            compatible = "mmio-sram";
> > +            reg = <0x2ffff000 0x1000>;
> > +            #address-cells = <1>;
> > +            #size-cells = <1>;
> > +            ranges = <0 0x2ffff000 0x1000>;
> > +
> > +            scmi_shm_tx: scmi-sram@0 {
> > +                compatible = "arm,scmi-shmem";
> > +                reg = <0 0x80>;
> > +            };
> > +
> > +            scmi_shm_rx: scmi-sram@100 {
> > +                compatible = "arm,scmi-shmem";
> > +                reg = <0x100 0x80>;
> > +            };
>
> There's no need to show providers in examples (unless the example is for
> the provider).

Ok, i'll simplify the example

Thanks,
Etienne

>
> Rob
  
Rob Herring Jan. 26, 2023, 2 p.m. UTC | #3
On Wed, Jan 25, 2023 at 3:00 PM Etienne Carriere
<etienne.carriere@linaro.org> wrote:
>
> On Wed, 25 Jan 2023 at 21:22, Rob Herring <robh@kernel.org> wrote:
> >
> > +Sudeep
> >
> > On Tue, Jan 24, 2023 at 11:56:41AM +0100, Etienne Carriere wrote:
> > > Adds an optional interrupt controller property to optee firmware node
> > > in the DT bindings. Optee driver may embeds an irqchip exposing
> > > interrupts notified by the TEE world. Optee registers up to 1 interrupt
> > > controller and identifies each line with a line number from 0 to
> > > UINT16_MAX.
> > >
> > > In the example, the platform SCMI device uses optee interrupt irq 5
> > > as async signal to trigger processing of an async incoming SCMI message,
> > > in the scope of a CPU DVFS control. A platform can have several SCMI
> > > channels driven this way. Optee irqs also permits small embedded devices
> > > to share e.g. a gpio expander, a group of wakeup sources, etc... between
> > > OP-TEE world (for sensitive services) and Linux world (for non-sensitive
> > > services). The physical controller is driven from the TEE which exposes
> > > some controls to Linux kernel.
> > >
> > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > > Cc: Marc Zyngier <maz@kernel.org>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > >
> > > Co-developed-by: Pascal Paillet <p.paillet@foss.st.com>
> > > Signed-off-by: Pascal Paillet <p.paillet@foss.st.com>
> > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > > ---
> > > Changes since v1:
> > > - Added a description to #interrupt-cells property.
> > > - Changed of example. Linux wakeup event was subject to discussion and
> > >   i don't know much about input events in Linux. So move to SCMI.
> > >   In the example, an SCMI server in OP-TEE world raises optee irq 5
> > >   so that Linux scmi optee channel &scmi_cpu_dvfs pushed in the incoming
> > >   SCMI message in the scmi device for liekly later processing in threaded
> > >   context. The example includes all parties: optee, scmi, sram, gic.
> > > - Obviously rephrased the commit message.
> > > - Added Cc: tags
> > > ---
> > >  .../arm/firmware/linaro,optee-tz.yaml         | 67 +++++++++++++++++++
> > >  1 file changed, 67 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > > index d4dc0749f9fd..9c00c27f8b2c 100644
> > > --- a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > > +++ b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > > @@ -40,6 +40,14 @@ properties:
> > >        HVC #0, register assignments
> > >        register assignments are specified in drivers/tee/optee/optee_smc.h
> > >
> > > +  interrupt-controller: true
> > > +
> > > +  "#interrupt-cells":
> > > +    const: 1
> > > +    description: |
> > > +      OP-TEE exposes irq for irp chip controllers from OP-TEE world. Each
> > > +      irq is assigned a single line number identifier used as first argument.
> > > +
> > >  required:
> > >    - compatible
> > >    - method
> > > @@ -64,3 +72,62 @@ examples:
> > >              method = "hvc";
> > >          };
> > >      };
> > > +
> > > +  - |
> > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > +    firmware  {
> > > +        optee: optee {
> > > +            compatible = "linaro,optee-tz";
> > > +            method = "smc";
> > > +            interrupts = <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>;
> > > +            interrupt-controller;
> > > +            #interrupt-cells = <1>;
> > > +        };
> > > +
> > > +        scmi {
> > > +            compatible = "linaro,scmi-optee";
> > > +            linaro,optee-channel-id = <0>;
> > > +            interrupt-parent = <&gic>;
> > > +            #address-cells = <1>;
> > > +            #size-cells = <0>;
> > > +
> > > +            scmi_cpu_dvfs: protocol@13 {
> > > +                reg = <0x13>;
> > > +                linaro,optee-channel-id = <1>;
> > > +                shmem = <&scmi_shm_tx>, <&scmi_shm_rx>;
> > > +                interrupts-extended = <&optee 5>;
> > > +                interrupt-names = "a2p";
> >
> > These properties aren't documented. Soon there will be a warning[1].
>
> They are.
> https://github.com/torvalds/linux/blob/v6.1/Documentation/devicetree/bindings/firmware/arm%2Cscmi.yaml#L45-L53

They are not. That's the scmi node, not a protocol node.

> In arm,scmi.yaml, interrupts optional property stands for interrupts
> and interrupts-extended, no?

Yes.

Rob
  
Etienne Carriere Jan. 26, 2023, 2:40 p.m. UTC | #4
On Thu, 26 Jan 2023 at 15:00, Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Jan 25, 2023 at 3:00 PM Etienne Carriere
> <etienne.carriere@linaro.org> wrote:
> >
> > On Wed, 25 Jan 2023 at 21:22, Rob Herring <robh@kernel.org> wrote:
> > >
> > > +Sudeep
> > >
> > > On Tue, Jan 24, 2023 at 11:56:41AM +0100, Etienne Carriere wrote:
> > > > Adds an optional interrupt controller property to optee firmware node
> > > > in the DT bindings. Optee driver may embeds an irqchip exposing
> > > > interrupts notified by the TEE world. Optee registers up to 1 interrupt
> > > > controller and identifies each line with a line number from 0 to
> > > > UINT16_MAX.
> > > >
> > > > In the example, the platform SCMI device uses optee interrupt irq 5
> > > > as async signal to trigger processing of an async incoming SCMI message,
> > > > in the scope of a CPU DVFS control. A platform can have several SCMI
> > > > channels driven this way. Optee irqs also permits small embedded devices
> > > > to share e.g. a gpio expander, a group of wakeup sources, etc... between
> > > > OP-TEE world (for sensitive services) and Linux world (for non-sensitive
> > > > services). The physical controller is driven from the TEE which exposes
> > > > some controls to Linux kernel.
> > > >
> > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > > > Cc: Marc Zyngier <maz@kernel.org>
> > > > Cc: Rob Herring <robh+dt@kernel.org>
> > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > >
> > > > Co-developed-by: Pascal Paillet <p.paillet@foss.st.com>
> > > > Signed-off-by: Pascal Paillet <p.paillet@foss.st.com>
> > > > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > > > ---
> > > > Changes since v1:
> > > > - Added a description to #interrupt-cells property.
> > > > - Changed of example. Linux wakeup event was subject to discussion and
> > > >   i don't know much about input events in Linux. So move to SCMI.
> > > >   In the example, an SCMI server in OP-TEE world raises optee irq 5
> > > >   so that Linux scmi optee channel &scmi_cpu_dvfs pushed in the incoming
> > > >   SCMI message in the scmi device for liekly later processing in threaded
> > > >   context. The example includes all parties: optee, scmi, sram, gic.
> > > > - Obviously rephrased the commit message.
> > > > - Added Cc: tags
> > > > ---
> > > >  .../arm/firmware/linaro,optee-tz.yaml         | 67 +++++++++++++++++++
> > > >  1 file changed, 67 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > > > index d4dc0749f9fd..9c00c27f8b2c 100644
> > > > --- a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > > > +++ b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > > > @@ -40,6 +40,14 @@ properties:
> > > >        HVC #0, register assignments
> > > >        register assignments are specified in drivers/tee/optee/optee_smc.h
> > > >
> > > > +  interrupt-controller: true
> > > > +
> > > > +  "#interrupt-cells":
> > > > +    const: 1
> > > > +    description: |
> > > > +      OP-TEE exposes irq for irp chip controllers from OP-TEE world. Each
> > > > +      irq is assigned a single line number identifier used as first argument.
> > > > +
> > > >  required:
> > > >    - compatible
> > > >    - method
> > > > @@ -64,3 +72,62 @@ examples:
> > > >              method = "hvc";
> > > >          };
> > > >      };
> > > > +
> > > > +  - |
> > > > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > +    firmware  {
> > > > +        optee: optee {
> > > > +            compatible = "linaro,optee-tz";
> > > > +            method = "smc";
> > > > +            interrupts = <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>;
> > > > +            interrupt-controller;
> > > > +            #interrupt-cells = <1>;
> > > > +        };
> > > > +
> > > > +        scmi {
> > > > +            compatible = "linaro,scmi-optee";
> > > > +            linaro,optee-channel-id = <0>;
> > > > +            interrupt-parent = <&gic>;
> > > > +            #address-cells = <1>;
> > > > +            #size-cells = <0>;
> > > > +
> > > > +            scmi_cpu_dvfs: protocol@13 {
> > > > +                reg = <0x13>;
> > > > +                linaro,optee-channel-id = <1>;
> > > > +                shmem = <&scmi_shm_tx>, <&scmi_shm_rx>;
> > > > +                interrupts-extended = <&optee 5>;
> > > > +                interrupt-names = "a2p";
> > >
> > > These properties aren't documented. Soon there will be a warning[1].
> >
> > They are.
> > https://github.com/torvalds/linux/blob/v6.1/Documentation/devicetree/bindings/firmware/arm%2Cscmi.yaml#L45-L53
>
> They are not. That's the scmi node, not a protocol node.

Ok,i should have written it this way then:

+        scmi {
+            compatible = "linaro,scmi-optee";
+            linaro,optee-channel-id = <0>;
+            shmem = <&scmi_shm_tx>, <&scmi_shm_rx>;
+            interrupts-extended = <&optee 5>;
+            interrupt-names = "a2p";
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            scmi_cpu_dvfs: protocol@13 {
+                reg = <0x13>;
+                #clock-cells = <1>;
+            };
+        };


>
> > In arm,scmi.yaml, interrupts optional property stands for interrupts
> > and interrupts-extended, no?
>
> Yes.
>
> Rob
  
Sudeep Holla Jan. 26, 2023, 2:53 p.m. UTC | #5
On Tue, Jan 24, 2023 at 11:56:41AM +0100, Etienne Carriere wrote:
> Adds an optional interrupt controller property to optee firmware node
> in the DT bindings. Optee driver may embeds an irqchip exposing
> interrupts notified by the TEE world. Optee registers up to 1 interrupt
> controller and identifies each line with a line number from 0 to
> UINT16_MAX.
> 
> In the example, the platform SCMI device uses optee interrupt irq 5
> as async signal to trigger processing of an async incoming SCMI message,
> in the scope of a CPU DVFS control. A platform can have several SCMI
> channels driven this way. Optee irqs also permits small embedded devices
> to share e.g. a gpio expander, a group of wakeup sources, etc... between
> OP-TEE world (for sensitive services) and Linux world (for non-sensitive
> services). The physical controller is driven from the TEE which exposes
> some controls to Linux kernel.
> 
> Cc: Jens Wiklander <jens.wiklander@linaro.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> 
> Co-developed-by: Pascal Paillet <p.paillet@foss.st.com>
> Signed-off-by: Pascal Paillet <p.paillet@foss.st.com>
> Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> ---
> Changes since v1:
> - Added a description to #interrupt-cells property.
> - Changed of example. Linux wakeup event was subject to discussion and
>   i don't know much about input events in Linux. So move to SCMI.
>   In the example, an SCMI server in OP-TEE world raises optee irq 5
>   so that Linux scmi optee channel &scmi_cpu_dvfs pushed in the incoming
>   SCMI message in the scmi device for liekly later processing in threaded
>   context. The example includes all parties: optee, scmi, sram, gic.
> - Obviously rephrased the commit message.
> - Added Cc: tags
> ---
>  .../arm/firmware/linaro,optee-tz.yaml         | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> index d4dc0749f9fd..9c00c27f8b2c 100644
> --- a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> +++ b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> @@ -40,6 +40,14 @@ properties:
>        HVC #0, register assignments
>        register assignments are specified in drivers/tee/optee/optee_smc.h
>  
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 1
> +    description: |
> +      OP-TEE exposes irq for irp chip controllers from OP-TEE world. Each
> +      irq is assigned a single line number identifier used as first argument.
> +
>  required:
>    - compatible
>    - method
> @@ -64,3 +72,62 @@ examples:
>              method = "hvc";
>          };
>      };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    firmware  {
> +        optee: optee {
> +            compatible = "linaro,optee-tz";
> +            method = "smc";
> +            interrupts = <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>;
> +            interrupt-controller;
> +            #interrupt-cells = <1>;
> +        };
> +
> +        scmi {
> +            compatible = "linaro,scmi-optee";
> +            linaro,optee-channel-id = <0>;
> +            interrupt-parent = <&gic>;
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            scmi_cpu_dvfs: protocol@13 {
> +                reg = <0x13>;
> +                linaro,optee-channel-id = <1>;
> +                shmem = <&scmi_shm_tx>, <&scmi_shm_rx>;
> +                interrupts-extended = <&optee 5>;

Just curious if this can discovered by some communication within OPTEE.
You know you are using optee-channel-id 0 for all SCMI and 1 for DVFS.
Is it not possible to get the information from OPTEE dynamically like
you do for shmem. It is offset within the notification bitmap IIUC, so
the question is can be get that from the firmware on the fly. It also
gives the firmware to reshuffle things around if needed and don't have to
worry about compatibility with DT ?
  
Etienne Carriere Jan. 26, 2023, 3:51 p.m. UTC | #6
Hi Sudeep,

On Thu, 26 Jan 2023 at 15:53, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Tue, Jan 24, 2023 at 11:56:41AM +0100, Etienne Carriere wrote:
> > Adds an optional interrupt controller property to optee firmware node
> > in the DT bindings. Optee driver may embeds an irqchip exposing
> > interrupts notified by the TEE world. Optee registers up to 1 interrupt
> > controller and identifies each line with a line number from 0 to
> > UINT16_MAX.
> >
> > In the example, the platform SCMI device uses optee interrupt irq 5
> > as async signal to trigger processing of an async incoming SCMI message,
> > in the scope of a CPU DVFS control. A platform can have several SCMI
> > channels driven this way. Optee irqs also permits small embedded devices
> > to share e.g. a gpio expander, a group of wakeup sources, etc... between
> > OP-TEE world (for sensitive services) and Linux world (for non-sensitive
> > services). The physical controller is driven from the TEE which exposes
> > some controls to Linux kernel.
> >
> > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Sumit Garg <sumit.garg@linaro.org>
> >
> > Co-developed-by: Pascal Paillet <p.paillet@foss.st.com>
> > Signed-off-by: Pascal Paillet <p.paillet@foss.st.com>
> > Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
> > ---
> > Changes since v1:
> > - Added a description to #interrupt-cells property.
> > - Changed of example. Linux wakeup event was subject to discussion and
> >   i don't know much about input events in Linux. So move to SCMI.
> >   In the example, an SCMI server in OP-TEE world raises optee irq 5
> >   so that Linux scmi optee channel &scmi_cpu_dvfs pushed in the incoming
> >   SCMI message in the scmi device for liekly later processing in threaded
> >   context. The example includes all parties: optee, scmi, sram, gic.
> > - Obviously rephrased the commit message.
> > - Added Cc: tags
> > ---
> >  .../arm/firmware/linaro,optee-tz.yaml         | 67 +++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > index d4dc0749f9fd..9c00c27f8b2c 100644
> > --- a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > +++ b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
> > @@ -40,6 +40,14 @@ properties:
> >        HVC #0, register assignments
> >        register assignments are specified in drivers/tee/optee/optee_smc.h
> >
> > +  interrupt-controller: true
> > +
> > +  "#interrupt-cells":
> > +    const: 1
> > +    description: |
> > +      OP-TEE exposes irq for irp chip controllers from OP-TEE world. Each
> > +      irq is assigned a single line number identifier used as first argument.
> > +
> >  required:
> >    - compatible
> >    - method
> > @@ -64,3 +72,62 @@ examples:
> >              method = "hvc";
> >          };
> >      };
> > +
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    firmware  {
> > +        optee: optee {
> > +            compatible = "linaro,optee-tz";
> > +            method = "smc";
> > +            interrupts = <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>;
> > +            interrupt-controller;
> > +            #interrupt-cells = <1>;
> > +        };
> > +
> > +        scmi {
> > +            compatible = "linaro,scmi-optee";
> > +            linaro,optee-channel-id = <0>;
> > +            interrupt-parent = <&gic>;
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            scmi_cpu_dvfs: protocol@13 {
> > +                reg = <0x13>;
> > +                linaro,optee-channel-id = <1>;
> > +                shmem = <&scmi_shm_tx>, <&scmi_shm_rx>;
> > +                interrupts-extended = <&optee 5>;
>
> Just curious if this can discovered by some communication within OPTEE.
> You know you are using optee-channel-id 0 for all SCMI and 1 for DVFS.
> Is it not possible to get the information from OPTEE dynamically like
> you do for shmem. It is offset within the notification bitmap IIUC, so
> the question is can be get that from the firmware on the fly. It also
> gives the firmware to reshuffle things around if needed and don't have to
> worry about compatibility with DT ?

There is not specific discussions with OP-TEE to know which
Yet, assuming we have that scmi channel notif ID from optee, how does
scmi/optee driver relates to optee device irqchip?
With DT, we get both the optee irqchip ref (phandle) and our channel
notif irq line number (phandle arg #1).
Without DT, we still need to reference optee's irqchip.

Best regards,
etienne



>
> --
> Regards,
> Sudeep
  

Patch

diff --git a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
index d4dc0749f9fd..9c00c27f8b2c 100644
--- a/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
+++ b/Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.yaml
@@ -40,6 +40,14 @@  properties:
       HVC #0, register assignments
       register assignments are specified in drivers/tee/optee/optee_smc.h
 
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 1
+    description: |
+      OP-TEE exposes irq for irp chip controllers from OP-TEE world. Each
+      irq is assigned a single line number identifier used as first argument.
+
 required:
   - compatible
   - method
@@ -64,3 +72,62 @@  examples:
             method = "hvc";
         };
     };
+
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    firmware  {
+        optee: optee {
+            compatible = "linaro,optee-tz";
+            method = "smc";
+            interrupts = <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>;
+            interrupt-controller;
+            #interrupt-cells = <1>;
+        };
+
+        scmi {
+            compatible = "linaro,scmi-optee";
+            linaro,optee-channel-id = <0>;
+            interrupt-parent = <&gic>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            scmi_cpu_dvfs: protocol@13 {
+                reg = <0x13>;
+                linaro,optee-channel-id = <1>;
+                shmem = <&scmi_shm_tx>, <&scmi_shm_rx>;
+                interrupts-extended = <&optee 5>;
+                interrupt-names = "a2p";
+                #clock-cells = <1>;
+            };
+        };
+    };
+
+    gic: interrupt-controller@a0021000 {
+        compatible = "arm,cortex-a7-gic";
+        reg = <0xa0021000 0x1000>, <0xa0022000 0x2000>;
+        interrupt-controller;
+        #interrupt-cells = <3>;
+    };
+
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        sram@2ffff000 {
+            compatible = "mmio-sram";
+            reg = <0x2ffff000 0x1000>;
+            #address-cells = <1>;
+            #size-cells = <1>;
+            ranges = <0 0x2ffff000 0x1000>;
+
+            scmi_shm_tx: scmi-sram@0 {
+                compatible = "arm,scmi-shmem";
+                reg = <0 0x80>;
+            };
+
+            scmi_shm_rx: scmi-sram@100 {
+                compatible = "arm,scmi-shmem";
+                reg = <0x100 0x80>;
+            };
+        };
+    };