2011-02-18 18:15:00

by John Linn

Subject: [PATCH V4 3/4] ARM: Xilinx: base header files and assembly macros

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

Signed-off-by: John Linn <[email protected]>
---

V4 Changes

Cleanup based on input from Jamie Iles.
Moved to PLAT_PHYS_OFFSET.
Remove headers file inclusions that weren't needed.
Added use of raw_readl/raw_writel in uncompressor uart code.
Added cpu_relax() in the uncompressor uart code.

V3 Changes

Updates based on Russell Kings' comments
Changed headers to update the license info and remove
the address

Updated the constant to use UL in vmalloc.h

V2 Changes

Updates based on Russell King's comments
moved to using entry-macro-gic.S
moved stuff from hardware.h to xilinx_soc.h
added IOMEM() in io.h to help with typing
Minor updates to IO_SPACE_LIMIT and __io()
Updated addresses to be clear with seperate virtual
and physical addresses

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 | 30 +++++++++++
arch/arm/mach-xilinx/include/mach/hardware.h | 18 +++++++
arch/arm/mach-xilinx/include/mach/io.h | 33 ++++++++++++
arch/arm/mach-xilinx/include/mach/irqs.h | 29 +++++++++++
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 | 25 +++++++++
arch/arm/mach-xilinx/include/mach/uncompress.h | 61 +++++++++++++++++++++++
arch/arm/mach-xilinx/include/mach/vmalloc.h | 20 +++++++
arch/arm/mach-xilinx/include/mach/xilinx_soc.h | 46 +++++++++++++++++
13 files changed, 404 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
create mode 100644 arch/arm/mach-xilinx/include/mach/xilinx_soc.h

diff --git a/arch/arm/mach-xilinx/include/mach/clkdev.h b/arch/arm/mach-xilinx/include/mach/clkdev.h
new file mode 100644
index 0000000..5dd1453
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/clkdev.h
@@ -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
diff --git a/arch/arm/mach-xilinx/include/mach/debug-macro.S b/arch/arm/mach-xilinx/include/mach/debug-macro.S
new file mode 100644
index 0000000..30d3d3d
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/debug-macro.S
@@ -0,0 +1,36 @@
+/* arch/arm/mach-xilinx/include/mach/debug-macro.S
+ *
+ * Debugging macro include header
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * 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.
+ */
+
+#include <mach/xilinx_soc.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
diff --git a/arch/arm/mach-xilinx/include/mach/entry-macro.S b/arch/arm/mach-xilinx/include/mach/entry-macro.S
new file mode 100644
index 0000000..11a2866
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/entry-macro.S
@@ -0,0 +1,30 @@
+/*
+ * 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 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.
+ */
+
+#include <mach/hardware.h>
+#include <asm/hardware/entry-macro-gic.S>
+
+ .macro disable_fiq
+ .endm
+
+ .macro arch_ret_to_user, tmp1, tmp2
+ .endm
diff --git a/arch/arm/mach-xilinx/include/mach/hardware.h b/arch/arm/mach-xilinx/include/mach/hardware.h
new file mode 100644
index 0000000..f9685c4
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/hardware.h
@@ -0,0 +1,18 @@
+/* arch/arm/mach-xilinx/include/mach/hardware.h
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * 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_HARDWARE_H__
+#define __MACH_HARDWARE_H__
+
+#endif
diff --git a/arch/arm/mach-xilinx/include/mach/io.h b/arch/arm/mach-xilinx/include/mach/io.h
new file mode 100644
index 0000000..ef69e65
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/io.h
@@ -0,0 +1,33 @@
+/* arch/arm/mach-xilinx/include/mach/io.h
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * 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_IO_H__
+#define __MACH_IO_H__
+
+/* Allow IO space to be anywhere in the memory */
+
+#define IO_SPACE_LIMIT 0xffff
+
+/* IO address mapping macros, nothing special at this time but required */
+
+#ifdef __ASSEMBLER__
+#define IOMEM(x) (x)
+#else
+#define IOMEM(x) ((void __force __iomem *)(x))
+#endif
+
+#define __io(a) __typesafe_io(a)
+#define __mem_pci(a) (a)
+
+#endif
diff --git a/arch/arm/mach-xilinx/include/mach/irqs.h b/arch/arm/mach-xilinx/include/mach/irqs.h
new file mode 100644
index 0000000..47a8162
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/irqs.h
@@ -0,0 +1,29 @@
+/* arch/arm/mach-xilinx/include/mach/irqs.h
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * 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_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
diff --git a/arch/arm/mach-xilinx/include/mach/memory.h b/arch/arm/mach-xilinx/include/mach/memory.h
new file mode 100644
index 0000000..3efde15
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/memory.h
@@ -0,0 +1,23 @@
+/* arch/arm/mach-xilinx/include/mach/memory.h
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * 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_MEMORY_H__
+#define __MACH_MEMORY_H__
+
+#include <asm/sizes.h>
+
+#define PHYS_OFFSET UL(0x0)
+#define MEM_SIZE SZ_256M
+
+#endif
diff --git a/arch/arm/mach-xilinx/include/mach/system.h b/arch/arm/mach-xilinx/include/mach/system.h
new file mode 100644
index 0000000..e8514a0
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/system.h
@@ -0,0 +1,28 @@
+/* arch/arm/mach-xilinx/include/mach/system.h
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * 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_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
diff --git a/arch/arm/mach-xilinx/include/mach/timex.h b/arch/arm/mach-xilinx/include/mach/timex.h
new file mode 100644
index 0000000..4ebc0a6
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/timex.h
@@ -0,0 +1,22 @@
+/* arch/arm/mach-xilinx/include/mach/timex.h
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * 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_TIMEX_H__
+#define __MACH_TIMEX_H__
+
+#define PERIPHERAL_CLOCK_RATE 2500000
+
+#define CLOCK_TICK_RATE (PERIPHERAL_CLOCK_RATE / 32)
+
+#endif
diff --git a/arch/arm/mach-xilinx/include/mach/uart.h b/arch/arm/mach-xilinx/include/mach/uart.h
new file mode 100644
index 0000000..7fca361
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/uart.h
@@ -0,0 +1,25 @@
+/* arch/arm/mach-xilinx/include/mach/uart.h
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * 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_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 */
+
+#endif
diff --git a/arch/arm/mach-xilinx/include/mach/uncompress.h b/arch/arm/mach-xilinx/include/mach/uncompress.h
new file mode 100644
index 0000000..ff3754c
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/uncompress.h
@@ -0,0 +1,61 @@
+/* arch/arm/mach-xilinx/include/mach/uncompress.h
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * 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_UNCOMPRESS_H__
+#define __MACH_UNCOMPRESS_H__
+
+#include <mach/xilinx_soc.h>
+#include <mach/uart.h>
+#include <asm/processor.h>
+#include <asm/io.h>
+#include <mach/io.h>
+
+void arch_decomp_setup(void)
+{
+}
+
+static inline void flush(void)
+{
+ u32 status;
+ /*
+ * Wait while the FIFO is not empty
+ */
+ while (1) {
+ status = __raw_readl(IOMEM(LL_UART_PADDR + UART_SR_OFFSET));
+ if (status & UART_SR_TXEMPTY)
+ break;
+ cpu_relax();
+ }
+}
+
+#define arch_decomp_wdog()
+
+static void putc(char ch)
+{
+ u32 status;
+
+ /*
+ * Wait for room in the FIFO, then write the char into the FIFO
+ */
+ while (1) {
+ status = __raw_readl(IOMEM(LL_UART_PADDR + UART_SR_OFFSET));
+ if (!(status & UART_SR_TXFULL))
+ break;
+ cpu_relax();
+ }
+
+ __raw_writel(ch, IOMEM(LL_UART_PADDR + UART_FIFO_OFFSET));
+}
+
+#endif
diff --git a/arch/arm/mach-xilinx/include/mach/vmalloc.h b/arch/arm/mach-xilinx/include/mach/vmalloc.h
new file mode 100644
index 0000000..aba20a3
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/vmalloc.h
@@ -0,0 +1,20 @@
+/* arch/arm/mach-xilinx/include/mach/vmalloc.h
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * 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_VMALLOC_H__
+#define __MACH_VMALLOC_H__
+
+#define VMALLOC_END 0xE0000000UL
+
+#endif
diff --git a/arch/arm/mach-xilinx/include/mach/xilinx_soc.h b/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
new file mode 100644
index 0000000..d01cde1
--- /dev/null
+++ b/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
@@ -0,0 +1,46 @@
+/* arch/arm/mach-xilinx/include/mach/xilinx_soc.h
+ *
+ * Copyright (C) 2011 Xilinx
+ *
+ * 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_XILINX_SOC_H__
+#define __MACH_XILINX_SOC_H__
+
+/* For now, all mappings are flat (physical = virtual)
+ */
+#define UART0_PHYS 0xE0000000
+#define UART0_VIRT UART0_PHYS
+
+#define TTC0_PHYS 0xF8001000
+#define TTC0_VIRT TTC0_PHYS
+
+#define PL310_L2CC_PHYS 0xF8F02000
+#define PL310_L2CC_VIRT PL310_L2CC_PHYS
+
+#define SCU_PERIPH_PHYS 0xF8F00000
+#define SCU_PERIPH_VIRT SCU_PERIPH_PHYS
+
+/* The following are intended for the devices that are mapped early */
+
+#define TTC0_BASE IOMEM(TTC0_VIRT)
+#define SCU_PERIPH_BASE IOMEM(SCU_PERIPH_VIRT)
+#define SCU_GIC_CPU_BASE (SCU_PERIPH_BASE + 0x100)
+#define SCU_GIC_DIST_BASE (SCU_PERIPH_BASE + 0x1000)
+#define PL310_L2CC_BASE IOMEM(PL310_L2CC_VIRT)
+
+/*
+ * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
+ */
+#define LL_UART_PADDR UART0_PHYS
+#define LL_UART_VADDR UART0_VIRT
+
+#endif
--
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.

2011-02-20 21:37:57

by Arnd Bergmann

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

On Friday 18 February 2011, John Linn wrote:
> +
> +/* IO address mapping macros, nothing special at this time but required */
> +
> +#ifdef __ASSEMBLER__
> +#define IOMEM(x) (x)
> +#else
> +#define IOMEM(x) ((void __force __iomem *)(x))
> +#endif
> +
> +#define __io(a) __typesafe_io(a)
> +#define __mem_pci(a) (a)

Are you planning to support PCI or PCMCIA? If so, the __io definition will have to
change so it points to the PIO register window.

> diff --git a/arch/arm/mach-xilinx/include/mach/timex.h b/arch/arm/mach-xilinx/include/mach/timex.h
> new file mode 100644
> index 0000000..4ebc0a6
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/timex.h
> +#ifndef __MACH_TIMEX_H__
> +#define __MACH_TIMEX_H__
> +
> +#define PERIPHERAL_CLOCK_RATE 2500000
> +
> +#define CLOCK_TICK_RATE (PERIPHERAL_CLOCK_RATE / 32)
> +
> +#endif

I thought we were at the point where CLOCK_TICK_RATE is no longer used.
Did the patches not make it in yet?

The rest looks fine to me.

Arnd

2011-02-21 00:18:30

by John Linn

Subject: RE: [PATCH V4 3/4] ARM: Xilinx: base header files and assembly macros

> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Sunday, February 20, 2011 2:38 PM
> To: John Linn
> Cc: [email protected];
[email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH V4 3/4] ARM: Xilinx: base header files and
assembly
> macros
>
> On Friday 18 February 2011, John Linn wrote:
> > +
> > +/* IO address mapping macros, nothing special at this time but
> required */
> > +
> > +#ifdef __ASSEMBLER__
> > +#define IOMEM(x) (x)
> > +#else
> > +#define IOMEM(x) ((void __force __iomem *)(x))
> > +#endif
> > +
> > +#define __io(a) __typesafe_io(a)
> > +#define __mem_pci(a) (a)
>
> Are you planning to support PCI or PCMCIA? If so, the __io definition
> will have to
> change so it points to the PIO register window.

No immediate plans for PCI or PCMCIA. Makes sense.

>
> > diff --git a/arch/arm/mach-xilinx/include/mach/timex.h
> b/arch/arm/mach-xilinx/include/mach/timex.h
> > new file mode 100644
> > index 0000000..4ebc0a6
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/timex.h
> > +#ifndef __MACH_TIMEX_H__
> > +#define __MACH_TIMEX_H__
> > +
> > +#define PERIPHERAL_CLOCK_RATE 2500000
> > +
> > +#define CLOCK_TICK_RATE (PERIPHERAL_CLOCK_RATE / 32)
> > +
> > +#endif
>
> I thought we were at the point where CLOCK_TICK_RATE is no longer
used.

The timer code in these patches is using it, no other comments on it so
far.

> Did the patches not make it in yet?
>

I haven't heard they made them in yet, just waiting and hoping for an
ack.

> The rest looks fine to me.

Great, appreciate the review and your time.

-- John

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

2011-02-21 08:36:53

by Arnd Bergmann

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

On Monday 21 February 2011, John Linn wrote:
> > > diff --git a/arch/arm/mach-xilinx/include/mach/timex.h
> > b/arch/arm/mach-xilinx/include/mach/timex.h
> > > new file mode 100644
> > > index 0000000..4ebc0a6
> > > --- /dev/null
> > > +++ b/arch/arm/mach-xilinx/include/mach/timex.h
> > > +#ifndef __MACH_TIMEX_H__
> > > +#define __MACH_TIMEX_H__
> > > +
> > > +#define PERIPHERAL_CLOCK_RATE 2500000
> > > +
> > > +#define CLOCK_TICK_RATE (PERIPHERAL_CLOCK_RATE / 32)
> > > +
> > > +#endif
> >
> > I thought we were at the point where CLOCK_TICK_RATE is no longer
> used.
>
> The timer code in these patches is using it, no other comments on it so
> far.
>
> > Did the patches not make it in yet?
> >
>
> I haven't heard they made them in yet, just waiting and hoping for an
> ack.

I meant the patches removing CLOCK_TICK_RATE from common code, not your
patches, sorry for being vague.

It would be better if you could avoid introducing new uses of
CLOCK_TICK_RATE, because that will have to be removed before we can
move to a real multi-platform kernel. For instance, you can put
PERIPHERAL_CLOCK_RATE in a hardware specific header and put a bogus
definition for CLOCK_TICK_RATE into timex.h

Arnd

2011-02-21 14:37:05

by John Linn

Subject: RE: [PATCH V4 3/4] ARM: Xilinx: base header files and assembly macros

> -----Original Message-----
> From: Arnd Bergmann [mailto:[email protected]]
> Sent: Monday, February 21, 2011 1:37 AM
> To: John Linn
> Cc: [email protected];
[email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH V4 3/4] ARM: Xilinx: base header files and
assembly
> macros
>
> On Monday 21 February 2011, John Linn wrote:
> > > > diff --git a/arch/arm/mach-xilinx/include/mach/timex.h
> > > b/arch/arm/mach-xilinx/include/mach/timex.h
> > > > new file mode 100644
> > > > index 0000000..4ebc0a6
> > > > --- /dev/null
> > > > +++ b/arch/arm/mach-xilinx/include/mach/timex.h
> > > > +#ifndef __MACH_TIMEX_H__
> > > > +#define __MACH_TIMEX_H__
> > > > +
> > > > +#define PERIPHERAL_CLOCK_RATE 2500000
> > > > +
> > > > +#define CLOCK_TICK_RATE (PERIPHERAL_CLOCK_RATE / 32)
> > > > +
> > > > +#endif
> > >
> > > I thought we were at the point where CLOCK_TICK_RATE is no longer
> > used.
> >
> > The timer code in these patches is using it, no other comments on it
> so
> > far.
> >
> > > Did the patches not make it in yet?
> > >
> >
> > I haven't heard they made them in yet, just waiting and hoping for
an
> > ack.
>
> I meant the patches removing CLOCK_TICK_RATE from common code, not
your
> patches, sorry for being vague.

Ahh... understand. No problem, wishful thinking on my part :)

>
> It would be better if you could avoid introducing new uses of
> CLOCK_TICK_RATE, because that will have to be removed before we can
> move to a real multi-platform kernel. For instance, you can put
> PERIPHERAL_CLOCK_RATE in a hardware specific header and put a bogus
> definition for CLOCK_TICK_RATE into timex.h

Hi Arnd,

I can remove the use of it from my timer code, but I still see
CLOCK_TICK_RATE
being used in linux/jiffies.h.

I tried removing it and got build errors. Am I missing something there?

Thanks,
John

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

2011-02-21 14:49:03

by Arnd Bergmann

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

On Monday 21 February 2011, John Linn wrote:
> > It would be better if you could avoid introducing new uses of
> > CLOCK_TICK_RATE, because that will have to be removed before we can
> > move to a real multi-platform kernel. For instance, you can put
> > PERIPHERAL_CLOCK_RATE in a hardware specific header and put a bogus
> > definition for CLOCK_TICK_RATE into timex.h
>
> Hi Arnd,
>
> I can remove the use of it from my timer code, but I still see
> CLOCK_TICK_RATE
> being used in linux/jiffies.h.
>
> I tried removing it and got build errors. Am I missing something there?
>

You still need to have some definition for CLOCK_TICK_RATE to make
the jiffies code compile, but the actual value no longer matters.

Traditionally, this was the i386-style PIT timer frequency of 11.93 MHz
for some things and the actual timer frequency for other things.
The timer code tries to correct the jiffies value based on difference
between the intended HZ frequency (100, 250 or 1000) and the actual
on (something very close to that, but not exact).

You can e.g. set it to (100 * HZ) to make something up that won't
hurt in that calculation.

Arnd

2011-02-21 15:06:06

by John Linn

Subject: RE: [PATCH V4 3/4] ARM: Xilinx: base header files and assembly macros

> -----Original Message-----
> From: [email protected] [mailto:linux-arm-
> [email protected]] On Behalf Of Arnd Bergmann
> Sent: Monday, February 21, 2011 7:49 AM
> To: John Linn
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH V4 3/4] ARM: Xilinx: base header files and
assembly
> macros
>
> On Monday 21 February 2011, John Linn wrote:
> > > It would be better if you could avoid introducing new uses of
> > > CLOCK_TICK_RATE, because that will have to be removed before we
can
> > > move to a real multi-platform kernel. For instance, you can put
> > > PERIPHERAL_CLOCK_RATE in a hardware specific header and put a
bogus
> > > definition for CLOCK_TICK_RATE into timex.h
> >
> > Hi Arnd,
> >
> > I can remove the use of it from my timer code, but I still see
> > CLOCK_TICK_RATE
> > being used in linux/jiffies.h.
> >
> > I tried removing it and got build errors. Am I missing something
> there?
> >
>
> You still need to have some definition for CLOCK_TICK_RATE to make
> the jiffies code compile, but the actual value no longer matters.

I see. Now that I re-read your last response I get that from it.

>
> Traditionally, this was the i386-style PIT timer frequency of 11.93
MHz
> for some things and the actual timer frequency for other things.
> The timer code tries to correct the jiffies value based on difference
> between the intended HZ frequency (100, 250 or 1000) and the actual
> on (something very close to that, but not exact).
>
> You can e.g. set it to (100 * HZ) to make something up that won't
> hurt in that calculation.
>

Gotcha. I now understand to stay away from using CLOCK_TICK_RATE.

Sounds like it's worth a comment in mach/timex.h that says it's not
really used as it seems a bit misleading for others.

Thanks,
John

> Arnd
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


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.

2011-02-21 15:18:10

by Russell King - ARM Linux

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

On Mon, Feb 21, 2011 at 03:48:36PM +0100, Arnd Bergmann wrote:
> On Monday 21 February 2011, John Linn wrote:
> > > It would be better if you could avoid introducing new uses of
> > > CLOCK_TICK_RATE, because that will have to be removed before we can
> > > move to a real multi-platform kernel. For instance, you can put
> > > PERIPHERAL_CLOCK_RATE in a hardware specific header and put a bogus
> > > definition for CLOCK_TICK_RATE into timex.h
> >
> > Hi Arnd,
> >
> > I can remove the use of it from my timer code, but I still see
> > CLOCK_TICK_RATE
> > being used in linux/jiffies.h.
> >
> > I tried removing it and got build errors. Am I missing something there?
> >
>
> You still need to have some definition for CLOCK_TICK_RATE to make
> the jiffies code compile, but the actual value no longer matters.

This is what I said a while back:
| If you switch to clocksource/clockevents, then I think CLOCK_TICK_RATE
| is irrelevant as time advances according to the interval measured by
| the previous and current clocksource read, rather than 1/HZ intervals.
|
| However, I'm never happy to say "just set CLOCK_TICK_RATE to some random
| value that's a multiple of HZ" because I can't convince myself that these
| don't have any effect when using clocksources. The list of symbols which
| depend on CLOCK_TICK_RATE are:
|
| ACTHZ
| LATCH
| TICK_NSEC
| TICK_USEC_TO_NSEC
| LOW_RES_NSEC
| MONOTONIC_RES_NSEC
| NSEC_PER_JIFFY
| KTIME_LOW_RES
|
| and if you grep for those outside of arch/, you find them being used in
| a fair amount of code under kernel/, as well as the odd driver here and
| there.

Eg, LOW_RES_NSEC is exported to userspace via the posix clocks interface.

NSEC_PER_SEC and TICK_NSEC are used for cmos clock updates, so probably
don't matter too much there. TICK_NSEC is also used by the scheduler,
time conversions (timespec/timeval to/from jiffies) and profiling code.

NSEC_PER_JIFFY is used by the jiffy clocksource code, which only matters
if you don't have your own clocksource.

So, I feel very uneasy about saying that CLOCK_TICK_RATE doesn't matter
anymore given all the places which reference something that's derived
from it.

Here's the result of grepping for the above symbols:

drivers/oprofile/timer_int.c: hrtimer_forward_now(hrtimer, ns_to_ktime(TICK_NSEC));
drivers/oprofile/timer_int.c: hrtimer_start(hrtimer, ns_to_ktime(TICK_NSEC),
include/linux/acct.h:#if (TICK_NSEC % (NSEC_PER_SEC / AHZ)) == 0
include/linux/acct.h: u64 tmp = (u64)x * TICK_NSEC;
include/linux/hrtimer.h:# define MONOTONIC_RES_NSEC HIGH_RES_NSEC
include/linux/hrtimer.h:# define MONOTONIC_RES_NSEC LOW_RES_NSEC
include/linux/hrtimer.h:# define KTIME_MONOTONIC_RES KTIME_LOW_RES
include/linux/jiffies.h:/* TICK_NSEC is the time between ticks in nsec assuming
real ACTHZ */
include/linux/jiffies.h:#define TICK_NSEC (SH_DIV (1000000UL * 1000, ACTHZ, 8))
include/linux/jiffies.h:/* TICK_USEC_TO_NSEC is the time between ticks in nsec assuming real ACTHZ and */
include/linux/jiffies.h:#define TICK_USEC_TO_NSEC(TUSEC) (SH_DIV (TUSEC * USER_HZ * 1000, ACTHZ, 8))
include/linux/jiffies.h: * is: TICK_NSEC (which is defined in timex.h). This
include/linux/jiffies.h:#if !((((NSEC_PER_SEC << 2) / TICK_NSEC) << (SEC_JIFFIE_SC - 2)) & 0x80000000)
include/linux/jiffies.h: TICK_NSEC -1) / (u64)TICK_NSEC))
include/linux/jiffies.h: TICK_NSEC -1) /
(u64)TICK_NSEC))
include/linux/jiffies.h: TICK_NSEC -1) /
(u64)TICK_NSEC))
include/linux/jiffies.h: (long)((u64)((u64)MAX_JIFFY_OFFSET * TICK_NSEC)
/ NSEC_PER_SEC)
include/linux/jiffies.h: (SH_DIV((MAX_JIFFY_OFFSET >> SEC_JIFFIE_SC) * TICK_NSEC, NSEC_PER_SEC, 1) - 1)
include/linux/ktime.h:#define LOW_RES_NSEC TICK_NSEC
include/linux/ktime.h:#define KTIME_LOW_RES (ktime_t){ .tv64 = LOW_RES_NSEC }
kernel/hrtimer.c: .resolution = KTIME_LOW_RES,
kernel/hrtimer.c: .resolution = KTIME_LOW_RES,
kernel/perf_event.c: u64 interval = (u64)cpuctx->jiffies_interval * TICK_NSEC;
kernel/perf_event.c: if (delta > 0 && delta < 2*TICK_NSEC)
kernel/posix-timers.c: *tp = ktime_to_timespec(KTIME_LOW_RES);
kernel/sched.c: * scheduler tick (TICK_NSEC). With tickless idle this will not be called
kernel/sched_clock.c: * scd->tick_gtod + TICK_NSEC);
kernel/sched_clock.c: max_clock = wrap_max(old_clock, scd->tick_gtod + TICK_NSEC);
kernel/time.c: * The TICK_NSEC - 1 rounds up the value to the next resolution.
Note
kernel/time.c: * nsec -= nsec % TICK_NSEC; is NOT a correct resolution rounding.kernel/time.c: long nsec = value->tv_nsec + TICK_NSEC - 1;
kernel/time.c: value->tv_sec = div_u64_rem((u64)jiffies * TICK_NSEC,
kernel/time.c: value->tv_sec = div_u64_rem((u64)jiffies * TICK_NSEC,
kernel/time.c:#if (TICK_NSEC % (NSEC_PER_SEC / USER_HZ)) == 0
kernel/time.c: return div_u64((u64)x * TICK_NSEC, NSEC_PER_SEC / USER_HZ);
kernel/time.c:#if (TICK_NSEC % (NSEC_PER_SEC / USER_HZ)) == 0
kernel/time.c: x = div_u64(x * TICK_NSEC, (NSEC_PER_SEC / USER_HZ));
kernel/time/jiffies.c:#define NSEC_PER_JIFFY ((u32)((((u64)NSEC_PER_SEC)<<8)/ACTHZ))
kernel/time/jiffies.c:/* Since jiffies uses a simple NSEC_PER_JIFFY multiplier
kernel/time/jiffies.c: * larger can result in overflows. NSEC_PER_JIFFY grows askernel/time/jiffies.c: .mult = NSEC_PER_JIFFY << JIFFIES_SHIFT, /* details above */
kernel/time/ntp.c: next.tv_nsec = (NSEC_PER_SEC / 2) - now.tv_nsec - (TICK_NSEC / 2);

2011-02-21 21:08:26

by Arnd Bergmann

Subject: CLOCK_TICK_RATE, was: Re: [PATCH V4 3/4] ARM: Xilinx: base header files and assembly macros

On Monday 21 February 2011, Russell King - ARM Linux wrote:
> Eg, LOW_RES_NSEC is exported to userspace via the posix clocks interface.
>
> NSEC_PER_SEC and TICK_NSEC are used for cmos clock updates, so probably
> don't matter too much there. TICK_NSEC is also used by the scheduler,
> time conversions (timespec/timeval to/from jiffies) and profiling code.
>
> NSEC_PER_JIFFY is used by the jiffy clocksource code, which only matters
> if you don't have your own clocksource.
>
> So, I feel very uneasy about saying that CLOCK_TICK_RATE doesn't matter
> anymore given all the places which reference something that's derived
> from it.

All the calculations based off of CLOCK_TICK_RATE are derived from ACTHZ,
which is either the correct value based on the underlying HW timer tick,
or slightly off, when either the HW tick or the value of CLOCK_TICK_RATE
is not a true multiple of HZ.

In fact, I'm pretty sure that it's off on a lot of machines:

arch/frv/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
arch/m68k/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
arch/mips/include/asm/timex.h:#define CLOCK_TICK_RATE 1193182
arch/parisc/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
arch/s390/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
arch/sh/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180
arch/x86/include/asm/timex.h:#define CLOCK_TICK_RATE PIT_TICK_RATE
arch/xtensa/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180 /* (everyone is using this value) */

None of these is actually using a PC-style PIT these days, the just copied the
definition blindly from old i386. I think a simple

#define ACTHZ (HZ << 8)

would fix more than it can break, and most likely nobody would ever notice
the difference. If we do that, CLOCK_TICK_RATE becomes unused.

Arnd

2011-02-21 21:51:29

by Thomas Gleixner

Subject: Re: CLOCK_TICK_RATE, was: Re: [PATCH V4 3/4] ARM: Xilinx: base header files and assembly macros

On Mon, 21 Feb 2011, Arnd Bergmann wrote:

> On Monday 21 February 2011, Russell King - ARM Linux wrote:
> > Eg, LOW_RES_NSEC is exported to userspace via the posix clocks interface.
> >
> > NSEC_PER_SEC and TICK_NSEC are used for cmos clock updates, so probably
> > don't matter too much there. TICK_NSEC is also used by the scheduler,
> > time conversions (timespec/timeval to/from jiffies) and profiling code.
> >
> > NSEC_PER_JIFFY is used by the jiffy clocksource code, which only matters
> > if you don't have your own clocksource.
> >
> > So, I feel very uneasy about saying that CLOCK_TICK_RATE doesn't matter
> > anymore given all the places which reference something that's derived
> > from it.
>
> All the calculations based off of CLOCK_TICK_RATE are derived from ACTHZ,
> which is either the correct value based on the underlying HW timer tick,
> or slightly off, when either the HW tick or the value of CLOCK_TICK_RATE
> is not a true multiple of HZ.
>
> In fact, I'm pretty sure that it's off on a lot of machines:
>
> arch/frv/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
> arch/m68k/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
> arch/mips/include/asm/timex.h:#define CLOCK_TICK_RATE 1193182
> arch/parisc/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
> arch/s390/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
> arch/sh/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180
> arch/x86/include/asm/timex.h:#define CLOCK_TICK_RATE PIT_TICK_RATE
> arch/xtensa/include/asm/timex.h:#define CLOCK_TICK_RATE 1193180 /* (everyone is using this value) */
>
> None of these is actually using a PC-style PIT these days, the just copied the
> definition blindly from old i386. I think a simple
>
> #define ACTHZ (HZ << 8)
>
> would fix more than it can break, and most likely nobody would ever notice
> the difference. If we do that, CLOCK_TICK_RATE becomes unused.

Indeed.

tglx

2011-02-28 11:18:44

by Jamie Iles

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

Hi John,

One more pedantic nitpick, but nothing major!

Jamie

On Fri, Feb 18, 2011 at 11:14:40AM -0700, John Linn wrote:
> These are the minimum needed to build the kernel for the new platform.
>
> Signed-off-by: John Linn <[email protected]>
> ---
>
> V4 Changes
>
> Cleanup based on input from Jamie Iles.
> Moved to PLAT_PHYS_OFFSET.
> Remove headers file inclusions that weren't needed.
> Added use of raw_readl/raw_writel in uncompressor uart code.
> Added cpu_relax() in the uncompressor uart code.
>
> V3 Changes
>
> Updates based on Russell Kings' comments
> Changed headers to update the license info and remove
> the address
>
> Updated the constant to use UL in vmalloc.h
>
> V2 Changes
>
> Updates based on Russell King's comments
> moved to using entry-macro-gic.S
> moved stuff from hardware.h to xilinx_soc.h
> added IOMEM() in io.h to help with typing
> Minor updates to IO_SPACE_LIMIT and __io()
> Updated addresses to be clear with seperate virtual
> and physical addresses
>
> 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 | 30 +++++++++++
> arch/arm/mach-xilinx/include/mach/hardware.h | 18 +++++++
> arch/arm/mach-xilinx/include/mach/io.h | 33 ++++++++++++
> arch/arm/mach-xilinx/include/mach/irqs.h | 29 +++++++++++
> 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 | 25 +++++++++
> arch/arm/mach-xilinx/include/mach/uncompress.h | 61 +++++++++++++++++++++++
> arch/arm/mach-xilinx/include/mach/vmalloc.h | 20 +++++++
> arch/arm/mach-xilinx/include/mach/xilinx_soc.h | 46 +++++++++++++++++
> 13 files changed, 404 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
> create mode 100644 arch/arm/mach-xilinx/include/mach/xilinx_soc.h
>
> diff --git a/arch/arm/mach-xilinx/include/mach/clkdev.h b/arch/arm/mach-xilinx/include/mach/clkdev.h
> new file mode 100644
> index 0000000..5dd1453
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/clkdev.h
> @@ -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
> diff --git a/arch/arm/mach-xilinx/include/mach/debug-macro.S b/arch/arm/mach-xilinx/include/mach/debug-macro.S
> new file mode 100644
> index 0000000..30d3d3d
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/debug-macro.S
> @@ -0,0 +1,36 @@
> +/* arch/arm/mach-xilinx/include/mach/debug-macro.S
> + *
> + * Debugging macro include header
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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.
> + */
> +
> +#include <mach/xilinx_soc.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
> diff --git a/arch/arm/mach-xilinx/include/mach/entry-macro.S b/arch/arm/mach-xilinx/include/mach/entry-macro.S
> new file mode 100644
> index 0000000..11a2866
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/entry-macro.S
> @@ -0,0 +1,30 @@
> +/*
> + * 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 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.
> + */
> +
> +#include <mach/hardware.h>
> +#include <asm/hardware/entry-macro-gic.S>
> +
> + .macro disable_fiq
> + .endm
> +
> + .macro arch_ret_to_user, tmp1, tmp2
> + .endm
> diff --git a/arch/arm/mach-xilinx/include/mach/hardware.h b/arch/arm/mach-xilinx/include/mach/hardware.h
> new file mode 100644
> index 0000000..f9685c4
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/hardware.h
> @@ -0,0 +1,18 @@
> +/* arch/arm/mach-xilinx/include/mach/hardware.h
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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_HARDWARE_H__
> +#define __MACH_HARDWARE_H__
> +
> +#endif
> diff --git a/arch/arm/mach-xilinx/include/mach/io.h b/arch/arm/mach-xilinx/include/mach/io.h
> new file mode 100644
> index 0000000..ef69e65
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/io.h
> @@ -0,0 +1,33 @@
> +/* arch/arm/mach-xilinx/include/mach/io.h
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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_IO_H__
> +#define __MACH_IO_H__
> +
> +/* Allow IO space to be anywhere in the memory */
> +
> +#define IO_SPACE_LIMIT 0xffff
> +
> +/* IO address mapping macros, nothing special at this time but required */
> +
> +#ifdef __ASSEMBLER__
> +#define IOMEM(x) (x)
> +#else
> +#define IOMEM(x) ((void __force __iomem *)(x))
> +#endif
> +
> +#define __io(a) __typesafe_io(a)
> +#define __mem_pci(a) (a)
> +
> +#endif
> diff --git a/arch/arm/mach-xilinx/include/mach/irqs.h b/arch/arm/mach-xilinx/include/mach/irqs.h
> new file mode 100644
> index 0000000..47a8162
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/irqs.h
> @@ -0,0 +1,29 @@
> +/* arch/arm/mach-xilinx/include/mach/irqs.h
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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_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
> diff --git a/arch/arm/mach-xilinx/include/mach/memory.h b/arch/arm/mach-xilinx/include/mach/memory.h
> new file mode 100644
> index 0000000..3efde15
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/memory.h
> @@ -0,0 +1,23 @@
> +/* arch/arm/mach-xilinx/include/mach/memory.h
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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_MEMORY_H__
> +#define __MACH_MEMORY_H__
> +
> +#include <asm/sizes.h>
> +
> +#define PHYS_OFFSET UL(0x0)
> +#define MEM_SIZE SZ_256M
> +
> +#endif
> diff --git a/arch/arm/mach-xilinx/include/mach/system.h b/arch/arm/mach-xilinx/include/mach/system.h
> new file mode 100644
> index 0000000..e8514a0
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/system.h
> @@ -0,0 +1,28 @@
> +/* arch/arm/mach-xilinx/include/mach/system.h
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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_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
> diff --git a/arch/arm/mach-xilinx/include/mach/timex.h b/arch/arm/mach-xilinx/include/mach/timex.h
> new file mode 100644
> index 0000000..4ebc0a6
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/timex.h
> @@ -0,0 +1,22 @@
> +/* arch/arm/mach-xilinx/include/mach/timex.h
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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_TIMEX_H__
> +#define __MACH_TIMEX_H__
> +
> +#define PERIPHERAL_CLOCK_RATE 2500000
> +
> +#define CLOCK_TICK_RATE (PERIPHERAL_CLOCK_RATE / 32)
> +
> +#endif
> diff --git a/arch/arm/mach-xilinx/include/mach/uart.h b/arch/arm/mach-xilinx/include/mach/uart.h
> new file mode 100644
> index 0000000..7fca361
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/uart.h
> @@ -0,0 +1,25 @@
> +/* arch/arm/mach-xilinx/include/mach/uart.h
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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_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 */
> +
> +#endif
> diff --git a/arch/arm/mach-xilinx/include/mach/uncompress.h b/arch/arm/mach-xilinx/include/mach/uncompress.h
> new file mode 100644
> index 0000000..ff3754c
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/uncompress.h
> @@ -0,0 +1,61 @@
> +/* arch/arm/mach-xilinx/include/mach/uncompress.h
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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_UNCOMPRESS_H__
> +#define __MACH_UNCOMPRESS_H__
> +
> +#include <mach/xilinx_soc.h>
> +#include <mach/uart.h>
> +#include <asm/processor.h>
> +#include <asm/io.h>
> +#include <mach/io.h>
> +
> +void arch_decomp_setup(void)
> +{
> +}
> +
> +static inline void flush(void)
> +{
> + u32 status;
> + /*
> + * Wait while the FIFO is not empty
> + */
> + while (1) {
> + status = __raw_readl(IOMEM(LL_UART_PADDR + UART_SR_OFFSET));
> + if (status & UART_SR_TXEMPTY)
> + break;
> + cpu_relax();
> + }

This could be written as:

static inline void flush(void)
{
while (!(__raw_readl(IOMEM(LL_UART_PADDR + UART_SR_OFFSET)) &
UART_SR_TXEMPTY))
cpu_relax();
}

> +}
> +
> +#define arch_decomp_wdog()
> +
> +static void putc(char ch)
> +{
> + u32 status;
> +
> + /*
> + * Wait for room in the FIFO, then write the char into the FIFO
> + */
> + while (1) {
> + status = __raw_readl(IOMEM(LL_UART_PADDR + UART_SR_OFFSET));
> + if (!(status & UART_SR_TXFULL))
> + break;
> + cpu_relax();
> + }
> +
> + __raw_writel(ch, IOMEM(LL_UART_PADDR + UART_FIFO_OFFSET));
> +}

static void putch(char ch)
{
while (__raw_readl(IOMEM(LL_UART_PADDR + UART_SR_OFFSET)) &
UART_SR_TXFULL)
cpu_relax();
__raw_writel(ch, IOMEM(LL_UART_PADDR + UART_FIFO_OFFSET));
}

> +
> +#endif
> diff --git a/arch/arm/mach-xilinx/include/mach/vmalloc.h b/arch/arm/mach-xilinx/include/mach/vmalloc.h
> new file mode 100644
> index 0000000..aba20a3
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/vmalloc.h
> @@ -0,0 +1,20 @@
> +/* arch/arm/mach-xilinx/include/mach/vmalloc.h
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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_VMALLOC_H__
> +#define __MACH_VMALLOC_H__
> +
> +#define VMALLOC_END 0xE0000000UL
> +
> +#endif
> diff --git a/arch/arm/mach-xilinx/include/mach/xilinx_soc.h b/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> new file mode 100644
> index 0000000..d01cde1
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> @@ -0,0 +1,46 @@
> +/* arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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_XILINX_SOC_H__
> +#define __MACH_XILINX_SOC_H__
> +
> +/* For now, all mappings are flat (physical = virtual)
> + */
> +#define UART0_PHYS 0xE0000000
> +#define UART0_VIRT UART0_PHYS
> +
> +#define TTC0_PHYS 0xF8001000
> +#define TTC0_VIRT TTC0_PHYS
> +
> +#define PL310_L2CC_PHYS 0xF8F02000
> +#define PL310_L2CC_VIRT PL310_L2CC_PHYS
> +
> +#define SCU_PERIPH_PHYS 0xF8F00000
> +#define SCU_PERIPH_VIRT SCU_PERIPH_PHYS
> +
> +/* The following are intended for the devices that are mapped early */
> +
> +#define TTC0_BASE IOMEM(TTC0_VIRT)
> +#define SCU_PERIPH_BASE IOMEM(SCU_PERIPH_VIRT)
> +#define SCU_GIC_CPU_BASE (SCU_PERIPH_BASE + 0x100)
> +#define SCU_GIC_DIST_BASE (SCU_PERIPH_BASE + 0x1000)
> +#define PL310_L2CC_BASE IOMEM(PL310_L2CC_VIRT)
> +
> +/*
> + * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
> + */
> +#define LL_UART_PADDR UART0_PHYS
> +#define LL_UART_VADDR UART0_VIRT
> +
> +#endif
> --
> 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.
>
>

2011-02-28 11:21:10

by Russell King - ARM Linux

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

On Fri, Feb 18, 2011 at 11:14:40AM -0700, John Linn wrote:
> diff --git a/arch/arm/mach-xilinx/include/mach/uncompress.h b/arch/arm/mach-xilinx/include/mach/uncompress.h
> new file mode 100644
> index 0000000..ff3754c
> --- /dev/null
> +++ b/arch/arm/mach-xilinx/include/mach/uncompress.h
> @@ -0,0 +1,61 @@
> +/* arch/arm/mach-xilinx/include/mach/uncompress.h
> + *
> + * Copyright (C) 2011 Xilinx
> + *
> + * 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_UNCOMPRESS_H__
> +#define __MACH_UNCOMPRESS_H__
> +
> +#include <mach/xilinx_soc.h>
> +#include <mach/uart.h>
> +#include <asm/processor.h>
> +#include <asm/io.h>
> +#include <mach/io.h>

You don't need mach/io.h here - it's already included by asm/io.h.
It's probably also worth trying linux/io.h instead of asm/io.h too.

2011-02-28 14:20:46

by John Linn

Subject: RE: [PATCH V4 3/4] ARM: Xilinx: base header files and assembly macros

> -----Original Message-----
> From: Jamie Iles [mailto:[email protected]]
> Sent: Monday, February 28, 2011 4:19 AM
> To: John Linn
> Cc: [email protected];
[email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
[email protected]
> Subject: Re: [PATCH V4 3/4] ARM: Xilinx: base header files and
assembly macros
>
> Hi John,
>
> One more pedantic nitpick, but nothing major!
>
> Jamie
>
> On Fri, Feb 18, 2011 at 11:14:40AM -0700, John Linn wrote:
> > These are the minimum needed to build the kernel for the new
platform.
> >
> > Signed-off-by: John Linn <[email protected]>
> > ---
> >
> > V4 Changes
> >
> > Cleanup based on input from Jamie Iles.
> > Moved to PLAT_PHYS_OFFSET.
> > Remove headers file inclusions that weren't needed.
> > Added use of raw_readl/raw_writel in uncompressor uart code.
> > Added cpu_relax() in the uncompressor uart code.
> >
> > V3 Changes
> >
> > Updates based on Russell Kings' comments
> > Changed headers to update the license info and remove
> > the address
> >
> > Updated the constant to use UL in vmalloc.h
> >
> > V2 Changes
> >
> > Updates based on Russell King's comments
> > moved to using entry-macro-gic.S
> > moved stuff from hardware.h to xilinx_soc.h
> > added IOMEM() in io.h to help with typing
> > Minor updates to IO_SPACE_LIMIT and __io()
> > Updated addresses to be clear with seperate virtual
> > and physical addresses
> >
> > 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 | 30 +++++++++++
> > arch/arm/mach-xilinx/include/mach/hardware.h | 18 +++++++
> > arch/arm/mach-xilinx/include/mach/io.h | 33 ++++++++++++
> > arch/arm/mach-xilinx/include/mach/irqs.h | 29 +++++++++++
> > 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 | 25 +++++++++
> > arch/arm/mach-xilinx/include/mach/uncompress.h | 61
+++++++++++++++++++++++
> > arch/arm/mach-xilinx/include/mach/vmalloc.h | 20 +++++++
> > arch/arm/mach-xilinx/include/mach/xilinx_soc.h | 46
+++++++++++++++++
> > 13 files changed, 404 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
> > create mode 100644 arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> >
> > diff --git a/arch/arm/mach-xilinx/include/mach/clkdev.h
b/arch/arm/mach-xilinx/include/mach/clkdev.h
> > new file mode 100644
> > index 0000000..5dd1453
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/clkdev.h
> > @@ -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
> > diff --git a/arch/arm/mach-xilinx/include/mach/debug-macro.S
b/arch/arm/mach-
> xilinx/include/mach/debug-macro.S
> > new file mode 100644
> > index 0000000..30d3d3d
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/debug-macro.S
> > @@ -0,0 +1,36 @@
> > +/* arch/arm/mach-xilinx/include/mach/debug-macro.S
> > + *
> > + * Debugging macro include header
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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.
> > + */
> > +
> > +#include <mach/xilinx_soc.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
> > diff --git a/arch/arm/mach-xilinx/include/mach/entry-macro.S
b/arch/arm/mach-
> xilinx/include/mach/entry-macro.S
> > new file mode 100644
> > index 0000000..11a2866
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/entry-macro.S
> > @@ -0,0 +1,30 @@
> > +/*
> > + * 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 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.
> > + */
> > +
> > +#include <mach/hardware.h>
> > +#include <asm/hardware/entry-macro-gic.S>
> > +
> > + .macro disable_fiq
> > + .endm
> > +
> > + .macro arch_ret_to_user, tmp1, tmp2
> > + .endm
> > diff --git a/arch/arm/mach-xilinx/include/mach/hardware.h
b/arch/arm/mach-
> xilinx/include/mach/hardware.h
> > new file mode 100644
> > index 0000000..f9685c4
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/hardware.h
> > @@ -0,0 +1,18 @@
> > +/* arch/arm/mach-xilinx/include/mach/hardware.h
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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_HARDWARE_H__
> > +#define __MACH_HARDWARE_H__
> > +
> > +#endif
> > diff --git a/arch/arm/mach-xilinx/include/mach/io.h
b/arch/arm/mach-xilinx/include/mach/io.h
> > new file mode 100644
> > index 0000000..ef69e65
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/io.h
> > @@ -0,0 +1,33 @@
> > +/* arch/arm/mach-xilinx/include/mach/io.h
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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_IO_H__
> > +#define __MACH_IO_H__
> > +
> > +/* Allow IO space to be anywhere in the memory */
> > +
> > +#define IO_SPACE_LIMIT 0xffff
> > +
> > +/* IO address mapping macros, nothing special at this time but
required */
> > +
> > +#ifdef __ASSEMBLER__
> > +#define IOMEM(x) (x)
> > +#else
> > +#define IOMEM(x) ((void __force __iomem *)(x))
> > +#endif
> > +
> > +#define __io(a) __typesafe_io(a)
> > +#define __mem_pci(a) (a)
> > +
> > +#endif
> > diff --git a/arch/arm/mach-xilinx/include/mach/irqs.h
b/arch/arm/mach-xilinx/include/mach/irqs.h
> > new file mode 100644
> > index 0000000..47a8162
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/irqs.h
> > @@ -0,0 +1,29 @@
> > +/* arch/arm/mach-xilinx/include/mach/irqs.h
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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_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
> > diff --git a/arch/arm/mach-xilinx/include/mach/memory.h
b/arch/arm/mach-xilinx/include/mach/memory.h
> > new file mode 100644
> > index 0000000..3efde15
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/memory.h
> > @@ -0,0 +1,23 @@
> > +/* arch/arm/mach-xilinx/include/mach/memory.h
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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_MEMORY_H__
> > +#define __MACH_MEMORY_H__
> > +
> > +#include <asm/sizes.h>
> > +
> > +#define PHYS_OFFSET UL(0x0)
> > +#define MEM_SIZE SZ_256M
> > +
> > +#endif
> > diff --git a/arch/arm/mach-xilinx/include/mach/system.h
b/arch/arm/mach-xilinx/include/mach/system.h
> > new file mode 100644
> > index 0000000..e8514a0
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/system.h
> > @@ -0,0 +1,28 @@
> > +/* arch/arm/mach-xilinx/include/mach/system.h
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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_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
> > diff --git a/arch/arm/mach-xilinx/include/mach/timex.h
b/arch/arm/mach-xilinx/include/mach/timex.h
> > new file mode 100644
> > index 0000000..4ebc0a6
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/timex.h
> > @@ -0,0 +1,22 @@
> > +/* arch/arm/mach-xilinx/include/mach/timex.h
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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_TIMEX_H__
> > +#define __MACH_TIMEX_H__
> > +
> > +#define PERIPHERAL_CLOCK_RATE 2500000
> > +
> > +#define CLOCK_TICK_RATE (PERIPHERAL_CLOCK_RATE / 32)
> > +
> > +#endif
> > diff --git a/arch/arm/mach-xilinx/include/mach/uart.h
b/arch/arm/mach-xilinx/include/mach/uart.h
> > new file mode 100644
> > index 0000000..7fca361
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/uart.h
> > @@ -0,0 +1,25 @@
> > +/* arch/arm/mach-xilinx/include/mach/uart.h
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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_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
*/
> > +
> > +#endif
> > diff --git a/arch/arm/mach-xilinx/include/mach/uncompress.h
b/arch/arm/mach-
> xilinx/include/mach/uncompress.h
> > new file mode 100644
> > index 0000000..ff3754c
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/uncompress.h
> > @@ -0,0 +1,61 @@
> > +/* arch/arm/mach-xilinx/include/mach/uncompress.h
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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_UNCOMPRESS_H__
> > +#define __MACH_UNCOMPRESS_H__
> > +
> > +#include <mach/xilinx_soc.h>
> > +#include <mach/uart.h>
> > +#include <asm/processor.h>
> > +#include <asm/io.h>
> > +#include <mach/io.h>
> > +
> > +void arch_decomp_setup(void)
> > +{
> > +}
> > +
> > +static inline void flush(void)
> > +{
> > + u32 status;
> > + /*
> > + * Wait while the FIFO is not empty
> > + */
> > + while (1) {
> > + status = __raw_readl(IOMEM(LL_UART_PADDR +
UART_SR_OFFSET));
> > + if (status & UART_SR_TXEMPTY)
> > + break;
> > + cpu_relax();
> > + }
>
> This could be written as:
>
> static inline void flush(void)
> {
> while (!(__raw_readl(IOMEM(LL_UART_PADDR + UART_SR_OFFSET)) &
> UART_SR_TXEMPTY))
> cpu_relax();
> }

Honestly I had it that way as I know most in the community do that and
terse-ness
is somewhat preferred.

I thought my way is a bit clearer as it was a bit harder to read the
other way.

Do you think everyone else would say to change it? No big deal to
change it.

Thanks,
John

>
> > +}
> > +
> > +#define arch_decomp_wdog()
> > +
> > +static void putc(char ch)
> > +{
> > + u32 status;
> > +
> > + /*
> > + * Wait for room in the FIFO, then write the char into the FIFO
> > + */
> > + while (1) {
> > + status = __raw_readl(IOMEM(LL_UART_PADDR +
UART_SR_OFFSET));
> > + if (!(status & UART_SR_TXFULL))
> > + break;
> > + cpu_relax();
> > + }
> > +
> > + __raw_writel(ch, IOMEM(LL_UART_PADDR + UART_FIFO_OFFSET));
> > +}
>
> static void putch(char ch)
> {
> while (__raw_readl(IOMEM(LL_UART_PADDR + UART_SR_OFFSET)) &
> UART_SR_TXFULL)
> cpu_relax();
> __raw_writel(ch, IOMEM(LL_UART_PADDR + UART_FIFO_OFFSET));
> }
>

Same thing.

> > +
> > +#endif
> > diff --git a/arch/arm/mach-xilinx/include/mach/vmalloc.h
b/arch/arm/mach-
> xilinx/include/mach/vmalloc.h
> > new file mode 100644
> > index 0000000..aba20a3
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/vmalloc.h
> > @@ -0,0 +1,20 @@
> > +/* arch/arm/mach-xilinx/include/mach/vmalloc.h
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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_VMALLOC_H__
> > +#define __MACH_VMALLOC_H__
> > +
> > +#define VMALLOC_END 0xE0000000UL
> > +
> > +#endif
> > diff --git a/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
b/arch/arm/mach-
> xilinx/include/mach/xilinx_soc.h
> > new file mode 100644
> > index 0000000..d01cde1
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> > @@ -0,0 +1,46 @@
> > +/* arch/arm/mach-xilinx/include/mach/xilinx_soc.h
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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_XILINX_SOC_H__
> > +#define __MACH_XILINX_SOC_H__
> > +
> > +/* For now, all mappings are flat (physical = virtual)
> > + */
> > +#define UART0_PHYS 0xE0000000
> > +#define UART0_VIRT UART0_PHYS
> > +
> > +#define TTC0_PHYS 0xF8001000
> > +#define TTC0_VIRT TTC0_PHYS
> > +
> > +#define PL310_L2CC_PHYS 0xF8F02000
> > +#define PL310_L2CC_VIRT PL310_L2CC_PHYS
> > +
> > +#define SCU_PERIPH_PHYS 0xF8F00000
> > +#define SCU_PERIPH_VIRT SCU_PERIPH_PHYS
> > +
> > +/* The following are intended for the devices that are mapped early
*/
> > +
> > +#define TTC0_BASE IOMEM(TTC0_VIRT)
> > +#define SCU_PERIPH_BASE IOMEM(SCU_PERIPH_VIRT)
> > +#define SCU_GIC_CPU_BASE (SCU_PERIPH_BASE + 0x100)
> > +#define SCU_GIC_DIST_BASE (SCU_PERIPH_BASE + 0x1000)
> > +#define PL310_L2CC_BASE IOMEM(PL310_L2CC_VIRT)
> > +
> > +/*
> > + * Mandatory for CONFIG_LL_DEBUG, UART is mapped virtual = physical
> > + */
> > +#define LL_UART_PADDR UART0_PHYS
> > +#define LL_UART_VADDR UART0_VIRT
> > +
> > +#endif
> > --
> > 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.
> >
> >


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.

2011-02-28 14:23:00

by John Linn

Subject: RE: [PATCH V4 3/4] ARM: Xilinx: base header files and assemblymacros

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:[email protected]]
> Sent: Monday, February 28, 2011 4:21 AM
> To: John Linn
> Cc: [email protected];
[email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH V4 3/4] ARM: Xilinx: base header files and
assemblymacros
>
> On Fri, Feb 18, 2011 at 11:14:40AM -0700, John Linn wrote:
> > diff --git a/arch/arm/mach-xilinx/include/mach/uncompress.h
b/arch/arm/mach-
> xilinx/include/mach/uncompress.h
> > new file mode 100644
> > index 0000000..ff3754c
> > --- /dev/null
> > +++ b/arch/arm/mach-xilinx/include/mach/uncompress.h
> > @@ -0,0 +1,61 @@
> > +/* arch/arm/mach-xilinx/include/mach/uncompress.h
> > + *
> > + * Copyright (C) 2011 Xilinx
> > + *
> > + * 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_UNCOMPRESS_H__
> > +#define __MACH_UNCOMPRESS_H__
> > +
> > +#include <mach/xilinx_soc.h>
> > +#include <mach/uart.h>
> > +#include <asm/processor.h>
> > +#include <asm/io.h>
> > +#include <mach/io.h>
>
> You don't need mach/io.h here - it's already included by asm/io.h.
> It's probably also worth trying linux/io.h instead of asm/io.h too.

Makes sense, will incorporate that into next patch version.

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.

2011-02-28 15:02:40

by Jamie Iles

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

On Mon, Feb 28, 2011 at 07:20:32AM -0700, John Linn wrote:
[...]
> > > +
> > > +static inline void flush(void)
> > > +{
> > > + u32 status;
> > > + /*
> > > + * Wait while the FIFO is not empty
> > > + */
> > > + while (1) {
> > > + status = __raw_readl(IOMEM(LL_UART_PADDR +
> UART_SR_OFFSET));
> > > + if (status & UART_SR_TXEMPTY)
> > > + break;
> > > + cpu_relax();
> > > + }
> >
> > This could be written as:
> >
> > static inline void flush(void)
> > {
> > while (!(__raw_readl(IOMEM(LL_UART_PADDR + UART_SR_OFFSET)) &
> > UART_SR_TXEMPTY))
> > cpu_relax();
> > }
>
> Honestly I had it that way as I know most in the community do that and
> terse-ness is somewhat preferred.
>
> I thought my way is a bit clearer as it was a bit harder to read the
> other way.
>
> Do you think everyone else would say to change it? No big deal to
> change it.

I guess it's largely down to personal preference. Personally I find the
infinite loop with conditional break harder to read but then I'm not
going to the the primary maintainer ;-)

Jamie