From mboxrd@z Thu Jan 1 00:00:00 1970 From: Albert ARIBAUD Date: Wed, 07 Jul 2010 12:50:12 +0200 Subject: [U-Boot] [PATCH V2 3/3] edmini: add IDE support In-Reply-To: References: <1278431949-11593-1-git-send-email-albert.aribaud@free.fr> <1278431949-11593-2-git-send-email-albert.aribaud@free.fr> <1278431949-11593-3-git-send-email-albert.aribaud@free.fr> Message-ID: <4C345BE4.9060809@free.fr> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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: . 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.