Add disk support to orion5x and edminiv2
Subsystems (3)
v7 cover 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
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.
--- 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(-)
arch/powerpc/include/asm/config.h +3 -0
@@ -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_ */
common/cmd_ide.c +9 -9
@@ -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)
doc/README.PXA_CF +8 -0
@@ -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
include/configs/ap325rxa.h +1 -0
@@ -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
include/configs/ms7720se.h +1 -0
@@ -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 */
include/configs/r2dplus.h +1 -0
@@ -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
include/configs/r7780mp.h +1 -0
@@ -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 */
Signature · 1 line
1.6.4.4
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.
--- 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
drivers/block/Makefile +4 -3
@@ -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
drivers/block/mvsata_ide.c new file +140 -0
140 lines not on this page · raw message
Signature · 1 line
1.6.4.4
PATCH v7 3/4 cmd_ide: add support for orion5x
Add MVSATAHC definitions to orion5x.
Add support for orion5x in cmd_ide.
--- arch/arm/include/asm/arch-orion5x/orion5x.h | 3 +++ common/cmd_ide.c | 5 +++++ 2 files changed, 8 insertions(+), 0 deletions(-)
arch/arm/include/asm/arch-orion5x/orion5x.h +3 -0
@@ -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
common/cmd_ide.c +5 -0
@@ -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>
Signature · 1 line
1.6.4.4
PATCH v7 4/4 edminiv2: add mvsata_ide and cmd_ide support
Add mvsata_ide and cmd_ide configuration in edminiv2 config
--- include/configs/edminiv2.h | 34 +++++++++++++++++++++++++++++++++- 1 files changed, 33 insertions(+), 1 deletions(-)
include/configs/edminiv2.h +33 -1
@@ -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
Signature · 1 line
1.6.4.4
PATCH v7 4/4 edminiv2: add mvsata_ide and cmd_ide support
Quoted 67 lines
-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Thursday, August 05, 2010 6:06 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH V7 4/4] edminiv2: addmvsata_ideandcmd_idesupportAdd
mvsata_ideandcmd_ideconfiguration in edminiv2 configSigned-off-by Albert Aribaud <[email protected]>--- include/configs/edminiv2.h | 34 +++++++++++++++++++++++++++++++++- 1 files changed, 33 insertions(+), 1 deletions(-)include/configs/edminiv2.h +26
-1@@ -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 */
include/configs/edminiv2.h line 187
Quoted 10 lines
include/configs/edminiv2.h +7
-0+#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
Regards..
Prafulla . .
PATCH v7 3/4 cmd_ide: add support for orion5x
In reply to Albert AribaudQuoted 32 lines
-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Thursday, August 05, 2010 6:06 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH V7 3/4]cmd_ide: add support for orion5xAdd MVSATAHC definitions to orion5x.
Add support for orion5x incmd_ide.Signed-off-by Albert Aribaud <[email protected]>--- arch/arm/include/asm/arch-orion5x/orion5x.h | 3 +++ common/cmd_ide.c | 5 +++++ 2 files changed, 8 insertions(+), 0 deletions(-)arch/arm/include/asm/arch-orion5x/orion5x.h +1
-0@@ -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))
arch/arm/include/asm/arch-orion5x/orion5x.h line 60
Quoted 2 lines
arch/arm/include/asm/arch-orion5x/orion5x.h +2
-0+#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?
Quoted 16 lines
/* Orion5x GbE controller has a single port */
#defineMAX_MVGBE_DEVS1common/cmd_ide.c +1
-0@@ -25,6 +25,7 @@/* * IDE support */ +#include <common.h> #include <config.h> #include <watchdog.h>
common/cmd_ide.c line 51
Quoted 6 lines
common/cmd_ide.c +3
-0@@ -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 . .
PATCH v7 3/4 cmd_ide: add support for orion5x
Le 05/08/2010 20:43, Prafulla Wadaskar a ?crit :
Quoted 29 lines
Quoted 29 lines
-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Thursday, August 05, 2010 6:06 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH V7 3/4]cmd_ide: add support for orion5xAdd MVSATAHC definitions to orion5x.
Add support for orion5x incmd_ide.Signed-off-by Albert Aribaud<[email protected]>--- arch/arm/include/asm/arch-orion5x/orion5x.h | 3 +++ common/cmd_ide.c | 5 +++++ 2 files changed, 8 insertions(+), 0 deletions(-)arch/arm/include/asm/arch-orion5x/orion5x.h +0
-0@@ -55,6 +55,9 @@#define ORION5X_USB20_PORT0_BASE (ORION5X_REGISTER(0x50000))#define ORION5X_USB20_PORT1_BASE (ORION5X_REGISTER(0xA0000))#define ORION5X_EGIGA_BASE
Quoted 8 lines
arch/arm/include/asm/arch-orion5x/orion5x.h line 60
Quoted 5 lines
arch/arm/include/asm/arch-orion5x/orion5x.h +1
-0(ORION5X_REGISTER(0x72000)) +#define ORION5X_SATA_BASE (
ORION5X_REGISTER(0x80000))arch/arm/include/asm/arch-orion5x/orion5x.h +2
-0+#define ORION5X_SATA_PORT0_OFFSET 0x2000 +#define ORION5X_SATA_PORT1_OFFSET 0x4000These 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.
Quoted 18 lines
Quoted 18 lines
/* Orion5x GbE controller has a single port */
#defineMAX_MVGBE_DEVS1common/cmd_ide.c +1
-0@@ -25,6 +25,7 @@/* * IDE support */ +#include <common.h> #include <config.h> #include <watchdog.h> @@ -45,6 +46,10 @@#include <mpc5xxx.h> #endif
Quoted 5 lines
common/cmd_ide.c line 51
Quoted 3 lines
common/cmd_ide.c +3
-0+#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.
Quoted 2 lines
Regards..
Prafulla . .
Amicalement,
Signature · 1 line
Albert.
PATCH v7 2/4 ide: add mvsata_ide driver
In reply to Albert AribaudQuoted 34 lines
-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Thursday, August 05, 2010 6:06 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH V7 2/4] ide: addmvsata_idedriverThis driver only provides initialization code; actual driving is done by
cmd_ide.cusing the ATA compatibility mode of the Marvell SATAHC controller.Signed-off-by Albert Aribaud <[email protected]>--- drivers/block/Makefile | 7 +- drivers/block/mvsata_ide.c | 140 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 3 deletions(-) create mode 100644 drivers/block/mvsata_ide.cdrivers/block/Makefile +1
-2@@ -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
drivers/block/Makefile include $(TOPDIR)/config.mk · line 39
Quoted 10 lines
drivers/block/Makefile +3
-1COBJS-$(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 . .
PATCH v7 2/4 ide: add mvsata_ide driver
Hi Prafulla,
Le 05/08/2010 21:09, Prafulla Wadaskar a ?crit :
Quoted 50 lines
Quoted 34 lines
-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Thursday, August 05, 2010 6:06 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH V7 2/4] ide: addmvsata_idedriverThis driver only provides initialization code; actual driving is done by
cmd_ide.cusing the ATA compatibility mode of the Marvell SATAHC controller.Signed-off-by Albert Aribaud<[email protected]>--- drivers/block/Makefile | 7 +- drivers/block/mvsata_ide.c | 140 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 3 deletions(-)create mode 100644
drivers/block/mvsata_ide.cdrivers/block/Makefile +1
-2@@ -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.odrivers/block/Makefile include $(TOPDIR)/config.mk · line 39
Quoted 10 lines
drivers/block/Makefile +3
-1COBJS-$(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.oHi Albert
You have merged two changes here,
1. reordering the entireMakefilewhich is good thing, but irrelevant here (could be separate patch).
2. mvsata support updates, which is only relevant to the patch subject here.
Quoted 1 line
Otherwise, ack for rest of the code
The reordering was asked for in a previous review round.
Quoted 2 lines
Regards..
Prafulla . .
Amicalement,
Signature · 1 line
Albert.
PATCH v7 2/4 ide: add mvsata_ide driver
Additional comment:
Le 05/08/2010 23:29, Albert ARIBAUD a ?crit :
Quoted 8 lines
Quoted 6 lines
Hi Albert
You have merged two changes here,
1. reordering the entireMakefilewhich 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
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,
Signature · 1 line
Albert.
PATCH v7 2/4 ide: add mvsata_ide driver
In reply to Albert AribaudHi 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: [email protected] [u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud [albert.aribaud at free.fr]
Sent: Thursday, August 05, 2010 6:05 PM
To: u-boot at 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.
--- 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
drivers/block/Makefile +4 -3
@@ -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
drivers/block/mvsata_ide.c new file +140 -0
140 lines not on this page · raw message
--
1.6.4.4
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH v7 2/4 ide: add mvsata_ide driver
Le 07/08/2010 08:39, Prafulla Wadaskar a ?crit :
Quoted 10 lines
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 typeRegards..
Prafulla . .
Will fix.
Amicalement,
Signature · 1 line
Albert.
PATCH v7 1/4 ide: add configuration CONFIG_IDE_SWAP_IO
In reply to Albert AribaudQuoted 197 lines
-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Thursday, August 05, 2010 6:06 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH V7 1/4] ide: add configurationCONFIG_IDE_SWAP_IOThis configuration option replaces a complex conditional in
cmd_ide.cwith an explicit define to be added to SoC or board configs.Signed-off-by Albert Aribaud <[email protected]>--- 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(-)arch/powerpc/include/asm/config.h +3
-0@@ -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_ */ common/cmd_ide.c +1
-1@@ -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)
{common/cmd_ide.c +3
-3@@ -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){
common/cmd_ide.c +4
-6@@ -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){
common/cmd_ide.c +1
-1@@ -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)
doc/README.PXA_CF +0
-0@@ -6,6 +6,14 @@ follow the connections of the standardlubbock. 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, havingCONFIG_PXA_PCMCIAdefineddoc/README.PXA_CF +7
-0+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 1include/configs/ap325rxa.h +0
-0@@ -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 */
+#defineCONFIG_IDE_SWAP_IO/* if you use all NOR Flash , you change dip-switch. Please
see Manual. */
#defineCONFIG_SYS_MAX_FLASH_BANKS1include/configs/ms7720se.h +0
-0@@ -122,5 +122,6 @@#define CONFIG_SYS_ATA_DATA_OFFSET 0/* data reg offset */
#defineCONFIG_SYS_ATA_REG_OFFSET0 /* reg offset */
#defineCONFIG_SYS_ATA_ALT_OFFSET0x200/*
alternate register offset */include/configs/ms7720se.h +1
-0+#define CONFIG_IDE_SWAP_IO #endif /* __MS7720SE_H */ include/configs/r2dplus.h +0
-0@@ -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 */
include/configs/r2dplus.h +1
-0+#define CONFIG_IDE_SWAP_IO /* * SuperH PCI Bridge Configration include/configs/r7780mp.h +0
-0@@ -171,6 +171,7 @@#define CONFIG_SYS_ATA_DATA_OFFSET 0x1000/* data reg offset */
#defineCONFIG_SYS_ATA_REG_OFFSET0x1000/*
reg offset */
#defineCONFIG_SYS_ATA_ALT_OFFSET0x800/*
alternate register offset */
+#defineCONFIG_IDE_SWAP_IO
#endif /*CONFIG_CMD_IDE*/
Quoted 2 lines
#endif /*
__R7780RP_H*/
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 . .
v7 cover Add disk support to orion5x and edminiv2
In reply to Albert AribaudThis 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
v7 cover Add disk support to orion5x and edminiv2
Dear Rogan Dawes,
Quoted 6 lines · In message <[email protected]> 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
Signature · 4 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de A rolling stone gathers momentum.
v7 cover Add disk support to orion5x and edminiv2
Le 05/08/2010 15:02, Wolfgang Denk a ?crit :
Quoted 17 lines
Dear Rogan Dawes,
Quoted 6 lines · In message<[email protected]> 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.
Quoted 8 lines
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,
Signature · 1 line
Albert.
v7 cover Add disk support to orion5x and edminiv2
Quoted 3 lines · Albert ARIBAUD
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
v7 cover Add disk support to orion5x and edminiv2
Le 05/08/2010 16:28, Rogan Dawes a ?crit :
Quoted 11 lines · Albert ARIBAUD
Quoted 3 lines
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,
Signature · 1 line
Albert.
v7 cover Add disk support to orion5x and edminiv2
Quoted 22 lines · Albert ARIBAUD
Le 05/08/2010 16:28, Rogan Dawes a ?crit :
Quoted 11 lines · Albert ARIBAUD
Quoted 3 lines
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.
Quoted 7 lines
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 :-)
RoganP.S. Is there any attention being paid to the ARM defconfig cleanup going on in the kernel, and thoughts of copying their approach?
v7 cover Add disk support to orion5x and edminiv2
In reply to Albert ARIBAUDDear Albert ARIBAUD,
Quoted 4 lines · In message <[email protected]> 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
Signature · 5 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de One difference between a man and a machine is that a machine is quiet when well oiled.
v7 cover Add disk support to orion5x and edminiv2
Quoted 17 lines
-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Wolfgang Denk
Sent: Thursday, August 05, 2010 11:55 PM
To: Albert ARIBAUD
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH V7 0/4] Add disk support to orion5x and edminiv2Dear Albert ARIBAUD,
Quoted 2 lines · In message <[email protected]> you wrote:
As for the configs, some u-boot boards do commonalize, see
for instance
Quoted 8 lines
Quoted 2 lines
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.hprovides 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 . .
v7 cover Add disk support to orion5x and edminiv2
Le 05/08/2010 20:37, Prafulla Wadaskar a ?crit :
Quoted 20 lines
Quoted 20 lines
-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Wolfgang Denk
Sent: Thursday, August 05, 2010 11:55 PM
To: Albert ARIBAUD
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH V7 0/4] Add disk support to orion5x and edminiv2Dear Albert ARIBAUD,
Quoted 2 lines · In message<[email protected]> you wrote:
As for the configs, some u-boot boards do commonalize, see
for instance
Quoted 2 lines
include/configs/spear*.h. That makes sense because there is a board family, with a common HW design and common external components.
Quoted 8 lines
Quoted 5 lines
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.hprovides a common look and feel for all boards by one specific vendor.That's good idea to generate
mv-common.hto 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?
Quoted 2 lines
Regards..
Prafulla . .
Amicalement,
Signature · 1 line
Albert.
v7 cover Add disk support to orion5x and edminiv2
Quoted 58 lines
-----Original Message-----
From: Albert ARIBAUD [mailto:albert.aribaud at free.fr]
Sent: Friday, August 06, 2010 12:39 PM
To: Prafulla Wadaskar
Cc: Wolfgang Denk; u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik
Subject: Re: [PATCH V7 0/4] Add disk support to orion5x and edminiv2Le 05/08/2010 20:37, Prafulla Wadaskar a ?crit :
Quoted 11 lines
Quoted 11 lines
-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Wolfgang Denk
Sent: Thursday, August 05, 2010 11:55 PM
To: Albert ARIBAUD
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH V7 0/4] Add disk support to orion5x and edminiv2Quoted 7 lines
Quoted 7 lines
Dear Albert ARIBAUD,
Quoted 2 lines · In message<[email protected]> you wrote:
As for the configs, some u-boot boards do commonalize, see
for instance
Quoted 1 line
include/configs/spear*.h. That makes sense because thereis a board
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
family, with a common HW design and common external components.
This is not actually a prerequisite. You can create a
common look and
Quoted 3 lines
Quoted 3 lines
feel across completely different boards and architectures.
For example,
include/configs/amcc-common.hprovides acommon look and
Quoted 3 lines
Quoted 1 line
feel for all boards by one specific vendor.
That's good idea to generate
mv-common.hto abstractMarvell (kirkwood+orion
Quoted 1 line
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".
Quoted 6 lines
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?
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 . .
v7 cover Add disk support to orion5x and edminiv2
Le 06/08/2010 10:18, Prafulla Wadaskar a ?crit :
Quoted 8 lines
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,
Signature · 1 line
Albert.