Skip to contentSkip to conversation

ARM: Xilinx: Adding Xilinx platform infrastructure support

PATCH New John Linn 4 participants 27 messages Feb 5, 2011 – Feb 8, 2011 4 messages not in the archive

Subsystems (3)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

John Linn

author
#1 lore raw

PATCH 1/4 ARM: Xilinx: Adding Xilinx platform infrastructure support

Minimum infrastructure to add the Xilinx machine and allow it to be selected in the build.

---
 arch/arm/Kconfig    |   13 +++++++++++++
 arch/arm/Makefile   |    1 +
 arch/arm/mm/Kconfig |    4 ++--
 3 files changed, 16 insertions(+), 2 deletions(-)
arch/arm/Kconfig +13 -0
index 5cff165..3317671 100644
@@ -875,6 +875,17 @@ config PLAT_SPEAR
 	help
 	  Support for ST's SPEAr platform (SPEAr3xx, SPEAr6xx and SPEAr13xx).
 
+config ARCH_XILINX
+	bool "Xilinx ARM Cortex A9 Based FPGA"
+	select CPU_V7
+	select GENERIC_TIME
+	select GENERIC_CLOCKEVENTS
+	select CLKDEV_LOOKUP
+	select ARM_GIC
+	select ARM_AMBA
+	help
+	  Support for Xilinx ARM Cortex A9 Based FPGA
+
 endchoice
 
 #
@@ -1009,6 +1020,8 @@ source "arch/arm/mach-vexpress/Kconfig"
 
 source "arch/arm/mach-w90x900/Kconfig"
 
+source "arch/arm/mach-xilinx/Kconfig"
+
 # Definitions to make life easier
 config ARCH_ACORN
 	bool
arch/arm/Makefile +1 -0
index c22c1ad..4bad6ef 100644
@@ -197,6 +197,7 @@ machine-$(CONFIG_MACH_SPEAR300)		:= spear3xx
 machine-$(CONFIG_MACH_SPEAR310)		:= spear3xx
 machine-$(CONFIG_MACH_SPEAR320)		:= spear3xx
 machine-$(CONFIG_MACH_SPEAR600)		:= spear6xx
+machine-$(CONFIG_ARCH_XILINX)		:= xilinx
 
 # Platform directory name.  This list is sorted alphanumerically
 # by CONFIG_* macro name.
arch/arm/mm/Kconfig +2 -2
index 9d30c6f..cf0c6f6 100644
@@ -415,7 +415,7 @@ config CPU_32v6K
 
 # ARMv7
 config CPU_V7
-	bool "Support ARM V7 processor" if ARCH_INTEGRATOR || MACH_REALVIEW_EB || MACH_REALVIEW_PBX
+	bool "Support ARM V7 processor" if ARCH_INTEGRATOR || MACH_REALVIEW_EB || MACH_REALVIEW_PBX || ARCH_XILINX
 	select CPU_32v6K if !ARCH_OMAP2
 	select CPU_32v7
 	select CPU_ABRT_EV7
@@ -813,7 +813,7 @@ config CACHE_L2X0
 	depends on REALVIEW_EB_ARM11MP || MACH_REALVIEW_PB11MP || MACH_REALVIEW_PB1176 || \
 		   REALVIEW_EB_A9MP || ARCH_MX35 || ARCH_MX31 || MACH_REALVIEW_PBX || \
 		   ARCH_NOMADIK || ARCH_OMAP4 || ARCH_S5PV310 || ARCH_TEGRA || \
-		   ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || ARCH_SHMOBILE
+		   ARCH_U8500 || ARCH_VEXPRESS_CA9X4 || ARCH_SHMOBILE || ARCH_XILINX
 	default y
 	select OUTER_CACHE
 	select OUTER_CACHE_SYNC
Signature · 5 lines
1.6.2.1



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.
#2 lore raw

PATCH 1/4 ARM: Xilinx: Adding Xilinx platform infrastructure support

arch/arm/Kconfig config PLAT_SPEAR · line 880

Quoted 3 lines · John Linn
arch/arm/Kconfig +3 -0
+config ARCH_XILINX
+	bool "Xilinx ARM Cortex A9 Based FPGA"
+	select CPU_V7

Ok, so your platform only supports ARMv7, so you select this symbol.

arch/arm/mm/Kconfig config CPU_32v6K · line 418

Quoted 10 lines
arch/arm/mm/Kconfig +1 -1
index 9d30c6f..cf0c6f6 100644
@@ -415,7 +415,7 @@ config CPU_32v6K
 
 # ARMv7
 config CPU_V7
-	bool "Support ARM V7 processor" if ARCH_INTEGRATOR || MACH_REALVIEW_EB || MACH_REALVIEW_PBX
+	bool "Support ARM V7 processor" if ARCH_INTEGRATOR || MACH_REALVIEW_EB || MACH_REALVIEW_PBX || ARCH_XILINX

However, this makes the prompt visible to the user, and with the 'select' above, the only possible answer is 'y'. You don't need to do this - just remove this change. That means "Support ARM V7 processor" won't be offered to the user, but it will be set in the configuration.

author
#3 lore raw

RE: [PATCH 1/4] ARM: Xilinx: Adding Xilinx platform infrastructuresupport

Quoted 5 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Saturday, February 05, 2011 9:16 AM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; kernel at vger.kernel.org;

catalin.marinas at arm.com;

Quoted 2 lines

glikely at secretlab.ca
Subject: Re: [PATCH 1/4] ARM: Xilinx: Adding Xilinx platform

infrastructuresupport

Quoted 8 lines · John Linn

arch/arm/Kconfig config PLAT_SPEAR · line 880

Quoted 3 lines
arch/arm/Kconfig +3 -0
+config ARCH_XILINX
+	bool "Xilinx ARM Cortex A9 Based FPGA"
+	select CPU_V7

Ok, so your platform only supports ARMv7, so you select this symbol.

Yes. I added linux-kernel list also as I had hosed it up.

Quoted 9 lines

arch/arm/mm/Kconfig config CPU_32v6K · line 418

Quoted 9 lines
arch/arm/mm/Kconfig +0 -1
index 9d30c6f..cf0c6f6 100644
@@ -415,7 +415,7 @@ config CPU_32v6K
 
 # ARMv7
 config CPU_V7
-	bool "Support ARM V7 processor" if ARCH_INTEGRATOR ||

MACH_REALVIEW_EB || MACH_REALVIEW_PBX

Quoted 1 line

arch/arm/mm/Kconfig config CPU_32v6K · line 418

Quoted 1 line
arch/arm/mm/Kconfig +1 -0
+	bool "Support ARM V7 processor" if ARCH_INTEGRATOR ||

MACH_REALVIEW_EB || MACH_REALVIEW_PBX ||

Quoted 3 lines

ARCH_XILINX

However, this makes the prompt visible to the user, and with the

'select'

Quoted 1 line

above, the only possible answer is 'y'. You don't need to do this -

just

Quoted 1 line

remove this change. That means "Support ARM V7 processor" won't be

offered

Quoted 1 line

to the user, but it will be set in the configuration.

Thanks Russell. Will do.

-- John

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.

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#5 lore raw

PATCH 2/4 ARM: Xilinx: Adding Xilinx board support

The 1st board support is minimal to get a system up and running on the Xilinx platform.

---
 arch/arm/mach-xilinx/Kconfig       |   14 ++++
 arch/arm/mach-xilinx/Makefile      |    6 ++
 arch/arm/mach-xilinx/Makefile.boot |    3 +
 arch/arm/mach-xilinx/board_ep107.c |   81 ++++++++++++++++++++++
 arch/arm/mach-xilinx/common.c      |  132 ++++++++++++++++++++++++++++++++++++
 arch/arm/mach-xilinx/common.h      |   32 +++++++++
 6 files changed, 268 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-xilinx/Kconfig
 create mode 100644 arch/arm/mach-xilinx/Makefile
 create mode 100644 arch/arm/mach-xilinx/Makefile.boot
 create mode 100644 arch/arm/mach-xilinx/board_ep107.c
 create mode 100644 arch/arm/mach-xilinx/common.c
 create mode 100644 arch/arm/mach-xilinx/common.h
arch/arm/mach-xilinx/Kconfig new file +14 -0
new file mode 100644
index 0000000..11f9e65
@@ -0,0 +1,14 @@
+if ARCH_XILINX
+
+choice
+        prompt "Board Selection"
+	default XILINX_EP107
+
+config XILINX_EP107
+	bool "Xilinx EP107 Board"
+	help
+	  Select if you are using a Xilinx EP107 board.
+
+endchoice
+
+endif
arch/arm/mach-xilinx/Makefile new file +6 -0
new file mode 100644
index 0000000..194f9e4
@@ -0,0 +1,6 @@
+#
+# Makefile for the linux kernel.
+#
+
+# Common support
+obj-y := common.o board_ep107.o timer.o
arch/arm/mach-xilinx/Makefile.boot new file +3 -0
new file mode 100644
index 0000000..67039c3
@@ -0,0 +1,3 @@
+   zreladdr-y	:= 0x00008000
+params_phys-y	:= 0x00000100
+initrd_phys-y	:= 0x00800000
arch/arm/mach-xilinx/board_ep107.c new file +81 -0

81 lines not on this page · raw message

arch/arm/mach-xilinx/common.c new file +132 -0

132 lines not on this page · raw message

arch/arm/mach-xilinx/common.h new file +32 -0
new file mode 100644
index 0000000..49de295
@@ -0,0 +1,32 @@
+/* arch/arm/mach-xilinx/common.h
+ *
+ * This file contains common function prototypes to avoid externs
+ * in the c files.
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __MACH_XILINX_COMMON_H__
+#define __MACH_XILINX_COMMON_H__
+
+#include <linux/init.h>
+#include <linux/platform_device.h>
+
+void __init system_init(void);
+void __init irq_init(void);
+void __init map_io(void);
+void __init platform_device_init(struct platform_device *pdevices[],
+					int count);
+
+extern struct sys_timer xttcpss_sys_timer;
+
+#endif
Signature · 5 lines
1.6.2.1



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.
#6 lore raw

PATCH 2/4 ARM: Xilinx: Adding Xilinx board support

arch/arm/mach-xilinx/common.c line 63

Quoted 3 lines · John Linn
arch/arm/mach-xilinx/common.c new file +3 -0
+void __init irq_init(void)
+{
+	gic_cpu_base_addr = (void __iomem *)SCU_GIC_CPU_BASE;

Just pass this address into gic_init - there's no need to initialize this variable yourself anymore.

Quoted 1 line
arch/arm/mach-xilinx/common.c new file +1 -0
+	gic_init(0, 29, (void __iomem *)SCU_GIC_DIST_BASE, gic_cpu_base_addr);

...

Quoted 1 line

+static struct map_desc io_desc[] __initdata = {

...

arch/arm/mach-xilinx/common.c line 96

Quoted 8 lines
arch/arm/mach-xilinx/common.c new file +8 -0
+#ifdef CONFIG_DEBUG_LL
+	{
+		.virtual	= UART0_BASE,
+		.pfn		= __phys_to_pfn(UART0_BASE),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	},
+#endif

You shouldn't need to do this if your addruart macro in mach/debug-macros.S is correctly returning the virtual and physical addresses in rp and rv.

author
#7 lore raw

PATCH 2/4 ARM: Xilinx: Adding Xilinx board support

Quoted 5 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Saturday, February 05, 2011 9:19 AM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; kernel at vger.kernel.org;

catalin.marinas at arm.com;

Quoted 10 lines

glikely at secretlab.ca
Subject: Re: [PATCH 2/4] ARM: Xilinx: Adding Xilinx board support

arch/arm/mach-xilinx/common.c line 63

Quoted 3 lines · John Linn
arch/arm/mach-xilinx/common.c new file +3 -0
+void __init irq_init(void)
+{
+	gic_cpu_base_addr = (void __iomem *)SCU_GIC_CPU_BASE;

Just pass this address into gic_init - there's no need to initialize this variable yourself anymore.

Ok, easy fix.

Quoted 2 lines

Review

Quoted 2 lines
(quoted diff) +1 -0
+	gic_init(0, 29, (void __iomem *)SCU_GIC_DIST_BASE,

gic_cpu_base_addr);

Quoted 13 lines

...

Quoted 1 line

+static struct map_desc io_desc[] __initdata = {

...

arch/arm/mach-xilinx/common.c line 96

Quoted 8 lines
arch/arm/mach-xilinx/common.c new file +8 -0
+#ifdef CONFIG_DEBUG_LL
+	{
+		.virtual	= UART0_BASE,
+		.pfn		= __phys_to_pfn(UART0_BASE),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	},
+#endif

You shouldn't need to do this if your addruart macro in

mach/debug-macros.S

Quoted 1 line

is correctly returning the virtual and physical addresses in rp and

rv.

Ok, will check that and make sure.

Thanks,
John

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.

author
#8 lore raw

PATCH 2/4 ARM: Xilinx: Adding Xilinx board support

In reply to Russell King - ARM Linux
Quoted 5 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Saturday, February 05, 2011 9:19 AM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; kernel at vger.kernel.org;

catalin.marinas at arm.com;

Quoted 4 lines

glikely at secretlab.ca
Subject: Re: [PATCH 2/4] ARM: Xilinx: Adding Xilinx board support

On Sat, Feb 05, 2011 at 09:08:42AM -0700, John Linn wrote:

Quoted 8 lines

arch/arm/mach-xilinx/common.c line 63

Quoted 3 lines
arch/arm/mach-xilinx/common.c new file +3 -0
+void __init irq_init(void)
+{
+	gic_cpu_base_addr = (void __iomem *)SCU_GIC_CPU_BASE;

Just pass this address into gic_init - there's no need to initialize this variable yourself anymore.

Review

Quoted 1 line
(quoted diff) +1 -0
+	gic_init(0, 29, (void __iomem *)SCU_GIC_DIST_BASE,

gic_cpu_base_addr);

Quoted 13 lines

...

Quoted 1 line

+static struct map_desc io_desc[] __initdata = {

...

arch/arm/mach-xilinx/common.c line 96

Quoted 8 lines
arch/arm/mach-xilinx/common.c new file +8 -0
+#ifdef CONFIG_DEBUG_LL
+	{
+		.virtual	= UART0_BASE,
+		.pfn		= __phys_to_pfn(UART0_BASE),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	},
+#endif

You shouldn't need to do this if your addruart macro in

mach/debug-macros.S

Quoted 1 line

is correctly returning the virtual and physical addresses in rp and

rv.

I'm sure there's something I'm overlooking here.

I'm seeing that when I remove that I get I/O just for a bit, then it stops. I think
that means my addruart macro is working correctly.

I know the UART gets an early mapping in the MMU in head.S. But does that mapping
stay when the map_io function is ran?

Thanks,
John

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.

author
#9 lore raw

PATCH 2/4 ARM: Xilinx: Adding Xilinx board support

In reply to Russell King - ARM Linux
Quoted 5 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Saturday, February 05, 2011 9:19 AM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; kernel at vger.kernel.org;

catalin.marinas at arm.com;

Quoted 4 lines

glikely at secretlab.ca
Subject: Re: [PATCH 2/4] ARM: Xilinx: Adding Xilinx board support

On Sat, Feb 05, 2011 at 09:08:42AM -0700, John Linn wrote:

Quoted 8 lines

arch/arm/mach-xilinx/common.c line 63

Quoted 3 lines
arch/arm/mach-xilinx/common.c new file +3 -0
+void __init irq_init(void)
+{
+	gic_cpu_base_addr = (void __iomem *)SCU_GIC_CPU_BASE;

Just pass this address into gic_init - there's no need to initialize this variable yourself anymore.

Review

Quoted 1 line
(quoted diff) +1 -0
+	gic_init(0, 29, (void __iomem *)SCU_GIC_DIST_BASE,

gic_cpu_base_addr);

Quoted 13 lines

...

Quoted 1 line

+static struct map_desc io_desc[] __initdata = {

...

arch/arm/mach-xilinx/common.c line 96

Quoted 8 lines
arch/arm/mach-xilinx/common.c new file +8 -0
+#ifdef CONFIG_DEBUG_LL
+	{
+		.virtual	= UART0_BASE,
+		.pfn		= __phys_to_pfn(UART0_BASE),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	},
+#endif

You shouldn't need to do this if your addruart macro in

mach/debug-macros.S

Quoted 1 line

is correctly returning the virtual and physical addresses in rp and

rv.

Hi Russell,

It's interesting as I'm looking at this further. Sure enough it looks to me like any
early mappings in the MMU get destroyed. Paging_init calls devicemaps_init which says
it destroys the early debug mappings. Maybe I'm misinterpreting that?

I notice that the mach-tegra platform does have a mapping in the table for iotable_init
but it's not conditional on the DEBUG_LL and it covers the APB which includes the UART
for early debug.

Sorry for my slowness, but it still seems like the early MMU mappings go away. What am I
not seeing that I should be?

Thanks, appreciate your patience and help
John

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.

#10 lore raw

PATCH 2/4 ARM: Xilinx: Adding Xilinx board support

Quoted 4 lines · John Linn

It's interesting as I'm looking at this further. Sure enough it looks to me like any early mappings in the MMU get destroyed. Paging_init calls devicemaps_init which says
it destroys the early debug mappings. Maybe I'm misinterpreting that?

I don't think so. Normally, platforms end up mapping it as part of a larger mappign. If you don't have that then it'll be necessary.

author
#11 lore raw

PATCH 2/4 ARM: Xilinx: Adding Xilinx board support

Quoted 5 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Monday, February 07, 2011 1:54 PM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; kernel at vger.kernel.org;

catalin.marinas at arm.com;

Quoted 5 lines

glikely at secretlab.ca
Subject: Re: [PATCH 2/4] ARM: Xilinx: Adding Xilinx board support

Quoted 1 line · John Linn

It's interesting as I'm looking at this further. Sure enough it

looks

Quoted 1 line
Quoted 1 line

to me like any early mappings in the MMU get destroyed. Paging_init

calls

Quoted 2 lines
Quoted 2 lines

devicemaps_init which says
it destroys the early debug mappings. Maybe I'm misinterpreting

that?

Quoted 3 lines

I don't think so. Normally, platforms end up mapping it as part of a larger mappign. If you don't have that then it'll be necessary.

Thanks, appreciate the feedback. Makes sense.

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.

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#13 lore raw

PATCH 3/4 ARM: Xilinx: Adding timer support to the platform

The timer driver supports the Xilinx PS Timer Counter IP.

---
 arch/arm/mach-xilinx/timer.c |  452 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 452 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-xilinx/timer.c
arch/arm/mach-xilinx/timer.c new file +452 -0
new file mode 100644
index 0000000..c713e3c
@@ -0,0 +1,452 @@
+/* arch/arm/mach-xilinx/timer.c
+ *
+ * This file contains driver for the Xilinx PS Timer Counter IP.
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * based on arch\mips\kernel\time.c timer driver
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/types.h>
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/io.h>
+#include <asm/mach/time.h>
+#include <mach/hardware.h>
+
+/*
+ * This driver configures the 2 16-bit count-up timers as follows:
+ *
+ * T1: Timer 1, clocksource for generic timekeeping
+ * T2: Timer 2, clockevent source for hrtimers
+ * T3: Timer 3, <unused>
+ *
+ * The input frequency to the timer module is 200MHz which is common to all the
+ * timer channel (T1, T2, and T3)
+ * Clocksource timer resolution is 160ns
+ * Clockevent timer resolution is 160ns
+ */
+#define XTTCPSS_CLOCKSOURCE	0	/* Timer 1 as a generic timekeeping */
+#define XTTCPSS_CLOCKEVENT	1	/* Timer 2 as a clock event */
+
+#define XTTCPSS_TIMER_BASE	TTC0_BASE
+
+/*
+ * Timer Register Offset Definitions of Timer 1, Increment base address by 4
+ * and use same offsets for Timer 2
+ */
+#define XTTCPSS_CLK_CNTRL_OFFSET	0x00 /* Clock Control Reg, RW */
+#define XTTCPSS_CNT_CNTRL_OFFSET	0x0C /* Counter Control Reg, RW */
+#define XTTCPSS_COUNT_VAL_OFFSET	0x18 /* Counter Value Reg, RO */
+#define XTTCPSS_INTR_VAL_OFFSET		0x24 /* Interval Count Reg, RW */
+#define XTTCPSS_MATCH_1_OFFSET		0x30 /* Match 1 Value Reg, RW */
+#define XTTCPSS_MATCH_2_OFFSET		0x3C /* Match 2 Value Reg, RW */
+#define XTTCPSS_MATCH_3_OFFSET		0x48 /* Match 3 Value Reg, RW */
+#define XTTCPSS_ISR_OFFSET		0x54 /* Interrupt Status Reg, RO */
+#define XTTCPSS_IER_OFFSET		0x60 /* Interrupt Enable Reg, RW */
+
+/*
+ * Bit mask to enable/disable the timer
+ */
+#define XTTCPSS_CNT_CNTRL_ENABLE_MASK	0xFFFFFFFE
+
+/*
+ * Definitions of the timer read/write macro
+ */
+#define xttcpss_read(addr)	__raw_readl((void __iomem *)addr)
+#define xttcpss_write(addr, val) __raw_writel(val, (void __iomem *)(addr))
+
+
+/**
+ * struct xttcpss_timer - This definition defines local timer structure
+ *
+ * @name:	Name of Timer
+ * @base_addr:	Base address of timer
+ * @timer_irq:	irqaction structure for the timer device
+ * @mode:       only valid for an clock event, periodic or one-shot
+ **/
+struct xttcpss_timer {
+	char *name;
+	unsigned long base_addr;
+	struct irqaction timer_irq;
+	enum clock_event_mode mode;
+};
+
+
+static struct xttcpss_timer timers[];
+static struct clock_event_device xttcpss_clockevent;
+
+/*
+ * xttcpss_timer_irqs - Timers IRQ number
+ */
+static int xttcpss_timer_irqs[2] = {
+	IRQ_TIMERCOUNTER0,	/* Timer 1 IRQ number */
+	IRQ_TIMERCOUNTER0 + 1,	/* Timer 2 IRQ number */
+};
+
+/**
+ * xttcpss_set_interval - Set the timer interval value
+ *
+ * @timer:	Pointer to the timer instance
+ * @cycles:	Timer interval ticks
+ **/
+static void xttcpss_set_interval(struct xttcpss_timer *timer,
+					unsigned long cycles)
+{
+	u32 ctrl_reg;
+
+	/* Disable the counter, set the counter value  and re-enable counter */
+	ctrl_reg = xttcpss_read(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET, ctrl_reg);
+
+	pr_debug("set_interval, name = %s, period = %08X\n",
+			timer->name, (unsigned int)cycles);
+
+	xttcpss_write(timer->base_addr + XTTCPSS_INTR_VAL_OFFSET, cycles);
+
+	/* Reset the counter (0x10) so that it starts from 0, one-shot
+	   mode makes this needed for timing to be right. */
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+	ctrl_reg |= 0x10;
+	xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET, ctrl_reg);
+}
+
+/**
+ * xttcpss_clock_source_interrupt - Clock source timer interrupt handler
+ *
+ * @irq:	IRQ number of the Timer
+ * @dev_id:	void pointer to the xttcpss_timer instance
+ *
+ * This will be called when 16-bit clock source counter wraps
+ *
+ * returns: Always IRQ_HANDLED - success
+ **/
+static irqreturn_t xttcpss_clock_source_interrupt(int irq, void *dev_id)
+{
+	struct xttcpss_timer *timer = dev_id;
+
+	/* Acknowledge the interrupt */
+	xttcpss_write(timer->base_addr + XTTCPSS_ISR_OFFSET,
+		xttcpss_read(timer->base_addr + XTTCPSS_ISR_OFFSET));
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * xttcpss_clock_event_interrupt - Clock event timer interrupt handler
+ *
+ * @irq:	IRQ number of the Timer
+ * @dev_id:	void pointer to the xttcpss_timer instance
+ *
+ * returns: Always IRQ_HANDLED - success
+ **/
+static irqreturn_t xttcpss_clock_event_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = &xttcpss_clockevent;
+	struct xttcpss_timer *timer = dev_id;
+	u32 ctrl_reg;
+
+	/* Acknowledge the interrupt and call event handler */
+	xttcpss_write(timer->base_addr + XTTCPSS_ISR_OFFSET,
+		xttcpss_read(timer->base_addr + XTTCPSS_ISR_OFFSET));
+
+	if (timer->mode == CLOCK_EVT_MODE_ONESHOT) {
+
+		/* Disable the counter as it would keep running. */
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+		xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+	}
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * struct xttcpss_timer timers - This definition defines local timers
+ */
+static struct xttcpss_timer timers[] = {
+	[XTTCPSS_CLOCKSOURCE] = {
+		.name = "xttcpss clocksource",
+		.timer_irq = {
+			.flags = IRQF_DISABLED | IRQF_TIMER,
+			.handler = xttcpss_clock_source_interrupt,
+		}
+	},
+	[XTTCPSS_CLOCKEVENT] = {
+		.name = "xttcpss clockevent",
+		.timer_irq = {
+			.flags = IRQF_DISABLED | IRQF_TIMER,
+			.handler = xttcpss_clock_event_interrupt,
+		}
+	},
+};
+
+
+/**
+ * xttcpss_timer_hardware_init - Initialize the timer hardware
+ *
+ * Initialize the hardware, registers the timer interrupts, set the clock source
+ * timer interval and enable the clock source timer
+ **/
+static void __init xttcpss_timer_hardware_init(void)
+{
+	int timer_id;
+	struct xttcpss_timer *timer;
+
+	for (timer_id = 0; timer_id < ARRAY_SIZE(timers); timer_id++) {
+		timer = &timers[timer_id];
+
+		if (!(timer->name))
+			continue;
+
+		timer->base_addr = XTTCPSS_TIMER_BASE + (4*timer_id);
+
+		/* Disable counter, Enable Interval mode, Count up timer,
+		 * Disable Match mode, Internal Clock source select, set
+		 * prescalar to 32, and Enable the Interval interrupt */
+		xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				0x23);
+		xttcpss_write(timer->base_addr + XTTCPSS_CLK_CNTRL_OFFSET, 0x9);
+		xttcpss_write(timer->base_addr + XTTCPSS_IER_OFFSET, 0x1);
+
+		/* Setup IRQ */
+		timer->timer_irq.name = timer->name;
+		timer->timer_irq.dev_id = (void *)timer;
+		if (timer->timer_irq.handler != NULL) {
+			setup_irq(xttcpss_timer_irqs[timer_id],
+				&timer->timer_irq);
+		}
+		if (timer_id == XTTCPSS_CLOCKSOURCE)
+			xttcpss_set_interval(timer, ~0);
+	}
+}
+
+/**
+ * xttcpss_read_cycles - Reads the timer counter register
+ *
+ * returns: Current timer counter register value
+ **/
+static cycle_t xttcpss_read_cycles(struct clocksource *cs)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKSOURCE];
+
+	return (cycle_t)xttcpss_read(timer->base_addr +
+				XTTCPSS_COUNT_VAL_OFFSET);
+}
+
+
+/*
+ * Instantiate and initialize the clock source structure
+ */
+static struct clocksource clocksource_xttcpss = {
+	.name		= "xttcpss_timer1",
+	.rating		= 200,			/* Reasonable clock source */
+	.read		= xttcpss_read_cycles,
+	.mask		= CLOCKSOURCE_MASK(16),
+	.shift		= 0,			/* Initialized to zero */
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+
+/**
+ * xttcpss_set_next_event - Sets the time interval for next event
+ *
+ * @cycles:	Timer interval ticks
+ * @evt:	Address of clock event instance
+ *
+ * returns: Always 0 - success
+ **/
+static int xttcpss_set_next_event(unsigned long cycles,
+					struct clock_event_device *evt)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
+
+	xttcpss_set_interval(timer, cycles);
+	return 0;
+}
+
+/**
+ * xttcpss_set_mode - Sets the mode of timer
+ *
+ * @mode:	Mode to be set
+ * @evt:	Address of clock event instance
+ **/
+static void xttcpss_set_mode(enum clock_event_mode mode,
+					struct clock_event_device *evt)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	timer->mode = mode;
+
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		xttcpss_set_interval(timer, CLOCK_TICK_RATE / HZ);
+		break;
+	case CLOCK_EVT_MODE_ONESHOT:
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+		xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+		break;
+	case CLOCK_EVT_MODE_RESUME:
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+		xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+		break;
+	}
+}
+
+/*
+ * Instantiate and initialize the clock event structure
+ */
+static struct clock_event_device xttcpss_clockevent = {
+	.name		= "xttcpss_timer2",
+	.features	= CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
+	.shift		= 0,		/* Initialized to zero */
+	.set_next_event	= xttcpss_set_next_event,
+	.set_mode	= xttcpss_set_mode,
+	.rating		= 200,
+};
+
+/**
+ * xttcpss_timer_init - Initialize the timer
+ *
+ * Initializes the timer hardware and registers the clock source and clock event
+ * timers with Linux kernal timer framework
+ **/
+static void __init xttcpss_timer_init(void)
+{
+	u32 shift;
+	u64 temp;
+
+	xttcpss_timer_hardware_init();
+
+	/* Calculate the nanoseconds to cycles divisor value for clock source
+	 * timer */
+	for (shift = 16; shift > 0; shift--) {
+		temp = (u64) NSEC_PER_SEC << shift;
+		do_div(temp, CLOCK_TICK_RATE);
+		if ((temp >> 32) == 0)
+			break;
+	}
+
+	/* Setup clocksource */
+	clocksource_xttcpss.shift = shift;
+	clocksource_xttcpss.mult =
+		clocksource_hz2mult(CLOCK_TICK_RATE, clocksource_xttcpss.shift);
+
+	if (clocksource_register(&clocksource_xttcpss))
+		printk(KERN_ERR "xttcpss_timer_init: can't register clocksource"
+				" for %s\n", clocksource_xttcpss.name);
+	/* Calculate the nanoseconds to cycles divisor value for clock event
+	 * timer */
+	for (shift = 16; shift > 0; shift--) {
+		temp = (u64) CLOCK_TICK_RATE << shift;
+		do_div(temp, NSEC_PER_SEC);
+		if ((temp >> 32) == 0)
+			break;
+	}
+
+	/* Setup clockevent */
+	xttcpss_clockevent.shift = shift;
+	xttcpss_clockevent.mult = div_sc(CLOCK_TICK_RATE, NSEC_PER_SEC,
+						 xttcpss_clockevent.shift);
+
+	xttcpss_clockevent.max_delta_ns =
+		clockevent_delta2ns(0xfffe, &xttcpss_clockevent);
+	xttcpss_clockevent.min_delta_ns =
+		clockevent_delta2ns(1, &xttcpss_clockevent);
+
+	xttcpss_clockevent.cpumask = cpumask_of(0);
+	clockevents_register_device(&xttcpss_clockevent);
+}
+
+#ifdef CONFIG_PM
+/**
+ * xttcpss_timer_suspend - Suspend the timer
+ *
+ * Disables all (clock source and clock event) the timers
+ **/
+static void xttcpss_timer_suspend(void)
+{
+	struct xttcpss_timer *source_timer = &timers[XTTCPSS_CLOCKSOURCE];
+	struct xttcpss_timer *event_timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	/* Disable clocksource timer */
+	ctrl_reg = xttcpss_read(source_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(source_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+
+	/* Disable clockevent timer */
+	ctrl_reg = xttcpss_read(event_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(event_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+}
+
+/**
+ * xttcpss_timer_resume - Resume the timer
+ *
+ * Enables  all (clock source and clock event) the timers
+ **/
+static void xttcpss_timer_resume(void)
+{
+	struct xttcpss_timer *source_timer = &timers[XTTCPSS_CLOCKSOURCE];
+	struct xttcpss_timer *event_timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	/* Enable clocksource timer */
+	ctrl_reg = xttcpss_read(source_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+	xttcpss_write(source_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+
+	/* Enable clockevent timer */
+	ctrl_reg = xttcpss_read(event_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+	xttcpss_write(event_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+}
+#else
+#define xttcpss_timer_suspend NULL
+#define xttcpss_timer_resume NULL
+#endif
+
+/*
+ * Instantiate and initialize the system timer structure
+ */
+struct sys_timer xttcpss_sys_timer = {
+	.init		= xttcpss_timer_init,
+	.suspend	= xttcpss_timer_suspend,
+	.resume		= xttcpss_timer_resume,
+};
Signature · 5 lines
1.6.2.1



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.
#14 lore raw

PATCH 3/4 ARM: Xilinx: Adding timer support to the platform

arch/arm/mach-xilinx/timer.c line 7

Quoted 13 lines · John Linn
arch/arm/mach-xilinx/timer.c new file +7 -0
new file mode 100644
index 0000000..c713e3c
@@ -0,0 +1,452 @@
+/* arch/arm/mach-xilinx/timer.c
+ *
+ * This file contains driver for the Xilinx PS Timer Counter IP.
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * based on arch\mips\kernel\time.c timer driver

Unix directory separators are / not \.

Quoted 21 lines
arch/arm/mach-xilinx/timer.c new file +21 -0
+ *
+ * This program is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License version 2 as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/types.h>
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/io.h>
+#include <asm/mach/time.h>
+#include <mach/hardware.h>

arch/arm/mach-xilinx/timer.c line 38

Quoted 10 lines
arch/arm/mach-xilinx/timer.c new file +10 -0
+
+/*
+ * This driver configures the 2 16-bit count-up timers as follows:
+ *
+ * T1: Timer 1, clocksource for generic timekeeping
+ * T2: Timer 2, clockevent source for hrtimers
+ * T3: Timer 3, <unused>
+ *
+ * The input frequency to the timer module is 200MHz which is common to all the
+ * timer channel (T1, T2, and T3)

You should probably mention that you clock the counters themselves at 6.25MHz due to the pre-scaler setting of 32. 200MHz and 16-bit counters didn't look too clever when I first read through this code.

Quoted 34 lines
arch/arm/mach-xilinx/timer.c new file +34 -0
+ * Clocksource timer resolution is 160ns
+ * Clockevent timer resolution is 160ns
+ */
+#define XTTCPSS_CLOCKSOURCE	0	/* Timer 1 as a generic timekeeping */
+#define XTTCPSS_CLOCKEVENT	1	/* Timer 2 as a clock event */
+
+#define XTTCPSS_TIMER_BASE	TTC0_BASE
+
+/*
+ * Timer Register Offset Definitions of Timer 1, Increment base address by 4
+ * and use same offsets for Timer 2
+ */
+#define XTTCPSS_CLK_CNTRL_OFFSET	0x00 /* Clock Control Reg, RW */
+#define XTTCPSS_CNT_CNTRL_OFFSET	0x0C /* Counter Control Reg, RW */
+#define XTTCPSS_COUNT_VAL_OFFSET	0x18 /* Counter Value Reg, RO */
+#define XTTCPSS_INTR_VAL_OFFSET		0x24 /* Interval Count Reg, RW */
+#define XTTCPSS_MATCH_1_OFFSET		0x30 /* Match 1 Value Reg, RW */
+#define XTTCPSS_MATCH_2_OFFSET		0x3C /* Match 2 Value Reg, RW */
+#define XTTCPSS_MATCH_3_OFFSET		0x48 /* Match 3 Value Reg, RW */
+#define XTTCPSS_ISR_OFFSET		0x54 /* Interrupt Status Reg, RO */
+#define XTTCPSS_IER_OFFSET		0x60 /* Interrupt Enable Reg, RW */
+
+/*
+ * Bit mask to enable/disable the timer
+ */
+#define XTTCPSS_CNT_CNTRL_ENABLE_MASK	0xFFFFFFFE
+
+/*
+ * Definitions of the timer read/write macro
+ */
+#define xttcpss_read(addr)	__raw_readl((void __iomem *)addr)
+#define xttcpss_write(addr, val) __raw_writel(val, (void __iomem *)(addr))
+
+

arch/arm/mach-xilinx/timer.c line 82

Quoted 10 lines
arch/arm/mach-xilinx/timer.c new file +10 -0
+/**
+ * struct xttcpss_timer - This definition defines local timer structure
+ *
+ * @name:	Name of Timer
+ * @base_addr:	Base address of timer
+ * @timer_irq:	irqaction structure for the timer device
+ * @mode:       only valid for an clock event, periodic or one-shot
+ **/
+struct xttcpss_timer {
+	char *name;

const?

arch/arm/mach-xilinx/timer.c line 84

Quoted 2 lines
arch/arm/mach-xilinx/timer.c new file +2 -0
+	unsigned long base_addr;
+	struct irqaction timer_irq;

irqaction also has a const char *name pointer - is it necessary to duplicate it above?

Quoted 50 lines
arch/arm/mach-xilinx/timer.c new file +50 -0
+	enum clock_event_mode mode;
+};
+
+
+static struct xttcpss_timer timers[];
+static struct clock_event_device xttcpss_clockevent;
+
+/*
+ * xttcpss_timer_irqs - Timers IRQ number
+ */
+static int xttcpss_timer_irqs[2] = {
+	IRQ_TIMERCOUNTER0,	/* Timer 1 IRQ number */
+	IRQ_TIMERCOUNTER0 + 1,	/* Timer 2 IRQ number */
+};
+
+/**
+ * xttcpss_set_interval - Set the timer interval value
+ *
+ * @timer:	Pointer to the timer instance
+ * @cycles:	Timer interval ticks
+ **/
+static void xttcpss_set_interval(struct xttcpss_timer *timer,
+					unsigned long cycles)
+{
+	u32 ctrl_reg;
+
+	/* Disable the counter, set the counter value  and re-enable counter */
+	ctrl_reg = xttcpss_read(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET, ctrl_reg);
+
+	pr_debug("set_interval, name = %s, period = %08X\n",
+			timer->name, (unsigned int)cycles);
+
+	xttcpss_write(timer->base_addr + XTTCPSS_INTR_VAL_OFFSET, cycles);
+
+	/* Reset the counter (0x10) so that it starts from 0, one-shot
+	   mode makes this needed for timing to be right. */
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+	ctrl_reg |= 0x10;
+	xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET, ctrl_reg);
+}
+
+/**
+ * xttcpss_clock_source_interrupt - Clock source timer interrupt handler
+ *
+ * @irq:	IRQ number of the Timer
+ * @dev_id:	void pointer to the xttcpss_timer instance
+ *
+ * This will be called when 16-bit clock source counter wraps

arch/arm/mach-xilinx/timer.c line 144

Quoted 10 lines
arch/arm/mach-xilinx/timer.c new file +10 -0
+ *
+ * returns: Always IRQ_HANDLED - success
+ **/
+static irqreturn_t xttcpss_clock_source_interrupt(int irq, void *dev_id)
+{
+	struct xttcpss_timer *timer = dev_id;
+
+	/* Acknowledge the interrupt */
+	xttcpss_write(timer->base_addr + XTTCPSS_ISR_OFFSET,
+		xttcpss_read(timer->base_addr + XTTCPSS_ISR_OFFSET));

Hmm. Is it necessary to do this to keep the counter running? If the counter stops then it's not a good clock source.

Quoted 70 lines
arch/arm/mach-xilinx/timer.c new file +70 -0
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * xttcpss_clock_event_interrupt - Clock event timer interrupt handler
+ *
+ * @irq:	IRQ number of the Timer
+ * @dev_id:	void pointer to the xttcpss_timer instance
+ *
+ * returns: Always IRQ_HANDLED - success
+ **/
+static irqreturn_t xttcpss_clock_event_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = &xttcpss_clockevent;
+	struct xttcpss_timer *timer = dev_id;
+	u32 ctrl_reg;
+
+	/* Acknowledge the interrupt and call event handler */
+	xttcpss_write(timer->base_addr + XTTCPSS_ISR_OFFSET,
+		xttcpss_read(timer->base_addr + XTTCPSS_ISR_OFFSET));
+
+	if (timer->mode == CLOCK_EVT_MODE_ONESHOT) {
+
+		/* Disable the counter as it would keep running. */
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+		xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+	}
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * struct xttcpss_timer timers - This definition defines local timers
+ */
+static struct xttcpss_timer timers[] = {
+	[XTTCPSS_CLOCKSOURCE] = {
+		.name = "xttcpss clocksource",
+		.timer_irq = {
+			.flags = IRQF_DISABLED | IRQF_TIMER,
+			.handler = xttcpss_clock_source_interrupt,
+		}
+	},
+	[XTTCPSS_CLOCKEVENT] = {
+		.name = "xttcpss clockevent",
+		.timer_irq = {
+			.flags = IRQF_DISABLED | IRQF_TIMER,
+			.handler = xttcpss_clock_event_interrupt,
+		}
+	},
+};
+
+
+/**
+ * xttcpss_timer_hardware_init - Initialize the timer hardware
+ *
+ * Initialize the hardware, registers the timer interrupts, set the clock source
+ * timer interval and enable the clock source timer
+ **/
+static void __init xttcpss_timer_hardware_init(void)
+{
+	int timer_id;
+	struct xttcpss_timer *timer;
+
+	for (timer_id = 0; timer_id < ARRAY_SIZE(timers); timer_id++) {

arch/arm/mach-xilinx/timer.c line 224

Quoted 10 lines
arch/arm/mach-xilinx/timer.c new file +10 -0
+		timer = &timers[timer_id];
+
+		if (!(timer->name))
+			continue;
+
+		timer->base_addr = XTTCPSS_TIMER_BASE + (4*timer_id);
+
+		/* Disable counter, Enable Interval mode, Count up timer,
+		 * Disable Match mode, Internal Clock source select, set
+		 * prescalar to 32, and Enable the Interval interrupt */

Isn't it "pre-scaler" ?

Quoted 130 lines
arch/arm/mach-xilinx/timer.c new file +130 -0
+		xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				0x23);
+		xttcpss_write(timer->base_addr + XTTCPSS_CLK_CNTRL_OFFSET, 0x9);
+		xttcpss_write(timer->base_addr + XTTCPSS_IER_OFFSET, 0x1);
+
+		/* Setup IRQ */
+		timer->timer_irq.name = timer->name;
+		timer->timer_irq.dev_id = (void *)timer;
+		if (timer->timer_irq.handler != NULL) {
+			setup_irq(xttcpss_timer_irqs[timer_id],
+				&timer->timer_irq);
+		}
+		if (timer_id == XTTCPSS_CLOCKSOURCE)
+			xttcpss_set_interval(timer, ~0);
+	}
+}
+
+/**
+ * xttcpss_read_cycles - Reads the timer counter register
+ *
+ * returns: Current timer counter register value
+ **/
+static cycle_t xttcpss_read_cycles(struct clocksource *cs)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKSOURCE];
+
+	return (cycle_t)xttcpss_read(timer->base_addr +
+				XTTCPSS_COUNT_VAL_OFFSET);
+}
+
+
+/*
+ * Instantiate and initialize the clock source structure
+ */
+static struct clocksource clocksource_xttcpss = {
+	.name		= "xttcpss_timer1",
+	.rating		= 200,			/* Reasonable clock source */
+	.read		= xttcpss_read_cycles,
+	.mask		= CLOCKSOURCE_MASK(16),
+	.shift		= 0,			/* Initialized to zero */
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+
+/**
+ * xttcpss_set_next_event - Sets the time interval for next event
+ *
+ * @cycles:	Timer interval ticks
+ * @evt:	Address of clock event instance
+ *
+ * returns: Always 0 - success
+ **/
+static int xttcpss_set_next_event(unsigned long cycles,
+					struct clock_event_device *evt)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
+
+	xttcpss_set_interval(timer, cycles);
+	return 0;
+}
+
+/**
+ * xttcpss_set_mode - Sets the mode of timer
+ *
+ * @mode:	Mode to be set
+ * @evt:	Address of clock event instance
+ **/
+static void xttcpss_set_mode(enum clock_event_mode mode,
+					struct clock_event_device *evt)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	timer->mode = mode;
+
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		xttcpss_set_interval(timer, CLOCK_TICK_RATE / HZ);
+		break;
+	case CLOCK_EVT_MODE_ONESHOT:
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+		xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+		break;
+	case CLOCK_EVT_MODE_RESUME:
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+		xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+		break;
+	}
+}
+
+/*
+ * Instantiate and initialize the clock event structure
+ */
+static struct clock_event_device xttcpss_clockevent = {
+	.name		= "xttcpss_timer2",
+	.features	= CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
+	.shift		= 0,		/* Initialized to zero */
+	.set_next_event	= xttcpss_set_next_event,
+	.set_mode	= xttcpss_set_mode,
+	.rating		= 200,
+};
+
+/**
+ * xttcpss_timer_init - Initialize the timer
+ *
+ * Initializes the timer hardware and registers the clock source and clock event
+ * timers with Linux kernal timer framework
+ **/
+static void __init xttcpss_timer_init(void)
+{
+	u32 shift;
+	u64 temp;
+
+	xttcpss_timer_hardware_init();
+
+	/* Calculate the nanoseconds to cycles divisor value for clock source
+	 * timer */
+	for (shift = 16; shift > 0; shift--) {
+		temp = (u64) NSEC_PER_SEC << shift;
+		do_div(temp, CLOCK_TICK_RATE);
+		if ((temp >> 32) == 0)
+			break;

arch/arm/mach-xilinx/timer.c line 364

Quoted 10 lines
arch/arm/mach-xilinx/timer.c new file +10 -0
+	}
+
+	/* Setup clocksource */
+	clocksource_xttcpss.shift = shift;
+	clocksource_xttcpss.mult =
+		clocksource_hz2mult(CLOCK_TICK_RATE, clocksource_xttcpss.shift);
+
+	if (clocksource_register(&clocksource_xttcpss))
+		printk(KERN_ERR "xttcpss_timer_init: can't register clocksource"
+				" for %s\n", clocksource_xttcpss.name);

There's code to calculate the shift and multipler for you. All you need do is replace the above with clocksource_register_hz() or clocksource_register_khz().

arch/arm/mach-xilinx/timer.c line 377

Quoted 13 lines
arch/arm/mach-xilinx/timer.c new file +13 -0
+	/* Calculate the nanoseconds to cycles divisor value for clock event
+	 * timer */
+	for (shift = 16; shift > 0; shift--) {
+		temp = (u64) CLOCK_TICK_RATE << shift;
+		do_div(temp, NSEC_PER_SEC);
+		if ((temp >> 32) == 0)
+			break;
+	}
+
+	/* Setup clockevent */
+	xttcpss_clockevent.shift = shift;
+	xttcpss_clockevent.mult = div_sc(CLOCK_TICK_RATE, NSEC_PER_SEC,
+						 xttcpss_clockevent.shift);

clockevents_calc_mult_shift(&xttcpss_clockevent, freq, minsec) should do away with all the above, here freq is the clock rate for the actual counter itself (after the pre-scaler.)

Quoted 52 lines
arch/arm/mach-xilinx/timer.c new file +52 -0
+
+	xttcpss_clockevent.max_delta_ns =
+		clockevent_delta2ns(0xfffe, &xttcpss_clockevent);
+	xttcpss_clockevent.min_delta_ns =
+		clockevent_delta2ns(1, &xttcpss_clockevent);
+
+	xttcpss_clockevent.cpumask = cpumask_of(0);
+	clockevents_register_device(&xttcpss_clockevent);
+}
+
+#ifdef CONFIG_PM
+/**
+ * xttcpss_timer_suspend - Suspend the timer
+ *
+ * Disables all (clock source and clock event) the timers
+ **/
+static void xttcpss_timer_suspend(void)
+{
+	struct xttcpss_timer *source_timer = &timers[XTTCPSS_CLOCKSOURCE];
+	struct xttcpss_timer *event_timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	/* Disable clocksource timer */
+	ctrl_reg = xttcpss_read(source_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(source_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+
+	/* Disable clockevent timer */
+	ctrl_reg = xttcpss_read(event_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(event_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+}
+
+/**
+ * xttcpss_timer_resume - Resume the timer
+ *
+ * Enables  all (clock source and clock event) the timers
+ **/
+static void xttcpss_timer_resume(void)
+{
+	struct xttcpss_timer *source_timer = &timers[XTTCPSS_CLOCKSOURCE];
+	struct xttcpss_timer *event_timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	/* Enable clocksource timer */
+	ctrl_reg = xttcpss_read(source_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;

arch/arm/mach-xilinx/timer.c line 439

Quoted 10 lines
arch/arm/mach-xilinx/timer.c new file +10 -0
+	xttcpss_write(source_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+
+	/* Enable clockevent timer */
+	ctrl_reg = xttcpss_read(event_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+	xttcpss_write(event_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+}

The generic time code should do PM for you. In fact, if you're using generic time code, these callbacks will never be used, as the code which calls them is surrounded by:

#if defined(CONFIG_PM) && !defined(CONFIG_GENERIC_CLOCKEVENTS)

Quoted 13 lines
arch/arm/mach-xilinx/timer.c new file +13 -0
+#else
+#define xttcpss_timer_suspend NULL
+#define xttcpss_timer_resume NULL
+#endif
+
+/*
+ * Instantiate and initialize the system timer structure
+ */
+struct sys_timer xttcpss_sys_timer = {
+	.init		= xttcpss_timer_init,
+	.suspend	= xttcpss_timer_suspend,
+	.resume		= xttcpss_timer_resume,
+};
author
#15 lore raw

PATCH 3/4 ARM: Xilinx: Adding timer support to the platform

Quoted 5 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Saturday, February 05, 2011 9:36 AM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; catalin.marinas at arm.com;

glikely at secretlab.ca; Kiran

Quoted 2 lines

Sutariya
Subject: Re: [PATCH 3/4] ARM: Xilinx: Adding timer support to the

platform

Quoted 3 lines · John Linn

Review

Quoted 1 line
(quoted diff) +0 -0

b/arch/arm/mach-xilinx/timer.c

Quoted 6 lines
Quoted 6 lines

new file mode 100644
index 0000000..c713e3c

arch/arm/mach-xilinx/timer.c new file +1 -0
@@ -0,0 +1,452 @@
+/* arch/arm/mach-xilinx/timer.c
Quoted 9 lines

arch/arm/mach-xilinx/timer.c line 7

Quoted 6 lines
arch/arm/mach-xilinx/timer.c new file +6 -0
+ *
+ * This file contains driver for the Xilinx PS Timer Counter IP.
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * based on arch\mips\kernel\time.c timer driver

Unix directory separators are / not \.

Yes, will fix those.

Quoted 2 lines

arch/arm/mach-xilinx/timer.c line 9

Quoted 2 lines
arch/arm/mach-xilinx/timer.c new file +2 -0
+ *
+ * This program is free software; you can redistribute it and/or

modify it under

Quoted 1 line

arch/arm/mach-xilinx/timer.c line 10

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+ * the terms of the GNU General Public License version 2 as

published by the

Quoted 1 line

arch/arm/mach-xilinx/timer.c line 11

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+ * Free Software Foundation; either version 2 of the License, or

(at your

Quoted 3 lines

arch/arm/mach-xilinx/timer.c

Quoted 3 lines
arch/arm/mach-xilinx/timer.c new file +3 -0
+ * option) any later version.
+ *
+ * You should have received a copy of the GNU General Public

License along with

Quoted 1 line

arch/arm/mach-xilinx/timer.c line 15

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+ * this program; if not, write to the Free Software Foundation,

Inc., 59 Temple

Quoted 14 lines
Quoted 14 lines
arch/arm/mach-xilinx/timer.c new file +14 -0
+ * Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/types.h>
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/io.h>
+#include <asm/mach/time.h>
+#include <mach/hardware.h>
+
Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+/*
+ * This driver configures the 2 16-bit count-up timers as follows:
+ *
+ * T1: Timer 1, clocksource for generic timekeeping
+ * T2: Timer 2, clockevent source for hrtimers
+ * T3: Timer 3, <unused>
+ *
+ * The input frequency to the timer module is 200MHz which is

common to all the

Quoted 4 lines

arch/arm/mach-xilinx/timer.c line 38

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+ * timer channel (T1, T2, and T3)

You should probably mention that you clock the counters themselves at 6.25MHz due to the pre-scaler setting of 32. 200MHz and 16-bit

counters

Quoted 1 line

didn't look too clever when I first read through this code.

Sure.

Quoted 5 lines

arch/arm/mach-xilinx/timer.c

Quoted 5 lines
arch/arm/mach-xilinx/timer.c new file +4 -0
+ * Clocksource timer resolution is 160ns
+ * Clockevent timer resolution is 160ns
+ */
+#define XTTCPSS_CLOCKSOURCE	0	/* Timer 1 as a generic

timekeeping */

Quoted 6 lines

arch/arm/mach-xilinx/timer.c

Quoted 6 lines
arch/arm/mach-xilinx/timer.c new file +6 -0
+#define XTTCPSS_CLOCKEVENT	1	/* Timer 2 as a clock event */
+
+#define XTTCPSS_TIMER_BASE	TTC0_BASE
+
+/*
+ * Timer Register Offset Definitions of Timer 1, Increment base

address by 4

Quoted 4 lines

arch/arm/mach-xilinx/timer.c

Quoted 4 lines
arch/arm/mach-xilinx/timer.c new file +4 -0
+ * and use same offsets for Timer 2
+ */
+#define XTTCPSS_CLK_CNTRL_OFFSET	0x00 /* Clock Control Reg, RW */
+#define XTTCPSS_CNT_CNTRL_OFFSET	0x0C /* Counter Control Reg, RW

*/

Quoted 2 lines

arch/arm/mach-xilinx/timer.c line 54

Quoted 2 lines
arch/arm/mach-xilinx/timer.c new file +2 -0
+#define XTTCPSS_COUNT_VAL_OFFSET	0x18 /* Counter Value Reg, RO */
+#define XTTCPSS_INTR_VAL_OFFSET		0x24 /* Interval Count

Reg, RW */

Quoted 1 line

arch/arm/mach-xilinx/timer.c line 55

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+#define XTTCPSS_MATCH_1_OFFSET		0x30 /* Match 1 Value

Reg, RW */

Quoted 1 line

arch/arm/mach-xilinx/timer.c line 56

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+#define XTTCPSS_MATCH_2_OFFSET		0x3C /* Match 2 Value

Reg, RW */

Quoted 1 line

arch/arm/mach-xilinx/timer.c line 57

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+#define XTTCPSS_MATCH_3_OFFSET		0x48 /* Match 3 Value

Reg, RW */

Quoted 1 line

arch/arm/mach-xilinx/timer.c line 58

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+#define XTTCPSS_ISR_OFFSET		0x54 /* Interrupt Status Reg, RO

*/

Quoted 1 line

arch/arm/mach-xilinx/timer.c line 59

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+#define XTTCPSS_IER_OFFSET		0x60 /* Interrupt Enable Reg, RW

*/

Quoted 11 lines

arch/arm/mach-xilinx/timer.c

Quoted 11 lines
arch/arm/mach-xilinx/timer.c new file +11 -0
+
+/*
+ * Bit mask to enable/disable the timer
+ */
+#define XTTCPSS_CNT_CNTRL_ENABLE_MASK	0xFFFFFFFE
+
+/*
+ * Definitions of the timer read/write macro
+ */
+#define xttcpss_read(addr)	__raw_readl((void __iomem *)addr)
+#define xttcpss_write(addr, val) __raw_writel(val, (void __iomem

*)(addr))

Quoted 4 lines

arch/arm/mach-xilinx/timer.c

Quoted 4 lines
arch/arm/mach-xilinx/timer.c new file +4 -0
+
+
+/**
+ * struct xttcpss_timer - This definition defines local timer

structure

Quoted 10 lines

arch/arm/mach-xilinx/timer.c line 82

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+ *
+ * @name:	Name of Timer
+ * @base_addr:	Base address of timer
+ * @timer_irq:	irqaction structure for the timer device
+ * @mode:       only valid for an clock event, periodic or one-shot
+ **/
+struct xttcpss_timer {
+	char *name;

const?

Yep looks like it can be. I'll add it.

Quoted 5 lines

arch/arm/mach-xilinx/timer.c line 84

Quoted 3 lines
arch/arm/mach-xilinx/timer.c new file +2 -0
+	unsigned long base_addr;
+	struct irqaction timer_irq;

irqaction also has a const char *name pointer - is it necessary to

duplicate

Quoted 2 lines

it above?

I see your point.

Quoted 19 lines
Quoted 19 lines
arch/arm/mach-xilinx/timer.c new file +19 -0
+	enum clock_event_mode mode;
+};
+
+
+static struct xttcpss_timer timers[];
+static struct clock_event_device xttcpss_clockevent;
+
+/*
+ * xttcpss_timer_irqs - Timers IRQ number
+ */
+static int xttcpss_timer_irqs[2] = {
+	IRQ_TIMERCOUNTER0,	/* Timer 1 IRQ number */
+	IRQ_TIMERCOUNTER0 + 1,	/* Timer 2 IRQ number */
+};
+
+/**
+ * xttcpss_set_interval - Set the timer interval value
+ *
+ * @timer:	Pointer to the timer instance
Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+ * @cycles:	Timer interval ticks
+ **/
+static void xttcpss_set_interval(struct xttcpss_timer *timer,
+					unsigned long cycles)
+{
+	u32 ctrl_reg;
+
+	/* Disable the counter, set the counter value  and re-enable

counter */

Quoted 1 line

arch/arm/mach-xilinx/timer.c line 112

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+	ctrl_reg = xttcpss_read(timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET);

Quoted 2 lines

arch/arm/mach-xilinx/timer.c line 114

Quoted 2 lines
arch/arm/mach-xilinx/timer.c new file +2 -0
+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,

ctrl_reg);

Quoted 5 lines

arch/arm/mach-xilinx/timer.c

Quoted 5 lines
arch/arm/mach-xilinx/timer.c new file +5 -0
+
+	pr_debug("set_interval, name = %s, period = %08X\n",
+			timer->name, (unsigned int)cycles);
+
+	xttcpss_write(timer->base_addr + XTTCPSS_INTR_VAL_OFFSET,

cycles);

Quoted 6 lines

arch/arm/mach-xilinx/timer.c line 173

Quoted 6 lines
arch/arm/mach-xilinx/timer.c new file +6 -0
+
+	/* Reset the counter (0x10) so that it starts from 0, one-shot
+	   mode makes this needed for timing to be right. */
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+	ctrl_reg |= 0x10;
+	xttcpss_write(timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,

ctrl_reg);

Quoted 4 lines

arch/arm/mach-xilinx/timer.c

Quoted 4 lines
arch/arm/mach-xilinx/timer.c new file +4 -0
+}
+
+/**
+ * xttcpss_clock_source_interrupt - Clock source timer interrupt

handler

Quoted 9 lines

arch/arm/mach-xilinx/timer.c

Quoted 9 lines
arch/arm/mach-xilinx/timer.c new file +9 -0
+ *
+ * @irq:	IRQ number of the Timer
+ * @dev_id:	void pointer to the xttcpss_timer instance
+ *
+ * This will be called when 16-bit clock source counter wraps
+ *
+ * returns: Always IRQ_HANDLED - success
+ **/
+static irqreturn_t xttcpss_clock_source_interrupt(int irq, void

*dev_id)

Quoted 9 lines

arch/arm/mach-xilinx/timer.c line 144

Quoted 6 lines
arch/arm/mach-xilinx/timer.c new file +6 -0
+{
+	struct xttcpss_timer *timer = dev_id;
+
+	/* Acknowledge the interrupt */
+	xttcpss_write(timer->base_addr + XTTCPSS_ISR_OFFSET,
+		xttcpss_read(timer->base_addr + XTTCPSS_ISR_OFFSET));

Hmm. Is it necessary to do this to keep the counter running? If the counter stops then it's not a good clock source.

I'm going to have to study this a bit more. I see your point. The interrupt doesn't
really do anything. I'll look to see if we can just disable the interrupt.

Quoted 7 lines

arch/arm/mach-xilinx/timer.c

Quoted 7 lines
arch/arm/mach-xilinx/timer.c new file +6 -0
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * xttcpss_clock_event_interrupt - Clock event timer interrupt

handler

Quoted 7 lines

arch/arm/mach-xilinx/timer.c

Quoted 7 lines
arch/arm/mach-xilinx/timer.c new file +7 -0
+ *
+ * @irq:	IRQ number of the Timer
+ * @dev_id:	void pointer to the xttcpss_timer instance
+ *
+ * returns: Always IRQ_HANDLED - success
+ **/
+static irqreturn_t xttcpss_clock_event_interrupt(int irq, void

*dev_id)

Quoted 8 lines
Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+{
+	struct clock_event_device *evt = &xttcpss_clockevent;
+	struct xttcpss_timer *timer = dev_id;
+	u32 ctrl_reg;
+
+	/* Acknowledge the interrupt and call event handler */
+	xttcpss_write(timer->base_addr + XTTCPSS_ISR_OFFSET,
+		xttcpss_read(timer->base_addr + XTTCPSS_ISR_OFFSET));
Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+
+	if (timer->mode == CLOCK_EVT_MODE_ONESHOT) {
+
+		/* Disable the counter as it would keep running. */
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+		xttcpss_write(timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

Quoted 10 lines

arch/arm/mach-xilinx/timer.c

Quoted 10 lines
arch/arm/mach-xilinx/timer.c new file +10 -0
+				ctrl_reg);
+	}
+
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+/*
+ * struct xttcpss_timer timers - This definition defines local

timers

Quoted 15 lines
Quoted 15 lines
arch/arm/mach-xilinx/timer.c new file +15 -0
+ */
+static struct xttcpss_timer timers[] = {
+	[XTTCPSS_CLOCKSOURCE] = {
+		.name = "xttcpss clocksource",
+		.timer_irq = {
+			.flags = IRQF_DISABLED | IRQF_TIMER,
+			.handler = xttcpss_clock_source_interrupt,
+		}
+	},
+	[XTTCPSS_CLOCKEVENT] = {
+		.name = "xttcpss clockevent",
+		.timer_irq = {
+			.flags = IRQF_DISABLED | IRQF_TIMER,
+			.handler = xttcpss_clock_event_interrupt,
+		}
Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+	},
+};
+
+
+/**
+ * xttcpss_timer_hardware_init - Initialize the timer hardware
+ *
+ * Initialize the hardware, registers the timer interrupts, set the

clock source

Quoted 8 lines
Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+ * timer interval and enable the clock source timer
+ **/
+static void __init xttcpss_timer_hardware_init(void)
+{
+	int timer_id;
+	struct xttcpss_timer *timer;
+
+	for (timer_id = 0; timer_id < ARRAY_SIZE(timers); timer_id++) {
Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+		timer = &timers[timer_id];
+
+		if (!(timer->name))
+			continue;
+
+		timer->base_addr = XTTCPSS_TIMER_BASE + (4*timer_id);
+
+		/* Disable counter, Enable Interval mode, Count up

timer,

Quoted 4 lines

arch/arm/mach-xilinx/timer.c line 224

Quoted 2 lines
arch/arm/mach-xilinx/timer.c new file +2 -0
+		 * Disable Match mode, Internal Clock source select, set
+		 * prescalar to 32, and Enable the Interval interrupt */

Isn't it "pre-scaler" ?

Ok, easy to fix.

Quoted 2 lines

arch/arm/mach-xilinx/timer.c line 225

Quoted 2 lines
arch/arm/mach-xilinx/timer.c new file +1 -0
+		xttcpss_write(timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

Quoted 2 lines

arch/arm/mach-xilinx/timer.c line 227

Quoted 2 lines
arch/arm/mach-xilinx/timer.c new file +2 -0
+				0x23);
+		xttcpss_write(timer->base_addr +

XTTCPSS_CLK_CNTRL_OFFSET, 0x9);

Quoted 1 line

arch/arm/mach-xilinx/timer.c line 228

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+		xttcpss_write(timer->base_addr + XTTCPSS_IER_OFFSET,

0x1);

Quoted 25 lines
Quoted 25 lines
arch/arm/mach-xilinx/timer.c new file +25 -0
+
+		/* Setup IRQ */
+		timer->timer_irq.name = timer->name;
+		timer->timer_irq.dev_id = (void *)timer;
+		if (timer->timer_irq.handler != NULL) {
+			setup_irq(xttcpss_timer_irqs[timer_id],
+				&timer->timer_irq);
+		}
+		if (timer_id == XTTCPSS_CLOCKSOURCE)
+			xttcpss_set_interval(timer, ~0);
+	}
+}
+
+/**
+ * xttcpss_read_cycles - Reads the timer counter register
+ *
+ * returns: Current timer counter register value
+ **/
+static cycle_t xttcpss_read_cycles(struct clocksource *cs)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKSOURCE];
+
+	return (cycle_t)xttcpss_read(timer->base_addr +
+				XTTCPSS_COUNT_VAL_OFFSET);
+}
Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+
+
+/*
+ * Instantiate and initialize the clock source structure
+ */
+static struct clocksource clocksource_xttcpss = {
+	.name		= "xttcpss_timer1",
+	.rating		= 200,			/* Reasonable clock

source */

Quoted 3 lines

arch/arm/mach-xilinx/timer.c

Quoted 3 lines
arch/arm/mach-xilinx/timer.c new file +3 -0
+	.read		= xttcpss_read_cycles,
+	.mask		= CLOCKSOURCE_MASK(16),
+	.shift		= 0,			/* Initialized to zero

*/

Quoted 38 lines
Quoted 38 lines
arch/arm/mach-xilinx/timer.c new file +38 -0
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+
+/**
+ * xttcpss_set_next_event - Sets the time interval for next event
+ *
+ * @cycles:	Timer interval ticks
+ * @evt:	Address of clock event instance
+ *
+ * returns: Always 0 - success
+ **/
+static int xttcpss_set_next_event(unsigned long cycles,
+					struct clock_event_device *evt)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
+
+	xttcpss_set_interval(timer, cycles);
+	return 0;
+}
+
+/**
+ * xttcpss_set_mode - Sets the mode of timer
+ *
+ * @mode:	Mode to be set
+ * @evt:	Address of clock event instance
+ **/
+static void xttcpss_set_mode(enum clock_event_mode mode,
+					struct clock_event_device *evt)
+{
+	struct xttcpss_timer *timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	timer->mode = mode;
+
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		xttcpss_set_interval(timer, CLOCK_TICK_RATE / HZ);
Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+		break;
+	case CLOCK_EVT_MODE_ONESHOT:
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+		xttcpss_write(timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

Quoted 7 lines

arch/arm/mach-xilinx/timer.c

Quoted 7 lines
arch/arm/mach-xilinx/timer.c new file +7 -0
+				ctrl_reg);
+		break;
+	case CLOCK_EVT_MODE_RESUME:
+		ctrl_reg = xttcpss_read(timer->base_addr +
+					XTTCPSS_CNT_CNTRL_OFFSET);
+		ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+		xttcpss_write(timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

Quoted 11 lines

arch/arm/mach-xilinx/timer.c

Quoted 11 lines
arch/arm/mach-xilinx/timer.c new file +11 -0
+				ctrl_reg);
+		break;
+	}
+}
+
+/*
+ * Instantiate and initialize the clock event structure
+ */
+static struct clock_event_device xttcpss_clockevent = {
+	.name		= "xttcpss_timer2",
+	.features	= CLOCK_EVT_FEAT_PERIODIC |

CLOCK_EVT_FEAT_ONESHOT,

Quoted 10 lines

arch/arm/mach-xilinx/timer.c

Quoted 10 lines
arch/arm/mach-xilinx/timer.c new file +10 -0
+	.shift		= 0,		/* Initialized to zero */
+	.set_next_event	= xttcpss_set_next_event,
+	.set_mode	= xttcpss_set_mode,
+	.rating		= 200,
+};
+
+/**
+ * xttcpss_timer_init - Initialize the timer
+ *
+ * Initializes the timer hardware and registers the clock source

and clock event

Quoted 10 lines

arch/arm/mach-xilinx/timer.c

Quoted 10 lines
arch/arm/mach-xilinx/timer.c new file +10 -0
+ * timers with Linux kernal timer framework
+ **/
+static void __init xttcpss_timer_init(void)
+{
+	u32 shift;
+	u64 temp;
+
+	xttcpss_timer_hardware_init();
+
+	/* Calculate the nanoseconds to cycles divisor value for clock

source

Quoted 4 lines
Quoted 4 lines
arch/arm/mach-xilinx/timer.c new file +4 -0
+	 * timer */
+	for (shift = 16; shift > 0; shift--) {
+		temp = (u64) NSEC_PER_SEC << shift;
+		do_div(temp, CLOCK_TICK_RATE);
Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+		if ((temp >> 32) == 0)
+			break;
+	}
+
+	/* Setup clocksource */
+	clocksource_xttcpss.shift = shift;
+	clocksource_xttcpss.mult =
+		clocksource_hz2mult(CLOCK_TICK_RATE,

clocksource_xttcpss.shift);

Quoted 3 lines

arch/arm/mach-xilinx/timer.c

Quoted 3 lines
arch/arm/mach-xilinx/timer.c new file +3 -0
+
+	if (clocksource_register(&clocksource_xttcpss))
+		printk(KERN_ERR "xttcpss_timer_init: can't register

clocksource"

Quoted 6 lines

arch/arm/mach-xilinx/timer.c line 364

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+				" for %s\n", clocksource_xttcpss.name);

There's code to calculate the shift and multipler for you. All you need do is replace the above with clocksource_register_hz() or clocksource_register_khz().

Ok, will look at that.

Quoted 1 line

arch/arm/mach-xilinx/timer.c line 365

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+	/* Calculate the nanoseconds to cycles divisor value for clock

event

Quoted 4 lines
Quoted 4 lines
arch/arm/mach-xilinx/timer.c new file +4 -0
+	 * timer */
+	for (shift = 16; shift > 0; shift--) {
+		temp = (u64) CLOCK_TICK_RATE << shift;
+		do_div(temp, NSEC_PER_SEC);
Quoted 8 lines

arch/arm/mach-xilinx/timer.c line 378

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+		if ((temp >> 32) == 0)
+			break;
+	}
+
+	/* Setup clockevent */
+	xttcpss_clockevent.shift = shift;
+	xttcpss_clockevent.mult = div_sc(CLOCK_TICK_RATE, NSEC_PER_SEC,
+

xttcpss_clockevent.shift);

Quoted 5 lines

clockevents_calc_mult_shift(&xttcpss_clockevent, freq, minsec) should do away with all the above, here freq is the clock rate for the actual counter itself (after the pre-scaler.)

Sounds good.

Quoted 11 lines
Quoted 11 lines
arch/arm/mach-xilinx/timer.c new file +11 -0
+
+	xttcpss_clockevent.max_delta_ns =
+		clockevent_delta2ns(0xfffe, &xttcpss_clockevent);
+	xttcpss_clockevent.min_delta_ns =
+		clockevent_delta2ns(1, &xttcpss_clockevent);
+
+	xttcpss_clockevent.cpumask = cpumask_of(0);
+	clockevents_register_device(&xttcpss_clockevent);
+}
+
+#ifdef CONFIG_PM
Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+/**
+ * xttcpss_timer_suspend - Suspend the timer
+ *
+ * Disables all (clock source and clock event) the timers
+ **/
+static void xttcpss_timer_suspend(void)
+{
+	struct xttcpss_timer *source_timer =

&timers[XTTCPSS_CLOCKSOURCE];

Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+	struct xttcpss_timer *event_timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	/* Disable clocksource timer */
+	ctrl_reg = xttcpss_read(source_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(source_timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

Quoted 10 lines
Quoted 10 lines
arch/arm/mach-xilinx/timer.c new file +10 -0
+				ctrl_reg);
+
+	/* Disable clockevent timer */
+	ctrl_reg = xttcpss_read(event_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg |= ~(XTTCPSS_CNT_CNTRL_ENABLE_MASK);
+	xttcpss_write(event_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+}
+
Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+/**
+ * xttcpss_timer_resume - Resume the timer
+ *
+ * Enables  all (clock source and clock event) the timers
+ **/
+static void xttcpss_timer_resume(void)
+{
+	struct xttcpss_timer *source_timer =

&timers[XTTCPSS_CLOCKSOURCE];

Quoted 8 lines

arch/arm/mach-xilinx/timer.c

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+	struct xttcpss_timer *event_timer = &timers[XTTCPSS_CLOCKEVENT];
+	u32 ctrl_reg;
+
+	/* Enable clocksource timer */
+	ctrl_reg = xttcpss_read(source_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+	xttcpss_write(source_timer->base_addr +

XTTCPSS_CNT_CNTRL_OFFSET,

Quoted 14 lines

arch/arm/mach-xilinx/timer.c line 439

Quoted 9 lines
arch/arm/mach-xilinx/timer.c new file +9 -0
+				ctrl_reg);
+
+	/* Enable clockevent timer */
+	ctrl_reg = xttcpss_read(event_timer->base_addr +
+				XTTCPSS_CNT_CNTRL_OFFSET);
+	ctrl_reg &= XTTCPSS_CNT_CNTRL_ENABLE_MASK;
+	xttcpss_write(event_timer->base_addr + XTTCPSS_CNT_CNTRL_OFFSET,
+				ctrl_reg);
+}

The generic time code should do PM for you. In fact, if you're using generic time code, these callbacks will never be used, as the code which calls them is surrounded by:

Quoted 2 lines

#if defined(CONFIG_PM) && !defined(CONFIG_GENERIC_CLOCKEVENTS)

Ok, will remove.

Thanks, will update based on these.

-- John

Quoted 5 lines
Quoted 5 lines
arch/arm/mach-xilinx/timer.c new file +5 -0
+#else
+#define xttcpss_timer_suspend NULL
+#define xttcpss_timer_resume NULL
+#endif
+
Quoted 8 lines

arch/arm/mach-xilinx/timer.c line 452

Quoted 8 lines
arch/arm/mach-xilinx/timer.c new file +8 -0
+/*
+ * Instantiate and initialize the system timer structure
+ */
+struct sys_timer xttcpss_sys_timer = {
+	.init		= xttcpss_timer_init,
+	.suspend	= xttcpss_timer_suspend,
+	.resume		= xttcpss_timer_resume,
+};

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.

#16 lore raw

PATCH 3/4 ARM: Xilinx: Adding timer support to the platform

In reply to John Linn

arch/arm/mach-xilinx/timer.c line 70

Quoted 5 lines · John Linn
arch/arm/mach-xilinx/timer.c new file +5 -0
+/*
+ * Definitions of the timer read/write macro
+ */
+#define xttcpss_read(addr)	__raw_readl((void __iomem *)addr)
+#define xttcpss_write(addr, val) __raw_writel(val, (void __iomem *)(addr))

Get rid of these casts. You should never need to cast to void __iomem *.

arch/arm/mach-xilinx/timer.c line 83

Quoted 13 lines
arch/arm/mach-xilinx/timer.c new file +13 -0
+
+
+/**
+ * struct xttcpss_timer - This definition defines local timer structure
+ *
+ * @name:	Name of Timer
+ * @base_addr:	Base address of timer
+ * @timer_irq:	irqaction structure for the timer device
+ * @mode:       only valid for an clock event, periodic or one-shot
+ **/
+struct xttcpss_timer {
+	char *name;
+	unsigned long base_addr;

Replace this with void __iomem *base_addr.

arch/arm/mach-xilinx/timer.c line 220

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+		timer->base_addr = XTTCPSS_TIMER_BASE + (4*timer_id);

Right, so XTTCPSS_TIMER_BASE should be typed as void __iomem *.

Quoted 1 line

+#define XTTCPSS_TIMER_BASE TTC0_BASE

which means TTC0_BASE should be void __iomem * as well as this is an iomem cookie, not a physical address.

author
#17 lore raw

PATCH 3/4 ARM: Xilinx: Adding timer support to the platform

Quoted 5 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Tuesday, February 08, 2011 7:35 AM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; catalin.marinas at arm.com;

glikely at secretlab.ca; Kiran

Quoted 2 lines

Sutariya
Subject: Re: [PATCH 3/4] ARM: Xilinx: Adding timer support to the

platform

Quoted 7 lines · John Linn

arch/arm/mach-xilinx/timer.c

Quoted 5 lines
arch/arm/mach-xilinx/timer.c new file +5 -0
+/*
+ * Definitions of the timer read/write macro
+ */
+#define xttcpss_read(addr)	__raw_readl((void __iomem *)addr)
+#define xttcpss_write(addr, val) __raw_writel(val, (void __iomem

*)(addr))

Quoted 2 lines

Get rid of these casts. You should never need to cast to void __iomem

*.

Thanks for continuing, I appreciate your time and will try not to waste it.

Agreed, that's done and makes sense.

Quoted 5 lines

arch/arm/mach-xilinx/timer.c

Quoted 5 lines
arch/arm/mach-xilinx/timer.c new file +4 -0
+
+
+/**
+ * struct xttcpss_timer - This definition defines local timer

structure

Quoted 12 lines

arch/arm/mach-xilinx/timer.c line 83

Quoted 9 lines

+ *
+ * @name: Name of Timer
+ * @base_addr: Base address of timer
+ * @timer_irq: irqaction structure for the timer device
+ * @mode: only valid for an clock event, periodic or one-shot
+ **/

arch/arm/mach-xilinx/timer.c new file +3 -0
+struct xttcpss_timer {
+	char *name;
+	unsigned long base_addr;

Replace this with void __iomem *base_addr.

Agreed, that's done and makes sense.

Quoted 8 lines

arch/arm/mach-xilinx/timer.c line 220

Quoted 1 line
arch/arm/mach-xilinx/timer.c new file +1 -0
+		timer->base_addr = XTTCPSS_TIMER_BASE + (4*timer_id);

Right, so XTTCPSS_TIMER_BASE should be typed as void __iomem *.

Quoted 1 line

+#define XTTCPSS_TIMER_BASE TTC0_BASE

which means TTC0_BASE should be void __iomem * as well as this is an iomem cookie, not a physical address.

In my latest revision of the patches I'm working on, here's what I did.

void __iomem *base_addr;

base_addr = IOMEM(XTTCPSS_TIMER_BASE)

This seems equivalent to what you're saying, but maybe I'm missing something.

Quoted 20 lines

From your other patch:

Quoted 1 line

+#define TTC0_BASE (PERIPH_BASE + 0x1000)

So, PERIPH_BASE is also an iomem cookie, not a physical address, so it too should be void __iomem *.

Hence:

Quoted 1 line

+#define PERIPH_BASE 0xF8000000

should be:

#define PERIPH_BASE IOMEM(0xF8000000)

But then in a different patch you do:

(quoted diff) +2 -0
+               .virtual        = TTC0_BASE,
+               .pfn            = __phys_to_pfn(TTC0_BASE),
Quoted 1 line

So it's also used as a physical address. Yuck.

I have been mapping everything flat (virt = phys) and I should have been clearer on that.

It seems like there are two sets of #defines expected, virtual and physical, even if they
are the same.

Thanks,
John

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.

#18 lore raw

PATCH 3/4 ARM: Xilinx: Adding timer support to the platform

Quoted 10 lines · John Linn
Quoted 10 lines
Quoted 1 line

+#define PERIPH_BASE 0xF8000000

should be:

#define PERIPH_BASE IOMEM(0xF8000000)

But then in a different patch you do:

(quoted diff) +2 -0
+               .virtual        = TTC0_BASE,
+               .pfn            = __phys_to_pfn(TTC0_BASE),
Quoted 7 lines
Quoted 1 line

So it's also used as a physical address. Yuck.

I have been mapping everything flat (virt = phys) and I should have been clearer on that.

It seems like there are two sets of #defines expected, virtual and physical, even if they are the same.

Yes, it quickly gets messy otherwise. Normally, you only need to specify very few physical addresses, e.g. the start of each register window. When you have separate defines for your virtual bases, you can also make them type safe right away, like:

#define PERIPH_BASE_PHYS 0xf800000
#define PERIPH_BASE IOMEM(PERIPH_BASE_PHYS)

#define TCC0_BASE (PERIPH_BASE + 0xA0000)
#define TCC1_BASE (PERIPH_BASE + 0xB0000)

This way, you don't need to convert the types in driver code.

Arnd

author
#19 lore raw

PATCH 3/4 ARM: Xilinx: Adding timer support to the platform

Quoted 5 lines

-----Original Message-----
From: Arnd Bergmann [mailto:arnd at arndb.de]
Sent: Tuesday, February 08, 2011 8:26 AM
To: linux-arm-kernel at lists.infradead.org
Cc: John Linn; Russell King - ARM Linux; catalin.marinas at arm.com;

glikely at secretlab.ca; Kiran Sutariya

Quoted 1 line

Subject: Re: [PATCH 3/4] ARM: Xilinx: Adding timer support to the

platform

Quoted 8 lines · John Linn
Quoted 6 lines
Quoted 6 lines
Quoted 1 line

+#define PERIPH_BASE 0xF8000000

should be:

#define PERIPH_BASE IOMEM(0xF8000000)

Quoted 7 lines
Quoted 7 lines
Quoted 5 lines

But then in a different patch you do:

(quoted diff) +2 -0
+               .virtual        = TTC0_BASE,
+               .pfn            = __phys_to_pfn(TTC0_BASE),

So it's also used as a physical address. Yuck.

I have been mapping everything flat (virt = phys) and I should have

been

Quoted 6 lines
Quoted 4 lines

clearer on that.

It seems like there are two sets of #defines expected, virtual and physical, even if they are the same.

Yes, it quickly gets messy otherwise. Normally, you only need to

specify

Quoted 4 lines

very few physical addresses, e.g. the start of each register window. When you have separate defines for your virtual bases, you can also make them type safe right away, like:

Quoted 8 lines

#define PERIPH_BASE_PHYS 0xf800000
#define PERIPH_BASE IOMEM(PERIPH_BASE_PHYS)

#define TCC0_BASE (PERIPH_BASE + 0xA0000)
#define TCC1_BASE (PERIPH_BASE + 0xB0000)

This way, you don't need to convert the types in driver code.

Thanks for clarification Arnd. Appreciate the help.

-- John

Quoted 1 line

Arnd

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.

author
#20 lore raw

PATCH 3/4 ARM: Xilinx: Adding timer support to the platform

In reply to Arnd Bergmann
Quoted 5 lines

-----Original Message-----
From: Arnd Bergmann [mailto:arnd at arndb.de]
Sent: Tuesday, February 08, 2011 8:26 AM
To: linux-arm-kernel at lists.infradead.org
Cc: John Linn; Russell King - ARM Linux; catalin.marinas at arm.com;

glikely at secretlab.ca; Kiran Sutariya

Quoted 1 line

Subject: Re: [PATCH 3/4] ARM: Xilinx: Adding timer support to the

platform

Quoted 8 lines · John Linn
Quoted 6 lines
Quoted 6 lines
Quoted 1 line

+#define PERIPH_BASE 0xF8000000

should be:

#define PERIPH_BASE IOMEM(0xF8000000)

Quoted 7 lines
Quoted 7 lines
Quoted 5 lines

But then in a different patch you do:

(quoted diff) +2 -0
+               .virtual        = TTC0_BASE,
+               .pfn            = __phys_to_pfn(TTC0_BASE),

So it's also used as a physical address. Yuck.

I have been mapping everything flat (virt = phys) and I should have

been

Quoted 6 lines
Quoted 4 lines

clearer on that.

It seems like there are two sets of #defines expected, virtual and physical, even if they are the same.

Yes, it quickly gets messy otherwise. Normally, you only need to

specify

Quoted 10 lines

very few physical addresses, e.g. the start of each register window. When you have separate defines for your virtual bases, you can also make them type safe right away, like:

#define PERIPH_BASE_PHYS 0xf800000
#define PERIPH_BASE IOMEM(PERIPH_BASE_PHYS)

#define TCC0_BASE (PERIPH_BASE + 0xA0000)
#define TCC1_BASE (PERIPH_BASE + 0xB0000)

Hi Arnd,

Maybe I'm being dense here, just wanting to be clear.

But that still doesn't provide physical addresses for each device like you would need to put in the resources
for platform devices.

Do you just cast them back to an integer then?

Thanks,
John

Quoted 3 lines

This way, you don't need to convert the types in driver code.

Arnd

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.

#21 lore raw

PATCH 3/4 ARM: Xilinx: Adding timer support to the platform

Quoted 5 lines · On Tuesday 08 February 2011 18:53:50 John Linn wrote:

Maybe I'm being dense here, just wanting to be clear.

But that still doesn't provide physical addresses for each device like you would need to put in the resources
for platform devices.

No, you're right. If you want to have them in proper resource structures, you need the physical addresses as unsigned long numbers, not the virtual address pointers.

Quoted 1 line

Do you just cast them back to an integer then?

No, you certainly never do that, it would be a clear violation of the type system. You should assume that physical and virtual addresses are different, and never build any driver code under the assumption that the virtual addresses are fixed (except perhaps the PCI PIO window).

My personal recommendation would be to either put the physical address into the resource structure and have the driver ioremap the registers, or to map the entire registers at boot time, and have the driver use the hardcoded values while ignoring the resources.

The ioremap approach is more flexible, in particular when you build a kernel that can have the devices at varying locations, but it doesn't work for all the low-level devices that you might need to talk to before the MMU code is fully initialized, such as timers and interrupt controllers.

Arnd

#22 lore raw

PATCH 3/4 ARM: Xilinx: Adding timer support to the platform

Quoted 26 lines · Arnd Bergmann
Quoted 5 lines · On Tuesday 08 February 2011 18:53:50 John Linn wrote:

Maybe I'm being dense here, just wanting to be clear.

But that still doesn't provide physical addresses for each device like you would need to put in the resources
for platform devices.

No, you're right. If you want to have them in proper resource structures, you need the physical addresses as unsigned long numbers, not the virtual address pointers.

Quoted 1 line

Do you just cast them back to an integer then?

No, you certainly never do that, it would be a clear violation of the type system. You should assume that physical and virtual addresses are different, and never build any driver code under the assumption that the virtual addresses are fixed (except perhaps the PCI PIO window).

My personal recommendation would be to either put the physical address into the resource structure and have the driver ioremap the registers, or to map the entire registers at boot time, and have the driver use the hardcoded values while ignoring the resources.

Quoted 5 lines

The ioremap approach is more flexible, in particular when you build a kernel that can have the devices at varying locations, but it doesn't work for all the low-level devices that you might need to talk to before the MMU code is fully initialized, such as timers and interrupt controllers.

Arnd's right. Don't let the code make any assumptions about phys==virt, use phys address in the resource structure, and ioremap whenever you can. Only when ioremap isn't available should you have virt addrss macros which are derived from the phys address ones.

g.

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#24 lore raw

PATCH 4/4 ARM: Xilinx: base header files and assembly macros

These are the minimum needed to build the kernel for the new platform.

---
 arch/arm/mach-xilinx/include/mach/clkdev.h      |   33 +++++++++
 arch/arm/mach-xilinx/include/mach/debug-macro.S |   36 +++++++++
 arch/arm/mach-xilinx/include/mach/entry-macro.S |   87 +++++++++++++++++++++++
 arch/arm/mach-xilinx/include/mach/hardware.h    |   44 ++++++++++++
 arch/arm/mach-xilinx/include/mach/io.h          |   27 +++++++
 arch/arm/mach-xilinx/include/mach/irqs.h        |   28 +++++++
 arch/arm/mach-xilinx/include/mach/memory.h      |   23 ++++++
 arch/arm/mach-xilinx/include/mach/system.h      |   28 +++++++
 arch/arm/mach-xilinx/include/mach/timex.h       |   22 ++++++
 arch/arm/mach-xilinx/include/mach/uart.h        |   30 ++++++++
 arch/arm/mach-xilinx/include/mach/uncompress.h  |   46 ++++++++++++
 arch/arm/mach-xilinx/include/mach/vmalloc.h     |   20 +++++
 12 files changed, 424 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-xilinx/include/mach/clkdev.h
 create mode 100644 arch/arm/mach-xilinx/include/mach/debug-macro.S
 create mode 100644 arch/arm/mach-xilinx/include/mach/entry-macro.S
 create mode 100644 arch/arm/mach-xilinx/include/mach/hardware.h
 create mode 100644 arch/arm/mach-xilinx/include/mach/io.h
 create mode 100644 arch/arm/mach-xilinx/include/mach/irqs.h
 create mode 100644 arch/arm/mach-xilinx/include/mach/memory.h
 create mode 100644 arch/arm/mach-xilinx/include/mach/system.h
 create mode 100644 arch/arm/mach-xilinx/include/mach/timex.h
 create mode 100644 arch/arm/mach-xilinx/include/mach/uart.h
 create mode 100644 arch/arm/mach-xilinx/include/mach/uncompress.h
 create mode 100644 arch/arm/mach-xilinx/include/mach/vmalloc.h
arch/arm/mach-xilinx/include/mach/clkdev.h new file +33 -0
new file mode 100644
index 0000000..5dd1453
@@ -0,0 +1,33 @@
+/*
+ * arch/arm/mach-xilinx/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__
+
+struct clk {
+	unsigned long rate;
+};
+
+static inline int __clk_get(struct clk *clk)
+{
+	return 1;
+}
+
+static inline void __clk_put(struct clk *clk)
+{
+}
+
+#endif
arch/arm/mach-xilinx/include/mach/debug-macro.S new file +36 -0
new file mode 100644
index 0000000..db16b13
@@ -0,0 +1,36 @@
+/* arch/arm/mach-xilinx/include/mach/debug-macro.S
+ *
+ * Debugging macro include header
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <mach/hardware.h>
+#include <mach/uart.h>
+
+		.macro	addruart, rp, rv
+		ldr	\rp, =LL_UART_PADDR	@ physical
+		ldr	\rv, =LL_UART_VADDR	@ virtual
+		.endm
+
+		.macro	senduart,rd,rx
+		str	\rd, [\rx, #UART_FIFO_OFFSET]	@ TXDATA
+		.endm
+
+		.macro	waituart,rd,rx
+		.endm
+
+		.macro	busyuart,rd,rx
+1002:		ldr	\rd, [\rx, #UART_SR_OFFSET]	@ get status register
+		tst	\rd, #UART_SR_TXFULL		@
+		bne	1002b			@ wait if FIFO is full
+		.endm
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +87 -0

87 lines not on this page · raw message

arch/arm/mach-xilinx/include/mach/hardware.h new file +44 -0

44 lines not on this page · raw message

arch/arm/mach-xilinx/include/mach/io.h new file +27 -0
new file mode 100644
index 0000000..2e69db7
@@ -0,0 +1,27 @@
+/* arch/arm/mach-xilinx/include/mach/io.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __MACH_IO_H__
+#define __MACH_IO_H__
+
+/* Allow IO space to be anywhere in the memory */
+
+#define IO_SPACE_LIMIT 0xffffffff
+
+/* IO address mapping macros, nothing special at this time but required */
+
+#define __io(a)			((void __iomem *)(a))
+#define __mem_pci(a)		(a)
+
+#endif
arch/arm/mach-xilinx/include/mach/irqs.h new file +28 -0
new file mode 100644
index 0000000..da2fadc
@@ -0,0 +1,28 @@
+/* arch/arm/mach-xilinx/include/mach/irqs.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+#ifndef __MACH_IRQS_H
+#define __MACH_IRQS_H
+
+#define ARCH_NR_GPIOS	118
+#define NR_IRQS		(128 + ARCH_NR_GPIOS)
+
+/*
+ * GIC Interrupts
+ */
+
+#define IRQ_GIC_SPI_START	32
+#define IRQ_TIMERCOUNTER0	42
+#define IRQ_UART0		59
+
+#endif
arch/arm/mach-xilinx/include/mach/memory.h new file +23 -0
new file mode 100644
index 0000000..2b6d1eb
@@ -0,0 +1,23 @@
+/* arch/arm/mach-xilinx/include/mach/memory.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __MACH_MEMORY_H__
+#define __MACH_MEMORY_H__
+
+#include <mach/hardware.h>
+
+#define PHYS_OFFSET             0x0
+#define MEM_SIZE		SZ_256M
+
+#endif
arch/arm/mach-xilinx/include/mach/system.h new file +28 -0
new file mode 100644
index 0000000..a5f1f0f
@@ -0,0 +1,28 @@
+/* arch/arm/mach-xilinx/include/mach/system.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __MACH_SYSTEM_H__
+#define __MACH_SYSTEM_H__
+
+static inline void arch_idle(void)
+{
+	cpu_do_idle();
+}
+
+static inline void arch_reset(char mode, const char *cmd)
+{
+	/* Add architecture specific reset processing here */
+}
+
+#endif
arch/arm/mach-xilinx/include/mach/timex.h new file +22 -0
new file mode 100644
index 0000000..0c1d292
@@ -0,0 +1,22 @@
+/* arch/arm/mach-xilinx/include/mach/timex.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __MACH_TIMEX_H__
+#define __MACH_TIMEX_H__
+
+#include <mach/hardware.h>
+
+#define CLOCK_TICK_RATE	(PERIPHERAL_CLOCK_RATE / 32)
+
+#endif
arch/arm/mach-xilinx/include/mach/uart.h new file +30 -0
new file mode 100644
index 0000000..2601143
@@ -0,0 +1,30 @@
+/* arch/arm/mach-xilinx/include/mach/uart.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+#ifndef __MACH_UART_H__
+#define __MACH_UART_H__
+
+#define UART_CR_OFFSET          0x00  /* Control Register [8:0] */
+#define UART_SR_OFFSET          0x2C  /* Channel Status [11:0] */
+#define UART_FIFO_OFFSET        0x30  /* FIFO [15:0] or [7:0] */
+
+#define UART_SR_TXFULL		0x00000010	/* TX FIFO full */
+#define UART_SR_TXEMPTY		0x00000008	/* TX FIFO empty */
+
+#define UART_FIFO_WRITE(base, value) \
+	(*(volatile unsigned int *)((base) + UART_FIFO_OFFSET) = (value))
+
+#define UART_STATUS(base) \
+	(*(volatile unsigned int *)((base) + UART_SR_OFFSET))
+
+#endif
arch/arm/mach-xilinx/include/mach/uncompress.h new file +46 -0

46 lines not on this page · raw message

arch/arm/mach-xilinx/include/mach/vmalloc.h new file +20 -0
new file mode 100644
index 0000000..b70ff29
@@ -0,0 +1,20 @@
+/* arch/arm/mach-xilinx/include/mach/vmalloc.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __MACH_VMALLOC_H__
+#define __MACH_VMALLOC_H__
+
+#define VMALLOC_END       IO_BASE
+
+#endif
Signature · 5 lines
1.6.2.1



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.
#25 lore raw

PATCH 4/4 ARM: Xilinx: base header files and assembly macros

Quoted 83 lines · John Linn
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +77 -0
new file mode 100644
index 0000000..becb4cd
@@ -0,0 +1,87 @@
+/*
+ * arch/arm/mach-xilinx/include/mach/entry-macro.S
+ *
+ * Low-level IRQ helper macros
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * based on arch/plat-mxc/include/mach/entry-macro.S
+ *
+ *  Copyright (C) 2007 Lennert Buytenhek <[email protected]>
+ *  Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+#include <mach/hardware.h>
+#include <asm/hardware/gic.h>
+
+		.macro	disable_fiq
+		.endm
+
+		.macro  get_irqnr_preamble, base, tmp
+		ldr	\base, =gic_cpu_base_addr
+		ldr	\base, [\base]
+		.endm
+
+		.macro  arch_ret_to_user, tmp1, tmp2
+		.endm
+
+		/*
+		 * The interrupt numbering scheme is defined in the
+		 * interrupt controller spec.
+		 *
+		 * Interrupts 0-15 are SFI and are filtered
+		 * Interrupts 31-1019 are normal and should be processed
+		 * 1020-1021 are reserved and are filtered
+		 * 1022 is not needed and filtered
+		 * 1023 is "spurious" (no interrupt) and is filtered
+		 *
+		 * A simple read from the controller will tell us the number
+		 * of the highest priority enabled interrupt.  We then just
+		 * need to check whether it is in the valid range for an IRQ
+		 * (16-1019 inclusive). If a valid interrupt, Z = 0 on return.
+		 */
+
+		.macro  get_irqnr_and_base, irqnr, irqstat, base, tmp
+
+		/* bits 12-10 = src CPU, 9-0 = int # */
+
+		ldr     \irqstat, [\base, #GIC_CPU_INTACK]
+		ldr	\tmp, =1020
+
+		bic     \irqnr, \irqstat, #0x1c00
+
+		cmp     \irqnr, #31
+		cmpcc	\irqnr, \irqnr
+		cmpne	\irqnr, \tmp
+		cmpcs	\irqnr, \irqnr
+
+		.endm
+
+		.macro test_for_ipi, irqnr, irqstat, base, tmp
+		bic	\irqnr, \irqstat, #0x1c00
+		cmp	\irqnr, #16
+		it	cc
+		strcc	\irqstat, [\base, #GIC_CPU_EOI]
+		it	cs
+		cmpcs	\irqnr, \irqnr
+		.endm
+
+		/* As above, this assumes irqstat and base are preserved */

arch/arm/mach-xilinx/include/mach/entry-macro.S line 87

Quoted 10 lines
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +10 -0
+
+		.macro test_for_ltirq, irqnr, irqstat, base, tmp
+		bic	\irqnr, \irqstat, #0x1c00
+		mov 	\tmp, #0
+		cmp	\irqnr, #29
+		itt	eq
+		moveq	\tmp, #1
+		streq	\irqstat, [\base, #GIC_CPU_EOI]
+		cmp	\tmp, #0
+		.endm

Would arch/arm/include/asm/hardware/entry-macro-gic.S be suitable for use here?

Quoted 40 lines
arch/arm/mach-xilinx/include/mach/hardware.h new file +34 -0
new file mode 100644
index 0000000..a635322
@@ -0,0 +1,44 @@
+/* arch/arm/mach-xilinx/include/mach/hardware.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __MACH_HARDWARE_H__
+#define __MACH_HARDWARE_H__
+
+#include <mach/memory.h>
+#include <mach/irqs.h>
+
+#define PERIPHERAL_CLOCK_RATE	2500000
+
+#define IO_BASE			0xE0000000
+#define UART0_BASE		(IO_BASE)
+
+#define PERIPH_BASE		0xF8000000
+#define TTC0_BASE		(PERIPH_BASE + 0x1000)
+
+#define SCU_PERIPH_BASE		0xF8F00000
+#define SCU_GIC_CPU_BASE	(SCU_PERIPH_BASE + 0x100)
+#define SCU_GLOBAL_TIMER_BASE	(SCU_PERIPH_BASE + 0x200)
+#define SCU_CPU_TIMER_BASE	(SCU_PERIPH_BASE + 0x600)
+#define SCU_WDT_BASE		(SCU_PERIPH_BASE + 0x620)
+#define SCU_GIC_DIST_BASE	(SCU_PERIPH_BASE + 0x1000)

arch/arm/mach-xilinx/include/mach/hardware.h line 44

Quoted 10 lines
arch/arm/mach-xilinx/include/mach/hardware.h new file +10 -0
+
+#define PL310_L2CC_BASE		0xF8F02000
+
+/*
+ * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
+ */
+#define LL_UART_PADDR	UART0_BASE
+#define LL_UART_VADDR	UART0_BASE
+
+#endif

This file is included by much of the kernel build, so symbol conflicts tend to be a concern. It may be better to move most of this to a private header instead.

You may also like to consider using something like this:

#ifndef __ASSEMBLER__
#define IOMEM(a) ((void __force __iomem *)(a))
#else
#define IOMEM(a) a
#endif

#define SCU_PERIPH_BASE IOMEM(0xF8F00000)

which means it's typed correctly for most of the kernel. The only place where you have to use casts is in the map_desc array - that's pretty much unavoidable though.

Quoted 16 lines
arch/arm/mach-xilinx/include/mach/io.h new file +10 -0
new file mode 100644
index 0000000..2e69db7
@@ -0,0 +1,27 @@
+/* arch/arm/mach-xilinx/include/mach/io.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License

arch/arm/mach-xilinx/include/mach/io.h line 20

Quoted 10 lines
arch/arm/mach-xilinx/include/mach/io.h new file +10 -0
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __MACH_IO_H__
+#define __MACH_IO_H__
+
+/* Allow IO space to be anywhere in the memory */
+
+#define IO_SPACE_LIMIT 0xffffffff

Probably should be 0xffff as that's the standard PCI/ISA IO window.

arch/arm/mach-xilinx/include/mach/io.h line 24

Quoted 4 lines
arch/arm/mach-xilinx/include/mach/io.h new file +4 -0
+
+/* IO address mapping macros, nothing special at this time but required */
+
+#define __io(a)			((void __iomem *)(a))

#define io(a) __typesafe_io(a)

gives a nice, simple, typesafe solution.

Quoted 16 lines
arch/arm/mach-xilinx/include/mach/memory.h new file +10 -0
new file mode 100644
index 0000000..2b6d1eb
@@ -0,0 +1,23 @@
+/* arch/arm/mach-xilinx/include/mach/memory.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License

arch/arm/mach-xilinx/include/mach/memory.h line 20

Quoted 10 lines
arch/arm/mach-xilinx/include/mach/memory.h new file +10 -0
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __MACH_MEMORY_H__
+#define __MACH_MEMORY_H__
+
+#include <mach/hardware.h>
+
+#define PHYS_OFFSET             0x0

Eventually this will need to become PLAT_PHYS_OFFSET during the next merge window.

author
#26 lore raw

PATCH 4/4 ARM: Xilinx: base header files and assembly macros

Quoted 5 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Saturday, February 05, 2011 9:43 AM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; catalin.marinas at arm.com;

glikely at secretlab.ca

Quoted 1 line

Subject: Re: [PATCH 4/4] ARM: Xilinx: base header files and assembly

macros

Quoted 3 lines · John Linn

Review

Quoted 1 line
(quoted diff) +0 -0

b/arch/arm/mach-

Quoted 17 lines

xilinx/include/mach/entry-macro.S

Quoted 6 lines

new file mode 100644
index 0000000..becb4cd

arch/arm/mach-xilinx/include/mach/entry-macro.S new file +1 -0
@@ -0,0 +1,87 @@
+/*

arch/arm/mach-xilinx/include/mach/entry-macro.S line 11

Quoted 10 lines
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +10 -0
+ * arch/arm/mach-xilinx/include/mach/entry-macro.S
+ *
+ * Low-level IRQ helper macros
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * based on arch/plat-mxc/include/mach/entry-macro.S
+ *
+ *  Copyright (C) 2007 Lennert Buytenhek <[email protected]>
+ *  Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights

Reserved.

Quoted 2 lines

arch/arm/mach-xilinx/include/mach/entry-macro.S line 13

Quoted 2 lines
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +2 -0
+ *
+ * This program is free software; you can redistribute it and/or

modify

Quoted 1 line

arch/arm/mach-xilinx/include/mach/entry-macro.S line 14

Quoted 1 line
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +1 -0
+ * it under the terms of the GNU General Public License as

published by

Quoted 1 line

arch/arm/mach-xilinx/include/mach/entry-macro.S line 15

Quoted 1 line
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +1 -0
+ * the Free Software Foundation; either version 2 of the License,

or

Quoted 3 lines

arch/arm/mach-xilinx/include/mach/entry-macro.S line 18

Quoted 3 lines
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +3 -0
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public

License

Quoted 2 lines

arch/arm/mach-xilinx/include/mach/entry-macro.S line 20

Quoted 2 lines
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +2 -0
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA

02111-1307 USA

Quoted 18 lines
Quoted 18 lines
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +18 -0
+ */
+#include <mach/hardware.h>
+#include <asm/hardware/gic.h>
+
+		.macro	disable_fiq
+		.endm
+
+		.macro  get_irqnr_preamble, base, tmp
+		ldr	\base, =gic_cpu_base_addr
+		ldr	\base, [\base]
+		.endm
+
+		.macro  arch_ret_to_user, tmp1, tmp2
+		.endm
+
+		/*
+		 * The interrupt numbering scheme is defined in the
+		 * interrupt controller spec.
Quoted 8 lines

arch/arm/mach-xilinx/include/mach/entry-macro.S

Quoted 8 lines
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +8 -0
+		 *
+		 * Interrupts 0-15 are SFI and are filtered
+		 * Interrupts 31-1019 are normal and should be processed
+		 * 1020-1021 are reserved and are filtered
+		 * 1022 is not needed and filtered
+		 * 1023 is "spurious" (no interrupt) and is filtered
+		 *
+		 * A simple read from the controller will tell us the

number

Quoted 1 line

arch/arm/mach-xilinx/include/mach/entry-macro.S line 47

Quoted 1 line
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +1 -0
+		 * of the highest priority enabled interrupt.  We then

just

Quoted 1 line

arch/arm/mach-xilinx/include/mach/entry-macro.S line 48

Quoted 1 line
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +1 -0
+		 * need to check whether it is in the valid range for an

IRQ

Quoted 1 line

arch/arm/mach-xilinx/include/mach/entry-macro.S line 49

Quoted 1 line
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +1 -0
+		 * (16-1019 inclusive). If a valid interrupt, Z = 0 on

return.

Quoted 20 lines
Quoted 20 lines
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +20 -0
+		 */
+
+		.macro  get_irqnr_and_base, irqnr, irqstat, base, tmp
+
+		/* bits 12-10 = src CPU, 9-0 = int # */
+
+		ldr     \irqstat, [\base, #GIC_CPU_INTACK]
+		ldr	\tmp, =1020
+
+		bic     \irqnr, \irqstat, #0x1c00
+
+		cmp     \irqnr, #31
+		cmpcc	\irqnr, \irqnr
+		cmpne	\irqnr, \tmp
+		cmpcs	\irqnr, \irqnr
+
+		.endm
+
+		.macro test_for_ipi, irqnr, irqstat, base, tmp
+		bic	\irqnr, \irqstat, #0x1c00
Quoted 8 lines

arch/arm/mach-xilinx/include/mach/entry-macro.S

Quoted 8 lines
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +8 -0
+		cmp	\irqnr, #16
+		it	cc
+		strcc	\irqstat, [\base, #GIC_CPU_EOI]
+		it	cs
+		cmpcs	\irqnr, \irqnr
+		.endm
+
+		/* As above, this assumes irqstat and base are preserved

*/

Quoted 5 lines
Quoted 5 lines
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +5 -0
+
+		.macro test_for_ltirq, irqnr, irqstat, base, tmp
+		bic	\irqnr, \irqstat, #0x1c00
+		mov 	\tmp, #0
+		cmp	\irqnr, #29
Quoted 8 lines

arch/arm/mach-xilinx/include/mach/entry-macro.S line 87

Quoted 5 lines
arch/arm/mach-xilinx/include/mach/entry-macro.S new file +5 -0
+		itt	eq
+		moveq	\tmp, #1
+		streq	\irqstat, [\base, #GIC_CPU_EOI]
+		cmp	\tmp, #0
+		.endm

Would arch/arm/include/asm/hardware/entry-macro-gic.S be suitable for use here?

Looks like it could be, I'll give it a try.

Quoted 2 lines

Review

Quoted 2 lines
(quoted diff) +0 -0

b/arch/arm/mach-

Quoted 11 lines

xilinx/include/mach/hardware.h

arch/arm/mach-xilinx/include/mach/hardware.h line 5

Quoted 10 lines

new file mode 100644
index 0000000..a635322

arch/arm/mach-xilinx/include/mach/hardware.h new file +5 -0
@@ -0,0 +1,44 @@
+/* arch/arm/mach-xilinx/include/mach/hardware.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or

modify

Quoted 1 line

arch/arm/mach-xilinx/include/mach/hardware.h line 6

Quoted 1 line
arch/arm/mach-xilinx/include/mach/hardware.h new file +1 -0
+ * it under the terms of the GNU General Public License as

published by

Quoted 1 line

arch/arm/mach-xilinx/include/mach/hardware.h line 7

Quoted 1 line
arch/arm/mach-xilinx/include/mach/hardware.h new file +1 -0
+ * the Free Software Foundation; either version 2 of the License,

or

Quoted 3 lines

arch/arm/mach-xilinx/include/mach/hardware.h line 10

Quoted 3 lines
arch/arm/mach-xilinx/include/mach/hardware.h new file +3 -0
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public

License

Quoted 2 lines

arch/arm/mach-xilinx/include/mach/hardware.h line 12

Quoted 2 lines
arch/arm/mach-xilinx/include/mach/hardware.h new file +2 -0
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA

02111-1307 USA

Quoted 28 lines
Quoted 28 lines
arch/arm/mach-xilinx/include/mach/hardware.h new file +28 -0
+ */
+
+#ifndef __MACH_HARDWARE_H__
+#define __MACH_HARDWARE_H__
+
+#include <mach/memory.h>
+#include <mach/irqs.h>
+
+#define PERIPHERAL_CLOCK_RATE	2500000
+
+#define IO_BASE			0xE0000000
+#define UART0_BASE		(IO_BASE)
+
+#define PERIPH_BASE		0xF8000000
+#define TTC0_BASE		(PERIPH_BASE + 0x1000)
+
+#define SCU_PERIPH_BASE		0xF8F00000
+#define SCU_GIC_CPU_BASE	(SCU_PERIPH_BASE + 0x100)
+#define SCU_GLOBAL_TIMER_BASE	(SCU_PERIPH_BASE + 0x200)
+#define SCU_CPU_TIMER_BASE	(SCU_PERIPH_BASE + 0x600)
+#define SCU_WDT_BASE		(SCU_PERIPH_BASE + 0x620)
+#define SCU_GIC_DIST_BASE	(SCU_PERIPH_BASE + 0x1000)
+
+#define PL310_L2CC_BASE		0xF8F02000
+
+/*
+ * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
+ */
Quoted 9 lines

arch/arm/mach-xilinx/include/mach/hardware.h line 44

Quoted 4 lines
arch/arm/mach-xilinx/include/mach/hardware.h new file +4 -0
+#define LL_UART_PADDR	UART0_BASE
+#define LL_UART_VADDR	UART0_BASE
+
+#endif

This file is included by much of the kernel build, so symbol conflicts tend to be a concern. It may be better to move most of this to a private header instead.

Sure. Makes sense.

Quoted 11 lines

You may also like to consider using something like this:

#ifndef __ASSEMBLER__
#define IOMEM(a) ((void __force __iomem *)(a))
#else
#define IOMEM(a) a
#endif

#define SCU_PERIPH_BASE IOMEM(0xF8F00000)

which means it's typed correctly for most of the kernel. The only

place

Quoted 2 lines

where you have to use casts is in the map_desc array - that's pretty much unavoidable though.

OK.

Quoted 2 lines

Review

Quoted 2 lines
(quoted diff) +0 -0

b/arch/arm/mach-xilinx/include/mach/io.h

Quoted 10 lines

arch/arm/mach-xilinx/include/mach/io.h line 5

Quoted 10 lines

new file mode 100644
index 0000000..2e69db7

arch/arm/mach-xilinx/include/mach/io.h new file +5 -0
@@ -0,0 +1,27 @@
+/* arch/arm/mach-xilinx/include/mach/io.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or

modify

Quoted 1 line

arch/arm/mach-xilinx/include/mach/io.h line 6

Quoted 1 line
arch/arm/mach-xilinx/include/mach/io.h new file +1 -0
+ * it under the terms of the GNU General Public License as

published by

Quoted 1 line

arch/arm/mach-xilinx/include/mach/io.h line 7

Quoted 1 line
arch/arm/mach-xilinx/include/mach/io.h new file +1 -0
+ * the Free Software Foundation; either version 2 of the License,

or

Quoted 3 lines

arch/arm/mach-xilinx/include/mach/io.h line 10

Quoted 3 lines
arch/arm/mach-xilinx/include/mach/io.h new file +3 -0
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public

License

Quoted 2 lines

arch/arm/mach-xilinx/include/mach/io.h line 12

Quoted 2 lines
arch/arm/mach-xilinx/include/mach/io.h new file +2 -0
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA

02111-1307 USA

Quoted 10 lines

arch/arm/mach-xilinx/include/mach/io.h line 20

Quoted 8 lines
arch/arm/mach-xilinx/include/mach/io.h new file +8 -0
+ */
+
+#ifndef __MACH_IO_H__
+#define __MACH_IO_H__
+
+/* Allow IO space to be anywhere in the memory */
+
+#define IO_SPACE_LIMIT 0xffffffff

Probably should be 0xffff as that's the standard PCI/ISA IO window.

Makes sense. Now that you say that I realize how it's used.

Quoted 3 lines

arch/arm/mach-xilinx/include/mach/io.h line 22

Quoted 3 lines
arch/arm/mach-xilinx/include/mach/io.h new file +2 -0
+
+/* IO address mapping macros, nothing special at this time but

required */

Quoted 6 lines

arch/arm/mach-xilinx/include/mach/io.h line 24

Quoted 2 lines
arch/arm/mach-xilinx/include/mach/io.h new file +2 -0
+
+#define __io(a)			((void __iomem *)(a))

#define io(a) __typesafe_io(a)

gives a nice, simple, typesafe solution.

Ok, good.

Quoted 2 lines

Review

Quoted 2 lines
(quoted diff) +0 -0

b/arch/arm/mach-xilinx/include/mach/memory.h

Quoted 10 lines

arch/arm/mach-xilinx/include/mach/memory.h line 5

Quoted 10 lines

new file mode 100644
index 0000000..2b6d1eb

arch/arm/mach-xilinx/include/mach/memory.h new file +5 -0
@@ -0,0 +1,23 @@
+/* arch/arm/mach-xilinx/include/mach/memory.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or

modify

Quoted 1 line

arch/arm/mach-xilinx/include/mach/memory.h line 6

Quoted 1 line
arch/arm/mach-xilinx/include/mach/memory.h new file +1 -0
+ * it under the terms of the GNU General Public License as

published by

Quoted 1 line

arch/arm/mach-xilinx/include/mach/memory.h line 7

Quoted 1 line
arch/arm/mach-xilinx/include/mach/memory.h new file +1 -0
+ * the Free Software Foundation; either version 2 of the License,

or

Quoted 3 lines

arch/arm/mach-xilinx/include/mach/memory.h line 10

Quoted 3 lines
arch/arm/mach-xilinx/include/mach/memory.h new file +3 -0
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public

License

Quoted 2 lines

arch/arm/mach-xilinx/include/mach/memory.h line 12

Quoted 2 lines
arch/arm/mach-xilinx/include/mach/memory.h new file +2 -0
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA

02111-1307 USA

Quoted 11 lines

arch/arm/mach-xilinx/include/mach/memory.h line 20

Quoted 8 lines
arch/arm/mach-xilinx/include/mach/memory.h new file +8 -0
+ */
+
+#ifndef __MACH_MEMORY_H__
+#define __MACH_MEMORY_H__
+
+#include <mach/hardware.h>
+
+#define PHYS_OFFSET             0x0

Eventually this will need to become PLAT_PHYS_OFFSET during the next merge window.

Yes, for now I'm assuming keep it the same.

I'm really hoping to get into the next merge window if possible.

Thanks again for your time,
John

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.

author
#27 lore raw

PATCH 4/4 ARM: Xilinx: base header files and assembly macros

In reply to Russell King - ARM Linux
Quoted 5 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Saturday, February 05, 2011 9:43 AM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; catalin.marinas at arm.com;

glikely at secretlab.ca

Quoted 1 line

Subject: Re: [PATCH 4/4] ARM: Xilinx: base header files and assembly

macros

Quoted 1 line

<snip>

Quoted 2 lines

Review

Quoted 2 lines
(quoted diff) +0 -0

b/arch/arm/mach-

Quoted 11 lines

xilinx/include/mach/hardware.h

arch/arm/mach-xilinx/include/mach/hardware.h line 5

Quoted 10 lines

new file mode 100644
index 0000000..a635322

arch/arm/mach-xilinx/include/mach/hardware.h new file +5 -0
@@ -0,0 +1,44 @@
+/* arch/arm/mach-xilinx/include/mach/hardware.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or

modify

Quoted 1 line

arch/arm/mach-xilinx/include/mach/hardware.h line 6

Quoted 1 line
arch/arm/mach-xilinx/include/mach/hardware.h new file +1 -0
+ * it under the terms of the GNU General Public License as

published by

Quoted 1 line

arch/arm/mach-xilinx/include/mach/hardware.h line 7

Quoted 1 line
arch/arm/mach-xilinx/include/mach/hardware.h new file +1 -0
+ * the Free Software Foundation; either version 2 of the License,

or

Quoted 3 lines

arch/arm/mach-xilinx/include/mach/hardware.h line 10

Quoted 3 lines
arch/arm/mach-xilinx/include/mach/hardware.h new file +3 -0
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public

License

Quoted 2 lines

arch/arm/mach-xilinx/include/mach/hardware.h line 12

Quoted 2 lines
arch/arm/mach-xilinx/include/mach/hardware.h new file +2 -0
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA

02111-1307 USA

Quoted 45 lines
Quoted 22 lines
arch/arm/mach-xilinx/include/mach/hardware.h new file +22 -0
+ */
+
+#ifndef __MACH_HARDWARE_H__
+#define __MACH_HARDWARE_H__
+
+#include <mach/memory.h>
+#include <mach/irqs.h>
+
+#define PERIPHERAL_CLOCK_RATE	2500000
+
+#define IO_BASE			0xE0000000
+#define UART0_BASE		(IO_BASE)
+
+#define PERIPH_BASE		0xF8000000
+#define TTC0_BASE		(PERIPH_BASE + 0x1000)
+
+#define SCU_PERIPH_BASE		0xF8F00000
+#define SCU_GIC_CPU_BASE	(SCU_PERIPH_BASE + 0x100)
+#define SCU_GLOBAL_TIMER_BASE	(SCU_PERIPH_BASE + 0x200)
+#define SCU_CPU_TIMER_BASE	(SCU_PERIPH_BASE + 0x600)
+#define SCU_WDT_BASE		(SCU_PERIPH_BASE + 0x620)
+#define SCU_GIC_DIST_BASE	(SCU_PERIPH_BASE + 0x1000)

arch/arm/mach-xilinx/include/mach/hardware.h line 44

Quoted 10 lines
arch/arm/mach-xilinx/include/mach/hardware.h new file +10 -0
+
+#define PL310_L2CC_BASE		0xF8F02000
+
+/*
+ * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
+ */
+#define LL_UART_PADDR	UART0_BASE
+#define LL_UART_VADDR	UART0_BASE
+
+#endif

This file is included by much of the kernel build, so symbol conflicts tend to be a concern. It may be better to move most of this to a private header instead.

You may also like to consider using something like this:

#ifndef __ASSEMBLER__
#define IOMEM(a) ((void __force __iomem *)(a))
#else
#define IOMEM(a) a
#endif

Quoted 3 lines

#define SCU_PERIPH_BASE IOMEM(0xF8F00000)

which means it's typed correctly for most of the kernel. The only

place

Quoted 2 lines

where you have to use casts is in the map_desc array - that's pretty much unavoidable though.

Hi Russell,

I'm assuming you meant to change all addresses of devices (#defines) to use the IOMEM.

I did that and it's ok. You also have to cast them to ints when putting the addresses into
the resource for a platform device.

It makes sense that the typing is now better in general.

Thanks,
John

Quoted 2 lines

Review

Quoted 2 lines
(quoted diff) +0 -0

b/arch/arm/mach-xilinx/include/mach/io.h

Quoted 10 lines

arch/arm/mach-xilinx/include/mach/io.h line 5

Quoted 10 lines

new file mode 100644
index 0000000..2e69db7

arch/arm/mach-xilinx/include/mach/io.h new file +5 -0
@@ -0,0 +1,27 @@
+/* arch/arm/mach-xilinx/include/mach/io.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or

modify

Quoted 1 line

arch/arm/mach-xilinx/include/mach/io.h line 6

Quoted 1 line
arch/arm/mach-xilinx/include/mach/io.h new file +1 -0
+ * it under the terms of the GNU General Public License as

published by

Quoted 1 line

arch/arm/mach-xilinx/include/mach/io.h line 7

Quoted 1 line
arch/arm/mach-xilinx/include/mach/io.h new file +1 -0
+ * the Free Software Foundation; either version 2 of the License,

or

Quoted 3 lines

arch/arm/mach-xilinx/include/mach/io.h line 10

Quoted 3 lines
arch/arm/mach-xilinx/include/mach/io.h new file +3 -0
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public

License

Quoted 2 lines

arch/arm/mach-xilinx/include/mach/io.h line 12

Quoted 2 lines
arch/arm/mach-xilinx/include/mach/io.h new file +2 -0
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA

02111-1307 USA

Quoted 11 lines

arch/arm/mach-xilinx/include/mach/io.h line 20

Quoted 8 lines
arch/arm/mach-xilinx/include/mach/io.h new file +8 -0
+ */
+
+#ifndef __MACH_IO_H__
+#define __MACH_IO_H__
+
+/* Allow IO space to be anywhere in the memory */
+
+#define IO_SPACE_LIMIT 0xffffffff

Probably should be 0xffff as that's the standard PCI/ISA IO window.

Quoted 2 lines

arch/arm/mach-xilinx/include/mach/io.h line 22

Quoted 2 lines
arch/arm/mach-xilinx/include/mach/io.h new file +2 -0
+
+/* IO address mapping macros, nothing special at this time but

required */

Quoted 8 lines

arch/arm/mach-xilinx/include/mach/io.h line 24

Quoted 2 lines
arch/arm/mach-xilinx/include/mach/io.h new file +2 -0
+
+#define __io(a)			((void __iomem *)(a))

#define io(a) __typesafe_io(a)

gives a nice, simple, typesafe solution.

Review

Quoted 1 line
(quoted diff) +0 -0

b/arch/arm/mach-xilinx/include/mach/memory.h

Quoted 10 lines

arch/arm/mach-xilinx/include/mach/memory.h line 5

Quoted 10 lines

new file mode 100644
index 0000000..2b6d1eb

arch/arm/mach-xilinx/include/mach/memory.h new file +5 -0
@@ -0,0 +1,23 @@
+/* arch/arm/mach-xilinx/include/mach/memory.h
+ *
+ *  Copyright (C) 2011 Xilinx
+ *
+ * This program is free software; you can redistribute it and/or

modify

Quoted 1 line

arch/arm/mach-xilinx/include/mach/memory.h line 6

Quoted 1 line
arch/arm/mach-xilinx/include/mach/memory.h new file +1 -0
+ * it under the terms of the GNU General Public License as

published by

Quoted 1 line

arch/arm/mach-xilinx/include/mach/memory.h line 7

Quoted 1 line
arch/arm/mach-xilinx/include/mach/memory.h new file +1 -0
+ * the Free Software Foundation; either version 2 of the License,

or

Quoted 3 lines

arch/arm/mach-xilinx/include/mach/memory.h line 10

Quoted 3 lines
arch/arm/mach-xilinx/include/mach/memory.h new file +3 -0
+ * (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public

License

Quoted 2 lines

arch/arm/mach-xilinx/include/mach/memory.h line 12

Quoted 2 lines
arch/arm/mach-xilinx/include/mach/memory.h new file +2 -0
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA

02111-1307 USA

Quoted 11 lines

arch/arm/mach-xilinx/include/mach/memory.h line 20

Quoted 8 lines
arch/arm/mach-xilinx/include/mach/memory.h new file +8 -0
+ */
+
+#ifndef __MACH_MEMORY_H__
+#define __MACH_MEMORY_H__
+
+#include <mach/hardware.h>
+
+#define PHYS_OFFSET             0x0

Eventually this will need to become PLAT_PHYS_OFFSET during the next merge window.

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.

#28 lore raw

PATCH 4/4 ARM: Xilinx: base header files and assembly macros

Quoted 6 lines · John Linn

I'm assuming you meant to change all addresses of devices (#defines) to use the IOMEM.

I did that and it's ok. You also have to cast them to ints when putting the addresses into
the resource for a platform device.

Hang on. The reason I mentioned it was because you were passing some of these into readl() et.al. readl() takes the mapped cookie, not the physical address.

Physical addresses should be integer like, virtual addresses should be pointer like.

author
#29 lore raw

PATCH 4/4 ARM: Xilinx: base header files and assembly macros

Quoted 5 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Tuesday, February 08, 2011 1:27 AM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; catalin.marinas at arm.com;

glikely at secretlab.ca

Quoted 1 line

Subject: Re: [PATCH 4/4] ARM: Xilinx: base header files and assembly

macros

Quoted 3 lines · John Linn
Quoted 1 line

I'm assuming you meant to change all addresses of devices (#defines)

to

Quoted 3 lines
Quoted 3 lines

use the IOMEM.

I did that and it's ok. You also have to cast them to ints when

putting

Quoted 9 lines
Quoted 2 lines

the addresses into
the resource for a platform device.

Hang on. The reason I mentioned it was because you were passing some of these into readl() et.al. readl() takes the mapped cookie, not the physical address.

Physical addresses should be integer like, virtual addresses should be pointer like.

Thanks for clarifying.

It seems like it's better to leave the #defines without IOMEM, then use IOMEM where a variable,
such as the address of the timer, is initialized with the #define.

I see what you mean with regard to readl taking the cookie but yet physical addresses are also
needed.

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.

#30 lore raw

PATCH 4/4 ARM: Xilinx: base header files and assembly macros

Quoted 26 lines · John Linn
Quoted 5 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Tuesday, February 08, 2011 1:27 AM
To: John Linn
Cc: linux-arm-kernel at lists.infradead.org; catalin.marinas at arm.com;

glikely at secretlab.ca

Quoted 1 line

Subject: Re: [PATCH 4/4] ARM: Xilinx: base header files and assembly

macros

Quoted 3 lines · John Linn
Quoted 1 line

I'm assuming you meant to change all addresses of devices (#defines)

to

Quoted 3 lines
Quoted 3 lines

use the IOMEM.

I did that and it's ok. You also have to cast them to ints when

putting

Quoted 9 lines
Quoted 2 lines

the addresses into
the resource for a platform device.

Hang on. The reason I mentioned it was because you were passing some of these into readl() et.al. readl() takes the mapped cookie, not the physical address.

Physical addresses should be integer like, virtual addresses should be pointer like.

Quoted 8 lines

Thanks for clarifying.

It seems like it's better to leave the #defines without IOMEM, then use IOMEM where a variable,
such as the address of the timer, is initialized with the #define.

I see what you mean with regard to readl taking the cookie but yet physical addresses are also needed.

readl does not take physical addresses. Ever.

I can't work out what's going on here at the moment, and until I do I don't see any justification to merging this as long as there is confusion between what is a physical address and what isn't.