[U-Boot] [PATCH V2 3/3] edmini: add IDE support

[Albert ARIBAUD]

Le 07/07/2010 10:38, Prafulla Wadaskar a écrit :

> don't you think it is better to add separate driver for Orion/Kirkwood/Marvell
> SATAC so that this code and even some configurations can be shared by other boards.

If you mean 'why use cmd_ide rather than the Marvell sata driver also 
posted recently', I have expressed my opinion there: 
<http://article.gmane.org/gmane.comp.boot-loaders.u-boot/80702>.

If you mean 'why not make this a standalone driver, e.g. 
driver/net/mvsata_ide.{ch} under a configuration option 
CONFIG_MVSATA_IDE' -- why not, but it would basically be empty of 
'driving' functionality (this is handled by cmd_ide) and would only 
provide initialization code, subject to board options (e.g. 
CONFIG_MVSATA_IDE_ENABLE_PORT{0,1}).

>> +	reg = readl(ORION5X_SATA_PORT1_SCONTROL_REG);
>> +	reg = (reg&  ~EDMINIV2_SCONTROL_MASK) | EDMINIV2_PORT_INIT;
>> +	writel(reg, ORION5X_SATA_PORT1_SCONTROL_REG);
>> +	reg = (reg&  ~EDMINIV2_SCONTROL_MASK) | EDMINIV2_PORT_USE;
>> +	writel(reg, ORION5X_SATA_PORT1_SCONTROL_REG);
>
> make use of c-structure for registers

Will do.

>> +#define CONFIG_SYS_ATA_DATA_OFFSET	(0x0100)
>> +#define CONFIG_SYS_ATA_REG_OFFSET	(0x0100)
>> +#define CONFIG_SYS_ATA_ALT_OFFSET	(0x0100)
>
> can you avoid magic numbers, if not some comments please

Will fix this.

Thanks Prafulla for your feedback.

Amicalement,
-- 
Albert.