Hey all-
I think everything is in a good state to merge now, from here its a
discussion of process.
Thanks to everyone involved.
---
Things have been relatively quiet on the Zynq front lately. This patchset does
a bit of cleanup of the Zynq subarchitecture. It was the necessary set of
things I had to do to get a zynq target booting with the upstream qemu model.
Patches 1 and 2 move zynq to use the GIC and pl310 L2 cache controller device
tree bindings respectively.
Patch 3 removes unused clock infrastructure. The plan is to rework the
out-of-tree Xilinx generic clk support into something suitable for merging.
What's in tree now just isn't used at all, and can be removed.
Patch 4 and 5 move around the static peripheral mappings into the vmalloc area.
---
Changes since v3:
- Patch 3 also removes the zynq "use" of versatile
Changes since v2:
- Reordered patchset to prevent remapping peripherals that were subsequently
removed from the static map
- Use DT bindings for the L2 cache controller
Changes since v1:
- Make sure arm at kernel.org was included
- Rebased on arm-soc/for-next
- Added a cover letter
- Elaborated a bit on why I removed CLKDEV_LOOKUP
---
Josh Cartwright (5):
zynq: use GIC device tree bindings
zynq: use pl310 device tree bindings
zynq: remove use of CLKDEV_LOOKUP
ARM: annotate VMALLOC_END definition with _AC
zynq: move static peripheral mappings
arch/arm/Kconfig | 1 -
arch/arm/Makefile | 1 -
arch/arm/boot/dts/zynq-ep107.dts | 17 +++++++++++++---
arch/arm/include/asm/pgtable.h | 2 +-
arch/arm/mach-zynq/common.c | 23 ++++++++++-----------
arch/arm/mach-zynq/include/mach/clkdev.h | 32 ------------------------------
arch/arm/mach-zynq/include/mach/zynq_soc.h | 29 ++++++++++++---------------
7 files changed, 38 insertions(+), 67 deletions(-)
delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h
--
1.8.0
@@ -55,12 +55,17 @@ static void __init xilinx_init_machine(void)of_platform_bus_probe(NULL,zynq_of_bus_ids,NULL);}+staticstructof_device_idirq_match[]__initdata={+{.compatible="arm,cortex-a9-gic",.data=gic_of_init,},+{}+};+/***xilinx_irq_init()-InterruptcontrollerinitializationfortheGIC.*/staticvoid__initxilinx_irq_init(void){-gic_init(0,29,SCU_GIC_DIST_BASE,SCU_GIC_CPU_BASE);+of_irq_init(irq_match);}/* The minimum devices needed to be mapped before the VM system is up and
The Zynq support in mainline does not (yet) make use of any of the
generic clk or clk lookup functionality. Remove what is upstream for
now, until the out-of-tree implementation is in suitable form for
merging.
An important side effect of this patch is that it allows the building of
a Zynq kernel without running into unresolved symbol problems:
drivers/built-in.o: In function `amba_get_enable_pclk':
clkdev.c:(.text+0x444): undefined reference to `clk_enable'
drivers/built-in.o: In function `amba_remove':
clkdev.c:(.text+0x488): undefined reference to `clk_disable'
drivers/built-in.o: In function `amba_probe':
clkdev.c:(.text+0x540): undefined reference to `clk_disable'
drivers/built-in.o: In function `amba_device_add':
clkdev.c:(.text+0x77c): undefined reference to `clk_disable'
drivers/built-in.o: In function `enable_clock':
clkdev.c:(.text+0x29738): undefined reference to `clk_enable'
drivers/built-in.o: In function `disable_clock':
clkdev.c:(.text+0x29778): undefined reference to `clk_disable'
drivers/built-in.o: In function `__pm_clk_remove':
clkdev.c:(.text+0x297f8): undefined reference to `clk_disable'
drivers/built-in.o: In function `pm_clk_suspend':
clkdev.c:(.text+0x29bc8): undefined reference to `clk_disable'
drivers/built-in.o: In function `pm_clk_resume':
clkdev.c:(.text+0x29c28): undefined reference to `clk_enable'
make[2]: *** [vmlinux] Error 1
make[1]: *** [sub-make] Error 2
make: *** [all] Error 2
In addition, eliminate Zynq's "use" of the versatile platform, as it is
no longer needed. As Nick Bowler points out:
For the record, I think this was introduced by commit 56a34b03ff427
("ARM: versatile: Make plat-versatile clock optional") which forgot to
select PLAT_VERSATILE_CLOCK on Zynq. This is not all that surprising,
because the fact that Zynq "uses" PLAT_VERSATILE is secretly hidden in
the Makefile.
Nevertheless, the only feature from versatile that Zynq needed was the
clock support, so this patch should *also* delete the secret use of
plat-versatile by removing this line from arch/arm/Makefile:
plat-$(CONFIG_ARCH_ZYNQ) += versatile
Signed-off-by: Josh Cartwright <redacted>
Cc: John Linn <redacted>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/Kconfig | 1 -
arch/arm/Makefile | 1 -
arch/arm/mach-zynq/common.c | 1 -
arch/arm/mach-zynq/include/mach/clkdev.h | 32 --------------------------------
4 files changed, 35 deletions(-)
delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h
@@ -1,32 +0,0 @@-/*- * arch/arm/mach-zynq/include/mach/clkdev.h- *- * Copyright (C) 2011 Xilinx, Inc.- *- * This software is licensed under the terms of the GNU General Public- * License version 2, as published by the Free Software Foundation, and- * may be copied, distributed, and modified under those terms.- *- * This program is distributed in the hope that it will be useful,- * but WITHOUT ANY WARRANTY; without even the implied warranty of- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the- * GNU General Public License for more details.- *- */--#ifndef __MACH_CLKDEV_H__-#define __MACH_CLKDEV_H__--#include <plat/clock.h>--struct clk {- unsigned long rate;- const struct clk_ops *ops;- const struct icst_params *params;- void __iomem *vcoreg;-};--#define __clk_get(clk) ({ 1; })-#define __clk_put(clk) do { } while (0)--#endif
This makes the definition of VMALLOC_END suitable for use within
assembly code. This is necessary to allow the use of VMALLOC_END in
defining where the early uart is mapped for use with DEBUG_LL.
Signed-off-by: Josh Cartwright <redacted>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/include/asm/pgtable.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Shifting them up into the vmalloc region prevents the following warning,
when booting a zynq qemu target with more than 512mb of RAM:
BUG: mapping for 0xe0000000 at 0xe0000000 out of vmalloc space
In addition, it allows for reuse of these mappings when the proper
drivers issue requests via ioremap().
Signed-off-by: Josh Cartwright <redacted>
Cc: John Linn <redacted>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/mach-zynq/common.c | 6 +++---
arch/arm/mach-zynq/include/mach/zynq_soc.h | 23 +++++++++++++----------
2 files changed, 16 insertions(+), 13 deletions(-)
From: Nick Bowler <hidden> Date: 2012-10-25 20:17:07
Hi Josh,
On 2012-10-24 15:04 -0500, Josh Cartwright wrote:
Shifting them up into the vmalloc region prevents the following warning,
when booting a zynq qemu target with more than 512mb of RAM:
[...]
-/* For now, all mappings are flat (physical = virtual)
+/* Static peripheral mappings are mapped at the top of the
+ * vmalloc region
*/
-#define UART0_PHYS 0xE0000000
-#define UART0_VIRT UART0_PHYS
+#define UART0_PHYS 0xE0000000
+#define UART0_SIZE SZ_4K
+#define UART0_VIRT (VMALLOC_END - UART0_SIZE)
Did you test this on any real hardware? I can't get the ZC702 to work
with the UART mapped at this address (this ends up being mapped at
0xFEFFF000), although I can't for the life of me figure out why the
virtual address even matters. Note that for the ZC702, the physical
address of the "main" UART is 0xE0001000.
All I end up seeing is "Uncompressing Linux... done, booting the
kernel." with no further messages. With the UART mapped at
0xF0001000, all printouts make it to the console. I tried a couple
different virtual addresses and I'm surprised at the results, since
the behaviour seems to vary wildly. I saw three behaviours depending
only on the virtual address of the static mapping; all results are 100%
reproducible:
"Works": all printouts make it to the console
"Fails": no printouts make it to the console after decompression
"Truncated": the first few lines of output do not make it to the
console, but after that it "Works". The first line
successfully printed is always
"Built 1 zonelists in Zone order, mobility grouping on. Total pages: 260096"
And here are the addresses I tested:
Address Result
-----------------------
0xf0000000 Truncated
0xf0001000 Works
0xf0007000 Truncated
0xf0008000 Fails
0xf0009000 Fails
0xf000e000 Truncated
0xf000f000 Fails
0xf8000000 Truncated
0xf8001000 Works
0xfef00000 Truncated
0xfef01000 Works
0xfef08000 Fails
0xfef0f000 Fails
0xfeff0000 Fails
0xfeff1000 Fails
0xfeffe000 Fails
0xfefff000 Fails
Judging by the list, the console seems to only work properly if the
defined virtual address is Fxxx1000 and xxx is not too big...
Confused,
--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)
On Thu, Oct 25, 2012 at 04:17:01PM -0400, Nick Bowler wrote:
Hi Josh,
On 2012-10-24 15:04 -0500, Josh Cartwright wrote:
quoted
Shifting them up into the vmalloc region prevents the following warning,
when booting a zynq qemu target with more than 512mb of RAM:
[...]
quoted
-/* For now, all mappings are flat (physical = virtual)
+/* Static peripheral mappings are mapped at the top of the
+ * vmalloc region
*/
-#define UART0_PHYS 0xE0000000
-#define UART0_VIRT UART0_PHYS
+#define UART0_PHYS 0xE0000000
+#define UART0_SIZE SZ_4K
+#define UART0_VIRT (VMALLOC_END - UART0_SIZE)
Did you test this on any real hardware? I can't get the ZC702 to work
with the UART mapped at this address (this ends up being mapped at
0xFEFFF000), although I can't for the life of me figure out why the
virtual address even matters. Note that for the ZC702, the physical
address of the "main" UART is 0xE0001000.
Ugh, not yet; My testing has been on a qemu model. I also
unfortunately neglected to mention I am carrying a qemu patch that
forces RX_EN/TX_EN of the uarts out of reset. There is an (incomplete)
thread on qemu-devel discussing whose responsibility it really is to
enable the uarts:
http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg03779.html
Clearly, though, if you are seeing the "Uncompressing Linux..."
messages, then the uart is enabled, so I don't think that's the problem.
All I end up seeing is "Uncompressing Linux... done, booting the
kernel." with no further messages. With the UART mapped at
0xF0001000, all printouts make it to the console. I tried a couple
different virtual addresses and I'm surprised at the results, since
the behaviour seems to vary wildly. I saw three behaviours depending
only on the virtual address of the static mapping; all results are 100%
reproducible:
"Works": all printouts make it to the console
"Fails": no printouts make it to the console after decompression
"Truncated": the first few lines of output do not make it to the
console, but after that it "Works". The first line
successfully printed is always
"Built 1 zonelists in Zone order, mobility grouping on. Total pages: 260096"
Odd, I'm wondering the uart gets into a weird state, and some bits get
knocked loose at console_initcall() time, when the console driver comes
up (Assuming CONFIG_SERIAL_XILINX_PS_UART)?
And here are the addresses I tested:
Address Result
-----------------------
0xf0000000 Truncated
0xf0001000 Works
0xf0007000 Truncated
0xf0008000 Fails
0xf0009000 Fails
0xf000e000 Truncated
0xf000f000 Fails
0xf8000000 Truncated
0xf8001000 Works
0xfef00000 Truncated
0xfef01000 Works
0xfef08000 Fails
0xfef0f000 Fails
0xfeff0000 Fails
0xfeff1000 Fails
0xfeffe000 Fails
0xfefff000 Fails
Judging by the list, the console seems to only work properly if the
defined virtual address is Fxxx1000 and xxx is not too big...
From: Nick Bowler <hidden> Date: 2012-10-25 22:41:12
On 2012-10-25 16:29 -0500, Josh Cartwright wrote:
On Thu, Oct 25, 2012 at 04:17:01PM -0400, Nick Bowler wrote:
quoted
Did you test this on any real hardware? I can't get the ZC702 to work
with the UART mapped at this address (this ends up being mapped at
0xFEFFF000), although I can't for the life of me figure out why the
virtual address even matters. Note that for the ZC702, the physical
address of the "main" UART is 0xE0001000.
Ugh, not yet; My testing has been on a qemu model. I also
unfortunately neglected to mention I am carrying a qemu patch that
forces RX_EN/TX_EN of the uarts out of reset. There is an (incomplete)
thread on qemu-devel discussing whose responsibility it really is to
enable the uarts:
http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg03779.html
Clearly, though, if you are seeing the "Uncompressing Linux..."
messages, then the uart is enabled, so I don't think that's the problem.
Yes, the uart is presumably enabled by u-boot.
quoted
"Works": all printouts make it to the console
"Fails": no printouts make it to the console after decompression
"Truncated": the first few lines of output do not make it to the
console, but after that it "Works". The first line
successfully printed is always
"Built 1 zonelists in Zone order, mobility grouping on. Total pages: 260096"
Odd, I'm wondering the uart gets into a weird state, and some bits get
knocked loose at console_initcall() time, when the console driver comes
up (Assuming CONFIG_SERIAL_XILINX_PS_UART)?
While I am using that driver, it is not initialized until relatively
late in the boot process. If I were to guess, I would guess that,
except for when it "Works", the really really early printk stuff isn't
actually hitting the uart at all. The "Fails" case would then be due to
the stray writes crashing the board, and the "Truncated" case due to the
stray writes being (ostensibly) benign.
But I really have no way right now to test this hypothesis, since I
can't print anything in the failing case.
quoted
And here are the addresses I tested:
Address Result
-----------------------
0xf0000000 Truncated
0xf0001000 Works
[...]
quoted
0xfefff000 Fails
Judging by the list, the console seems to only work properly if the
defined virtual address is Fxxx1000 and xxx is not too big...
Very odd. Do you mind sending out your patch allowing the selection of
the secondary uart for DEBUG_LL?
I will follow up with the version that applies on top of your series in
a moment. I'm confident that the UART works on the ZC702 when mapped at
0xf0001000, since I've been running with that since I first got my hands
on one of these boards.
But you don't need any patch to do the same tests I was doing above:
you can just change UART0_PHYS to 0xe0001000 and then set UART0_VIRT
accordingly (you may need to move the TTC/SCU mappings depending where
you put the UART, of course).
Cheers,
--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)
From: Nick Bowler <hidden> Date: 2012-10-25 22:48:20
The main UART on the Xilinx ZC702 board is UART1, located at address
e0001000. Add a Kconfig option to select this device as the low-level
debugging port. This allows the really early boot printouts to reach
the USB serial adaptor on this board.
For consistency's sake, add a choice entry for UART0 even though it is
the the default if UART1 is not selected.
As there are currently known issues related to the UART virtual
mappings, this is KNOWN BROKEN, not to be merged yet!
Not-Yet-Signed-off-by: Nick Bowler [off-list ref]
---
arch/arm/Kconfig.debug | 17 +++++++++++++++++
arch/arm/mach-zynq/common.c | 6 +++---
arch/arm/mach-zynq/include/mach/zynq_soc.h | 18 ++++++++++++------
3 files changed, 32 insertions(+), 9 deletions(-)
@@ -132,6 +132,23 @@ choicetheiroutputtoUART1serialportonDaVinciTNETV107Xdevices.+configDEBUG_ZYNQ_UART0+bool"Kernel low-level debugging on Xilinx Zynq using UART0"+depends onARCH_ZYNQ+help+SayYhereifyouwantthedebugprintroutinestodirect+theiroutputtoUART0ontheZynqplatform.++configDEBUG_ZYNQ_UART1+bool"Kernel low-level debugging on Xilinx Zynq using UART1"+depends onARCH_ZYNQ+help+SayYhereifyouwantthedebugprintroutinestodirect+theiroutputtoUART1ontheZynqplatform.++IfyouhaveaZC702boardandwantearlybootmessagesto+appearontheUSBserialadaptor,selectthisoption.+configDEBUG_DC21285_PORTbool"Kernel low-level debugging messages via footbridge serial port"depends onFOOTBRIDGE
On Thu, Oct 25, 2012 at 06:41:08PM -0400, Nick Bowler wrote:
On 2012-10-25 16:29 -0500, Josh Cartwright wrote:
quoted
On Thu, Oct 25, 2012 at 04:17:01PM -0400, Nick Bowler wrote:
quoted
Did you test this on any real hardware? I can't get the ZC702 to work
with the UART mapped at this address (this ends up being mapped at
0xFEFFF000), although I can't for the life of me figure out why the
virtual address even matters. Note that for the ZC702, the physical
address of the "main" UART is 0xE0001000.
Good news is you're not crazy; I was able to duplicate the problem here.
If I were to guess, I would guess that, except for when it "Works",
the really really early printk stuff isn't actually hitting the uart
at all. The "Fails" case would then be due to the stray writes
crashing the board, and the "Truncated" case due to the stray writes
being (ostensibly) benign.
If I'm not mistaken, this hypothesis is predicated on the early bootup
code establishing a (linear?) mapping for addresses > VMALLOC_START;
before the mdesc->map_io() is even handled. That seems odd to me.
But I really have no way right now to test this hypothesis, since I
can't print anything in the failing case.
Not sure if I'll be able to get anything meaningful out of it yet (I've
not historically had good luck with Xilinx's debugging tools), but I did
finally get a JTAG debugger hooked up to the zc702. I'll see if I can
get any useful information tomorrow.
Thanks,
Josh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121025/1c0dba3d/attachment-0001.sig>
From: Michal Simek <hidden> Date: 2012-10-27 13:39:09
Hi Josh,
quoted hunk
-----Original Message-----
From: Josh Cartwright [mailto:josh.cartwright at ni.com]
Sent: Wednesday, October 24, 2012 10:03 PM
To: arm at kernel.org; Arnd Bergmann
Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org; John
Linn; Nick Bowler; Michal Simek
Subject: [PATCH v4 1/5] zynq: use GIC device tree bindings
The Zynq uses the cortex-a9-gic. This eliminates the need to hardcode register
addresses.
Signed-off-by: Josh Cartwright <redacted>
Cc: John Linn <redacted>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/boot/dts/zynq-ep107.dts | 8 +++++---
arch/arm/mach-zynq/common.c | 7 ++++++-
arch/arm/mach-zynq/include/mach/zynq_soc.h | 2 --
3 files changed, 11 insertions(+), 6 deletions(-)
If you change git to 3 cells format you should also change it for uart below.
Also will be the best to remove this dts entirely and replace it by existing
Platform.
Thanks,
Michal
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
From: Michal Simek <hidden> Date: 2012-10-27 13:40:42
quoted hunk
-----Original Message-----
From: Josh Cartwright [mailto:josh.cartwright at ni.com]
Sent: Wednesday, October 24, 2012 10:04 PM
To: arm at kernel.org; Arnd Bergmann
Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org; John
Linn; Nick Bowler; Michal Simek
Subject: [PATCH v4 2/5] zynq: use pl310 device tree bindings
The Zynq has a PL310 L2 cache controller. Convert in-tree uses to using the
device tree.
Signed-off-by: Josh Cartwright <redacted>
Cc: John Linn <redacted>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/boot/dts/zynq-ep107.dts | 9 +++++++++
arch/arm/mach-zynq/common.c | 9 +--------
arch/arm/mach-zynq/include/mach/zynq_soc.h | 4 ----
3 files changed, 10 insertions(+), 12 deletions(-)
This is ok. No changes are necessary.
Acked-by: Michal Simek <redacted>
Please add my acked-by line to this patch to the v5 series.
When all patches are ready I will apply it to zynq next branch at git.xilnx.com.
Thanks,
Michal
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
From: Michal Simek <hidden> Date: 2012-10-27 13:59:21
quoted hunk
-----Original Message-----
From: Josh Cartwright [mailto:josh.cartwright at ni.com]
Sent: Wednesday, October 24, 2012 10:05 PM
To: arm at kernel.org; Arnd Bergmann
Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org; John
Linn; Nick Bowler; Michal Simek
Subject: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC
This makes the definition of VMALLOC_END suitable for use within assembly
code. This is necessary to allow the use of VMALLOC_END in defining where the
early uart is mapped for use with DEBUG_LL.
Signed-off-by: Josh Cartwright <redacted>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/include/asm/pgtable.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
This shouldn't be the part of this series but should go to mainline through different tree.
Arnd, Olof: Can you take this patch to your arm-soc tree?
I don't think it is a good workstyle to propose it to mainline through zynq soc tree.
What do you think?
Thanks,
Michal
On Sat, Oct 27, 2012 at 01:39:00PM +0000, Michal Simek wrote:
Hi Josh,
quoted
-----Original Message-----
From: Josh Cartwright [mailto:josh.cartwright at ni.com]
Sent: Wednesday, October 24, 2012 10:03 PM
To: arm at kernel.org; Arnd Bergmann
Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org; John
Linn; Nick Bowler; Michal Simek
Subject: [PATCH v4 1/5] zynq: use GIC device tree bindings
The Zynq uses the cortex-a9-gic. This eliminates the need to hardcode register
addresses.
Signed-off-by: Josh Cartwright <redacted>
Cc: John Linn <redacted>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/boot/dts/zynq-ep107.dts | 8 +++++---
arch/arm/mach-zynq/common.c | 7 ++++++-
arch/arm/mach-zynq/include/mach/zynq_soc.h | 2 --
3 files changed, 11 insertions(+), 6 deletions(-)
If you change git to 3 cells format you should also change it for uart below.
Also will be the best to remove this dts entirely and replace it by existing
Platform.
Do you mean to say get rid of the ep107 entirely and replace it with,
for example, a zc702 dts?
I have a follow up patchset which splits off a zynq-7000.dtsi and a
zynq-zc702.dts, and which also fixes the 3 interrupt cell problem.
Would you like for me to pull this into v5, or spin up a separate patch
series?
Thanks,
Josh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121027/46954293/attachment.sig>
From: Michal Simek <hidden> Date: 2012-10-27 14:08:13
Hi Josh
-----Original Message-----
From: Josh Cartwright [mailto:josh.cartwright at ni.com]
Sent: Saturday, October 27, 2012 4:01 PM
To: Michal Simek
Cc: arm at kernel.org; Arnd Bergmann; linux-kernel at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; John Linn; Nick Bowler
Subject: Re: [PATCH v4 1/5] zynq: use GIC device tree bindings
On Sat, Oct 27, 2012 at 01:39:00PM +0000, Michal Simek wrote:
quoted
Hi Josh,
quoted
-----Original Message-----
From: Josh Cartwright [mailto:josh.cartwright at ni.com]
Sent: Wednesday, October 24, 2012 10:03 PM
To: arm at kernel.org; Arnd Bergmann
Cc: linux-kernel at vger.kernel.org;
linux-arm-kernel at lists.infradead.org; John Linn; Nick Bowler; Michal
Simek
Subject: [PATCH v4 1/5] zynq: use GIC device tree bindings
The Zynq uses the cortex-a9-gic. This eliminates the need to
hardcode register addresses.
Signed-off-by: Josh Cartwright <redacted>
Cc: John Linn <redacted>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/boot/dts/zynq-ep107.dts | 8 +++++---
arch/arm/mach-zynq/common.c | 7 ++++++-
arch/arm/mach-zynq/include/mach/zynq_soc.h | 2 --
3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/arch/arm/boot/dts/zynq-ep107.dts
b/arch/arm/boot/dts/zynq- ep107.dts index 37ca192..7bfff4a 100644
If you change git to 3 cells format you should also change it for uart below.
Also will be the best to remove this dts entirely and replace it by
existing Platform.
Do you mean to say get rid of the ep107 entirely and replace it with, for
example, a zc702 dts?
Yes, Ep107 platform is nothing what you can buy, that's why it is not the platform
Which should be supported in mainline. Supporting zc702 make sense.
I have a follow up patchset which splits off a zynq-7000.dtsi and a zynq-
zc702.dts, and which also fixes the 3 interrupt cell problem.
I am not big fan to use dtsi solution because dts can be simple generated directly
From Xilinx design tool based on your hw design. That's why I can't see any benefit
To have dtsi file.
Would you like for me to pull this into v5, or spin up a separate patch series?
Definitely not. I have checked patches but haven't got it work on the zc702.
Not sure if you have run it on real hw or just on the qemu as you have mentioned
In 5/5.
Thanks,
Michal
On Sat, Oct 27, 2012 at 02:06:45PM +0000, Michal Simek wrote:
[...]
I am not big fan to use dtsi solution because dts can be simple generated directly
From Xilinx design tool based on your hw design. That's why I can't see any benefit
To have dtsi file.
Can I ask you to reconsider? We, for example, don't make any use of the
Xilinx dev tools to generate our device trees. Having a dtsi allows for
easy extension of the zynq-7000 platform for our boards, without having
to carry duplicate data.
Is it going to be expected that users building kernels for their zynq
targets have access to the Xilinx EDK?
quoted
Would you like for me to pull this into v5, or spin up a separate patch series?
Definitely not. I have checked patches but haven't got it work on the zc702.
Not sure if you have run it on real hw or just on the qemu as you have mentioned
In 5/5.
You're likely running into the issue Nick has identified in the thread
for patch 5 where the chosen virtual address for the uart doesn't seem
to work: http://www.spinics.net/lists/arm-kernel/msg203141.html
We haven't yet identified the root cause; any insight you can provide
here would be beneficial.
Otherwise, I'm considering reworking patch 5 to move the uart mapping to
a known working location.
Thanks,
Josh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121027/1fdbdb53/attachment.sig>
From: Michal Simek <hidden> Date: 2012-10-27 15:21:11
-----Original Message-----
From: Josh Cartwright [mailto:josh.cartwright at ni.com]
Sent: Saturday, October 27, 2012 4:43 PM
To: Michal Simek
Cc: arm at kernel.org; Arnd Bergmann; linux-kernel at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; John Linn; Nick Bowler
Subject: Re: [PATCH v4 1/5] zynq: use GIC device tree bindings
On Sat, Oct 27, 2012 at 02:06:45PM +0000, Michal Simek wrote:
[...]
quoted
I am not big fan to use dtsi solution because dts can be simple
generated directly From Xilinx design tool based on your hw design.
That's why I can't see any benefit To have dtsi file.
Can I ask you to reconsider?
I am open to all solution which will help others. I am not definitely saying NO for this features
I just haven't found a reason to support it.
We, for example, don't make any use of the Xilinx
dev tools to generate our device trees.
Ok. How does your working flow looks like?
I mean you don't get any information from hardware guys how does your hw design look like?
Having a dtsi allows for easy extension
of the zynq-7000 platform for our boards, without having to carry duplicate data.
ok. I think that make sense if you send the next your series as RFC to see how exactly
you would like to use it.
In my workflow we generate DTS directly from design tool which I expect your hw
guys use because it is probably needed to generate boot.bin/fsbl/etc.
Then there is one more additional step to setup device-tree bsp to generate DTS
which directly fits to your HW design.
If you have the same boards with different programmable logic I understand
that you would like to share that PS part and then just add it that IPs in PL.
Is it going to be expected that users building kernels for their zynq targets have
access to the Xilinx EDK?
Definitely not. You can do it just once and of course you can write it by hand
and then just reusing.
From my point of view. You have to use design tools at least once to get bitstream
and boot.bin with fsbl. Please correct me if I am wrong.
In this step you can use device-tree BSP to generate DTS ( I doesn't need to be perfect with
all attached devices on i2c,spi, phys, etc but it reflects your hardware).
You will get it in some seconds and your dts has correct irq numbers/ip description, compatible strings,
addresses, position in the system - if you use bus bridges, etc) and you can just directly use it
and kernel will boot. If you need to do changes in dts by hand, you can of course do it.
quoted
quoted
Would you like for me to pull this into v5, or spin up a separate patch series?
Definitely not. I have checked patches but haven't got it work on the zc702.
Not sure if you have run it on real hw or just on the qemu as you have
mentioned In 5/5.
You're likely running into the issue Nick has identified in the thread for patch 5
where the chosen virtual address for the uart doesn't seem to work:
http://www.spinics.net/lists/arm-kernel/msg203141.html
We haven't yet identified the root cause; any insight you can provide here
would be beneficial.
Otherwise, I'm considering reworking patch 5 to move the uart mapping to a
known working location.
I just need to get some time to catch you.
thanks,
Michal
From: Michal Simek <hidden> Date: 2012-10-27 16:47:57
quoted hunk
-----Original Message-----
From: Josh Cartwright [mailto:josh.cartwright at ni.com]
Sent: Wednesday, October 24, 2012 10:04 PM
To: arm at kernel.org; Arnd Bergmann
Cc: linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org; John
Linn; Nick Bowler; Michal Simek
Subject: [PATCH v4 3/5] zynq: remove use of CLKDEV_LOOKUP
The Zynq support in mainline does not (yet) make use of any of the generic clk
or clk lookup functionality. Remove what is upstream for now, until the out-of-
tree implementation is in suitable form for merging.
An important side effect of this patch is that it allows the building of a Zynq
kernel without running into unresolved symbol problems:
drivers/built-in.o: In function `amba_get_enable_pclk':
clkdev.c:(.text+0x444): undefined reference to `clk_enable'
drivers/built-in.o: In function `amba_remove':
clkdev.c:(.text+0x488): undefined reference to `clk_disable'
drivers/built-in.o: In function `amba_probe':
clkdev.c:(.text+0x540): undefined reference to `clk_disable'
drivers/built-in.o: In function `amba_device_add':
clkdev.c:(.text+0x77c): undefined reference to `clk_disable'
drivers/built-in.o: In function `enable_clock':
clkdev.c:(.text+0x29738): undefined reference to `clk_enable'
drivers/built-in.o: In function `disable_clock':
clkdev.c:(.text+0x29778): undefined reference to `clk_disable'
drivers/built-in.o: In function `__pm_clk_remove':
clkdev.c:(.text+0x297f8): undefined reference to `clk_disable'
drivers/built-in.o: In function `pm_clk_suspend':
clkdev.c:(.text+0x29bc8): undefined reference to `clk_disable'
drivers/built-in.o: In function `pm_clk_resume':
clkdev.c:(.text+0x29c28): undefined reference to `clk_enable'
make[2]: *** [vmlinux] Error 1
make[1]: *** [sub-make] Error 2
make: *** [all] Error 2
In addition, eliminate Zynq's "use" of the versatile platform, as it is no longer
needed. As Nick Bowler points out:
For the record, I think this was introduced by commit 56a34b03ff427
("ARM: versatile: Make plat-versatile clock optional") which forgot to
select PLAT_VERSATILE_CLOCK on Zynq. This is not all that surprising,
because the fact that Zynq "uses" PLAT_VERSATILE is secretly hidden in
the Makefile.
Nevertheless, the only feature from versatile that Zynq needed was the
clock support, so this patch should *also* delete the secret use of
plat-versatile by removing this line from arch/arm/Makefile:
plat-$(CONFIG_ARCH_ZYNQ) += versatile
Signed-off-by: Josh Cartwright <redacted>
Cc: John Linn <redacted>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm/Kconfig | 1 -
arch/arm/Makefile | 1 -
arch/arm/mach-zynq/common.c | 1 -
arch/arm/mach-zynq/include/mach/clkdev.h | 32 --------------------------------
4 files changed, 35 deletions(-)
delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index cce4f8d..de70d99 100644--- a/arch/arm/Kconfig+++ b/arch/arm/Kconfig
@@ -1,32 +0,0 @@-/*- * arch/arm/mach-zynq/include/mach/clkdev.h- *- * Copyright (C) 2011 Xilinx, Inc.- *- * This software is licensed under the terms of the GNU General Public- * License version 2, as published by the Free Software Foundation, and- * may be copied, distributed, and modified under those terms.- *- * This program is distributed in the hope that it will be useful,- * but WITHOUT ANY WARRANTY; without even the implied warranty of- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the- * GNU General Public License for more details.- *- */--#ifndef __MACH_CLKDEV_H__-#define __MACH_CLKDEV_H__--#include <plat/clock.h>--struct clk {- unsigned long rate;- const struct clk_ops *ops;- const struct icst_params *params;- void __iomem *vcoreg;-};--#define __clk_get(clk) ({ 1; })-#define __clk_put(clk) do { } while (0)--#endif
Tested-by: Michal Simek <redacted>
Thanks
Michal
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
From: Michal Simek <hidden> Date: 2012-10-27 16:52:47
HI Josh and Nick,
look below.
-----Original Message-----
From: Josh Cartwright [mailto:josh.cartwright at ni.com]
Sent: Friday, October 26, 2012 3:03 AM
To: Nick Bowler
Cc: arm at kernel.org; Arnd Bergmann; linux-kernel at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; John Linn; Michal Simek
Subject: Re: [PATCH v4 5/5] zynq: move static peripheral mappings
On Thu, Oct 25, 2012 at 06:41:08PM -0400, Nick Bowler wrote:
quoted
On 2012-10-25 16:29 -0500, Josh Cartwright wrote:
quoted
On Thu, Oct 25, 2012 at 04:17:01PM -0400, Nick Bowler wrote:
quoted
Did you test this on any real hardware? I can't get the ZC702 to
work with the UART mapped at this address (this ends up being
mapped at 0xFEFFF000), although I can't for the life of me figure
out why the virtual address even matters. Note that for the
ZC702, the physical address of the "main" UART is 0xE0001000.
Good news is you're not crazy; I was able to duplicate the problem here.
quoted
If I were to guess, I would guess that, except for when it "Works",
the really really early printk stuff isn't actually hitting the uart
at all. The "Fails" case would then be due to the stray writes
crashing the board, and the "Truncated" case due to the stray writes
being (ostensibly) benign.
If I'm not mistaken, this hypothesis is predicated on the early bootup code
establishing a (linear?) mapping for addresses > VMALLOC_START; before the
mdesc->map_io() is even handled. That seems odd to me.
quoted
But I really have no way right now to test this hypothesis, since I
can't print anything in the failing case.
Not sure if I'll be able to get anything meaningful out of it yet (I've not
historically had good luck with Xilinx's debugging tools), but I did finally get a
JTAG debugger hooked up to the zc702. I'll see if I can get any useful
information tomorrow.
I have seen the same problem on zc702. I will debug it.
Josh: the best will be if you can send v5 for patches 1-3 (1 with small changes in dts - uart) which I will apply
to arm-next.
4/5 should go out of zynq subtree, it means directly to arm-soc or via Russel's tree.
5/5 + Nick patch should be tested.
Thanks,
Michal
On Thu, Oct 25, 2012 at 06:47:34PM -0400, Nick Bowler wrote:
The main UART on the Xilinx ZC702 board is UART1, located at address
e0001000. Add a Kconfig option to select this device as the low-level
debugging port. This allows the really early boot printouts to reach
the USB serial adaptor on this board.
For consistency's sake, add a choice entry for UART0 even though it is
the the default if UART1 is not selected.
As there are currently known issues related to the UART virtual
mappings, this is KNOWN BROKEN, not to be merged yet!
Not-Yet-Signed-off-by: Nick Bowler [off-list ref]
Tested-by: Josh Cartwright <redacted>
Now that v5 of the initial zynq cleanup patchset is queued up for
merging (with a workaround for the uart mapping problem), what would it
take for you to sign off on this patch?
There is some trivial merging that has to be done to get it to apply
cleanly on v5. See a rebased version below.
Thanks,
Josh
@@ -132,6 +132,23 @@ choicetheiroutputtoUART1serialportonDaVinciTNETV107Xdevices.+configDEBUG_ZYNQ_UART0+bool"Kernel low-level debugging on Xilinx Zynq using UART0"+depends onARCH_ZYNQ+help+SayYhereifyouwantthedebugprintroutinestodirect+theiroutputtoUART0ontheZynqplatform.++configDEBUG_ZYNQ_UART1+bool"Kernel low-level debugging on Xilinx Zynq using UART1"+depends onARCH_ZYNQ+help+SayYhereifyouwantthedebugprintroutinestodirect+theiroutputtoUART1ontheZynqplatform.++IfyouhaveaZC702boardandwantearlybootmessagesto+appearontheUSBserialadaptor,selectthisoption.+configDEBUG_DC21285_PORTbool"Kernel low-level debugging messages via footbridge serial port"depends onFOOTBRIDGE
From: Nick Bowler <hidden> Date: 2012-10-29 18:13:44
On 2012-10-29 10:56 -0600, Josh Cartwright wrote:
On Thu, Oct 25, 2012 at 06:47:34PM -0400, Nick Bowler wrote:
quoted
The main UART on the Xilinx ZC702 board is UART1, located at address
e0001000. Add a Kconfig option to select this device as the low-level
debugging port. This allows the really early boot printouts to reach
the USB serial adaptor on this board.
For consistency's sake, add a choice entry for UART0 even though it is
the the default if UART1 is not selected.
As there are currently known issues related to the UART virtual
mappings, this is KNOWN BROKEN, not to be merged yet!
Not-Yet-Signed-off-by: Nick Bowler [off-list ref]
Tested-by: Josh Cartwright <redacted>
Now that v5 of the initial zynq cleanup patchset is queued up for
merging (with a workaround for the uart mapping problem), what would it
take for you to sign off on this patch?
Great, I've tested this on top of the other 4 and the boot console is
working now. I will resend the patch with my signoff.
(I wonder if UART0 has similar address problems on the ZC702 if it is
selected as the boot console... but this is harder to test as AFAIK it's
not connected to anything on this board by default, but it could in
principle be connected to something in the PL. We can cross that bridge
if and when we get to it, I guess).
There is some trivial merging that has to be done to get it to apply
cleanly on v5. See a rebased version below.
This shouldn't be the part of this series but should go to mainline through different tree.
Arnd, Olof: Can you take this patch to your arm-soc tree?
I don't think it is a good workstyle to propose it to mainline through zynq soc tree.
What do you think?
The arm-soc tree is not the right place either, this is architecture code which is
in Russell's domain. I would suggest getting an Ack from Russell if he's ok with
it and then merging it together with your other changes into arm-soc.
Arnd
From: Michal Simek <hidden> Date: 2012-10-31 08:43:50
-----Original Message-----
From: Arnd Bergmann [mailto:arnd at arndb.de]
Sent: Tuesday, October 30, 2012 11:22 PM
To: Michal Simek
Cc: Josh Cartwright; arm at kernel.org; linux-kernel at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; John Linn; Nick Bowler; Russell King - ARM Linux
Subject: Re: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC
On Saturday 27 October 2012, Michal Simek wrote:
quoted
quoted
diff --git a/arch/arm/include/asm/pgtable.h
b/arch/arm/include/asm/pgtable.h index 08c1231..72904a2 100644
This shouldn't be the part of this series but should go to mainline through
different tree.
quoted
Arnd, Olof: Can you take this patch to your arm-soc tree?
I don't think it is a good workstyle to propose it to mainline through zynq soc
tree.
quoted
What do you think?
The arm-soc tree is not the right place either, this is architecture code which is in
Russell's domain. I would suggest getting an Ack from Russell if he's ok with it
and then merging it together with your other changes into arm-soc.
That's what I thought too.
Not sure if Josh wants to push this to mainline.
Thanks,
Michal
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
On Wed, Oct 31, 2012 at 08:43:35AM +0000, Michal Simek wrote:
quoted
-----Original Message-----
From: Arnd Bergmann [mailto:arnd at arndb.de]
Sent: Tuesday, October 30, 2012 11:22 PM
To: Michal Simek
Cc: Josh Cartwright; arm at kernel.org; linux-kernel at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; John Linn; Nick Bowler; Russell King - ARM Linux
Subject: Re: [PATCH v4 4/5] ARM: annotate VMALLOC_END definition with _AC
On Saturday 27 October 2012, Michal Simek wrote:
quoted
quoted
diff --git a/arch/arm/include/asm/pgtable.h
b/arch/arm/include/asm/pgtable.h index 08c1231..72904a2 100644
This shouldn't be the part of this series but should go to mainline through
different tree.
quoted
Arnd, Olof: Can you take this patch to your arm-soc tree?
I don't think it is a good workstyle to propose it to mainline through zynq soc
tree.
quoted
What do you think?
The arm-soc tree is not the right place either, this is architecture code which is in
Russell's domain. I would suggest getting an Ack from Russell if he's ok with it
and then merging it together with your other changes into arm-soc.
That's what I thought too.
Not sure if Josh wants to push this to mainline.
The patch is not relevant anymore. It was needed when the virtual
address of the early uart mapping was defined via VMALLOC_END. That was
causing us problems, so we ended up choosing a fixed, known working
address within the vmalloc region to map the uart.
Thanks,
Josh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121031/9c397885/attachment.sig>
On Sat, Oct 27, 2012 at 03:20:59PM +0000, Michal Simek wrote:
On Saturday, October 27, 2012 4:43 PM, Josh Cartwright wrote:
quoted
On Sat, Oct 27, 2012 at 02:06:45PM +0000, Michal Simek wrote:
[...]
quoted
I am not big fan to use dtsi solution because dts can be simple
generated directly From Xilinx design tool based on your hw design.
That's why I can't see any benefit To have dtsi file.
Can I ask you to reconsider?
I am open to all solution which will help others. I am not definitely
saying NO for this features I just haven't found a reason to support
it.
quoted
We, for example, don't make any use of the Xilinx
dev tools to generate our device trees.
Ok. How does your working flow looks like? I mean you don't get any
information from hardware guys how does your hw design look like?
Our usecase may admittedly be a bit weird, because what logic is in the
PL is ultimately determined (and even implemented) by the end user and
is loaded at runtime. There is a lot of machinery to make that happen,
but the point is that I don't have sufficient knowledge upfront to
generate appropriate bindings for what's in the PL.
quoted
Having a dtsi allows for easy extension of the zynq-7000 platform
for our boards, without having to carry duplicate data.
ok. I think that make sense if you send the next your series as RFC to
see how exactly you would like to use it.
It seems like you caught a glimpse of this in my COMMON_CLK patchset. :)
In my workflow we generate DTS directly from design tool which I
expect your hw guys use because it is probably needed to generate
boot.bin/fsbl/etc. Then there is one more additional step to setup
device-tree bsp to generate DTS which directly fits to your HW design.
If you have the same boards with different programmable logic I
understand that you would like to share that PS part and then just add
it that IPs in PL.
[..]
From my point of view. You have to use design tools at least once to
get bitstream and boot.bin with fsbl. Please correct me if I am wrong.
In this step you can use device-tree BSP to generate DTS ( I doesn't
need to be perfect with all attached devices on i2c,spi, phys, etc but
it reflects your hardware). You will get it in some seconds and your
dts has correct irq numbers/ip description, compatible strings,
addresses, position in the system - if you use bus bridges, etc) and
you can just directly use it and kernel will boot. If you need to do
changes in dts by hand, you can of course do it.
I wouldn't be as opposed to device tree generation if the device tree
generator was in tree. Device tree bindings change, how would/could an
out-of-tree generator possibly handle changes in bindings? Would a user
target the generator at a specific version of the kernel? (An in tree
generator would also seem to necessitate a more formal specification
language for DT bindings).
It is odd to me that the use of a generator would be required to create
what is completely static data. What I'm referring to here is the
collection of peripherals on the zynq-7000 that are not in the PL. For
me, this requirement adds an unnecessary dependency on the Xilinx EDK
that I would like to avoid.
Would it make sense to limit what the device tree generator produces to
only what is in the PL? (Forgive my ignorance about this tool, I've
never used it.)
This could just be an extension of what I've started to do with the
COMMON_CLK patchset. A zynq board would be described using several
device tree snippets, one for the baked-in zynq-7000 peripheral set, one
for a generated description of what is in the PL, and one describing any
board-specific details (phy, etc). Something like below:
zynq-7000.dtsi : description of static zynq-7000 peripherals
/ {
amba : amba at 0 {
slcr: slcr at f8000000 {
clocks {
ps_clk : ps_clk {
compatible = "fixed-clock";
};
...
}
};
i2c0 : i2c0 at e0004000 {
compatible = "xlnx,i2cps";
...
};
eth0 : eth0 at e000b000 {
compatible = "xlnx,emacps";
...
};
};
};
zynq-zc702-pl.dtsi : generated description of what is in the PL
&amba {
/* PL IP generated descriptions here. */
};
zynq-zc702.dts : board-specific descriptions (osc freq, i2c, spi, phys, etc)
/include/ "zynq-7000.dtsi"
/include/ "zynq-zc702-pl.dtsi"
&ps_clk {
clock-frequency = <33333330>;
};
&i2c0 {
pca9548 at 74 {
...
reg = <0x74>;
...
};
};
ð0 {
phy at 7 {
compatible = "marvell,888e1116r";
...
reg = <0x7>;
};
};
Thoughts?
Thanks,
Josh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121105/261d059b/attachment-0001.sig>
From: Michal Simek <hidden> Date: 2012-11-07 12:06:01
2012/11/5 Josh Cartwright [off-list ref]:
On Sat, Oct 27, 2012 at 03:20:59PM +0000, Michal Simek wrote:
quoted
On Saturday, October 27, 2012 4:43 PM, Josh Cartwright wrote:
quoted
On Sat, Oct 27, 2012 at 02:06:45PM +0000, Michal Simek wrote:
[...]
quoted
I am not big fan to use dtsi solution because dts can be simple
generated directly From Xilinx design tool based on your hw design.
That's why I can't see any benefit To have dtsi file.
Can I ask you to reconsider?
I am open to all solution which will help others. I am not definitely
saying NO for this features I just haven't found a reason to support
it.
quoted
We, for example, don't make any use of the Xilinx
dev tools to generate our device trees.
Ok. How does your working flow looks like? I mean you don't get any
information from hardware guys how does your hw design look like?
Our usecase may admittedly be a bit weird, because what logic is in the
PL is ultimately determined (and even implemented) by the end user and
is loaded at runtime. There is a lot of machinery to make that happen,
but the point is that I don't have sufficient knowledge upfront to
generate appropriate bindings for what's in the PL.
ok. It means that you need to use just the part of DTS without PL logic at all.
Does it mean that PL will be connected with any DTS fragment?
quoted
quoted
Having a dtsi allows for easy extension of the zynq-7000 platform
for our boards, without having to carry duplicate data.
ok. I think that make sense if you send the next your series as RFC to
see how exactly you would like to use it.
It seems like you caught a glimpse of this in my COMMON_CLK patchset. :)
Yes. Just need to get some time to analyze it.
quoted
In my workflow we generate DTS directly from design tool which I
expect your hw guys use because it is probably needed to generate
boot.bin/fsbl/etc. Then there is one more additional step to setup
device-tree bsp to generate DTS which directly fits to your HW design.
If you have the same boards with different programmable logic I
understand that you would like to share that PS part and then just add
it that IPs in PL.
[..]
quoted
From my point of view. You have to use design tools at least once to
get bitstream and boot.bin with fsbl. Please correct me if I am wrong.
In this step you can use device-tree BSP to generate DTS ( I doesn't
need to be perfect with all attached devices on i2c,spi, phys, etc but
it reflects your hardware). You will get it in some seconds and your
dts has correct irq numbers/ip description, compatible strings,
addresses, position in the system - if you use bus bridges, etc) and
you can just directly use it and kernel will boot. If you need to do
changes in dts by hand, you can of course do it.
I wouldn't be as opposed to device tree generation if the device tree
generator was in tree.
Which tree do you exactly mean? Linux kernel or just any git tree?
Let me give you more information about the generator. It uses TCL in SDK
where it provides all structure from the system. It means device-tree generator
will read all information from design tool and based on that will generate
DTS file. It also means if user will setup specific irq lines in design, special
paramters setting in registers then all these values will be added to DTS.
.
Device tree bindings change, how would/could an
out-of-tree generator possibly handle changes in bindings?
What do you mean by that? Any example?
Generator will generate DTS on the current configuration which you have setup
in EDK.
For your case when you generate PL part at runtime you probably will use
just the part of DTS (that hard IPs)
Would a user
target the generator at a specific version of the kernel? (An in tree
generator would also seem to necessitate a more formal specification
language for DT bindings).
Device tree description in the hardware description which is not changing
across kernel versions. Xilinx device tree generator generates DTS
which reflects all standards.
We can send the generated output to device-tree mainling list and ask
for review.
For clk common patches this features is not implemented yet but it is easy
to add it and all generated DTSes will contains it.
If you think to label generator with specific version then we can use
any tag in the device-tree BSP tree.
It is odd to me that the use of a generator would be required to create
what is completely static data. What I'm referring to here is the
collection of peripherals on the zynq-7000 that are not in the PL. For
me, this requirement adds an unnecessary dependency on the Xilinx EDK
that I would like to avoid.
I am not saying that you need to use it. If you want to write your DTS
by hand, you still can
but I expect that the most of zynq users will use generator and
generate it because
it is just easier than to describe it by hand and they can be sure
that all parameters are
correctly generated.
If you are using any non-standard solution where you will load pl
logic at runtime
then you can use just generated DTS for hardblock or write it by hand.
Would it make sense to limit what the device tree generator produces to
only what is in the PL? (Forgive my ignorance about this tool, I've
never used it.)
It is tcl script everything is possible.
Are you interesting to use it just for PL?
This could just be an extension of what I've started to do with the
COMMON_CLK patchset. A zynq board would be described using several
device tree snippets, one for the baked-in zynq-7000 peripheral set, one
for a generated description of what is in the PL, and one describing any
board-specific details (phy, etc). Something like below:
zynq-7000.dtsi : description of static zynq-7000 peripherals
/ {
amba : amba at 0 {
slcr: slcr at f8000000 {
clocks {
ps_clk : ps_clk {
compatible = "fixed-clock";
};
...
}
};
i2c0 : i2c0 at e0004000 {
compatible = "xlnx,i2cps";
...
};
eth0 : eth0 at e000b000 {
compatible = "xlnx,emacps";
...
};
};
};
zynq-zc702-pl.dtsi : generated description of what is in the PL
&amba {
/* PL IP generated descriptions here. */
};
zynq-zc702.dts : board-specific descriptions (osc freq, i2c, spi, phys, etc)
/include/ "zynq-7000.dtsi"
/include/ "zynq-zc702-pl.dtsi"
&ps_clk {
clock-frequency = <33333330>;
};
&i2c0 {
pca9548 at 74 {
...
reg = <0x74>;
...
};
};
ð0 {
phy at 7 {
compatible = "marvell,888e1116r";
...
reg = <0x7>;
};
};
Thoughts?
The problem which I have with it is that if we accept this solution then
we will have to use 3(4-because of skeleton.dtsi) files where 2 files will fix
the origin model.
I can't see the reason why not to use just one file which will
describe it directly.
If you want to use solution with several dtsi files and compose it as
you describe
then it is completely fine but forcing others to use this structure
and write dts by hand will be big pain for a lot of users.
Also in design tools you can setup if you use qspi,nor,nand flash
memory interface.
memory interface, baudrates, dma, ports to PL logic, connections, etc.
and from my point of view is very complicated to describe it by
There are a lot of combination which you can have on one reference board.
You can't enable all hard IPs at one time and use all of them that's
why you shouldn't
list all of them in the kernel.
From my point of view make sense to have one DTS file in the kernel
and one defconfig
for the most popular zynq board where will be exactly written that this DTS is
connected to this reference hw design. If you want to get more reference design
go to this page and download it.
Adding all DTSes for zynq boards to the kernel is overkill.
If you want to use your hw design you can use this generator and
generate it or write it by
hand.
Thanks,
Michal
On Wed, Nov 07, 2012 at 01:05:57PM +0100, Michal Simek wrote:
2012/11/5 Josh Cartwright [off-list ref]:
[..]
quoted
Our usecase may admittedly be a bit weird, because what logic is in the
PL is ultimately determined (and even implemented) by the end user and
is loaded at runtime. There is a lot of machinery to make that happen,
but the point is that I don't have sufficient knowledge upfront to
generate appropriate bindings for what's in the PL.
ok. It means that you need to use just the part of DTS without PL logic at all.
Does it mean that PL will be connected with any DTS fragment?
Yes. For the time being, this is true. We have our own mechanisms for
enumerating IP at runtime.
quoted
quoted
quoted
Having a dtsi allows for easy extension of the zynq-7000
platform for our boards, without having to carry duplicate data.
ok. I think that make sense if you send the next your series as
RFC to see how exactly you would like to use it.
It seems like you caught a glimpse of this in my COMMON_CLK
patchset. :)
Yes. Just need to get some time to analyze it.
[..]
quoted
I wouldn't be as opposed to device tree generation if the device tree
generator was in tree.
Which tree do you exactly mean? Linux kernel or just any git tree?
No, I mean in the upstream Linux kernel tree. I don't think this is
likely to happen. My point here is that the generator necessarily has a
dependency on how the bindings are written. If those bindings change
(or new bindings are added), the generator must be updated to generate
device trees according to the new bindings.
I fail to see how these changes are handled with your generator.
Let me give you more information about the generator. It uses TCL in SDK
where it provides all structure from the system. It means device-tree generator
will read all information from design tool and based on that will generate
DTS file. It also means if user will setup specific irq lines in design, special
paramters setting in registers then all these values will be added to DTS.
quoted
Device tree bindings change, how would/could an out-of-tree
generator possibly handle changes in bindings?
What do you mean by that? Any example?
Yes, I have a real life example. In 3.2 (?), GIC bindings were added to
the kernel. It was necessary for us to update our board descriptions to
reflect the new #interrupt-cells = <3>; and figure out the appropriate
interrupt numbers (which differed from how they were specified before).
How would your generator have known whether or not I was targetting a
kernel with the GIC bindings, and appropriately generate the GIC node,
and generate interruptspecs for all children with #interrupt-cells = <3>?
Or, maybe another example: say clk bindings are added to the upstream
kernel, and I would like to use a kernel that contains them on my board.
Say this has all happened before Xilinx has even released a new version
of their SDK. How could I use your dts generator to output proper clk
nodes in my dts?
It seems the only way that Xilinx can possibly handle this is to tightly
couple the version of the kernel and their generator.
With increasing support for Zynq in the mainline kernel tree, it may
become more palatable for some existing users to switch to using the
upstream kernel instead of the Xilinx tree for their boards, and
coupling between the generator and target kernel version will be broken.
[..]
quoted
It is odd to me that the use of a generator would be required to create
what is completely static data. What I'm referring to here is the
collection of peripherals on the zynq-7000 that are not in the PL. For
me, this requirement adds an unnecessary dependency on the Xilinx EDK
that I would like to avoid.
I am not saying that you need to use it. If you want to write your DTS
by hand, you still can but I expect that the most of zynq users will
use generator and generate it because it is just easier than to
describe it by hand and they can be sure that all parameters are
correctly generated.
Again, you can only make this assurance _for a specific version of the
kernel_. If a user is not using the version of the kernel that came
with the SDK (and, maybe instead using a vanilla upstream kernel), all
bets are off.
If you are using any non-standard solution where you will load pl
logic at runtime then you can use just generated DTS for hardblock or
write it by hand.
I choose 'write it by hand'. I want what I write by hand to also be
useful to others by including the zynq-7000.dtsi in the upstream kernel.
[..]
If you want to use solution with several dtsi files and compose it as
you describe then it is completely fine but forcing others to use this
structure and write dts by hand will be big pain for a lot of users.
Using a composed model in the upstream kernel doesn't force anything
upon the existing users of your generator. They can still use whatever
gets spit out of your generator (assuming it generates nodes with
appropriate bindings). Unless I'm missing something here.
Also in design tools you can setup if you use qspi,nor,nand flash
memory interface.
memory interface, baudrates, dma, ports to PL logic, connections, etc.
and from my point of view is very complicated to describe it by
There are a lot of combination which you can have on one reference
board. You can't enable all hard IPs at one time and use all of them
that's why you shouldn't list all of them in the kernel.
I disagree with this. In my opinion, all of the "hard IPs" should be
described in the zynq-7000.dtsi, and those nodes which aren't available
explicitly disabled in the board-specific file.
From my point of view make sense to have one DTS file in the kernel
and one defconfig for the most popular zynq board where will be
exactly written that this DTS is connected to this reference hw
design. If you want to get more reference design go to this page and
download it. Adding all DTSes for zynq boards to the kernel is
overkill. If you want to use your hw design you can use this
generator and generate it or write it by hand.
All I'm asking for is for there to be a common zynq-7000.dtsi that
describes all of the static PS logic ("hard IPs") in the upstream kernel
source that I can include in my own (hand maintained) board
descriptions. It would be nice if there was an example of its use, like
with a zc702 board file also upstream, but it is not really important to
me.
I do not want a dependency on the EDK.
My request does not sound unreasonable to me and is what other platforms
are doing.
Josh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121107/84878c5b/attachment.sig>
From: John Linn <hidden> Date: 2012-11-08 00:38:42
-----Original Message-----
From: Josh Cartwright [mailto:josh.cartwright at ni.com]
Sent: Wednesday, November 07, 2012 6:18 AM
To: Michal Simek
Cc: arm at kernel.org; Arnd Bergmann; linux-kernel at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
John Linn; Nick Bowler
Subject: Re: [PATCH v4 1/5] zynq: use GIC device tree bindings
On Wed, Nov 07, 2012 at 01:05:57PM +0100, Michal Simek wrote:
quoted
2012/11/5 Josh Cartwright [off-list ref]:
[..]
quoted
quoted
Our usecase may admittedly be a bit weird, because what logic is in the
PL is ultimately determined (and even implemented) by the end user and
is loaded at runtime. There is a lot of machinery to make that happen,
but the point is that I don't have sufficient knowledge upfront to
generate appropriate bindings for what's in the PL.
ok. It means that you need to use just the part of DTS without PL logic at all.
Does it mean that PL will be connected with any DTS fragment?
Yes. For the time being, this is true. We have our own mechanisms for
enumerating IP at runtime.
quoted
quoted
quoted
quoted
Having a dtsi allows for easy extension of the zynq-7000
platform for our boards, without having to carry duplicate data.
ok. I think that make sense if you send the next your series as
RFC to see how exactly you would like to use it.
It seems like you caught a glimpse of this in my COMMON_CLK
patchset. :)
Yes. Just need to get some time to analyze it.
[..]
quoted
quoted
I wouldn't be as opposed to device tree generation if the device tree
generator was in tree.
Which tree do you exactly mean? Linux kernel or just any git tree?
No, I mean in the upstream Linux kernel tree. I don't think this is
likely to happen. My point here is that the generator necessarily has a
dependency on how the bindings are written. If those bindings change
(or new bindings are added), the generator must be updated to generate
device trees according to the new bindings.
I fail to see how these changes are handled with your generator.
quoted
Let me give you more information about the generator. It uses TCL in SDK
where it provides all structure from the system. It means device-tree generator
will read all information from design tool and based on that will generate
DTS file. It also means if user will setup specific irq lines in design, special
paramters setting in registers then all these values will be added to DTS.
quoted
Device tree bindings change, how would/could an out-of-tree
generator possibly handle changes in bindings?
What do you mean by that? Any example?
Yes, I have a real life example. In 3.2 (?), GIC bindings were added to
the kernel. It was necessary for us to update our board descriptions to
reflect the new #interrupt-cells = <3>; and figure out the appropriate
interrupt numbers (which differed from how they were specified before).
How would your generator have known whether or not I was targetting a
kernel with the GIC bindings, and appropriately generate the GIC node,
and generate interruptspecs for all children with #interrupt-cells = <3>?
Hi Josh,
Yes we see this problem coming too.
Or, maybe another example: say clk bindings are added to the upstream
kernel, and I would like to use a kernel that contains them on my board.
Say this has all happened before Xilinx has even released a new version
of their SDK. How could I use your dts generator to output proper clk
nodes in my dts?
It seems the only way that Xilinx can possibly handle this is to tightly
couple the version of the kernel and their generator.
With increasing support for Zynq in the mainline kernel tree, it may
become more palatable for some existing users to switch to using the
upstream kernel instead of the Xilinx tree for their boards, and
coupling between the generator and target kernel version will be broken.
[..]
quoted
quoted
It is odd to me that the use of a generator would be required to create
what is completely static data. What I'm referring to here is the
collection of peripherals on the zynq-7000 that are not in the PL. For
me, this requirement adds an unnecessary dependency on the Xilinx EDK
that I would like to avoid.
I am not saying that you need to use it. If you want to write your DTS
by hand, you still can but I expect that the most of zynq users will
use generator and generate it because it is just easier than to
describe it by hand and they can be sure that all parameters are
correctly generated.
Again, you can only make this assurance _for a specific version of the
kernel_. If a user is not using the version of the kernel that came
with the SDK (and, maybe instead using a vanilla upstream kernel), all
bets are off.
quoted
If you are using any non-standard solution where you will load pl
logic at runtime then you can use just generated DTS for hardblock or
write it by hand.
I choose 'write it by hand'. I want what I write by hand to also be
useful to others by including the zynq-7000.dtsi in the upstream kernel.
Yes agreed.
[..]
quoted
If you want to use solution with several dtsi files and compose it as
you describe then it is completely fine but forcing others to use this
structure and write dts by hand will be big pain for a lot of users.
Using a composed model in the upstream kernel doesn't force anything
upon the existing users of your generator. They can still use whatever
gets spit out of your generator (assuming it generates nodes with
appropriate bindings). Unless I'm missing something here.
quoted
Also in design tools you can setup if you use qspi,nor,nand flash
memory interface.
memory interface, baudrates, dma, ports to PL logic, connections, etc.
and from my point of view is very complicated to describe it by
There are a lot of combination which you can have on one reference
board. You can't enable all hard IPs at one time and use all of them
that's why you shouldn't list all of them in the kernel.
I disagree with this. In my opinion, all of the "hard IPs" should be
described in the zynq-7000.dtsi, and those nodes which aren't available
explicitly disabled in the board-specific file.
It looks like most everyone is doing this in their device trees.
quoted
From my point of view make sense to have one DTS file in the kernel
and one defconfig for the most popular zynq board where will be
exactly written that this DTS is connected to this reference hw
design. If you want to get more reference design go to this page and
download it. Adding all DTSes for zynq boards to the kernel is
overkill. If you want to use your hw design you can use this
generator and generate it or write it by hand.
All I'm asking for is for there to be a common zynq-7000.dtsi that
describes all of the static PS logic ("hard IPs") in the upstream kernel
source that I can include in my own (hand maintained) board
descriptions. It would be nice if there was an example of its use, like
with a zc702 board file also upstream, but it is not really important to
me.
Yes I understand and see the benefits too.
I do not want a dependency on the EDK.
Understood and it makes sense what you're saying.
My request does not sound unreasonable to me and is what other platforms
are doing.
Agreed. We'll wrestle with this some more within Xilinx.
Thanks,
John