2021-03-18 08:36:34

by Michael Walle

[permalink] [raw]
Subject: [PATCH] arm64: dts: ls1028a: fix optee node

Don't enable the optee node in the SoC include. It is an optional
component and actually, if enabled, breaks boards which doesn't have it.

This reverts commit 48787485f8de ("arm64: dts: ls1028a: enable optee
node") and enables the node per board, assuming the intend of the
original author was to enable OPTEE for the LS1028A-RDB and the
LS1028A-QDS.

Fixes: 48787485f8de ("arm64: dts: ls1028a: enable optee node")
Reported-by: Guillaume Tucker <[email protected]>
Reported-by: "kernelci.org bot" <[email protected]>
Tested-by: Michael Walle <[email protected]>
Signed-off-by: Michael Walle <[email protected]>
---
arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 4 ++++
arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 4 ++++
arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 3 ++-
3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
index fbcba9cb8503..060d3c79244d 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
@@ -327,6 +327,10 @@
status = "okay";
};

+&optee {
+ status = "okay";
+};
+
&sai1 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
index 41ae6e7675ba..1bdf0104d492 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
@@ -274,6 +274,10 @@
status = "okay";
};

+&optee {
+ status = "okay";
+};
+
&sai4 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index 50d277eb2a54..e2007ebacd69 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -92,9 +92,10 @@
};

firmware {
- optee {
+ optee: optee {
compatible = "linaro,optee-tz";
method = "smc";
+ status = "disabled";
};
};

--
2.20.1


2021-03-18 09:23:15

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ls1028a: fix optee node

Am 2021-03-18 09:34, schrieb Michael Walle:
> Don't enable the optee node in the SoC include. It is an optional
> component and actually, if enabled, breaks boards which doesn't have
> it.
>
> This reverts commit 48787485f8de ("arm64: dts: ls1028a: enable optee
> node") and enables the node per board, assuming the intend of the
> original author was to enable OPTEE for the LS1028A-RDB and the
> LS1028A-QDS.
>
> Fixes: 48787485f8de ("arm64: dts: ls1028a: enable optee node")
> Reported-by: Guillaume Tucker <[email protected]>
> Reported-by: "kernelci.org bot" <[email protected]>
> Tested-by: Michael Walle <[email protected]>
> Signed-off-by: Michael Walle <[email protected]>
> ---

Please disregard this patch, because the offending patch was
already dropped:
https://lore.kernel.org/lkml/20210318084029.GY11246@dragon/

Sahil, if you like you can pick it up to enable the nodes for
your ls1028a boards.

-michael

2021-03-22 11:41:10

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ls1028a: fix optee node

Hi Sahil,

Am 2021-03-22 12:33, schrieb Sahil Malhotra:
> Thanks for the fix, and currently we support optee only on ls1028a-rdb
> boards.
> Does enabling the optee node this way, solves the issue ?

What do you mean? Please note, that Shawn already reverted your commit.
Therefore, I suggest you make a new commit where you enable optee only
for the ls1028a-rdb board.

-michael

2021-03-22 13:37:15

by Michael Walle

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ls1028a: fix optee node

Am 2021-03-22 14:04, schrieb Sahil Malhotra:
>> Sahil, if you like you can pick it up to enable the nodes for your
>> ls1028a boards.
> Michael, If we enable the optee node like this, will this gets resolved
> ?

I don't know what you mean. This was a fix for the initial patch. So,
I guess the answer is yes it will not break my board if you don't
enable optee globally, but just for your board.

-michael

2021-03-22 23:14:14

by Leo Li

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ls1028a: fix optee node

On Thu, Mar 18, 2021 at 3:36 AM Michael Walle <[email protected]> wrote:
>
> Don't enable the optee node in the SoC include. It is an optional
> component and actually, if enabled, breaks boards which doesn't have it.

Hi Shawn,

Shall we make this a general rule? I see quite a few SoC dtsi files
are having the optee node enabled by default.

Regards,
Leo

>
> This reverts commit 48787485f8de ("arm64: dts: ls1028a: enable optee
> node") and enables the node per board, assuming the intend of the
> original author was to enable OPTEE for the LS1028A-RDB and the
> LS1028A-QDS.
>
> Fixes: 48787485f8de ("arm64: dts: ls1028a: enable optee node")
> Reported-by: Guillaume Tucker <[email protected]>
> Reported-by: "kernelci.org bot" <[email protected]>
> Tested-by: Michael Walle <[email protected]>
> Signed-off-by: Michael Walle <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 4 ++++
> arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 4 ++++
> arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 3 ++-
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> index fbcba9cb8503..060d3c79244d 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> @@ -327,6 +327,10 @@
> status = "okay";
> };
>
> +&optee {
> + status = "okay";
> +};
> +
> &sai1 {
> status = "okay";
> };
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> index 41ae6e7675ba..1bdf0104d492 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> @@ -274,6 +274,10 @@
> status = "okay";
> };
>
> +&optee {
> + status = "okay";
> +};
> +
> &sai4 {
> status = "okay";
> };
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> index 50d277eb2a54..e2007ebacd69 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
> @@ -92,9 +92,10 @@
> };
>
> firmware {
> - optee {
> + optee: optee {
> compatible = "linaro,optee-tz";
> method = "smc";
> + status = "disabled";
> };
> };
>
> --
> 2.20.1
>

2021-03-29 00:40:23

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: ls1028a: fix optee node

On Mon, Mar 22, 2021 at 06:12:06PM -0500, Li Yang wrote:
> On Thu, Mar 18, 2021 at 3:36 AM Michael Walle <[email protected]> wrote:
> >
> > Don't enable the optee node in the SoC include. It is an optional
> > component and actually, if enabled, breaks boards which doesn't have it.
>
> Hi Shawn,
>
> Shall we make this a general rule? I see quite a few SoC dtsi files
> are having the optee node enabled by default.


Yeah, we should probably make it a general rule considering the issue
reported here. I thought that optee driver is smart enough to stop
probing if there is no optee os/firmware support found on given platform.

Shawn