2012-10-24 00:32:28

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH v3 0/5] zynq subarch cleanups

Hey all-

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 mappings 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.

Arnd-

I intentionally did not Cc stable on patch 5, even though you had
suggested otherwise. I do not think it will apply cleanly to the stable
trees independent of the other patches. Additionally, with the current
state of zynq upstream, I'm not convinced there would be enough users to
make it worth the effort.

Additionally, I've left the SCU static mapping around, even though its
currently unused. We'll eventually need it around (maybe in a different form)
when SMP support is added.

---
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 [email protected] 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/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 ++++++++++++---------------
6 files changed, 38 insertions(+), 66 deletions(-)
delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h

--
1.8.0


2012-10-24 00:34:03

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH v3 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 <[email protected]>
Cc: John Linn <[email protected]>
---
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
--- a/arch/arm/boot/dts/zynq-ep107.dts
+++ b/arch/arm/boot/dts/zynq-ep107.dts
@@ -36,10 +36,12 @@
ranges;

intc: interrupt-controller@f8f01000 {
+ compatible = "arm,cortex-a9-gic";
+ #interrupt-cells = <3>;
+ #address-cells = <1>;
interrupt-controller;
- compatible = "arm,gic";
- reg = <0xF8F01000 0x1000>;
- #interrupt-cells = <2>;
+ reg = <0xF8F01000 0x1000>,
+ <0xF8F00100 0x100>;
};

uart0: uart@e0000000 {
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index ab5cfdd..d73963b 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -55,12 +55,17 @@ static void __init xilinx_init_machine(void)
of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
}

+static struct of_device_id irq_match[] __initdata = {
+ { .compatible = "arm,cortex-a9-gic", .data = gic_of_init, },
+ { }
+};
+
/**
* xilinx_irq_init() - Interrupt controller initialization for the GIC.
*/
static void __init xilinx_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
diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
index d0d3f8f..3d1c6a6 100644
--- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
+++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
@@ -35,8 +35,6 @@

#define TTC0_BASE IOMEM(TTC0_VIRT)
#define SCU_PERIPH_BASE IOMEM(SCU_PERIPH_VIRT)
-#define SCU_GIC_CPU_BASE (SCU_PERIPH_BASE + 0x100)
-#define SCU_GIC_DIST_BASE (SCU_PERIPH_BASE + 0x1000)
#define PL310_L2CC_BASE IOMEM(PL310_L2CC_VIRT)

/*
--
1.8.0

2012-10-24 00:34:21

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH v3 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 <[email protected]>
Cc: John Linn <[email protected]>
---
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(-)

diff --git a/arch/arm/boot/dts/zynq-ep107.dts b/arch/arm/boot/dts/zynq-ep107.dts
index 7bfff4a..87204d7 100644
--- a/arch/arm/boot/dts/zynq-ep107.dts
+++ b/arch/arm/boot/dts/zynq-ep107.dts
@@ -44,6 +44,15 @@
<0xF8F00100 0x100>;
};

+ L2: cache-controller {
+ compatible = "arm,pl310-cache";
+ reg = <0xF8F02000 0x1000>;
+ arm,data-latency = <2 3 2>;
+ arm,tag-latency = <2 3 2>;
+ cache-unified;
+ cache-level = <2>;
+ };
+
uart0: uart@e0000000 {
compatible = "xlnx,xuartps";
reg = <0xE0000000 0x1000>;
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index d73963b..056091a 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -45,12 +45,10 @@ static struct of_device_id zynq_of_bus_ids[] __initdata = {
*/
static void __init xilinx_init_machine(void)
{
-#ifdef CONFIG_CACHE_L2X0
/*
* 64KB way size, 8-way associativity, parity disabled
*/
- l2x0_init(PL310_L2CC_BASE, 0x02060000, 0xF0F0FFFF);
-#endif
+ l2x0_of_init(0x02060000, 0xF0F0FFFF);

of_platform_bus_probe(NULL, zynq_of_bus_ids, NULL);
}
@@ -83,11 +81,6 @@ static struct map_desc io_desc[] __initdata = {
.pfn = __phys_to_pfn(SCU_PERIPH_PHYS),
.length = SZ_8K,
.type = MT_DEVICE,
- }, {
- .virtual = PL310_L2CC_VIRT,
- .pfn = __phys_to_pfn(PL310_L2CC_PHYS),
- .length = SZ_4K,
- .type = MT_DEVICE,
},

#ifdef CONFIG_DEBUG_LL
diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
index 3d1c6a6..218283a 100644
--- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
+++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
@@ -25,9 +25,6 @@
#define TTC0_PHYS 0xF8001000
#define TTC0_VIRT TTC0_PHYS

-#define PL310_L2CC_PHYS 0xF8F02000
-#define PL310_L2CC_VIRT PL310_L2CC_PHYS
-
#define SCU_PERIPH_PHYS 0xF8F00000
#define SCU_PERIPH_VIRT SCU_PERIPH_PHYS

@@ -35,7 +32,6 @@

#define TTC0_BASE IOMEM(TTC0_VIRT)
#define SCU_PERIPH_BASE IOMEM(SCU_PERIPH_VIRT)
-#define PL310_L2CC_BASE IOMEM(PL310_L2CC_VIRT)

/*
* Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
--
1.8.0

2012-10-24 00:34:48

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH v3 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

Signed-off-by: Josh Cartwright <[email protected]>
Cc: John Linn <[email protected]>
---
arch/arm/Kconfig | 1 -
arch/arm/mach-zynq/common.c | 1 -
arch/arm/mach-zynq/include/mach/clkdev.h | 32 --------------------------------
3 files changed, 34 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
@@ -946,7 +946,6 @@ config ARCH_ZYNQ
bool "Xilinx Zynq ARM Cortex A9 Platform"
select ARM_AMBA
select ARM_GIC
- select CLKDEV_LOOKUP
select CPU_V7
select GENERIC_CLOCKEVENTS
select ICST
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index 056091a..ba48f06 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -31,7 +31,6 @@
#include <asm/hardware/cache-l2x0.h>

#include <mach/zynq_soc.h>
-#include <mach/clkdev.h>
#include "common.h"

static struct of_device_id zynq_of_bus_ids[] __initdata = {
diff --git a/arch/arm/mach-zynq/include/mach/clkdev.h b/arch/arm/mach-zynq/include/mach/clkdev.h
deleted file mode 100644
index c6e73d8..0000000
--- a/arch/arm/mach-zynq/include/mach/clkdev.h
+++ /dev/null
@@ -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
--
1.8.0

2012-10-24 00:35:08

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH v3 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 <[email protected]>
---
arch/arm/include/asm/pgtable.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 08c1231..72904a2 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -40,7 +40,7 @@
*/
#define VMALLOC_OFFSET (8*1024*1024)
#define VMALLOC_START (((unsigned long)high_memory + VMALLOC_OFFSET) & ~(VMALLOC_OFFSET-1))
-#define VMALLOC_END 0xff000000UL
+#define VMALLOC_END _AC(0xff000000,UL)

#define LIBRARY_TEXT_START 0x0c000000

--
1.8.0

2012-10-24 00:35:29

by Josh Cartwright

[permalink] [raw]
Subject: [PATCH v3 5/5] zynq: move static peripheral mappings

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 <[email protected]>
Cc: John Linn <[email protected]>
---
arch/arm/mach-zynq/common.c | 6 +++---
arch/arm/mach-zynq/include/mach/zynq_soc.h | 23 +++++++++++++----------
2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index ba48f06..ba8d14f 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -73,12 +73,12 @@ static struct map_desc io_desc[] __initdata = {
{
.virtual = TTC0_VIRT,
.pfn = __phys_to_pfn(TTC0_PHYS),
- .length = SZ_4K,
+ .length = TTC0_SIZE,
.type = MT_DEVICE,
}, {
.virtual = SCU_PERIPH_VIRT,
.pfn = __phys_to_pfn(SCU_PERIPH_PHYS),
- .length = SZ_8K,
+ .length = SCU_PERIPH_SIZE,
.type = MT_DEVICE,
},

@@ -86,7 +86,7 @@ static struct map_desc io_desc[] __initdata = {
{
.virtual = UART0_VIRT,
.pfn = __phys_to_pfn(UART0_PHYS),
- .length = SZ_4K,
+ .length = UART0_SIZE,
.type = MT_DEVICE,
},
#endif
diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
index 218283a..c6b9b67 100644
--- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
+++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
@@ -15,27 +15,30 @@
#ifndef __MACH_XILINX_SOC_H__
#define __MACH_XILINX_SOC_H__

+#include <asm/pgtable.h>
+
#define PERIPHERAL_CLOCK_RATE 2500000

-/* 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)

-#define TTC0_PHYS 0xF8001000
-#define TTC0_VIRT TTC0_PHYS
+#define TTC0_PHYS 0xF8001000
+#define TTC0_SIZE SZ_4K
+#define TTC0_VIRT (UART0_VIRT - TTC0_SIZE)

-#define SCU_PERIPH_PHYS 0xF8F00000
-#define SCU_PERIPH_VIRT SCU_PERIPH_PHYS
+#define SCU_PERIPH_PHYS 0xF8F00000
+#define SCU_PERIPH_SIZE SZ_8K
+#define SCU_PERIPH_VIRT (TTC0_VIRT - SCU_PERIPH_SIZE)

/* The following are intended for the devices that are mapped early */

#define TTC0_BASE IOMEM(TTC0_VIRT)
#define SCU_PERIPH_BASE IOMEM(SCU_PERIPH_VIRT)

-/*
- * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
- */
#define LL_UART_PADDR UART0_PHYS
#define LL_UART_VADDR UART0_VIRT

--
1.8.0

2012-10-24 12:04:09

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 1/5] zynq: use GIC device tree bindings

On Wednesday 24 October 2012, Josh Cartwright wrote:
> The Zynq uses the cortex-a9-gic. This eliminates the need to hardcode
> register addresses.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> Cc: John Linn <[email protected]>

Acked-by: Arnd Bergmann <[email protected]>

2012-10-24 12:04:32

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] zynq: use pl310 device tree bindings

On Wednesday 24 October 2012, Josh Cartwright wrote:
> The Zynq has a PL310 L2 cache controller. Convert in-tree uses to using
> the device tree.
>
> Signed-off-by: Josh Cartwright <[email protected]>
> Cc: John Linn <[email protected]>

Acked-by: Arnd Bergmann <[email protected]>

2012-10-24 12:05:50

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] zynq: remove use of CLKDEV_LOOKUP

On Wednesday 24 October 2012, Josh Cartwright wrote:
> 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
>
> Signed-off-by: Josh Cartwright <[email protected]>
> Cc: John Linn <[email protected]>

Acked-by: Arnd Bergmann <[email protected]>

I think I forgot to mention in the previous review round that it would be nice
to just enable CONFIG_COMMON_CLK right away. Not important though.

Arnd

2012-10-24 12:08:06

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 4/5] ARM: annotate VMALLOC_END definition with _AC

On Wednesday 24 October 2012, Josh Cartwright wrote:
> 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 <[email protected]>

Acked-by: Arnd Bergmann <[email protected]>

2012-10-24 12:08:41

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] zynq: move static peripheral mappings

On Wednesday 24 October 2012, 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:
>
> 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 <[email protected]>
> Cc: John Linn <[email protected]>

Acked-by: Arnd Bergmann <[email protected]>

2012-10-24 12:10:03

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v3 0/5] zynq subarch cleanups

On Wednesday 24 October 2012, Josh Cartwright wrote:
> 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 mappings 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.

Looks all good to me now.

>
> I intentionally did not Cc stable on patch 5, even though you had
> suggested otherwise. I do not think it will apply cleanly to the stable
> trees independent of the other patches. Additionally, with the current
> state of zynq upstream, I'm not convinced there would be enough users to
> make it worth the effort.

Ok, fair enough.

> Additionally, I've left the SCU static mapping around, even though its
> currently unused. We'll eventually need it around (maybe in a different form)
> when SMP support is added.

Right.

John, are you going to pick up these patches and send a pull request?

Arnd

2012-10-24 13:32:35

by Nick Bowler

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] zynq: remove use of CLKDEV_LOOKUP

On 2012-10-23 19:34 -0500, Josh Cartwright wrote:
> 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'

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

> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index cce4f8d..de70d99 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -946,7 +946,6 @@ config ARCH_ZYNQ
> bool "Xilinx Zynq ARM Cortex A9 Platform"
> select ARM_AMBA
> select ARM_GIC
> - select CLKDEV_LOOKUP
> select CPU_V7
> select GENERIC_CLOCKEVENTS
> select ICST

I'd prefer if we just added "select COMMON_CLK" instead of removing this
so we don't have to re-add this later, but I guess it doesn't really
matter either way.

Cheers,
--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

2012-10-24 18:17:08

by Josh Cartwright

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] zynq: remove use of CLKDEV_LOOKUP

On Wed, Oct 24, 2012 at 09:32:32AM -0400, Nick Bowler wrote:
> On 2012-10-23 19:34 -0500, Josh Cartwright wrote:
> > 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'
>
> 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.

Yes, indeed. I did try to fix my problems by having ARCH_ZYNQ select
PLAT_VERSATILE_CLOCK, but I recall running into additional build
problems...

But now that I just tried it again, it all seems to work, barring
Kconfig complaining I've selected PLAT_VERSATILE_CLOCK, but not
PLAT_VERSATILE. (Having ARCH_ZYNQ select PLAT_VERSATILE, however, leads
to additional build problems).

> 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

Yes, indeed it should.

Thanks,

Josh


Attachments:
(No filename) (1.57 kB)
(No filename) (836.00 B)
Download all attachments