Support 64bit Cortex A57 based Exynos7 SoC
Subsystems (4)
cover Support 64bit Cortex A57 based Exynos7 SoC
This patchset supports new Exynos7 Samsung SoC based on Cortex-A57. Exynos7 is a System-On-Chip (SoC) that is based on 64-bit ARMv8 RISC processor.
This patchset includes the following:
- Support for pinctrl on Exynos7
- Support for clock control on Exynos7 using common clk framework
- DTS and DTSI file addition for Exynos7 based Espresso board
- Support for serial driver on Exynos7
- Support for HSI2C driver on Exynos7
- Support for Watchdog driver on Exynos7
NOTE:
We tested these patches with the
"arm64: dts: add <dt-bindings/> symlink" change posted @
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-January/224629.html
The following patches are tested based on Kgene's for-next tree.
https://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/log/?h=for-next
Alim Akhtar (2):
arm64: exynos7: Enable ARMv8 based Exynos7 (SoC) support tty/serial: samsung: add support for exynos7
Naveen Krishna Chatradhi (12):
clk: samsung: add support for 145xx and 1460x PLLs
clk: samsung: Factor out the common code to clk.c
clk: samsung: Add fixed_factor_clocks field to struct exynos_cmu_info
clk: samsung: Add clock description for basic CMU blocks
pinctrl: exynos: Add driver data for Exynos7
pinctrl: samsung: use CONFIG_PINCTRL_SAMSUNG symbol in makefile
i2c: exynos: add support for HSI2C module on Exynos7
i2c: exynos5: Add Kconfig dependencies
watchdog: s3c2410_wdt: Add support for Watchdog device on Exynos7
arm64: dts: add pinctrl support to EXYNOS7
arm64: dts: Add initial device tree support for EXYNOS7
arm64: dts: add Exynos7 based Espresso board dts file
.../devicetree/bindings/clock/exynos7-clock.txt | 209 +++ .../devicetree/bindings/i2c/i2c-exynos5.txt | 2 + .../bindings/pinctrl/samsung-pinctrl.txt | 1 + .../devicetree/bindings/watchdog/samsung-wdt.txt | 1 + arch/arm64/Kconfig | 17 + arch/arm64/boot/dts/Makefile | 1 + arch/arm64/boot/dts/exynos7-espresso.dts | 69 + arch/arm64/boot/dts/exynos7-pinctrl.dtsi | 691 +++++++ arch/arm64/boot/dts/exynos7.dtsi | 553 ++++++ drivers/clk/samsung/Makefile | 1 + drivers/clk/samsung/clk-exynos5260.c | 181 +- drivers/clk/samsung/clk-exynos7.c | 1929 ++++++++++++++++++++ drivers/clk/samsung/clk-pll.c | 50 +- drivers/clk/samsung/clk-pll.h | 4 + drivers/clk/samsung/clk.c | 104 ++ drivers/clk/samsung/clk.h | 37 + drivers/i2c/busses/Kconfig | 2 +- drivers/i2c/busses/i2c-exynos5.c | 71 +- drivers/pinctrl/Makefile | 2 +- drivers/pinctrl/samsung/Kconfig | 2 +- drivers/pinctrl/samsung/pinctrl-exynos.c | 145 ++ drivers/pinctrl/samsung/pinctrl-samsung.c | 2 + drivers/pinctrl/samsung/pinctrl-samsung.h | 1 + drivers/tty/serial/Kconfig | 6 +- drivers/tty/serial/samsung.c | 2 +- drivers/watchdog/s3c2410_wdt.c | 11 + include/dt-bindings/clock/exynos7-clk.h | 236 +++ 27 files changed, 4151 insertions(+), 179 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/exynos7-clock.txt create mode 100644 arch/arm64/boot/dts/exynos7-espresso.dts create mode 100644 arch/arm64/boot/dts/exynos7-pinctrl.dtsi create mode 100644 arch/arm64/boot/dts/exynos7.dtsi create mode 100644 drivers/clk/samsung/clk-exynos7.c create mode 100644 include/dt-bindings/clock/exynos7-clk.h
Signature · 1 line
1.7.9.5
PATCH 10/14 arm64: dts: add pinctrl support to EXYNOS7
Add the required pin configuration support to EXYNOS7 using pinctrl interface.
--- arch/arm64/boot/dts/exynos7-pinctrl.dtsi | 691 ++++++++++++++++++++++++++++++ 1 file changed, 691 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7-pinctrl.dtsi
arch/arm64/boot/dts/exynos7-pinctrl.dtsi new file +691 -0
@@ -0,0 +1,691 @@ +/* + * Samsung's Exynos7 SoC pin-mux and pin-config device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Samsung's Exynos7 SoC pin-mux and pin-config options are listed as + * device tree nodes are listed in this file. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ + +/ { + /* ALIVE block @10580000 */ + pinctrl at 10580000 { + gpa0: gpa0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + interrupt-parent = <&gic>; + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>; + }; + + gpa1: gpa1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpa2: gpa2 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpa3: gpa3 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + /* AUD block @114B0000 */ + pinctrl at 114B0000 { + gpz0: gpz0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpz1: gpz1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + /* BUS0 block @13470000 */ + pinctrl at 13470000 { + gpb0: gpb0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpc0: gpc0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpc1: gpc1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpc2: gpc2 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpc3: gpc3 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd0: gpd0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd1: gpd1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd2: gpd2 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd4: gpd4 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd5: gpd5 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd6: gpd6 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd7: gpd7 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpd8: gpd8 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpg0: gpg0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpg3: gpg3 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + /* HSI2C Sensor 0 */ + hs_i2c10_bus: hs-i2c10-bus { + samsung,pins = "gpb0-1", "gpb0-0"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + /* HSI2C Sensor 1 */ + hs_i2c11_bus: hs-i2c11-bus { + samsung,pins = "gpb0-3", "gpb0-2"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + hs_i2c2_bus: hs-i2c2-bus { + samsung,pins = "gpd0-3", "gpd0-2"; + samsung,pin-function = <3>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + uart0_data: uart0-data { + samsung,pins = "gpd0-0", "gpd0-1"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + uart0_fctl: uart0-fctl { + samsung,pins = "gpd0-2", "gpd0-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + uart2_data: uart2-data { + samsung,pins = "gpd1-4", "gpd1-5"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + hs_i2c3_bus: hs-i2c3-bus { + samsung,pins = "gpd1-3", "gpd1-2"; + samsung,pin-function = <3>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + uart1_data: uart1-data { + samsung,pins = "gpd1-0", "gpd1-1"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + uart1_fctl: uart1-fctl { + samsung,pins = "gpd1-2", "gpd1-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + hs_i2c0_bus: hs-i2c0-bus { + samsung,pins = "gpd2-1", "gpd2-0"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + hs_i2c1_bus: hs-i2c1-bus { + samsung,pins = "gpd2-3", "gpd2-2"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + hs_i2c9_bus: hs-i2c9-bus { + samsung,pins = "gpd2-7", "gpd2-6"; + samsung,pin-function = <3>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + hs_i2c8_bus: hs-i2c8-bus { + samsung,pins = "gpd5-3", "gpd5-2"; + samsung,pin-function = <3>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + uart3_data: uart3-data { + samsung,pins = "gpd5-0", "gpd5-1"; + samsung,pin-function = <3>; + samsung,pin-pud = <0>; + samsung,pin-drv = <0>; + }; + + disp_teson: disp_teson { + samsung,pins = "gpg0-1"; + samsung,pin-function = <2>; + }; + + disp_tesoff: disp_tesoff { + samsung,pins = "gpg0-1"; + samsung,pin-function = <0>; + }; + + spi2_bus: spi2-bus { + samsung,pins = "gpd5-0", "gpd5-1", "gpd5-2", "gpd5-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + spi1_bus: spi1-bus { + samsung,pins = "gpd6-2", "gpd6-3", "gpd6-4", "gpd6-5"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + spi0_bus: spi0-bus { + samsung,pins = "gpd8-0", "gpd8-1", "gpd6-0", "gpd6-1"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + /* HSI2C sensor 2 */ + hs_i2c4_bus: hs-i2c4-bus { + samsung,pins = "gpg3-1", "gpg3-0"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + + /* HSI2C sensor 3 */ + hs_i2c5_bus: hs-i2c5-bus { + samsung,pins = "gpg3-3", "gpg3-2"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + }; + + /* BUS1 block @14870000 */ + pinctrl at 14870000 { + etc1: etc1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpf0: gpf0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpf1: gpf1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpf2: gpf2 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpf3: gpf3 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpf4: gpf4 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpf5: gpf5 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpg1: gpg1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpg2: gpg2 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gph1: gph1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpv6: gpv6 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + }; + + /* NFC block @14CD0000 */ + pinctrl at 14CD0000 { + gpj0: gpj0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + hs_i2c6_bus: hs-i2c6-bus { + samsung,pins = "gpj0-1", "gpj0-0"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + }; + + /* Touch block @14CE0000 */ + pinctrl at 14CE0000 { + gpj1: gpj1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + hs_i2c7_bus: hs-i2c7-bus { + samsung,pins = "gpj1-1", "gpj1-0"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + }; + + /* FF block @14C90000 */ + pinctrl at 14C90000 { + gpg4: gpg4 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + spi3_bus: spi3-bus { + samsung,pins = "gpg4-0", "gpg4-1", "gpg4-2", "gpg4-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + }; + + /* ESE block @14CA0000 */ + pinctrl at 14CA0000 { + gpv7: gpv7 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + spi4_bus: spi4-bus { + samsung,pins = "gpv7-0", "gpv7-1", "gpv7-2", "gpv7-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <0>; + }; + }; + + /* FSYS0 block @10E60000 */ + pinctrl at 10E60000 { + gpr4: gpr4 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + sd2_clk: sd2-clk { + samsung,pins = "gpr4-0"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <3>; + }; + + sd2_cmd: sd2-cmd { + samsung,pins = "gpr4-1"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <3>; + }; + + sd2_cd: sd2-cd { + samsung,pins = "gpr4-2"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <3>; + }; + + sd2_bus1: sd2-bus-width1 { + samsung,pins = "gpr4-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <3>; + }; + + sd2_bus4: sd2-bus-width4 { + samsung,pins = "gpr4-4", "gpr4-5", "gpr4-6"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <3>; + }; + }; + + /* FSYS1 block @15690000 */ + pinctrl at 15690000 { + gpr0: gpr0 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpr1: gpr1 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpr2: gpr2 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + gpr3: gpr3 { + gpio-controller; + #gpio-cells = <2>; + + interrupt-controller; + #interrupt-cells = <2>; + }; + + sd0_clk: sd0-clk { + samsung,pins = "gpr0-0"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <3>; + }; + + sd0_cmd: sd0-cmd { + samsung,pins = "gpr0-1"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <3>; + }; + + sd0_rdqs: sd0-rdqs { + samsung,pins = "gpr0-2"; + samsung,pin-function = <2>; + samsung,pin-pud = <1>; + samsung,pin-drv = <3>; + }; + + sd0_qrdy: sd0-qrdy { + samsung,pins = "gpr0-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <1>; + samsung,pin-drv = <3>; + }; + + sd0_bus1: sd0-bus-width1 { + samsung,pins = "gpr1-0"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <3>; + }; + + sd0_bus4: sd0-bus-width4 { + samsung,pins = "gpr1-1", "gpr1-2", "gpr1-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <3>; + }; + + sd0_bus8: sd0-bus-width8 { + samsung,pins = "gpr1-4", "gpr1-5", "gpr1-6", "gpr1-7"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <3>; + }; + + sd1_clk: sd1-clk { + samsung,pins = "gpr2-0"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <2>; + }; + + sd1_cmd: sd1-cmd { + samsung,pins = "gpr2-1"; + samsung,pin-function = <2>; + samsung,pin-pud = <0>; + samsung,pin-drv = <2>; + }; + + sd1_ds: sd1-ds { + samsung,pins = "gpr2-2"; + samsung,pin-function = <2>; + samsung,pin-pud = <1>; + samsung,pin-drv = <6>; + }; + + sd1_qrdy: sd1-qrdy { + samsung,pins = "gpr2-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <1>; + samsung,pin-drv = <6>; + }; + + sd1_int: sd1-int { + samsung,pins = "gpr2-4"; + samsung,pin-function = <2>; + samsung,pin-pud = <1>; + samsung,pin-drv = <6>; + }; + + sd1_bus1: sd1-bus-width1 { + samsung,pins = "gpr3-0"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <2>; + }; + + sd1_bus4: sd1-bus-width4 { + samsung,pins = "gpr3-1", "gpr3-2", "gpr3-3"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <2>; + }; + + sd1_bus8: sd1-bus-width8 { + samsung,pins = "gpr3-4", "gpr3-5", "gpr3-6", "gpr3-7"; + samsung,pin-function = <2>; + samsung,pin-pud = <3>; + samsung,pin-drv = <2>; + }; + }; +};
Signature · 1 line
1.7.9.5
PATCH 10/14 arm64: dts: add pinctrl support to EXYNOS7
Hi Naveen,
Please see my comments inline.
Quoted 1 line · Naveen Krishna Chatradhi
Add the required pin configuration support to EXYNOS7
[snip]
arch/arm64/boot/dts/exynos7-pinctrl.dtsi line 17
Quoted 3 lines
arch/arm64/boot/dts/exynos7-pinctrl.dtsi new file +3
-0+/ { + /* ALIVE block @10580000 */ + pinctrl at 10580000 {
This syntax repeating the whole hierarchy of nodes in every dts file is considered obsolete in case of Exynos DT sources. Please switch to reference-based one. Please see related files for Exynos5260 and 5800 as examples.
[snip]
arch/arm64/boot/dts/exynos7-pinctrl.dtsi line 302
Quoted 8 lines
arch/arm64/boot/dts/exynos7-pinctrl.dtsi new file +8
-0+ disp_teson: disp_teson { + samsung,pins = "gpg0-1"; + samsung,pin-function = <2>; + }; + + disp_tesoff: disp_tesoff { + samsung,pins = "gpg0-1"; + samsung,pin-function = <0>;
This is not a special function, so I don't think it belongs to this file.
Best regards,
Tomasz
PATCH 10/14 arm64: dts: add pinctrl support to EXYNOS7
Hello Tomasz,
Quoted 9 lines · Tomasz Figa
Hi Naveen,
Please see my comments inline.
Quoted 1 line · Naveen Krishna Chatradhi
Add the required pin configuration support to EXYNOS7
[snip]
Quoted 8 lines
arch/arm64/boot/dts/exynos7-pinctrl.dtsi line 17
Quoted 3 lines
arch/arm64/boot/dts/exynos7-pinctrl.dtsi new file +3
-0+/ { + /* ALIVE block @10580000 */ + pinctrl at 10580000 {This syntax repeating the whole hierarchy of nodes in every dts file is considered obsolete in case of Exynos DT sources. Please switch to reference-based one. Please see related files for Exynos5260 and 5800 as examples.
Sure, will respin with reference based usage.
Quoted 12 lines
[snip]
arch/arm64/boot/dts/exynos7-pinctrl.dtsi line 302
Quoted 8 lines
arch/arm64/boot/dts/exynos7-pinctrl.dtsi new file +8
-0+ disp_teson: disp_teson { + samsung,pins = "gpg0-1"; + samsung,pin-function = <2>; + }; + + disp_tesoff: disp_tesoff { + samsung,pins = "gpg0-1"; + samsung,pin-function = <0>;
Quoted 4 lines
This is not a special function, so I don't think it belongs to this file.
Best regards,
Tomasz
Thanks for the in depth review.
Will fixes the comments.
Signature · 2 lines
Thanks & Regards, (: Nav :)
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
In reply to Naveen Krishna ChatradhiAdd initial device tree nodes for EXYNOS7 SoC. Also, includes the dt-binding definitions for clock ids.
--- arch/arm64/boot/dts/exynos7.dtsi | 553 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 553 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7.dtsi
arch/arm64/boot/dts/exynos7.dtsi new file +553 -0
@@ -0,0 +1,553 @@ +/* + * SAMSUNG EXYNOS7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * SAMSUNG EXYNOS7 SoC device nodes are listed in this file. + * EXYNOS7 based board files can include this file and provide + * values for board specfic bindings. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <dt-bindings/clock/exynos7-clk.h> + +/ { + compatible = "samsung,exynos7"; + interrupt-parent = <&gic>; + #address-cells = <1>; + #size-cells = <1>; + + aliases { + pinctrl0 = &pinctrl_0; + pinctrl1 = &pinctrl_1; + pinctrl2 = &pinctrl_2; + pinctrl3 = &pinctrl_3; + pinctrl4 = &pinctrl_4; + pinctrl5 = &pinctrl_5; + pinctrl6 = &pinctrl_6; + pinctrl7 = &pinctrl_7; + pinctrl8 = &pinctrl_8; + pinctrl9 = &pinctrl_9; + mshc0 = &mmc_0; + mshc2 = &mmc_2; + }; + + chipid at 10000000 { + compatible = "samsung,exynos4210-chipid"; + reg = <0x10000000 0x100>; + }; + + cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 0 { + device_type = "cpu"; + compatible = "arm,cortex-a57", "arm,armv8"; + reg = <0x0 0x0>; + }; + }; + + fin_pll: xxti { + compatible = "fixed-clock"; + clock-frequency = <24000000>; + clock-output-names = "fin_pll"; + #clock-cells = <0>; + }; + + gic: interrupt-controller at 11001000 { + compatible = "arm,gic-400"; + #interrupt-cells = <3>; + #address-cells = <0>; + interrupt-controller; + reg = <0x11001000 0x1000>, + <0x11002000 0x1000>, + <0x11004000 0x2000>, + <0x11006000 0x2000>; + }; + + hsi2c_0: hsi2c at 13640000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x13640000 0x1000>; + interrupts = <0 441 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c0_bus>; + clocks = <&clock_peric0 PCLK_HSI2C0>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_1: hsi2c at 13650000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x13650000 0x1000>; + interrupts = <0 442 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c1_bus>; + clocks = <&clock_peric0 PCLK_HSI2C1>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_2: hsi2c at 14E60000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x14E60000 0x1000>; + interrupts = <0 459 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c2_bus>; + clocks = <&clock_peric1 PCLK_HSI2C2>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_3: hsi2c at 14E70000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x14E70000 0x1000>; + interrupts = <0 460 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c3_bus>; + clocks = <&clock_peric1 PCLK_HSI2C3>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_4: hsi2c at 13660000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x13660000 0x1000>; + interrupts = <0 443 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c4_bus>; + clocks = <&clock_peric0 PCLK_HSI2C4>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_5: hsi2c at 13670000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x13670000 0x1000>; + interrupts = <0 444 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c5_bus>; + clocks = <&clock_peric0 PCLK_HSI2C5>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_6: hsi2c at 14E00000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x14E00000 0x1000>; + interrupts = <0 461 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c6_bus>; + clocks = <&clock_peric1 PCLK_HSI2C6>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_7: hsi2c at 13E10000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x13E10000 0x1000>; + interrupts = <0 462 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c7_bus>; + clocks = <&clock_peric1 PCLK_HSI2C7>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_8: hsi2c at 14E20000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x14E20000 0x1000>; + interrupts = <0 463 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c8_bus>; + clocks = <&clock_peric1 PCLK_HSI2C8>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_9: hsi2c at 13680000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x13680000 0x1000>; + interrupts = <0 445 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c9_bus>; + clocks = <&clock_peric0 PCLK_HSI2C9>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_10: hsi2c at 13690000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x13690000 0x1000>; + interrupts = <0 446 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c10_bus>; + clocks = <&clock_peric0 PCLK_HSI2C10>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_11: hsi2c at 136A0000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x136A0000 0x1000>; + interrupts = <0 447 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c11_bus>; + clocks = <&clock_peric0 PCLK_HSI2C11>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + mct at 101C0000 { + compatible = "samsung,exynos4210-mct"; + reg = <0x101C0000 0x800>; + interrupt-controller; + #interrupt-cells = <1>; + interrupt-parent = <&mct_map>; + interrupts = <0>, <1>, <2>, <3>, + <4>, <5>, <6>, <7>, + <8>, <9>, <10>, <11>; + clocks = <&fin_pll>, <&clock_peris PCLK_MCT>; + clock-names = "fin_pll", "mct"; + + mct_map: mct-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gic 0 112 0>, + <1 &gic 0 113 0>, + <2 &gic 0 114 0>, + <3 &gic 0 115 0>, + <4 &gic 0 116 0>, + <5 &gic 0 117 0>, + <6 &gic 0 118 0>, + <7 &gic 0 119 0>, + <8 &gic 0 120 0>, + <9 &gic 0 121 0>, + <10 &gic 0 122 0>, + <11 &gic 0 123 0>; + }; + }; + + mmc_0: mmc at 15740000 { + compatible = "samsung,exynos7-dw-mshc-smu"; + interrupts = <0 201 0>; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x15740000 0x2000>; + clocks = <&clock_fsys1 ACLK_MMC0>, + <&clock_top1 CLK_SCLK_MMC0>; + clock-names = "biu", "ciu"; + fifo-depth = <0x40>; + status = "disabled"; + }; + + mmc_2: mmc at 15560000 { + compatible = "samsung,exynos7-dw-mshc-smu"; + interrupts = <0 216 0>; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x15560000 0x2000>; + clocks = <&clock_fsys0 ACLK_MMC2>, + <&clock_top1 CLK_SCLK_MMC2>; + clock-names = "biu", "ciu"; + fifo-depth = <0x40>; + status = "disabled"; + }; + + pinctrl_0: pinctrl at 10580000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x10580000 0x1000>; + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>; + wakeup-interrupt-controller { + compatible = "samsung,exynos4210-wakeup-eint"; + interrupt-parent = <&gic>; + interrupts = <0 16 0>; + }; + }; + + pinctrl_1: pinctrl at 114B0000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x114B0000 0x1000>; + interrupts = <0 92 0>; + }; + + pinctrl_2: pinctrl at 13470000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x13470000 0x1000>; + interrupts = <0 383 0>; + }; + + pinctrl_3: pinctrl at 14870000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x14870000 0x1000>; + interrupts = <0 384 0>; + }; + + pinctrl_4: pinctrl at 14CD0000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x14CD0000 0x1000>; + interrupts = <0 473 0>; + }; + + pinctrl_5: pinctrl at 14CE0000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x14CE0000 0x1000>; + interrupts = <0 474 0>; + }; + + pinctrl_6: pinctrl at 14C90000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x14C90000 0x1000>; + interrupts = <0 475 0>; + }; + + pinctrl_7: pinctrl at 14CA0000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x14CA0000 0x1000>; + interrupts = <0 476 0>; + }; + + pinctrl_8: pinctrl at 10E60000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x10E60000 0x1000>; + interrupts = <0 221 0>; + }; + + pinctrl_9: pinctrl at 15690000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x15690000 0x1000>; + interrupts = <0 203 0>; + }; + + pwm: pwm at 136C0000 { + compatible = "samsung,exynos4210-pwm"; + reg = <0x136C0000 0x100>; + samsung,pwm-outputs = <0>, <1>, <2>, <3>; + #pwm-cells = <3>; + clocks = <&clock_peric0 PCLK_PWM>; + clock-names = "timers"; + status = "disabled"; + }; + + rtc at 10590000 { + compatible = "samsung,s3c6410-rtc"; + reg = <0x10590000 0x100>; + interrupts = <0 355 0>, <0 356 0>; + clocks = <&clock_ccore PCLK_RTC>; + clock-names = "rtc"; + }; + + serial at 13630000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x13630000 0x100>; + interrupts = <0 440 0>; + clocks = <&clock_peric0 PCLK_UART0>, <&clock_peric0 SCLK_UART0>; + clock-names = "uart", "clk_uart_baud0"; + status = "okay"; + }; + + serial at 14C20000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x14C20000 0x100>; + interrupts = <0 456 0>; + clocks = <&clock_peric1 PCLK_UART1>, <&clock_peric1 SCLK_UART1>; + clock-names = "uart", "clk_uart_baud1"; + status = "okay"; + }; + + serial at 14C30000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x14C30000 0x100>; + interrupts = <0 457 0>; + clocks = <&clock_peric1 PCLK_UART2>, <&clock_peric1 SCLK_UART2>; + clock-names = "uart", "clk_uart_baud2"; + status = "okay"; + }; + + serial at 14C40000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x14C40000 0x100>; + interrupts = <0 458 0>; + clocks = <&clock_peric1 PCLK_UART3>, <&clock_peric1 SCLK_UART3>; + clock-names = "uart", "clk_uart_baud3"; + status = "okay"; + }; + + /* The Clock nodes are ordered as per the usermanual. */ + clock_topc: clock-controller at 10570000 { + compatible = "samsung,exynos7-clock-topc"; + reg = <0x10570000 0x10000>; + #clock-cells = <1>; + }; + + clock_top0: clock-controller at 105D0000 { + compatible = "samsung,exynos7-clock-top0"; + reg = <0x105D0000 0xB000>; + #clock-cells = <1>; + }; + + clock_top1: clock-controller at 105E0000 { + compatible = "samsung,exynos7-clock-top1"; + reg = <0x105E0000 0xB000>; + #clock-cells = <1>; + }; + + clock_atlas: clock-controller at 11800000 { + compatible = "samsung,exynos7-clock-atlas"; + reg = <0x11800000 0x1100>; + #clock-cells = <1>; + }; + + clock_g3d: clock-controller at 14AA0000 { + compatible = "samsung,exynos7-clock-g3d"; + reg = <0x14AA0000 0x1100>; + #clock-cells = <1>; + }; + + clock_mif0: clock-controller at 10850000 { + compatible = "samsung,exynos7-clock-mif0"; + reg = <0x10850000 0x1100>; + #clock-cells = <1>; + }; + + clock_mif1: clock-controller at 10950000 { + compatible = "samsung,exynos7-clock-mif1"; + reg = <0x10950000 0x1100>; + #clock-cells = <1>; + }; + + clock_mif2: clock-controller at 10A50000 { + compatible = "samsung,exynos7-clock-mif2"; + reg = <0x10A50000 0x1100>; + #clock-cells = <1>; + }; + + clock_mif3: clock-controller at 10B50000 { + compatible = "samsung,exynos7-clock-mif3"; + reg = <0x10B50000 0x1100>; + #clock-cells = <1>; + }; + + clock_ccore: clock-controller at 105B0000 { + compatible = "samsung,exynos7-clock-ccore"; + reg = <0x105B0000 0xD00>; + #clock-cells = <1>; + }; + + clock_imem: clock-controller at 11060000 { + compatible = "samsung,exynos7-clock-imem"; + reg = <0x11060000 0xD00>; + #clock-cells = <1>; + }; + + clock_peric0: clock-controller at 13610000 { + compatible = "samsung,exynos7-clock-peric0"; + reg = <0x13610000 0xD00>; + #clock-cells = <1>; + }; + + clock_peric1: clock-controller at 14C80000 { + compatible = "samsung,exynos7-clock-peric1"; + reg = <0x14C80000 0xD00>; + #clock-cells = <1>; + }; + + clock_peris: clock-controller at 10040000 { + compatible = "samsung,exynos7-clock-peris"; + reg = <0x10040000 0xD00>; + #clock-cells = <1>; + }; + + clock_bus0: clock-controller at 13400000 { + compatible = "samsung,exynos7-clock-bus0"; + reg = <0x13400000 0xD00>; + #clock-cells = <1>; + }; + + clock_bus1: clock-controller at 14800000 { + compatible = "samsung,exynos7-clock-bus1"; + reg = <0x14800000 0xD00>; + #clock-cells = <1>; + }; + + clock_disp: clock-controller at 13AD0000 { + compatible = "samsung,exynos7-clock-disp"; + reg = <0x13AD0000 0xD00>; + #clock-cells = <1>; + }; + + clock_aud: clock-controller at 114C0000 { + compatible = "samsung,exynos7-clock-aud"; + reg = <0x114C0000 0xD00>; + #clock-cells = <1>; + }; + + clock_fsys0: clock-controller at 10E90000 { + compatible = "samsung,exynos7-clock-fsys0"; + reg = <0x10E90000 0xD00>; + #clock-cells = <1>; + }; + + clock_fsys1: clock-controller at 156E0000 { + compatible = "samsung,exynos7-clock-fsys1"; + reg = <0x156E0000 0xD00>; + #clock-cells = <1>; + }; + + clock_mscl: clock-controller at 150D0000 { + compatible = "samsung,exynos7-clock-mscl"; + reg = <0x150D0000 0xD00>; + #clock-cells = <1>; + }; + + clock_mfc: clock-controller at 15280000 { + compatible = "samsung,exynos7-clock-mfc"; + reg = <0x15280000 0xD00>; + #clock-cells = <1>; + }; + + timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, + <1 14 0xff01>, + <1 11 0xff01>, + <1 10 0xff01>; + clock-frequency = <24000000>; + use-clocksource-only; + use-physical-timer; + }; +}; + +#include "exynos7-pinctrl.dtsi"
Signature · 1 line
1.7.9.5
Mark Rutland
reviewerPATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Hi Naveen,
Quoted 2 lines · Naveen Krishna Chatradhi
Add initial device tree nodes for EXYNOS7 SoC. Also, includes the dt-binding definitions for clock ids.
Fallout from a rebase? That latter part doesn't seem to be relevant.
Quoted 27 lines
Signed-off-by Naveen Krishna Chatradhi <[email protected]>Cc Thomas Abraham <[email protected]>Cc Rob Herring <[email protected]>Cc Catalin Marinas <[email protected]>--- arch/arm64/boot/dts/exynos7.dtsi | 553 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 553 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7.dtsiarch/arm64/boot/dts/exynos7.dtsi new file +12
-0@@ -0,0 +1,553 @@ +/* + * SAMSUNG EXYNOS7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * SAMSUNG EXYNOS7 SoC device nodes are listed in this file. + * EXYNOS7 based board files can include this file and provide + * values for board specfic bindings. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as
arch/arm64/boot/dts/exynos7.dtsi line 22
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ * published by the Free Software Foundation. + */ + +#include <dt-bindings/clock/exynos7-clk.h> + +/ { + compatible = "samsung,exynos7"; + interrupt-parent = <&gic>; + #address-cells = <1>; + #size-cells = <1>;
Can we guarantee everything going to live within 0x0 - 0xffffffff for all boards using the SoC?
I suspect that we can't, so the addresses and sizes at the top level should be two cells. At some point there is bound to be something above 4GB that we'll need to map, so to save us from a painful dts refactoring we should have the dts organised to support that from the outside.
[...]
arch/arm64/boot/dts/exynos7.dtsi line 53
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 0 { + device_type = "cpu"; + compatible = "arm,cortex-a57", "arm,armv8"; + reg = <0x0 0x0>; + }; + };
Only UP?
[...]
arch/arm64/boot/dts/exynos7.dtsi line 71
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ gic: interrupt-controller at 11001000 { + compatible = "arm,gic-400"; + #interrupt-cells = <3>; + #address-cells = <0>; + interrupt-controller; + reg = <0x11001000 0x1000>, + <0x11002000 0x1000>, + <0x11004000 0x2000>, + <0x11006000 0x2000>; + };
Nice to see GICV and GICH.
[...]
Quoted 20 lines
arch/arm64/boot/dts/exynos7.dtsi new file +20
-0+ mct at 101C0000 { + compatible = "samsung,exynos4210-mct"; + reg = <0x101C0000 0x800>; + interrupt-controller; + #interrupt-cells = <1>; + interrupt-parent = <&mct_map>; + interrupts = <0>, <1>, <2>, <3>, + <4>, <5>, <6>, <7>, + <8>, <9>, <10>, <11>; + clocks = <&fin_pll>, <&clock_peris PCLK_MCT>; + clock-names = "fin_pll", "mct"; + + mct_map: mct-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gic 0 112 0>, + <1 &gic 0 113 0>, + <2 &gic 0 114 0>, + <3 &gic 0 115 0>,
arch/arm64/boot/dts/exynos7.dtsi line 258
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ <4 &gic 0 116 0>, + <5 &gic 0 117 0>, + <6 &gic 0 118 0>, + <7 &gic 0 119 0>, + <8 &gic 0 120 0>, + <9 &gic 0 121 0>, + <10 &gic 0 122 0>, + <11 &gic 0 123 0>; + }; + };
I don't see why need the map here. Why can't we describe the GIC interrupts directly?
[...]
arch/arm64/boot/dts/exynos7.dtsi line 547
Quoted 7 lines
arch/arm64/boot/dts/exynos7.dtsi new file +7
-0+ timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, + <1 14 0xff01>, + <1 11 0xff01>, + <1 10 0xff01>; + clock-frequency = <24000000>;
Your firmware/bootloader should configure CNTFRQ, and this shouldn't be necessary. The clock-frequency property is an incomplete workaround for broken firmware that in an ideal world we could kill off.
arch/arm64/boot/dts/exynos7.dtsi line 549
Quoted 2 lines
arch/arm64/boot/dts/exynos7.dtsi new file +2
-0+ use-clocksource-only; + use-physical-timer;
Neither of these properties were introduced by this series, and no rationale was given.
What are these properties for, and why do you believe they are necessary?
Thanks,
Mark.
Catalin Marinas
maintainerPATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 12 lines · Mark Rutland
arch/arm64/boot/dts/exynos7.dtsi line 53
Quoted 10 lines · Naveen Krishna Chatradhi
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 0 { + device_type = "cpu"; + compatible = "arm,cortex-a57", "arm,armv8"; + reg = <0x0 0x0>; + }; + };
Quoted 1 line
Only UP?
I see what you meant. Maybe this is populated by firmware and that's only an example.
Signature · 1 line
Catalin
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
In reply to Mark RutlandHi Mark,
Quoted 7 lines · Mark Rutland
Hi Naveen,
Quoted 2 lines · Naveen Krishna Chatradhi
Add initial device tree nodes for EXYNOS7 SoC. Also, includes the dt-binding definitions for clock ids.
Fallout from a rebase? That latter part doesn't seem to be relevant.
Yes, this is fixed now.
Quoted 42 lines
Quoted 28 lines
Signed-off-by Naveen Krishna Chatradhi <[email protected]>Cc Thomas Abraham <[email protected]>Cc Rob Herring <[email protected]>Cc Catalin Marinas <[email protected]>--- arch/arm64/boot/dts/exynos7.dtsi | 553 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 553 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7.dtsiarch/arm64/boot/dts/exynos7.dtsi new file +12
-0@@ -0,0 +1,553 @@ +/* + * SAMSUNG EXYNOS7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * SAMSUNG EXYNOS7 SoC device nodes are listed in this file. + * EXYNOS7 based board files can include this file and provide + * values for board specfic bindings. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 asarch/arm64/boot/dts/exynos7.dtsi line 22
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ * published by the Free Software Foundation. + */ + +#include <dt-bindings/clock/exynos7-clk.h> + +/ { + compatible = "samsung,exynos7"; + interrupt-parent = <&gic>; + #address-cells = <1>; + #size-cells = <1>;Can we guarantee everything going to live within
0x0-0xfffffffffor all boards using the SoC?
Quoted 4 lines
I suspect that we can't, so the addresses and sizes at the top level should be two cells. At some point there is bound to be something above 4GB that we'll need to map, so to save us from a painful dts refactoring we should have the dts organised to support that from the outside.
Ok, this is fixed with #address-cells = 2 and #size-cells = 2.
Quoted 14 lines
[...]
arch/arm64/boot/dts/exynos7.dtsi line 53
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 0 { + device_type = "cpu"; + compatible = "arm,cortex-a57", "arm,armv8"; + reg = <0x0 0x0>; + }; + };
Quoted 1 line
Only UP?
This is a quad core SoC and the rest of the CPU nodes have been added in the second version of this series.
Quoted 49 lines
[...]
arch/arm64/boot/dts/exynos7.dtsi line 71
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ gic: interrupt-controller at 11001000 { + compatible = "arm,gic-400"; + #interrupt-cells = <3>; + #address-cells = <0>; + interrupt-controller; + reg = <0x11001000 0x1000>, + <0x11002000 0x1000>, + <0x11004000 0x2000>, + <0x11006000 0x2000>; + };Nice to see GICV and GICH.
[...]
Quoted 20 lines
arch/arm64/boot/dts/exynos7.dtsi new file +20
-0+ mct at 101C0000 { + compatible = "samsung,exynos4210-mct"; + reg = <0x101C0000 0x800>; + interrupt-controller; + #interrupt-cells = <1>; + interrupt-parent = <&mct_map>; + interrupts = <0>, <1>, <2>, <3>, + <4>, <5>, <6>, <7>, + <8>, <9>, <10>, <11>; + clocks = <&fin_pll>, <&clock_peris PCLK_MCT>; + clock-names = "fin_pll", "mct"; + + mct_map: mct-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gic 0 112 0>, + <1 &gic 0 113 0>, + <2 &gic 0 114 0>, + <3 &gic 0 115 0>,arch/arm64/boot/dts/exynos7.dtsi line 258
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ <4 &gic 0 116 0>, + <5 &gic 0 117 0>, + <6 &gic 0 118 0>, + <7 &gic 0 119 0>, + <8 &gic 0 120 0>, + <9 &gic 0 121 0>, + <10 &gic 0 122 0>, + <11 &gic 0 123 0>; + }; + };
Quoted 2 lines
I don't see why need the map here. Why can't we describe the GIC interrupts directly?
Right, it was not required. Also, we will try and use only arch timer and not MCT.
Quoted 15 lines
[...]
arch/arm64/boot/dts/exynos7.dtsi line 547
Quoted 7 lines
arch/arm64/boot/dts/exynos7.dtsi new file +7
-0+ timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, + <1 14 0xff01>, + <1 11 0xff01>, + <1 10 0xff01>; + clock-frequency = <24000000>;Your
firmware/bootloadershould configure CNTFRQ, and this shouldn't be necessary. The clock-frequency property is an incomplete workaround for broken firmware that in an ideal world we could kill off.
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi line 549
Quoted 2 lines
arch/arm64/boot/dts/exynos7.dtsi new file +2
-0+ use-clocksource-only; + use-physical-timer;Neither of these properties were introduced by this series, and no rationale was given.
What are these properties for, and why do you believe they are necessary?
Sorry, that was a mistake. This has been fixed.
Quoted 3 lines
Thanks,
Mark.
Thanks.
Signature · 2 lines
Shine bright, (: Nav :)
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
In reply to Naveen Krishna ChatradhiHi Naveen,
Please see my comments inline.
Quoted 44 lines · Naveen Krishna Chatradhi
Add initial device tree nodes for EXYNOS7 SoC. Also, includes the dt-binding definitions for clock ids.
Signed-off-by Naveen Krishna Chatradhi <[email protected]>Cc Thomas Abraham <[email protected]>Cc Rob Herring <[email protected]>Cc Catalin Marinas <[email protected]>--- arch/arm64/boot/dts/exynos7.dtsi | 553 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 553 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7.dtsiarch/arm64/boot/dts/exynos7.dtsi new file +26
-0@@ -0,0 +1,553 @@ +/* + * SAMSUNG EXYNOS7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * SAMSUNG EXYNOS7 SoC device nodes are listed in this file. + * EXYNOS7 based board files can include this file and provide + * values for board specfic bindings. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <dt-bindings/clock/exynos7-clk.h> + +/ { + compatible = "samsung,exynos7"; + interrupt-parent = <&gic>; + #address-cells = <1>; + #size-cells = <1>; + + aliases { + pinctrl0 = &pinctrl_0; + pinctrl1 = &pinctrl_1;
arch/arm64/boot/dts/exynos7.dtsi line 36
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ pinctrl2 = &pinctrl_2; + pinctrl3 = &pinctrl_3; + pinctrl4 = &pinctrl_4; + pinctrl5 = &pinctrl_5; + pinctrl6 = &pinctrl_6; + pinctrl7 = &pinctrl_7; + pinctrl8 = &pinctrl_8; + pinctrl9 = &pinctrl_9; + mshc0 = &mmc_0; + mshc2 = &mmc_2;
Please add aliases for serial controllers. Refer to relevant DT binding documentation for more information.
arch/arm64/boot/dts/exynos7.dtsi line 42
Quoted 6 lines
arch/arm64/boot/dts/exynos7.dtsi new file +6
-0+ }; + + chipid at 10000000 { + compatible = "samsung,exynos4210-chipid"; + reg = <0x10000000 0x100>; + };
Please put SoC components (except cpus node) under a simple-bus node called "soc". Please see exynos5260.dtsi as an example.
arch/arm64/boot/dts/exynos7.dtsi line 52
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ + cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 0 { + device_type = "cpu"; + compatible = "arm,cortex-a57", "arm,armv8"; + reg = <0x0 0x0>; + };
Does this SoC really has only one CPU or this is a workaround for things like missing CPU bring-up code?
Quoted 1 line
arch/arm64/boot/dts/exynos7.dtsi new file +1
-0+ };
[snip]
arch/arm64/boot/dts/exynos7.dtsi line 233
Quoted 5 lines
arch/arm64/boot/dts/exynos7.dtsi new file +5
-0+ mct at 101C0000 { + compatible = "samsung,exynos4210-mct"; + reg = <0x101C0000 0x800>; + interrupt-controller; + #interrupt-cells = <1>;
MCT is not an interrupt controller.
Quoted 13 lines
arch/arm64/boot/dts/exynos7.dtsi new file +13
-0+ interrupt-parent = <&mct_map>; + interrupts = <0>, <1>, <2>, <3>, + <4>, <5>, <6>, <7>, + <8>, <9>, <10>, <11>; + clocks = <&fin_pll>, <&clock_peris PCLK_MCT>; + clock-names = "fin_pll", "mct"; + + mct_map: mct-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gic 0 112 0>, + <1 &gic 0 113 0>,
arch/arm64/boot/dts/exynos7.dtsi line 256
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ <2 &gic 0 114 0>, + <3 &gic 0 115 0>, + <4 &gic 0 116 0>, + <5 &gic 0 117 0>, + <6 &gic 0 118 0>, + <7 &gic 0 119 0>, + <8 &gic 0 120 0>, + <9 &gic 0 121 0>, + <10 &gic 0 122 0>, + <11 &gic 0 123 0>;
All inputs of this interrupt map come from the same interrupt controller. What's the point of this map then?
Quoted 26 lines
arch/arm64/boot/dts/exynos7.dtsi new file +26
-0+ }; + }; + + mmc_0: mmc at 15740000 { + compatible = "samsung,exynos7-dw-mshc-smu"; + interrupts = <0 201 0>; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x15740000 0x2000>; + clocks = <&clock_fsys1 ACLK_MMC0>, + <&clock_top1 CLK_SCLK_MMC0>; + clock-names = "biu", "ciu"; + fifo-depth = <0x40>; + status = "disabled"; + }; + + mmc_2: mmc at 15560000 { + compatible = "samsung,exynos7-dw-mshc-smu"; + interrupts = <0 216 0>; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x15560000 0x2000>; + clocks = <&clock_fsys0 ACLK_MMC2>, + <&clock_top1 CLK_SCLK_MMC2>; + clock-names = "biu", "ciu"; + fifo-depth = <0x40>;
arch/arm64/boot/dts/exynos7.dtsi line 292
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ status = "disabled"; + }; + + pinctrl_0: pinctrl at 10580000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x10580000 0x1000>; + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>, + <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>;
According to patch 5/14, this bank supports only wake-up interrupts. Their interrupt specifiers should be specified either in the wake-up interrupt controller node (for muxed wake-up interrupts) or in nodes of respective banks (for direct wake-up interrupts).
Quoted 6 lines
arch/arm64/boot/dts/exynos7.dtsi new file +6
-0+ wakeup-interrupt-controller { + compatible = "samsung,exynos4210-wakeup-eint"; + interrupt-parent = <&gic>; + interrupts = <0 16 0>; + }; + };
[snip]
arch/arm64/boot/dts/exynos7.dtsi line 378
Quoted 7 lines
arch/arm64/boot/dts/exynos7.dtsi new file +7
-0+ serial at 13630000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x13630000 0x100>; + interrupts = <0 440 0>; + clocks = <&clock_peric0 PCLK_UART0>, <&clock_peric0 SCLK_UART0>; + clock-names = "uart", "clk_uart_baud0"; + status = "okay";
No need to explicitly specify the status as "okay" in top level dtsi file.
arch/arm64/boot/dts/exynos7.dtsi line 386
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi new file +8
-0+ }; + + serial at 14C20000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x14C20000 0x100>; + interrupts = <0 456 0>; + clocks = <&clock_peric1 PCLK_UART1>, <&clock_peric1 SCLK_UART1>; + clock-names = "uart", "clk_uart_baud1";
The "clk_uart_baud1" clock doesn't seem right. The N in "clk_uart_baudN" stands for the input of internal clock source mux, not index of the IP block in the SoC. Please make sure this is defined correctly.
arch/arm64/boot/dts/exynos7.dtsi
Quoted 1 line
arch/arm64/boot/dts/exynos7.dtsi new file +1
-0+ status = "okay";
Ditto.
arch/arm64/boot/dts/exynos7.dtsi line 395
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi new file +8
-0+ }; + + serial at 14C30000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x14C30000 0x100>; + interrupts = <0 457 0>; + clocks = <&clock_peric1 PCLK_UART2>, <&clock_peric1 SCLK_UART2>; + clock-names = "uart", "clk_uart_baud2";
Ditto.
arch/arm64/boot/dts/exynos7.dtsi
Quoted 1 line
arch/arm64/boot/dts/exynos7.dtsi new file +1
-0+ status = "okay";
Ditto.
arch/arm64/boot/dts/exynos7.dtsi line 404
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi new file +8
-0+ }; + + serial at 14C40000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x14C40000 0x100>; + interrupts = <0 458 0>; + clocks = <&clock_peric1 PCLK_UART3>, <&clock_peric1 SCLK_UART3>; + clock-names = "uart", "clk_uart_baud3";
Ditto.
arch/arm64/boot/dts/exynos7.dtsi
Quoted 1 line
arch/arm64/boot/dts/exynos7.dtsi new file +1
-0+ status = "okay";
Ditto.
Best regards,
Tomasz
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Hi Tomasz,
Quoted 54 lines · Tomasz Figa
Hi Naveen,
Please see my comments inline.
Quoted 49 lines · Naveen Krishna Chatradhi
Add initial device tree nodes for EXYNOS7 SoC. Also, includes the dt-binding definitions for clock ids.
Signed-off-by Naveen Krishna Chatradhi <[email protected]>Cc Thomas Abraham <[email protected]>Cc Rob Herring <[email protected]>Cc Catalin Marinas <[email protected]>--- arch/arm64/boot/dts/exynos7.dtsi | 553 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 553 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7.dtsiarch/arm64/boot/dts/exynos7.dtsi new file +31
-0@@ -0,0 +1,553 @@ +/* + * SAMSUNG EXYNOS7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * SAMSUNG EXYNOS7 SoC device nodes are listed in this file. + * EXYNOS7 based board files can include this file and provide + * values for board specfic bindings. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <dt-bindings/clock/exynos7-clk.h> + +/ { + compatible = "samsung,exynos7"; + interrupt-parent = <&gic>; + #address-cells = <1>; + #size-cells = <1>; + + aliases { + pinctrl0 = &pinctrl_0; + pinctrl1 = &pinctrl_1; + pinctrl2 = &pinctrl_2; + pinctrl3 = &pinctrl_3; + pinctrl4 = &pinctrl_4; + pinctrl5 = &pinctrl_5; + pinctrl6 = &pinctrl_6;
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi line 36
Quoted 5 lines
arch/arm64/boot/dts/exynos7.dtsi new file +5
-0+ pinctrl7 = &pinctrl_7; + pinctrl8 = &pinctrl_8; + pinctrl9 = &pinctrl_9; + mshc0 = &mmc_0; + mshc2 = &mmc_2;Please add aliases for serial controllers. Refer to relevant DT binding documentation for more information.
Ok.
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi line 42
Quoted 7 lines
arch/arm64/boot/dts/exynos7.dtsi new file +6
-0+ }; + + chipid at 10000000 { + compatible = "samsung,exynos4210-chipid"; + reg = <0x10000000 0x100>; + };Please put SoC components (except cpus node) under a simple-bus node called "soc". Please see
exynos5260.dtsias an example.
Ok.
Quoted 87 lines
arch/arm64/boot/dts/exynos7.dtsi line 52
Quoted 11 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ + cpus { + #address-cells = <2>; + #size-cells = <0>; + + cpu at 0 { + device_type = "cpu"; + compatible = "arm,cortex-a57", "arm,armv8"; + reg = <0x0 0x0>; + };Does this SoC really has only one CPU or this is a workaround for things like missing CPU bring-up code?
Quoted 1 line
arch/arm64/boot/dts/exynos7.dtsi new file +1
-0+ };[snip]
arch/arm64/boot/dts/exynos7.dtsi line 233
Quoted 5 lines
arch/arm64/boot/dts/exynos7.dtsi new file +5
-0+ mct at 101C0000 { + compatible = "samsung,exynos4210-mct"; + reg = <0x101C0000 0x800>; + interrupt-controller; + #interrupt-cells = <1>;MCT is not an interrupt controller.
Quoted 13 lines
arch/arm64/boot/dts/exynos7.dtsi new file +13
-0+ interrupt-parent = <&mct_map>; + interrupts = <0>, <1>, <2>, <3>, + <4>, <5>, <6>, <7>, + <8>, <9>, <10>, <11>; + clocks = <&fin_pll>, <&clock_peris PCLK_MCT>; + clock-names = "fin_pll", "mct"; + + mct_map: mct-map { + #interrupt-cells = <1>; + #address-cells = <0>; + #size-cells = <0>; + interrupt-map = <0 &gic 0 112 0>, + <1 &gic 0 113 0>,arch/arm64/boot/dts/exynos7.dtsi line 256
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ <2 &gic 0 114 0>, + <3 &gic 0 115 0>, + <4 &gic 0 116 0>, + <5 &gic 0 117 0>, + <6 &gic 0 118 0>, + <7 &gic 0 119 0>, + <8 &gic 0 120 0>, + <9 &gic 0 121 0>, + <10 &gic 0 122 0>, + <11 &gic 0 123 0>;All inputs of this interrupt map come from the same interrupt controller. What's the point of this map then?
Quoted 33 lines
arch/arm64/boot/dts/exynos7.dtsi new file +33
-0+ }; + }; + + mmc_0: mmc at 15740000 { + compatible = "samsung,exynos7-dw-mshc-smu"; + interrupts = <0 201 0>; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x15740000 0x2000>; + clocks = <&clock_fsys1 ACLK_MMC0>, + <&clock_top1 CLK_SCLK_MMC0>; + clock-names = "biu", "ciu"; + fifo-depth = <0x40>; + status = "disabled"; + }; + + mmc_2: mmc at 15560000 { + compatible = "samsung,exynos7-dw-mshc-smu"; + interrupts = <0 216 0>; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x15560000 0x2000>; + clocks = <&clock_fsys0 ACLK_MMC2>, + <&clock_top1 CLK_SCLK_MMC2>; + clock-names = "biu", "ciu"; + fifo-depth = <0x40>; + status = "disabled"; + }; + + pinctrl_0: pinctrl at 10580000 { + compatible = "samsung,exynos7-pinctrl"; + reg = <0x10580000 0x1000>; + interrupts = <0 0 0>, <0 1 0>, <0 2 0>, <0 3 0>,
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi line 292
Quoted 3 lines
arch/arm64/boot/dts/exynos7.dtsi new file +3
-0+ <0 4 0>, <0 5 0>, <0 6 0>, <0 7 0>, + <0 8 0>, <0 9 0>, <0 10 0>, <0 11 0>, + <0 12 0>, <0 13 0>, <0 14 0>, <0 15 0>;According to patch 5/14, this bank supports only wake-up interrupts. Their interrupt specifiers should be specified either in the wake-up interrupt controller node (for muxed wake-up interrupts) or in nodes of respective banks (for direct wake-up interrupts).
Ok, will fix.
Quoted 18 lines
Quoted 7 lines
arch/arm64/boot/dts/exynos7.dtsi new file +6
-0+ wakeup-interrupt-controller { + compatible = "samsung,exynos4210-wakeup-eint"; + interrupt-parent = <&gic>; + interrupts = <0 16 0>; + }; + };[snip]
arch/arm64/boot/dts/exynos7.dtsi line 378
Quoted 7 lines
arch/arm64/boot/dts/exynos7.dtsi new file +7
-0+ serial at 13630000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x13630000 0x100>; + interrupts = <0 440 0>; + clocks = <&clock_peric0 PCLK_UART0>, <&clock_peric0 SCLK_UART0>; + clock-names = "uart", "clk_uart_baud0"; + status = "okay";
Quoted 1 line
No need to explicitly specify the status as "okay" in top level dtsi file.
Ok. Fixed.
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi line 386
Quoted 9 lines
arch/arm64/boot/dts/exynos7.dtsi new file +8
-0+ }; + + serial at 14C20000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x14C20000 0x100>; + interrupts = <0 456 0>; + clocks = <&clock_peric1 PCLK_UART1>, <&clock_peric1 SCLK_UART1>; + clock-names = "uart", "clk_uart_baud1";
Quoted 3 lines
The "
clk_uart_baud1" clock doesn't seem right. The N in "clk_uart_baudN" stands for the input of internal clock source mux, not index of the IP block in the SoC. Please make sure this is defined correctly.
Right. That was a mistake. This is fixed.
Quoted 29 lines
Quoted 2 lines
+ status = "okay";
Ditto.
arch/arm64/boot/dts/exynos7.dtsi line 395
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi new file +8
-0+ }; + + serial at 14C30000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x14C30000 0x100>; + interrupts = <0 457 0>; + clocks = <&clock_peric1 PCLK_UART2>, <&clock_peric1 SCLK_UART2>; + clock-names = "uart", "clk_uart_baud2";Ditto.
Quoted 1 line
+ status = "okay";
Ditto.
arch/arm64/boot/dts/exynos7.dtsi line 404
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi new file +8
-0+ }; + + serial at 14C40000 { + compatible = "samsung,exynos4210-uart"; + reg = <0x14C40000 0x100>; + interrupts = <0 458 0>; + clocks = <&clock_peric1 PCLK_UART3>, <&clock_peric1 SCLK_UART3>; + clock-names = "uart", "clk_uart_baud3";
Quoted 8 lines
Ditto.
Quoted 1 line
+ status = "okay";
Ditto.
Best regards,
Tomasz
Thanks for your comments.
Signature · 2 lines
Shine bright, (: Nav :)
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
In reply to Naveen Krishna ChatradhiHi,
Quoted 2 lines · Naveen Krishna Chatradhi
Add initial device tree nodes for EXYNOS7 SoC. Also, includes the dt-binding definitions for clock ids.
Uh, no -- it just adds the dtsi.
arch/arm64/boot/dts/exynos7.dtsi
Quoted 14 lines
Signed-off-by Naveen Krishna Chatradhi <[email protected]>Cc Thomas Abraham <[email protected]>Cc Rob Herring <[email protected]>Cc Catalin Marinas <[email protected]>--- arch/arm64/boot/dts/exynos7.dtsi | 553 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 553 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7.dtsiarch/arm64/boot/dts/exynos7.dtsi new file +0
-0
Let's not make the same mistake as on 32-bit, and go with a directory hierarchy here from day one.
So, please create a exynos subdirectory for this file. You also need a Makefile when you add a board dts.
Quoted 13 lines
arch/arm64/boot/dts/exynos7.dtsi new file +12
-0@@ -0,0 +1,553 @@ +/* + * SAMSUNG EXYNOS7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * SAMSUNG EXYNOS7 SoC device nodes are listed in this file. + * EXYNOS7 based board files can include this file and provide + * values for board specfic bindings. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as
arch/arm64/boot/dts/exynos7.dtsi line 22
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ * published by the Free Software Foundation. + */ + +#include <dt-bindings/clock/exynos7-clk.h> + +/ { + compatible = "samsung,exynos7"; + interrupt-parent = <&gic>; + #address-cells = <1>; + #size-cells = <1>;
You should probably use address-cells/size-cells 2/2 on a 64-bit platform.
Quoted 13 lines
arch/arm64/boot/dts/exynos7.dtsi new file +13
-0+ aliases { + pinctrl0 = &pinctrl_0; + pinctrl1 = &pinctrl_1; + pinctrl2 = &pinctrl_2; + pinctrl3 = &pinctrl_3; + pinctrl4 = &pinctrl_4; + pinctrl5 = &pinctrl_5; + pinctrl6 = &pinctrl_6; + pinctrl7 = &pinctrl_7; + pinctrl8 = &pinctrl_8; + pinctrl9 = &pinctrl_9; + mshc0 = &mmc_0; + mshc2 = &mmc_2;
arch/arm64/boot/dts/exynos7.dtsi line 46
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ }; + + chipid at 10000000 { + compatible = "samsung,exynos4210-chipid"; + reg = <0x10000000 0x100>; + }; + + cpus { + #address-cells = <2>; + #size-cells = <0>;
Why size-cells=2? Can you not fit a cpuid in 32 bits?
Quoted 17 lines
arch/arm64/boot/dts/exynos7.dtsi new file +17
-0+ cpu at 0 { + device_type = "cpu"; + compatible = "arm,cortex-a57", "arm,armv8"; + reg = <0x0 0x0>; + }; + }; + + fin_pll: xxti { + compatible = "fixed-clock"; + clock-frequency = <24000000>; + clock-output-names = "fin_pll"; + #clock-cells = <0>; + }; + + gic: interrupt-controller at 11001000 { + compatible = "arm,gic-400"; + #interrupt-cells = <3>;
arch/arm64/boot/dts/exynos7.dtsi line 74
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ #address-cells = <0>; + interrupt-controller; + reg = <0x11001000 0x1000>, + <0x11002000 0x1000>, + <0x11004000 0x2000>, + <0x11006000 0x2000>; + }; + + hsi2c_0: hsi2c at 13640000 { + compatible = "samsung,exynos7-hsi2c";
Is the i2c controller here completely new?
Also, please use 'i2c' for node name on these nodes.
Quoted 15 lines
arch/arm64/boot/dts/exynos7.dtsi new file +15
-0+ reg = <0x13640000 0x1000>; + interrupts = <0 441 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c0_bus>; + clocks = <&clock_peric0 PCLK_HSI2C0>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_1: hsi2c at 13650000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x13650000 0x1000>; + interrupts = <0 442 0>;
arch/arm64/boot/dts/exynos7.dtsi line 99
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c1_bus>; + clocks = <&clock_peric0 PCLK_HSI2C1>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_2: hsi2c at 14E60000 {
I much prefer lowercase hex in unit addresses (and reg entries) below. I know 32-bit uses uppercase, but let's switch going forward here.
arch/arm64/boot/dts/exynos7.dtsi line 230
Quoted 2 lines
arch/arm64/boot/dts/exynos7.dtsi new file +2
-0+ mct at 101C0000 { + compatible = "samsung,exynos4210-mct";
Please just do away with MCT here, and use architected timers going forward. There really shouldn't be a need to keep supporting MCT any more -- it's a construct from before arch timers on Cortex-A9.
arch/arm64/boot/dts/exynos7.dtsi line 101
Quoted 2 lines
arch/arm64/boot/dts/exynos7.dtsi new file +2
-0+ mmc_0: mmc at 15740000 { + compatible = "samsung,exynos7-dw-mshc-smu";
Is this controller backwards compatible with exynos5 ones?
arch/arm64/boot/dts/exynos7.dtsi line 408
Quoted 1 line
arch/arm64/boot/dts/exynos7.dtsi new file +1
-0+ /* The Clock nodes are ordered as per the usermanual. */
"The clock"
"user manual"
arch/arm64/boot/dts/exynos7.dtsi line 549
Quoted 9 lines
arch/arm64/boot/dts/exynos7.dtsi new file +9
-0+ timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, + <1 14 0xff01>, + <1 11 0xff01>, + <1 10 0xff01>; + clock-frequency = <24000000>; + use-clocksource-only; + use-physical-timer;
These two properties are not standard, and I would expect any 64-bit platform to come with PSCI such that you have a way to initialize the virtual timers.
-Olof
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 160 lines · Olof Johansson
Hi,
Quoted 2 lines · Naveen Krishna Chatradhi
Add initial device tree nodes for EXYNOS7 SoC. Also, includes the dt-binding definitions for clock ids.
Uh, no -- it just adds the dtsi.
arch/arm64/boot/dts/exynos7.dtsi
Quoted 14 lines
Signed-off-by Naveen Krishna Chatradhi <[email protected]>Cc Thomas Abraham <[email protected]>Cc Rob Herring <[email protected]>Cc Catalin Marinas <[email protected]>--- arch/arm64/boot/dts/exynos7.dtsi | 553 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 553 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7.dtsiarch/arm64/boot/dts/exynos7.dtsi new file +0
-0Let's not make the same mistake as on 32-bit, and go with a directory hierarchy here from day one.
So, please create a exynos subdirectory for this file. You also need a
Makefilewhen you add a board dts.Quoted 13 lines
arch/arm64/boot/dts/exynos7.dtsi new file +12
-0@@ -0,0 +1,553 @@ +/* + * SAMSUNG EXYNOS7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * SAMSUNG EXYNOS7 SoC device nodes are listed in this file. + * EXYNOS7 based board files can include this file and provide + * values for board specfic bindings. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 asarch/arm64/boot/dts/exynos7.dtsi line 22
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ * published by the Free Software Foundation. + */ + +#include <dt-bindings/clock/exynos7-clk.h> + +/ { + compatible = "samsung,exynos7"; + interrupt-parent = <&gic>; + #address-cells = <1>; + #size-cells = <1>;You should probably use address-cells/size-cells 2/2 on a 64-bit platform.
Quoted 13 lines
arch/arm64/boot/dts/exynos7.dtsi new file +13
-0+ aliases { + pinctrl0 = &pinctrl_0; + pinctrl1 = &pinctrl_1; + pinctrl2 = &pinctrl_2; + pinctrl3 = &pinctrl_3; + pinctrl4 = &pinctrl_4; + pinctrl5 = &pinctrl_5; + pinctrl6 = &pinctrl_6; + pinctrl7 = &pinctrl_7; + pinctrl8 = &pinctrl_8; + pinctrl9 = &pinctrl_9; + mshc0 = &mmc_0; + mshc2 = &mmc_2;arch/arm64/boot/dts/exynos7.dtsi line 46
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ }; + + chipid at 10000000 { + compatible = "samsung,exynos4210-chipid"; + reg = <0x10000000 0x100>; + }; + + cpus { + #address-cells = <2>; + #size-cells = <0>;Why size-cells=2? Can you not fit a cpuid in 32 bits?
Quoted 17 lines
arch/arm64/boot/dts/exynos7.dtsi new file +17
-0+ cpu at 0 { + device_type = "cpu"; + compatible = "arm,cortex-a57", "arm,armv8"; + reg = <0x0 0x0>; + }; + }; + + fin_pll: xxti { + compatible = "fixed-clock"; + clock-frequency = <24000000>; + clock-output-names = "fin_pll"; + #clock-cells = <0>; + }; + + gic: interrupt-controller at 11001000 { + compatible = "arm,gic-400"; + #interrupt-cells = <3>;arch/arm64/boot/dts/exynos7.dtsi line 74
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ #address-cells = <0>; + interrupt-controller; + reg = <0x11001000 0x1000>, + <0x11002000 0x1000>, + <0x11004000 0x2000>, + <0x11006000 0x2000>; + }; + + hsi2c_0: hsi2c at 13640000 { + compatible = "samsung,exynos7-hsi2c";Is the i2c controller here completely new?
Also, please use 'i2c' for node name on these nodes.
Quoted 15 lines
arch/arm64/boot/dts/exynos7.dtsi new file +15
-0+ reg = <0x13640000 0x1000>; + interrupts = <0 441 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c0_bus>; + clocks = <&clock_peric0 PCLK_HSI2C0>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_1: hsi2c at 13650000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x13650000 0x1000>; + interrupts = <0 442 0>;arch/arm64/boot/dts/exynos7.dtsi line 99
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c1_bus>; + clocks = <&clock_peric0 PCLK_HSI2C1>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_2: hsi2c at 14E60000 {I much prefer lowercase hex in unit addresses (and reg entries) below. I know 32-bit uses uppercase, but let's switch going forward here.
arch/arm64/boot/dts/exynos7.dtsi line 230
Quoted 2 lines
arch/arm64/boot/dts/exynos7.dtsi new file +2
-0+ mct at 101C0000 { + compatible = "samsung,exynos4210-mct";Please just do away with MCT here, and use architected timers going forward. There really shouldn't be a need to keep supporting MCT any more -- it's a construct from before arch timers on Cortex-A9.
arch/arm64/boot/dts/exynos7.dtsi line 101
Quoted 2 lines
arch/arm64/boot/dts/exynos7.dtsi new file +2
-0+ mmc_0: mmc at 15740000 { + compatible = "samsung,exynos7-dw-mshc-smu";Is this controller backwards compatible with exynos5 ones?
arch/arm64/boot/dts/exynos7.dtsi line 408
Quoted 1 line
arch/arm64/boot/dts/exynos7.dtsi new file +1
-0+ /* The Clock nodes are ordered as per the usermanual. */"The clock"
"user manual"
Quoted 13 lines
arch/arm64/boot/dts/exynos7.dtsi line 549
Quoted 9 lines
arch/arm64/boot/dts/exynos7.dtsi new file +9
-0+ timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, + <1 14 0xff01>, + <1 11 0xff01>, + <1 10 0xff01>; + clock-frequency = <24000000>; + use-clocksource-only; + use-physical-timer;These two properties are not standard, and I would expect any 64-bit platform to come with PSCI such that you have a way to initialize the virtual timers.
It really sickens me that this is the n-th iteration of a Samsung SoC having the generic timer (basically since the 5250 came out), and still it is littered with stupid firmware bugs:
- Broken CNTFRQ (as outlined by the need of clock-frequency)
- Broken CNTVOFF (as hinted by the reliance on the physical timer)
You would think that after over two years, someone would have a clue and added the missing 4 instructions to the boot ROM.
Or not.
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
Mark Rutland
reviewerPATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
In reply to Olof JohanssonHi,
Quoted 5 lines
arch/arm64/boot/dts/exynos7.dtsi line 46
Quoted 3 lines
arch/arm64/boot/dts/exynos7.dtsi new file +3
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>;Why size-cells=2? Can you not fit a cpuid in 32 bits?
As of commit 72aea393a2e7 (arm64: smp: honour #address-size when parsing CPU reg property) Linux can handle single-cell cpu node reg entries where /cpus/#address-cells = <1>.
I can't make any guarantees about other code (e.g. bootloaders) which might try to do things with cpu nodes, YMMV.
[...]
Quoted 4 lines
arch/arm64/boot/dts/exynos7.dtsi line 99
Quoted 1 line
arch/arm64/boot/dts/exynos7.dtsi new file +1
-0+ hsi2c_2: hsi2c at 14E60000 {I much prefer lowercase hex in unit addresses (and reg entries) below. I know 32-bit uses uppercase, but let's switch going forward here.
My preference also; I'm happy to enforce that on new dts.
[...]
Quoted 5 lines
Quoted 5 lines
arch/arm64/boot/dts/exynos7.dtsi new file +5
-0+ timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, + <1 14 0xff01>, + <1 11 0xff01>,
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi line 549
Quoted 4 lines
arch/arm64/boot/dts/exynos7.dtsi new file +4
-0+ <1 10 0xff01>; + clock-frequency = <24000000>; + use-clocksource-only; + use-physical-timer;These two properties are not standard, and I would expect any 64-bit platform to come with PSCI such that you have a way to initialize the virtual timers.
Likewise with clock-frequency. It's not a full workaround, and it's not hard to initialise CNTFRQ on each CPU.
Mark.
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 6 lines · Mark Rutland
Hi,
Quoted 4 lines
Quoted 3 lines
arch/arm64/boot/dts/exynos7.dtsi new file +3
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>;
Quoted 8 lines
Quoted 1 line
Why size-cells=2? Can you not fit a cpuid in 32 bits?
As of commit
72aea393a2e7(arm64: smp: honour #address-size when parsing CPU reg property) Linux can handle single-cell cpu node reg entries where /cpus/#address-cells = <1>.I can't make any guarantees about other code (e.g. bootloaders) which might try to do things with cpu nodes, YMMV.
Ok. If address-cells is kept at 2 the unit address needs to be changed to "0,0". So one or the other has to be changed.
Quoted 21 lines
[...]
Quoted 4 lines
Quoted 1 line
+
hsi2c_2: hsi2c at 14E60000 {I much prefer lowercase hex in unit addresses (and reg entries) below. I know 32-bit uses uppercase, but let's switch going forward here.
My preference also; I'm happy to enforce that on new dts.
[...]
Quoted 10 lines
Quoted 9 lines
arch/arm64/boot/dts/exynos7.dtsi new file +9
-0+ timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, + <1 14 0xff01>, + <1 11 0xff01>, + <1 10 0xff01>; + clock-frequency = <24000000>; + use-clocksource-only; + use-physical-timer;
Quoted 6 lines
Quoted 3 lines
These two properties are not standard, and I would expect any 64-bit platform to come with PSCI such that you have a way to initialize the virtual timers.
Likewise with clock-frequency. It's not a full workaround, and it's not hard to initialise CNTFRQ on each CPU.
Technically clock-frequency is documented, but not recommended to be used unless needed for working around firmware that doesn't setup the register value. :)
In this case it's likely a cargo cult carry over from 5250 where the CNTFRQ requirement happened around the same time as we were working on it so that generation firmware lacked support for it -- it should since then have been fixed properly.
-Olof
Mark Rutland
reviewerPATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 13 lines · Olof Johansson
Quoted 12 lines · Mark Rutland
Hi,
Quoted 5 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>;Why size-cells=2? Can you not fit a cpuid in 32 bits?
As of commit
72aea393a2e7(arm64: smp: honour #address-size when parsing CPU reg property) Linux can handle single-cell cpu node reg entries where /cpus/#address-cells = <1>.
Quoted 5 lines
Quoted 2 lines
I can't make any guarantees about other code (e.g. bootloaders) which might try to do things with cpu nodes, YMMV.
Ok. If address-cells is kept at 2 the unit address needs to be changed to "0,0". So one or the other has to be changed.
I'm happy either way.
I'm not sure the rest of the tree had "0," prefixes on all of the unit-addresses for 64-bit addresses that were under 4GB, and I'm not sure that existing dts consistently do that either.
Do we want to enforce that for all 64-bit unit-addresses?
Quoted 25 lines
Quoted 25 lines
[...]
Quoted 4 lines
Quoted 1 line
+
hsi2c_2: hsi2c at 14E60000 {I much prefer lowercase hex in unit addresses (and reg entries) below. I know 32-bit uses uppercase, but let's switch going forward here.
My preference also; I'm happy to enforce that on new dts.
[...]
Quoted 13 lines
Review
Quoted 9 lines
(quoted diff) +9
-0+ timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, + <1 14 0xff01>, + <1 11 0xff01>, + <1 10 0xff01>; + clock-frequency = <24000000>; + use-clocksource-only; + use-physical-timer;These two properties are not standard, and I would expect any 64-bit platform to come with PSCI such that you have a way to initialize the virtual timers.
Quoted 6 lines
Quoted 2 lines
Likewise with clock-frequency. It's not a full workaround, and it's not hard to initialise CNTFRQ on each CPU.
Technically clock-frequency is documented, but not recommended to be used unless needed for working around firmware that doesn't setup the register value. :)
True.
Quoted 4 lines
In this case it's likely a cargo cult carry over from 5250 where the CNTFRQ requirement happened around the same time as we were working on it so that generation firmware lacked support for it -- it should since then have been fixed properly.
It's probably unhelpful that the documentation isn't explicit about that. On that front, how about the patch below?
Mark.
---->8----
Quoted 1 line
From
67104ad5a56e4c18f9c41f06af028b7561740afdMon Sep 17 00:00:00 2001
From: Mark Rutland <[email protected]>
Date: Thu, 28 Aug 2014 17:41:03 +0100
Subject: [PATCH] Doc: dt: arch_timer: discourage clock-frequency use
The ARM Generic Timer (AKA the architected timer, arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states.
The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed the example set by these early SoCs.
This patch updates the arch timer binding documentation to make it clearer that the use of the clock-frequency property is a poor work-around. The MMIO generic timer binding is similarly updated, though this is less of a concern as there is generally no need to expose the MMIO timers to guest OSs.
--- Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/arm/arch_timer.txt +6 -2
@@ -17,7 +17,10 @@ to deliver its interrupts via SPIs. - interrupts : Interrupt list for secure, non-secure, virtual and hypervisor timers, in that order. -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only where necessary to work around BROKEN firmware which does not configure + CNTFRQ on all CPUs to a uniform correct value. Use of this property is + STRONGLY DISCOURAGED; fix your firmware unless absolutely impossible. - always-on : a boolean property. If present, the timer is powered through an always-on power domain, therefore it never loses context.
@@ -38,7 +41,8 @@ Example: - compatible : Should at least contain "arm,armv7-timer-mem". -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only when firmware has not configured the MMIO CNTFRQ registers. - reg : The control frame base address.
Signature · 1 line
1.9.1
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 20 lines · Mark Rutland
Quoted 13 lines · Olof Johansson
Quoted 12 lines · Mark Rutland
Hi,
Quoted 5 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>;Why size-cells=2? Can you not fit a cpuid in 32 bits?
As of commit
72aea393a2e7(arm64: smp: honour #address-size when parsing CPU reg property) Linux can handle single-cell cpu node reg entries where /cpus/#address-cells = <1>.Quoted 5 lines
Quoted 2 lines
I can't make any guarantees about other code (e.g. bootloaders) which might try to do things with cpu nodes, YMMV.
Ok. If address-cells is kept at 2 the unit address needs to be changed to "0,0". So one or the other has to be changed.
Quoted 7 lines
I'm happy either way.
I'm not sure the rest of the tree had "0," prefixes on all of the unit-addresses for 64-bit addresses that were under 4GB, and I'm not sure that existing dts consistently do that either.
Do we want to enforce that for all 64-bit unit-addresses?
Yeah, I believe that's the only valid format for a 2-address-cell unit address.
Quoted 63 lines
Quoted 25 lines
Quoted 25 lines
[...]
Quoted 4 lines
Quoted 1 line
+
hsi2c_2: hsi2c at 14E60000 {I much prefer lowercase hex in unit addresses (and reg entries) below. I know 32-bit uses uppercase, but let's switch going forward here.
My preference also; I'm happy to enforce that on new dts.
[...]
Quoted 13 lines
Review
Quoted 9 lines
(quoted diff) +9
-0+ timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, + <1 14 0xff01>, + <1 11 0xff01>, + <1 10 0xff01>; + clock-frequency = <24000000>; + use-clocksource-only; + use-physical-timer;These two properties are not standard, and I would expect any 64-bit platform to come with PSCI such that you have a way to initialize the virtual timers.
Quoted 6 lines
Quoted 2 lines
Likewise with clock-frequency. It's not a full workaround, and it's not hard to initialise CNTFRQ on each CPU.
Technically clock-frequency is documented, but not recommended to be used unless needed for working around firmware that doesn't setup the register value. :)
True.
Quoted 4 lines
In this case it's likely a cargo cult carry over from 5250 where the CNTFRQ requirement happened around the same time as we were working on it so that generation firmware lacked support for it -- it should since then have been fixed properly.
It's probably unhelpful that the documentation isn't explicit about that. On that front, how about the patch below?
Mark.
---->8----
From67104ad5a56e4c18f9c41f06af028b7561740afdMon Sep 17 00:00:00 2001
From: Mark Rutland <[email protected]>
Date: Thu, 28 Aug 2014 17:41:03 +0100
Subject: [PATCH] Doc: dt:arch_timer: discourage clock-frequency useThe ARM Generic Timer (AKA the architected timer,
arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states.The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed the example set by these early SoCs.
Quoted 8 lines
This patch updates the arch timer binding documentation to make it clearer that the use of the clock-frequency property is a poor work-around. The MMIO generic timer binding is similarly updated, though this is less of a concern as there is generally no need to expose the MMIO timers to guest OSs.
Signed-off-by Mark Rutland <[email protected]>Cc Marc Zyngier <[email protected]>
With caps fixed:
Quoted 9 lines
--- Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/arm/arch_timer.txt +0
-0
Documentation/devicetree/bindings/arm/arch_timer.txt to deliver its interrupts via SPIs. · line 21
Quoted 6 lines
Documentation/devicetree/bindings/arm/arch_timer.txt +2
-1@@ -17,7 +17,10 @@ to deliver its interrupts via SPIs. - interrupts : Interrupt list for secure, non-secure, virtual and hypervisor timers, in that order. -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only where necessary to work around BROKEN firmware which does not configure
No need to do broken in all caps. In reality I don't expect it to make a difference on people complying or not. :)
Documentation/devicetree/bindings/arm/arch_timer.txt to deliver its interrupts via SPIs. · line 23
Quoted 2 lines
Documentation/devicetree/bindings/arm/arch_timer.txt +2
-0+ CNTFRQ on all CPUs to a uniform correct value. Use of this property is + STRONGLY DISCOURAGED; fix your firmware unless absolutely impossible.
Same here.
Quoted 16 lines
Documentation/devicetree/bindings/arm/arch_timer.txt +2
-2- always-on : a boolean property. If present, the timer is powered through an always-on power domain, therefore it never loses context.@@ -38,7 +41,8 @@ Example: - compatible : Should at least contain "arm,armv7-timer-mem". -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only when firmware has not configured the MMIO CNTFRQ registers. - reg : The control frame base address. --1.9.1
Mark Rutland
reviewerPATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 23 lines · Olof Johansson
Quoted 22 lines · Mark Rutland
Quoted 13 lines · Olof Johansson
Quoted 12 lines · Mark Rutland
Hi,
Quoted 5 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>;Why size-cells=2? Can you not fit a cpuid in 32 bits?
As of commit
72aea393a2e7(arm64: smp: honour #address-size when parsing CPU reg property) Linux can handle single-cell cpu node reg entries where /cpus/#address-cells = <1>.Quoted 5 lines
Quoted 2 lines
I can't make any guarantees about other code (e.g. bootloaders) which might try to do things with cpu nodes, YMMV.
Ok. If address-cells is kept at 2 the unit address needs to be changed to "0,0". So one or the other has to be changed.
I'm happy either way.
Quoted 7 lines
Quoted 5 lines
I'm not sure the rest of the tree had "0," prefixes on all of the unit-addresses for 64-bit addresses that were under 4GB, and I'm not sure that existing dts consistently do that either.
Do we want to enforce that for all 64-bit unit-addresses?
Yeah, I believe that's the only valid format for a 2-address-cell unit address.
Fair enough. I didn't spot this explicitly mentioned anywhere in ePAPR, but the examples match.
I should probably re-jig that checkpatch test I had for unit-addresses.
Quoted 69 lines
Quoted 69 lines
Quoted 25 lines
Quoted 25 lines
[...]
Quoted 4 lines
Quoted 1 line
+
hsi2c_2: hsi2c at 14E60000 {I much prefer lowercase hex in unit addresses (and reg entries) below. I know 32-bit uses uppercase, but let's switch going forward here.
My preference also; I'm happy to enforce that on new dts.
[...]
Quoted 13 lines
Review
Quoted 9 lines
(quoted diff) +9
-0+ timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, + <1 14 0xff01>, + <1 11 0xff01>, + <1 10 0xff01>; + clock-frequency = <24000000>; + use-clocksource-only; + use-physical-timer;These two properties are not standard, and I would expect any 64-bit platform to come with PSCI such that you have a way to initialize the virtual timers.
Quoted 6 lines
Quoted 2 lines
Likewise with clock-frequency. It's not a full workaround, and it's not hard to initialise CNTFRQ on each CPU.
Technically clock-frequency is documented, but not recommended to be used unless needed for working around firmware that doesn't setup the register value. :)
True.
Quoted 4 lines
In this case it's likely a cargo cult carry over from 5250 where the CNTFRQ requirement happened around the same time as we were working on it so that generation firmware lacked support for it -- it should since then have been fixed properly.
It's probably unhelpful that the documentation isn't explicit about that. On that front, how about the patch below?
Mark.
---->8----
From67104ad5a56e4c18f9c41f06af028b7561740afdMon Sep 17 00:00:00 2001
From: Mark Rutland <[email protected]>
Date: Thu, 28 Aug 2014 17:41:03 +0100
Subject: [PATCH] Doc: dt:arch_timer: discourage clock-frequency useThe ARM Generic Timer (AKA the architected timer,
arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states.The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed the example set by these early SoCs.
This patch updates the arch timer binding documentation to make it clearer that the use of the clock-frequency property is a poor work-around. The MMIO generic timer binding is similarly updated, though this is less of a concern as there is generally no need to expose the MMIO timers to guest OSs.
Quoted 6 lines
Quoted 2 lines
Signed-off-by Mark Rutland <[email protected]>Cc Marc Zyngier <[email protected]>With caps fixed:
Acked-by Olof Johansson <[email protected]>
Cheers.
Quoted 12 lines
Quoted 12 lines
--- Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/arm/arch_timer.txt +0
-0@@ -17,7 +17,10 @@ to deliver its interrupts via SPIs. - interrupts : Interrupt list for secure, non-secure, virtual and hypervisor timers, in that order.
Quoted 6 lines
Documentation/devicetree/bindings/arm/arch_timer.txt to deliver its interrupts via SPIs. · line 21
Quoted 3 lines
Documentation/devicetree/bindings/arm/arch_timer.txt +2
-1-- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only where necessary to work around BROKEN firmware which does not configureNo need to do broken in all caps. In reality I don't expect it to make a difference on people complying or not. :)
Sure. I'll save the caps for replies to violators ;)
Mark.
Quoted 22 lines
Documentation/devicetree/bindings/arm/arch_timer.txt to deliver its interrupts via SPIs. · line 23
Quoted 2 lines
Documentation/devicetree/bindings/arm/arch_timer.txt +2
-0+ CNTFRQ on all CPUs to a uniform correct value. Use of this property is + STRONGLY DISCOURAGED; fix your firmware unless absolutely impossible.Same here.
Quoted 16 lines
Documentation/devicetree/bindings/arm/arch_timer.txt +2
-2- always-on : a boolean property. If present, the timer is powered through an always-on power domain, therefore it never loses context.@@ -38,7 +41,8 @@ Example: - compatible : Should at least contain "arm,armv7-timer-mem". -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only when firmware has not configured the MMIO CNTFRQ registers. - reg : The control frame base address. --1.9.1
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Hi Mark,
Quoted 9 lines · Mark Rutland
Quoted 9 lines
Quoted 9 lines
Quoted 2 lines
Ok. If address-cells is kept at 2 the unit address needs to be changed to "0,0". So one or the other has to be changed.
I'm happy either way.
I'm not sure the rest of the tree had "0," prefixes on all of the unit-addresses for 64-bit addresses that were under 4GB, and I'm not sure that existing dts consistently do that either.
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Do we want to enforce that for all 64-bit unit-addresses?
Yeah, I believe that's the only valid format for a 2-address-cell unit address.
Fair enough. I didn't spot this explicitly mentioned anywhere in ePAPR, but the examples match.
I couldn't find much about how the unit-addresses should really look like.
Power_ePAPR_APPROVED_v1.1.pdf:
"The unit-address component of the name is specific to the bus type on which the node sits. It consists
of one or more ASCII characters from the set of characters in Table 2-1. The unit-address must
match the first address specified in the reg property of the node. If the node has no reg property, the
@ and unit-address must be omitted and the node-name alone differentiates the node from other nodes
at the same level in the tree. The binding for a particular bus may specify additional, more specific
requirements for the format of reg and the unit-address."
"Table 2.1" contains lot of characters, definitely not limited to hex numbers. Also nothing about (not) needing a "0x" prefix.
Quoted 1 line
I should probably re-jig that checkpatch test I had for unit-addresses.
It would be great if dtc started complaining about unit-addresses not matching the first reg property.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Mark Rutland
reviewerPATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 34 lines · Geert Uytterhoeven
Hi Mark,
Quoted 9 lines · Mark Rutland
Quoted 9 lines
Quoted 9 lines
Quoted 2 lines
Ok. If address-cells is kept at 2 the unit address needs to be changed to "0,0". So one or the other has to be changed.
I'm happy either way.
I'm not sure the rest of the tree had "0," prefixes on all of the unit-addresses for 64-bit addresses that were under 4GB, and I'm not sure that existing dts consistently do that either.
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Do we want to enforce that for all 64-bit unit-addresses?
Yeah, I believe that's the only valid format for a 2-address-cell unit address.
Fair enough. I didn't spot this explicitly mentioned anywhere in ePAPR, but the examples match.
I couldn't find much about how the unit-addresses should really look like.
Power_ePAPR_APPROVED_v1.1.pdf:
"The unit-address component of the name is specific to the bus type on which the node sits. It consists
of one or more ASCII characters from the set of characters in Table 2-1. The unit-address must
match the first address specified in the reg property of the node. If the node has no reg property, the
@ and unit-address must be omitted and the node-name alone differentiates the node from other nodes
at the same level in the tree. The binding for a particular bus may specify additional, more specific
requirements for the format of reg and the unit-address."
Quoted 2 lines
"Table 2.1" contains lot of characters, definitely not limited to hex numbers. Also nothing about (not) needing a "0x" prefix.
This is unfortunate. I guess this was assumed to be implied by way of the examples. :/
Quoted 4 lines
Quoted 1 line
I should probably re-jig that checkpatch test I had for unit-addresses.
It would be great if dtc started complaining about unit-addresses not matching the first reg property.
Agreed.
When I last tried I thought that required more complex parsing than could be done with a regex.
That said, I'd forgotten that properties must come before child nodes, so I though I had to at least balance '{' and '}' for children. I guess all we need to do is find a line beginning with '\s*reg\s*=\s*<' before the next '{' or '}'.
Maybe this will be easier than previously thought. :)
Mark.
Rob Herring
maintainerPATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
In reply to Olof JohanssonQuoted 23 lines · Olof Johansson
Quoted 22 lines · Mark Rutland
Quoted 13 lines · Olof Johansson
Quoted 12 lines · Mark Rutland
Hi,
Quoted 5 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>;Why size-cells=2? Can you not fit a cpuid in 32 bits?
As of commit
72aea393a2e7(arm64: smp: honour #address-size when parsing CPU reg property) Linux can handle single-cell cpu node reg entries where /cpus/#address-cells = <1>.Quoted 5 lines
Quoted 2 lines
I can't make any guarantees about other code (e.g. bootloaders) which might try to do things with cpu nodes, YMMV.
Ok. If address-cells is kept at 2 the unit address needs to be changed to "0,0". So one or the other has to be changed.
I'm happy either way.
Quoted 7 lines
Quoted 5 lines
I'm not sure the rest of the tree had "0," prefixes on all of the unit-addresses for 64-bit addresses that were under 4GB, and I'm not sure that existing dts consistently do that either.
Do we want to enforce that for all 64-bit unit-addresses?
Yeah, I believe that's the only valid format for a 2-address-cell unit address.
But we don't do leading 0's anywhere else like single cell unit addresses. Buses expressed with ranges and offsets are one example. Also, I2C addresses have a 32-bit size in DT yet are only 8-bit and we don't do leading zero's there.
Rob
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 28 lines · Rob Herring
Quoted 27 lines · Olof Johansson
Quoted 26 lines · Mark Rutland
Quoted 13 lines · Olof Johansson
Quoted 12 lines · Mark Rutland
Hi,
Quoted 5 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>;Why size-cells=2? Can you not fit a cpuid in 32 bits?
As of commit
72aea393a2e7(arm64: smp: honour #address-size when parsing CPU reg property) Linux can handle single-cell cpu node reg entries where /cpus/#address-cells = <1>.Quoted 5 lines
Quoted 2 lines
I can't make any guarantees about other code (e.g. bootloaders) which might try to do things with cpu nodes, YMMV.
Ok. If address-cells is kept at 2 the unit address needs to be changed to "0,0". So one or the other has to be changed.
I'm happy either way.
I'm not sure the rest of the tree had "0," prefixes on all of the unit-addresses for 64-bit addresses that were under 4GB, and I'm not sure that existing dts consistently do that either.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Do we want to enforce that for all 64-bit unit-addresses?
Yeah, I believe that's the only valid format for a 2-address-cell unit address.
But we don't do leading 0's anywhere else like single cell unit addresses. Buses expressed with ranges and offsets are one example. Also, I2C addresses have a 32-bit size in DT yet are only 8-bit and we don't do leading zero's there.
Ok, I'm happily proven wrong here, also by confirming how this is done on "real" OF.
According to benh:
15:20 <benh> ojn: 0,0 is not quite right, it's supposed to be used when the two numbers are different things, like device,fn on PCI
The same is true for >2^32 unit addresses, they just use the one integer instead of x,y.
So, I take back all I've said on this in the last 72 hours. :) It looks like we might need to revisit some of the 32-bit DTs. Simon, drop the series you had. :)
-Olof
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 46 lines · Olof Johansson
Quoted 28 lines · Rob Herring
Quoted 27 lines · Olof Johansson
Quoted 26 lines · Mark Rutland
Quoted 13 lines · Olof Johansson
Quoted 12 lines · Mark Rutland
Hi,
Quoted 5 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ cpus { + #address-cells = <2>; + #size-cells = <0>;Why size-cells=2? Can you not fit a cpuid in 32 bits?
As of commit
72aea393a2e7(arm64: smp: honour #address-size when parsing CPU reg property) Linux can handle single-cell cpu node reg entries where /cpus/#address-cells = <1>.Quoted 5 lines
Quoted 2 lines
I can't make any guarantees about other code (e.g. bootloaders) which might try to do things with cpu nodes, YMMV.
Ok. If address-cells is kept at 2 the unit address needs to be changed to "0,0". So one or the other has to be changed.
I'm happy either way.
I'm not sure the rest of the tree had "0," prefixes on all of the unit-addresses for 64-bit addresses that were under 4GB, and I'm not sure that existing dts consistently do that either.
Quoted 8 lines
Quoted 3 lines
Quoted 1 line
Do we want to enforce that for all 64-bit unit-addresses?
Yeah, I believe that's the only valid format for a 2-address-cell unit address.
But we don't do leading 0's anywhere else like single cell unit addresses. Buses expressed with ranges and offsets are one example. Also, I2C addresses have a 32-bit size in DT yet are only 8-bit and we don't do leading zero's there.
Ok, I'm happily proven wrong here, also by confirming how this is done on "real" OF.
According to benh:
15:20 <benh> ojn: 0,0 is not quite right, it's supposed to be used when the two numbers are different things, like device,fn on PCI
Quoted 6 lines
The same is true for >2^32 unit addresses, they just use the one integer instead of x,y.
So, I take back all I've said on this in the last 72 hours. :) It looks like we might need to revisit some of the 32-bit DTs. Simon, drop the series you had. :)
Thanks will do.
Is the way forward for me to apply my original patch to change memory at 180000000 to memory at 140000000? And abandon all the ',' only changes?This one:
This is the original patch:
From: Simon Horman <[email protected]>
[PATCH] ARM: shmobile: lager: correct memory map
The base address of the second memory region on the lager board is 0x140000000. Update the tag used in the dts file accordingly.
This is a documentation fix and should have no run-time affect.
This problem was introduced when the second memory region was added to the lager dts file by 62bc32a2573c4219 ("ARM: shmobile: Include all 4 GiB of memory on Lager)" in v3.14.
--- arch/arm/boot/dts/r8a7790-lager.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/r8a7790-lager.dts +1 -1
@@ -32,7 +32,7 @@ reg = <0 0x40000000 0 0x40000000>; }; - memory at 180000000 { + memory at 140000000 { device_type = "memory"; reg = <1 0x40000000 0 0xc0000000>; };
Signature · 1 line
2.0.1
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
In reply to Mark RutlandQuoted 11 lines · Mark Rutland
From
67104ad5a56e4c18f9c41f06af028b7561740afdMon Sep 17 00:00:00 2001
From: Mark Rutland <[email protected]>
Date: Thu, 28 Aug 2014 17:41:03 +0100
Subject: [PATCH] Doc: dt:arch_timer: discourage clock-frequency useThe ARM Generic Timer (AKA the architected timer,
arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states.
Quoted 5 lines
The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed
I believe Xen is also affected by this.
Quoted 10 lines
the example set by these early SoCs.
This patch updates the arch timer binding documentation to make it clearer that the use of the clock-frequency property is a poor work-around. The MMIO generic timer binding is similarly updated, though this is less of a concern as there is generally no need to expose the MMIO timers to guest OSs.
Signed-off-by Mark Rutland <[email protected]>Cc Marc Zyngier <[email protected]>
Short of more explicit threats:
Quoted 31 lines
--- Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/arm/arch_timer.txt +6
-2@@ -17,7 +17,10 @@ to deliver its interrupts via SPIs. - interrupts : Interrupt list for secure, non-secure, virtual and hypervisor timers, in that order. -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only where necessary to work around BROKEN firmware which does not configure + CNTFRQ on all CPUs to a uniform correct value. Use of this property is + STRONGLY DISCOURAGED; fix your firmware unless absolutely impossible. - always-on : a boolean property. If present, the timer is powered through an always-on power domain, therefore it never loses context.@@ -38,7 +41,8 @@ Example: - compatible : Should at least contain "arm,armv7-timer-mem". -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only when firmware has not configured the MMIO CNTFRQ registers. - reg : The control frame base address.
Signature · 1 line
Jazz is not dead. It just smells funny...
Mark Rutland
reviewerPATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 13 lines · Marc Zyngier
Quoted 11 lines · Mark Rutland
From
67104ad5a56e4c18f9c41f06af028b7561740afdMon Sep 17 00:00:00 2001
From: Mark Rutland <[email protected]>
Date: Thu, 28 Aug 2014 17:41:03 +0100
Subject: [PATCH] Doc: dt:arch_timer: discourage clock-frequency useThe ARM Generic Timer (AKA the architected timer,
arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states.
Quoted 7 lines
Quoted 5 lines
The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed
I believe Xen is also affected by this.
True.
s/KVM/KVM\/Xen/, then?
Quoted 8 lines
Quoted 8 lines
the example set by these early SoCs.
This patch updates the arch timer binding documentation to make it clearer that the use of the clock-frequency property is a poor work-around. The MMIO generic timer binding is similarly updated, though this is less of a concern as there is generally no need to expose the MMIO timers to guest OSs.
Quoted 6 lines
Quoted 2 lines
Signed-off-by Mark Rutland <[email protected]>Cc Marc Zyngier <[email protected]>Short of more explicit threats:
Acked-by Marc Zyngier <[email protected]>
Cheers.
Mark.
Quoted 36 lines
Quoted 32 lines
--- Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)Documentation/devicetree/bindings/arm/arch_timer.txt +6
-2@@ -17,7 +17,10 @@ to deliver its interrupts via SPIs. - interrupts : Interrupt list for secure, non-secure, virtual and hypervisor timers, in that order. -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only where necessary to work around BROKEN firmware which does not configure + CNTFRQ on all CPUs to a uniform correct value. Use of this property is + STRONGLY DISCOURAGED; fix your firmware unless absolutely impossible. - always-on : a boolean property. If present, the timer is powered through an always-on power domain, therefore it never loses context.@@ -38,7 +41,8 @@ Example: - compatible : Should at least contain "arm,armv7-timer-mem". -- clock-frequency : The frequency of the main counter, in Hz. Optional. +- clock-frequency : The frequency of the main counter, in Hz. Should be present + only when firmware has not configured the MMIO CNTFRQ registers. - reg : The control frame base address.Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 20 lines · Mark Rutland
Quoted 19 lines · Marc Zyngier
Quoted 16 lines · Mark Rutland
From
67104ad5a56e4c18f9c41f06af028b7561740afdMon Sep 17 00:00:00 2001
From: Mark Rutland <[email protected]>
Date: Thu, 28 Aug 2014 17:41:03 +0100
Subject: [PATCH] Doc: dt:arch_timer: discourage clock-frequency useThe ARM Generic Timer (AKA the architected timer,
arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states.The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed
Quoted 5 lines
Quoted 1 line
I believe Xen is also affected by this.
True.
s/KVM/KVM\/Xen/, then?
Yup. Or "including guests running under a hypervisor", I expect this to be such a fundamental problem that all hypervisors will trip over on that one (Jailhouse definitely does).
Thanks,
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
Mark Rutland
reviewerPATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 21 lines · Marc Zyngier
Quoted 20 lines · Mark Rutland
Quoted 19 lines · Marc Zyngier
Quoted 16 lines · Mark Rutland
From
67104ad5a56e4c18f9c41f06af028b7561740afdMon Sep 17 00:00:00 2001
From: Mark Rutland <[email protected]>
Date: Thu, 28 Aug 2014 17:41:03 +0100
Subject: [PATCH] Doc: dt:arch_timer: discourage clock-frequency useThe ARM Generic Timer (AKA the architected timer,
arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states.The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
I believe Xen is also affected by this.
True.
s/KVM/KVM\/Xen/, then?
Yup. Or "including guests running under a hypervisor"
Ah, that sounds better. I'll use that for the next posting.
Quoted 2 lines
I expect this to be such a fundamental problem that all hypervisors will trip over on that one (Jailhouse definitely does).
Yeah, this is a generic problem.
Mark.
Rob Herring
maintainerPATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
In reply to Marc ZyngierQuoted 29 lines · Marc Zyngier
Quoted 11 lines · Mark Rutland
From
67104ad5a56e4c18f9c41f06af028b7561740afdMon Sep 17 00:00:00 2001
From: Mark Rutland <[email protected]>
Date: Thu, 28 Aug 2014 17:41:03 +0100
Subject: [PATCH] Doc: dt:arch_timer: discourage clock-frequency useThe ARM Generic Timer (AKA the architected timer,
arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states.Quoted 5 lines
The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed
I believe Xen is also affected by this.
Quoted 8 lines
the example set by these early SoCs.
This patch updates the arch timer binding documentation to make it clearer that the use of the clock-frequency property is a poor work-around. The MMIO generic timer binding is similarly updated, though this is less of a concern as there is generally no need to expose the MMIO timers to guest OSs.
Quoted 4 lines
Quoted 2 lines
Signed-off-by Mark Rutland <[email protected]>Cc Marc Zyngier <[email protected]>Short of more explicit threats:
Why not also add WARNs (and mark for stable). People tend to notice and fix those. If not the vendors, those pesky customers always complain (the same ones that get concerned about BogoMIPS values being too low).
Rob
Mark Rutland
reviewerPATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
Quoted 33 lines · Rob Herring
Quoted 32 lines · Marc Zyngier
Quoted 11 lines · Mark Rutland
From
67104ad5a56e4c18f9c41f06af028b7561740afdMon Sep 17 00:00:00 2001
From: Mark Rutland <[email protected]>
Date: Thu, 28 Aug 2014 17:41:03 +0100
Subject: [PATCH] Doc: dt:arch_timer: discourage clock-frequency useThe ARM Generic Timer (AKA the architected timer,
arm_arch_timer) features a CPU register (CNTFRQ) which firmware is intended to initialize, and non-secure software can read to determine the frequency of the timer. On CPUs with secure state, this register cannot be written from non-secure states.Quoted 5 lines
The firmware of early SoCs featuring the timer did not correctly initialize CNTFRQ correctly on all CPUs, requiring the frequency to be described in DT as a workaround. This workaround is not complete however as CNTFRQ is exposed to all software in a privileged non-secure mode, including KVM guests. The firmware and DTs for recent SoCs have followed
I believe Xen is also affected by this.
Quoted 10 lines
the example set by these early SoCs.
This patch updates the arch timer binding documentation to make it clearer that the use of the clock-frequency property is a poor work-around. The MMIO generic timer binding is similarly updated, though this is less of a concern as there is generally no need to expose the MMIO timers to guest OSs.
Signed-off-by Mark Rutland <[email protected]>Cc Marc Zyngier <[email protected]>
Quoted 6 lines
Quoted 1 line
Short of more explicit threats:
Why not also add WARNs (and mark for stable). People tend to notice and fix those. If not the vendors, those pesky customers always complain (the same ones that get concerned about BogoMIPS values being too low).
I had a go a while back but it was a bit painful becuase the MMIO and cpu timer code is intertwined, and a clock-frequency property on the MMIO timers isn't all that problematic (though admittedly still a workaround for FW not initialising something it was intended to).
I was hoping I'd have the chance to split the driver in two first, so as to sidestep that. I'll take another look.
Mark.
PATCH 11/14 arm64: dts: Add initial device tree support for EXYNOS7
In reply to Olof JohanssonHi Olof,
Quoted 7 lines · Olof Johansson
Hi,
Quoted 2 lines · Naveen Krishna Chatradhi
Add initial device tree nodes for EXYNOS7 SoC. Also, includes the dt-binding definitions for clock ids.
Uh, no -- it just adds the dtsi.
Ok. Will fix.
Quoted 19 lines
arch/arm64/boot/dts/exynos7.dtsi
Quoted 15 lines
Signed-off-by Naveen Krishna Chatradhi <[email protected]>Cc Thomas Abraham <[email protected]>Cc Rob Herring <[email protected]>Cc Catalin Marinas <[email protected]>--- arch/arm64/boot/dts/exynos7.dtsi | 553 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 553 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7.dtsiarch/arm64/boot/dts/exynos7.dtsi new file +0
-0Let's not make the same mistake as on 32-bit, and go with a directory hierarchy here from day one.
Quoted 2 lines
So, please create a exynos subdirectory for this file. You also need a
Makefilewhen you add a board dts.
Ok. Will fix.
Quoted 18 lines
Quoted 18 lines
arch/arm64/boot/dts/exynos7.dtsi new file +16
-0@@ -0,0 +1,553 @@ +/* + * SAMSUNG EXYNOS7 SoC device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * SAMSUNG EXYNOS7 SoC device nodes are listed in this file. + * EXYNOS7 based board files can include this file and provide + * values for board specfic bindings. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <dt-bindings/clock/exynos7-clk.h>
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi line 22
Quoted 6 lines
arch/arm64/boot/dts/exynos7.dtsi new file +6
-0+ +/ { + compatible = "samsung,exynos7"; + interrupt-parent = <&gic>; + #address-cells = <1>; + #size-cells = <1>;You should probably use address-cells/size-cells 2/2 on a 64-bit platform.
Ok.
Quoted 48 lines
Quoted 14 lines
arch/arm64/boot/dts/exynos7.dtsi new file +13
-0+ aliases { + pinctrl0 = &pinctrl_0; + pinctrl1 = &pinctrl_1; + pinctrl2 = &pinctrl_2; + pinctrl3 = &pinctrl_3; + pinctrl4 = &pinctrl_4; + pinctrl5 = &pinctrl_5; + pinctrl6 = &pinctrl_6; + pinctrl7 = &pinctrl_7; + pinctrl8 = &pinctrl_8; + pinctrl9 = &pinctrl_9; + mshc0 = &mmc_0; + mshc2 = &mmc_2;arch/arm64/boot/dts/exynos7.dtsi line 46
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +10
-0+ }; + + chipid at 10000000 { + compatible = "samsung,exynos4210-chipid"; + reg = <0x10000000 0x100>; + }; + + cpus { + #address-cells = <2>; + #size-cells = <0>;Why size-cells=2? Can you not fit a cpuid in 32 bits?
Quoted 21 lines
arch/arm64/boot/dts/exynos7.dtsi new file +21
-0+ cpu at 0 { + device_type = "cpu"; + compatible = "arm,cortex-a57", "arm,armv8"; + reg = <0x0 0x0>; + }; + }; + + fin_pll: xxti { + compatible = "fixed-clock"; + clock-frequency = <24000000>; + clock-output-names = "fin_pll"; + #clock-cells = <0>; + }; + + gic: interrupt-controller at 11001000 { + compatible = "arm,gic-400"; + #interrupt-cells = <3>; + #address-cells = <0>; + interrupt-controller; + reg = <0x11001000 0x1000>, + <0x11002000 0x1000>,
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi line 74
Quoted 6 lines
arch/arm64/boot/dts/exynos7.dtsi new file +6
-0+ <0x11004000 0x2000>, + <0x11006000 0x2000>; + }; + + hsi2c_0: hsi2c at 13640000 { + compatible = "samsung,exynos7-hsi2c";Is the i2c controller here completely new?
It is almost the same as in Exynos5 but few register bits have been changed. So we have added a new compatible string.
Quoted 2 lines
Also, please use 'i2c' for node name on these nodes.
Ok.
Quoted 21 lines
Quoted 21 lines
arch/arm64/boot/dts/exynos7.dtsi new file +20
-0+ reg = <0x13640000 0x1000>; + interrupts = <0 441 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c0_bus>; + clocks = <&clock_peric0 PCLK_HSI2C0>; + clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_1: hsi2c at 13650000 { + compatible = "samsung,exynos7-hsi2c"; + reg = <0x13650000 0x1000>; + interrupts = <0 442 0>; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-names = "default"; + pinctrl-0 = <&hs_i2c1_bus>; + clocks = <&clock_peric0 PCLK_HSI2C1>;
Quoted 8 lines
arch/arm64/boot/dts/exynos7.dtsi line 99
Quoted 5 lines
arch/arm64/boot/dts/exynos7.dtsi new file +5
-0+ clock-names = "hsi2c"; + status = "disabled"; + }; + + hsi2c_2: hsi2c at 14E60000 {I much prefer lowercase hex in unit addresses (and reg entries) below. I know 32-bit uses uppercase, but let's switch going forward here.
Ok. Will fix.
Quoted 7 lines
arch/arm64/boot/dts/exynos7.dtsi line 230
Quoted 3 lines
arch/arm64/boot/dts/exynos7.dtsi new file +2
-0+ mct at 101C0000 { + compatible = "samsung,exynos4210-mct";Please just do away with MCT here, and use architected timers going forward. There really shouldn't be a need to keep supporting MCT any more -- it's a construct from before arch timers on Cortex-A9.
Ok.
Quoted 5 lines
arch/arm64/boot/dts/exynos7.dtsi line 101
Quoted 3 lines
arch/arm64/boot/dts/exynos7.dtsi new file +2
-0+ mmc_0: mmc at 15740000 { + compatible = "samsung,exynos7-dw-mshc-smu";Is this controller backwards compatible with exynos5 ones?
The dwmmc controller in Exynos7 is not fully backward compatible with Exynos5. Specifically, it requires 64-bit related changes for IDMAC and handling the changes in register offsets.
Quoted 6 lines
arch/arm64/boot/dts/exynos7.dtsi line 408
Quoted 2 lines
arch/arm64/boot/dts/exynos7.dtsi new file +1
-0+ /* The Clock nodes are ordered as per the usermanual. */"The clock"
"user manual"
Ok.
Quoted 11 lines
arch/arm64/boot/dts/exynos7.dtsi line 549
Quoted 10 lines
arch/arm64/boot/dts/exynos7.dtsi new file +9
-0+ timer { + compatible = "arm,armv8-timer"; + interrupts = <1 13 0xff01>, + <1 14 0xff01>, + <1 11 0xff01>, + <1 10 0xff01>; + clock-frequency = <24000000>; + use-clocksource-only; + use-physical-timer;
Quoted 3 lines
These two properties are not standard, and I would expect any 64-bit platform to come with PSCI such that you have a way to initialize the virtual timers.
Ok.
Quoted 3 lines
-Olof
Thanks for your comments.
Signature · 2 lines
Shine bright, (: Nav :)
PATCH 12/14 arm64: dts: add Exynos7 based Espresso board dts file
In reply to Naveen Krishna ChatradhiThis patch adds initial dts file for the Espresso board based on Exynos7 from Samsung.
--- arch/arm64/boot/dts/Makefile | 1 + arch/arm64/boot/dts/exynos7-espresso.dts | 69 ++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7-espresso.dts
arch/arm64/boot/dts/Makefile +1 -0
@@ -1,5 +1,6 @@ dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb +dtb-$(CONFIG_ARCH_EXYNOS7) += exynos7-espresso.dtb targets += dtbs targets += $(dtb-y)
arch/arm64/boot/dts/exynos7-espresso.dts new file +69 -0
69 lines not on this page · raw message
Signature · 1 line
1.7.9.5
PATCH 12/14 arm64: dts: add Exynos7 based Espresso board dts file
Hi Naveen,
Please see my comments inline.
Quoted 2 lines · Naveen Krishna Chatradhi
This patch adds initial dts file for the Espresso board based on Exynos7 from Samsung.
[snip]
arch/arm64/boot/dts/exynos7-espresso.dts line 29
Quoted 1 line
arch/arm64/boot/dts/exynos7-espresso.dts new file +1
-0+ mmc at 15740000 {
Please migrate this dts file to the reference-based syntax.
Quoted 8 lines
arch/arm64/boot/dts/exynos7-espresso.dts new file +8
-0+ status = "okay"; + num-slots = <1>; + broken-cd; + caps2-mmc-hs200-1_8v; + supports-highspeed; + non-removable; + card-detect-delay = <200>; + clock-frequency = <800000000>;
arch/arm64/boot/dts/exynos7-espresso.dts line 47
Quoted 10 lines
arch/arm64/boot/dts/exynos7-espresso.dts new file +10
-0+ samsung,dw-mshc-ciu-div = <3>; + samsung,dw-mshc-sdr-timing = <0 4>; + samsung,dw-mshc-ddr-timing = <0 2>; + pinctrl-names = "default"; + pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_rdqs &sd0_bus1 &sd0_bus4 &sd0_bus8>; + bus-width = <8>; + + slot at 0 { + reg = <0>; + bus-width = <8>;
Why is bus-width repeated?
Best regards,
Tomasz
PATCH 12/14 arm64: dts: add Exynos7 based Espresso board dts file
In reply to Naveen Krishna ChatradhiHi,
Quoted 17 lines · Naveen Krishna Chatradhi
This patch adds initial dts file for the Espresso board based on Exynos7 from Samsung.
Signed-off-by Naveen Krishna Chatradhi <[email protected]>Cc Rob Herring <[email protected]>Cc Catalin Marinas <[email protected]>--- arch/arm64/boot/dts/Makefile | 1 + arch/arm64/boot/dts/exynos7-espresso.dts | 69 ++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7-espresso.dtsarch/arm64/boot/dts/Makefile +0
-0
arch/arm64/boot/dts/Makefile line 3
Quoted 3 lines
arch/arm64/boot/dts/Makefile +1
-0@@ -1,5 +1,6 @@ dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb +dtb-$(CONFIG_ARCH_EXYNOS7) += exynos7-espresso.dtb
As mentioned on the dtsi patch, please create a subdirectory.
Quoted 19 lines
arch/arm64/boot/dts/Makefile +0
-0targets += dtbs targets += $(dtb-y)arch/arm64/boot/dts/exynos7-espresso.dts new file +11
-0@@ -0,0 +1,69 @@ +/* + * SAMSUNG Espresso board device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ +
arch/arm64/boot/dts/exynos7-espresso.dts line 21
Quoted 10 lines
arch/arm64/boot/dts/exynos7-espresso.dts new file +10
-0+/dts-v1/; +#include <dt-bindings/gpio/gpio.h> +#include "exynos7.dtsi" + +/ { + model = "Samsung ESPRESSO board based on EXYNOS7"; + compatible = "samsung,espresso", "samsung,exynos7"; + + chosen { + bootargs = "console=ttySAC2,115200n8 root=/dev/ram0 ramdisk=16384 initrd=0x42000000,16M init=/linuxrc";
Having ramdisk bootargs like these in the upstream dts doesn't make much sense. Configure your bootloader to pass them in instead.
You might want to specify the console through chosen instead, there's been recent support added for that.
Quoted 16 lines
arch/arm64/boot/dts/exynos7-espresso.dts new file +16
-0+ + memory at 40000000 { + device_type = "memory"; + reg = <0x40000000 0xC0000000>; + }; + + mmc at 15740000 { + status = "okay"; + num-slots = <1>; + broken-cd; + caps2-mmc-hs200-1_8v; + supports-highspeed; + non-removable; + card-detect-delay = <200>; + clock-frequency = <800000000>; + samsung,dw-mshc-ciu-div = <3>;
arch/arm64/boot/dts/exynos7-espresso.dts line 48
Quoted 10 lines
arch/arm64/boot/dts/exynos7-espresso.dts new file +10
-0+ samsung,dw-mshc-sdr-timing = <0 4>; + samsung,dw-mshc-ddr-timing = <0 2>; + pinctrl-names = "default"; + pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_rdqs &sd0_bus1 &sd0_bus4 &sd0_bus8>; + bus-width = <8>; + + slot at 0 { + reg = <0>; + bus-width = <8>; + };
I thought the use of slot was deprecated. Why do you have it here?
Also, as mentioned by others elsewhere, please use references instead of duplicating the structure here.
-Olof
PATCH 12/14 arm64: dts: add Exynos7 based Espresso board dts file
Hello Olof,
Quoted 15 lines · Olof Johansson
Hi,
Quoted 12 lines · Naveen Krishna Chatradhi
This patch adds initial dts file for the Espresso board based on Exynos7 from Samsung.
Signed-off-by Naveen Krishna Chatradhi <[email protected]>Cc Rob Herring <[email protected]>Cc Catalin Marinas <[email protected]>--- arch/arm64/boot/dts/Makefile | 1 + arch/arm64/boot/dts/exynos7-espresso.dts | 69 ++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 arch/arm64/boot/dts/exynos7-espresso.dts
Quoted 10 lines
arch/arm64/boot/dts/Makefile line 3
Quoted 8 lines
arch/arm64/boot/dts/Makefile +1
-0@@ -1,5 +1,6 @@ dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb foundation-v8.dtb dtb-$(CONFIG_ARCH_XGENE) += apm-mustang.dtb +dtb-$(CONFIG_ARCH_EXYNOS7) += exynos7-espresso.dtbAs mentioned on the dtsi patch, please create a subdirectory.
Sure, Will make it.
Quoted 25 lines
Quoted 25 lines
arch/arm64/boot/dts/Makefile +0
-0targets += dtbs targets += $(dtb-y)arch/arm64/boot/dts/exynos7-espresso.dts new file +16
-0@@ -0,0 +1,69 @@ +/* + * SAMSUNG Espresso board device tree source + * + * Copyright (c) 2014 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ + +/dts-v1/; +#include <dt-bindings/gpio/gpio.h> +#include "exynos7.dtsi" + +/ {
Quoted 8 lines
arch/arm64/boot/dts/exynos7-espresso.dts line 21
Quoted 5 lines
arch/arm64/boot/dts/exynos7-espresso.dts new file +5
-0+ model = "Samsung ESPRESSO board based on EXYNOS7"; + compatible = "samsung,espresso", "samsung,exynos7"; + + chosen { + bootargs = "console=ttySAC2,115200n8 root=/dev/ram0 ramdisk=16384 initrd=0x42000000,16M init=/linuxrc";Having ramdisk bootargs like these in the upstream dts doesn't make much sense. Configure your bootloader to pass them in instead.
Right, We have tested with ramdisk. So, Left it that way. Will remove this.
Quoted 33 lines
You might want to specify the console through chosen instead, there's been recent support added for that.
Quoted 16 lines
arch/arm64/boot/dts/exynos7-espresso.dts new file +16
-0+ + memory at 40000000 { + device_type = "memory"; + reg = <0x40000000 0xC0000000>; + }; + + mmc at 15740000 { + status = "okay"; + num-slots = <1>; + broken-cd; + caps2-mmc-hs200-1_8v; + supports-highspeed; + non-removable; + card-detect-delay = <200>; + clock-frequency = <800000000>; + samsung,dw-mshc-ciu-div = <3>;arch/arm64/boot/dts/exynos7-espresso.dts line 48
Quoted 10 lines
arch/arm64/boot/dts/exynos7-espresso.dts new file +10
-0+ samsung,dw-mshc-sdr-timing = <0 4>; + samsung,dw-mshc-ddr-timing = <0 2>; + pinctrl-names = "default"; + pinctrl-0 = <&sd0_clk &sd0_cmd &sd0_rdqs &sd0_bus1 &sd0_bus4 &sd0_bus8>; + bus-width = <8>; + + slot at 0 { + reg = <0>; + bus-width = <8>; + };I thought the use of slot was deprecated. Why do you have it here?
Quoted 2 lines
Also, as mentioned by others elsewhere, please use references instead of duplicating the structure here.
Sure, will rebase with the references.
Quoted 3 lines
-Olof
Thanks for the review.
Signature · 2 lines
Thanks & Regards, (: Nav :)
PATCH 13/14 arm64: exynos7: Enable ARMv8 based Exynos7 (SoC) support
In reply to Naveen Krishna ChatradhiFrom: Alim Akhtar <[email protected]>
This patch adds the necessary Kconfig entries to enable support for the ARMv8 based Exynos7 SoC.
--- arch/arm64/Kconfig | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
arch/arm64/Kconfig +17 -0
@@ -134,6 +134,23 @@ source "kernel/Kconfig.freezer" menu "Platform selection" +config ARCH_EXYNOS7 + bool "ARMv8 based SAMSUNG EXYNOS7" + select HAVE_S3C2410_WATCHDOG if WATCHDOG + select CLKSRC_OF + select COMMON_CLK_SAMSUNG + select GPIOLIB + select PINCTRL + select PINCTRL_EXYNOS + select RTC_CLASS + select HAVE_S3C_RTC + select GENERIC_GPIO + select ARCH_REQUIRE_GPIOLIB + select HAVE_CLK + select HAVE_SMP + help + This enables support for SAMSUNG exynos7 SoC family + config ARCH_VEXPRESS bool "ARMv8 software model (Versatile Express)" select ARCH_REQUIRE_GPIOLIB
Signature · 1 line
1.7.9.5
Mark Rutland
reviewerPATCH 13/14 arm64: exynos7: Enable ARMv8 based Exynos7 (SoC) support
Hi,
Quoted 18 lines · Naveen Krishna Chatradhi
From: Alim Akhtar <[email protected]>
This patch adds the necessary
Kconfigentries to enable support for the ARMv8 based Exynos7 SoC.Signed-off-by Alim Akhtar <[email protected]>Signed-off-by Naveen Krishna Chatradhi <[email protected]>Cc Rob Herring <[email protected]>Cc Catalin Marinas <[email protected]>--- arch/arm64/Kconfig | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)arch/arm64/Kconfig +0
-0
arch/arm64/Kconfig source "kernel/Kconfig.freezer" · line 140
Quoted 7 lines
arch/arm64/Kconfig +4
-0@@ -134,6 +134,23 @@ source "kernel/Kconfig.freezer" menu "Platform selection" +config ARCH_EXYNOS7 + bool "ARMv8 based SAMSUNG EXYNOS7" + select HAVE_S3C2410_WATCHDOG if WATCHDOG + select CLKSRC_OF
This seems to be implied by ARM_ARCH_TIMER in the core arm64 Kconfig, so I'm not sure this is necessary.
arch/arm64/Kconfig source "kernel/Kconfig.freezer" · line 142
Quoted 2 lines
arch/arm64/Kconfig +2
-0+ select COMMON_CLK_SAMSUNG + select GPIOLIB
You select ARCH_REQUIRE_GPIOLIB below, so is this necessary?
arch/arm64/Kconfig source "kernel/Kconfig.freezer" · line 149
Quoted 7 lines
arch/arm64/Kconfig +7
-0+ select PINCTRL + select PINCTRL_EXYNOS + select RTC_CLASS + select HAVE_S3C_RTC + select GENERIC_GPIO + select ARCH_REQUIRE_GPIOLIB + select HAVE_CLK
Isn't this selected already through the core arm64 Kconfig?
It looks like we have COMMON_CLK, which selects CLKDEV_LOOKUP, which selects HAVE_CLK.
arch/arm64/Kconfig source "kernel/Kconfig.freezer" · line 150
Quoted 1 line
arch/arm64/Kconfig +1
-0+ select HAVE_SMP
I may have missed something, but I didn't see any SMP support in this series.
Thanks,
Mark.
Catalin Marinas
maintainerPATCH 13/14 arm64: exynos7: Enable ARMv8 based Exynos7 (SoC) support
Quoted 5 lines · Mark Rutland
arch/arm64/Kconfig source "kernel/Kconfig.freezer" · line 150
Quoted 1 line · Naveen Krishna Chatradhi
arch/arm64/Kconfig +1
-0+ select HAVE_SMPI may have missed something, but I didn't see any SMP support in this series.
We don't even have HAVE_SMP defined for arm64 (just SMP, which is on in defconfig).
In theory we don't need any specific SMP support for a new SoC as long as it uses a standard booting protocol (e.g. PSCI). But for whatever reason, I only received about four patches in this series, so can't see the rest.
Signature · 1 line
Catalin
PATCH 13/14 arm64: exynos7: Enable ARMv8 based Exynos7 (SoC) support
Quoted 7 lines · Catalin Marinas
Quoted 5 lines · Mark Rutland
arch/arm64/Kconfig source "kernel/Kconfig.freezer" · line 150
Quoted 1 line · Naveen Krishna Chatradhi
arch/arm64/Kconfig +1
-0+ select HAVE_SMPI may have missed something, but I didn't see any SMP support in this series.
Quoted 7 lines
We don't even have
HAVE_SMPdefined for arm64 (just SMP, which is on in defconfig).In theory we don't need any specific SMP support for a new SoC as long as it uses a standard booting protocol (e.g. PSCI). But for whatever reason, I only received about four patches in this series, so can't see the rest.
Seems like about 9 of them made it to the list for me.
-Olof
PATCH 13/14 arm64: exynos7: Enable ARMv8 based Exynos7 (SoC) support
In reply to Mark RutlandHi Mark,
Quoted 24 lines · Mark Rutland
Hi,
Quoted 21 lines · Naveen Krishna Chatradhi
From: Alim Akhtar <[email protected]>
This patch adds the necessary
Kconfigentries to enable support for the ARMv8 based Exynos7 SoC.Signed-off-by Alim Akhtar <[email protected]>Signed-off-by Naveen Krishna Chatradhi <[email protected]>Cc Rob Herring <[email protected]>Cc Catalin Marinas <[email protected]>--- arch/arm64/Kconfig | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)arch/arm64/Kconfig +0
-0@@ -134,6 +134,23 @@ source "kernel/Kconfig.freezer" menu "Platform selection"
Quoted 7 lines
arch/arm64/Kconfig source "kernel/Kconfig.freezer" · line 140
Quoted 4 lines
arch/arm64/Kconfig +4
-0+config ARCH_EXYNOS7 + bool "ARMv8 based SAMSUNG EXYNOS7" + select HAVE_S3C2410_WATCHDOG if WATCHDOG + select CLKSRC_OFThis seems to be implied by
ARM_ARCH_TIMERin the core arm64Kconfig, so I'm not sure this is necessary.
Ok.
Quoted 5 lines
arch/arm64/Kconfig source "kernel/Kconfig.freezer" · line 142
Quoted 3 lines
arch/arm64/Kconfig +2
-0+ select COMMON_CLK_SAMSUNG + select GPIOLIBYou select
ARCH_REQUIRE_GPIOLIBbelow, so is this necessary?
Yes, this is not required. Will remove.
Quoted 9 lines
arch/arm64/Kconfig source "kernel/Kconfig.freezer" · line 149
Quoted 8 lines
arch/arm64/Kconfig +7
-0+ select PINCTRL + select PINCTRL_EXYNOS + select RTC_CLASS + select HAVE_S3C_RTC + select GENERIC_GPIO + select ARCH_REQUIRE_GPIOLIB + select HAVE_CLK
Quoted 4 lines
Isn't this selected already through the core arm64
Kconfig?It looks like we have
COMMON_CLK, which selectsCLKDEV_LOOKUP, which selectsHAVE_CLK.
Ok.
Quoted 5 lines
arch/arm64/Kconfig source "kernel/Kconfig.freezer" · line 150
Quoted 2 lines
arch/arm64/Kconfig +1
-0+ select HAVE_SMPI may have missed something, but I didn't see any SMP support in this series.
As Catalin suggested, will remove this.
Quoted 3 lines
Thanks,
Mark.
Thanks for your comments.
Signature · 2 lines
Shine bright, (: Nav :)
cover Support 64bit Cortex A57 based Exynos7 SoC
In reply to Naveen Krishna ChatradhiHi Naveen,
Quoted 3 lines · Naveen Krishna Chatradhi
This patchset supports new Exynos7 Samsung SoC based on Cortex-A57. Exynos7 is a System-On-Chip (SoC) that is based on 64-bit ARMv8 RISC processor.
A general issue with this series is that it introduces a very generic name for a very specific single SoC. We remember exactly the same cases for Exynos4 (original name used for Exynos5250) and Exynos5 (Exynos5250) and we should have learned that it is better to use a specific name first and only then try to make things common if it turns out to be appropriate.
Best regards,
Tomasz
cover Support 64bit Cortex A57 based Exynos7 SoC
Quoted 7 lines · Tomasz Figa
Hi Naveen,
Quoted 3 lines · Naveen Krishna Chatradhi
This patchset supports new Exynos7 Samsung SoC based on Cortex-A57. Exynos7 is a System-On-Chip (SoC) that is based on 64-bit ARMv8 RISC processor.
Quoted 6 lines
A general issue with this series is that it introduces a very generic name for a very specific single SoC. We remember exactly the same cases for Exynos4 (original name used for Exynos5250) and Exynos5 (Exynos5250) and we should have learned that it is better to use a specific name first and only then try to make things common if it turns out to be appropriate.
Any comments on this?
cover Support 64bit Cortex A57 based Exynos7 SoC
Hi Tomasz,
Quoted 8 lines · Tomasz Figa
Quoted 7 lines · Tomasz Figa
Hi Naveen,
Quoted 3 lines · Naveen Krishna Chatradhi
This patchset supports new Exynos7 Samsung SoC based on Cortex-A57. Exynos7 is a System-On-Chip (SoC) that is based on 64-bit ARMv8 RISC processor.
Quoted 8 lines
Quoted 6 lines
A general issue with this series is that it introduces a very generic name for a very specific single SoC. We remember exactly the same cases for Exynos4 (original name used for Exynos5250) and Exynos5 (Exynos5250) and we should have learned that it is better to use a specific name first and only then try to make things common if it turns out to be appropriate.
Any comments on this?
The name of the SoC is Exynos7 and so all these patches are using Exynos7 in dt compatible string and file names. If there are any variants of Exynos7 SoC in future, support those variants will be based on what is already merged for Exynos7 SoC. There is no specific generalization targeted in this series by using Exynos7 name.
Thanks,
Thomas.
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
cover Support 64bit Cortex A57 based Exynos7 SoC
In reply to Naveen Krishna ChatradhiNaveen,
Quoted 3 lines · Naveen Krishna Chatradhi
This patchset supports new Exynos7 Samsung SoC based on Cortex-A57. Exynos7 is a System-On-Chip (SoC) that is based on 64-bit ARMv8 RISC processor.
Your emails haven't threaded properly, so the series of patches are not recognized as comments to the envelope (00/14) email. It makes it harder to review them in order, and some mailers might not group them well.
Quoted 3 lines
We tested these patches with the
"arm64: dts: add <dt-bindings/> symlink" change posted @
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-January/224629.html
That means that these patches can't be picked up until a permanent solution for that has gone in.
-Olof