[U-Boot] [PATCH V7 0/4] Add disk support to orion5x and edminiv2

Patchset history:
V1: Initial 4-patch set -- not cleanly submitted (does not appear on gmane for instance) and contained some unrelated changes.
V2: Slimmed down to 3 patches and removed unrelated changes.
V3: Back to 4 patches (cmd_ide improvement, driver addition, cmd_ide support for orion, edminiv2 support for sata); made initialization code reuseable as a block driver ; moved to C structures; commented some constants.
V4: Reordered objects alphabetically in drivers/block/Makefile. Shortened mv_sata to mvsata. Removed volatile qualifier in mvsata port structure. Moved port pointer definition from SoC to board code. Removed unrelated changes.
V5: Fixed typo in mvsata_ide driver (IMP, should have been IPM).
V6: Fixed case in ide register struct. Fixed warning in deminiv2.c Put IDE configuration under condition.
V7: Reworked the initialization logic to poll status register. Moved initialization from board code to driver code. Removed driver header file and moved its content to driver code. Moved CONFIG_IDE_PREINIT from SoC to board config file.
The patches have been run through checkpatch.pl --no-tree with no errors or warnings, and have been tested on an ED Mini V2.
Albert Aribaud (4): ide: add configuration CONFIG_IDE_SWAP_IO ide: add mvsata_ide driver cmd_ide: add support for orion5x edminiv2: add mvsata_ide and cmd_ide support
arch/arm/include/asm/arch-orion5x/orion5x.h | 3 + arch/powerpc/include/asm/config.h | 3 + common/cmd_ide.c | 23 +++-- doc/README.PXA_CF | 8 ++ drivers/block/Makefile | 7 +- drivers/block/mvsata_ide.c | 140 +++++++++++++++++++++++++++ include/configs/ap325rxa.h | 1 + include/configs/edminiv2.h | 34 ++++++- include/configs/ms7720se.h | 1 + include/configs/r2dplus.h | 1 + include/configs/r7780mp.h | 1 + 11 files changed, 209 insertions(+), 13 deletions(-) create mode 100644 drivers/block/mvsata_ide.c

This configuration option replaces a complex conditional in cmd_ide.c with an explicit define to be added to SoC or board configs.
Signed-off-by: Albert Aribaud albert.aribaud@free.fr --- arch/powerpc/include/asm/config.h | 3 +++ common/cmd_ide.c | 18 +++++++++--------- doc/README.PXA_CF | 8 ++++++++ include/configs/ap325rxa.h | 1 + include/configs/ms7720se.h | 1 + include/configs/r2dplus.h | 1 + include/configs/r7780mp.h | 1 + 7 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/config.h b/arch/powerpc/include/asm/config.h index f70699d..d098657 100644 --- a/arch/powerpc/include/asm/config.h +++ b/arch/powerpc/include/asm/config.h @@ -95,4 +95,7 @@ #define CONFIG_FSL_LBC #endif
+/* All PPC boards must swap IDE bytes */ +#define CONFIG_IDE_SWAP_IO + #endif /* _ASM_CONFIG_H_ */ diff --git a/common/cmd_ide.c b/common/cmd_ide.c index c0fb88d..d423e53 100644 --- a/common/cmd_ide.c +++ b/common/cmd_ide.c @@ -847,7 +847,7 @@ input_swap_data(int dev, ulong *sect_buf, int words) #endif /* __LITTLE_ENDIAN || CONFIG_AU1X00 */
-#if defined(__PPC__) || defined(CONFIG_PXA_PCMCIA) || defined(CONFIG_SH) +#if defined(CONFIG_IDE_SWAP_IO) static void output_data(int dev, ulong *sect_buf, int words) { @@ -891,15 +891,15 @@ output_data(int dev, ulong *sect_buf, int words) } #endif } -#else /* ! __PPC__ */ +#else /* ! CONFIG_IDE_SWAP_IO */ static void output_data(int dev, ulong *sect_buf, int words) { outsw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, words<<1); } -#endif /* __PPC__ */ +#endif /* CONFIG_IDE_SWAP_IO */
-#if defined(__PPC__) || defined(CONFIG_PXA_PCMCIA) || defined(CONFIG_SH) +#if defined(CONFIG_IDE_SWAP_IO) static void input_data(int dev, ulong *sect_buf, int words) { @@ -949,14 +949,14 @@ input_data(int dev, ulong *sect_buf, int words) } #endif } -#else /* ! __PPC__ */ +#else /* ! CONFIG_IDE_SWAP_IO */ static void input_data(int dev, ulong *sect_buf, int words) { insw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, words << 1); }
-#endif /* __PPC__ */ +#endif /* CONFIG_IDE_SWAP_IO */
/* ------------------------------------------------------------------------- */ @@ -1573,7 +1573,7 @@ int ide_device_present(int dev) * ATAPI Support */
-#if defined(__PPC__) || defined(CONFIG_PXA_PCMCIA) +#if defined(CONFIG_IDE_SWAP_IO) /* since ATAPI may use commands with not 4 bytes alligned length * we have our own transfer functions, 2 bytes alligned */ static void @@ -1640,7 +1640,7 @@ input_data_shorts(int dev, ushort *sect_buf, int shorts) #endif }
-#else /* ! __PPC__ */ +#else /* ! CONFIG_IDE_SWAP_IO */ static void output_data_shorts(int dev, ushort *sect_buf, int shorts) { @@ -1653,7 +1653,7 @@ input_data_shorts(int dev, ushort *sect_buf, int shorts) insw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, shorts); }
-#endif /* __PPC__ */ +#endif /* CONFIG_IDE_SWAP_IO */
/* * Wait until (Status & mask) == res, or timeout (in ms) diff --git a/doc/README.PXA_CF b/doc/README.PXA_CF index 6a0f236..1d76b32 100644 --- a/doc/README.PXA_CF +++ b/doc/README.PXA_CF @@ -6,6 +6,14 @@ follow the connections of the standard lubbock. Anyway just the block marked memory configuration should be touched since the other parameters are imposed by the PXA architecture.
+EDIT 2010-07-01: in common/cmd_ide.c, having CONFIG_PXA_PCMCIA defined +would cause looping on inw()/outw() rather than using insw()/outsw(), +thus making sure IDE / ATA bytes are properly swapped. This behaviour +is now controlled by CONFIG_IDE_SWAP_IO, therefore PXA boards with +PCMCIA should #define CONFIG_IDE_SWAP_IO. + +#define CONFIG_IDE_SWAP_IO + #define CONFIG_PXA_PCMCIA 1 #define CONFIG_PXA_IDE 1
diff --git a/include/configs/ap325rxa.h b/include/configs/ap325rxa.h index 70dd47e..80a5797 100644 --- a/include/configs/ap325rxa.h +++ b/include/configs/ap325rxa.h @@ -138,6 +138,7 @@ #define CONFIG_SYS_ATA_DATA_OFFSET 0x200 /* data reg offset */ #define CONFIG_SYS_ATA_REG_OFFSET 0x200 /* reg offset */ #define CONFIG_SYS_ATA_ALT_OFFSET 0x210 /* alternate register offset */ +#define CONFIG_IDE_SWAP_IO
/* if you use all NOR Flash , you change dip-switch. Please see Manual. */ #define CONFIG_SYS_MAX_FLASH_BANKS 1 diff --git a/include/configs/ms7720se.h b/include/configs/ms7720se.h index ba0a3f8..0ea3527 100644 --- a/include/configs/ms7720se.h +++ b/include/configs/ms7720se.h @@ -122,5 +122,6 @@ #define CONFIG_SYS_ATA_DATA_OFFSET 0 /* data reg offset */ #define CONFIG_SYS_ATA_REG_OFFSET 0 /* reg offset */ #define CONFIG_SYS_ATA_ALT_OFFSET 0x200 /* alternate register offset */ +#define CONFIG_IDE_SWAP_IO
#endif /* __MS7720SE_H */ diff --git a/include/configs/r2dplus.h b/include/configs/r2dplus.h index 8931b97..955f3ff 100644 --- a/include/configs/r2dplus.h +++ b/include/configs/r2dplus.h @@ -96,6 +96,7 @@ #define CONFIG_SYS_ATA_DATA_OFFSET 0x1000 /* data reg offset */ #define CONFIG_SYS_ATA_REG_OFFSET 0x1000 /* reg offset */ #define CONFIG_SYS_ATA_ALT_OFFSET 0x800 /* alternate register offset */ +#define CONFIG_IDE_SWAP_IO
/* * SuperH PCI Bridge Configration diff --git a/include/configs/r7780mp.h b/include/configs/r7780mp.h index 71c570e..3afe93a 100644 --- a/include/configs/r7780mp.h +++ b/include/configs/r7780mp.h @@ -171,6 +171,7 @@ #define CONFIG_SYS_ATA_DATA_OFFSET 0x1000 /* data reg offset */ #define CONFIG_SYS_ATA_REG_OFFSET 0x1000 /* reg offset */ #define CONFIG_SYS_ATA_ALT_OFFSET 0x800 /* alternate register offset */ +#define CONFIG_IDE_SWAP_IO #endif /* CONFIG_CMD_IDE */
#endif /* __R7780RP_H */

This driver only provides initialization code; actual driving is done by cmd_ide.c using the ATA compatibility mode of the Marvell SATAHC controller.
Signed-off-by: Albert Aribaud albert.aribaud@free.fr --- drivers/block/Makefile | 7 +- drivers/block/mvsata_ide.c | 140 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 drivers/block/mvsata_ide.c
diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 3f6ad5c..64dcf4e 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -25,15 +25,16 @@ include $(TOPDIR)/config.mk
LIB := $(obj)libblock.a
+COBJS-$(CONFIG_SCSI_AHCI) += ahci.o COBJS-$(CONFIG_ATA_PIIX) += ata_piix.o -COBJS-$(CONFIG_CMD_MG_DISK) += mg_disk.o COBJS-$(CONFIG_FSL_SATA) += fsl_sata.o -COBJS-$(CONFIG_IDE_SIL680) += sil680.o COBJS-$(CONFIG_LIBATA) += libata.o +COBJS-$(CONFIG_CMD_MG_DISK) += mg_disk.o +COBJS-$(CONFIG_MVSATA_IDE) += mvsata_ide.o COBJS-$(CONFIG_PATA_BFIN) += pata_bfin.o COBJS-$(CONFIG_SATA_DWC) += sata_dwc.o COBJS-$(CONFIG_SATA_SIL3114) += sata_sil3114.o -COBJS-$(CONFIG_SCSI_AHCI) += ahci.o +COBJS-$(CONFIG_IDE_SIL680) += sil680.o COBJS-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o COBJS-$(CONFIG_SYSTEMACE) += systemace.o
diff --git a/drivers/block/mvsata_ide.c b/drivers/block/mvsata_ide.c new file mode 100644 index 0000000..8ac84ea --- /dev/null +++ b/drivers/block/mvsata_ide.c @@ -0,0 +1,140 @@ +/* + * Copyright (C) 2010 Albert ARIBAUD albert.aribaud@free.fr + * + * Written-by: Albert ARIBAUD albert.aribaud@free.fr + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA + */ + +#include <common.h> +#include <asm/io.h> + +#if defined(CONFIG_ORION5X) +#include <asm/arch/orion5x.h> +#elif defined(CONFIG_KIRKWOOD) +#include <asm/arch/kirkwood.h> +#endif + +/* SATA port registers */ +struct mvsata_port_registers { + u32 reserved1[192]; + /* offset 0x300 : ATA Interface registers */ + u32 sstatus; + u32 serror; + u32 scontrol; + u32 ltmode; + u32 phymode3; + u32 phymode4; + u32 reserved2[5]; + u32 phymode1; + u32 phymode2; + u32 bist_cr; + u32 bist_dw1; + u32 bist_dw2; + u32 serrorintrmask; +}; + +/* + * Sanity checks: + * - to compile at all, we need CONFIG_SYS_ATA_BASE_ADDR. + * - for ide_preinit to make sense, we need at least one of + * CONFIG_SYS_ATA_IDE0_OFFSET or CONFIG_SYS_ATA_IDE0_OFFSET; + * - for inde_preinit to be called, we need CONFIG_IDE_PREINIT. + * Fail with an explanation message if these conditions are not met. + * This is particularly important for CONFIG_IDE_PREINIT, because + * its lack would not cause a build error. + */ + +#if !defined(CONFIG_SYS_ATA_BASE_ADDR) +#error CONFIG_SYS_ATA_BASE_ADDR must be defined +#elif !defined(CONFIG_SYS_ATA_IDE0_OFFSET) \ + && !defined(CONFIG_SYS_ATA_IDE1_OFFSET) +#error CONFIG_SYS_ATA_IDE0_OFFSET or CONFIG_SYS_ATA_IDE1_OFFSET \ + must be defined +#elif !defined(CONFIG_IDE_PREINIT) +#error CONFIG_IDE_PREINIT must be defined +#endif + +/* + * Masks and values for SControl DETection and Interface Power Management, + * and for SStatus DETection. + */ + +#define MVSATA_SCONTROL_DET_MASK 0x0000000F +#define MVSATA_SCONTROL_DET_NONE 0x00000000 +#define MVSATA_SCONTROL_DET_INIT 0x00000001 +#define MVSATA_SCONTROL_IPM_MASK 0x00000F00 +#define MVSATA_SCONTROL_IPM_NO_LP_ALLOWED 0x00000300 +#define MVSATA_SCONTROL_MASK \ + (MVSATA_SCONTROL_DET_MASK|MVSATA_SCONTROL_IPM_MASK) +#define MVSATA_PORT_INIT \ + (MVSATA_SCONTROL_DET_INIT|MVSATA_SCONTROL_IPM_NO_LP_ALLOWED) +#define MVSATA_PORT_USE \ + (MVSATA_SCONTROL_DET_NONE|MVSATA_SCONTROL_IPM_NO_LP_ALLOWED) +#define MVSATA_SSTATUS_DET_MASK 0x0000000F +#define MVSATA_SSTATUS_DET_DEVCOMM 0x00000003 + +/* + * 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 + * DET back to "no action". + */ + +static void mvsata_ide_initialize_port(struct mvsata_port_registers *port) +{ + u32 control; + u32 status; + u32 tout = 50; /* wait at most 50 us for SATA reset to complete */ + + control = readl(&port->scontrol); + control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_INIT; + writel(control, &port->scontrol); + while (--tout) { + status = readl(&port->sstatus) & MVSATA_SSTATUS_DET_MASK; + if (status == MVSATA_SSTATUS_DET_DEVCOMM) + break; + udelay(1); + } + control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_USE; + writel(control, &port->scontrol); +} + +/* + * ide_preinit() will be called by ide_init in cmd_ide.c and will + * reset the MVSTATHC ports needed by the board. + */ + +int ide_preinit(void) +{ + /* Enable ATA port 0 (could be SATA port 0 or 1) if declared */ +#if defined(CONFIG_SYS_ATA_IDE0_OFFSET) + mvsata_ide_initialize_port( + (struct mv_sata_port_registers *) + (CONFIG_SYS_ATA_BASE_ADDR + CONFIG_SYS_ATA_IDE0_OFFSET)); +#endif + /* Enable ATA port 1 (could be SATA port 0 or 1) if declared */ +#if defined(CONFIG_SYS_ATA_IDE1_OFFSET) + mvsata_ide_initialize_port( + (struct mv_sata_port_registers *) + (CONFIG_SYS_ATA_BASE_ADDR + CONFIG_SYS_ATA_IDE1_OFFSET)); +#endif + /* return 0 as we always succeed */ + return 0; +}

Add MVSATAHC definitions to orion5x. Add support for orion5x in cmd_ide.
Signed-off-by: Albert Aribaud albert.aribaud@free.fr --- arch/arm/include/asm/arch-orion5x/orion5x.h | 3 +++ common/cmd_ide.c | 5 +++++ 2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/arch-orion5x/orion5x.h b/arch/arm/include/asm/arch-orion5x/orion5x.h index d257b66..e3d3f76 100644 --- a/arch/arm/include/asm/arch-orion5x/orion5x.h +++ b/arch/arm/include/asm/arch-orion5x/orion5x.h @@ -55,6 +55,9 @@ #define ORION5X_USB20_PORT0_BASE (ORION5X_REGISTER(0x50000)) #define ORION5X_USB20_PORT1_BASE (ORION5X_REGISTER(0xA0000)) #define ORION5X_EGIGA_BASE (ORION5X_REGISTER(0x72000)) +#define ORION5X_SATA_BASE (ORION5X_REGISTER(0x80000)) +#define ORION5X_SATA_PORT0_OFFSET 0x2000 +#define ORION5X_SATA_PORT1_OFFSET 0x4000
/* Orion5x GbE controller has a single port */ #define MAX_MVGBE_DEVS 1 diff --git a/common/cmd_ide.c b/common/cmd_ide.c index d423e53..b23db3f 100644 --- a/common/cmd_ide.c +++ b/common/cmd_ide.c @@ -25,6 +25,7 @@ /* * IDE support */ + #include <common.h> #include <config.h> #include <watchdog.h> @@ -45,6 +46,10 @@ #include <mpc5xxx.h> #endif
+#ifdef CONFIG_ORION5X +#include <asm/arch/orion5x.h> +#endif + #include <ide.h> #include <ata.h>

Add mvsata_ide and cmd_ide configuration in edminiv2 config
Signed-off-by: Albert Aribaud albert.aribaud@free.fr --- include/configs/edminiv2.h | 34 +++++++++++++++++++++++++++++++++- 1 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/include/configs/edminiv2.h b/include/configs/edminiv2.h index 055931c..57dd165 100644 --- a/include/configs/edminiv2.h +++ b/include/configs/edminiv2.h @@ -60,7 +60,7 @@
#define ORION5X_MPP0_7 0x00000003 #define ORION5X_MPP8_15 0x55550000 -#define ORION5X_MPP16_23 0x00000000 +#define ORION5X_MPP16_23 0x00005555
/* * Board-specific values for Orion5x GPIO low level init: @@ -131,6 +131,7 @@ * Commands configuration - using default command set for now */ #include <config_cmd_default.h> +#define CONFIG_CMD_IDE
/* * Network @@ -150,6 +151,37 @@ #endif
/* + * IDE + */ +#ifdef CONFIG_CMD_IDE +#define __io +#define CONFIG_IDE_PREINIT +#define CONFIG_DOS_PARTITION +#define CONFIG_CMD_EXT2 +/* ED Mini V has an IDE-compatible SATA connector for port 1 */ +#define CONFIG_MVSATA_IDE +#define CONFIG_MVSATA_IDE_USE_PORT1 +/* Needs byte-swapping for ATA data register */ +#define CONFIG_IDE_SWAP_IO +/* Data, registers and alternate blocks are at the same offset */ +#define CONFIG_SYS_ATA_DATA_OFFSET (0x0100) +#define CONFIG_SYS_ATA_REG_OFFSET (0x0100) +#define CONFIG_SYS_ATA_ALT_OFFSET (0x0100) +/* Each 8-bit ATA register is aligned to a 4-bytes address */ +#define CONFIG_SYS_ATA_STRIDE 4 +/* Controller supports 48-bits LBA addressing */ +#define CONFIG_LBA48 +/* A single bus, a single device */ +#define CONFIG_SYS_IDE_MAXBUS 1 +#define CONFIG_SYS_IDE_MAXDEVICE 1 +/* 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 +/* end of IDE defines */ +#endif /* CMD_IDE */ + +/* * Environment variables configurations */ #define CONFIG_ENV_IS_IN_FLASH 1

-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert Aribaud Sent: Thursday, August 05, 2010 6:06 PM To: u-boot@lists.denx.de Subject: [U-Boot] [PATCH V7 4/4] edminiv2: add mvsata_ide and cmd_ide support
Add mvsata_ide and cmd_ide configuration in edminiv2 config
Signed-off-by: Albert Aribaud albert.aribaud@free.fr
include/configs/edminiv2.h | 34 +++++++++++++++++++++++++++++++++- 1 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/include/configs/edminiv2.h b/include/configs/edminiv2.h index 055931c..57dd165 100644 --- a/include/configs/edminiv2.h +++ b/include/configs/edminiv2.h @@ -60,7 +60,7 @@
#define ORION5X_MPP0_7 0x00000003 #define ORION5X_MPP8_15 0x55550000 -#define ORION5X_MPP16_23 0x00000000 +#define ORION5X_MPP16_23 0x00005555
/*
- Board-specific values for Orion5x GPIO low level init:
@@ -131,6 +131,7 @@
- Commands configuration - using default command set for now
*/ #include <config_cmd_default.h> +#define CONFIG_CMD_IDE
/*
- Network
@@ -150,6 +151,37 @@ #endif
/*
- IDE
- */
+#ifdef CONFIG_CMD_IDE +#define __io +#define CONFIG_IDE_PREINIT +#define CONFIG_DOS_PARTITION +#define CONFIG_CMD_EXT2 +/* ED Mini V has an IDE-compatible SATA connector for port 1 */ +#define CONFIG_MVSATA_IDE +#define CONFIG_MVSATA_IDE_USE_PORT1 +/* Needs byte-swapping for ATA data register */ +#define CONFIG_IDE_SWAP_IO +/* Data, registers and alternate blocks are at the same offset */ +#define CONFIG_SYS_ATA_DATA_OFFSET (0x0100) +#define CONFIG_SYS_ATA_REG_OFFSET (0x0100) +#define CONFIG_SYS_ATA_ALT_OFFSET (0x0100) +/* Each 8-bit ATA register is aligned to a 4-bytes address */ +#define CONFIG_SYS_ATA_STRIDE 4 +/* Controller supports 48-bits LBA addressing */ +#define CONFIG_LBA48 +/* A single bus, a single device */ +#define CONFIG_SYS_IDE_MAXBUS 1 +#define CONFIG_SYS_IDE_MAXDEVICE 1 +/* 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 +/* end of IDE defines */ +#endif /* CMD_IDE */
+/*
- Environment variables configurations
*/
#define CONFIG_ENV_IS_IN_FLASH 1
Acked-by: Prafulla Wadaskar prafulla@marvell.com
Regards.. Prafulla . .

-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert Aribaud Sent: Thursday, August 05, 2010 6:06 PM To: u-boot@lists.denx.de Subject: [U-Boot] [PATCH V7 3/4] cmd_ide: add support for orion5x
Add MVSATAHC definitions to orion5x. Add support for orion5x in cmd_ide.
Signed-off-by: Albert Aribaud albert.aribaud@free.fr
arch/arm/include/asm/arch-orion5x/orion5x.h | 3 +++ common/cmd_ide.c | 5 +++++ 2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/arch-orion5x/orion5x.h b/arch/arm/include/asm/arch-orion5x/orion5x.h index d257b66..e3d3f76 100644 --- a/arch/arm/include/asm/arch-orion5x/orion5x.h +++ b/arch/arm/include/asm/arch-orion5x/orion5x.h @@ -55,6 +55,9 @@ #define ORION5X_USB20_PORT0_BASE (ORION5X_REGISTER(0x50000)) #define ORION5X_USB20_PORT1_BASE (ORION5X_REGISTER(0xA0000)) #define ORION5X_EGIGA_BASE (ORION5X_REGISTER(0x72000)) +#define ORION5X_SATA_BASE (ORION5X_REGISTER(0x80000)) +#define ORION5X_SATA_PORT0_OFFSET 0x2000 +#define ORION5X_SATA_PORT1_OFFSET 0x4000
These two macros are mvsata specific and going to duplicate in Kirkwood port too. Why not to move them to mvsata.c?
/* Orion5x GbE controller has a single port */ #define MAX_MVGBE_DEVS 1 diff --git a/common/cmd_ide.c b/common/cmd_ide.c index d423e53..b23db3f 100644 --- a/common/cmd_ide.c +++ b/common/cmd_ide.c @@ -25,6 +25,7 @@ /*
- IDE support
*/
#include <common.h> #include <config.h> #include <watchdog.h> @@ -45,6 +46,10 @@ #include <mpc5xxx.h> #endif
+#ifdef CONFIG_ORION5X +#include <asm/arch/orion5x.h> +#endif
What makes it important to include this file here?
Regards.. Prafulla . .

Le 05/08/2010 20:43, Prafulla Wadaskar a écrit :
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert Aribaud Sent: Thursday, August 05, 2010 6:06 PM To: u-boot@lists.denx.de Subject: [U-Boot] [PATCH V7 3/4] cmd_ide: add support for orion5x
Add MVSATAHC definitions to orion5x. Add support for orion5x in cmd_ide.
Signed-off-by: Albert Aribaudalbert.aribaud@free.fr
arch/arm/include/asm/arch-orion5x/orion5x.h | 3 +++ common/cmd_ide.c | 5 +++++ 2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/arch-orion5x/orion5x.h b/arch/arm/include/asm/arch-orion5x/orion5x.h index d257b66..e3d3f76 100644 --- a/arch/arm/include/asm/arch-orion5x/orion5x.h +++ b/arch/arm/include/asm/arch-orion5x/orion5x.h @@ -55,6 +55,9 @@ #define ORION5X_USB20_PORT0_BASE (ORION5X_REGISTER(0x50000)) #define ORION5X_USB20_PORT1_BASE (ORION5X_REGISTER(0xA0000)) #define ORION5X_EGIGA_BASE (ORION5X_REGISTER(0x72000)) +#define ORION5X_SATA_BASE (ORION5X_REGISTER(0x80000)) +#define ORION5X_SATA_PORT0_OFFSET 0x2000 +#define ORION5X_SATA_PORT1_OFFSET 0x4000
These two macros are mvsata specific and going to duplicate in Kirkwood port too. Why not to move them to mvsata.c?
Because the abstracted driver does not know at what address it is instantiated in each SoC; this is known at SoC level.
/* Orion5x GbE controller has a single port */ #define MAX_MVGBE_DEVS 1 diff --git a/common/cmd_ide.c b/common/cmd_ide.c index d423e53..b23db3f 100644 --- a/common/cmd_ide.c +++ b/common/cmd_ide.c @@ -25,6 +25,7 @@ /*
- IDE support
*/
- #include<common.h> #include<config.h> #include<watchdog.h>
@@ -45,6 +46,10 @@ #include<mpc5xxx.h> #endif
+#ifdef CONFIG_ORION5X +#include<asm/arch/orion5x.h> +#endif
What makes it important to include this file here?
This file uses the CONFIG_SYS_ATA_xxx macros, which expand to ORION5X (and KW for kirkwood) macros.
The macros and include are actually based on the same principle as for the mvgbe driver base and port offset.
Regards.. Prafulla . .
Amicalement,

-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert Aribaud Sent: Thursday, August 05, 2010 6:06 PM To: u-boot@lists.denx.de Subject: [U-Boot] [PATCH V7 2/4] ide: add mvsata_ide driver
This driver only provides initialization code; actual driving is done by cmd_ide.c using the ATA compatibility mode of the Marvell SATAHC controller.
Signed-off-by: Albert Aribaud albert.aribaud@free.fr
drivers/block/Makefile | 7 +- drivers/block/mvsata_ide.c | 140 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 drivers/block/mvsata_ide.c
diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 3f6ad5c..64dcf4e 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -25,15 +25,16 @@ include $(TOPDIR)/config.mk
LIB := $(obj)libblock.a
+COBJS-$(CONFIG_SCSI_AHCI) += ahci.o COBJS-$(CONFIG_ATA_PIIX) += ata_piix.o -COBJS-$(CONFIG_CMD_MG_DISK) += mg_disk.o COBJS-$(CONFIG_FSL_SATA) += fsl_sata.o -COBJS-$(CONFIG_IDE_SIL680) += sil680.o COBJS-$(CONFIG_LIBATA) += libata.o +COBJS-$(CONFIG_CMD_MG_DISK) += mg_disk.o +COBJS-$(CONFIG_MVSATA_IDE) += mvsata_ide.o COBJS-$(CONFIG_PATA_BFIN) += pata_bfin.o COBJS-$(CONFIG_SATA_DWC) += sata_dwc.o COBJS-$(CONFIG_SATA_SIL3114) += sata_sil3114.o -COBJS-$(CONFIG_SCSI_AHCI) += ahci.o +COBJS-$(CONFIG_IDE_SIL680) += sil680.o COBJS-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o COBJS-$(CONFIG_SYSTEMACE) += systemace.o
Hi Albert You have merged two changes here, 1. reordering the entire Makefile which is good thing, but irrelevant here (could be separate patch). 2. mvsata support updates, which is only relevant to the patch subject here.
Otherwise, ack for rest of the code Regards.. Prafulla . .

Hi Prafulla,
Le 05/08/2010 21:09, Prafulla Wadaskar a écrit :
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert Aribaud Sent: Thursday, August 05, 2010 6:06 PM To: u-boot@lists.denx.de Subject: [U-Boot] [PATCH V7 2/4] ide: add mvsata_ide driver
This driver only provides initialization code; actual driving is done by cmd_ide.c using the ATA compatibility mode of the Marvell SATAHC controller.
Signed-off-by: Albert Aribaudalbert.aribaud@free.fr
drivers/block/Makefile | 7 +- drivers/block/mvsata_ide.c | 140 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 drivers/block/mvsata_ide.c
diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 3f6ad5c..64dcf4e 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -25,15 +25,16 @@ include $(TOPDIR)/config.mk
LIB := $(obj)libblock.a
+COBJS-$(CONFIG_SCSI_AHCI) += ahci.o COBJS-$(CONFIG_ATA_PIIX) += ata_piix.o -COBJS-$(CONFIG_CMD_MG_DISK) += mg_disk.o COBJS-$(CONFIG_FSL_SATA) += fsl_sata.o -COBJS-$(CONFIG_IDE_SIL680) += sil680.o COBJS-$(CONFIG_LIBATA) += libata.o +COBJS-$(CONFIG_CMD_MG_DISK) += mg_disk.o +COBJS-$(CONFIG_MVSATA_IDE) += mvsata_ide.o COBJS-$(CONFIG_PATA_BFIN) += pata_bfin.o COBJS-$(CONFIG_SATA_DWC) += sata_dwc.o COBJS-$(CONFIG_SATA_SIL3114) += sata_sil3114.o -COBJS-$(CONFIG_SCSI_AHCI) += ahci.o +COBJS-$(CONFIG_IDE_SIL680) += sil680.o COBJS-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o COBJS-$(CONFIG_SYSTEMACE) += systemace.o
Hi Albert You have merged two changes here,
- reordering the entire Makefile which is good thing, but irrelevant here (could be separate patch).
- mvsata support updates, which is only relevant to the patch subject here.
Otherwise, ack for rest of the code
The reordering was asked for in a previous review round.
Regards.. Prafulla . .
Amicalement,

Additional comment:
Le 05/08/2010 23:29, Albert ARIBAUD a écrit :
Hi Albert You have merged two changes here,
- reordering the entire Makefile which is good thing, but irrelevant here (could be separate patch).
- mvsata support updates, which is only relevant to the patch subject here.
Otherwise, ack for rest of the code
The reordering was asked for in a previous review round.
... which I had taken to mean 'please take the opportunity of this patch to also reorder', but of course I can and will do the split in two different patches in the set.
Amicalement,

Hi Albert
can you pls fix these warnings too
mvsata_ide.c: In function ‘ide_preinit’: mvsata_ide.c:130: warning: passing argument 1 of ‘mvsata_ide_initialize_port’ from incompatible pointer type mvsata_ide.c:136: warning: passing argument 1 of ‘mvsata_ide_initialize_port’ from incompatible pointer type
Regards.. Prafulla . . ________________________________________ From: u-boot-bounces@lists.denx.de [u-boot-bounces@lists.denx.de] On Behalf Of Albert Aribaud [albert.aribaud@free.fr] Sent: Thursday, August 05, 2010 6:05 PM To: u-boot@lists.denx.de Subject: [U-Boot] [PATCH V7 2/4] ide: add mvsata_ide driver
This driver only provides initialization code; actual driving is done by cmd_ide.c using the ATA compatibility mode of the Marvell SATAHC controller.
Signed-off-by: Albert Aribaud albert.aribaud@free.fr --- drivers/block/Makefile | 7 +- drivers/block/mvsata_ide.c | 140 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 drivers/block/mvsata_ide.c
diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 3f6ad5c..64dcf4e 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -25,15 +25,16 @@ include $(TOPDIR)/config.mk
LIB := $(obj)libblock.a
+COBJS-$(CONFIG_SCSI_AHCI) += ahci.o COBJS-$(CONFIG_ATA_PIIX) += ata_piix.o -COBJS-$(CONFIG_CMD_MG_DISK) += mg_disk.o COBJS-$(CONFIG_FSL_SATA) += fsl_sata.o -COBJS-$(CONFIG_IDE_SIL680) += sil680.o COBJS-$(CONFIG_LIBATA) += libata.o +COBJS-$(CONFIG_CMD_MG_DISK) += mg_disk.o +COBJS-$(CONFIG_MVSATA_IDE) += mvsata_ide.o COBJS-$(CONFIG_PATA_BFIN) += pata_bfin.o COBJS-$(CONFIG_SATA_DWC) += sata_dwc.o COBJS-$(CONFIG_SATA_SIL3114) += sata_sil3114.o -COBJS-$(CONFIG_SCSI_AHCI) += ahci.o +COBJS-$(CONFIG_IDE_SIL680) += sil680.o COBJS-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o COBJS-$(CONFIG_SYSTEMACE) += systemace.o
diff --git a/drivers/block/mvsata_ide.c b/drivers/block/mvsata_ide.c new file mode 100644 index 0000000..8ac84ea --- /dev/null +++ b/drivers/block/mvsata_ide.c @@ -0,0 +1,140 @@ +/* + * Copyright (C) 2010 Albert ARIBAUD albert.aribaud@free.fr + * + * Written-by: Albert ARIBAUD albert.aribaud@free.fr + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA + */ + +#include <common.h> +#include <asm/io.h> + +#if defined(CONFIG_ORION5X) +#include <asm/arch/orion5x.h> +#elif defined(CONFIG_KIRKWOOD) +#include <asm/arch/kirkwood.h> +#endif + +/* SATA port registers */ +struct mvsata_port_registers { + u32 reserved1[192]; + /* offset 0x300 : ATA Interface registers */ + u32 sstatus; + u32 serror; + u32 scontrol; + u32 ltmode; + u32 phymode3; + u32 phymode4; + u32 reserved2[5]; + u32 phymode1; + u32 phymode2; + u32 bist_cr; + u32 bist_dw1; + u32 bist_dw2; + u32 serrorintrmask; +}; + +/* + * Sanity checks: + * - to compile at all, we need CONFIG_SYS_ATA_BASE_ADDR. + * - for ide_preinit to make sense, we need at least one of + * CONFIG_SYS_ATA_IDE0_OFFSET or CONFIG_SYS_ATA_IDE0_OFFSET; + * - for inde_preinit to be called, we need CONFIG_IDE_PREINIT. + * Fail with an explanation message if these conditions are not met. + * This is particularly important for CONFIG_IDE_PREINIT, because + * its lack would not cause a build error. + */ + +#if !defined(CONFIG_SYS_ATA_BASE_ADDR) +#error CONFIG_SYS_ATA_BASE_ADDR must be defined +#elif !defined(CONFIG_SYS_ATA_IDE0_OFFSET) \ + && !defined(CONFIG_SYS_ATA_IDE1_OFFSET) +#error CONFIG_SYS_ATA_IDE0_OFFSET or CONFIG_SYS_ATA_IDE1_OFFSET \ + must be defined +#elif !defined(CONFIG_IDE_PREINIT) +#error CONFIG_IDE_PREINIT must be defined +#endif + +/* + * Masks and values for SControl DETection and Interface Power Management, + * and for SStatus DETection. + */ + +#define MVSATA_SCONTROL_DET_MASK 0x0000000F +#define MVSATA_SCONTROL_DET_NONE 0x00000000 +#define MVSATA_SCONTROL_DET_INIT 0x00000001 +#define MVSATA_SCONTROL_IPM_MASK 0x00000F00 +#define MVSATA_SCONTROL_IPM_NO_LP_ALLOWED 0x00000300 +#define MVSATA_SCONTROL_MASK \ + (MVSATA_SCONTROL_DET_MASK|MVSATA_SCONTROL_IPM_MASK) +#define MVSATA_PORT_INIT \ + (MVSATA_SCONTROL_DET_INIT|MVSATA_SCONTROL_IPM_NO_LP_ALLOWED) +#define MVSATA_PORT_USE \ + (MVSATA_SCONTROL_DET_NONE|MVSATA_SCONTROL_IPM_NO_LP_ALLOWED) +#define MVSATA_SSTATUS_DET_MASK 0x0000000F +#define MVSATA_SSTATUS_DET_DEVCOMM 0x00000003 + +/* + * 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 + * DET back to "no action". + */ + +static void mvsata_ide_initialize_port(struct mvsata_port_registers *port) +{ + u32 control; + u32 status; + u32 tout = 50; /* wait at most 50 us for SATA reset to complete */ + + control = readl(&port->scontrol); + control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_INIT; + writel(control, &port->scontrol); + while (--tout) { + status = readl(&port->sstatus) & MVSATA_SSTATUS_DET_MASK; + if (status == MVSATA_SSTATUS_DET_DEVCOMM) + break; + udelay(1); + } + control = (control & ~MVSATA_SCONTROL_MASK) | MVSATA_PORT_USE; + writel(control, &port->scontrol); +} + +/* + * ide_preinit() will be called by ide_init in cmd_ide.c and will + * reset the MVSTATHC ports needed by the board. + */ + +int ide_preinit(void) +{ + /* Enable ATA port 0 (could be SATA port 0 or 1) if declared */ +#if defined(CONFIG_SYS_ATA_IDE0_OFFSET) + mvsata_ide_initialize_port( + (struct mv_sata_port_registers *) + (CONFIG_SYS_ATA_BASE_ADDR + CONFIG_SYS_ATA_IDE0_OFFSET)); +#endif + /* Enable ATA port 1 (could be SATA port 0 or 1) if declared */ +#if defined(CONFIG_SYS_ATA_IDE1_OFFSET) + mvsata_ide_initialize_port( + (struct mv_sata_port_registers *) + (CONFIG_SYS_ATA_BASE_ADDR + CONFIG_SYS_ATA_IDE1_OFFSET)); +#endif + /* return 0 as we always succeed */ + return 0; +} -- 1.6.4.4
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Le 07/08/2010 08:39, Prafulla Wadaskar a écrit :
Hi Albert
can you pls fix these warnings too
mvsata_ide.c: In function ‘ide_preinit’: mvsata_ide.c:130: warning: passing argument 1 of ‘mvsata_ide_initialize_port’ from incompatible pointer type mvsata_ide.c:136: warning: passing argument 1 of ‘mvsata_ide_initialize_port’ from incompatible pointer type
Regards.. Prafulla . .
Will fix.
Amicalement,

-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Albert Aribaud Sent: Thursday, August 05, 2010 6:06 PM To: u-boot@lists.denx.de Subject: [U-Boot] [PATCH V7 1/4] ide: add configuration CONFIG_IDE_SWAP_IO
This configuration option replaces a complex conditional in cmd_ide.c with an explicit define to be added to SoC or board configs.
Signed-off-by: Albert Aribaud albert.aribaud@free.fr
arch/powerpc/include/asm/config.h | 3 +++ common/cmd_ide.c | 18 +++++++++--------- doc/README.PXA_CF | 8 ++++++++ include/configs/ap325rxa.h | 1 + include/configs/ms7720se.h | 1 + include/configs/r2dplus.h | 1 + include/configs/r7780mp.h | 1 + 7 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/config.h b/arch/powerpc/include/asm/config.h index f70699d..d098657 100644 --- a/arch/powerpc/include/asm/config.h +++ b/arch/powerpc/include/asm/config.h @@ -95,4 +95,7 @@ #define CONFIG_FSL_LBC #endif
+/* All PPC boards must swap IDE bytes */ +#define CONFIG_IDE_SWAP_IO
#endif /* _ASM_CONFIG_H_ */ diff --git a/common/cmd_ide.c b/common/cmd_ide.c index c0fb88d..d423e53 100644 --- a/common/cmd_ide.c +++ b/common/cmd_ide.c @@ -847,7 +847,7 @@ input_swap_data(int dev, ulong *sect_buf, int words) #endif /* __LITTLE_ENDIAN || CONFIG_AU1X00 */
-#if defined(__PPC__) || defined(CONFIG_PXA_PCMCIA) || defined(CONFIG_SH) +#if defined(CONFIG_IDE_SWAP_IO) static void output_data(int dev, ulong *sect_buf, int words) { @@ -891,15 +891,15 @@ output_data(int dev, ulong *sect_buf, int words) } #endif } -#else /* ! __PPC__ */ +#else /* ! CONFIG_IDE_SWAP_IO */ static void output_data(int dev, ulong *sect_buf, int words) { outsw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, words<<1); } -#endif /* __PPC__ */ +#endif /* CONFIG_IDE_SWAP_IO */
-#if defined(__PPC__) || defined(CONFIG_PXA_PCMCIA) || defined(CONFIG_SH) +#if defined(CONFIG_IDE_SWAP_IO) static void input_data(int dev, ulong *sect_buf, int words) { @@ -949,14 +949,14 @@ input_data(int dev, ulong *sect_buf, int words) } #endif } -#else /* ! __PPC__ */ +#else /* ! CONFIG_IDE_SWAP_IO */ static void input_data(int dev, ulong *sect_buf, int words) { insw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, words << 1); }
-#endif /* __PPC__ */ +#endif /* CONFIG_IDE_SWAP_IO */
/*
*/ @@ -1573,7 +1573,7 @@ int ide_device_present(int dev)
- ATAPI Support
*/
-#if defined(__PPC__) || defined(CONFIG_PXA_PCMCIA) +#if defined(CONFIG_IDE_SWAP_IO) /* since ATAPI may use commands with not 4 bytes alligned length
- we have our own transfer functions, 2 bytes alligned */
static void @@ -1640,7 +1640,7 @@ input_data_shorts(int dev, ushort *sect_buf, int shorts) #endif }
-#else /* ! __PPC__ */ +#else /* ! CONFIG_IDE_SWAP_IO */ static void output_data_shorts(int dev, ushort *sect_buf, int shorts) { @@ -1653,7 +1653,7 @@ input_data_shorts(int dev, ushort *sect_buf, int shorts) insw(ATA_CURR_BASE(dev)+ATA_DATA_REG, sect_buf, shorts); }
-#endif /* __PPC__ */ +#endif /* CONFIG_IDE_SWAP_IO */
/*
- Wait until (Status & mask) == res, or timeout (in ms)
diff --git a/doc/README.PXA_CF b/doc/README.PXA_CF index 6a0f236..1d76b32 100644 --- a/doc/README.PXA_CF +++ b/doc/README.PXA_CF @@ -6,6 +6,14 @@ follow the connections of the standard lubbock. Anyway just the block marked memory configuration should be touched since the other parameters are imposed by the PXA architecture.
+EDIT 2010-07-01: in common/cmd_ide.c, having CONFIG_PXA_PCMCIA defined +would cause looping on inw()/outw() rather than using insw()/outsw(), +thus making sure IDE / ATA bytes are properly swapped. This behaviour +is now controlled by CONFIG_IDE_SWAP_IO, therefore PXA boards with +PCMCIA should #define CONFIG_IDE_SWAP_IO.
+#define CONFIG_IDE_SWAP_IO
#define CONFIG_PXA_PCMCIA 1 #define CONFIG_PXA_IDE 1
diff --git a/include/configs/ap325rxa.h b/include/configs/ap325rxa.h index 70dd47e..80a5797 100644 --- a/include/configs/ap325rxa.h +++ b/include/configs/ap325rxa.h @@ -138,6 +138,7 @@ #define CONFIG_SYS_ATA_DATA_OFFSET 0x200 /* data reg offset */ #define CONFIG_SYS_ATA_REG_OFFSET 0x200 /* reg offset */ #define CONFIG_SYS_ATA_ALT_OFFSET 0x210 /* alternate register offset */ +#define CONFIG_IDE_SWAP_IO
/* if you use all NOR Flash , you change dip-switch. Please see Manual. */ #define CONFIG_SYS_MAX_FLASH_BANKS 1 diff --git a/include/configs/ms7720se.h b/include/configs/ms7720se.h index ba0a3f8..0ea3527 100644 --- a/include/configs/ms7720se.h +++ b/include/configs/ms7720se.h @@ -122,5 +122,6 @@ #define CONFIG_SYS_ATA_DATA_OFFSET 0 /* data reg offset */ #define CONFIG_SYS_ATA_REG_OFFSET 0 /* reg offset */ #define CONFIG_SYS_ATA_ALT_OFFSET 0x200 /* alternate register offset */ +#define CONFIG_IDE_SWAP_IO
#endif /* __MS7720SE_H */ diff --git a/include/configs/r2dplus.h b/include/configs/r2dplus.h index 8931b97..955f3ff 100644 --- a/include/configs/r2dplus.h +++ b/include/configs/r2dplus.h @@ -96,6 +96,7 @@ #define CONFIG_SYS_ATA_DATA_OFFSET 0x1000 /* data reg offset */ #define CONFIG_SYS_ATA_REG_OFFSET 0x1000 /* reg offset */ #define CONFIG_SYS_ATA_ALT_OFFSET 0x800 /* alternate register offset */ +#define CONFIG_IDE_SWAP_IO
/*
- SuperH PCI Bridge Configration
diff --git a/include/configs/r7780mp.h b/include/configs/r7780mp.h index 71c570e..3afe93a 100644 --- a/include/configs/r7780mp.h +++ b/include/configs/r7780mp.h @@ -171,6 +171,7 @@ #define CONFIG_SYS_ATA_DATA_OFFSET 0x1000 /* data reg offset */ #define CONFIG_SYS_ATA_REG_OFFSET 0x1000 /* reg offset */ #define CONFIG_SYS_ATA_ALT_OFFSET 0x800 /* alternate register offset */ +#define CONFIG_IDE_SWAP_IO #endif /* CONFIG_CMD_IDE */
#endif /* __R7780RP_H */
Acked-by: Prafulla Wadaskar prafulla@marvell.com Tested-by: Prafulla Wadaskar prafulla@marvell.com
Hi Wolfgang, This is a standalone patch. I have tested it on Kirkwood platform, but it would be good if someone can test it on PPC boards.
Otherwise there is no harm to pull this patch.
Regards.. Prafulla . .

This may be a stupid comment, but from my perspective implementing Albert's orion5x changes for my DNS323, all I am doing is copying a lot of what Albert is doing for the edminiv2 verbatim.
Would it not make sense perhaps to define defaults in a SoC config file, and then allow them to be overridden as required for each specific board?
That way, if the defaults were ok for both boards (as they are in a lot of places), it simplifies the work required to keep both up to date.
Regards,
Rogan

Dear Rogan Dawes,
In message 4C5AB1A4.3000409@dawes.za.net you wrote:
This may be a stupid comment, but from my perspective implementing Albert's orion5x changes for my DNS323, all I am doing is copying a lot of what Albert is doing for the edminiv2 verbatim.
Would it not make sense perhaps to define defaults in a SoC config file, and then allow them to be overridden as required for each specific board?
That would make sense, but quite often you don't know what will be common code and what not when writing the first version of such code.
I recommend that you discuss with Albert (as part of the review process here) what should be handled as common driver code that you can easily reuse.
It is pretty likely that your copied code would not be accepted for mainline, and you will be requested to factor out common parts when you submit it (that's quite often the fate for the second or third in such a row).
Best regards,
Wolfgang Denk

Le 05/08/2010 15:02, Wolfgang Denk a écrit :
Dear Rogan Dawes,
In message4C5AB1A4.3000409@dawes.za.net you wrote:
This may be a stupid comment, but from my perspective implementing Albert's orion5x changes for my DNS323, all I am doing is copying a lot of what Albert is doing for the edminiv2 verbatim.
Would it not make sense perhaps to define defaults in a SoC config file, and then allow them to be overridden as required for each specific board?
That would make sense, but quite often you don't know what will be common code and what not when writing the first version of such code.
I recommend that you discuss with Albert (as part of the review process here) what should be handled as common driver code that you can easily reuse.
It is pretty likely that your copied code would not be accepted for mainline, and you will be requested to factor out common parts when you submit it (that's quite often the fate for the second or third in such a row).
Best regards,
Wolfgang Denk
Rogan,
As Wolfgang points out, we need to know which code can be common and which cannot. IIRC, you are creating a new board for dns323, so you most probably copied the edminiv2 board code (board/LaCie/edminiv2/) and the edminiv2 config (include/configs/edminiv2).
As for configurations, I think they should be kept separate -- unless Wolfgang wants to do something similar to what is happening at the moment with ARM defconfigs in the Linux kernel, but he showed no sign of it. :)
As for the edminiv2 board code, it only provides three functions, board_flash_get_legacy(), board_init() and reset_phy().
We know that flash_get_legacy() cannot be shared (we both have non-CFI compliant flash chips, but they're different). Granted, we could rewrite it as a common/ function and keep the board-specifics in CONFIG_FLASH_LEGACY_xxxx defines (or a struct), but I don't see a benefit in code size, simplicity or clarity in this.
We could try to commonalize board_init(), which should only differ in machine type, but it is very small, so I don't see a great benefit in factorizing, and if any initialization is to be added later on to one board, we'd have to revert somehow.
What we could share is reset_phy() if your board has an MV88E1116 Ethernet phy; then you could move reset_phy() to a driver in net/ that would be built conditionally to e.g. CONFIG_PHY_MV88E1116, and then you would add the corresponding #define to the edminiv2 and dns323 config files.
The rest of my devs (gbe, ide) is only a matter of setting the right configuration. Granted, that makes our config files more similar, but as I said, just because they are similar is not enough of a reason to factorize these IMO.
Rogan, what other code do you think you'd be duplicating?
Amicalement,

Le 05/08/2010 16:28, Rogan Dawes a écrit :
On 2010/08/05 4:11 PM, Albert ARIBAUD wrote:
Rogan, what other code do you think you'd be duplicating?
Amicalement,
In truth, I was really thinking only of the config files, not really the code per se.
I suppose that there is not really that much else that really is common.
Rogan
As for the configs, some u-boot boards do commonalize, see for instance include/configs/spear*.h. That makes sense because there is a board family, with a common HW design and common external components.
For instance, the recently submitted patch for the kirkwood-based openrd family could probably commonalize since most of the OpenRD Base and OpenRD Client HW design is shared, and I assume Ultimate reuses it a lot too.
In our case, the boards are from different manufacturers, so commonalization seems less obvious to me, except for MV88E1116 if both boards have it, but the config for it is a single define (CONFIG_RESET_PHY_R) which makes putting it in a common config header overkill (but commonalization of reset code would make sense though).
Amicalement,

On 2010/08/05 5:12 PM, Albert ARIBAUD wrote:
Le 05/08/2010 16:28, Rogan Dawes a écrit :
On 2010/08/05 4:11 PM, Albert ARIBAUD wrote:
Rogan, what other code do you think you'd be duplicating?
Amicalement,
In truth, I was really thinking only of the config files, not really the code per se.
I suppose that there is not really that much else that really is common.
Rogan
As for the configs, some u-boot boards do commonalize, see for instance include/configs/spear*.h. That makes sense because there is a board family, with a common HW design and common external components.
For instance, the recently submitted patch for the kirkwood-based openrd family could probably commonalize since most of the OpenRD Base and OpenRD Client HW design is shared, and I assume Ultimate reuses it a lot too.
In our case, the boards are from different manufacturers, so commonalization seems less obvious to me, except for MV88E1116 if both boards have it, but the config for it is a single define (CONFIG_RESET_PHY_R) which makes putting it in a common config header overkill (but commonalization of reset code would make sense though).
Amicalement,
The DNS323 uses the Marvell 88E1111, but it is working with the same config as yours, so I suppose they are compatible.
I was really thinking in terms of all the peripheral configs, the SATA, USB, network, etc, which *should* be the same, since they are all based on the same SoC.
While your edmini only uses a single SATA port, that could easily be overcome by defining addresses for both in the board config, and just #undef'ing it for those that do not use the second port. (It seems more descriptive to have both ports defined in the same place, and then say that the edmini doesn't use the second one).
I might just be being lazy, of course :-)
Rogan P.S. Is there any attention being paid to the ARM defconfig cleanup going on in the kernel, and thoughts of copying their approach?

Dear Albert ARIBAUD,
In message 4C5AD4F7.2030604@free.fr you wrote:
As for the configs, some u-boot boards do commonalize, see for instance include/configs/spear*.h. That makes sense because there is a board family, with a common HW design and common external components.
This is not actually a prerequisite. You can create a common look and feel across completely different boards and architectures.
For example, include/configs/amcc-common.h provides a common look and feel for all boards by one specific vendor.
Best regards,
Wolfgang Denk

-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Wolfgang Denk Sent: Thursday, August 05, 2010 11:55 PM To: Albert ARIBAUD Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH V7 0/4] Add disk support to orion5x and edminiv2
Dear Albert ARIBAUD,
In message 4C5AD4F7.2030604@free.fr you wrote:
As for the configs, some u-boot boards do commonalize, see
for instance
include/configs/spear*.h. That makes sense because there is a board family, with a common HW design and common external components.
This is not actually a prerequisite. You can create a common look and feel across completely different boards and architectures.
For example, include/configs/amcc-common.h provides a common look and feel for all boards by one specific vendor.
That's good idea to generate mv-common.h to abstract Marvell (kirkwood+orion specific )common definitions across the boards
Regards.. Prafulla . .

Le 05/08/2010 20:37, Prafulla Wadaskar a écrit :
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Wolfgang Denk Sent: Thursday, August 05, 2010 11:55 PM To: Albert ARIBAUD Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH V7 0/4] Add disk support to orion5x and edminiv2
Dear Albert ARIBAUD,
In message4C5AD4F7.2030604@free.fr you wrote:
As for the configs, some u-boot boards do commonalize, see
for instance
include/configs/spear*.h. That makes sense because there is a board family, with a common HW design and common external components.
This is not actually a prerequisite. You can create a common look and feel across completely different boards and architectures.
For example, include/configs/amcc-common.h provides a common look and feel for all boards by one specific vendor.
That's good idea to generate mv-common.h to abstract Marvell (kirkwood+orion specific )common definitions across the boards
Why not? The only drawback would be that some board would want to use a config different from what the common config has set (e.g., an Orion board with no MVSATAHC) but even then, the board config file would simply not include the common config, or include it and undefine or redefine some config elements.
Two (independant) comments:
1. If abstracting SoC configuration (which I'm fine with), maybe it would make more sense to abstract by SoC (i.e., kirkwood-common.h, orion5x-common.h...) -- or even by IP (i.e. mvgbe-common.h, mvsata-common.h...) rather than by "commonality".
2. Would it be worthwhile, from a readability/maintenability standpoint, to generalize the idea of SoC- and board-specific configs, and thus have board-specific includes in one directory and SoC-specific includes in another?
Regards.. Prafulla . .
Amicalement,

-----Original Message----- From: Albert ARIBAUD [mailto:albert.aribaud@free.fr] Sent: Friday, August 06, 2010 12:39 PM To: Prafulla Wadaskar Cc: Wolfgang Denk; u-boot@lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik Subject: Re: [PATCH V7 0/4] Add disk support to orion5x and edminiv2
Le 05/08/2010 20:37, Prafulla Wadaskar a écrit :
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Wolfgang Denk Sent: Thursday, August 05, 2010 11:55 PM To: Albert ARIBAUD Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH V7 0/4] Add disk support to orion5x and edminiv2
Dear Albert ARIBAUD,
In message4C5AD4F7.2030604@free.fr you wrote:
As for the configs, some u-boot boards do commonalize, see
for instance
include/configs/spear*.h. That makes sense because there
is a board
family, with a common HW design and common external components.
This is not actually a prerequisite. You can create a
common look and
feel across completely different boards and architectures.
For example, include/configs/amcc-common.h provides a
common look and
feel for all boards by one specific vendor.
That's good idea to generate mv-common.h to abstract
Marvell (kirkwood+orion
specific )common definitions across the boards
Why not? The only drawback would be that some board would want to use a config different from what the common config has set (e.g., an Orion board with no MVSATAHC) but even then, the board config file would simply not include the common config, or include it and undefine or redefine some config elements.
Two (independant) comments:
- If abstracting SoC configuration (which I'm fine with), maybe it
would make more sense to abstract by SoC (i.e., kirkwood-common.h, orion5x-common.h...) -- or even by IP (i.e. mvgbe-common.h, mvsata-common.h...) rather than by "commonality".
- Would it be worthwhile, from a readability/maintenability
standpoint, to generalize the idea of SoC- and board-specific configs, and thus have board-specific includes in one directory and SoC-specific includes in another?
Any System = f(board, soc). We have board configs header file in place. Just having soc-config header file would be enough, We should have <asm/arch/config.h> included within board config file. Any unwanted stuff can be undefed below it.
Regards.. Prafulla . .

Le 06/08/2010 10:18, Prafulla Wadaskar a écrit :
Any System = f(board, soc). We have board configs header file in place. Just having soc-config header file would be enough, We should have<asm/arch/config.h> included within board config file. Any unwanted stuff can be undefed below it.
Regards.. Prafulla . .
I'm fine with that (although to be fair, I feel the 'asm' part in the include path seems a bit misleading, but that's no big deal).
Amicalement,
participants (5)
-
Albert ARIBAUD
-
Albert Aribaud
-
Prafulla Wadaskar
-
Rogan Dawes
-
Wolfgang Denk