2014-10-28 13:37:54

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform

From: Suravee Suthikulpanit <[email protected]>

Initial revision of device tree for AMD Seattle platform

Cc: Mark Rutland <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Catalin Marinas <[email protected]>
Signed-off-by: Suravee Suthikulpanit <[email protected]>
Signed-off-by: Thomas Lendacky <[email protected]>
Signed-off-by: Joel Schopp <[email protected]>
---
Change in V3:
* Change sata compatible-id to "snps,dwc-ahci"

arch/arm64/Kconfig | 5 +
arch/arm64/boot/dts/Makefile | 1 +
arch/arm64/boot/dts/amd-seattle-periph.dtsi | 164 ++++++++++++++++++++++++++++
arch/arm64/boot/dts/amd-seattle.dts | 122 +++++++++++++++++++++
4 files changed, 292 insertions(+)
create mode 100644 arch/arm64/boot/dts/amd-seattle-periph.dtsi
create mode 100644 arch/arm64/boot/dts/amd-seattle.dts

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index ef8b4f2..83fa301 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -143,6 +143,11 @@ source "kernel/Kconfig.freezer"

menu "Platform selection"

+config ARCH_SEATTLE
+ bool "AMD Seattle SoC Family"
+ help
+ This enables support for AMD Seattle SOC Family
+
config ARCH_THUNDER
bool "Cavium Inc. Thunder SoC Family"
help
diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
index f8001a6..604af09 100644
--- a/arch/arm64/boot/dts/Makefile
+++ b/arch/arm64/boot/dts/Makefile
@@ -1,3 +1,4 @@
+dtb-$(CONFIG_ARCH_SEATTLE) += amd-seattle.dtb
dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
diff --git a/arch/arm64/boot/dts/amd-seattle-periph.dtsi b/arch/arm64/boot/dts/amd-seattle-periph.dtsi
new file mode 100644
index 0000000..5a093a3
--- /dev/null
+++ b/arch/arm64/boot/dts/amd-seattle-periph.dtsi
@@ -0,0 +1,164 @@
+/*
+ * DTS file for AMD Seattle Peripheral
+ *
+ * Copyright (C) 2014 Advanced Micro Devices, Inc.
+ */
+
+motherboard {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ adl3clk_100mhz: clk100mhz_0 {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <100000000>;
+ clock-output-names = "adl3clk_100mhz";
+ };
+
+ ccpclk_375mhz: clk375mhz {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <375000000>;
+ clock-output-names = "ccpclk_375mhz";
+ };
+
+ sataclk_333mhz: clk333mhz {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <333000000>;
+ clock-output-names = "sataclk_333mhz";
+ };
+
+ pcieclk_500mhz: clk500mhz_0 {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <500000000>;
+ clock-output-names = "pcieclk_500mhz";
+ };
+
+ dmaclk_500mhz: clk500mhz_1 {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <500000000>;
+ clock-output-names = "dmaclk_500mhz";
+ };
+
+ miscclk_250mhz: clk250mhz_4 {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <250000000>;
+ clock-output-names = "miscclk_250mhz";
+ };
+
+ uartspiclk_100mhz: clk100mhz_1 {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <100000000>;
+ clock-output-names = "uartspiclk_100mhz";
+ };
+
+ dma0: dma@0500000 {
+ compatible = "arm,pl330", "arm,primecell";
+ reg = <0 0x0500000 0 0x1000>;
+ interrupts =
+ <0 368 4>,
+ <0 369 4>,
+ <0 370 4>,
+ <0 371 4>,
+ <0 372 4>,
+ <0 373 4>,
+ <0 374 4>,
+ <0 375 4>;
+ clocks = <&dmaclk_500mhz>;
+ clock-names = "apb_pclk";
+ #dma-cells = <1>;
+ };
+
+ sata0: sata@00300000 {
+ compatible = "snps,dwc-ahci";
+ reg = <0 0x300000 0 0x800>;
+ interrupts = <0 355 4>;
+ clocks = <&sataclk_333mhz>;
+ clock-names = "apb_pclk";
+ dma-coherent;
+ };
+
+ i2c@1000000 {
+ compatible = "snps,designware-i2c";
+ reg = <0 0x01000000 0 0x1000>;
+ interrupts = <0 357 4>;
+ clocks = <&uartspiclk_100mhz>;
+ clock-names = "apb_pclk";
+ };
+
+ serial0: serial@1010000 {
+ compatible = "arm,pl011", "arm,primecell";
+ reg = <0 0x1010000 0 0x1000>;
+ interrupts = <0 328 4>;
+ clocks = <&uartspiclk_100mhz>, <&uartspiclk_100mhz>;
+ clock-names = "uartclk", "apb_pclk";
+ };
+
+ ssp@1020000 {
+ compatible = "arm,pl022", "arm,primecell";
+ #gpio-cells = <2>;
+ reg = <0 0x1020000 0 0x1000>;
+ spi-controller;
+ interrupts = <0 330 4>;
+ clocks = <&uartspiclk_100mhz>;
+ clock-names = "apb_pclk";
+ };
+
+ ssp@1030000 {
+ compatible = "arm,pl022", "arm,primecell";
+ #gpio-cells = <2>;
+ reg = <0 0x1030000 0 0x1000>;
+ spi-controller;
+ interrupts = <0 329 4>;
+ clocks = <&uartspiclk_100mhz>;
+ clock-names = "apb_pclk";
+ num-cs = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sdcard@0 {
+ compatible = "mmc-spi-slot";
+ reg = <0>;
+ spi-max-frequency = <20000000>;
+ pl022,hierarchy = <0>;
+ pl022,interface = <0>;
+ pl022,com-mode = <0x0>;
+ pl022,rx-level-trig = <0>;
+ pl022,tx-level-trig = <0>;
+ };
+ };
+
+ gpio0: gpio@1040000 {
+ compatible = "arm,pl061", "arm,primecell";
+ #gpio-cells = <2>;
+ reg = <0 0x1040000 0 0x1000>;
+ gpio-controller;
+ interrupts = <0 359 4>;
+ clocks = <&uartspiclk_100mhz>;
+ clock-names = "apb_pclk";
+ };
+
+ gpio1: gpio@1050000 {
+ compatible = "arm,pl061", "arm,primecell";
+ #gpio-cells = <2>;
+ reg = <0 0x1050000 0 0x1000>;
+ gpio-controller;
+ interrupts = <0 358 4>;
+ clocks = <&uartspiclk_100mhz>;
+ clock-names = "apb_pclk";
+ };
+
+ ccp: ccp@00100000 {
+ compatible = "amd,ccp-seattle-v1a";
+ reg = <0 0x00100000 0 0x10000>;
+ interrupts = <0 3 4>;
+ dma-coherent;
+ };
+};
diff --git a/arch/arm64/boot/dts/amd-seattle.dts b/arch/arm64/boot/dts/amd-seattle.dts
new file mode 100644
index 0000000..726e444
--- /dev/null
+++ b/arch/arm64/boot/dts/amd-seattle.dts
@@ -0,0 +1,122 @@
+/*
+ * DTS file for AMD Seattle
+ *
+ * Copyright (C) 2014 Advanced Micro Devices, Inc.
+ */
+
+/dts-v1/;
+
+/ {
+ compatible = "amd,seattle";
+ interrupt-parent = <&gic>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ chosen {
+ stdout-path = &serial0;
+ linux,pci-probe-only;
+ };
+
+ gic: interrupt-controller@e1101000 {
+ compatible = "arm,gic-400", "arm,cortex-a15-gic";
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ reg = <0x0 0xe1110000 0 0x1000>,
+ <0x0 0xe112f000 0 0x2000>,
+ <0x0 0xe1140000 0 0x10000>,
+ <0x0 0xe1160000 0 0x10000>;
+ interrupts = <1 8 0xf04>;
+ ranges;
+ v2m0: v2m@e1180000 {
+ compatible = "arm,gic-v2m-frame";
+ msi-controller;
+ arm,msi-base-spi = <64>;
+ arm,msi-num-spis = <256>;
+ reg = <0x0 0xe1180000 0 0x1000>;
+ };
+ };
+
+ timer {
+ compatible = "arm,armv8-timer";
+ interrupts = <1 13 0xff01>,
+ <1 14 0xff01>,
+ <1 11 0xff01>,
+ <1 10 0xff01>;
+ };
+
+ pmu {
+ compatible = "arm,armv8-pmuv3";
+ interrupts = <0 7 4>,
+ <0 8 4>,
+ <0 9 4>,
+ <0 10 4>,
+ <0 11 4>,
+ <0 12 4>,
+ <0 13 4>,
+ <0 14 4>;
+ };
+
+ /* This entry is modified by UEFI */
+ pcie0: pcie-controller{
+ compatible = "pci-host-ecam-generic";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ device_type = "pci";
+ bus-range = <0 0xff>;
+ reg = <0 0xf0000000 0 0x10000000>;
+ dma-coherent;
+ msi-parent = <&v2m0>;
+
+ interrupts =
+ <0 320 4>, /* ioc_soc_serr */
+ <0 321 4>; /* ioc_soc_sci */
+
+ ranges =
+ /* I/O Memory (size=64K) */
+ <0x01000000 0x00 0xefff0000 0x00 0xefff0000 0x00 0x00010000>,
+
+ /* Non-Pref 32-bit MMIO (size=512M) */
+ <0x02000000 0x00 0x40000000 0x00 0x40000000 0x00 0x20000000>,
+
+ /* Non-Pref 32-bit MMIO (size=512M) */
+ <0x02000000 0x00 0x60000000 0x00 0x60000000 0x00 0x20000000>,
+
+ /* Non-Pref 32-bit MMIO (size=512M) */
+ <0x02000000 0x00 0x80000000 0x00 0x80000000 0x00 0x20000000>,
+
+ /* Non-Pref 32-bit MMIO (size=512M) */
+ <0x02000000 0x00 0xa0000000 0x00 0xa0000000 0x00 0x20000000>,
+
+ /* Pref 64-bit MMIO (size= 4G) */
+ <0x43000000 0x01 0x00000000 0x01 0x00000000 0x01 0x00000000>,
+
+ /* Pref 64-bit MMIO (size= 8G) */
+ <0x43000000 0x02 0x00000000 0x02 0x00000000 0x02 0x00000000>,
+
+ /* Pref 64-bit MMIO (size=16G) */
+ <0x43000000 0x04 0x00000000 0x04 0x00000000 0x04 0x00000000>,
+
+ /* Pref 64-bit MMIO (size=32G) */
+ <0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000>,
+
+ /* Pref 64-bit MMIO (size=64G) */
+ <0x43000000 0x10 0x00000000 0x10 0x00000000 0x10 0x00000000>,
+
+ /* Pref 64-bit MMIO (size=128G) */
+ <0x43000000 0x20 0x00000000 0x20 0x00000000 0x20 0x00000000>,
+
+ /* Pref 64-bit MMIO (size=256G) */
+ <0x43000000 0x40 0x00000000 0x40 0x00000000 0x40 0x00000000>;
+ };
+
+ smb {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges = <0 0 0 0xE0000000 0 0x01300000>;
+
+ /include/ "amd-seattle-periph.dtsi"
+ };
+};
--
1.9.3


2014-11-11 01:24:52

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform

Ping.

Are there any other concerns about this patch?

Thanks,

Suravee

On 10/28/14 20:36, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> Initial revision of device tree for AMD Seattle platform
>
> Cc: Mark Rutland <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> Signed-off-by: Thomas Lendacky <[email protected]>
> Signed-off-by: Joel Schopp <[email protected]>
> ---
> Change in V3:
> * Change sata compatible-id to "snps,dwc-ahci"
>
> arch/arm64/Kconfig | 5 +
> arch/arm64/boot/dts/Makefile | 1 +
> arch/arm64/boot/dts/amd-seattle-periph.dtsi | 164 ++++++++++++++++++++++++++++
> arch/arm64/boot/dts/amd-seattle.dts | 122 +++++++++++++++++++++
> 4 files changed, 292 insertions(+)
> create mode 100644 arch/arm64/boot/dts/amd-seattle-periph.dtsi
> create mode 100644 arch/arm64/boot/dts/amd-seattle.dts
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index ef8b4f2..83fa301 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -143,6 +143,11 @@ source "kernel/Kconfig.freezer"
>
> menu "Platform selection"
>
> +config ARCH_SEATTLE
> + bool "AMD Seattle SoC Family"
> + help
> + This enables support for AMD Seattle SOC Family
> +
> config ARCH_THUNDER
> bool "Cavium Inc. Thunder SoC Family"
> help
> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> index f8001a6..604af09 100644
> --- a/arch/arm64/boot/dts/Makefile
> +++ b/arch/arm64/boot/dts/Makefile
> @@ -1,3 +1,4 @@
> +dtb-$(CONFIG_ARCH_SEATTLE) += amd-seattle.dtb
> dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
> dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
> dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
> diff --git a/arch/arm64/boot/dts/amd-seattle-periph.dtsi b/arch/arm64/boot/dts/amd-seattle-periph.dtsi
> new file mode 100644
> index 0000000..5a093a3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amd-seattle-periph.dtsi
> @@ -0,0 +1,164 @@
> +/*
> + * DTS file for AMD Seattle Peripheral
> + *
> + * Copyright (C) 2014 Advanced Micro Devices, Inc.
> + */
> +
> +motherboard {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + adl3clk_100mhz: clk100mhz_0 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <100000000>;
> + clock-output-names = "adl3clk_100mhz";
> + };
> +
> + ccpclk_375mhz: clk375mhz {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <375000000>;
> + clock-output-names = "ccpclk_375mhz";
> + };
> +
> + sataclk_333mhz: clk333mhz {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <333000000>;
> + clock-output-names = "sataclk_333mhz";
> + };
> +
> + pcieclk_500mhz: clk500mhz_0 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <500000000>;
> + clock-output-names = "pcieclk_500mhz";
> + };
> +
> + dmaclk_500mhz: clk500mhz_1 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <500000000>;
> + clock-output-names = "dmaclk_500mhz";
> + };
> +
> + miscclk_250mhz: clk250mhz_4 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <250000000>;
> + clock-output-names = "miscclk_250mhz";
> + };
> +
> + uartspiclk_100mhz: clk100mhz_1 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <100000000>;
> + clock-output-names = "uartspiclk_100mhz";
> + };
> +
> + dma0: dma@0500000 {
> + compatible = "arm,pl330", "arm,primecell";
> + reg = <0 0x0500000 0 0x1000>;
> + interrupts =
> + <0 368 4>,
> + <0 369 4>,
> + <0 370 4>,
> + <0 371 4>,
> + <0 372 4>,
> + <0 373 4>,
> + <0 374 4>,
> + <0 375 4>;
> + clocks = <&dmaclk_500mhz>;
> + clock-names = "apb_pclk";
> + #dma-cells = <1>;
> + };
> +
> + sata0: sata@00300000 {
> + compatible = "snps,dwc-ahci";
> + reg = <0 0x300000 0 0x800>;
> + interrupts = <0 355 4>;
> + clocks = <&sataclk_333mhz>;
> + clock-names = "apb_pclk";
> + dma-coherent;
> + };
> +
> + i2c@1000000 {
> + compatible = "snps,designware-i2c";
> + reg = <0 0x01000000 0 0x1000>;
> + interrupts = <0 357 4>;
> + clocks = <&uartspiclk_100mhz>;
> + clock-names = "apb_pclk";
> + };
> +
> + serial0: serial@1010000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0 0x1010000 0 0x1000>;
> + interrupts = <0 328 4>;
> + clocks = <&uartspiclk_100mhz>, <&uartspiclk_100mhz>;
> + clock-names = "uartclk", "apb_pclk";
> + };
> +
> + ssp@1020000 {
> + compatible = "arm,pl022", "arm,primecell";
> + #gpio-cells = <2>;
> + reg = <0 0x1020000 0 0x1000>;
> + spi-controller;
> + interrupts = <0 330 4>;
> + clocks = <&uartspiclk_100mhz>;
> + clock-names = "apb_pclk";
> + };
> +
> + ssp@1030000 {
> + compatible = "arm,pl022", "arm,primecell";
> + #gpio-cells = <2>;
> + reg = <0 0x1030000 0 0x1000>;
> + spi-controller;
> + interrupts = <0 329 4>;
> + clocks = <&uartspiclk_100mhz>;
> + clock-names = "apb_pclk";
> + num-cs = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + sdcard@0 {
> + compatible = "mmc-spi-slot";
> + reg = <0>;
> + spi-max-frequency = <20000000>;
> + pl022,hierarchy = <0>;
> + pl022,interface = <0>;
> + pl022,com-mode = <0x0>;
> + pl022,rx-level-trig = <0>;
> + pl022,tx-level-trig = <0>;
> + };
> + };
> +
> + gpio0: gpio@1040000 {
> + compatible = "arm,pl061", "arm,primecell";
> + #gpio-cells = <2>;
> + reg = <0 0x1040000 0 0x1000>;
> + gpio-controller;
> + interrupts = <0 359 4>;
> + clocks = <&uartspiclk_100mhz>;
> + clock-names = "apb_pclk";
> + };
> +
> + gpio1: gpio@1050000 {
> + compatible = "arm,pl061", "arm,primecell";
> + #gpio-cells = <2>;
> + reg = <0 0x1050000 0 0x1000>;
> + gpio-controller;
> + interrupts = <0 358 4>;
> + clocks = <&uartspiclk_100mhz>;
> + clock-names = "apb_pclk";
> + };
> +
> + ccp: ccp@00100000 {
> + compatible = "amd,ccp-seattle-v1a";
> + reg = <0 0x00100000 0 0x10000>;
> + interrupts = <0 3 4>;
> + dma-coherent;
> + };
> +};
> diff --git a/arch/arm64/boot/dts/amd-seattle.dts b/arch/arm64/boot/dts/amd-seattle.dts
> new file mode 100644
> index 0000000..726e444
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amd-seattle.dts
> @@ -0,0 +1,122 @@
> +/*
> + * DTS file for AMD Seattle
> + *
> + * Copyright (C) 2014 Advanced Micro Devices, Inc.
> + */
> +
> +/dts-v1/;
> +
> +/ {
> + compatible = "amd,seattle";
> + interrupt-parent = <&gic>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + chosen {
> + stdout-path = &serial0;
> + linux,pci-probe-only;
> + };
> +
> + gic: interrupt-controller@e1101000 {
> + compatible = "arm,gic-400", "arm,cortex-a15-gic";
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + reg = <0x0 0xe1110000 0 0x1000>,
> + <0x0 0xe112f000 0 0x2000>,
> + <0x0 0xe1140000 0 0x10000>,
> + <0x0 0xe1160000 0 0x10000>;
> + interrupts = <1 8 0xf04>;
> + ranges;
> + v2m0: v2m@e1180000 {
> + compatible = "arm,gic-v2m-frame";
> + msi-controller;
> + arm,msi-base-spi = <64>;
> + arm,msi-num-spis = <256>;
> + reg = <0x0 0xe1180000 0 0x1000>;
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = <1 13 0xff01>,
> + <1 14 0xff01>,
> + <1 11 0xff01>,
> + <1 10 0xff01>;
> + };
> +
> + pmu {
> + compatible = "arm,armv8-pmuv3";
> + interrupts = <0 7 4>,
> + <0 8 4>,
> + <0 9 4>,
> + <0 10 4>,
> + <0 11 4>,
> + <0 12 4>,
> + <0 13 4>,
> + <0 14 4>;
> + };
> +
> + /* This entry is modified by UEFI */
> + pcie0: pcie-controller{
> + compatible = "pci-host-ecam-generic";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + bus-range = <0 0xff>;
> + reg = <0 0xf0000000 0 0x10000000>;
> + dma-coherent;
> + msi-parent = <&v2m0>;
> +
> + interrupts =
> + <0 320 4>, /* ioc_soc_serr */
> + <0 321 4>; /* ioc_soc_sci */
> +
> + ranges =
> + /* I/O Memory (size=64K) */
> + <0x01000000 0x00 0xefff0000 0x00 0xefff0000 0x00 0x00010000>,
> +
> + /* Non-Pref 32-bit MMIO (size=512M) */
> + <0x02000000 0x00 0x40000000 0x00 0x40000000 0x00 0x20000000>,
> +
> + /* Non-Pref 32-bit MMIO (size=512M) */
> + <0x02000000 0x00 0x60000000 0x00 0x60000000 0x00 0x20000000>,
> +
> + /* Non-Pref 32-bit MMIO (size=512M) */
> + <0x02000000 0x00 0x80000000 0x00 0x80000000 0x00 0x20000000>,
> +
> + /* Non-Pref 32-bit MMIO (size=512M) */
> + <0x02000000 0x00 0xa0000000 0x00 0xa0000000 0x00 0x20000000>,
> +
> + /* Pref 64-bit MMIO (size= 4G) */
> + <0x43000000 0x01 0x00000000 0x01 0x00000000 0x01 0x00000000>,
> +
> + /* Pref 64-bit MMIO (size= 8G) */
> + <0x43000000 0x02 0x00000000 0x02 0x00000000 0x02 0x00000000>,
> +
> + /* Pref 64-bit MMIO (size=16G) */
> + <0x43000000 0x04 0x00000000 0x04 0x00000000 0x04 0x00000000>,
> +
> + /* Pref 64-bit MMIO (size=32G) */
> + <0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000>,
> +
> + /* Pref 64-bit MMIO (size=64G) */
> + <0x43000000 0x10 0x00000000 0x10 0x00000000 0x10 0x00000000>,
> +
> + /* Pref 64-bit MMIO (size=128G) */
> + <0x43000000 0x20 0x00000000 0x20 0x00000000 0x20 0x00000000>,
> +
> + /* Pref 64-bit MMIO (size=256G) */
> + <0x43000000 0x40 0x00000000 0x40 0x00000000 0x40 0x00000000>;
> + };
> +
> + smb {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0 0 0 0xE0000000 0 0x01300000>;
> +
> + /include/ "amd-seattle-periph.dtsi"
> + };
> +};
>

2014-11-13 11:00:21

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform

On Tue, Nov 11, 2014 at 01:24:39AM +0000, Suravee Suthikulpanit wrote:
> Are there any other concerns about this patch?

Let's cc the arm-soc guys, they are now handling SoC code for arm64.

> On 10/28/14 20:36, [email protected] wrote:
> > From: Suravee Suthikulpanit <[email protected]>
> >
> > Initial revision of device tree for AMD Seattle platform
> >
> > Cc: Mark Rutland <[email protected]>
> > Cc: Will Deacon <[email protected]>
> > Cc: Catalin Marinas <[email protected]>
> > Signed-off-by: Suravee Suthikulpanit <[email protected]>
> > Signed-off-by: Thomas Lendacky <[email protected]>
> > Signed-off-by: Joel Schopp <[email protected]>
> > ---
> > Change in V3:
> > * Change sata compatible-id to "snps,dwc-ahci"
> >
> > arch/arm64/Kconfig | 5 +
> > arch/arm64/boot/dts/Makefile | 1 +
> > arch/arm64/boot/dts/amd-seattle-periph.dtsi | 164 ++++++++++++++++++++++++++++
> > arch/arm64/boot/dts/amd-seattle.dts | 122 +++++++++++++++++++++
> > 4 files changed, 292 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/amd-seattle-periph.dtsi
> > create mode 100644 arch/arm64/boot/dts/amd-seattle.dts
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index ef8b4f2..83fa301 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -143,6 +143,11 @@ source "kernel/Kconfig.freezer"
> >
> > menu "Platform selection"
> >
> > +config ARCH_SEATTLE
> > + bool "AMD Seattle SoC Family"
> > + help
> > + This enables support for AMD Seattle SOC Family
> > +
> > config ARCH_THUNDER
> > bool "Cavium Inc. Thunder SoC Family"
> > help
> > diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> > index f8001a6..604af09 100644
> > --- a/arch/arm64/boot/dts/Makefile
> > +++ b/arch/arm64/boot/dts/Makefile
> > @@ -1,3 +1,4 @@
> > +dtb-$(CONFIG_ARCH_SEATTLE) += amd-seattle.dtb
> > dtb-$(CONFIG_ARCH_THUNDER) += thunder-88xx.dtb
> > dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb
> > dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb
> > diff --git a/arch/arm64/boot/dts/amd-seattle-periph.dtsi b/arch/arm64/boot/dts/amd-seattle-periph.dtsi
> > new file mode 100644
> > index 0000000..5a093a3
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/amd-seattle-periph.dtsi
> > @@ -0,0 +1,164 @@
> > +/*
> > + * DTS file for AMD Seattle Peripheral
> > + *
> > + * Copyright (C) 2014 Advanced Micro Devices, Inc.
> > + */
> > +
> > +motherboard {
> > + compatible = "simple-bus";
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
> > +
> > + adl3clk_100mhz: clk100mhz_0 {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <100000000>;
> > + clock-output-names = "adl3clk_100mhz";
> > + };
> > +
> > + ccpclk_375mhz: clk375mhz {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <375000000>;
> > + clock-output-names = "ccpclk_375mhz";
> > + };
> > +
> > + sataclk_333mhz: clk333mhz {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <333000000>;
> > + clock-output-names = "sataclk_333mhz";
> > + };
> > +
> > + pcieclk_500mhz: clk500mhz_0 {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <500000000>;
> > + clock-output-names = "pcieclk_500mhz";
> > + };
> > +
> > + dmaclk_500mhz: clk500mhz_1 {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <500000000>;
> > + clock-output-names = "dmaclk_500mhz";
> > + };
> > +
> > + miscclk_250mhz: clk250mhz_4 {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <250000000>;
> > + clock-output-names = "miscclk_250mhz";
> > + };
> > +
> > + uartspiclk_100mhz: clk100mhz_1 {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <100000000>;
> > + clock-output-names = "uartspiclk_100mhz";
> > + };
> > +
> > + dma0: dma@0500000 {
> > + compatible = "arm,pl330", "arm,primecell";
> > + reg = <0 0x0500000 0 0x1000>;
> > + interrupts =
> > + <0 368 4>,
> > + <0 369 4>,
> > + <0 370 4>,
> > + <0 371 4>,
> > + <0 372 4>,
> > + <0 373 4>,
> > + <0 374 4>,
> > + <0 375 4>;
> > + clocks = <&dmaclk_500mhz>;
> > + clock-names = "apb_pclk";
> > + #dma-cells = <1>;
> > + };
> > +
> > + sata0: sata@00300000 {
> > + compatible = "snps,dwc-ahci";
> > + reg = <0 0x300000 0 0x800>;
> > + interrupts = <0 355 4>;
> > + clocks = <&sataclk_333mhz>;
> > + clock-names = "apb_pclk";
> > + dma-coherent;
> > + };
> > +
> > + i2c@1000000 {
> > + compatible = "snps,designware-i2c";
> > + reg = <0 0x01000000 0 0x1000>;
> > + interrupts = <0 357 4>;
> > + clocks = <&uartspiclk_100mhz>;
> > + clock-names = "apb_pclk";
> > + };
> > +
> > + serial0: serial@1010000 {
> > + compatible = "arm,pl011", "arm,primecell";
> > + reg = <0 0x1010000 0 0x1000>;
> > + interrupts = <0 328 4>;
> > + clocks = <&uartspiclk_100mhz>, <&uartspiclk_100mhz>;
> > + clock-names = "uartclk", "apb_pclk";
> > + };
> > +
> > + ssp@1020000 {
> > + compatible = "arm,pl022", "arm,primecell";
> > + #gpio-cells = <2>;
> > + reg = <0 0x1020000 0 0x1000>;
> > + spi-controller;
> > + interrupts = <0 330 4>;
> > + clocks = <&uartspiclk_100mhz>;
> > + clock-names = "apb_pclk";
> > + };
> > +
> > + ssp@1030000 {
> > + compatible = "arm,pl022", "arm,primecell";
> > + #gpio-cells = <2>;
> > + reg = <0 0x1030000 0 0x1000>;
> > + spi-controller;
> > + interrupts = <0 329 4>;
> > + clocks = <&uartspiclk_100mhz>;
> > + clock-names = "apb_pclk";
> > + num-cs = <1>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + sdcard@0 {
> > + compatible = "mmc-spi-slot";
> > + reg = <0>;
> > + spi-max-frequency = <20000000>;
> > + pl022,hierarchy = <0>;
> > + pl022,interface = <0>;
> > + pl022,com-mode = <0x0>;
> > + pl022,rx-level-trig = <0>;
> > + pl022,tx-level-trig = <0>;
> > + };
> > + };
> > +
> > + gpio0: gpio@1040000 {
> > + compatible = "arm,pl061", "arm,primecell";
> > + #gpio-cells = <2>;
> > + reg = <0 0x1040000 0 0x1000>;
> > + gpio-controller;
> > + interrupts = <0 359 4>;
> > + clocks = <&uartspiclk_100mhz>;
> > + clock-names = "apb_pclk";
> > + };
> > +
> > + gpio1: gpio@1050000 {
> > + compatible = "arm,pl061", "arm,primecell";
> > + #gpio-cells = <2>;
> > + reg = <0 0x1050000 0 0x1000>;
> > + gpio-controller;
> > + interrupts = <0 358 4>;
> > + clocks = <&uartspiclk_100mhz>;
> > + clock-names = "apb_pclk";
> > + };
> > +
> > + ccp: ccp@00100000 {
> > + compatible = "amd,ccp-seattle-v1a";
> > + reg = <0 0x00100000 0 0x10000>;
> > + interrupts = <0 3 4>;
> > + dma-coherent;
> > + };
> > +};
> > diff --git a/arch/arm64/boot/dts/amd-seattle.dts b/arch/arm64/boot/dts/amd-seattle.dts
> > new file mode 100644
> > index 0000000..726e444
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/amd-seattle.dts
> > @@ -0,0 +1,122 @@
> > +/*
> > + * DTS file for AMD Seattle
> > + *
> > + * Copyright (C) 2014 Advanced Micro Devices, Inc.
> > + */
> > +
> > +/dts-v1/;
> > +
> > +/ {
> > + compatible = "amd,seattle";
> > + interrupt-parent = <&gic>;
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + chosen {
> > + stdout-path = &serial0;
> > + linux,pci-probe-only;
> > + };
> > +
> > + gic: interrupt-controller@e1101000 {
> > + compatible = "arm,gic-400", "arm,cortex-a15-gic";
> > + interrupt-controller;
> > + #interrupt-cells = <3>;
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + reg = <0x0 0xe1110000 0 0x1000>,
> > + <0x0 0xe112f000 0 0x2000>,
> > + <0x0 0xe1140000 0 0x10000>,
> > + <0x0 0xe1160000 0 0x10000>;
> > + interrupts = <1 8 0xf04>;
> > + ranges;
> > + v2m0: v2m@e1180000 {
> > + compatible = "arm,gic-v2m-frame";
> > + msi-controller;
> > + arm,msi-base-spi = <64>;
> > + arm,msi-num-spis = <256>;
> > + reg = <0x0 0xe1180000 0 0x1000>;
> > + };
> > + };
> > +
> > + timer {
> > + compatible = "arm,armv8-timer";
> > + interrupts = <1 13 0xff01>,
> > + <1 14 0xff01>,
> > + <1 11 0xff01>,
> > + <1 10 0xff01>;
> > + };
> > +
> > + pmu {
> > + compatible = "arm,armv8-pmuv3";
> > + interrupts = <0 7 4>,
> > + <0 8 4>,
> > + <0 9 4>,
> > + <0 10 4>,
> > + <0 11 4>,
> > + <0 12 4>,
> > + <0 13 4>,
> > + <0 14 4>;
> > + };
> > +
> > + /* This entry is modified by UEFI */
> > + pcie0: pcie-controller{
> > + compatible = "pci-host-ecam-generic";
> > + #address-cells = <3>;
> > + #size-cells = <2>;
> > + device_type = "pci";
> > + bus-range = <0 0xff>;
> > + reg = <0 0xf0000000 0 0x10000000>;
> > + dma-coherent;
> > + msi-parent = <&v2m0>;
> > +
> > + interrupts =
> > + <0 320 4>, /* ioc_soc_serr */
> > + <0 321 4>; /* ioc_soc_sci */
> > +
> > + ranges =
> > + /* I/O Memory (size=64K) */
> > + <0x01000000 0x00 0xefff0000 0x00 0xefff0000 0x00 0x00010000>,
> > +
> > + /* Non-Pref 32-bit MMIO (size=512M) */
> > + <0x02000000 0x00 0x40000000 0x00 0x40000000 0x00 0x20000000>,
> > +
> > + /* Non-Pref 32-bit MMIO (size=512M) */
> > + <0x02000000 0x00 0x60000000 0x00 0x60000000 0x00 0x20000000>,
> > +
> > + /* Non-Pref 32-bit MMIO (size=512M) */
> > + <0x02000000 0x00 0x80000000 0x00 0x80000000 0x00 0x20000000>,
> > +
> > + /* Non-Pref 32-bit MMIO (size=512M) */
> > + <0x02000000 0x00 0xa0000000 0x00 0xa0000000 0x00 0x20000000>,
> > +
> > + /* Pref 64-bit MMIO (size= 4G) */
> > + <0x43000000 0x01 0x00000000 0x01 0x00000000 0x01 0x00000000>,
> > +
> > + /* Pref 64-bit MMIO (size= 8G) */
> > + <0x43000000 0x02 0x00000000 0x02 0x00000000 0x02 0x00000000>,
> > +
> > + /* Pref 64-bit MMIO (size=16G) */
> > + <0x43000000 0x04 0x00000000 0x04 0x00000000 0x04 0x00000000>,
> > +
> > + /* Pref 64-bit MMIO (size=32G) */
> > + <0x43000000 0x08 0x00000000 0x08 0x00000000 0x08 0x00000000>,
> > +
> > + /* Pref 64-bit MMIO (size=64G) */
> > + <0x43000000 0x10 0x00000000 0x10 0x00000000 0x10 0x00000000>,
> > +
> > + /* Pref 64-bit MMIO (size=128G) */
> > + <0x43000000 0x20 0x00000000 0x20 0x00000000 0x20 0x00000000>,
> > +
> > + /* Pref 64-bit MMIO (size=256G) */
> > + <0x43000000 0x40 0x00000000 0x40 0x00000000 0x40 0x00000000>;
> > + };
> > +
> > + smb {
> > + compatible = "simple-bus";
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges = <0 0 0 0xE0000000 0 0x01300000>;
> > +
> > + /include/ "amd-seattle-periph.dtsi"
> > + };
> > +};

2014-11-13 11:30:22

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform

On Tuesday 28 October 2014 08:36:54 [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> Initial revision of device tree for AMD Seattle platform

Sorry for not looking at this earlier in enough detail.

> + dma0: dma@0500000 {
> + compatible = "arm,pl330", "arm,primecell";
> + reg = <0 0x0500000 0 0x1000>;
> + interrupts =
> + <0 368 4>,
> + <0 369 4>,
> + <0 370 4>,
> + <0 371 4>,
> + <0 372 4>,
> + <0 373 4>,
> + <0 374 4>,
> + <0 375 4>;
> + clocks = <&dmaclk_500mhz>;
> + clock-names = "apb_pclk";
> + #dma-cells = <1>;
> + };

Is this device cache-coherent?

Does it support larger than 32-bit DMA addresses?

> + sata0: sata@00300000 {
> + compatible = "snps,dwc-ahci";
> + reg = <0 0x300000 0 0x800>;
> + interrupts = <0 355 4>;
> + clocks = <&sataclk_333mhz>;
> + clock-names = "apb_pclk";
> + dma-coherent;
> + };

Same here: you list it as coherent, but not 64-bit DMA capable.
Is that intentional?

> + i2c@1000000 {
> + compatible = "snps,designware-i2c";
> + reg = <0 0x01000000 0 0x1000>;
> + interrupts = <0 357 4>;
> + clocks = <&uartspiclk_100mhz>;
> + clock-names = "apb_pclk";
> + };
> +
> + serial0: serial@1010000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0 0x1010000 0 0x1000>;
> + interrupts = <0 328 4>;
> + clocks = <&uartspiclk_100mhz>, <&uartspiclk_100mhz>;
> + clock-names = "uartclk", "apb_pclk";
> + };
> +
> + ssp@1020000 {
> + compatible = "arm,pl022", "arm,primecell";
> + #gpio-cells = <2>;
> + reg = <0 0x1020000 0 0x1000>;
> + spi-controller;
> + interrupts = <0 330 4>;
> + clocks = <&uartspiclk_100mhz>;
> + clock-names = "apb_pclk";
> + };

Should these three be connected to the DMA engine?

> + ccp: ccp@00100000 {
> + compatible = "amd,ccp-seattle-v1a";
> + reg = <0 0x00100000 0 0x10000>;
> + interrupts = <0 3 4>;
> + dma-coherent;
> + };

I see the driver hacks an 48-bit DMA mask into this one.
Please fix the driver and add an appropriate dma-ranges property.

> + /* This entry is modified by UEFI */

Can you explain which parts are modified by UEFI?

> + pcie0: pcie-controller{
> + compatible = "pci-host-ecam-generic";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + bus-range = <0 0xff>;
> + reg = <0 0xf0000000 0 0x10000000>;
> + dma-coherent;
> + msi-parent = <&v2m0>;

This surely needs a dma-ranges property to allow larger than 32-bit DMA.



> + interrupts =
> + <0 320 4>, /* ioc_soc_serr */
> + <0 321 4>; /* ioc_soc_sci */

The pci-host-ecam-generic binding does not allow an interrupts property.

You seem to be missing an interrupt-map property.


> + ranges =
> + /* I/O Memory (size=64K) */
> + <0x01000000 0x00 0xefff0000 0x00 0xefff0000 0x00 0x00010000>,

Are you able to map the I/O space to bus address zero instead in the
firmware? This looks like a firmware bug, I/O space should not
be identity-mapped but is normally expected to have low port numbers.

> + /* Non-Pref 32-bit MMIO (size=512M) */
> + <0x02000000 0x00 0x40000000 0x00 0x40000000 0x00 0x20000000>,
> +
> + /* Non-Pref 32-bit MMIO (size=512M) */
> + <0x02000000 0x00 0x60000000 0x00 0x60000000 0x00 0x20000000>,
> +
> + /* Non-Pref 32-bit MMIO (size=512M) */
> + <0x02000000 0x00 0x80000000 0x00 0x80000000 0x00 0x20000000>,
> +
> + /* Non-Pref 32-bit MMIO (size=512M) */
> + <0x02000000 0x00 0xa0000000 0x00 0xa0000000 0x00 0x20000000>,

I don't understand why you use distinct ranges here and below. These are all
contiguous, so why not collapse them into one logical range.

> + smb {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0 0 0 0xE0000000 0 0x01300000>;
> +
> + /include/ "amd-seattle-periph.dtsi"
> + };

I would put the smb node into the other file and move the include statement to the
top level.

Please use lowercase characters for the address.

Arnd

2014-11-20 12:34:28

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform

On Thursday 13 November 2014, Catalin Marinas wrote:
> On Tue, Nov 11, 2014 at 01:24:39AM +0000, Suravee Suthikulpanit wrote:
> > Are there any other concerns about this patch?
>
> Let's cc the arm-soc guys, they are now handling SoC code for arm64.

Any update on the submission? I had some comments, am now waiting for a new
version to put into 3.19.

Arnd

2014-11-20 15:16:59

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform

On Thu, Nov 13, 2014 at 5:29 AM, Arnd Bergmann <[email protected]> wrote:
> On Tuesday 28 October 2014 08:36:54 [email protected] wrote:
>> From: Suravee Suthikulpanit <[email protected]>
>>
>> Initial revision of device tree for AMD Seattle platform
>
> Sorry for not looking at this earlier in enough detail.
>
>> + dma0: dma@0500000 {
>> + compatible = "arm,pl330", "arm,primecell";
>> + reg = <0 0x0500000 0 0x1000>;
>> + interrupts =
>> + <0 368 4>,
>> + <0 369 4>,
>> + <0 370 4>,
>> + <0 371 4>,
>> + <0 372 4>,
>> + <0 373 4>,
>> + <0 374 4>,
>> + <0 375 4>;
>> + clocks = <&dmaclk_500mhz>;
>> + clock-names = "apb_pclk";
>> + #dma-cells = <1>;
>> + };
>
> Is this device cache-coherent?
>
> Does it support larger than 32-bit DMA addresses?

No.

>
>> + sata0: sata@00300000 {
>> + compatible = "snps,dwc-ahci";

This should have an AMD specific compatible string in addition in case
you have AMD specific configuration or bugs.

>> + reg = <0 0x300000 0 0x800>;
>> + interrupts = <0 355 4>;
>> + clocks = <&sataclk_333mhz>;
>> + clock-names = "apb_pclk";

This name is obviously wrong and copied from other (ARM Primecell)
bindings as this IP does not have an APB PCLK. You can drop the name
as it is optional.

>> + dma-coherent;
>> + };
>
> Same here: you list it as coherent, but not 64-bit DMA capable.
> Is that intentional?

AHCI controllers are probeable as to whether they support 64-bit DMA or not.

Rob

2014-11-20 15:23:33

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform

On Thursday 20 November 2014 09:16:31 Rob Herring wrote:
>
> >> + dma-coherent;
> >> + };
> >
> > Same here: you list it as coherent, but not 64-bit DMA capable.
> > Is that intentional?
>
> AHCI controllers are probeable as to whether they support 64-bit DMA or not.
>

The controller is, but the bus is not, so we also need the dma-ranges
property in the bus.

Arnd

2014-11-21 01:12:54

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform

On 11/13/14 18:29, Arnd Bergmann wrote:
> On Tuesday 28 October 2014 08:36:54 [email protected] wrote:
>> From: Suravee Suthikulpanit <[email protected]>
>>
>> Initial revision of device tree for AMD Seattle platform
>
> Sorry for not looking at this earlier in enough detail.
>
>> + dma0: dma@0500000 {
>> + compatible = "arm,pl330", "arm,primecell";
>> + reg = <0 0x0500000 0 0x1000>;
>> + interrupts =
>> + <0 368 4>,
>> + <0 369 4>,
>> + <0 370 4>,
>> + <0 371 4>,
>> + <0 372 4>,
>> + <0 373 4>,
>> + <0 374 4>,
>> + <0 375 4>;
>> + clocks = <&dmaclk_500mhz>;
>> + clock-names = "apb_pclk";
>> + #dma-cells = <1>;
>> + };
>
> Is this device cache-coherent?
>
> Does it support larger than 32-bit DMA addresses?

The pl330 is only 32-bit DMA addressable, and need to be used with
the smmu (not yet included here) before it can be used in the system.
Therefore, it should be cache coherent by the virtue of the SMMU.

I?ll remove this until the SMMU stuff is tested and ready.

>
>> + sata0: sata@00300000 {
>> + compatible = "snps,dwc-ahci";
>> + reg = <0 0x300000 0 0x800>;
>> + interrupts = <0 355 4>;
>> + clocks = <&sataclk_333mhz>;
>> + clock-names = "apb_pclk";
>> + dma-coherent;
>> + };
>
> Same here: you list it as coherent, but not 64-bit DMA capable.
> Is that intentional?

Correct me if I am wrong, but I didn't think that we need to specify
here since the AHCI platform driver determines the DMA bitness by
checking struct ahci_host_priv.cap for HOST_CAP_64 (see
drivers/ata/libahci_platform.c).

However, based on the conversation on the IRC, I?ll add the dma-ranges
in the motherboard level.

>
>> + i2c@1000000 {
>> + compatible = "snps,designware-i2c";
>> + reg = <0 0x01000000 0 0x1000>;
>> + interrupts = <0 357 4>;
>> + clocks = <&uartspiclk_100mhz>;
>> + clock-names = "apb_pclk";
>> + };
>> +
>> + serial0: serial@1010000 {
>> + compatible = "arm,pl011", "arm,primecell";
>> + reg = <0 0x1010000 0 0x1000>;
>> + interrupts = <0 328 4>;
>> + clocks = <&uartspiclk_100mhz>, <&uartspiclk_100mhz>;
>> + clock-names = "uartclk", "apb_pclk";
>> + };
>> +
>> + ssp@1020000 {
>> + compatible = "arm,pl022", "arm,primecell";
>> + #gpio-cells = <2>;
>> + reg = <0 0x1020000 0 0x1000>;
>> + spi-controller;
>> + interrupts = <0 330 4>;
>> + clocks = <&uartspiclk_100mhz>;
>> + clock-names = "apb_pclk";
>> + };
>
> Should these three be connected to the DMA engine?

It doesn't do DMA. Only PCI devices, XGBE, and SATA do DMA.

>
>> + ccp: ccp@00100000 {
>> + compatible = "amd,ccp-seattle-v1a";
>> + reg = <0 0x00100000 0 0x10000>;
>> + interrupts = <0 3 4>;
>> + dma-coherent;
>> + };
>
> I see the driver hacks an 48-bit DMA mask into this one.
> Please fix the driver and add an appropriate dma-ranges property.
>

ok

>> + /* This entry is modified by UEFI */
>
> Can you explain which parts are modified by UEFI?
>

Actually, I need to remove this comment for now. I believe it was going
to be needed for the smmu stuff to specify the stream-id of each
end-point device.

>> + pcie0: pcie-controller{
>> + compatible = "pci-host-ecam-generic";
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> + device_type = "pci";
>> + bus-range = <0 0xff>;
>> + reg = <0 0xf0000000 0 0x10000000>;
>> + dma-coherent;
>> + msi-parent = <&v2m0>;
>
> This surely needs a dma-ranges property to allow larger than 32-bit DMA.

So, I assume this will also need dma-range handling code to be added to
the PCI generic host driver.

>
>> + interrupts =
>> + <0 320 4>, /* ioc_soc_serr */
>> + <0 321 4>; /* ioc_soc_sci */
>
> The pci-host-ecam-generic binding does not allow an interrupts property.
>
> You seem to be missing an interrupt-map property.

I?ll look into this.
>
>
>> + ranges =
>> + /* I/O Memory (size=64K) */
>> + <0x01000000 0x00 0xefff0000 0x00 0xefff0000 0x00 0x00010000>,
>
> Are you able to map the I/O space to bus address zero instead in the
> firmware? This looks like a firmware bug, I/O space should not
> be identity-mapped but is normally expected to have low port numbers.

I?ll look into this.
>
>> + /* Non-Pref 32-bit MMIO (size=512M) */
>> + <0x02000000 0x00 0x40000000 0x00 0x40000000 0x00 0x20000000>,
>> +
>> + /* Non-Pref 32-bit MMIO (size=512M) */
>> + <0x02000000 0x00 0x60000000 0x00 0x60000000 0x00 0x20000000>,
>> +
>> + /* Non-Pref 32-bit MMIO (size=512M) */
>> + <0x02000000 0x00 0x80000000 0x00 0x80000000 0x00 0x20000000>,
>> +
>> + /* Non-Pref 32-bit MMIO (size=512M) */
>> + <0x02000000 0x00 0xa0000000 0x00 0xa0000000 0x00 0x20000000>,
>
> I don't understand why you use distinct ranges here and below. These are
>all
> contiguous, so why not collapse them into one logical range.

I'll collapse them.
>
>> + smb {
>> + compatible = "simple-bus";
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges = <0 0 0 0xE0000000 0 0x01300000>;
>> +
>> + /include/ "amd-seattle-periph.dtsi"
>> + };
>
> I would put the smb node into the other file and move the include
>statement to the
> top level.
>
> Please use lowercase characters for the address.

I will made the changes accordingly.

Thanks,

Suravee

>
> Arnd
>

2014-11-21 12:38:56

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform

On Friday 21 November 2014 01:12:45 Suthikulpanit, Suravee wrote:
> On 11/13/14 18:29, Arnd Bergmann wrote:
> > On Tuesday 28 October 2014 08:36:54 [email protected] wrote:
> >> From: Suravee Suthikulpanit <[email protected]>
> >>
> >> Initial revision of device tree for AMD Seattle platform
> >
> > Sorry for not looking at this earlier in enough detail.
> >
> >> + dma0: dma@0500000 {
> >> + compatible = "arm,pl330", "arm,primecell";
> >> + reg = <0 0x0500000 0 0x1000>;
> >> + interrupts =
> >> + <0 368 4>,
> >> + <0 369 4>,
> >> + <0 370 4>,
> >> + <0 371 4>,
> >> + <0 372 4>,
> >> + <0 373 4>,
> >> + <0 374 4>,
> >> + <0 375 4>;
> >> + clocks = <&dmaclk_500mhz>;
> >> + clock-names = "apb_pclk";
> >> + #dma-cells = <1>;
> >> + };
> >
> > Is this device cache-coherent?
> >
> > Does it support larger than 32-bit DMA addresses?
>
> The pl330 is only 32-bit DMA addressable, and need to be used with
> the smmu (not yet included here) before it can be used in the system.
> Therefore, it should be cache coherent by the virtue of the SMMU.
>
> I?ll remove this until the SMMU stuff is tested and ready.

Ok, makes sense.

> >
> >> + sata0: sata@00300000 {
> >> + compatible = "snps,dwc-ahci";
> >> + reg = <0 0x300000 0 0x800>;
> >> + interrupts = <0 355 4>;
> >> + clocks = <&sataclk_333mhz>;
> >> + clock-names = "apb_pclk";
> >> + dma-coherent;
> >> + };
> >
> > Same here: you list it as coherent, but not 64-bit DMA capable.
> > Is that intentional?
>
> Correct me if I am wrong, but I didn't think that we need to specify
> here since the AHCI platform driver determines the DMA bitness by
> checking struct ahci_host_priv.cap for HOST_CAP_64 (see
> drivers/ata/libahci_platform.c).

No, the actual DMA mask that gets used is the combination of what
the device claims to support and what the bus can do. Without the
dma-ranges property, the bus will be seen as 32-bit only, so we won't
allow high DMA transfers for devices that can do it.

This is the same way we have to treat any PCI device as well, since
a lot of PCI devices can do 64-bit DMA, but they can also be connected
to a pci host bridge that sits on a 32-bit bus and has no supported
IOMMU.

> However, based on the conversation on the IRC, I?ll add the dma-ranges
> in the motherboard level.

Ok.

> >
> >> + i2c@1000000 {
> >> + compatible = "snps,designware-i2c";
> >> + reg = <0 0x01000000 0 0x1000>;
> >> + interrupts = <0 357 4>;
> >> + clocks = <&uartspiclk_100mhz>;
> >> + clock-names = "apb_pclk";
> >> + };
> >> +
> >> + serial0: serial@1010000 {
> >> + compatible = "arm,pl011", "arm,primecell";
> >> + reg = <0 0x1010000 0 0x1000>;
> >> + interrupts = <0 328 4>;
> >> + clocks = <&uartspiclk_100mhz>, <&uartspiclk_100mhz>;
> >> + clock-names = "uartclk", "apb_pclk";
> >> + };
> >> +
> >> + ssp@1020000 {
> >> + compatible = "arm,pl022", "arm,primecell";
> >> + #gpio-cells = <2>;
> >> + reg = <0 0x1020000 0 0x1000>;
> >> + spi-controller;
> >> + interrupts = <0 330 4>;
> >> + clocks = <&uartspiclk_100mhz>;
> >> + clock-names = "apb_pclk";
> >> + };
> >
> > Should these three be connected to the DMA engine?
>
> It doesn't do DMA. Only PCI devices, XGBE, and SATA do DMA.

What is the pl330 connected to then? It's very common for pl011
and pl022 to be used in combination with a pl330 in order to
do DMA.

> >> + pcie0: pcie-controller{
> >> + compatible = "pci-host-ecam-generic";
> >> + #address-cells = <3>;
> >> + #size-cells = <2>;
> >> + device_type = "pci";
> >> + bus-range = <0 0xff>;
> >> + reg = <0 0xf0000000 0 0x10000000>;
> >> + dma-coherent;
> >> + msi-parent = <&v2m0>;
> >
> > This surely needs a dma-ranges property to allow larger than 32-bit DMA.
>
> So, I assume this will also need dma-range handling code to be added to
> the PCI generic host driver.

Yes, good point.


> I will made the changes accordingly.

thanks,

Arnd

2014-11-21 12:58:07

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform

Hi Suravee,

On 28/10/14 13:36, [email protected] wrote:
> From: Suravee Suthikulpanit <[email protected]>
>
> Initial revision of device tree for AMD Seattle platform
>
> Cc: Mark Rutland <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> Signed-off-by: Thomas Lendacky <[email protected]>
> Signed-off-by: Joel Schopp <[email protected]>
> ---
> Change in V3:
> * Change sata compatible-id to "snps,dwc-ahci"
>
> arch/arm64/Kconfig | 5 +
> arch/arm64/boot/dts/Makefile | 1 +
> arch/arm64/boot/dts/amd-seattle-periph.dtsi | 164 ++++++++++++++++++++++++++++
> arch/arm64/boot/dts/amd-seattle.dts | 122 +++++++++++++++++++++
> 4 files changed, 292 insertions(+)
> create mode 100644 arch/arm64/boot/dts/amd-seattle-periph.dtsi
> create mode 100644 arch/arm64/boot/dts/amd-seattle.dts
>

[...]

> diff --git a/arch/arm64/boot/dts/amd-seattle.dts b/arch/arm64/boot/dts/amd-seattle.dts
> new file mode 100644
> index 0000000..726e444
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amd-seattle.dts
> @@ -0,0 +1,122 @@
> +/*
> + * DTS file for AMD Seattle
> + *
> + * Copyright (C) 2014 Advanced Micro Devices, Inc.
> + */
> +
> +/dts-v1/;
> +
> +/ {
> + compatible = "amd,seattle";
> + interrupt-parent = <&gic>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + chosen {
> + stdout-path = &serial0;
> + linux,pci-probe-only;
> + };
> +
> + gic: interrupt-controller@e1101000 {
> + compatible = "arm,gic-400", "arm,cortex-a15-gic";
> + interrupt-controller;
> + #interrupt-cells = <3>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + reg = <0x0 0xe1110000 0 0x1000>,
> + <0x0 0xe112f000 0 0x2000>,
> + <0x0 0xe1140000 0 0x10000>,
> + <0x0 0xe1160000 0 0x10000>;
> + interrupts = <1 8 0xf04>;

Are you sure about this one? ARM systems usually have this wired on PPI9
(interrupt 25)...

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2014-11-21 14:40:22

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform



On 11/21/14, 19:57, "Marc Zyngier" <[email protected]> wrote:

>>+ gic: interrupt-controller@e1101000 {
>>+ compatible = "arm,gic-400", "arm,cortex-a15-gic";
>>+ interrupt-controller;
>>+ #interrupt-cells = <3>;
>>+ #address-cells = <2>;
>>+ #size-cells = <2>;
>>+ reg = <0x0 0xe1110000 0 0x1000>,
>>+ <0x0 0xe112f000 0 0x2000>,
>>+ <0x0 0xe1140000 0 0x10000>,
>>+ <0x0 0xe1160000 0 0x10000>;
>>+ interrupts = <1 8 0xf04>;
>
>Are you sure about this one? ARM systems usually have this wired on PPI9
>(interrupt 25)...
>
>Thanks,
>
> M.

I think you might be right. GIC-400 TRM says that this is should be 25,
and used
for virtual maintenance interrupts. How can I verify this in Linux? KVM?

Thanks,

Suravee

2014-11-21 14:48:39

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform

On 21/11/14 14:40, Suthikulpanit, Suravee wrote:
>
>
> On 11/21/14, 19:57, "Marc Zyngier" <[email protected]> wrote:
>
>>> + gic: interrupt-controller@e1101000 {
>>> + compatible = "arm,gic-400", "arm,cortex-a15-gic";
>>> + interrupt-controller;
>>> + #interrupt-cells = <3>;
>>> + #address-cells = <2>;
>>> + #size-cells = <2>;
>>> + reg = <0x0 0xe1110000 0 0x1000>,
>>> + <0x0 0xe112f000 0 0x2000>,
>>> + <0x0 0xe1140000 0 0x10000>,
>>> + <0x0 0xe1160000 0 0x10000>;
>>> + interrupts = <1 8 0xf04>;
>>
>> Are you sure about this one? ARM systems usually have this wired on PPI9
>> (interrupt 25)...
>>
>> Thanks,
>>
>> M.
>
> I think you might be right. GIC-400 TRM says that this is should be
> 25, and used for virtual maintenance interrupts. How can I verify
> this in Linux? KVM?

KVM is one way, but you'll never see the interrupt firing (we kill the
interrupt while in HYP, before the kernel gets a chance to see it).

If you effectively have GIC400 on this system, then I know for sure this
is interrupt 25.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2014-11-21 16:26:05

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH V3] arm64: amd-seattle: Adding device tree for AMD Seattle platform



On 11/21/14, 21:48, "Marc Zyngier" <[email protected]> wrote:

>On 21/11/14 14:40, Suthikulpanit, Suravee wrote:
>>
>>
>> On 11/21/14, 19:57, "Marc Zyngier" <[email protected]> wrote:
>>
>>>> + gic: interrupt-controller@e1101000 {
>>>> + compatible = "arm,gic-400", "arm,cortex-a15-gic";
>>>> + interrupt-controller;
>>>> + #interrupt-cells = <3>;
>>>> + #address-cells = <2>;
>>>> + #size-cells = <2>;
>>>> + reg = <0x0 0xe1110000 0 0x1000>,
>>>> + <0x0 0xe112f000 0 0x2000>,
>>>> + <0x0 0xe1140000 0 0x10000>,
>>>> + <0x0 0xe1160000 0 0x10000>;
>>>> + interrupts = <1 8 0xf04>;
>>>
>>> Are you sure about this one? ARM systems usually have this wired on
>>>PPI9
>>> (interrupt 25)...
>>>
>>> Thanks,
>>>
>>> M.
>>
>> I think you might be right. GIC-400 TRM says that this is should be
>> 25, and used for virtual maintenance interrupts. How can I verify
>> this in Linux? KVM?
>
>KVM is one way, but you'll never see the interrupt firing (we kill the
>interrupt while in HYP, before the kernel gets a chance to see it).
>
>If you effectively have GIC400 on this system, then I know for sure this
>is interrupt 25.

I?ll make the change. Thanks for clarification.

Suravee
>
>Thanks,
>
> M.
>--
>Jazz is not dead. It just smells funny...