Skip to contentSkip to conversation

Add disk support to orion5x and edminiv2

PATCH Superseded Albert Aribaud 4 participants 25 messages Aug 5, 2010 – Aug 7, 2010

Subsystems (3)

v7 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

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
#1 lore raw

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
index f70699d..d098657 100644
@@ -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
index c0fb88d..d423e53 100644
@@ -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
index 6a0f236..1d76b32 100644
@@ -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
index 70dd47e..80a5797 100644
@@ -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
index ba0a3f8..0ea3527 100644
@@ -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
index 8931b97..955f3ff 100644
@@ -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
index 71c570e..3afe93a 100644
@@ -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
#2 lore raw

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
index 3f6ad5c..64dcf4e 100644
@@ -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
#3 lore raw

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
index d257b66..e3d3f76 100644
@@ -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
index d423e53..b23db3f 100644
@@ -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
#4 lore raw

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
index 055931c..57dd165 100644
@@ -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
#5 lore raw

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: 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 +26 -1
index 055931c..57dd165 100644
@@ -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 . .

#6 lore raw

PATCH v7 3/4 cmd_ide: add support for orion5x

In reply to Albert Aribaud
Quoted 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 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 +1 -0
index d257b66..e3d3f76 100644
@@ -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 */
#define MAX_MVGBE_DEVS 1

common/cmd_ide.c +1 -0
index d423e53..b23db3f 100644
@@ -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 . .

#7 lore raw

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 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 +0 -0
index d257b66..e3d3f76 100644
@@ -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		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.

Quoted 18 lines
Quoted 18 lines

/* Orion5x GbE controller has a single port */
#define MAX_MVGBE_DEVS 1

common/cmd_ide.c +1 -0
index d423e53..b23db3f 100644
@@ -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.
#8 lore raw

PATCH v7 2/4 ide: add mvsata_ide driver

In reply to Albert Aribaud
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: 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 +1 -2
index 3f6ad5c..64dcf4e 100644
@@ -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 -1
 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 . .

#9 lore raw

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: 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 +1 -2
index 3f6ad5c..64dcf4e 100644
@@ -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 -1
  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.

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.
#10 lore raw

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 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

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.
#11 lore raw

PATCH v7 2/4 ide: add mvsata_ide driver

In reply to Albert Aribaud

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: [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
index 3f6ad5c..64dcf4e 100644
@@ -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

#12 lore raw

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 type

Regards..
Prafulla . .

Will fix.

Amicalement,

Signature · 1 line
Albert.
#13 lore raw

PATCH v7 1/4 ide: add configuration CONFIG_IDE_SWAP_IO

In reply to Albert Aribaud
Quoted 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 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
index f70699d..d098657 100644
@@ -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
index c0fb88d..d423e53 100644
@@ -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
index 6a0f236..1d76b32 100644
@@ -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

doc/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 1
include/configs/ap325rxa.h +0 -0
index 70dd47e..80a5797 100644
@@ -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 +0 -0
index ba0a3f8..0ea3527 100644
@@ -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 */

include/configs/ms7720se.h +1 -0
+#define CONFIG_IDE_SWAP_IO
 
 #endif	/* __MS7720SE_H */
include/configs/r2dplus.h +0 -0
index 8931b97..955f3ff 100644
@@ -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
index 71c570e..3afe93a 100644
@@ -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 */

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 . .

#14 lore raw

v7 cover Add disk support to orion5x and edminiv2

In reply to Albert Aribaud

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

#15 lore raw

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.
#16 lore raw

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.
#17 lore raw

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

#18 lore raw

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.
#19 lore raw

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 :-)

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?

#20 lore raw

v7 cover Add disk support to orion5x and edminiv2

In reply to Albert ARIBAUD

Dear 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.
#21 lore raw

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 edminiv2

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 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.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 . .

#22 lore raw

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 edminiv2

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 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.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?

Quoted 2 lines

Regards..
Prafulla . .

Amicalement,

Signature · 1 line
Albert.
#23 lore raw

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 edminiv2

Le 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 edminiv2

Quoted 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 there

is 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.h provides a

common look and

Quoted 3 lines
Quoted 1 line

feel for all boards by one specific vendor.

That's good idea to generate mv-common.h to abstract

Marvell (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 . .

#24 lore raw

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.