[U-Boot] [PATCH] debug_print macros support

[U-Boot] [PATCH] debug_print macros support

[Prafulla Wadaskar]

From: prafulla_wadaskar <prafulla@marvell.com>

This debug_prints strategy provides-
A. pre-formatted debug and print macros to be used in a code
B. flexiblility to enable selective debug prints without
   modifying a source code
For more details refer doc/README.debug_prints

Signed-off-by: prafulla_wadaskar <prafulla@marvell.com>
---
 config.mk               |    6 +++
 doc/README.debug_prints |   89 +++++++++++++++++++++++++++++++++++++++++++++++
 include/debug_prints.h  |   83 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+), 0 deletions(-)
 create mode 100644 doc/README.debug_prints
 create mode 100644 include/debug_prints.h

diff --git a/config.mk b/config.mk
index b1254e9..6e85cb5 100644
--- a/config.mk
+++ b/config.mk
@@ -198,6 +198,12 @@ ifeq ($(PCI_CLOCK),PCI_66M)
 CFLAGS := $(CFLAGS) -DPCI_66M
 endif
 
+CFLAGS := $(CFLAGS) $(shell for dbgprn in $(DEBUG_PRINTS); do \
+			if [ "$$dbgprn" != "" ]; then \
+				echo "-D$$dbgprn "; \
+			fi ;	\
+		done)
+
 #########################################################################
 
 export	HPATH HOSTCC HOSTCFLAGS CROSS_COMPILE \
diff --git a/doc/README.debug_prints b/doc/README.debug_prints
new file mode 100644
index 0000000..f03cb4c
--- /dev/null
+++ b/doc/README.debug_prints
@@ -0,0 +1,89 @@
+#
+# (C) Copyright 2009
+# Marvell Semiconductor <www.marvell.com>
+# Prafulla Wadaskar <prafulla@marvell.com>
+#
+# 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
+#
+
+1.1 Debugging Strategy
+======================
+This debug strategy provides predefined macros to be used in code
+
+DEBUGGING LEVELS
+0 To disable all debug messages
+1 To enable normal debug macro- debug_print
+2 To enable flow trace debug macro- debug_print_ftrace
+4 To enable interrupt and timer debug macroc- debug_print4
+8 To enable any special debug messages defined by macro- debug_print8
+
+1.2 How to use Debugging strategy in a code ?
+=============================================
+
+1. make sure #include <debug_prints.h> line present in c code
+2. define following lines at the top in your code
+	#ifndef <DBG_FLAG_NAME>
+	#define <DBG_FLAG_NAME>	0
+	#endif
+	#define DEBUG_PRINT	<DBG_FLAG_NAME>
+   DBG_FLAG_NAME is an identification that is used during build to enable
+   debug prints
+3. define DEBUG_PFX to a small string to identify debug message in a code
+	This is an optional setting, if you don't define DEBUG_PFX,
+	by default "dbg: " will be used.
+4. use the debug_print, debug_ftrace, debug_print4, debug_print8 macros
+   in a code wjere ever required
+
+1.3 How to activate debug messages?
+====================================
+
+Debug messages can be activated during build time by passing desired
+debug level
+
+1. Enabling Debug messages by passing additional parameter to make
+	This is a recommended method of debug messages implimentation.
+	this method give flexibility to enable/disable debug messages
+	during build without modifying code
+	Additional command line parameters:-
+	(a) To enable debug_print messages:-
+		DEBUG_PRINTS=<DBG_FLAG_NAME>
+	(b) To enable debug_print_ftrace function:-
+		DEBUG_PRINTS=<DBG_FLAG_NAME>=2
+	(c) To enable debug_print4 messages:-
+		DEBUG_PRINTS=<DBG_FLAG_NAME>=4
+	(d) To enable debug_print8 messages:-
+		DEBUG_PRINTS=<DBG_FLAG_NAME>=8
+	(e) you can enable selective debug prints.
+	    for ex. if you want to enable debug_print and debug_print4 messages
+	    then you can pass ORed debug level value (i.e. 1 for debug_print and
+	    4 for debug_print4 (1 | 4 = 5)
+		DEBUG_PRINTS=<DBG_FLAG_NAME>=5
+	(f) if you want to enable debug_print defined in more than one c files
+	then you can pass additional debug flag names as
+		DEBUG_PRINTS="<DBG_FLAG_NAME1>=1 <DBG_FLAG_NAME2>=2
+	the above parameters will enable debug_print messages in a code where
+	<DBG_FLAG_NAME1> is defined and will enable debug_print_ftrace functions
+	in a code where <DBG_FLAG_NAME2> is defined
+
+2. Enabling Debug messages by hardcoding in source file
+	This is simplest implimentation, just define DEBUG_PRINT to
+	desired debug level and compile the code, the disadvantage of this
+	method is, it does not offer flexibility and code with debug message
+	may become part of your release if not taken care properly.
+
diff --git a/include/debug_prints.h b/include/debug_prints.h
new file mode 100644
index 0000000..8a374ff
--- /dev/null
+++ b/include/debug_prints.h
@@ -0,0 +1,83 @@
+/*
+ * (C) Copyright 2009
+ * Marvell Semiconductor <www.marvell.com>
+ * Prafulla Wadaskar <prafulla@marvell.com>
+ *
+ * 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
+ */
+
+/*
+ * refer doc/README.debug_prints for more details
+ */
+
+#ifndef __DEBUG_PRINTS_H__
+#define __DEBUG_PRINTS_H__
+
+#ifndef KERN_ERR
+#define KERN_ERR	"Err :"
+#endif
+
+#ifndef KERN_INFO
+#define KERN_INFO	"Info:"
+#endif
+
+#ifndef KERN_WARNING
+#define KERN_WARNING	"Warn:"
+#endif
+
+#ifndef KERN_DBG
+#define KERN_DBG	"dbg :"
+#endif
+
+#ifndef DEBUG_PRINT_PFX
+#define DEBUG_PRINT_PFX	""
+#endif
+
+#ifndef DEBUG_PRINT
+#define DEBUG_PRINT 	0
+#endif
+
+#define error_print(format, arg...)	\
+		printf(KERN_ERR DEBUG_PRINT_PFX format "\n" , ## arg)
+#define info_print(format, arg...) 	\
+		printf(KERN_INFO DEBUG_PRINT_PFX format "\n" , ## arg)
+#define warn_print(format, arg...) 	\
+		printf(KERN_WARNING DEBUG_PRINT_PFX format "\n" , ## arg)
+
+#define debug_print(format, arg...)		\
+		(DEBUG_PRINT & 1) ? 	\
+		 (printf(KERN_DBG DEBUG_PRINT_PFX format "\n" , ## arg))\
+		 : ({do {} while (0);})
+
+#define debug_print_ftrace(format, arg...)	\
+		(DEBUG_PRINT & 2) ?	\
+		 (printf(KERN_DBG DEBUG_PRINT_PFX "%s() called\n",	\
+		 ( __FUNCTION__ ))) : ({do {} while (0);})
+
+#define debug_print4(format, arg...)		\
+		(DEBUG_PRINT & 4) ?	\
+		 (printf(KERN_DBG DEBUG_PRINT_PFX format "\n" , ## arg))\
+		 : ({do {} while (0);})
+
+#define debug_print8(format, arg...)		\
+		(DEBUG_PRINT & 8) ?		\
+		 (printf(KERN_DBG DEBUG_PRINT_PFX format "\n" , ## arg))\
+		 : ({do {} while (0);})
+
+#endif /* __DEBUG_PRINTS_H__ */
-- 
1.5.3.3

[U-Boot] [PATCH] bin_dep.sh Support

[Prafulla Wadaskar]

From: prafulla_wadaskar <prafulla@marvell.com>

In some cases the u-boot.bin need to be processed further
to create bootable u-boot binary from boot device
This processing may be cpu,soc and/or board spcific
bin_dep.sh provides a mechanism to execute bin_dep.sh
if present in above platform specific folders

Signed-off-by: prafulla_wadaskar <prafulla@marvell.com>
---
 Makefile         |    2 +
 tools/bin_dep.sh |   79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 0 deletions(-)
 create mode 100755 tools/bin_dep.sh

diff --git a/Makefile b/Makefile
index f857641..8bf36ce 100644
--- a/Makefile
+++ b/Makefile
@@ -318,6 +318,7 @@ $(obj)u-boot.srec:	$(obj)u-boot
 
 $(obj)u-boot.bin:	$(obj)u-boot
 		$(OBJCOPY) ${OBJCFLAGS} -O binary $< $@
+		@./tools/bin_dep.sh $(CPU) $(BOARD) $(VENDOR) $(SOC) $@
 
 $(obj)u-boot.ldr:	$(obj)u-boot
 		$(LDR) -T $(CONFIG_BFIN_CPU) -c $@ $< $(LDR_FLAGS)
@@ -3506,6 +3507,7 @@ clean:
 	       $(obj)tools/gen_eth_addr    $(obj)tools/img2srec		  \
 	       $(obj)tools/mkimage	   $(obj)tools/mpc86x_clk	  \
 	       $(obj)tools/ncb		   $(obj)tools/ubsha1
+	@./tools/bin_dep.sh $(CPU) $(BOARD) $(VENDOR) $(SOC) $@
 	@rm -f $(obj)board/cray/L1/{bootscript.c,bootscript.image}	  \
 	       $(obj)board/netstar/{eeprom,crcek,crcit,*.srec,*.bin}	  \
 	       $(obj)board/trab/trab_fkt   $(obj)board/voiceblue/eeprom   \
diff --git a/tools/bin_dep.sh b/tools/bin_dep.sh
new file mode 100755
index 0000000..b32e2ab
--- /dev/null
+++ b/tools/bin_dep.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+
+# (C) Copyright 2009
+# Marvell Semiconductor <www.marvell.com>
+# Prafulla Wadaskar <prafulla@marvell.com>
+#
+# 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
+#
+
+# Some platform may require additional post processing of u-boot.bin
+# to create usefull binaries
+# This script will be executed after each make u-boot.bin and make clean operation
+
+CUR_DIR=`pwd`
+CPU_DIR=$CUR_DIR/cpu/$1
+SOC_DIR=$CPU_DIR/$4
+BOARD_DIR=$CUR_DIR/board/$2
+VENDOR_DIR=$CUR_DIR/board/$3
+VENDOR_BOARD_DIR=$CUR_DIR/board/$3/$2
+DEFAULT_ARG=$5
+#default argument will be <binary name> to be processed in case of build
+#default argument will be "clean" in case of mrproper/distclean operation
+BIN_ARGS=$DEFAULT_ARG
+
+#echo CPU_DIR $CPU_DIR
+#echo SOC_DIR $SOC_DIR
+#echo BOARD_DIR $BOARD_DIR
+#echo VENDOR_DIR $VENDOR_DIR
+#echo VENDOR_BOARD_DIR $VENDOR_BOARD_DIR
+#echo BIN_ARGS $BIN_ARGS
+
+if [ -d $CPU_DIR ]; then
+	if [ -f $CPU_DIR/bin_dep.sh ]; then
+		$CPU_DIR/bin_dep.sh $BIN_ARGS $CPU_DIR
+	else
+		if [ -d $SOC_DIR ]; then
+			if [ -f $SOC_DIR/bin_dep.sh ]; then
+				$SOC_DIR/bin_dep.sh $BIN_ARGS $SOC_DIR
+			fi
+		fi
+	fi
+fi
+
+
+if [ -d $BOARD_DIR ]; then
+	if [ -f $BOARD_DIR/bin_dep.sh ]; then
+		$BOARD_DIR/bin_dep.sh $BIN_ARGS $BOARD_DIR
+	fi
+else
+	if [ -d $VENDOR_DIR ]; then
+		if [ -f $VENDOR_DIR/bin_dep.sh ]; then
+			$VENDOR_DIR/bin_dep.sh $BIN_ARGS $VENDOR_DIR
+		else
+			if [ -d $VENDOR_BOARD_DIR ]; then
+				if [ -f $VENDOR_BOARD_DIR/bin_dep.sh ]; then
+					$VENDOR_BOARD_DIR/bin_dep.sh $BIN_ARGS $VENDOR_BOARD_DIR
+				fi
+			fi
+		fi
+	fi
+fi
+
+
-- 
1.5.3.3

[U-Boot] [PATCH] Macronix MX25xx MTD SPI flash driver

[Prafulla Wadaskar]

From: prafulla_wadaskar <prafulla@marvell.com>

Added macronix SF driver for MTD framework
MX25L12805D is supported and tested
TBD: sector erase implementation, other deivces support

Signed-off-by: prafulla_wadaskar <prafulla@marvell.com>
Tested by: prafulla_wadaskar <prafulla@marvell.com>
---
 drivers/mtd/spi/Makefile    |    1 +
 drivers/mtd/spi/macronix.c  |  319 +++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi/spi_flash.c |    6 +-
 3 files changed, 325 insertions(+), 1 deletions(-)
 create mode 100644 drivers/mtd/spi/macronix.c

diff --git a/drivers/mtd/spi/Makefile b/drivers/mtd/spi/Makefile
index 3d4f892..e527eff 100644
--- a/drivers/mtd/spi/Makefile
+++ b/drivers/mtd/spi/Makefile
@@ -28,6 +28,7 @@ LIB	:= $(obj)libspi_flash.a
 COBJS-$(CONFIG_SPI_FLASH)	+= spi_flash.o
 COBJS-$(CONFIG_SPI_FLASH_ATMEL)	+= atmel.o
 COBJS-$(CONFIG_SPI_FLASH_STMICRO)	+= stmicro.o
+COBJS-$(CONFIG_SPI_FLASH_MACRONIX)	+= macronix.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
diff --git a/drivers/mtd/spi/macronix.c b/drivers/mtd/spi/macronix.c
new file mode 100644
index 0000000..f9bce15
--- /dev/null
+++ b/drivers/mtd/spi/macronix.c
@@ -0,0 +1,319 @@
+/*
+ * Copyright 2009(C) Marvell International Ltd. and its affiliates
+ * Prafulla Wadaskar <prafulla@marvell.com>
+ *
+ * Based on drivers/mtd/spi/stmicor.c
+ *
+ * Copyright 2008, Network Appliance Inc.
+ * Jason McMullan <mcmullan@netapp.com>
+ *
+ * Copyright (C) 2004-2007 Freescale Semiconductor, Inc.
+ * TsiChung Liew (Tsi-Chung.Liew at freescale.com)
+ *
+ * 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 <malloc.h>
+#include <spi_flash.h>
+
+#include "spi_flash_internal.h"
+
+/* MX25Pxx-specific commands */
+#define CMD_MX25PXX_WREN	0x06	/* Write Enable */
+#define CMD_MX25PXX_WRDI	0x04	/* Write Disable */
+#define CMD_MX25PXX_RDSR	0x05	/* Read Status Register */
+#define CMD_MX25PXX_WRSR	0x01	/* Write Status Register */
+#define CMD_MX25PXX_READ	0x03	/* Read Data Bytes */
+#define CMD_MX25PXX_FAST_READ	0x0b	/* Read Data Bytes at Higher Speed */
+#define CMD_MX25PXX_PP		0x02	/* Page Program */
+#define CMD_MX25PXX_SE		0x20	/* Sector Erase */
+#define CMD_MX25PXX_BE		0xD8	/* Block Erase */
+#define CMD_MX25PXX_CE		0xc7	/* Chip Erase */
+#define CMD_MX25PXX_DP		0xb9	/* Deep Power-down */
+#define CMD_MX25PXX_RES		0xab	/* Release from DP, and Read Signature */
+
+#define MXIC_ID_MX25P16		0x15
+#define MXIC_ID_MX25P20		0x12
+#define MXIC_ID_MX25P32		0x16
+#define MXIC_ID_MX25P40		0x13
+#define MXIC_ID_MX25P64		0x17
+#define MXIC_ID_MX25P80		0x14
+#define MXIC_ID_MX25P128	0x18
+
+#define MACRONIX_SR_WIP		(1 << 0)	/* Write-in-Progress */
+
+struct macronix_spi_flash_params {
+	u8 idcode1;
+	u16 page_size;
+	u16 pages_per_sector;
+	u16 sectors_per_block;
+	u16 nr_blocks;
+	const char *name;
+};
+
+struct macronix_spi_flash {
+	const struct macronix_spi_flash_params *params;
+	struct spi_flash flash;
+};
+
+static inline struct macronix_spi_flash *to_macronix_spi_flash(struct spi_flash
+							     *flash)
+{
+	return container_of(flash, struct macronix_spi_flash, flash);
+}
+
+static const struct macronix_spi_flash_params macronix_spi_flash_table[] = {
+	{
+		.idcode1 = MXIC_ID_MX25P128,
+		.page_size = 256,
+		.pages_per_sector = 16,
+		.sectors_per_block = 16,
+		.nr_blocks = 256,
+		.name = "MX25L12805D",
+	},
+};
+
+static int macronix_wait_ready(struct spi_flash *flash, unsigned long timeout)
+{
+	struct spi_slave *spi = flash->spi;
+	unsigned long timebase;
+	int ret;
+	u8 status;
+	u8 cmd[4] = { CMD_MX25PXX_RDSR, 0xff, 0xff, 0xff };
+
+	ret = spi_xfer(spi, 32, &cmd[0], NULL, SPI_XFER_BEGIN);
+	if (ret) {
+		debug("SF: Failed to send command %02x: %d\n", cmd[0], ret);
+		return ret;
+	}
+
+	timebase = get_timer(0);
+	do {
+		ret = spi_xfer(spi, 8, NULL, &status, 0);
+		if (ret)
+			return -1;
+
+		if ((status & MACRONIX_SR_WIP) == 0)
+			break;
+
+	} while (get_timer(timebase) < timeout);
+
+	spi_xfer(spi, 0, NULL, NULL, SPI_XFER_END);
+
+	if ((status & MACRONIX_SR_WIP) == 0)
+		return 0;
+
+	/* Timed out */
+	return -1;
+}
+
+static int macronix_read_fast(struct spi_flash *flash,
+			     u32 offset, size_t len, void *buf)
+{
+	struct macronix_spi_flash *stm = to_macronix_spi_flash(flash);
+	unsigned long page_addr;
+	unsigned long page_size;
+	u8 cmd[5];
+
+	page_size = stm->params->page_size;
+	page_addr = offset / page_size;
+
+	cmd[0] = CMD_READ_ARRAY_FAST;
+	cmd[1] = page_addr >> 8;
+	cmd[2] = page_addr;
+	cmd[3] = offset % page_size;
+	cmd[4] = 0x00;
+
+	return spi_flash_read_common(flash, cmd, sizeof(cmd), buf, len);
+}
+
+static int macronix_write(struct spi_flash *flash,
+			 u32 offset, size_t len, const void *buf)
+{
+	struct macronix_spi_flash *stm = to_macronix_spi_flash(flash);
+	unsigned long page_addr;
+	unsigned long byte_addr;
+	unsigned long page_size;
+	size_t chunk_len;
+	size_t actual;
+	int ret;
+	u8 cmd[4];
+
+	page_size = stm->params->page_size;
+	page_addr = offset / page_size;
+	byte_addr = offset % page_size;
+
+	ret = spi_claim_bus(flash->spi);
+	if (ret) {
+		debug("SF: Unable to claim SPI bus\n");
+		return ret;
+	}
+
+	ret = 0;
+	for (actual = 0; actual < len; actual += chunk_len) {
+		chunk_len = min(len - actual, page_size - byte_addr);
+
+		cmd[0] = CMD_MX25PXX_PP;
+		cmd[1] = page_addr >> 8;
+		cmd[2] = page_addr;
+		cmd[3] = byte_addr;
+
+		debug
+		    ("PP: 0x%p => cmd = { 0x%02x 0x%02x%02x%02x } chunk_len = %d\n",
+		     buf + actual, cmd[0], cmd[1], cmd[2], cmd[3], chunk_len);
+
+		ret = spi_flash_cmd(flash->spi, CMD_MX25PXX_WREN, NULL, 0);
+		if (ret < 0) {
+			debug("SF: Enabling Write failed\n");
+			break;
+		}
+
+		ret = spi_flash_cmd_write(flash->spi, cmd, 4,
+					  buf + actual, chunk_len);
+		if (ret < 0) {
+			debug("SF: STMicro Page Program failed\n");
+			break;
+		}
+
+		ret = macronix_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT);
+		if (ret < 0) {
+			debug("SF: STMicro page programming timed out\n");
+			break;
+		}
+
+		page_addr++;
+		byte_addr = 0;
+	}
+
+	debug("SF: STMicro: Successfully programmed %u bytes @ 0x%x\n",
+	      len, offset);
+
+	spi_release_bus(flash->spi);
+	return ret;
+}
+
+int macronix_erase(struct spi_flash *flash, u32 offset, size_t len)
+{
+	struct macronix_spi_flash *stm = to_macronix_spi_flash(flash);
+	unsigned long sector_size;
+	size_t actual;
+	int ret;
+	u8 cmd[4];
+
+	/*
+	 * This function currently uses sector erase only.
+	 * probably speed things up by using bulk erase
+	 * when possible.
+	 */
+
+	sector_size = stm->params->page_size * stm->params->pages_per_sector
+			* stm->params->sectors_per_block;
+
+	if (offset % sector_size || len % sector_size) {
+		debug("SF: Erase offset/length not multiple of sector size\n");
+		return -1;
+	}
+
+	len /= sector_size;
+	cmd[0] = CMD_MX25PXX_BE;
+	cmd[2] = 0x00;
+	cmd[3] = 0x00;
+
+	ret = spi_claim_bus(flash->spi);
+	if (ret) {
+		debug("SF: Unable to claim SPI bus\n");
+		return ret;
+	}
+
+	ret = 0;
+	for (actual = 0; actual < len; actual++) {
+		cmd[1] = (offset / sector_size) + actual;
+
+		ret = spi_flash_cmd(flash->spi, CMD_MX25PXX_WREN, NULL, 0);
+		if (ret < 0) {
+			debug("SF: Enabling Write failed\n");
+			break;
+		}
+
+		ret = spi_flash_cmd_write(flash->spi, cmd, 4, NULL, 0);
+		if (ret < 0) {
+			debug("SF: STMicro page erase failed\n");
+			break;
+		}
+
+		/* Up to 2 seconds */
+		ret = macronix_wait_ready(flash, 2 * CONFIG_SYS_HZ);
+		if (ret < 0) {
+			debug("SF: STMicro page erase timed out\n");
+			break;
+		}
+	}
+
+	debug("SF: STMicro: Successfully erased %u bytes @ 0x%x\n",
+	      len * sector_size, offset);
+
+	spi_release_bus(flash->spi);
+	return ret;
+}
+
+struct spi_flash *spi_flash_probe_macronix(struct spi_slave *spi, u8 * idcode)
+{
+	const struct macronix_spi_flash_params *params;
+	struct macronix_spi_flash *stm;
+	unsigned int i;
+	int ret;
+	u8 id[3];
+
+	ret = spi_flash_cmd(spi, CMD_READ_ID, id, sizeof(id));
+	if (ret)
+		return NULL;
+
+	for (i = 0; i < ARRAY_SIZE(macronix_spi_flash_table); i++) {
+		params = &macronix_spi_flash_table[i];
+		if (params->idcode1 == idcode[2]) {
+			break;
+		}
+	}
+
+	if (i == ARRAY_SIZE(macronix_spi_flash_table)) {
+		debug("SF: Unsupported STMicro ID %02x\n", id[1]);
+		return NULL;
+	}
+
+	stm = malloc(sizeof(struct macronix_spi_flash));
+	if (!stm) {
+		debug("SF: Failed to allocate memory\n");
+		return NULL;
+	}
+
+	stm->params = params;
+	stm->flash.spi = spi;
+	stm->flash.name = params->name;
+
+	stm->flash.write = macronix_write;
+	stm->flash.erase = macronix_erase;
+	stm->flash.read = macronix_read_fast;
+	stm->flash.size = params->page_size * params->pages_per_sector
+	    * params->sectors_per_block * params->nr_blocks;
+
+	printf("SF: Detected %s with page size %u, total %u bytes\n",
+	      params->name, params->page_size, stm->flash.size);
+
+	return &stm->flash;
+}
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index d1d81af..bafdfe7 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -3,7 +3,6 @@
  *
  * Copyright (C) 2008 Atmel Corporation
  */
-#define DEBUG
 #include <common.h>
 #include <malloc.h>
 #include <spi.h>
@@ -134,6 +133,11 @@ struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs,
 		flash = spi_flash_probe_atmel(spi, idcode);
 		break;
 #endif
+#ifdef CONFIG_SPI_FLASH_MACRONIX
+	case 0xc2:
+		flash = spi_flash_probe_macronix(spi, idcode);
+		break;
+#endif
 #ifdef CONFIG_SPI_FLASH_STMICRO
 	case 0x20:
 		flash = spi_flash_probe_stmicro(spi, idcode);
-- 
1.5.3.3

[U-Boot] [PATCH] Macronix MX25xx MTD SPI flash driver

[Mike Frysinger]

On Friday 03 April 2009 07:49:19 Prafulla Wadaskar wrote:
> --- a/drivers/mtd/spi/Makefile
> +++ b/drivers/mtd/spi/Makefile
> @@ -28,6 +28,7 @@ LIB	:= $(obj)libspi_flash.a
>  COBJS-$(CONFIG_SPI_FLASH)	+= spi_flash.o
>  COBJS-$(CONFIG_SPI_FLASH_ATMEL)	+= atmel.o
>  COBJS-$(CONFIG_SPI_FLASH_STMICRO)	+= stmicro.o
> +COBJS-$(CONFIG_SPI_FLASH_MACRONIX)	+= macronix.o

please keep the list sorted

> --- /dev/null
> +++ b/drivers/mtd/spi/macronix.c
> @@ -0,0 +1,319 @@
> + * Based on drivers/mtd/spi/stmicor.c

typo in file name

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

please add a new line there

> +/* MX25Pxx-specific commands */
> +#define CMD_MX25PXX_WREN	0x06	/* Write Enable */
> +#define CMD_MX25PXX_WRDI	0x04	/* Write Disable */
> +#define CMD_MX25PXX_RDSR	0x05	/* Read Status Register */
> +#define CMD_MX25PXX_WRSR	0x01	/* Write Status Register */
> +#define CMD_MX25PXX_READ	0x03	/* Read Data Bytes */
> +#define CMD_MX25PXX_FAST_READ	0x0b	/* Read Data Bytes at Higher Speed */
> +#define CMD_MX25PXX_PP		0x02	/* Page Program */
> +#define CMD_MX25PXX_SE		0x20	/* Sector Erase */
> +#define CMD_MX25PXX_BE		0xD8	/* Block Erase */
> +#define CMD_MX25PXX_CE		0xc7	/* Chip Erase */
> +#define CMD_MX25PXX_DP		0xb9	/* Deep Power-down */
> +#define CMD_MX25PXX_RES		0xab	/* Release from DP, and Read Signature */

is it really MX25PXX ?  or should it be MX25XX ?  the P looks like a hold over 
from the stmicro driver.

> +struct macronix_spi_flash {
> +	const struct macronix_spi_flash_params *params;
> +	struct spi_flash flash;
> +};

the spi_flash struct needs to be the first member in order to prevent crashes 
when working with the spi flash layer

> +	u8 cmd[4] = { CMD_MX25PXX_RDSR, 0xff, 0xff, 0xff };
> +
> +	ret = spi_xfer(spi, 32, &cmd[0], NULL, SPI_XFER_BEGIN);

do you actually need to read/write 4 bytes here ?  shouldnt it be:
u8 cmd = CMD_MX25PXX_RDSR;
ret = spi_xfer(spi, 8, &cmd, NULL, SPI_XFER_BEGIN);

> +	if (ret) {
> +		debug("SF: Failed to send command %02x: %d\n", cmd[0], ret);

then you'll need to change to "cmd"

> +			debug("SF: STMicro Page Program failed\n");
> +	debug("SF: STMicro: Successfully programmed %u bytes @ 0x%x\n",
> +	      len, offset);
> +			debug("SF: STMicro page erase failed\n");
> +			debug("SF: STMicro page erase timed out\n");
> +	debug("SF: STMicro: Successfully erased %u bytes @ 0x%x\n",
> +	      len * sector_size, offset);
> +		debug("SF: Unsupported STMicro ID %02x\n", id[1]);

this isnt STMicro anymore ...

> +		/* Up to 2 seconds */
> +		ret = macronix_wait_ready(flash, 2 * CONFIG_SYS_HZ);

there's a common flash erase timeout define

> +*spi_flash_probe_macronix(struct spi_slave *spi, u8 * idcode)

no space between "*" and "idcode"

> +	u8 id[3];
> +
> +	ret = spi_flash_cmd(spi, CMD_READ_ID, id, sizeof(id));
> +	if (ret)
> +		return NULL;

no need to read id here ... use the idcode passed in to you

> +		if (params->idcode1 == idcode[2]) {
> +			break;
> +		}

drop the {} braces

> --- a/drivers/mtd/spi/spi_flash.c
> +++ b/drivers/mtd/spi/spi_flash.c
> @@ -3,7 +3,6 @@
>   *
>   * Copyright (C) 2008 Atmel Corporation
>   */
> -#define DEBUG
>  #include <common.h>
>  #include <malloc.h>
>  #include <spi.h>

please drop this hunk ... it doesnt belong in this patch (and it's already 
been merged elsewhere)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090403/0c24ba2e/attachment.pgp 

[U-Boot] [PATCH] debug_print macros support

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <1238759359-6544-1-git-send-email-prafulla@marvell.com> you wrote:
> From: prafulla_wadaskar <prafulla@marvell.com>
> 
> This debug_prints strategy provides-
> A. pre-formatted debug and print macros to be used in a code
> B. flexiblility to enable selective debug prints without
>    modifying a source code
> For more details refer doc/README.debug_prints
> 
> Signed-off-by: prafulla_wadaskar <prafulla@marvell.com>

This seems to be a lot of effort, from code to command line pollution
- for what exactly?

Which is the problem you are trying to solve?

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
Wisdom is one of the few things that looks bigger the further away it
is.                               - Terry Pratchett, _Witches Abroad_

[U-Boot] [PATCH] bin_dep.sh Support

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <1238759359-6544-2-git-send-email-prafulla@marvell.com> you wrote:
> From: prafulla_wadaskar <prafulla@marvell.com>
> 
> In some cases the u-boot.bin need to be processed further
> to create bootable u-boot binary from boot device
> This processing may be cpu,soc and/or board spcific
> bin_dep.sh provides a mechanism to execute bin_dep.sh
> if present in above platform specific folders
> 
> Signed-off-by: prafulla_wadaskar <prafulla@marvell.com>
> ---
>  Makefile         |    2 +
>  tools/bin_dep.sh |   79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 81 insertions(+), 0 deletions(-)
>  create mode 100755 tools/bin_dep.sh

I don't see use cases for this.  This should probably remain out of
tree.

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
I can't understand it. I can't even understand  the  people  who  can
understand it.                    - Queen Juliana of the Netherlands.

[U-Boot] [PATCH] debug_print macros support

[Prafulla Wadaskar]

Dear Wolfgang Denk

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Friday, April 03, 2009 11:23 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ronen Shitrit
> Subject: Re: [U-Boot] [PATCH] debug_print macros support
> 
> Dear Prafulla Wadaskar,
> 
> In message 
> <1238759359-6544-1-git-send-email-prafulla@marvell.com> you wrote:
> > From: prafulla_wadaskar <prafulla@marvell.com>
> >
> > This debug_prints strategy provides-
> > A. pre-formatted debug and print macros to be used in a code B. 
> > flexiblility to enable selective debug prints without
> >    modifying a source code
> > For more details refer doc/README.debug_prints
> >
> > Signed-off-by: prafulla_wadaskar <prafulla@marvell.com>
> 
> This seems to be a lot of effort, from code to command line pollution
> - for what exactly?
> 
> Which is the problem you are trying to solve?
If I understood current u-boot debugging properly, the debug statements are enabled by editing src file by adding "#define DEBUG"
Doing this the release contents will be tampered,
this may not be relevant from development point of view.
But the same release will be used by validation/testing/QA guys and to debug anything out of released contents, no one would like to tamper the source code subject to condition proper debugging strategy is in place.

Secondly, even during development it is a pain to enable/disable debug statements for their needs and finally remove them before release. I suffered this pain that's why discovered this strategy.

The suggested patch provides totally optional strategy that offers-
A) enabling selective debug statements without modifying source code
B) enabling selective debug statements only from specific source file/s
C) Last but not least- simple, small, easy to understand code.

Again.. it is an optional value added tool for u-boot developers :-)

Regards..
Prafulla . .

> 
> 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 Wisdom is one of the few things that looks bigger 
> the further away it
> is.                               - Terry Pratchett, _Witches Abroad_
> 

[U-Boot] [PATCH] debug_print macros support

[Mike Frysinger]

On Monday 06 April 2009 01:32:30 Prafulla Wadaskar wrote:
> If I understood current u-boot debugging properly, the debug statements are
> enabled by editing src file by adding "#define DEBUG" Doing this the
> release contents will be tampered,
> this may not be relevant from development point of view.
> But the same release will be used by validation/testing/QA guys and to
> debug anything out of released contents, no one would like to tamper the
> source code subject to condition proper debugging strategy is in place.

not really true ... you can do:
make DBGFLAGS=-DDEBUG

but that will probably enable debug() statements in a lot of things you dont 
care about ...

i dont really have an opinion on the idea in general
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090406/c530368d/attachment.pgp 

[U-Boot] [PATCH] debug_print macros support

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Mike Frysinger [mailto:vapier at gentoo.org] 
> Sent: Monday, April 06, 2009 12:01 PM
> To: u-boot at lists.denx.de
> Cc: Prafulla Wadaskar; Wolfgang Denk; Ronen Shitrit
> Subject: Re: [U-Boot] [PATCH] debug_print macros support
> 
> On Monday 06 April 2009 01:32:30 Prafulla Wadaskar wrote:
> > If I understood current u-boot debugging properly, the debug 
> > statements are enabled by editing src file by adding 
> "#define DEBUG" 
> > Doing this the release contents will be tampered, this may not be 
> > relevant from development point of view.
> > But the same release will be used by validation/testing/QA 
> guys and to 
> > debug anything out of released contents, no one would like 
> to tamper 
> > the source code subject to condition proper debugging 
> strategy is in place.
> 
> not really true ... you can do:
> make DBGFLAGS=-DDEBUG
> 
> but that will probably enable debug() statements in a lot of 
> things you dont care about ...
Even this can be resolved by this patch implementation
Regards..
Prafulla . .

> 
> i dont really have an opinion on the idea in general -mike
> 

[U-Boot] [PATCH] Macronix MX25xx MTD SPI flash driver

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Mike Frysinger [mailto:vapier at gentoo.org] 
> Sent: Friday, April 03, 2009 7:44 PM
> To: u-boot at lists.denx.de
> Cc: Prafulla Wadaskar; Ronen Shitrit
> Subject: Re: [U-Boot] [PATCH] Macronix MX25xx MTD SPI flash driver
> 
> On Friday 03 April 2009 07:49:19 Prafulla Wadaskar wrote:
> > --- a/drivers/mtd/spi/Makefile
> > +++ b/drivers/mtd/spi/Makefile
> > @@ -28,6 +28,7 @@ LIB	:= $(obj)libspi_flash.a
> >  COBJS-$(CONFIG_SPI_FLASH)	+= spi_flash.o
> >  COBJS-$(CONFIG_SPI_FLASH_ATMEL)	+= atmel.o
> >  COBJS-$(CONFIG_SPI_FLASH_STMICRO)	+= stmicro.o
> > +COBJS-$(CONFIG_SPI_FLASH_MACRONIX)	+= macronix.o
> 
> please keep the list sorted
Done..

> 
> > --- /dev/null
> > +++ b/drivers/mtd/spi/macronix.c
> > @@ -0,0 +1,319 @@
> > + * Based on drivers/mtd/spi/stmicor.c
> 
> typo in file name
Corrected..

> 
> > + * 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>
> 
> please add a new line there
Added..

> 
> > +/* MX25Pxx-specific commands */
> > +#define CMD_MX25PXX_WREN	0x06	/* Write Enable */
> > +#define CMD_MX25PXX_WRDI	0x04	/* Write Disable */
> > +#define CMD_MX25PXX_RDSR	0x05	/* Read Status Register */
> > +#define CMD_MX25PXX_WRSR	0x01	/* Write Status Register */
> > +#define CMD_MX25PXX_READ	0x03	/* Read Data Bytes */
> > +#define CMD_MX25PXX_FAST_READ	0x0b	/* Read Data 
> Bytes at Higher Speed */
> > +#define CMD_MX25PXX_PP		0x02	/* Page Program */
> > +#define CMD_MX25PXX_SE		0x20	/* Sector Erase */
> > +#define CMD_MX25PXX_BE		0xD8	/* Block Erase */
> > +#define CMD_MX25PXX_CE		0xc7	/* Chip Erase */
> > +#define CMD_MX25PXX_DP		0xb9	/* Deep Power-down */
> > +#define CMD_MX25PXX_RES		0xab	/* Release from 
> DP, and Read Signature */
> 
> is it really MX25PXX ?  or should it be MX25XX ?  the P looks 
> like a hold over from the stmicro driver.
You are right
It should be MX25LXX for 3.0 volt operation And it should be 
MX27VXX for 2.5 volt operation To address a common driver for 
both, I am making it MX25XX

> 
> > +struct macronix_spi_flash {
> > +	const struct macronix_spi_flash_params *params;
> > +	struct spi_flash flash;
> > +};
> 
> the spi_flash struct needs to be the first member in order to 
> prevent crashes when working with the spi flash layer
Done..

> 
> > +	u8 cmd[4] = { CMD_MX25PXX_RDSR, 0xff, 0xff, 0xff };
> > +
> > +	ret = spi_xfer(spi, 32, &cmd[0], NULL, SPI_XFER_BEGIN);
> 
> do you actually need to read/write 4 bytes here ?  shouldnt it be:
> u8 cmd = CMD_MX25PXX_RDSR;
> ret = spi_xfer(spi, 8, &cmd, NULL, SPI_XFER_BEGIN);
> 
Yes, I have checked specs too, we need only 8bit transfer here
Done...

> > +	if (ret) {
> > +		debug("SF: Failed to send command %02x: %d\n", 
> cmd[0], ret);
> 
> then you'll need to change to "cmd"
> 
> > +			debug("SF: STMicro Page Program failed\n");
> > +	debug("SF: STMicro: Successfully programmed %u bytes @ 0x%x\n",
> > +	      len, offset);
> > +			debug("SF: STMicro page erase failed\n");
> > +			debug("SF: STMicro page erase timed out\n");
> > +	debug("SF: STMicro: Successfully erased %u bytes @ 0x%x\n",
> > +	      len * sector_size, offset);
> > +		debug("SF: Unsupported STMicro ID %02x\n", id[1]);
> 
> this isnt STMicro anymore ...
Corrected... Very sorry for this :-(

> 
> > +		/* Up to 2 seconds */
> > +		ret = macronix_wait_ready(flash, 2 * CONFIG_SYS_HZ);
> 
> there's a common flash erase timeout define
Block erase time for Micronix are different than specified in spi_flash_internals.h
Hence MICRONIX spefic timouts defined and used in macronix.c
 
> 
> > +*spi_flash_probe_macronix(struct spi_slave *spi, u8 * idcode)
> 
> no space between "*" and "idcode"
Done..

> 
> > +	u8 id[3];
> > +
> > +	ret = spi_flash_cmd(spi, CMD_READ_ID, id, sizeof(id));
> > +	if (ret)
> > +		return NULL;
> 
> no need to read id here ... use the idcode passed in to you
> 
Done..

> > +		if (params->idcode1 == idcode[2]) {
> > +			break;
> > +		}
> 
> drop the {} braces
Dropped..

> 
> > --- a/drivers/mtd/spi/spi_flash.c
> > +++ b/drivers/mtd/spi/spi_flash.c
> > @@ -3,7 +3,6 @@
> >   *
> >   * Copyright (C) 2008 Atmel Corporation
> >   */
> > -#define DEBUG
> >  #include <common.h>
> >  #include <malloc.h>
> >  #include <spi.h>
> 
> please drop this hunk ... it doesnt belong in this patch (and 
> it's already been merged elsewhere) -mike
Okay Dropped

I am ready with the updated patch,
How should I release it the community?
1. new patch with review feedback (with same name)
Or
2. Delta on the top of previous patch

Any suggestion...?

Regards..
Prafulla . .

> 

[U-Boot] [PATCH] bin_dep.sh Support

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Friday, April 03, 2009 11:25 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ronen Shitrit
> Subject: Re: [U-Boot] [PATCH] bin_dep.sh Support
> 
> Dear Prafulla Wadaskar,
> 
> In message 
> <1238759359-6544-2-git-send-email-prafulla@marvell.com> you wrote:
> > From: prafulla_wadaskar <prafulla@marvell.com>
> >
> > In some cases the u-boot.bin need to be processed further to create 
> > bootable u-boot binary from boot device This processing may 
> be cpu,soc 
> > and/or board spcific bin_dep.sh provides a mechanism to execute 
> > bin_dep.sh if present in above platform specific folders
> >
> > Signed-off-by: prafulla_wadaskar <prafulla@marvell.com>
> > ---
> >  Makefile         |    2 +
> >  tools/bin_dep.sh |   79 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 81 insertions(+), 0 deletions(-)  create 
> mode 100755 
> > tools/bin_dep.sh
> 
> I don't see use cases for this.  This should probably remain 
> out of tree.
Dear Wolfgang Denk
Relevant use case will be Marvell Kirkwood SOC, this patch will be dependency for the same

Regards..
Prafulla . .

> 
> 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 I can't understand it. I can't even understand  
> the  people  who  can
> understand it.                    - Queen Juliana of the Netherlands.
> 

[U-Boot] [PATCH] Macronix MX25xx MTD SPI flash driver

[Mike Frysinger]

On Monday 06 April 2009 03:23:30 Prafulla Wadaskar wrote:
> > > +		/* Up to 2 seconds */
> > > +		ret = macronix_wait_ready(flash, 2 * CONFIG_SYS_HZ);
> >
> > there's a common flash erase timeout define
>
> Block erase time for Micronix are different than specified in
> spi_flash_internals.h Hence MICRONIX spefic timouts defined and used in
> macronix.c

every flash will have a different erase time, but i have a hard time believing 
you picked 2*HZ (2 seconds) because of your part's requirements.  more likely 
you copied that from the stmicro driver (like every other spi flash driver 
until i cleaned them up).  the latest tree increases the common timeouts to 
seconds rather than milliseconds, so unless your flash is truly truly awful 
and requires longer than that, you should use the common define.

> I am ready with the updated patch,
> How should I release it the community?
> 1. new patch with review feedback (with same name)

the current accepted practice is for you to:
 - update the patch with all the comments
 - add a "v2" to the subject (so it'd be "[PATCH v2] Macronix .....")
 - in the comments section of the formatted patch (after the --- marker and 
above the diffstat), add notes about what changed in the updated patch
 - send it with the reply to header set to the last relevant message in the 
patches' thread

> 2. Delta on the top of previous patch

nooooooooo dont ever do this :)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090406/be05af06/attachment.pgp 

[U-Boot] [PATCH] debug_print macros support

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <73173D32E9439E4ABB5151606C3E19E2508402D4@SC-VEXCH1.marvell.com> you wrote:
> 
> > This seems to be a lot of effort, from code to command line pollution
> > - for what exactly?
> > 
> > Which is the problem you are trying to solve?
> If I understood current u-boot debugging properly, the debug statements are
>  enabled by editing src file by adding "#define DEBUG"
> Doing this the release contents will be tampered,

This is just one way to solve this, which is most convenient when you
want to enable debugging on a  per-file  level.  Of  course  you  can
acchieve  the  same  effect  by  just  passing "-DDEBUG" as a compile
option. The Makefile allows to do this globally - there are currently
no fine-grained selections available by default.

On the other hand, this is debug code which is  not  supposed  to  be
present  in  a  production  release,  so modifying the source seems a
minor issue to me.

> But the same release will be used by validation/testing/QA guys and to debu
> g anything out of released contents, no one would like to tamper the source
>  code subject to condition proper debugging strategy is in place.

As mentioned, you can enable this by compile options as well.

> The suggested patch provides totally optional strategy that offers-
> A) enabling selective debug statements without modifying source code
> B) enabling selective debug statements only from specific source file/s
> C) Last but not least- simple, small, easy to understand code.
> 
> Again.. it is an optional value added tool for u-boot developers :-)

To be honest: I see more overhead added. Looks like overkill to me.

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
Even if you can deceive people about  a  product  through  misleading
statements, sooner or later the product will speak for itself.
- Hajime Karatsu

[U-Boot] [PATCH] bin_dep.sh Support

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <73173D32E9439E4ABB5151606C3E19E2508402F4@SC-VEXCH1.marvell.com> you wrote:
> 
> > I don't see use cases for this.  This should probably remain 
> > out of tree.
>
> Relevant use case will be Marvell Kirkwood SOC, this patch will be dependen
> cy for the same

It is obvious that you can invent some new stuff (or rather reinvent
existing stuff) and then use it for your own boards.

The questions remain if this makes sense, and if this should be added
to mainline.

[U-Boot] [PATCH] debug_print macros support

[Mike Frysinger]

On Monday 06 April 2009 03:56:39 Wolfgang Denk wrote:
> In message Prafulla Wadaskar wrote:
> > > This seems to be a lot of effort, from code to command line pollution
> > > - for what exactly?
> > >
> > > Which is the problem you are trying to solve?
> >
> > If I understood current u-boot debugging properly, the debug statements
> > are enabled by editing src file by adding "#define DEBUG"
> > Doing this the release contents will be tampered,
>
> This is just one way to solve this, which is most convenient when you
> want to enable debugging on a  per-file  level.  Of  course  you  can
> acchieve  the  same  effect  by  just  passing "-DDEBUG" as a compile
> option. The Makefile allows to do this globally - there are currently
> no fine-grained selections available by default.

i havent tested it, but i think this should get fine-grained flags in a 
flexible manner ?  (the out of tree isnt exact, but you see what i'm after)
-mike

diff --git a/config.mk b/config.mk
index b1254e9..8791caa 100644
--- a/config.mk
+++ b/config.mk
@@ -209,20 +209,20 @@ export	TEXT_BASE PLATFORM_CPPFLAGS PLATFORM_RELFLAGS 
CPPFLAGS CFLAGS AFLAGS
 ifndef REMOTE_BUILD
 
 %.s:	%.S
-	$(CPP) $(AFLAGS) -o $@ $<
+	$(CPP) $(AFLAGS) $(AFLAGS_$@) -o $@ $<
 %.o:	%.S
-	$(CC) $(AFLAGS) -c -o $@ $<
+	$(CC) $(AFLAGS) $(AFLAGS_$@) -c -o $@ $<
 %.o:	%.c
-	$(CC) $(CFLAGS) -c -o $@ $<
+	$(CC) $(CFLAGS) $(CFLAGS_$@) -c -o $@ $<
 
 else
 
 $(obj)%.s:	%.S
-	$(CPP) $(AFLAGS) -o $@ $<
+	$(CPP) $(AFLAGS) $(AFLAGS_$@) -o $@ $<
 $(obj)%.o:	%.S
-	$(CC) $(AFLAGS) -c -o $@ $<
+	$(CC) $(AFLAGS) $(AFLAGS_$@) -c -o $@ $<
 $(obj)%.o:	%.c
-	$(CC) $(CFLAGS) -c -o $@ $<
+	$(CC) $(CFLAGS) $(CFLAGS_$@) -c -o $@ $<
 endif
 
 #########################################################################

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090406/3b97a2d1/attachment.pgp 

[U-Boot] [PATCH] bin_dep.sh Support

[Ronen Shitrit]

Hi

I'm not sure we are on the same page here, the Kirkwood has an internal bootROM which can only boot an image if this image is wrapped by a specific header.

The do_image should take the u-boot bin and wrap it with this header.

How do u suggest we will support this header generation for the U-Boot?

Thanks for your help
Ronen Shitrit


-----Original Message-----
From: Wolfgang Denk [mailto:wd at denx.de]
Sent: Monday, April 06, 2009 11:05 AM
To: Prafulla Wadaskar
Cc: u-boot at lists.denx.de; Ronen Shitrit
Subject: Re: [U-Boot] [PATCH] bin_dep.sh Support

Dear Prafulla Wadaskar,

In message <73173D32E9439E4ABB5151606C3E19E2508402F4@SC-VEXCH1.marvell.com> you wrote:
>
> > I don't see use cases for this.  This should probably remain
> > out of tree.
>
> Relevant use case will be Marvell Kirkwood SOC, this patch will be dependen
> cy for the same

It is obvious that you can invent some new stuff (or rather reinvent
existing stuff) and then use it for your own boards.

The questions remain if this makes sense, and if this should be added
to mainline.

[U-Boot] [PATCH] bin_dep.sh Support

[Wolfgang Denk]

Dear Ronen Shitrit,

In message <309002C0DA137042828828FC53D7A9347E13537FE0@IL-MB01.marvell.com> you wrote:
>
> I'm not sure we are on the same page here, the Kirkwood has an
> internal bootROM which can only boot an image if this image is
> wrapped by a specific header.

What a stupid (read: broken) hardware design.

There are tons of standards for image formats, and even more commonly
used formats I would not dare to call standard; but this processor
has to invent yet another one?

> The do_image should take the u-boot bin and wrap it with this header.
>
> How do u suggest we will support this header generation for the U-Boot?

Fix the hardware design?

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
"When anyone says `theoretically,' they really mean `not really.'"
- David Parnas

[U-Boot] [PATCH] Macronix MX25xx MTD SPI flash driver

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Mike Frysinger [mailto:vapier at gentoo.org] 
> Sent: Monday, April 06, 2009 1:09 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Ronen Shitrit
> Subject: Re: [U-Boot] [PATCH] Macronix MX25xx MTD SPI flash driver
> 
> On Monday 06 April 2009 03:23:30 Prafulla Wadaskar wrote:
> > > > +		/* Up to 2 seconds */
> > > > +		ret = macronix_wait_ready(flash, 2 * 
> CONFIG_SYS_HZ);
> > >
> > > there's a common flash erase timeout define
> >
> > Block erase time for Micronix are different than specified in 
> > spi_flash_internals.h Hence MICRONIX spefic timouts defined 
> and used 
> > in macronix.c
> 
> every flash will have a different erase time, but i have a 
> hard time believing you picked 2*HZ (2 seconds) because of 
> your part's requirements.  more likely you copied that from 
> the stmicro driver (like every other spi flash driver until i 
> cleaned them up).  the latest tree increases the common 
> timeouts to seconds rather than milliseconds, so unless your 
> flash is truly truly awful and requires longer than that, you 
> should use the common define.
The MX25L12805D specifies Block erase max time to 2sec
Even sector erase time is higher (300milsec)
Specs can be found at (page 37) http://www.macronix.com/QuickPlace/hq/PageLibrary4825740B00298A3B.nsf/h_Index/7CE3021F57B8EA4848257412002E26CC/$File/MX25L12805D-1.1.pdf
 
> 
> > I am ready with the updated patch,
> > How should I release it the community?
> > 1. new patch with review feedback (with same name)
> 
> the current accepted practice is for you to:
>  - update the patch with all the comments
>  - add a "v2" to the subject (so it'd be "[PATCH v2] Macronix .....")
>  - in the comments section of the formatted patch (after the 
> --- marker and above the diffstat), add notes about what 
> changed in the updated patch
>  - send it with the reply to header set to the last relevant 
> message in the patches' thread
I will do the needful accordingly

> 
> > 2. Delta on the top of previous patch
> 
> nooooooooo dont ever do this :)
Thanks....
Stored in my mind
Regards..
Prafulla . .

> -mike
> 

[U-Boot] [PATCH] bin_dep.sh Support

[Ronen Shitrit]

Dear Ronen Shitrit,

In message <309002C0DA137042828828FC53D7A9347E13537FE0@IL-MB01.marvell.com> you wrote:
>
> I'm not sure we are on the same page here, the Kirkwood has an
> internal bootROM which can only boot an image if this image is
> wrapped by a specific header.

There are tons of standards for image formats, and even more commonly
used formats I would not dare to call standard; but this processor
has to invent yet another one?

[Ronen Shitrit] I still think we are not on the same page here.
This isn't another image format...
Why do we need the bootrom?
For example the KW doesn't have HW NAND ECC support :(
But we still want to support boot from NAND and ve protected from upto 4 bit ECC.
So the CPU wakes up and start running code from the bootROM (embedded ROM in the Kirkwood) this code read the NAND and verify its ECC, then according to the binary image header in the NAND it initialize the DRAM for this specific board and copy the binary image from the NAND to the DRAM and start running from there.
I hope now its more clear...


> The do_image should take the u-boot bin and wrap it with this header.
>
> How do u suggest we will support this header generation for the U-Boot?

Fix the hardware design?
[Ronen Shitrit] probably won't happen...

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
"When anyone says `theoretically,' they really mean `not really.'"
- David Parnas

[U-Boot] [PATCH] Macronix MX25xx MTD SPI flash driver

[Mike Frysinger]

On Monday 06 April 2009 04:27:59 Prafulla Wadaskar wrote:
> From: Mike Frysinger [mailto:vapier at gentoo.org]
> > On Monday 06 April 2009 03:23:30 Prafulla Wadaskar wrote:
> > > > > +		/* Up to 2 seconds */
> > > > > +		ret = macronix_wait_ready(flash, 2 * CONFIG_SYS_HZ);
> > > >
> > > > there's a common flash erase timeout define
> > >
> > > Block erase time for Micronix are different than specified in
> > > spi_flash_internals.h Hence MICRONIX spefic timouts defined and used
> > > in macronix.c
> >
> > every flash will have a different erase time, but i have a
> > hard time believing you picked 2*HZ (2 seconds) because of
> > your part's requirements.  more likely you copied that from
> > the stmicro driver (like every other spi flash driver until i
> > cleaned them up).  the latest tree increases the common
> > timeouts to seconds rather than milliseconds, so unless your
> > flash is truly truly awful and requires longer than that, you
> > should use the common define.
>
> The MX25L12805D specifies Block erase max time to 2sec

touche, i stand corrected

> Even sector erase time is higher (300milsec)

as i said, the latest code is updated to order of seconds.  specifically, 
write is now 2 seconds while erase is 5 seconds.  so both of these are OK for 
your chip and thus you should use them.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090406/f28f5acb/attachment.pgp 

[U-Boot] [PATCH] bin_dep.sh Support

[Mike Frysinger]

On Monday 06 April 2009 04:24:39 Wolfgang Denk wrote:
> In message Ronen Shitrit wrote:
> > I'm not sure we are on the same page here, the Kirkwood has an
> > internal bootROM which can only boot an image if this image is
> > wrapped by a specific header.
>
> What a stupid (read: broken) hardware design.
>
> There are tons of standards for image formats, and even more commonly
> used formats I would not dare to call standard; but this processor
> has to invent yet another one?

true there are plenty of formats out there, but there arent really ones geared 
towards booting a processor.  the ELF format is way too bloated/complex (and 
yet still not flexible enough) to be usable, and that's about the closest 
applicable format i'm familiar with.  beyond the simple "processor will start 
execution at address XXXX", there isnt any standardization here.

the Blackfin processor too has a custom format (LDR) for booting code.  it's 
akin to the PHDRs in ELF, plus some additional flag bits to control behavior.  
but in our world, i wrote external utilities to manage ELF->LDR conversion and 
so i didnt need a dedicated script in u-boot to do that ugly conversion mojo, 
just a new Makefile target (u-boot.ldr).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090406/0819dade/attachment.pgp 

[U-Boot] [PATCH] bin_dep.sh Support

[Wolfgang Denk]

Dear Ronen Shitrit,

In message <309002C0DA137042828828FC53D7A9347E13538016@IL-MB01.marvell.com> you wrote:
> 
> Dear Ronen Shitrit,
> 
> In message <309002C0DA137042828828FC53D7A9347E13537FE0@IL-MB01.marvell.com>=
>  you wrote:
> >
> > I'm not sure we are on the same page here, the Kirkwood has an
> > internal bootROM which can only boot an image if this image is
> > wrapped by a specific header.
> 
> There are tons of standards for image formats, and even more commonly
> used formats I would not dare to call standard; but this processor
> has to invent yet another one?

Could you please use standard quoting rules? See
http://www.netmeister.org/news/learn2quote.html

Aplso, please restrict your line lengh to some 70 characters or so.

Thanks.

> [Ronen Shitrit] I still think we are not on the same page here.
> This isn't another image format...

But yes, it is.

> Why do we need the bootrom?
> For example the KW doesn't have HW NAND ECC support :(
> But we still want to support boot from NAND and ve protected from upto 4 bi
> t ECC.
> So the CPU wakes up and start running code from the bootROM (embedded ROM i
> n the Kirkwood) this code read the NAND and verify its ECC, then according
> to the binary image header in the NAND it initialize the DRAM for this spec

You see? "binary image header" Here you say yourself that this is a
binary image header, i. e. a prorpietary image format.

> Fix the hardware design?
> [Ronen Shitrit] probably won't happen...

Though so.

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
backups: always in season, never out of style.

[U-Boot] [PATCH] bin_dep.sh Support

[Wolfgang Denk]

Dear Mike,

in message <200904060503.18864.vapier@gentoo.org> you wrote:
>
> the Blackfin processor too has a custom format (LDR) for booting code.  it's 
> akin to the PHDRs in ELF, plus some additional flag bits to control behavior.  
> but in our world, i wrote external utilities to manage ELF->LDR conversion and 
> so i didnt need a dedicated script in u-boot to do that ugly conversion mojo, 
> just a new Makefile target (u-boot.ldr).

...which seems to be an acceptable (to both sides) compromise to me.

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
"No matter where you go, there you are..."          - Buckaroo Banzai

[U-Boot] [PATCH] bin_dep.sh Support

[Mike Frysinger]

On Monday 06 April 2009 05:16:02 Wolfgang Denk wrote:
> in message Mike wrote:
> > the Blackfin processor too has a custom format (LDR) for booting code. 
> > it's akin to the PHDRs in ELF, plus some additional flag bits to control
> > behavior. but in our world, i wrote external utilities to manage ELF->LDR
> > conversion and so i didnt need a dedicated script in u-boot to do that
> > ugly conversion mojo, just a new Makefile target (u-boot.ldr).
>
> ...which seems to be an acceptable (to both sides) compromise to me.

minus support for embedding the environment into the image ... i have a patch 
that adds an option to tools/envcrc to export the environment as a binary blob 
so that it can be passed to the external utility, but you wouldnt take it 
because you wanted me to fix my hardware instead ;).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090406/0cbe828b/attachment.pgp 

[U-Boot] [PATCH] bin_dep.sh Support

[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: Monday, April 06, 2009 2:46 PM
> To: Mike Frysinger
> Cc: u-boot at lists.denx.de; Ronen Shitrit
> Subject: Re: [U-Boot] [PATCH] bin_dep.sh Support
> 
> Dear Mike,
> 
> in message <200904060503.18864.vapier@gentoo.org> you wrote:
> >
> > the Blackfin processor too has a custom format (LDR) for 
> booting code.  
> > it's akin to the PHDRs in ELF, plus some additional flag 
> bits to control behavior.
> > but in our world, i wrote external utilities to manage ELF->LDR 
> > conversion and so i didnt need a dedicated script in u-boot 
> to do that 
> > ugly conversion mojo, just a new Makefile target (u-boot.ldr).
> 
> ...which seems to be an acceptable (to both sides) compromise to me.
I understood from the discussion so far that-
1. There are several cases where u-boot.bin does not help alone
2. in some cases there are external tools to be used for the conversion
   but ideally it is not possible to include each tool in a project

Let's consider specific case - 
a board having serial console, flash, dram, a processor and supported u-boot.

As a user ideally I expect u-boot.bin generated by u-boot make
should go onto flash and board should work.
But if this doesn't work then how should we educate the uses and address their
questions whereas he might have stumped flashing wrong u-boot.bin to his board...?

This is case with Kirkwood.
I don't think it is ONLY processor having this boot sequence architecture,
there may be several in pipeline...who knows.
Kirkwood is today's need to be addressed.

In my opinion objective of any boot loader is to provide flashable binary.

Bin_dep.sh provides a framework inside u-boot,
whether to use it or not is sole decision of board/processor specific code.
So why not include it :-)

Regards..
Prafulla . .

 

[U-Boot] [PATCH] bin_dep.sh Support

[Detlev Zundel]

Hi,

> Dear Prafulla Wadaskar,
>
> In message <1238759359-6544-2-git-send-email-prafulla@marvell.com> you wrote:
>> From: prafulla_wadaskar <prafulla@marvell.com>
>> 
>> In some cases the u-boot.bin need to be processed further
>> to create bootable u-boot binary from boot device
>> This processing may be cpu,soc and/or board spcific
>> bin_dep.sh provides a mechanism to execute bin_dep.sh
>> if present in above platform specific folders
>> 
>> Signed-off-by: prafulla_wadaskar <prafulla@marvell.com>
>> ---
>>  Makefile         |    2 +
>>  tools/bin_dep.sh |   79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 81 insertions(+), 0 deletions(-)
>>  create mode 100755 tools/bin_dep.sh
>
> I don't see use cases for this.  This should probably remain out of
> tree.

If the wrapping is needed to get a working U-Boot image for this
specific board, I'll vote to add the code.

On the other hand, I wouldn't probably add this to the u-boot.bin
target.  A makefile should really specify what file is generated from
what and generating an extra target in the u-boot.bin rule somewhat
violates this rule.

I'd say, add another target and the script.  Hopefully we can make this
as generic as possible so that other boards needing this can only plug
in their script.

Cheers
  Detlev

-- 
I will use free software even if it is less powerful, or less reliable,
because freedom is the most important thing, and that is what the Free
Software movement is about.  How we get freedom.
                       -- Richard M. Stallman
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] bin_dep.sh Support

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Detlev Zundel [mailto:dzu at denx.de] 
> Sent: Monday, April 06, 2009 3:10 PM
> To: Wolfgang Denk
> Cc: Prafulla Wadaskar; u-boot at lists.denx.de; Ronen Shitrit
> Subject: Re: [U-Boot] [PATCH] bin_dep.sh Support
> 
> Hi,
> 
> > Dear Prafulla Wadaskar,
> >
> > In message 
> <1238759359-6544-2-git-send-email-prafulla@marvell.com> you wrote:
> >> From: prafulla_wadaskar <prafulla@marvell.com>
> >> 
> >> In some cases the u-boot.bin need to be processed further 
> to create 
> >> bootable u-boot binary from boot device This processing may be 
> >> cpu,soc and/or board spcific bin_dep.sh provides a mechanism to 
> >> execute bin_dep.sh if present in above platform specific folders
> >> 
> >> Signed-off-by: prafulla_wadaskar <prafulla@marvell.com>
> >> ---
> >>  Makefile         |    2 +
> >>  tools/bin_dep.sh |   79 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 81 insertions(+), 0 deletions(-)  create mode 
> >> 100755 tools/bin_dep.sh
> >
> > I don't see use cases for this.  This should probably remain out of 
> > tree.
> 
> If the wrapping is needed to get a working U-Boot image for 
> this specific board, I'll vote to add the code.
Dear Detlev
Thanks for your vote.

Regards..
Prafulla . .

[U-Boot] [PATCH] bin_dep.sh Support

[Mike Frysinger]

On Monday 06 April 2009 05:38:59 Prafulla Wadaskar wrote:
> As a user ideally I expect u-boot.bin generated by u-boot make
> should go onto flash and board should work.

i disagree.  u-boot.bin should be the exact flat binary of converting from the 
u-boot ELF file using objdump.

> But if this doesn't work then how should we educate the uses and address
> their questions whereas he might have stumped flashing wrong u-boot.bin to
> his board...?

yes.  we havent had a problem in the Blackfin world with educating people.  if 
the bootrom is used, then they have to use u-boot.ldr.  if they're using an 
older Blackfin part and the one bootmode that supports direct execution (the 
part starts running at a hardcoded address), then and only then can they use 
u-boot.bin.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090406/c3bde7fe/attachment.pgp 

[U-Boot] [PATCH] bin_dep.sh Support

[Wolfgang Denk]

Dear Mike Frysinger,

In message <200904060533.47911.vapier@gentoo.org> you wrote:
>
> > ...which seems to be an acceptable (to both sides) compromise to me.
>
> minus support for embedding the environment into the image ... i have a patch 
> that adds an option to tools/envcrc to export the environment as a binary blob 
> so that it can be passed to the external utility, but you wouldnt take it 
> because you wanted me to fix my hardware instead ;).

Not true. I can see that such a tool makes sense - it can be always
usefult if you want to pre-initialize the environment on boards where
the environment sector(s) is not embedded with the U-Boot image
itself.

Just post your code :-)

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
"Athens  built  the  Acropolis.  Corinth  was  a   commercial   city,
interested  in  purely  materialistic things. Today we admire Athens,
visit it, preserve the old temples, yet we hardly ever  set  foot  in
Corinth."              - Dr. Harold Urey, Nobel Laureate in chemistry

[U-Boot] [PATCH] bin_dep.sh Support

[Mike Frysinger]

On Monday 06 April 2009 06:28:33 Wolfgang Denk wrote:
> In message Mike Frysinger wrote:
> > > ...which seems to be an acceptable (to both sides) compromise to me.
> >
> > minus support for embedding the environment into the image ... i have a
> > patch that adds an option to tools/envcrc to export the environment as a
> > binary blob so that it can be passed to the external utility, but you
> > wouldnt take it because you wanted me to fix my hardware instead ;).
>
> Not true. I can see that such a tool makes sense - it can be always
> usefult if you want to pre-initialize the environment on boards where
> the environment sector(s) is not embedded with the U-Boot image
> itself.
>
> Just post your code :-)

ok, extracting the env as a blob isnt problematic, it's the addition of 
"ENV_IS_EMBEDDED_CUSTOM".  the env is embedded in the final image that gets 
burned into flash, but not by u-boot.  the external util takes cares of 
combining the u-boot ELF and the environment blob.

but i guess i can split the patch into (1) add --binary to tools/envcrc and 
(2) add ENV_IS_EMBEDDED_CUSTOM.  that way when (2) gets shot down again, at 
least (1) was added so it's one less thing for me to maintain out of tree.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090406/7e5aabd5/attachment-0001.pgp 

[U-Boot] [PATCH] bin_dep.sh Support

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <73173D32E9439E4ABB5151606C3E19E250840309@SC-VEXCH1.marvell.com> you wrote:
> 
> > ...which seems to be an acceptable (to both sides) compromise to me.
> I understood from the discussion so far that-
> 1. There are several cases where u-boot.bin does not help alone
> 2. in some cases there are external tools to be used for the conversion
>    but ideally it is not possible to include each tool in a project

The U-Boot source tree includes the source code for the U-Boot boot
loader. We do not attempt to include source code for all additional
tools that may be needed to build a working U-Boot image - we don;t
include GCC source code, nor binutils, nor any other tools that are
not specific to U-Boot.

> As a user ideally I expect u-boot.bin generated by u-boot make
> should go onto flash and board should work.

Not really. "u-boot.bin" is supposed to be a raw binary image; this
may be directly executable on most processors, but depending on where
and how it is stored additional provisions may be necessary. Such
tools are out of the scope of the U-Boot project.

> But if this doesn't work then how should we educate the uses and address their
> questions whereas he might have stumped flashing wrong u-boot.bin to his board...?

By providing adequate documentation?

You invent a processor with special requirements, so it seems only
natural to me that you educate the users of your processor about the
requirements.

> I don't think it is ONLY processor having this boot sequence architecture,
> there may be several in pipeline...who knows.

Try and talk to your hardware design team about the problems such a
complicated design is causing on the software side...

> Bin_dep.sh provides a framework inside u-boot,
> whether to use it or not is sole decision of board/processor specific code.
> So why not include it :-)

Because we do not attempt to include all possible code for any
proprietary tool that may be needed to implement all the fancy stuff
that might be useful for someone.

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
Given a choice between two theories, take the one which is funnier.

[U-Boot] [PATCH] bin_dep.sh Support

[Scott Wood]

On Mon, Apr 06, 2009 at 10:24:39AM +0200, Wolfgang Denk wrote:
> Dear Ronen Shitrit,
> 
> In message <309002C0DA137042828828FC53D7A9347E13537FE0@IL-MB01.marvell.com> you wrote:
> >
> > I'm not sure we are on the same page here, the Kirkwood has an
> > internal bootROM which can only boot an image if this image is
> > wrapped by a specific header.
> 
> What a stupid (read: broken) hardware design.
> 
> There are tons of standards for image formats, and even more commonly
> used formats I would not dare to call standard; but this processor
> has to invent yet another one?

The same could be said about u-boot requiring its own image format (ELF
and raw binary images can't be passed a device tree or bd_t, AFAICT).

-Scott

[U-Boot] [PATCH] bin_dep.sh Support

[Wolfgang Denk]

Dear Scott Wood,

In message <20090406193049.GB4439@ld0162-tx32.am.freescale.net> you wrote:
>
> > There are tons of standards for image formats, and even more commonly
> > used formats I would not dare to call standard; but this processor
> > has to invent yet another one?
> 
> The same could be said about u-boot requiring its own image format (ELF
> and raw binary images can't be passed a device tree or bd_t, AFAICT).

When the U-Boot (or rather PPCBoot at that time) development was
starte, there was a very important reason for not using ELF: wasting
a full 64 kB for the standard ELF file header was unthinkable on most
embedded devices of that time.

Now, on fat systems with ample resource on one side, and on the other
side with the infrastructure more or less in place to compose a bootm
command from small,  separate  building  blocks,  it  should  be  not
difficult to add such a feature, too.

On the other hand, ELF images are missing a few key points  available
in  U-Boot  images  so  I would not recommend using plain ELF for any
system where reliability or just ease of use are important (which was
the other part of the rationale that led to that format).

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
"The first rule of magic is simple. Don't waste your time waving your
hands and hoping when a rock or a club will do."
                                               - McCloctnik the Lucid

[U-Boot] [PATCH] bin_dep.sh Support

[Mike Frysinger]

On Monday 06 April 2009 15:49:43 Wolfgang Denk wrote:
> In message Scott Wood wrote:
> > > There are tons of standards for image formats, and even more commonly
> > > used formats I would not dare to call standard; but this processor
> > > has to invent yet another one?
> >
> > The same could be said about u-boot requiring its own image format (ELF
> > and raw binary images can't be passed a device tree or bd_t, AFAICT).
>
> When the U-Boot (or rather PPCBoot at that time) development was
> starte, there was a very important reason for not using ELF: wasting
> a full 64 kB for the standard ELF file header was unthinkable on most
> embedded devices of that time.
>
> Now, on fat systems with ample resource on one side, and on the other
> side with the infrastructure more or less in place to compose a bootm
> command from small,  separate  building  blocks,  it  should  be  not
> difficult to add such a feature, too.
>
> On the other hand, ELF images are missing a few key points  available
> in  U-Boot  images  so  I would not recommend using plain ELF for any
> system where reliability or just ease of use are important (which was
> the other part of the rationale that led to that format).

which are the same reasons processors do not implement support for ELF as a 
booting source ...

lets say i was to design a brand new part today with an on-chip rom that 
supports booting from a variety of sources (UART, SPI/I2C/NAND flashes, 
directly addressable NOR flashes, as a slave device to SPI/I2C, etc...), what 
format would you recommend that could satisfy all of these requirements ?  i'm 
not aware of any which is why the Blackfin processor has its own stripped down 
LDR format.
-mike

[U-Boot] [PATCH] bin_dep.sh Support

[Stefan Roese]

On Monday 06 April 2009 11:38:59 Prafulla Wadaskar wrote:
> Bin_dep.sh provides a framework inside u-boot,
> whether to use it or not is sole decision of board/processor specific code.
> So why not include it :-)

Since I'm currently working with a Kirkwood platform, I would like to 
"reactivate" this discussion about support for (external) tools for image 
creation.

On Kirkwood this image header can include the SDRAM intialization parameters 
(SDRAM controller registers settings). So this information which is currently 
*not* included in the Kirkwood U-Boot ports is very important and should be 
included into the U-Boot ports (from my point of view). Enabling support for 
such external tools or perhaps even including this tool directly into U-Boot 
seems to be the way to go for me. Then we could add this additional 
information like SDRAM init parameters into the U-Boot build process again. 
And it would be integrated into the mainline U-Boot git repository which is 
really important from my point of view.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================