Re: [PATCH v7 4/4] arm64: dts: Add Caninos Loucos Labrador v3
Em 9/22/20 7:26 AM, Catalin Marinas escreveu:
On Tue, Sep 22, 2020 at 10:32:06AM +0200, Arnd Bergmann wrote:
On Tue, Sep 22, 2020 at 8:15 AM Manivannan Sadhasivam
wrote:
On Mon, Sep 21, 2020 at 11:43:02PM -0300, Matheus Castello wrote:
+ /* Labrador v3 firmware does not support PSCI */
Oops. This is unfortunate... I'm not sure if this is even acceptable for
ARM64 machines.
Let me add Olof and Arnd...
Adding Catalin and Will for additional input as well, this is more their
area than ours.
I don't think we have formalized this as a policy, but we clearly don't
want new boards to use the spin table hack. As there are other
boards using psci on the same chip, I don't think this is a
hardware bug.
I fully agree, we shouldn't allow new boards to use the spin-table
method unless EL3 is missing on the CPU implementation (not the case
here; only the APM hardware has this issue). Unfortunately we missed
another platform with A53, see commit bc66392d8258 ("arm64: dts: fsl:
Add device tree for S32V234-EVB").
The kernel relies on firmware for other things (power management, errata
workarounds), so an SMC calling convention compliant firmware is highly
recommended. I also don't see why it would be that hard to add PSCI.
Even if you don't port something like Trusted Firmware, U-Boot has basic
support for PSCI.
So from my perspective, NAK on this patch.
Thanks Arnd and Catalin, this is really just a limitation of the
bootloader developed by manufactures that comes embedded in the board. I
will drop this in the next version.
I'm tempted to also modify smp_spin_table_cpu_init() to print a big
warning and return an error if this is attempted on new platforms. IOW,
we make it a policy from now on.
Re: [PATCH v7 4/4] arm64: dts: Add Caninos Loucos Labrador v3
On Tue, Sep 22, 2020 at 10:32:06AM +0200, Arnd Bergmann wrote:
> On Tue, Sep 22, 2020 at 8:15 AM Manivannan Sadhasivam
> wrote:
> > On Mon, Sep 21, 2020 at 11:43:02PM -0300, Matheus Castello wrote:
> > > + /* Labrador v3 firmware does not support PSCI */
> >
> > Oops. This is unfortunate... I'm not sure if this is even acceptable for
> > ARM64 machines.
> >
> > Let me add Olof and Arnd...
>
> Adding Catalin and Will for additional input as well, this is more their
> area than ours.
>
> I don't think we have formalized this as a policy, but we clearly don't
> want new boards to use the spin table hack. As there are other
> boards using psci on the same chip, I don't think this is a
> hardware bug.
I fully agree, we shouldn't allow new boards to use the spin-table
method unless EL3 is missing on the CPU implementation (not the case
here; only the APM hardware has this issue). Unfortunately we missed
another platform with A53, see commit bc66392d8258 ("arm64: dts: fsl:
Add device tree for S32V234-EVB").
The kernel relies on firmware for other things (power management, errata
workarounds), so an SMC calling convention compliant firmware is highly
recommended. I also don't see why it would be that hard to add PSCI.
Even if you don't port something like Trusted Firmware, U-Boot has basic
support for PSCI.
So from my perspective, NAK on this patch.
I'm tempted to also modify smp_spin_table_cpu_init() to print a big
warning and return an error if this is attempted on new platforms. IOW,
we make it a policy from now on.
--
Catalin
Re: [PATCH v7 4/4] arm64: dts: Add Caninos Loucos Labrador v3
On Tue, Sep 22, 2020 at 8:15 AM Manivannan Sadhasivam wrote: > On Mon, Sep 21, 2020 at 11:43:02PM -0300, Matheus Castello wrote: > > + /* Labrador v3 firmware does not support PSCI */ > > Oops. This is unfortunate... I'm not sure if this is even acceptable for > ARM64 machines. > > Let me add Olof and Arnd... Adding Catalin and Will for additional input as well, this is more their area than ours. I don't think we have formalized this as a policy, but we clearly don't want new boards to use the spin table hack. As there are other boards using psci on the same chip, I don't think this is a hardware bug. Matheus: can you explain what keeps you from fixing the bootloader instead? Arnd
Re: [PATCH v7 4/4] arm64: dts: Add Caninos Loucos Labrador v3
Hi,
On Mon, Sep 21, 2020 at 11:43:02PM -0300, Matheus Castello wrote:
> Add Device Trees for Caninos Loucos Labrador CoM Core v3 and base board
> M v2. Based on the work of Andreas Färber on Cubieboard 7 device tree.
>
> Signed-off-by: Matheus Castello
> ---
> arch/arm64/boot/dts/actions/Makefile | 2 +
> .../dts/actions/s700-labrador-base-m2.dts | 34 +
> .../boot/dts/actions/s700-labrador-v3.dtsi| 122 ++
> 3 files changed, 158 insertions(+)
> create mode 100644 arch/arm64/boot/dts/actions/s700-labrador-base-m2.dts
> create mode 100644 arch/arm64/boot/dts/actions/s700-labrador-v3.dtsi
>
> diff --git a/arch/arm64/boot/dts/actions/Makefile
> b/arch/arm64/boot/dts/actions/Makefile
> index b57fd2372ecd..3765697fa91e 100644
> --- a/arch/arm64/boot/dts/actions/Makefile
> +++ b/arch/arm64/boot/dts/actions/Makefile
> @@ -2,4 +2,6 @@
>
> dtb-$(CONFIG_ARCH_ACTIONS) += s700-cubieboard7.dtb
>
> +dtb-$(CONFIG_ARCH_ACTIONS) += s700-labrador-base-m2.dtb
> +
> dtb-$(CONFIG_ARCH_ACTIONS) += s900-bubblegum-96.dtb
> diff --git a/arch/arm64/boot/dts/actions/s700-labrador-base-m2.dts
> b/arch/arm64/boot/dts/actions/s700-labrador-base-m2.dts
> new file mode 100644
> index ..63bbca46475b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/actions/s700-labrador-base-m2.dts
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2020 Matheus Castello
> + */
> +
> +/dts-v1/;
> +
> +#include "s700-labrador-v3.dtsi"
> +
> +/ {
> + compatible = "caninos,labrador-base-m2",
> + "caninos,labrador-v3", "actions,s700";
> + model = "Caninos Labrador Core v3 on Labrador Base-M v2";
> +
> + aliases {
> + serial3 =
> + };
> +
> + chosen {
> + stdout-path = "serial3:115200n8";
> + };
> +};
> +
> + {
> + status = "okay";
No fixed clock?
> +};
> +
> + {
> + status = "okay";
> +};
> +
> + {
> + status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/actions/s700-labrador-v3.dtsi
> b/arch/arm64/boot/dts/actions/s700-labrador-v3.dtsi
> new file mode 100644
> index ..91addba6382b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/actions/s700-labrador-v3.dtsi
> @@ -0,0 +1,122 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2020 Matheus Castello
> + */
> +
> +#include "s700.dtsi"
> +
> +/ {
> + compatible = "caninos,labrador-v3", "actions,s700";
> + model = "Caninos Labrador Core v3.1";
> +
> + memory@0 {
> + device_type = "memory";
> + reg = <0x0 0x0 0x0 0x8000>;
> + };
> +
> + memory@1,e000 {
> + device_type = "memory";
> + reg = <0x1 0xe000 0x0 0x0>;
> + };
> +
What is the size of this memory? The datasheet says only 512MB Max.
> + /* Labrador v3 firmware does not support PSCI */
Oops. This is unfortunate... I'm not sure if this is even acceptable for
ARM64 machines.
Let me add Olof and Arnd...
Thanks,
Mani
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x0>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0 0x1f20>;
> + next-level-cache = <>;
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x1>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0 0x1f20>;
> + next-level-cache = <>;
> + };
> +
> + cpu2: cpu@2 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x2>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0 0x1f20>;
> + next-level-cache = <>;
> + };
> +
> + cpu3: cpu@3 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x3>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0 0x1f20>;
> + next-level-cache = <>;
> + };
> + };
> +
> + L2: l2-cache {
> + compatible = "cache";
> + cache-level = <2>;
> + };
> +};
> +
> + {
> + clocks = <>;
> +};
> +
> + {
> + pinctrl-names = "default";
> + pinctrl-0 = <_default>;
> +};
> +
> + {
> + pinctrl-names = "default";
> + pinctrl-0 = <_default>;
> +};
> +
> + {
> + pinctrl-names = "default";
> + pinctrl-0 = <_default>;
> +};
> +
> + {
> + i2c0_default: i2c0_default {
> + pinmux {
>
[PATCH v7 4/4] arm64: dts: Add Caninos Loucos Labrador v3
Add Device Trees for Caninos Loucos Labrador CoM Core v3 and base board
M v2. Based on the work of Andreas Färber on Cubieboard 7 device tree.
Signed-off-by: Matheus Castello
---
arch/arm64/boot/dts/actions/Makefile | 2 +
.../dts/actions/s700-labrador-base-m2.dts | 34 +
.../boot/dts/actions/s700-labrador-v3.dtsi| 122 ++
3 files changed, 158 insertions(+)
create mode 100644 arch/arm64/boot/dts/actions/s700-labrador-base-m2.dts
create mode 100644 arch/arm64/boot/dts/actions/s700-labrador-v3.dtsi
diff --git a/arch/arm64/boot/dts/actions/Makefile
b/arch/arm64/boot/dts/actions/Makefile
index b57fd2372ecd..3765697fa91e 100644
--- a/arch/arm64/boot/dts/actions/Makefile
+++ b/arch/arm64/boot/dts/actions/Makefile
@@ -2,4 +2,6 @@
dtb-$(CONFIG_ARCH_ACTIONS) += s700-cubieboard7.dtb
+dtb-$(CONFIG_ARCH_ACTIONS) += s700-labrador-base-m2.dtb
+
dtb-$(CONFIG_ARCH_ACTIONS) += s900-bubblegum-96.dtb
diff --git a/arch/arm64/boot/dts/actions/s700-labrador-base-m2.dts
b/arch/arm64/boot/dts/actions/s700-labrador-base-m2.dts
new file mode 100644
index ..63bbca46475b
--- /dev/null
+++ b/arch/arm64/boot/dts/actions/s700-labrador-base-m2.dts
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2020 Matheus Castello
+ */
+
+/dts-v1/;
+
+#include "s700-labrador-v3.dtsi"
+
+/ {
+ compatible = "caninos,labrador-base-m2",
+"caninos,labrador-v3", "actions,s700";
+ model = "Caninos Labrador Core v3 on Labrador Base-M v2";
+
+ aliases {
+ serial3 =
+ };
+
+ chosen {
+ stdout-path = "serial3:115200n8";
+ };
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/actions/s700-labrador-v3.dtsi
b/arch/arm64/boot/dts/actions/s700-labrador-v3.dtsi
new file mode 100644
index ..91addba6382b
--- /dev/null
+++ b/arch/arm64/boot/dts/actions/s700-labrador-v3.dtsi
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2020 Matheus Castello
+ */
+
+#include "s700.dtsi"
+
+/ {
+ compatible = "caninos,labrador-v3", "actions,s700";
+ model = "Caninos Labrador Core v3.1";
+
+ memory@0 {
+ device_type = "memory";
+ reg = <0x0 0x0 0x0 0x8000>;
+ };
+
+ memory@1,e000 {
+ device_type = "memory";
+ reg = <0x1 0xe000 0x0 0x0>;
+ };
+
+ /* Labrador v3 firmware does not support PSCI */
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu0: cpu@0 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x0>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0 0x1f20>;
+ next-level-cache = <>;
+ };
+
+ cpu1: cpu@1 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x1>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0 0x1f20>;
+ next-level-cache = <>;
+ };
+
+ cpu2: cpu@2 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x2>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0 0x1f20>;
+ next-level-cache = <>;
+ };
+
+ cpu3: cpu@3 {
+ device_type = "cpu";
+ compatible = "arm,cortex-a53";
+ reg = <0x3>;
+ enable-method = "spin-table";
+ cpu-release-addr = <0 0x1f20>;
+ next-level-cache = <>;
+ };
+ };
+
+ L2: l2-cache {
+ compatible = "cache";
+ cache-level = <2>;
+ };
+};
+
+ {
+ clocks = <>;
+};
+
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_default>;
+};
+
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_default>;
+};
+
+ {
+ pinctrl-names = "default";
+ pinctrl-0 = <_default>;
+};
+
+ {
+ i2c0_default: i2c0_default {
+ pinmux {
+ groups = "i2c0_mfp";
+ function = "i2c0";
+ };
+ pinconf {
+ pins = "i2c0_sclk", "i2c0_sdata";
+ bias-pull-up;
+ };
+ };
+
+ i2c1_default: i2c1_default {
+ pinmux {
+ groups = "i2c1_dummy";
+ function = "i2c1";
+ };
+ pinconf {
+
