[U-Boot] [PATCH V5 4/4] edminiv2: add mvsata_ide and cmd_ide support

[Prafulla Wadaskar]

> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud at free.fr] 
> Sent: Thursday, August 05, 2010 2:06 AM
> To: Prafulla Wadaskar; Wolfgang Denk
> Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik; 
> Prabhanjan at theia.denx.de; tanmay.upadhyay at einfochips.com
> Subject: Re: [PATCH V5 4/4] edminiv2: add mvsata_ide and 
> cmd_ide support
> 
> Some thoughts, and then a question for Wolfgang regarding IDE 
> config macros
> 
> Le 04/08/2010 08:08, Prafulla Wadaskar a écrit :
> 
> > I have two more suggestion,
> > 1. put ide_preinit function in mvsata driver, define 
> CONFIG_IDE_PREINIT in board config file
> 
> This could help reducing the source code size, because then we don't 
> need an mvsata_ide.h any more as mvsata_ide_initialize_port() would 
> become a static function ov mvsata_ide.c, called by ide_preinit().

I vote for this approach

> 
> One issue is that ide_preinit() should initialize only those 
> ports that 
> exist and are used in a given board. If we put it in 
> mvsata_ide.c, then 
> we need to rely on config macros to tell ide_preinit() where 
> port 0 is, 
> and if port 1 exists/is used, where it is too.
> 
> Note that this info is exactly the same as that given in configs for 
> cmd_ide.
> 
> For instance, for ED Mini V2 (uses port 1 only) we'd have 
> something like
> 
> /* ATA registers base is at SATA controller base */
> #define CONFIG_SYS_ATA_BASE_ADDR	ORION5X_SATA_BASE
> /* ATA bus 0 is orion5x port 1 on ED Mini V2 */
> #define CONFIG_SYS_ATA_IDE0_OFFSET	ORION5X_SATA_PORT1_OFFSET
> 
> ... and for OpenRD, which has two SATA ports:
> 
> /* ATA registers base is at SATA controller base */
> #define CONFIG_SYS_ATA_BASE_ADDR	KW_SATA_BASE
> /* ATA bus 0 is orion5x port 1 on ED Mini V2 */
> #define CONFIG_SYS_ATA_IDE0_OFFSET	KW_SATA_PORT0_OFFSET
> #define CONFIG_SYS_ATA_IDE1_OFFSET	KW_SATA_PORT1_OFFSET
> 
> Theoretically I should create a similar set of macros for mvsata_ide, 
> but it seems to be that since mvsata_ide is intended to 
> always be used 
> along with cmd_ide, it would be simpler to use the cmd_ide macros 
> CONFIG_SYS_ATA_x macros in mvsata_ide.c to determine port0's address 
> and, if port1 exists, port1's address.
> 
> Wolfgang, would it be allowable for mvsata_ide to use macros intended 
> for cmd_ide?

I don't think there will be any issue, let's wait for Wolfgang's feedback

Regards..
Prafulla . .