Hi,
here's the complete series now. One fix for cpufreq from our vendor tree
and migration to pre-processor syntax.
Thanks,
Sören
Changes in v2:
- Send full series (added patches 1, 4, 5)
Soren Brinkmann (5):
ARM: zynq: DT: Add 'clock-latency' property
ARM: zynq: dt: Convert to preprocessor includes
ARM: zynq: dt: Use #defines for interrupt specifiers
dt-bindings: clock: Add Zynq-7000 header
ARM: zynq: dt: Use #defines for clock specifiers
arch/arm/boot/dts/zynq-7000.dtsi | 64 ++++++++++++++++++++++-------------
arch/arm/boot/dts/zynq-zc702.dts | 2 +-
arch/arm/boot/dts/zynq-zc706.dts | 2 +-
arch/arm/boot/dts/zynq-zed.dts | 2 +-
include/dt-bindings/clock/zynq-7000.h | 64 +++++++++++++++++++++++++++++++++++
5 files changed, 107 insertions(+), 27 deletions(-)
create mode 100644 include/dt-bindings/clock/zynq-7000.h
--
1.9.1.1.gbb9f595
Add header file with symbolic names for Zynq's clocks.
Signed-off-by: Soren Brinkmann <[email protected]>
---
Changes in v2:
- this patch has been added
---
include/dt-bindings/clock/zynq-7000.h | 64 +++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
create mode 100644 include/dt-bindings/clock/zynq-7000.h
diff --git a/include/dt-bindings/clock/zynq-7000.h b/include/dt-bindings/clock/zynq-7000.h
new file mode 100644
index 000000000000..851f5cffe481
--- /dev/null
+++ b/include/dt-bindings/clock/zynq-7000.h
@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2014 Xilinx Inc.
+ * Author: Sören Brinkmann <[email protected]>
+ *
+ * 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.
+ *
+ * Device Tree binding constants for Zynq 7000 clock controller.
+*/
+
+#ifndef _DT_BINDINGS_CLOCK_ZYNQ_7000_H
+#define _DT_BINDINGS_CLOCK_ZYNQ_7000_H
+
+#define ZYNQ_CLK_ARMPLL 0
+#define ZYNQ_CLK_DDRPLL 1
+#define ZYNQ_CLK_IOPLL 2
+#define ZYNQ_CLK_CPU_6OR4X 3
+#define ZYNQ_CLK_CPU_3OR2X 4
+#define ZYNQ_CLK_CPU_2X 5
+#define ZYNQ_CLK_CPU_1X 6
+#define ZYNQ_CLK_DDR2X 7
+#define ZYNQ_CLK_DDR3X 8
+#define ZYNQ_CLK_DCI 9
+#define ZYNQ_CLK_LQSPI 10
+#define ZYNQ_CLK_SMC 11
+#define ZYNQ_CLK_PCAP 12
+#define ZYNQ_CLK_GEM0 13
+#define ZYNQ_CLK_GEM1 14
+#define ZYNQ_CLK_FCLK0 15
+#define ZYNQ_CLK_FCLK1 16
+#define ZYNQ_CLK_FCLK2 17
+#define ZYNQ_CLK_FCLK3 18
+#define ZYNQ_CLK_CAN0 19
+#define ZYNQ_CLK_CAN1 20
+#define ZYNQ_CLK_SDIO0 21
+#define ZYNQ_CLK_SDIO1 22
+#define ZYNQ_CLK_UART0 23
+#define ZYNQ_CLK_UART1 24
+#define ZYNQ_CLK_SPI0 25
+#define ZYNQ_CLK_SPI1 26
+#define ZYNQ_CLK_DMA 27
+#define ZYNQ_CLK_USB0_APER 28
+#define ZYNQ_CLK_USB1_APER 29
+#define ZYNQ_CLK_GEM0_APER 30
+#define ZYNQ_CLK_GEM1_APER 31
+#define ZYNQ_CLK_SDIO0_APER 32
+#define ZYNQ_CLK_SDIO1_APER 33
+#define ZYNQ_CLK_SPI0_APER 34
+#define ZYNQ_CLK_SPI1_APER 35
+#define ZYNQ_CLK_CAN0_APER 36
+#define ZYNQ_CLK_CAN1_APER 37
+#define ZYNQ_CLK_I2C0_APER 38
+#define ZYNQ_CLK_I2C1_APER 39
+#define ZYNQ_CLK_UART0_APER 40
+#define ZYNQ_CLK_UART1_APER 41
+#define ZYNQ_CLK_GPIO_APER 42
+#define ZYNQ_CLK_LQSPI_APER 43
+#define ZYNQ_CLK_SMC_APER 44
+#define ZYNQ_CLK_SWDT 45
+#define ZYNQ_CLK_DBG_TRC 46
+#define ZYNQ_CLK_DBG_APB 47
+
+#endif /* _DT_BINDINGS_CLOCK_ZYNQ_7000_H */
--
1.9.1.1.gbb9f595
Specify the 'clock-latency' property to avoid certain cpufreq governors
from refusing to work with the following error:
ondemand governor failed, too long transition latency of HW, fallback to performance governor
Reported-by: Mike Looijmans <[email protected]>
Signed-off-by: Soren Brinkmann <[email protected]>
Tested-by: Mike Looijmans <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
---
This is a fix from our vendor tree
Changes in v2:
- This patch has been added
---
arch/arm/boot/dts/zynq-7000.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 789d0bacc110..20a13cba65a3 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -24,6 +24,7 @@
device_type = "cpu";
reg = <0>;
clocks = <&clkc 3>;
+ clock-latency = <1000>;
operating-points = <
/* kHz uV */
666667 1000000
--
1.9.1.1.gbb9f595
Use symbolic names instead of bare numbers to specify clocks.
Signed-off-by: Soren Brinkmann <[email protected]>
---
Changes in v2:
- this patch has been added
---
arch/arm/boot/dts/zynq-7000.dtsi | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 36a34ffa30af..9a54b49d0fd2 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -13,6 +13,7 @@
#include "skeleton.dtsi"
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/zynq-7000.h>
/ {
compatible = "xlnx,zynq-7000";
@@ -25,8 +26,8 @@
compatible = "arm,cortex-a9";
device_type = "cpu";
reg = <0>;
- clocks = <&clkc 3>;
clock-latency = <1000>;
+ clocks = <&clkc ZYNQ_CLK_CPU_6OR4X>;
operating-points = <
/* kHz uV */
666667 1000000
@@ -39,7 +40,7 @@
compatible = "arm,cortex-a9";
device_type = "cpu";
reg = <1>;
- clocks = <&clkc 3>;
+ clocks = <&clkc ZYNQ_CLK_CPU_6OR4X>;
};
};
@@ -78,7 +79,8 @@
uart0: uart@e0000000 {
compatible = "xlnx,xuartps";
status = "disabled";
- clocks = <&clkc 23>, <&clkc 40>;
+ clocks = <&clkc ZYNQ_CLK_UART0>,
+ <&clkc ZYNQ_CLK_UART0_APER>;
clock-names = "ref_clk", "aper_clk";
reg = <0xE0000000 0x1000>;
interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
@@ -87,7 +89,8 @@
uart1: uart@e0001000 {
compatible = "xlnx,xuartps";
status = "disabled";
- clocks = <&clkc 24>, <&clkc 41>;
+ clocks = <&clkc ZYNQ_CLK_UART1>,
+ <&clkc ZYNQ_CLK_UART1_APER>;
clock-names = "ref_clk", "aper_clk";
reg = <0xE0001000 0x1000>;
interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
@@ -98,7 +101,9 @@
reg = <0xe000b000 0x4000>;
status = "disabled";
interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&clkc 30>, <&clkc 30>, <&clkc 13>;
+ clocks = <&clkc ZYNQ_CLK_GEM0_APER>,
+ <&clkc ZYNQ_CLK_GEM0_APER>,
+ <&clkc ZYNQ_CLK_GEM0>;
clock-names = "pclk", "hclk", "tx_clk";
};
@@ -107,7 +112,9 @@
reg = <0xe000c000 0x4000>;
status = "disabled";
interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&clkc 31>, <&clkc 31>, <&clkc 14>;
+ clocks = <&clkc ZYNQ_CLK_GEM1_APER>,
+ <&clkc ZYNQ_CLK_GEM1_APER>,
+ <&clkc ZYNQ_CLK_GEM1>;
clock-names = "pclk", "hclk", "tx_clk";
};
@@ -115,7 +122,8 @@
compatible = "arasan,sdhci-8.9a";
status = "disabled";
clock-names = "clk_xin", "clk_ahb";
- clocks = <&clkc 21>, <&clkc 32>;
+ clocks = <&clkc ZYNQ_CLK_SDIO0>,
+ <&clkc ZYNQ_CLK_SDIO0_APER>;
interrupt-parent = <&intc>;
interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
reg = <0xe0100000 0x1000>;
@@ -125,7 +133,8 @@
compatible = "arasan,sdhci-8.9a";
status = "disabled";
clock-names = "clk_xin", "clk_ahb";
- clocks = <&clkc 22>, <&clkc 33>;
+ clocks = <&clkc ZYNQ_CLK_SDIO1>,
+ <&clkc ZYNQ_CLK_SDIO1_APER>;
interrupt-parent = <&intc>;
interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
reg = <0xe0101000 0x1000>;
@@ -163,7 +172,7 @@
reg = <0xf8f00200 0x20>;
interrupts = <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_EDGE_RISING)>;
interrupt-parent = <&intc>;
- clocks = <&clkc 4>;
+ clocks = <&clkc ZYNQ_CLK_CPU_3OR2X>;
};
ttc0: ttc0@f8001000 {
@@ -172,7 +181,7 @@
<GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
compatible = "cdns,ttc";
- clocks = <&clkc 6>;
+ clocks = <&clkc ZYNQ_CLK_CPU_1X>;
reg = <0xF8001000 0x1000>;
};
@@ -182,7 +191,7 @@
<GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
compatible = "cdns,ttc";
- clocks = <&clkc 6>;
+ clocks = <&clkc ZYNQ_CLK_CPU_1X>;
reg = <0xF8002000 0x1000>;
};
scutimer: scutimer@f8f00600 {
@@ -190,7 +199,7 @@
interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_EDGE_RISING)>;
compatible = "arm,cortex-a9-twd-timer";
reg = < 0xf8f00600 0x20 >;
- clocks = <&clkc 4>;
+ clocks = <&clkc ZYNQ_CLK_CPU_3OR2X>;
} ;
};
};
--
1.9.1.1.gbb9f595
Use #defines from the common include file to describe
interrupt specifiers.
Signed-off-by: Soren Brinkmann <[email protected]>
---
Changes in v2: None
arch/arm/boot/dts/zynq-7000.dtsi | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 66613d04de5d..36a34ffa30af 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -12,6 +12,7 @@
*/
#include "skeleton.dtsi"
+#include <dt-bindings/interrupt-controller/arm-gic.h>
/ {
compatible = "xlnx,zynq-7000";
@@ -44,7 +45,7 @@
pmu {
compatible = "arm,cortex-a9-pmu";
- interrupts = <0 5 4>, <0 6 4>;
+ interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
interrupt-parent = <&intc>;
reg = < 0xf8891000 0x1000 0xf8893000 0x1000 >;
};
@@ -80,7 +81,7 @@
clocks = <&clkc 23>, <&clkc 40>;
clock-names = "ref_clk", "aper_clk";
reg = <0xE0000000 0x1000>;
- interrupts = <0 27 4>;
+ interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
};
uart1: uart@e0001000 {
@@ -89,14 +90,14 @@
clocks = <&clkc 24>, <&clkc 41>;
clock-names = "ref_clk", "aper_clk";
reg = <0xE0001000 0x1000>;
- interrupts = <0 50 4>;
+ interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
};
gem0: ethernet@e000b000 {
compatible = "cdns,gem";
reg = <0xe000b000 0x4000>;
status = "disabled";
- interrupts = <0 22 4>;
+ interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clkc 30>, <&clkc 30>, <&clkc 13>;
clock-names = "pclk", "hclk", "tx_clk";
};
@@ -105,7 +106,7 @@
compatible = "cdns,gem";
reg = <0xe000c000 0x4000>;
status = "disabled";
- interrupts = <0 45 4>;
+ interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clkc 31>, <&clkc 31>, <&clkc 14>;
clock-names = "pclk", "hclk", "tx_clk";
};
@@ -116,7 +117,7 @@
clock-names = "clk_xin", "clk_ahb";
clocks = <&clkc 21>, <&clkc 32>;
interrupt-parent = <&intc>;
- interrupts = <0 24 4>;
+ interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
reg = <0xe0100000 0x1000>;
} ;
@@ -126,7 +127,7 @@
clock-names = "clk_xin", "clk_ahb";
clocks = <&clkc 22>, <&clkc 33>;
interrupt-parent = <&intc>;
- interrupts = <0 47 4>;
+ interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
reg = <0xe0101000 0x1000>;
} ;
@@ -160,14 +161,16 @@
global_timer: timer@f8f00200 {
compatible = "arm,cortex-a9-global-timer";
reg = <0xf8f00200 0x20>;
- interrupts = <1 11 0x301>;
+ interrupts = <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_EDGE_RISING)>;
interrupt-parent = <&intc>;
clocks = <&clkc 4>;
};
ttc0: ttc0@f8001000 {
interrupt-parent = <&intc>;
- interrupts = < 0 10 4 0 11 4 0 12 4 >;
+ interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
compatible = "cdns,ttc";
clocks = <&clkc 6>;
reg = <0xF8001000 0x1000>;
@@ -175,14 +178,16 @@
ttc1: ttc1@f8002000 {
interrupt-parent = <&intc>;
- interrupts = < 0 37 4 0 38 4 0 39 4 >;
+ interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
compatible = "cdns,ttc";
clocks = <&clkc 6>;
reg = <0xF8002000 0x1000>;
};
scutimer: scutimer@f8f00600 {
interrupt-parent = <&intc>;
- interrupts = < 1 13 0x301 >;
+ interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_EDGE_RISING)>;
compatible = "arm,cortex-a9-twd-timer";
reg = < 0xf8f00600 0x20 >;
clocks = <&clkc 4>;
--
1.9.1.1.gbb9f595
Convert all Zynq DT files to the dtc preprocessor include syntax.
This allows to include header files in the devicetrees like other
SoC-types already do.
Inspired-by: Steffen Trumtrar <[email protected]>
(http://www.spinics.net/lists/arm-kernel/msg319832.html)
Signed-off-by: Soren Brinkmann <[email protected]>
---
Changes in v2: None
arch/arm/boot/dts/zynq-7000.dtsi | 3 ++-
arch/arm/boot/dts/zynq-zc702.dts | 2 +-
arch/arm/boot/dts/zynq-zc706.dts | 2 +-
arch/arm/boot/dts/zynq-zed.dts | 2 +-
4 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 20a13cba65a3..66613d04de5d 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -10,7 +10,8 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
-/include/ "skeleton.dtsi"
+
+#include "skeleton.dtsi"
/ {
compatible = "xlnx,zynq-7000";
diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts
index c913f77a21eb..07713d3c716a 100644
--- a/arch/arm/boot/dts/zynq-zc702.dts
+++ b/arch/arm/boot/dts/zynq-zc702.dts
@@ -12,7 +12,7 @@
* GNU General Public License for more details.
*/
/dts-v1/;
-/include/ "zynq-7000.dtsi"
+#include "zynq-7000.dtsi"
/ {
model = "Zynq ZC702 Development Board";
diff --git a/arch/arm/boot/dts/zynq-zc706.dts b/arch/arm/boot/dts/zynq-zc706.dts
index 88f62c50382e..cf6566cdb02a 100644
--- a/arch/arm/boot/dts/zynq-zc706.dts
+++ b/arch/arm/boot/dts/zynq-zc706.dts
@@ -13,7 +13,7 @@
* GNU General Public License for more details.
*/
/dts-v1/;
-/include/ "zynq-7000.dtsi"
+#include "zynq-7000.dtsi"
/ {
model = "Zynq ZC706 Development Board";
diff --git a/arch/arm/boot/dts/zynq-zed.dts b/arch/arm/boot/dts/zynq-zed.dts
index 82d7ef1a9a9c..1541716e2cfb 100644
--- a/arch/arm/boot/dts/zynq-zed.dts
+++ b/arch/arm/boot/dts/zynq-zed.dts
@@ -13,7 +13,7 @@
* GNU General Public License for more details.
*/
/dts-v1/;
-/include/ "zynq-7000.dtsi"
+#include "zynq-7000.dtsi"
/ {
model = "Zynq Zed Development Board";
--
1.9.1.1.gbb9f595
On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
> Specify the 'clock-latency' property to avoid certain cpufreq governors
> from refusing to work with the following error:
> ondemand governor failed, too long transition latency of HW, fallback to performance governor
>
> Reported-by: Mike Looijmans <[email protected]>
> Signed-off-by: Soren Brinkmann <[email protected]>
> Tested-by: Mike Looijmans <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>
> ---
> This is a fix from our vendor tree
>
> Changes in v2:
> - This patch has been added
>
> ---
> arch/arm/boot/dts/zynq-7000.dtsi | 1 +
> 1 file changed, 1 insertion(+)
Applied to zynq-dt.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
Hi Soren,
On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
> Convert all Zynq DT files to the dtc preprocessor include syntax.
> This allows to include header files in the devicetrees like other
> SoC-types already do.
>
> Inspired-by: Steffen Trumtrar <[email protected]>
> (http://www.spinics.net/lists/arm-kernel/msg319832.html)
>
> Signed-off-by: Soren Brinkmann <[email protected]>
These 4 patches needs more wider discussion if this is helpful or
not. Currently I can't see any value in it because everything
is just generated and fixed. I think I had the same discussion
with Laurent some weeks ago regarding this.
IRC the origin idea to use this was especially for people who
writing these DTS by hand which is not our case - at least
for majority of our customers.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
On 04/07/2014 07:58 AM, Michal Simek wrote:
> Hi Soren,
>
> On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
>> Convert all Zynq DT files to the dtc preprocessor include syntax.
>> This allows to include header files in the devicetrees like other
>> SoC-types already do.
>>
>> Inspired-by: Steffen Trumtrar <[email protected]>
>> (http://www.spinics.net/lists/arm-kernel/msg319832.html)
>>
>> Signed-off-by: Soren Brinkmann <[email protected]>
>
> These 4 patches needs more wider discussion if this is helpful or
> not. Currently I can't see any value in it because everything
> is just generated and fixed. I think I had the same discussion
> with Laurent some weeks ago regarding this.
I would be kinda neutral here. I'd consider it helpful, it improves
readability (regardless of whether they are generated or hand crafted). That's
convenient for things like interrupt sensitivity, I can't remember whether 4
is level or edge type. On the other hand, the clock indices are just as much
magic numbers as the memory addresses. If I suspect an error in that area, I'd
start by lokking in /sys/kernel/debug/clk but wouldn't start in the DT.
> IRC the origin idea to use this was especially for people who
> writing these DTS by hand which is not our case - at least
> for majority of our customers.
I write them by hand. Is there any other way?
Mike.
Met vriendelijke groet / kind regards,
Mike Looijmans
TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) (0) 499 33 69 79
Telefax: (+31) (0) 499 33 69 70
E-mail: [email protected]
Website: http://www.topic.nl
Please consider the environment before printing this e-mail
Visit us at the Hannover Messe 7 - 11 April 2014 - Hall 002/D10 (Dutch Pavillion)
http://www.hannovermesse.de/exhibitor/topic-embedded-products/V229623
Hi Mike,
On 04/07/2014 02:17 PM, Mike Looijmans wrote:
> On 04/07/2014 07:58 AM, Michal Simek wrote:
>> Hi Soren,
>>
>> On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
>>> Convert all Zynq DT files to the dtc preprocessor include syntax.
>>> This allows to include header files in the devicetrees like other
>>> SoC-types already do.
>>>
>>> Inspired-by: Steffen Trumtrar <[email protected]>
>>> (http://www.spinics.net/lists/arm-kernel/msg319832.html)
>>>
>>> Signed-off-by: Soren Brinkmann <[email protected]>
>>
>> These 4 patches needs more wider discussion if this is helpful or
>> not. Currently I can't see any value in it because everything
>> is just generated and fixed. I think I had the same discussion
>> with Laurent some weeks ago regarding this.
>
> I would be kinda neutral here. I'd consider it helpful, it improves readability (regardless of whether they
are generated or hand crafted). That's convenient for things like interrupt sensitivity,
I can't remember whether 4 is level or edge type. On the other hand, the clock indices are just
as much magic numbers as the memory addresses. If I suspect an error in that area, I'd start by lokking
in /sys/kernel/debug/clk but wouldn't start in the DT.
>
>> IRC the origin idea to use this was especially for people who
>> writing these DTS by hand which is not our case - at least
>> for majority of our customers.
>
> I write them by hand. Is there any other way?
Device-tree BSP and in 2014.01 there will be new BSP which just generate
them directly from the Vivado tools which just target your reference design.
You can connect your custom IP (or Xilinx or 3rd party) directly to the GIC
which using different IRQ sensitivity with whatever register addresses
and make no sense to write it by hand.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
On Mon, 2014-04-07 at 02:17PM +0200, Mike Looijmans wrote:
> On 04/07/2014 07:58 AM, Michal Simek wrote:
> >Hi Soren,
> >
> >On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
> >>Convert all Zynq DT files to the dtc preprocessor include syntax.
> >>This allows to include header files in the devicetrees like other
> >>SoC-types already do.
> >>
> >>Inspired-by: Steffen Trumtrar <[email protected]>
> >>(http://www.spinics.net/lists/arm-kernel/msg319832.html)
> >>
> >>Signed-off-by: Soren Brinkmann <[email protected]>
> >
> >These 4 patches needs more wider discussion if this is helpful or
> >not. Currently I can't see any value in it because everything
> >is just generated and fixed. I think I had the same discussion
> >with Laurent some weeks ago regarding this.
>
> I would be kinda neutral here. I'd consider it helpful, it improves
> readability (regardless of whether they are generated or hand
> crafted). That's convenient for things like interrupt sensitivity, I
> can't remember whether 4 is level or edge type. On the other hand,
> the clock indices are just as much magic numbers as the memory
> addresses. If I suspect an error in that area, I'd start by lokking
> in /sys/kernel/debug/clk but wouldn't start in the DT.
I agree. Readability is better this way. And whether our generator spits
out magic numbers of these symbolic names should not be a big
difference, is it?
Sören
On Mon, Apr 07, 2014 at 02:24:07PM +0200, Michal Simek wrote:
> Device-tree BSP and in 2014.01 there will be new BSP which just
> generate them directly from the Vivado tools which just target your
> reference design. You can connect your custom IP (or Xilinx or 3rd
> party) directly to the GIC which using different IRQ sensitivity
> with whatever register addresses and make no sense to write it by
> hand.
On our Zynq design here we ended up being unwilling to use platform
generation from Vivado. Basically all our IP was custom, so there was
no win at all to invoking the complexity of the automatic tools.
Thus we write the DT by hand, and our DT is complex, integrating
peripherals that span two FPGAs.
I think the in-kernel DT should use the kernel conventions, which
means using #include and the binding constants over magic values.
Jason
Hi!
On Mon, Apr 07, 2014 at 11:10:12AM -0600, Jason Gunthorpe wrote:
> On Mon, Apr 07, 2014 at 02:24:07PM +0200, Michal Simek wrote:
>
> > Device-tree BSP and in 2014.01 there will be new BSP which just
> > generate them directly from the Vivado tools which just target your
> > reference design. You can connect your custom IP (or Xilinx or 3rd
> > party) directly to the GIC which using different IRQ sensitivity
> > with whatever register addresses and make no sense to write it by
> > hand.
>
> On our Zynq design here we ended up being unwilling to use platform
> generation from Vivado. Basically all our IP was custom, so there was
> no win at all to invoking the complexity of the automatic tools.
>
> Thus we write the DT by hand, and our DT is complex, integrating
> peripherals that span two FPGAs.
>
> I think the in-kernel DT should use the kernel conventions, which
> means using #include and the binding constants over magic values.
>
ACK.
If in doubt follow common mainline practice. Although using includes
for DT is not necessarily common practice, readability of DTs is
really important IMHO.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 04/07/2014 08:02 PM, Steffen Trumtrar wrote:
> Hi!
>
> On Mon, Apr 07, 2014 at 11:10:12AM -0600, Jason Gunthorpe wrote:
>> On Mon, Apr 07, 2014 at 02:24:07PM +0200, Michal Simek wrote:
>>
>>> Device-tree BSP and in 2014.01 there will be new BSP which just
>>> generate them directly from the Vivado tools which just target your
>>> reference design. You can connect your custom IP (or Xilinx or 3rd
>>> party) directly to the GIC which using different IRQ sensitivity
>>> with whatever register addresses and make no sense to write it by
>>> hand.
>>
>> On our Zynq design here we ended up being unwilling to use platform
>> generation from Vivado. Basically all our IP was custom, so there was
>> no win at all to invoking the complexity of the automatic tools.
>>
>> Thus we write the DT by hand, and our DT is complex, integrating
>> peripherals that span two FPGAs.
>>
>> I think the in-kernel DT should use the kernel conventions, which
>> means using #include and the binding constants over magic values.
>>
>
> ACK.
>
> If in doubt follow common mainline practice. Although using includes
> for DT is not necessarily common practice, readability of DTs is
> really important IMHO.
Let me give you one example. When you add xilinx intc controller
to zynq HW design which uses gic with headers you are using
then you will find out that sensitivity for both controllers
are just different.
This is just one case I am aware of. I expect there will be one more.
Also dtses from kernel are copied to other project because separation
from kernel hasn't be done but there is any plan regarding this.
Using dtc preprocessor and macros improve DTS readability but at the same
time force other users to copy all necessary files from the kernel
to that projects which is just hassle.
With example above there are also cases where using macros is just wrong
that's why I don't want to start to use it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
On Tue, Apr 08, 2014 at 09:03:27AM +0200, Michal Simek wrote:
> > If in doubt follow common mainline practice. Although using includes
> > for DT is not necessarily common practice, readability of DTs is
> > really important IMHO.
>
> Let me give you one example. When you add xilinx intc controller
> to zynq HW design which uses gic with headers you are using
> then you will find out that sensitivity for both controllers
> are just different.
> This is just one case I am aware of. I expect there will be one more.
I'm not sure I see the problem here, just because some bindings can't
use the standard shared constants doesn't mean the GIC bindings and
users should avoid them. The binding documentation is supposed to make
it clear what is correct.
It is just as easy to get confused with numbers, does 4 mean
XILINX_INTC_IRQ_RISING or IRQ_TYPE_LEVEL_HIGH ?
> Using dtc preprocessor and macros improve DTS readability but at the same
> time force other users to copy all necessary files from the kernel
> to that projects which is just hassle.
You can run the DTS through cpp before you export it out of the kernel
environment, then you get a flat file with no includes.
The shared kernel conventions are more important than constraints from
outside projects.
Jason
On 04/08/2014 07:27 PM, Jason Gunthorpe wrote:
> On Tue, Apr 08, 2014 at 09:03:27AM +0200, Michal Simek wrote:
>
>>> If in doubt follow common mainline practice. Although using includes
>>> for DT is not necessarily common practice, readability of DTs is
>>> really important IMHO.
>>
>> Let me give you one example. When you add xilinx intc controller
>> to zynq HW design which uses gic with headers you are using
>> then you will find out that sensitivity for both controllers
>> are just different.
>> This is just one case I am aware of. I expect there will be one more.
>
> I'm not sure I see the problem here, just because some bindings can't
> use the standard shared constants doesn't mean the GIC bindings and
> users should avoid them. The binding documentation is supposed to make
> it clear what is correct.
>
> It is just as easy to get confused with numbers, does 4 mean
> XILINX_INTC_IRQ_RISING or IRQ_TYPE_LEVEL_HIGH ?
That's why you have there biding documentation to exactly know what it is.
>> Using dtc preprocessor and macros improve DTS readability but at the same
>> time force other users to copy all necessary files from the kernel
>> to that projects which is just hassle.
>
> You can run the DTS through cpp before you export it out of the kernel
> environment, then you get a flat file with no includes.
What's the result?
1. DTSI and DTS together which completely break hierarchy
2. DTS without comments
It means, yes, you get a file when you go through cpp but different
then you have now.
> The shared kernel conventions are more important than constraints from
> outside projects.
zynq-7000.dtsi is fixed and you can't just change it based on your project.
For things which are in your board file like zynq-zc702 then you can use
whatever you like.
Maybe I just need some time to get used to it but currently...
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
On Mon, 2014-04-07 at 07:46AM +0200, Michal Simek wrote:
> On 04/05/2014 01:14 AM, Soren Brinkmann wrote:
> > Specify the 'clock-latency' property to avoid certain cpufreq governors
> > from refusing to work with the following error:
> > ondemand governor failed, too long transition latency of HW, fallback to performance governor
> >
> > Reported-by: Mike Looijmans <[email protected]>
> > Signed-off-by: Soren Brinkmann <[email protected]>
> > Tested-by: Mike Looijmans <[email protected]>
> > Signed-off-by: Michal Simek <[email protected]>
> > ---
> > This is a fix from our vendor tree
> >
> > Changes in v2:
> > - This patch has been added
> >
> > ---
> > arch/arm/boot/dts/zynq-7000.dtsi | 1 +
> > 1 file changed, 1 insertion(+)
>
> Applied to zynq-dt.
Actually, this should probably go in as fix into the current (3.15)
release cycle.
Thanks,
Sören