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. It removes some unused clock infrastructure,
adds DT support for the GIC, and moves around peripheral mappings.
On the CLKDEV_LOOKUP removal: 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.
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 (4):
ARM: annotate VMALLOC_END definition with _AC
zynq: move static peripheral mappings
zynq: use GIC device tree bindings
zynq: remove use of CLKDEV_LOOKUP
arch/arm/Kconfig | 1 -
arch/arm/boot/dts/zynq-ep107.dts | 8 ++++---
arch/arm/include/asm/pgtable.h | 2 +-
arch/arm/mach-zynq/common.c | 16 ++++++++-----
arch/arm/mach-zynq/include/mach/clkdev.h | 32 --------------------------
arch/arm/mach-zynq/include/mach/zynq_soc.h | 36 ++++++++++++++++--------------
6 files changed, 35 insertions(+), 60 deletions(-)
delete mode 100644 arch/arm/mach-zynq/include/mach/clkdev.h
--
1.8.0
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]>
---
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 e4e422b..e10268c 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
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
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]>
---
arch/arm/mach-zynq/common.c | 8 +++----
arch/arm/mach-zynq/include/mach/zynq_soc.h | 38 +++++++++++++++++-------------
2 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/arch/arm/mach-zynq/common.c b/arch/arm/mach-zynq/common.c
index ab5cfdd..b33f12f 100644
--- a/arch/arm/mach-zynq/common.c
+++ b/arch/arm/mach-zynq/common.c
@@ -71,17 +71,17 @@ 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,
}, {
.virtual = PL310_L2CC_VIRT,
.pfn = __phys_to_pfn(PL310_L2CC_PHYS),
- .length = SZ_4K,
+ .length = PL310_L2CC_SIZE,
.type = MT_DEVICE,
},
@@ -89,7 +89,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 d0d3f8f..ae3b236 100644
--- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
+++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
@@ -15,33 +15,37 @@
#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 PL310_L2CC_PHYS 0xF8F02000
-#define PL310_L2CC_VIRT PL310_L2CC_PHYS
+#define PL310_L2CC_PHYS 0xF8F02000
+#define PL310_L2CC_SIZE SZ_4K
+#define PL310_L2CC_VIRT (TTC0_VIRT - PL310_L2CC_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 (PL310_L2CC_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)
-#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)
+#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)
-/*
- * 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
The Zynq uses the cortex-a9-gic. This eliminates the need to hardcode
register addresses.
Signed-off-by: Josh Cartwright <[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 b33f12f..e4e422b 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 ae3b236..9156914 100644
--- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
+++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
@@ -42,8 +42,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)
#define LL_UART_PADDR UART0_PHYS
--
1.8.0
On Monday 22 October 2012, Josh Cartwright wrote:
> 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. It removes some unused clock infrastructure,
> adds DT support for the GIC, and moves around peripheral mappings.
Ok, thanks for these patches. As we have moved on for some of these issues,
I'll comment on how to go even further. On a global scale, I wonder
if there are any obstacles for enabling CONFIG_MULTIPLATFORM on Zynq
as we have done for most of the other recently added platform.
Arnd
On Monday 22 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]>
This looks like a bug fix that should be backported to older kernels,
so it would be good to add 'Cc: [email protected]' below your
Signed-off-by.
> diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
> index d0d3f8f..ae3b236 100644
> --- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
> +++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
> @@ -15,33 +15,37 @@
> #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)
There are plans to move the uart location into a fixed virtual
address in the future, but it hasn't been decided yet.
It will still need a fixed mapping though, just to a different
address.
> -#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)
It's quite likely that this does not have to be a fixed mapping
any more. Just have a look at how drivers/clocksource/dw_apb_timer_of.c
calls of_iomap() to get the address.
> -#define PL310_L2CC_PHYS 0xF8F02000
> -#define PL310_L2CC_VIRT PL310_L2CC_PHYS
> +#define PL310_L2CC_PHYS 0xF8F02000
> +#define PL310_L2CC_SIZE SZ_4K
> +#define PL310_L2CC_VIRT (TTC0_VIRT - PL310_L2CC_SIZE)
This address would not need a fixed mapping by calling l2x0_of_init
rather than l2x0_init.
> -#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 (PL310_L2CC_VIRT - SCU_PERIPH_SIZE)
And your patch 3 already obsoletes this mapping.
Arnd
Hey Arnd-
Thanks for the review/suggestions.
On Tue, Oct 23, 2012 at 02:50:11PM +0000, Arnd Bergmann wrote:
> On Monday 22 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]>
>
> This looks like a bug fix that should be backported to older kernels,
> so it would be good to add 'Cc: [email protected]' below your
> Signed-off-by.
Will-do, thanks.
> > diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
> > index d0d3f8f..ae3b236 100644
> > --- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
> > +++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
> > @@ -15,33 +15,37 @@
> > #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)
>
> There are plans to move the uart location into a fixed virtual
> address in the future, but it hasn't been decided yet.
> It will still need a fixed mapping though, just to a different
> address.
>
> > -#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)
>
> It's quite likely that this does not have to be a fixed mapping
> any more. Just have a look at how drivers/clocksource/dw_apb_timer_of.c
> calls of_iomap() to get the address.
Yes, this is already on my list of plans. The in-tree TTC driver
unfortunately doesn't yet support device tree bindings. Are you
comfortable waiting on the DT-ification of the TTC in a follow-up
patchset?
> > -#define PL310_L2CC_PHYS 0xF8F02000
> > -#define PL310_L2CC_VIRT PL310_L2CC_PHYS
> > +#define PL310_L2CC_PHYS 0xF8F02000
> > +#define PL310_L2CC_SIZE SZ_4K
> > +#define PL310_L2CC_VIRT (TTC0_VIRT - PL310_L2CC_SIZE)
>
> This address would not need a fixed mapping by calling l2x0_of_init
> rather than l2x0_init.
Great, I'll take care of this.
> > -#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 (PL310_L2CC_VIRT - SCU_PERIPH_SIZE)
>
> And your patch 3 already obsoletes this mapping.
I'll spin up a reordered patchset to eliminate this odd state.
Thanks again,
Josh
On Tuesday 23 October 2012, Josh Cartwright wrote:
> On Tue, Oct 23, 2012 at 02:50:11PM +0000, Arnd Bergmann wrote:
> > On Monday 22 October 2012, Josh Cartwright wrote:
>
> > > diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
> > > index d0d3f8f..ae3b236 100644
> > > --- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
> > > +++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
> > > -#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)
> >
> > It's quite likely that this does not have to be a fixed mapping
> > any more. Just have a look at how drivers/clocksource/dw_apb_timer_of.c
> > calls of_iomap() to get the address.
>
> Yes, this is already on my list of plans. The in-tree TTC driver
> unfortunately doesn't yet support device tree bindings. Are you
> comfortable waiting on the DT-ification of the TTC in a follow-up
> patchset?
Yes, that's fine. If you do that, you can also move the driver to
drivers/clocksource in another patch.
Arnd
On 10/23/2012 09:50 AM, Arnd Bergmann wrote:
> On Monday 22 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]>
>
> This looks like a bug fix that should be backported to older kernels,
> so it would be good to add 'Cc: [email protected]' below your
> Signed-off-by.
>
>
>> diff --git a/arch/arm/mach-zynq/include/mach/zynq_soc.h b/arch/arm/mach-zynq/include/mach/zynq_soc.h
>> index d0d3f8f..ae3b236 100644
>> --- a/arch/arm/mach-zynq/include/mach/zynq_soc.h
>> +++ b/arch/arm/mach-zynq/include/mach/zynq_soc.h
>> @@ -15,33 +15,37 @@
>> #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)
>
> There are plans to move the uart location into a fixed virtual
> address in the future, but it hasn't been decided yet.
> It will still need a fixed mapping though, just to a different
> address.
>
>> -#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)
>
> It's quite likely that this does not have to be a fixed mapping
> any more. Just have a look at how drivers/clocksource/dw_apb_timer_of.c
> calls of_iomap() to get the address.
>
>> -#define PL310_L2CC_PHYS 0xF8F02000
>> -#define PL310_L2CC_VIRT PL310_L2CC_PHYS
>> +#define PL310_L2CC_PHYS 0xF8F02000
>> +#define PL310_L2CC_SIZE SZ_4K
>> +#define PL310_L2CC_VIRT (TTC0_VIRT - PL310_L2CC_SIZE)
>
> This address would not need a fixed mapping by calling l2x0_of_init
> rather than l2x0_init.
>
>> -#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 (PL310_L2CC_VIRT - SCU_PERIPH_SIZE)
>
> And your patch 3 already obsoletes this mapping.
Actually, it's probably still needed. The smp platform code typically
reads the number of cores from the SCU and the mapping has to be in
place before ioremap is up. I don't think there is an architected way to
get the number of cores, but it would be nice to avoid this early SCU
access. We could also mandate getting the core count from DT instead.
Also, the physical address can be read with this on A9's:
asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
Rob
On 10/23/2012 09:41 AM, Arnd Bergmann wrote:
> On Monday 22 October 2012, Josh Cartwright wrote:
>> 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. It removes some unused clock infrastructure,
>> adds DT support for the GIC, and moves around peripheral mappings.
>
> Ok, thanks for these patches. As we have moved on for some of these issues,
> I'll comment on how to go even further. On a global scale, I wonder
> if there are any obstacles for enabling CONFIG_MULTIPLATFORM on Zynq
> as we have done for most of the other recently added platform.
IIRC, the only thing I saw preventing it was the custom clk usage.
Rob
On Tue, Oct 23, 2012 at 03:09:23PM -0500, Rob Herring wrote:
> On 10/23/2012 09:50 AM, Arnd Bergmann wrote:
> > On Monday 22 October 2012, Josh Cartwright wrote:
> >> -#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 (PL310_L2CC_VIRT - SCU_PERIPH_SIZE)
> >
> > And your patch 3 already obsoletes this mapping.
>
> Actually, it's probably still needed. The smp platform code typically
> reads the number of cores from the SCU and the mapping has to be in
> place before ioremap is up. I don't think there is an architected way to
> get the number of cores, but it would be nice to avoid this early SCU
> access. We could also mandate getting the core count from DT instead.
>
> Also, the physical address can be read with this on A9's:
>
> asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
For the sake of the zynq cleanups, I think it may still make sense to
remove the SCU peripheral mappings for now. By the time we're ready to
push in SMP support for zynq, maybe we can tackle the problem of how to
solve the SCU mapping problem generically.
If we're already considering a architecture-agnostic way to maintain the
early uart mapping, would it make sense to handle this in a similar way?
Thanks,
Josh
On Tue, Oct 23, 2012 at 05:17:42PM -0400, Nick Bowler wrote:
> On 2012-10-23 15:53 -0500, Josh Cartwright wrote:
> > On Tue, Oct 23, 2012 at 03:09:23PM -0500, Rob Herring wrote:
> > > On 10/23/2012 09:50 AM, Arnd Bergmann wrote:
> > > > On Monday 22 October 2012, Josh Cartwright wrote:
> > > >> -#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 (PL310_L2CC_VIRT - SCU_PERIPH_SIZE)
> > > >
> > > > And your patch 3 already obsoletes this mapping.
> > >
> > > Actually, it's probably still needed. The smp platform code typically
> > > reads the number of cores from the SCU and the mapping has to be in
> > > place before ioremap is up. I don't think there is an architected way to
> > > get the number of cores, but it would be nice to avoid this early SCU
> > > access. We could also mandate getting the core count from DT instead.
> > >
> > > Also, the physical address can be read with this on A9's:
> > >
> > > asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
> >
> > For the sake of the zynq cleanups, I think it may still make sense to
> > remove the SCU peripheral mappings for now. By the time we're ready to
> > push in SMP support for zynq, maybe we can tackle the problem of how to
> > solve the SCU mapping problem generically.
>
> Then the static mapping can be removed if and when the we "solve the SCU
> mapping problem generically". There's no point in removing it until
> then since it doesn't cause any actual problems, does it?
That's also fine with me as well. I'm not strongly opinionated, and not
convinced it really matters much.
Thanks,
Josh
On Tue, Oct 23, 2012 at 04:27:03PM -0400, Nick Bowler wrote:
>
> Just FYI, I sent a patch to fix the same bug a while back
>
> https://patchwork.kernel.org/patch/1156361/
>
> together with other patches to fix early printk on the ZC702 serial
> console. Admittedly, I dropped the ball on these as other issues
> came up so I was away from the Zynq for a while.
>
> However, I'm now getting back on the Zynq and have a bunch of patches to
> make it all work on the ZC702 board. I've respun the ZC702 early boot
> fixes against newer git but they're obviously going to conflict with
> this series. Should I resend them anyway?
If you have other fixes for the zc702, that'd be great. Most of my
testing has been in a qemu model; I haven't had a chance to try getting
the zc702 booting yet.
The first stumbling block is that it looks like the secondary uart is
the primary uart on the zc702.
> I also have a DT binding for the TTC driver, I can send that.
That'd be great!
Thanks,
Josh
On 2012-10-23 11:26 -0500, Josh Cartwright wrote:
> On Tue, Oct 23, 2012 at 02:50:11PM +0000, Arnd Bergmann wrote:
> > On Monday 22 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]>
> >
> > This looks like a bug fix that should be backported to older kernels,
> > so it would be good to add 'Cc: [email protected]' below your
> > Signed-off-by.
>
> Will-do, thanks.
Just FYI, I sent a patch to fix the same bug a while back
https://patchwork.kernel.org/patch/1156361/
together with other patches to fix early printk on the ZC702 serial
console. Admittedly, I dropped the ball on these as other issues
came up so I was away from the Zynq for a while.
However, I'm now getting back on the Zynq and have a bunch of patches to
make it all work on the ZC702 board. I've respun the ZC702 early boot
fixes against newer git but they're obviously going to conflict with
this series. Should I resend them anyway?
> > > -#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)
> >
> > It's quite likely that this does not have to be a fixed mapping
> > any more. Just have a look at how drivers/clocksource/dw_apb_timer_of.c
> > calls of_iomap() to get the address.
>
> Yes, this is already on my list of plans. The in-tree TTC driver
> unfortunately doesn't yet support device tree bindings. Are you
> comfortable waiting on the DT-ification of the TTC in a follow-up
> patchset?
I also have a DT binding for the TTC driver, I can send that.
Cheers,
--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)
On 2012-10-23 15:53 -0500, Josh Cartwright wrote:
> On Tue, Oct 23, 2012 at 03:09:23PM -0500, Rob Herring wrote:
> > On 10/23/2012 09:50 AM, Arnd Bergmann wrote:
> > > On Monday 22 October 2012, Josh Cartwright wrote:
> > >> -#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 (PL310_L2CC_VIRT - SCU_PERIPH_SIZE)
> > >
> > > And your patch 3 already obsoletes this mapping.
> >
> > Actually, it's probably still needed. The smp platform code typically
> > reads the number of cores from the SCU and the mapping has to be in
> > place before ioremap is up. I don't think there is an architected way to
> > get the number of cores, but it would be nice to avoid this early SCU
> > access. We could also mandate getting the core count from DT instead.
> >
> > Also, the physical address can be read with this on A9's:
> >
> > asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (base));
>
> For the sake of the zynq cleanups, I think it may still make sense to
> remove the SCU peripheral mappings for now. By the time we're ready to
> push in SMP support for zynq, maybe we can tackle the problem of how to
> solve the SCU mapping problem generically.
Then the static mapping can be removed if and when the we "solve the SCU
mapping problem generically". There's no point in removing it until
then since it doesn't cause any actual problems, does it?
Cheers,
--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)
On 2012-10-23 18:42 -0500, Josh Cartwright wrote:
> On Tue, Oct 23, 2012 at 04:27:03PM -0400, Nick Bowler wrote:
> > Just FYI, I sent a patch to fix the same bug a while back
> >
> > https://patchwork.kernel.org/patch/1156361/
> >
> > together with other patches to fix early printk on the ZC702 serial
> > console. Admittedly, I dropped the ball on these as other issues
> > came up so I was away from the Zynq for a while.
> >
> > However, I'm now getting back on the Zynq and have a bunch of patches to
> > make it all work on the ZC702 board. I've respun the ZC702 early boot
> > fixes against newer git but they're obviously going to conflict with
> > this series. Should I resend them anyway?
>
> If you have other fixes for the zc702, that'd be great. Most of my
> testing has been in a qemu model; I haven't had a chance to try getting
> the zc702 booting yet.
>
> The first stumbling block is that it looks like the secondary uart is
> the primary uart on the zc702.
Yes, that is indeed the case, and was what I tried to address with my
earlier patches.
> > I also have a DT binding for the TTC driver, I can send that.
>
> That'd be great!
OK, I will respin and test this stuff on top of your v4 series and send
them out.
Cheers,
--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)