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
--
1.7.9.5
@@ -0,0 +1,553 @@+/*+*SAMSUNGEXYNOS7SoCdevicetreesource+*+*Copyright(c)2014SamsungElectronicsCo.,Ltd.+*http://www.samsung.com+*+*SAMSUNGEXYNOS7SoCdevicenodesarelistedinthisfile.+*EXYNOS7basedboardfilescanincludethisfileandprovide+*valuesforboardspecficbindings.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseversion2as+*publishedbytheFreeSoftwareFoundation.+*/++#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;+};++chipidat10000000{+compatible="samsung,exynos4210-chipid";+reg=<0x100000000x100>;+};++cpus{+#address-cells=<2>;+#size-cells=<0>;++cpuat0{+device_type="cpu";+compatible="arm,cortex-a57","arm,armv8";+reg=<0x00x0>;+};+};++fin_pll:xxti{+compatible="fixed-clock";+clock-frequency=<24000000>;+clock-output-names="fin_pll";+#clock-cells=<0>;+};++gic:interrupt-controllerat11001000{+compatible="arm,gic-400";+#interrupt-cells=<3>;+#address-cells=<0>;+interrupt-controller;+reg=<0x110010000x1000>,+<0x110020000x1000>,+<0x110040000x2000>,+<0x110060000x2000>;+};++hsi2c_0:hsi2cat13640000{+compatible="samsung,exynos7-hsi2c";+reg=<0x136400000x1000>;+interrupts=<04410>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c0_bus>;+clocks=<&clock_peric0PCLK_HSI2C0>;+clock-names="hsi2c";+status="disabled";+};++hsi2c_1:hsi2cat13650000{+compatible="samsung,exynos7-hsi2c";+reg=<0x136500000x1000>;+interrupts=<04420>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c1_bus>;+clocks=<&clock_peric0PCLK_HSI2C1>;+clock-names="hsi2c";+status="disabled";+};++hsi2c_2:hsi2cat14E60000{+compatible="samsung,exynos7-hsi2c";+reg=<0x14E600000x1000>;+interrupts=<04590>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c2_bus>;+clocks=<&clock_peric1PCLK_HSI2C2>;+clock-names="hsi2c";+status="disabled";+};++hsi2c_3:hsi2cat14E70000{+compatible="samsung,exynos7-hsi2c";+reg=<0x14E700000x1000>;+interrupts=<04600>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c3_bus>;+clocks=<&clock_peric1PCLK_HSI2C3>;+clock-names="hsi2c";+status="disabled";+};++hsi2c_4:hsi2cat13660000{+compatible="samsung,exynos7-hsi2c";+reg=<0x136600000x1000>;+interrupts=<04430>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c4_bus>;+clocks=<&clock_peric0PCLK_HSI2C4>;+clock-names="hsi2c";+status="disabled";+};++hsi2c_5:hsi2cat13670000{+compatible="samsung,exynos7-hsi2c";+reg=<0x136700000x1000>;+interrupts=<04440>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c5_bus>;+clocks=<&clock_peric0PCLK_HSI2C5>;+clock-names="hsi2c";+status="disabled";+};++hsi2c_6:hsi2cat14E00000{+compatible="samsung,exynos7-hsi2c";+reg=<0x14E000000x1000>;+interrupts=<04610>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c6_bus>;+clocks=<&clock_peric1PCLK_HSI2C6>;+clock-names="hsi2c";+status="disabled";+};++hsi2c_7:hsi2cat13E10000{+compatible="samsung,exynos7-hsi2c";+reg=<0x13E100000x1000>;+interrupts=<04620>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c7_bus>;+clocks=<&clock_peric1PCLK_HSI2C7>;+clock-names="hsi2c";+status="disabled";+};++hsi2c_8:hsi2cat14E20000{+compatible="samsung,exynos7-hsi2c";+reg=<0x14E200000x1000>;+interrupts=<04630>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c8_bus>;+clocks=<&clock_peric1PCLK_HSI2C8>;+clock-names="hsi2c";+status="disabled";+};++hsi2c_9:hsi2cat13680000{+compatible="samsung,exynos7-hsi2c";+reg=<0x136800000x1000>;+interrupts=<04450>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c9_bus>;+clocks=<&clock_peric0PCLK_HSI2C9>;+clock-names="hsi2c";+status="disabled";+};++hsi2c_10:hsi2cat13690000{+compatible="samsung,exynos7-hsi2c";+reg=<0x136900000x1000>;+interrupts=<04460>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c10_bus>;+clocks=<&clock_peric0PCLK_HSI2C10>;+clock-names="hsi2c";+status="disabled";+};++hsi2c_11:hsi2cat136A0000{+compatible="samsung,exynos7-hsi2c";+reg=<0x136A00000x1000>;+interrupts=<04470>;+#address-cells=<1>;+#size-cells=<0>;+pinctrl-names="default";+pinctrl-0=<&hs_i2c11_bus>;+clocks=<&clock_peric0PCLK_HSI2C11>;+clock-names="hsi2c";+status="disabled";+};++mctat101C0000{+compatible="samsung,exynos4210-mct";+reg=<0x101C00000x800>;+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_perisPCLK_MCT>;+clock-names="fin_pll","mct";++mct_map:mct-map{+#interrupt-cells=<1>;+#address-cells=<0>;+#size-cells=<0>;+interrupt-map=<0&gic01120>,+<1&gic01130>,+<2&gic01140>,+<3&gic01150>,+<4&gic01160>,+<5&gic01170>,+<6&gic01180>,+<7&gic01190>,+<8&gic01200>,+<9&gic01210>,+<10&gic01220>,+<11&gic01230>;+};+};++mmc_0:mmcat15740000{+compatible="samsung,exynos7-dw-mshc-smu";+interrupts=<02010>;+#address-cells=<1>;+#size-cells=<0>;+reg=<0x157400000x2000>;+clocks=<&clock_fsys1ACLK_MMC0>,+<&clock_top1CLK_SCLK_MMC0>;+clock-names="biu","ciu";+fifo-depth=<0x40>;+status="disabled";+};++mmc_2:mmcat15560000{+compatible="samsung,exynos7-dw-mshc-smu";+interrupts=<02160>;+#address-cells=<1>;+#size-cells=<0>;+reg=<0x155600000x2000>;+clocks=<&clock_fsys0ACLK_MMC2>,+<&clock_top1CLK_SCLK_MMC2>;+clock-names="biu","ciu";+fifo-depth=<0x40>;+status="disabled";+};++pinctrl_0:pinctrlat10580000{+compatible="samsung,exynos7-pinctrl";+reg=<0x105800000x1000>;+interrupts=<000>,<010>,<020>,<030>,+<040>,<050>,<060>,<070>,+<080>,<090>,<0100>,<0110>,+<0120>,<0130>,<0140>,<0150>;+wakeup-interrupt-controller{+compatible="samsung,exynos4210-wakeup-eint";+interrupt-parent=<&gic>;+interrupts=<0160>;+};+};++pinctrl_1:pinctrlat114B0000{+compatible="samsung,exynos7-pinctrl";+reg=<0x114B00000x1000>;+interrupts=<0920>;+};++pinctrl_2:pinctrlat13470000{+compatible="samsung,exynos7-pinctrl";+reg=<0x134700000x1000>;+interrupts=<03830>;+};++pinctrl_3:pinctrlat14870000{+compatible="samsung,exynos7-pinctrl";+reg=<0x148700000x1000>;+interrupts=<03840>;+};++pinctrl_4:pinctrlat14CD0000{+compatible="samsung,exynos7-pinctrl";+reg=<0x14CD00000x1000>;+interrupts=<04730>;+};++pinctrl_5:pinctrlat14CE0000{+compatible="samsung,exynos7-pinctrl";+reg=<0x14CE00000x1000>;+interrupts=<04740>;+};++pinctrl_6:pinctrlat14C90000{+compatible="samsung,exynos7-pinctrl";+reg=<0x14C900000x1000>;+interrupts=<04750>;+};++pinctrl_7:pinctrlat14CA0000{+compatible="samsung,exynos7-pinctrl";+reg=<0x14CA00000x1000>;+interrupts=<04760>;+};++pinctrl_8:pinctrlat10E60000{+compatible="samsung,exynos7-pinctrl";+reg=<0x10E600000x1000>;+interrupts=<02210>;+};++pinctrl_9:pinctrlat15690000{+compatible="samsung,exynos7-pinctrl";+reg=<0x156900000x1000>;+interrupts=<02030>;+};++pwm:pwmat136C0000{+compatible="samsung,exynos4210-pwm";+reg=<0x136C00000x100>;+samsung,pwm-outputs=<0>,<1>,<2>,<3>;+#pwm-cells=<3>;+clocks=<&clock_peric0PCLK_PWM>;+clock-names="timers";+status="disabled";+};++rtcat10590000{+compatible="samsung,s3c6410-rtc";+reg=<0x105900000x100>;+interrupts=<03550>,<03560>;+clocks=<&clock_ccorePCLK_RTC>;+clock-names="rtc";+};++serialat13630000{+compatible="samsung,exynos4210-uart";+reg=<0x136300000x100>;+interrupts=<04400>;+clocks=<&clock_peric0PCLK_UART0>,<&clock_peric0SCLK_UART0>;+clock-names="uart","clk_uart_baud0";+status="okay";+};++serialat14C20000{+compatible="samsung,exynos4210-uart";+reg=<0x14C200000x100>;+interrupts=<04560>;+clocks=<&clock_peric1PCLK_UART1>,<&clock_peric1SCLK_UART1>;+clock-names="uart","clk_uart_baud1";+status="okay";+};++serialat14C30000{+compatible="samsung,exynos4210-uart";+reg=<0x14C300000x100>;+interrupts=<04570>;+clocks=<&clock_peric1PCLK_UART2>,<&clock_peric1SCLK_UART2>;+clock-names="uart","clk_uart_baud2";+status="okay";+};++serialat14C40000{+compatible="samsung,exynos4210-uart";+reg=<0x14C400000x100>;+interrupts=<04580>;+clocks=<&clock_peric1PCLK_UART3>,<&clock_peric1SCLK_UART3>;+clock-names="uart","clk_uart_baud3";+status="okay";+};++/* The Clock nodes are ordered as per the usermanual. */+clock_topc:clock-controllerat10570000{+compatible="samsung,exynos7-clock-topc";+reg=<0x105700000x10000>;+#clock-cells=<1>;+};++clock_top0:clock-controllerat105D0000{+compatible="samsung,exynos7-clock-top0";+reg=<0x105D00000xB000>;+#clock-cells=<1>;+};++clock_top1:clock-controllerat105E0000{+compatible="samsung,exynos7-clock-top1";+reg=<0x105E00000xB000>;+#clock-cells=<1>;+};++clock_atlas:clock-controllerat11800000{+compatible="samsung,exynos7-clock-atlas";+reg=<0x118000000x1100>;+#clock-cells=<1>;+};++clock_g3d:clock-controllerat14AA0000{+compatible="samsung,exynos7-clock-g3d";+reg=<0x14AA00000x1100>;+#clock-cells=<1>;+};++clock_mif0:clock-controllerat10850000{+compatible="samsung,exynos7-clock-mif0";+reg=<0x108500000x1100>;+#clock-cells=<1>;+};++clock_mif1:clock-controllerat10950000{+compatible="samsung,exynos7-clock-mif1";+reg=<0x109500000x1100>;+#clock-cells=<1>;+};++clock_mif2:clock-controllerat10A50000{+compatible="samsung,exynos7-clock-mif2";+reg=<0x10A500000x1100>;+#clock-cells=<1>;+};++clock_mif3:clock-controllerat10B50000{+compatible="samsung,exynos7-clock-mif3";+reg=<0x10B500000x1100>;+#clock-cells=<1>;+};++clock_ccore:clock-controllerat105B0000{+compatible="samsung,exynos7-clock-ccore";+reg=<0x105B00000xD00>;+#clock-cells=<1>;+};++clock_imem:clock-controllerat11060000{+compatible="samsung,exynos7-clock-imem";+reg=<0x110600000xD00>;+#clock-cells=<1>;+};++clock_peric0:clock-controllerat13610000{+compatible="samsung,exynos7-clock-peric0";+reg=<0x136100000xD00>;+#clock-cells=<1>;+};++clock_peric1:clock-controllerat14C80000{+compatible="samsung,exynos7-clock-peric1";+reg=<0x14C800000xD00>;+#clock-cells=<1>;+};++clock_peris:clock-controllerat10040000{+compatible="samsung,exynos7-clock-peris";+reg=<0x100400000xD00>;+#clock-cells=<1>;+};++clock_bus0:clock-controllerat13400000{+compatible="samsung,exynos7-clock-bus0";+reg=<0x134000000xD00>;+#clock-cells=<1>;+};++clock_bus1:clock-controllerat14800000{+compatible="samsung,exynos7-clock-bus1";+reg=<0x148000000xD00>;+#clock-cells=<1>;+};++clock_disp:clock-controllerat13AD0000{+compatible="samsung,exynos7-clock-disp";+reg=<0x13AD00000xD00>;+#clock-cells=<1>;+};++clock_aud:clock-controllerat114C0000{+compatible="samsung,exynos7-clock-aud";+reg=<0x114C00000xD00>;+#clock-cells=<1>;+};++clock_fsys0:clock-controllerat10E90000{+compatible="samsung,exynos7-clock-fsys0";+reg=<0x10E900000xD00>;+#clock-cells=<1>;+};++clock_fsys1:clock-controllerat156E0000{+compatible="samsung,exynos7-clock-fsys1";+reg=<0x156E00000xD00>;+#clock-cells=<1>;+};++clock_mscl:clock-controllerat150D0000{+compatible="samsung,exynos7-clock-mscl";+reg=<0x150D00000xD00>;+#clock-cells=<1>;+};++clock_mfc:clock-controllerat15280000{+compatible="samsung,exynos7-clock-mfc";+reg=<0x152800000xD00>;+#clock-cells=<1>;+};++timer{+compatible="arm,armv8-timer";+interrupts=<1130xff01>,+<1140xff01>,+<1110xff01>,+<1100xff01>;+clock-frequency=<24000000>;+use-clocksource-only;+use-physical-timer;+};+};++#include"exynos7-pinctrl.dtsi"
From: Alim Akhtar <alim.akhtar@samsung.com>
This patch adds the necessary Kconfig entries to enable
support for the ARMv8 based Exynos7 SoC.
Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Naveen Krishna Chatradhi <redacted>
Cc: Rob Herring <robh@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/Kconfig | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
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.
[...]
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.
+ 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.
From: Mark Rutland <mark.rutland@arm.com> Date: 2014-08-27 11:09:28
Hi,
On Wed, Aug 27, 2014 at 10:44:20AM +0100, Naveen Krishna Chatradhi wrote:
quoted hunk
From: Alim Akhtar <alim.akhtar@samsung.com>
This patch adds the necessary Kconfig entries to enable
support for the ARMv8 based Exynos7 SoC.
Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Naveen Krishna Chatradhi <redacted>
Cc: Rob Herring <robh@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/Kconfig | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
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]
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).
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.
From: Tomasz Figa <hidden> Date: 2014-08-27 11:34:50
Hi Naveen,
On 27.08.2014 11:44, Naveen Krishna Chatradhi wrote:
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
On Wed, Aug 27, 2014 at 12:09:28PM +0100, Mark Rutland wrote:
On Wed, Aug 27, 2014 at 10:44:20AM +0100, Naveen Krishna Chatradhi wrote:
quoted
+ select HAVE_SMP
I 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.
--
Catalin
From: Olof Johansson <hidden> Date: 2014-08-28 03:47:58
Naveen,
On Wed, Aug 27, 2014 at 03:14:16PM +0530, Naveen Krishna Chatradhi wrote:
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.
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 hunk
@@ -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>;
You should probably use address-cells/size-cells 2/2 on a 64-bit platform.
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.
+ 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.
+ mmc_0: mmc at 15740000 {
+ compatible = "samsung,exynos7-dw-mshc-smu";
Is this controller backwards compatible with exynos5 ones?
+ /* The Clock nodes are ordered as per the usermanual. */
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
@@ -0,0 +1,69 @@+/*+*SAMSUNGEspressoboarddevicetreesource+*+*Copyright(c)2014SamsungElectronicsCo.,Ltd.+*http://www.samsung.com+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseversion2as+*publishedbytheFreeSoftwareFoundation.+*/++/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.
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
From: Olof Johansson <hidden> Date: 2014-08-28 04:05:17
On Wed, Aug 27, 2014 at 03:50:21PM +0100, Catalin Marinas wrote:
On Wed, Aug 27, 2014 at 12:09:28PM +0100, Mark Rutland wrote:
quoted
On Wed, Aug 27, 2014 at 10:44:20AM +0100, Naveen Krishna Chatradhi wrote:
quoted
+ select HAVE_SMP
I 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.
Seems like about 9 of them made it to the list for me.
-Olof
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
@@ -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>;
You should probably use address-cells/size-cells 2/2 on a 64-bit platform.
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.
quoted
+ 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.
quoted
+ mmc_0: mmc at 15740000 {
+ compatible = "samsung,exynos7-dw-mshc-smu";
Is this controller backwards compatible with exynos5 ones?
quoted
+ /* The Clock nodes are ordered as per the usermanual. */
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.
--
Jazz is not dead. It just smells funny...
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
+ 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.
[...]
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.
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
quoted
+ 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.
[...]
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
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.
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
[...]
quoted
quoted
+ 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.
[...]
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. :)
True.
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----
From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
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.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <redacted>
---
Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
@@ -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.
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.
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
quoted
[...]
quoted
quoted
+ 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.
[...]
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. :)
True.
quoted
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----
From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
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.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <redacted>
With caps fixed:
Acked-by: Olof Johansson <redacted>
@@ -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. :)
+ 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 hunk
- 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.--
From: Marc Zyngier <hidden> Date: 2014-08-28 17:27:04
On 28/08/14 18:03, Mark Rutland wrote:
From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
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
I believe Xen is also affected by this.
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 <mark.rutland@arm.com>
Cc: Marc Zyngier <redacted>
Short of more explicit threats:
Acked-by: Marc Zyngier <redacted>
@@ -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.
From: Mark Rutland <mark.rutland@arm.com> Date: 2014-08-28 17:30:37
On Thu, Aug 28, 2014 at 06:27:04PM +0100, Marc Zyngier wrote:
On 28/08/14 18:03, Mark Rutland wrote:
quoted
From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
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
I believe Xen is also affected by this.
True.
s/KVM/KVM\/Xen/, then?
quoted
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 <mark.rutland@arm.com>
Cc: Marc Zyngier <redacted>
Short of more explicit threats:
Acked-by: Marc Zyngier <redacted>
@@ -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.
From: Marc Zyngier <hidden> Date: 2014-08-28 17:37:19
On 28/08/14 18:30, Mark Rutland wrote:
On Thu, Aug 28, 2014 at 06:27:04PM +0100, Marc Zyngier wrote:
quoted
On 28/08/14 18:03, Mark Rutland wrote:
quoted
From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
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
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.
--
Jazz is not dead. It just smells funny...
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.
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.
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
quoted
quoted
[...]
quoted
quoted
+ 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.
[...]
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. :)
True.
quoted
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----
From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
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.
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <redacted>
With caps fixed:
Acked-by: Olof Johansson <redacted>
@@ -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. :)
Sure. I'll save the caps for replies to violators ;)
Mark.
quoted
+ 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
- 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.--
From: Mark Rutland <mark.rutland@arm.com> Date: 2014-08-28 17:45:03
On Thu, Aug 28, 2014 at 06:37:19PM +0100, Marc Zyngier wrote:
On 28/08/14 18:30, Mark Rutland wrote:
quoted
On Thu, Aug 28, 2014 at 06:27:04PM +0100, Marc Zyngier wrote:
quoted
On 28/08/14 18:03, Mark Rutland wrote:
quoted
From 67104ad5a56e4c18f9c41f06af028b7561740afd Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
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
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.
I expect this to be such a fundamental problem that all hypervisors
will trip over on that one (Jailhouse definitely does).
Hi Mark,
On Thu, Aug 28, 2014 at 7:39 PM, Mark Rutland [off-list ref] wrote:
quoted
quoted
quoted
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?
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.
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
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.
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.
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
From: Mark Rutland <mark.rutland@arm.com> Date: 2014-08-28 18:17:28
On Thu, Aug 28, 2014 at 06:47:00PM +0100, Geert Uytterhoeven wrote:
Hi Mark,
On Thu, Aug 28, 2014 at 7:39 PM, Mark Rutland [off-list ref] wrote:
quoted
quoted
quoted
quoted
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?
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.
This is unfortunate. I guess this was assumed to be implied by way of
the examples. :/
quoted
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.
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.
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.
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
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.
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.
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. :)
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 <redacted>
[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.
Reported-by: NAOYA SHIIBA <redacted>
Signed-off-by: Simon Horman <redacted>
---
arch/arm/boot/dts/r8a7790-lager.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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.
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.
Sure, will rebase with the references.
-Olof
Thanks for the review.
--
Thanks & Regards,
(: Nav :)
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.
Ok, this is fixed with #address-cells = 2 and #size-cells = 2.
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.
quoted
+ 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?
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).
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.
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.
Ok. Will fix.
quoted
@@ -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>;
You should probably use address-cells/size-cells 2/2 on a 64-bit platform.
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
+ 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
+ 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
+ /* The Clock nodes are ordered as per the usermanual. */
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.
-Olof
Thanks for your comments.
--
Shine bright,
(: Nav :)
Hi Mark,
On 27 August 2014 16:39, Mark Rutland [off-list ref] wrote:
Hi,
On Wed, Aug 27, 2014 at 10:44:20AM +0100, Naveen Krishna Chatradhi wrote:
quoted
From: Alim Akhtar <alim.akhtar@samsung.com>
This patch adds the necessary Kconfig entries to enable
support for the ARMv8 based Exynos7 SoC.
Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Naveen Krishna Chatradhi <redacted>
Cc: Rob Herring <robh@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm64/Kconfig | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
From: Tomasz Figa <hidden> Date: 2014-09-13 10:57:40
On 27.08.2014 13:34, Tomasz Figa wrote:
Hi Naveen,
On 27.08.2014 11:44, Naveen Krishna Chatradhi wrote:
quoted
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.
From: Thomas Abraham <hidden> Date: 2014-09-14 13:45:14
Hi Tomasz,
On Sat, Sep 13, 2014 at 4:27 PM, Tomasz Figa [off-list ref] wrote:
On 27.08.2014 13:34, Tomasz Figa wrote:
quoted
Hi Naveen,
On 27.08.2014 11:44, Naveen Krishna Chatradhi wrote:
quoted
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.
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.
--
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