On 2020-05-13 13:55, Lars Povlsen wrote:
> This adds the basic DT structure for the Microchip Sparx5 SoC, and the
> reference boards, pcb125, pcb134 and pcb135. The two latter have a
> NAND vs a eMMC centric variant (as a mount option),
>
> Reviewed-by: Alexandre Belloni <[email protected]>
> Signed-off-by: Lars Povlsen <[email protected]>
> ---
> MAINTAINERS | 1 +
> arch/arm64/boot/dts/Makefile | 1 +
> arch/arm64/boot/dts/microchip/Makefile | 4 +
> arch/arm64/boot/dts/microchip/sparx5.dtsi | 135 ++++++++++++++++++
> .../boot/dts/microchip/sparx5_pcb125.dts | 17 +++
> .../boot/dts/microchip/sparx5_pcb134.dts | 17 +++
> .../dts/microchip/sparx5_pcb134_board.dtsi | 15 ++
> .../boot/dts/microchip/sparx5_pcb134_emmc.dts | 17 +++
> .../boot/dts/microchip/sparx5_pcb135.dts | 17 +++
> .../dts/microchip/sparx5_pcb135_board.dtsi | 15 ++
> .../boot/dts/microchip/sparx5_pcb135_emmc.dts | 17 +++
> .../boot/dts/microchip/sparx5_pcb_common.dtsi | 15 ++
> 12 files changed, 271 insertions(+)
> create mode 100644 arch/arm64/boot/dts/microchip/Makefile
> create mode 100644 arch/arm64/boot/dts/microchip/sparx5.dtsi
> create mode 100644 arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
> create mode 100644 arch/arm64/boot/dts/microchip/sparx5_pcb134.dts
> create mode 100644
> arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
> create mode 100644
> arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts
> create mode 100644 arch/arm64/boot/dts/microchip/sparx5_pcb135.dts
> create mode 100644
> arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
> create mode 100644
> arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts
> create mode 100644
> arch/arm64/boot/dts/microchip/sparx5_pcb_common.dtsi
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1b5a18d3dbb9f..5aa28d6e39d4f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2084,6 +2084,7 @@ M: Lars Povlsen <[email protected]>
> M: Steen Hegelund <[email protected]>
> M: Microchip Linux Driver Support <[email protected]>
> L: [email protected] (moderated for
> non-subscribers)
> +F: arch/arm64/boot/dts/microchip/
> N: sparx5
> S: Supported
>
> diff --git a/arch/arm64/boot/dts/Makefile
> b/arch/arm64/boot/dts/Makefile
> index f19b762c008d8..9680a7f20c307 100644
> --- a/arch/arm64/boot/dts/Makefile
> +++ b/arch/arm64/boot/dts/Makefile
> @@ -17,6 +17,7 @@ subdir-y += intel
> subdir-y += lg
> subdir-y += marvell
> subdir-y += mediatek
> +subdir-y += microchip
> subdir-y += nvidia
> subdir-y += qcom
> subdir-y += realtek
> diff --git a/arch/arm64/boot/dts/microchip/Makefile
> b/arch/arm64/boot/dts/microchip/Makefile
> new file mode 100644
> index 0000000000000..c6e0313eea0f9
> --- /dev/null
> +++ b/arch/arm64/boot/dts/microchip/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_ARCH_SPARX5) += sparx5_pcb125.dtb
> +dtb-$(CONFIG_ARCH_SPARX5) += sparx5_pcb134.dtb sparx5_pcb134_emmc.dtb
> +dtb-$(CONFIG_ARCH_SPARX5) += sparx5_pcb135.dtb sparx5_pcb135_emmc.dtb
> diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi
> b/arch/arm64/boot/dts/microchip/sparx5.dtsi
> new file mode 100644
> index 0000000000000..3136b4369f507
> --- /dev/null
> +++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
> @@ -0,0 +1,135 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2020 Microchip Technology Inc. and its subsidiaries.
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> + compatible = "microchip,sparx5";
> + interrupt-parent = <&gic>;
> + #address-cells = <2>;
> + #size-cells = <1>;
> +
> + aliases {
> + serial0 = &uart0;
> + serial1 = &uart1;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> + cpu-map {
> + cluster0 {
> + core0 {
> + cpu = <&cpu0>;
> + };
> + core1 {
> + cpu = <&cpu1>;
> + };
> + };
> + };
> + cpu0: cpu@0 {
> + compatible = "arm,cortex-a53", "arm,armv8";
> + device_type = "cpu";
> + reg = <0x0 0x0>;
> + enable-method = "spin-table";
Really? This is 2020, not 2012 any more. Surely a new platform
boots using PSCI, and not *this*.
> + cpu-release-addr = <0x0 0x0000fff8>;
> + next-level-cache = <&L2_0>;
> + };
> + cpu1: cpu@1 {
> + compatible = "arm,cortex-a53", "arm,armv8";
> + device_type = "cpu";
> + reg = <0x0 0x1>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0x0 0x0000fff8>;
> + next-level-cache = <&L2_0>;
> + };
> + L2_0: l2-cache0 {
> + compatible = "cache";
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts =
> + <GIC_PPI 13
> + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 14
> + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 11
> + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 10
> + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
You have a GICv3. These interrupt specifiers are not the ones GICv3
expects.
> + };
> +
> + clocks: clocks {
> + #address-cells = <2>;
> + #size-cells = <1>;
> + ranges;
> + ahb_clk: ahb-clk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <250000000>;
> + };
> + sys_clk: sys-clk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <625000000>;
> + };
> + };
> +
> + axi: axi@600000000 {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <1>;
> + ranges;
> +
> + gic: interrupt-controller@600300000 {
> + compatible = "arm,gic-v3";
> + #interrupt-cells = <3>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + interrupt-controller;
> + reg = <0x6 0x00300000 0x20000>, /* GICD */
> + <0x6 0x00340000 0x1000000>; /* GICR */
You are missing the GICv3 compatibility interfaces (GICV/GICH), which
are implemented by the CPUs.
> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + uart0: serial@600100000 {
> + compatible = "ns16550a";
> + reg = <0x6 0x00100000 0x20>;
> + clocks = <&ahb_clk>;
> + reg-io-width = <4>;
> + reg-shift = <2>;
> + interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +
> + status = "disabled";
> + };
> +
> + uart1: serial@600102000 {
> + compatible = "ns16550a";
> + reg = <0x6 0x00102000 0x20>;
> + clocks = <&ahb_clk>;
> + reg-io-width = <4>;
> + reg-shift = <2>;
> + interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> +
> + status = "disabled";
> + };
> +
> + timer1: timer@600105000 {
> + compatible = "snps,dw-apb-timer";
> + reg = <0x6 0x00105000 0x1000>;
> + clocks = <&ahb_clk>;
> + clock-names = "timer";
> + interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + };
> +};
Where is the PMU node?
Thanks,
M.
--
Who you jivin' with that Cosmik Debris?
Marc Zyngier writes:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 2020-05-13 13:55, Lars Povlsen wrote:
>> This adds the basic DT structure for the Microchip Sparx5 SoC, and the
>> reference boards, pcb125, pcb134 and pcb135. The two latter have a
>> NAND vs a eMMC centric variant (as a mount option),
>>
>> Reviewed-by: Alexandre Belloni <[email protected]>
>> Signed-off-by: Lars Povlsen <[email protected]>
>> ---
>> MAINTAINERS | 1 +
>> arch/arm64/boot/dts/Makefile | 1 +
>> arch/arm64/boot/dts/microchip/Makefile | 4 +
>> arch/arm64/boot/dts/microchip/sparx5.dtsi | 135 ++++++++++++++++++
>> .../boot/dts/microchip/sparx5_pcb125.dts | 17 +++
>> .../boot/dts/microchip/sparx5_pcb134.dts | 17 +++
>> .../dts/microchip/sparx5_pcb134_board.dtsi | 15 ++
>> .../boot/dts/microchip/sparx5_pcb134_emmc.dts | 17 +++
>> .../boot/dts/microchip/sparx5_pcb135.dts | 17 +++
>> .../dts/microchip/sparx5_pcb135_board.dtsi | 15 ++
>> .../boot/dts/microchip/sparx5_pcb135_emmc.dts | 17 +++
>> .../boot/dts/microchip/sparx5_pcb_common.dtsi | 15 ++
>> 12 files changed, 271 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/microchip/Makefile
>> create mode 100644 arch/arm64/boot/dts/microchip/sparx5.dtsi
>> create mode 100644 arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
>> create mode 100644 arch/arm64/boot/dts/microchip/sparx5_pcb134.dts
>> create mode 100644
>> arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
>> create mode 100644
>> arch/arm64/boot/dts/microchip/sparx5_pcb134_emmc.dts
>> create mode 100644 arch/arm64/boot/dts/microchip/sparx5_pcb135.dts
>> create mode 100644
>> arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
>> create mode 100644
>> arch/arm64/boot/dts/microchip/sparx5_pcb135_emmc.dts
>> create mode 100644
>> arch/arm64/boot/dts/microchip/sparx5_pcb_common.dtsi
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1b5a18d3dbb9f..5aa28d6e39d4f 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2084,6 +2084,7 @@ M: Lars Povlsen <[email protected]>
>> M: Steen Hegelund <[email protected]>
>> M: Microchip Linux Driver Support <[email protected]>
>> L: [email protected] (moderated for
>> non-subscribers)
>> +F: arch/arm64/boot/dts/microchip/
>> N: sparx5
>> S: Supported
>>
>> diff --git a/arch/arm64/boot/dts/Makefile
>> b/arch/arm64/boot/dts/Makefile
>> index f19b762c008d8..9680a7f20c307 100644
>> --- a/arch/arm64/boot/dts/Makefile
>> +++ b/arch/arm64/boot/dts/Makefile
>> @@ -17,6 +17,7 @@ subdir-y += intel
>> subdir-y += lg
>> subdir-y += marvell
>> subdir-y += mediatek
>> +subdir-y += microchip
>> subdir-y += nvidia
>> subdir-y += qcom
>> subdir-y += realtek
>> diff --git a/arch/arm64/boot/dts/microchip/Makefile
>> b/arch/arm64/boot/dts/microchip/Makefile
>> new file mode 100644
>> index 0000000000000..c6e0313eea0f9
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/microchip/Makefile
>> @@ -0,0 +1,4 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +dtb-$(CONFIG_ARCH_SPARX5) += sparx5_pcb125.dtb
>> +dtb-$(CONFIG_ARCH_SPARX5) += sparx5_pcb134.dtb sparx5_pcb134_emmc.dtb
>> +dtb-$(CONFIG_ARCH_SPARX5) += sparx5_pcb135.dtb sparx5_pcb135_emmc.dtb
>> diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi
>> b/arch/arm64/boot/dts/microchip/sparx5.dtsi
>> new file mode 100644
>> index 0000000000000..3136b4369f507
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
>> @@ -0,0 +1,135 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2020 Microchip Technology Inc. and its subsidiaries.
>> + */
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +/ {
>> + compatible = "microchip,sparx5";
>> + interrupt-parent = <&gic>;
>> + #address-cells = <2>;
>> + #size-cells = <1>;
>> +
>> + aliases {
>> + serial0 = &uart0;
>> + serial1 = &uart1;
>> + };
>> +
>> + chosen {
>> + stdout-path = "serial0:115200n8";
>> + };
>> +
>> + cpus {
>> + #address-cells = <2>;
>> + #size-cells = <0>;
>> + cpu-map {
>> + cluster0 {
>> + core0 {
>> + cpu = <&cpu0>;
>> + };
>> + core1 {
>> + cpu = <&cpu1>;
>> + };
>> + };
>> + };
>> + cpu0: cpu@0 {
>> + compatible = "arm,cortex-a53", "arm,armv8";
>> + device_type = "cpu";
>> + reg = <0x0 0x0>;
>> + enable-method = "spin-table";
>
> Really? This is 2020, not 2012 any more. Surely a new platform
> boots using PSCI, and not *this*.
>
We don't currently support PSCI. The platform does not have TrustZone,
hence we don't use ATF.
We use U-Boot as the (only) bootloader, providing the spintable. I looked
at adding PSCI (in U-Boot) initially, but as most other platforms were
using ATF the PSCI support was buggy and caused me to go for spintable.
Is spintable being deprecated?
>> + cpu-release-addr = <0x0 0x0000fff8>;
>> + next-level-cache = <&L2_0>;
>> + };
>> + cpu1: cpu@1 {
>> + compatible = "arm,cortex-a53", "arm,armv8";
>> + device_type = "cpu";
>> + reg = <0x0 0x1>;
>> + enable-method = "spin-table";
>> + cpu-release-addr = <0x0 0x0000fff8>;
>> + next-level-cache = <&L2_0>;
>> + };
>> + L2_0: l2-cache0 {
>> + compatible = "cache";
>> + };
>> + };
>> +
>> + timer {
>> + compatible = "arm,armv8-timer";
>> + interrupts =
>> + <GIC_PPI 13
>> + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
>> + <GIC_PPI 14
>> + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
>> + <GIC_PPI 11
>> + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
>> + <GIC_PPI 10
>> + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
>
> You have a GICv3. These interrupt specifiers are not the ones GICv3
> expects.
I'll go back and look at this.
>
>> + };
>> +
>> + clocks: clocks {
>> + #address-cells = <2>;
>> + #size-cells = <1>;
>> + ranges;
>> + ahb_clk: ahb-clk {
>> + compatible = "fixed-clock";
>> + #clock-cells = <0>;
>> + clock-frequency = <250000000>;
>> + };
>> + sys_clk: sys-clk {
>> + compatible = "fixed-clock";
>> + #clock-cells = <0>;
>> + clock-frequency = <625000000>;
>> + };
>> + };
>> +
>> + axi: axi@600000000 {
>> + compatible = "simple-bus";
>> + #address-cells = <2>;
>> + #size-cells = <1>;
>> + ranges;
>> +
>> + gic: interrupt-controller@600300000 {
>> + compatible = "arm,gic-v3";
>> + #interrupt-cells = <3>;
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + interrupt-controller;
>> + reg = <0x6 0x00300000 0x20000>, /* GICD */
>> + <0x6 0x00340000 0x1000000>; /* GICR */
>
> You are missing the GICv3 compatibility interfaces (GICV/GICH), which
> are implemented by the CPUs.
I'll get these sorted out.
>
>> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>> +
>> + uart0: serial@600100000 {
>> + compatible = "ns16550a";
>> + reg = <0x6 0x00100000 0x20>;
>> + clocks = <&ahb_clk>;
>> + reg-io-width = <4>;
>> + reg-shift = <2>;
>> + interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> + status = "disabled";
>> + };
>> +
>> + uart1: serial@600102000 {
>> + compatible = "ns16550a";
>> + reg = <0x6 0x00102000 0x20>;
>> + clocks = <&ahb_clk>;
>> + reg-io-width = <4>;
>> + reg-shift = <2>;
>> + interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> + status = "disabled";
>> + };
>> +
>> + timer1: timer@600105000 {
>> + compatible = "snps,dw-apb-timer";
>> + reg = <0x6 0x00105000 0x1000>;
>> + clocks = <&ahb_clk>;
>> + clock-names = "timer";
>> + interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>> +
>> + };
>> +};
>
> Where is the PMU node?
>
I'll add the PMU node.
Thank you for your comments!
> Thanks,
>
> M.
--
Lars Povlsen,
Microchip
On 2020-05-15 16:09, Lars Povlsen wrote:
[...]
>>> + cpu0: cpu@0 {
>>> + compatible = "arm,cortex-a53", "arm,armv8";
Side note: only one compatible string for the real CPU please, running a
DT bindings check should complain about that.
>>> + device_type = "cpu";
>>> + reg = <0x0 0x0>;
>>> + enable-method = "spin-table";
>>
>> Really? This is 2020, not 2012 any more. Surely a new platform
>> boots using PSCI, and not *this*.
>>
>
> We don't currently support PSCI. The platform does not have TrustZone,
> hence we don't use ATF.
AIUI, part of the purpose of ATF is to provide a nice standardised
platform interface regardless of whether you care about Secure software
or not. It shouldn't take much to knock up a trivial ATF port that just
uses an internal spin-table for its PSCI backend - in fact I suspect
that's probably just a copy-paste from the RPi3 port ;)
Robin.
Robin Murphy writes:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 2020-05-15 16:09, Lars Povlsen wrote:
> [...]
>>>> + cpu0: cpu@0 {
>>>> + compatible = "arm,cortex-a53", "arm,armv8";
>
> Side note: only one compatible string for the real CPU please, running a
> DT bindings check should complain about that.
>
I'll change this.
>>>> + device_type = "cpu";
>>>> + reg = <0x0 0x0>;
>>>> + enable-method = "spin-table";
>>>
>>> Really? This is 2020, not 2012 any more. Surely a new platform
>>> boots using PSCI, and not *this*.
>>>
>>
>> We don't currently support PSCI. The platform does not have TrustZone,
>> hence we don't use ATF.
> AIUI, part of the purpose of ATF is to provide a nice standardised
> platform interface regardless of whether you care about Secure software
> or not. It shouldn't take much to knock up a trivial ATF port that just
> uses an internal spin-table for its PSCI backend - in fact I suspect
> that's probably just a copy-paste from the RPi3 port ;)
>
I'll change this to PSCI if that's whats expected these days. We
actually already have an ATF port. I fully understand the desire to
standardize on PSCI.
> Robin.
--
Lars Povlsen,
Microchip