[edk2,edk2-platforms,1/1] Silicon/SynQuacer/DeviceTree: add OP-TEE driver node

Message ID 1532070779-7544-1-git-send-email-sumit.garg@linaro.org
State New
Headers show
Series
  • [edk2,edk2-platforms,1/1] Silicon/SynQuacer/DeviceTree: add OP-TEE driver node
Related show

Commit Message

Sumit Garg July 20, 2018, 7:12 a.m.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

---
 Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

-- 
2.7.4

Comments

Ard Biesheuvel July 20, 2018, 10:01 a.m. | #1
On 20 July 2018 at 16:12, Sumit Garg <sumit.garg@linaro.org> wrote:
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> Cc: Leif Lindholm <leif.lindholm@linaro.org>

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

> ---

>  Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 7 +++++++

>  1 file changed, 7 insertions(+)

>

> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> index 37d642e4b237..d109a5742793 100644

> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> @@ -574,6 +574,13 @@

>          #address-cells = <1>;

>          #size-cells = <0>;

>      };

> +

> +    firmware {

> +        optee {

> +            compatible = "linaro,optee-tz";

> +            method = "smc";

> +        };

> +    };

>  };

>

>  #include "SynQuacerCaches.dtsi"


Hello Sumit,

Is it safe to provide this node when optee is not running?

If not, this should be made dependent on that, so we should probably
add status = 'disabled', and remove it in PlatformDxe if the DIP
switch is not set, and/or if there are other ways we might know that
optee is not running.
Sumit Garg July 20, 2018, 10:13 a.m. | #2
On Fri, 20 Jul 2018 at 15:31, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>

> On 20 July 2018 at 16:12, Sumit Garg <sumit.garg@linaro.org> wrote:

> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> > Cc: Leif Lindholm <leif.lindholm@linaro.org>

> > Contributed-under: TianoCore Contribution Agreement 1.1

> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

> > ---

> >  Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 7 +++++++

> >  1 file changed, 7 insertions(+)

> >

> > diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> > index 37d642e4b237..d109a5742793 100644

> > --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> > +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> > @@ -574,6 +574,13 @@

> >          #address-cells = <1>;

> >          #size-cells = <0>;

> >      };

> > +

> > +    firmware {

> > +        optee {

> > +            compatible = "linaro,optee-tz";

> > +            method = "smc";

> > +        };

> > +    };

> >  };

> >

> >  #include "SynQuacerCaches.dtsi"

>

> Hello Sumit,

>

> Is it safe to provide this node when optee is not running?


Yes it is safe. If optee is not running then Linux TEE driver exits
gracefully with below info:

[    1.976021] optee: probing for conduit method from DT.
[    1.976033] optee: api uid mismatch

-Sumit

>

> If not, this should be made dependent on that, so we should probably

> add status = 'disabled', and remove it in PlatformDxe if the DIP

> switch is not set, and/or if there are other ways we might know that

> optee is not running.
Ard Biesheuvel July 20, 2018, 10:19 a.m. | #3
On 20 July 2018 at 19:13, Sumit Garg <sumit.garg@linaro.org> wrote:
> On Fri, 20 Jul 2018 at 15:31, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>>

>> On 20 July 2018 at 16:12, Sumit Garg <sumit.garg@linaro.org> wrote:

>> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> > Cc: Leif Lindholm <leif.lindholm@linaro.org>

>> > Contributed-under: TianoCore Contribution Agreement 1.1

>> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

>> > ---

>> >  Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 7 +++++++

>> >  1 file changed, 7 insertions(+)

>> >

>> > diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

>> > index 37d642e4b237..d109a5742793 100644

>> > --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

>> > +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

>> > @@ -574,6 +574,13 @@

>> >          #address-cells = <1>;

>> >          #size-cells = <0>;

>> >      };

>> > +

>> > +    firmware {

>> > +        optee {

>> > +            compatible = "linaro,optee-tz";

>> > +            method = "smc";

>> > +        };

>> > +    };

>> >  };

>> >

>> >  #include "SynQuacerCaches.dtsi"

>>

>> Hello Sumit,

>>

>> Is it safe to provide this node when optee is not running?

>

> Yes it is safe. If optee is not running then Linux TEE driver exits

> gracefully with below info:

>

> [    1.976021] optee: probing for conduit method from DT.

> [    1.976033] optee: api uid mismatch

>


Ok, so it is safe but it still prints a nasty error.

So let's fix this properly: you can check the existing code to find
out how the mmc DT node gets enabled in PlatformDxe.
Sumit Garg July 20, 2018, 10:27 a.m. | #4
On Fri, 20 Jul 2018 at 15:49, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>

> On 20 July 2018 at 19:13, Sumit Garg <sumit.garg@linaro.org> wrote:

> > On Fri, 20 Jul 2018 at 15:31, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> >>

> >> On 20 July 2018 at 16:12, Sumit Garg <sumit.garg@linaro.org> wrote:

> >> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >> > Cc: Leif Lindholm <leif.lindholm@linaro.org>

> >> > Contributed-under: TianoCore Contribution Agreement 1.1

> >> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>

> >> > ---

> >> >  Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi | 7 +++++++

> >> >  1 file changed, 7 insertions(+)

> >> >

> >> > diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> >> > index 37d642e4b237..d109a5742793 100644

> >> > --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> >> > +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi

> >> > @@ -574,6 +574,13 @@

> >> >          #address-cells = <1>;

> >> >          #size-cells = <0>;

> >> >      };

> >> > +

> >> > +    firmware {

> >> > +        optee {

> >> > +            compatible = "linaro,optee-tz";

> >> > +            method = "smc";

> >> > +        };

> >> > +    };

> >> >  };

> >> >

> >> >  #include "SynQuacerCaches.dtsi"

> >>

> >> Hello Sumit,

> >>

> >> Is it safe to provide this node when optee is not running?

> >

> > Yes it is safe. If optee is not running then Linux TEE driver exits

> > gracefully with below info:

> >

> > [    1.976021] optee: probing for conduit method from DT.

> > [    1.976033] optee: api uid mismatch

> >

>

> Ok, so it is safe but it still prints a nasty error.

>

> So let's fix this properly: you can check the existing code to find

> out how the mmc DT node gets enabled in PlatformDxe.


Sure will have a look at mmc DT addition code and come up with a more
clean solution.

-Sumit

Patch

diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
index 37d642e4b237..d109a5742793 100644
--- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
+++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
@@ -574,6 +574,13 @@ 
         #address-cells = <1>;
         #size-cells = <0>;
     };
+
+    firmware {
+        optee {
+            compatible = "linaro,optee-tz";
+            method = "smc";
+        };
+    };
 };
 
 #include "SynQuacerCaches.dtsi"