Re: [PATCH 2/9] ARM: Zynq: Add new architecture zynq

From: Josh Cartwright <joshc@eso.teric.us>
To: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH 2/9] ARM: Zynq: Add new architecture zynq
Date: Tue, 12 Mar 2013 08:42:34 -0500	[thread overview]
Message-ID: <20130312134234.GQ16050@kryptos> (raw)
In-Reply-To: <1362993306-19262-3-git-send-email-s.trumtrar@pengutronix.de>

On Mon, Mar 11, 2013 at 10:14:59AM +0100, Steffen Trumtrar wrote:
> Add basic support for the Xilinx Zynq-7000 EPP architecture.
> The Zynq-7000 is an embedded processing platform that combines a Cortex A9
> dualcore MPSoC with an Artix-7 FPGA.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
[..]
> diff --git a/arch/arm/mach-zynq/include/mach/barebox.lds.h b/arch/arm/mach-zynq/include/mach/barebox.lds.h
> new file mode 100644
> index 0000000..674a4ac
> --- /dev/null
> +++ b/arch/arm/mach-zynq/include/mach/barebox.lds.h
> @@ -0,0 +1,9 @@
> +#define PRE_IMAGE \
> +	.pre_image : {					\
> +		KEEP(*(.flash_header_start*))		\
> +		. = 0x20;				\
> +		KEEP(*(.flash_header_0x0*))		\
> +		. = 0xa0;				\
> +		KEEP(*(.ps7reg_entry_0x0A0))		\
> +		. = 0x8c0;				\

> +	}
[..]
> diff --git a/arch/arm/mach-zynq/include/mach/zynq-flash-header.h b/arch/arm/mach-zynq/include/mach/zynq-flash-header.h
> new file mode 100644
> index 0000000..a0251cb
> --- /dev/null
> +++ b/arch/arm/mach-zynq/include/mach/zynq-flash-header.h
> @@ -0,0 +1,40 @@
> +#ifndef __MACH_FLASH_HEADER_H
> +#define __MACH_FLASH_HEADER_H
> +
> +#include <asm-generic/sections.h>
> +
> +#define __flash_header_start		__section(.flash_header_start)
> +
> +#define __flash_header_section		__section(.flash_header_0x0)
> +#define __ps7reg_entry_section		__section(.ps7reg_entry_0x0A0)
> +#define __image_len_section		__section(.image_len_0x08c0)
> +#define FLASH_HEADER_OFFSET		0x0
> +#define IMAGE_OFFSET			0x8c0
> +
> +#define DEST_BASE			0x8c0
> +#define FLASH_HEADER_BASE	(DEST_BASE + FLASH_HEADER_OFFSET)
> +
> +struct zynq_reg_entry {
> +	__le32 addr;
> +	__le32 val;
> +};

Was there a particular reason you kept the ps7reg_entry in a separate
section?  Since the register initialization section of BOOT.BIN is of
fixed size, it may make more sense/be simpler if you just pull it into
the zynq_flash_header (see below):

> +
> +#define WIDTH_DETECTION_MAGIC	0xAA995566
> +#define IMAGE_IDENTIFICATION	0x584C4E58	/* "XLNX" */
> +
> +struct zynq_flash_header {
> +	uint32_t width_det;
> +	uint32_t image_id;
> +	uint32_t enc_stat;
> +	uint32_t user;
> +	uint32_t flash_offset;
> +	uint32_t length;
> +	uint32_t res0;
> +	uint32_t start_of_exec;
> +	uint32_t total_len;
> +	uint32_t res1;
> +	uint32_t checksum;
> +	uint32_t res2;
> +};

These should probably also be of type __le32.

struct zynq_flash_header {
	__le32Â width_det;
	__le32Â image_id;
	__le32Â enc_stat;
	__le32Â user;
	__le32Â flash_offset;
	__le32Â length;
	__le32Â res0;
	__le32Â start_of_exec;
	__le32Â total_len;
	__le32Â res1;
	__le32Â checksum;
	__le32Â res2[0x15];
	structÂ {
	Â Â Â Â Â Â Â Â __le32Â addr;
	Â Â Â Â Â Â Â Â __le32Â val;
	}Â reg_init[0x100];
	__le32Â res3[0x8];
};

Also, the REG() macro you've introduced in the board file seems
like it should go in this file instead.

#define REG(a,v)	{ .addr = cpu_to_le32(a), .val = cpu_to_le32(v) }
#define END_REG		REG(0xFFFFFFFF, 0)

> +
> +#endif /* __MACH_FLASH_HEADER_H */
[..]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox