From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Babic <sbabic@denx.de>
Date: Sat, 07 Jul 2012 19:43:50 +0200
Subject: [U-Boot] [PATCH 4/5] [PATCH 4/5] Add FPGA support for the apf27
	board
In-Reply-To: <1340915772-30493-4-git-send-email-tremyfr@yahoo.fr>
References: <1340915772-30493-1-git-send-email-tremyfr@yahoo.fr>
	<1340915772-30493-4-git-send-email-tremyfr@yahoo.fr>
Message-ID: <4FF87556.4010401@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 28/06/2012 22:36, Philippe Reynes wrote:
> Signed-off-by: Philippe Reynes <tremyfr@yahoo.fr>
> Signed-off-by: Eric Jarrige <eric.jarrige@armadeus.org>
> ---

Hi Philippe,


> + *
> + */
> +#include <common.h>
> +
> +#if defined(CONFIG_FPGA)

Maybe you can drop this and compiling this file only if CONFIG_FPGA is
set, changing your Makefile

> +
> +#define GPIO_PORT(x)  ((x & GPIO_PORT_MASK) >> GPIO_PORT_SHIFT)
> +#define GPIO_SET(x)   writel(readl(&gpregs->port[GPIO_PORT(x)].dr) | \
> +		(1 << (x & GPIO_PIN_MASK)), &gpregs->port[GPIO_PORT(x)].dr)
> +
> +/* (DR(GPIO_PORT(x)) |= (1<<(x & GPIO_PIN_MASK))) */
> +#define GPIO_CLEAR(x) writel(readl(&gpregs->port[GPIO_PORT(x)].dr) & \
> +		~(1 << (x & GPIO_PIN_MASK)), &gpregs->port[GPIO_PORT(x)].dr)
> +/* (DR(GPIO_PORT(x)) &= ~(1<<(x & GPIO_PIN_MASK))) */
> +#define GPIO_WRITE(x, y) (y ? GPIO_SET(x) : GPIO_CLEAR(x))
> +#define GPIO_READ(x) ((readl(&gpregs->port[GPIO_PORT(x)].ssr) & \
> +		(1 << (x & GPIO_PIN_MASK))) >> (x & GPIO_PIN_MASK))

No, we have in u-boot a GPIO API to access the GPIOs. Check
./include/asm-generic/gpio.h, and the implementation for other i.MX SOCs.


> +#ifdef FPGA_DEBUG
> +#define	PRINTF(fmt, args...)	printf(fmt, ##args)
> +#else
> +#define	PRINTF(fmt, args...)
> +#endif

Do not set your special version - use DEBUG instead

> +
> +/* Note that these are pointers to code that is in Flash.  They will be
> + * relocated at runtime.
> + * Spartan2 code is used to download our Spartan 3 :) code is compatible.
> + * Just take care about the file size
> +*/

Wrong multiline comment - it should be like this:

/*
 * Part of your multiline
 * comment
 */

Please check this issue everywhere in your code

> +Xilinx_Spartan3_Slave_Parallel_fns fpga_fns = {
> +	fpga_pre_fn,
> +	fpga_pgm_fn,
> +	fpga_init_fn,
> +	NULL,
> +	fpga_done_fn,
> +	fpga_clk_fn,
> +	fpga_cs_fn,
> +	fpga_wr_fn,
> +	fpga_rdata_fn,
> +	fpga_wdata_fn,
> +	fpga_busy_fn,
> +	fpga_abort_fn,
> +	fpga_post_fn,
> +};
> +
> +Xilinx_desc fpga[CONFIG_FPGA_COUNT] = {
> +	{Xilinx_Spartan3,
> +	 slave_parallel,
> +	 1196128l/8,
> +	 (void *) &fpga_fns,
> +	 0}
> +};
> +
> +/*
> + * nitialize GPIO port B before download

initialize

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================