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

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

[Albert Aribaud]

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

[U-Boot] [PATCH V7 1/4] ide: add configuration CONFIG_IDE_SWAP_IO

[Albert Aribaud]

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 */
-- 
1.6.4.4

[U-Boot] [PATCH V7 2/4] ide: add mvsata_ide driver

[Albert Aribaud]

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] [PATCH V7 3/4] cmd_ide: add support for orion5x

[Albert Aribaud]

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

[U-Boot] [PATCH V7 4/4] edminiv2: add mvsata_ide and cmd_ide support

[Albert Aribaud]

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

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

[Rogan Dawes]

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

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

[Wolfgang Denk]

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

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

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

[Albert ARIBAUD]

Le 05/08/2010 15:02, Wolfgang Denk a ?crit :
> 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

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

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

[Rogan Dawes]

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

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

[Albert ARIBAUD]

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

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

[Rogan Dawes]

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?

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

[Wolfgang Denk]

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

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

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

[Prafulla Wadaskar]

 

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

[U-Boot] [PATCH V7 3/4] cmd_ide: add support for orion5x

[Prafulla Wadaskar]

 

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

[U-Boot] [PATCH V7 4/4] edminiv2: add mvsata_ide and cmd_ide support

[Prafulla Wadaskar]

 

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

[U-Boot] [PATCH V7 1/4] ide: add configuration CONFIG_IDE_SWAP_IO

[Prafulla Wadaskar]

 

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

[U-Boot] [PATCH V7 2/4] ide: add mvsata_ide driver

[Prafulla Wadaskar]

 

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

[U-Boot] [PATCH V7 3/4] cmd_ide: add support for orion5x

[Albert ARIBAUD]

Le 05/08/2010 20:43, Prafulla Wadaskar a ?crit :
>
>
>> -----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.
>>
>> 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?

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

[U-Boot] [PATCH V7 2/4] ide: add mvsata_ide driver

[Albert ARIBAUD]

Hi Prafulla,

Le 05/08/2010 21:09, Prafulla Wadaskar a ?crit :
>
>
>> -----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.
>>
>> 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

The reordering was asked for in a previous review round.

> Regards..
> Prafulla . .

Amicalement,
-- 
Albert.

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

[Albert ARIBAUD]

Le 05/08/2010 20:37, Prafulla Wadaskar a ?crit :
>
>
>> -----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,
>>
>> 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

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

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

[Prafulla Wadaskar]

 

> -----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 :
> >
> >
> >> -----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,
> >>
> >> 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
> 
> 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?

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

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

[Albert ARIBAUD]

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

[U-Boot] [PATCH V7 2/4] ide: add mvsata_ide driver

[Albert ARIBAUD]

Additional comment:

Le 05/08/2010 23:29, Albert ARIBAUD a ?crit :

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

[U-Boot] [PATCH V7 2/4] ide: add mvsata_ide driver

[Prafulla Wadaskar]

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

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 at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH V7 2/4] ide: add mvsata_ide driver

[Albert ARIBAUD]

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