[U-Boot] [PATCH v2] add Armada XP SATA support

[Stefan Roese]

Hi Anton,

On 15.07.2015 14:03, Anton Schubert wrote:
> This patch initializes the SATA address windows on Armada XP and
> allows it to work with the existing mvsata_ide driver.
> It also adds the necessary configuration for the db-mv784mp-gp board.
>
> Changes v2:
>   - add second bus offset
>   - only allow one device per bus
>   - enable CONFIG_CMD_IDE per default
>
> Signed-off-by: Anton Schubert <anton.schubert at gmx.de>
> Cc: Stefan Roese <sr at denx.de>
> Cc: Luka Perkov <luka.perkov at sartura.hr>

Thanks, this looks much better. The device is now listed only once. Only 
some nitpicking comments left. Otherwise you can add my:

Tested-by: Stefan Roese <sr at denx.de>

to the next patch version.

> ---

The first minor comment is, to better move the patch history (Changes in 
v2:...) below this line "---". The revision comments will then not be 
added to the git repository. This is common practice.

>   arch/arm/mach-mvebu/include/mach/soc.h |  1 +
>   drivers/block/mvsata_ide.c             | 41 ++++++++++++++++++++++++++++++++++
>   include/configs/db-mv784mp-gp.h        | 29 ++++++++++++++++++++++++
>   3 files changed, 71 insertions(+)
>
> diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> index 1aaea67..619bc7e 100644
> --- a/arch/arm/mach-mvebu/include/mach/soc.h
> +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> @@ -52,6 +52,7 @@
>   #define MVEBU_USB20_BASE	(MVEBU_REGISTER(0x58000))
>   #define MVEBU_EGIGA0_BASE	(MVEBU_REGISTER(0x70000))
>   #define MVEBU_EGIGA1_BASE	(MVEBU_REGISTER(0x74000))
> +#define MVEBU_AXP_SATA_BASE	(MVEBU_REGISTER(0xa0000))
>   #define MVEBU_SATA0_BASE	(MVEBU_REGISTER(0xa8000))
>   #define MVEBU_SDIO_BASE		(MVEBU_REGISTER(0xd8000))
>
> diff --git a/drivers/block/mvsata_ide.c b/drivers/block/mvsata_ide.c
> index e54d564..52c1602 100644
> --- a/drivers/block/mvsata_ide.c
> +++ b/drivers/block/mvsata_ide.c
> @@ -13,6 +13,8 @@
>   #include <asm/arch/orion5x.h>
>   #elif defined(CONFIG_KIRKWOOD)
>   #include <asm/arch/soc.h>
> +#elif defined(CONFIG_ARMADA_XP)
> +#include <linux/mbus.h>
>   #endif
>
>   /* SATA port registers */
> @@ -90,6 +92,41 @@ struct mvsata_port_registers {
>   #define MVSATA_STATUS_TIMEOUT	-1
>
>   /*
> + * Registers for SATA MBUS memory windows
> + */
> +
> +#define MVSATA_WIN_CONTROL(w)	(MVEBU_AXP_SATA_BASE + 0x30 + ((w) << 4))
> +#define MVSATA_WIN_BASE(w)	(MVEBU_AXP_SATA_BASE + 0x34 + ((w) << 4))
> +
> +/*
> + * Initialize SATA memory windows for Armada XP
> + */
> +
> +#ifdef CONFIG_ARMADA_XP
> +static void mvsata_ide_conf_mbus_windows(void)
> +{
> +	const struct mbus_dram_target_info *dram;
> +	int i;
> +
> +	dram = mvebu_mbus_dram_info();
> +
> +	/* Disable windows, Set Size/Base to 0  */
> +	for (i = 0; i < 4; i++) {
> +		writel(0, MVSATA_WIN_CONTROL(i));
> +		writel(0, MVSATA_WIN_BASE(i));
> +	}
> +
> +	for (i = 0; i < dram->num_cs; i++) {
> +		const struct mbus_dram_window *cs = dram->cs + i;
> +		writel(((cs->size - 1) & 0xffff0000) | (cs->mbus_attr << 8) |
> +				(dram->mbus_dram_target_id << 4) | 1,
> +				MVSATA_WIN_CONTROL(i));
> +		writel(cs->base & 0xffff0000, MVSATA_WIN_BASE(i));
> +	}
> +}
> +#endif
> +
> +/*
>    * Initialize one MVSATAHC port: set SControl's IPM to "always active"
>    * and DET to "reset", then wait for SStatus's DET to become "device and
>    * comm ok" (or time out after 50 us if no device), then set SControl's
> @@ -137,6 +174,10 @@ int ide_preinit(void)
>   	int ret = MVSATA_STATUS_TIMEOUT;
>   	int status;
>
> +#ifdef CONFIG_ARMADA_XP
> +	mvsata_ide_conf_mbus_windows();
> +#endif
> +
>   	/* Enable ATA port 0 (could be SATA port 0 or 1) if declared */
>   #if defined(CONFIG_SYS_ATA_IDE0_OFFSET)
>   	status = mvsata_ide_initialize_port(
> diff --git a/include/configs/db-mv784mp-gp.h b/include/configs/db-mv784mp-gp.h
> index c33a588..0fdf3a5 100644
> --- a/include/configs/db-mv784mp-gp.h
> +++ b/include/configs/db-mv784mp-gp.h
> @@ -32,6 +32,7 @@
>   #define CONFIG_CMD_SPI
>   #define CONFIG_CMD_TFTPPUT
>   #define CONFIG_CMD_TIME
> +#define CONFIG_CMD_IDE

Please keep the list sorted.

>   /* I2C */
>   #define CONFIG_SYS_I2C
> @@ -60,6 +61,34 @@
>   #define CONFIG_SYS_CONSOLE_INFO_QUIET	/* don't print console @ startup */
>   #define CONFIG_SYS_ALT_MEMTEST
>
> +/* SATA support */
> +#ifdef CONFIG_CMD_IDE
> +#define __io
> +#define CONFIG_IDE_PREINIT
> +#define CONFIG_MVSATA_IDE
> +
> +/* Needs byte-swapping for ATA data register */
> +#define CONFIG_IDE_SWAP_IO
> +
> +#define CONFIG_SYS_ATA_REG_OFFSET	0x0100      /* Offset for normal register accesses*/

This comment breaks the 80 chars rule. Please use the checkpatch tool 
for such issues. Either make the comment shorter, or move it to a 
separate line. And please a space before the "*/".

Thanks,
Stefan