[PATCH V2 1/4] ARM: Xilinx: Adding Xilinx board support

[John Linn]

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Sent: Saturday, February 12, 2011 5:13 AM
> To: John Linn
> Cc: linux-arm-kernel at lists.infradead.org;
linux-kernel at vger.kernel.org; catalin.marinas at arm.com;
> glikely at secretlab.ca
> Subject: Re: [PATCH V2 1/4] ARM: Xilinx: Adding Xilinx board support
> 
> On Wed, Feb 09, 2011 at 10:00:42AM -0700, John Linn wrote:
> > +/* arch/arm/mach-xilinx/board_ep107.c
> > + *
> > + * This file contains code specific to the Xilinx EP107 board.
> > + *
> > + *  Copyright (C) 2011 Xilinx
> > + *
> > + * based on /arch/arm/mach-realview/core.c
> > + *
> > + *  Copyright (C) 1999 - 2003 ARM Limited
> > + *  Copyright (C) 2000 Deep Blue Solutions Ltd
> > + *
> > + * 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
> 
> You probably don't want to include this paragraph - their address
keeps
> changing, so will require files to be constantly updated.

Makes sense. Easy to fix.

> 
> > + */
> > +
> > +#include <linux/platform_device.h>
> > +#include <asm/mach-types.h>
> > +#include <asm/mach/arch.h>
> > +#include <mach/xilinx_soc.h>
> > +#include <mach/irqs.h>
> > +#include <linux/clkdev.h>
> 
> Please group linux/ includes together, then asm/ includes, lastly
mach/
> includes.
> 
> > @@ -0,0 +1,113 @@
> > +/* arch/arm/mach-xilinx/common.c
> ...
> > +/**
> > + * system_init - System specific initialization, intended to be
called from
> > + *			board specific initialization.
> > + *
> > + **/
> > +void __init system_init(void)
> 
> Is there a better name for this - maybe prefixing it with xilinx_ ?

Sure, that's easy.

> 
> > +{
> > +#ifdef CONFIG_CACHE_L2X0
> > +	/*
> > +	 * 64KB way size, 8-way associativity, parity disabled
> > +	 */
> > +	l2x0_init(PL310_L2CC_BASE, 0x02060000, 0xF0F0FFFF);
> > +#endif
> > +}
> > +
> > +/**
> > + * irq_init - Interrupt controller initialization for the GIC.
> > + *
> > + **/
> > +void __init irq_init(void)
> 
> Ditto.

Same thing, easy.

> 
> > +/**
> > + * map_io - Create memory mappings needed for early I/O.
> > + *
> > + **/
> > +void __init map_io(void)
> 
> Ditto.

Yep.


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.