misc patches

misc patches

[Sascha Hauer]

Some cleanup patches and also one patch which should
really be in the bootm series. The new uimage code uses
lseek. So to make it work on nand devices lseek is implemented
on bb devices. This currently is limited to readonly mode
which is enough for booting images.

Sascha Hauer (7):
      remove unused watchdog header
      remove unused keyboard.h file
      ARM cpuinfo: decode more bits, use ARRAY_SIZE
      ARM: remove unused icache command
      common.h: remove unused function declarations
      cdev: pass flags to open function
      nand-bb: implement lseek in readonly mode

 arch/arm/cpu/cpu.c         |   34 ----------------
 arch/arm/cpu/cpuinfo.c     |    6 +-
 arch/ppc/lib/board.c       |    1 -
 arch/ppc/lib/ppclinux.c    |    1 -
 arch/ppc/lib/ticks.S       |    1 -
 commands/bootm.c           |    1 -
 drivers/mtd/nand/nand-bb.c |   34 ++++++++++++++++-
 drivers/mtd/ubi/cdev.c     |    2 +-
 fs/devfs-core.c            |    2 +-
 fs/devfs.c                 |    2 +-
 include/common.h           |    5 --
 include/driver.h           |    2 +-
 include/keyboard.h         |   22 ----------
 include/watchdog.h         |   92 --------------------------------------------
 lib/readline_simple.c      |    1 -
 net/net.c                  |    1 -
 16 files changed, 40 insertions(+), 167 deletions(-)
 delete mode 100644 include/keyboard.h
 delete mode 100644 include/watchdog.h

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/7] remove unused watchdog header

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/ppc/lib/board.c    |    1 -
 arch/ppc/lib/ppclinux.c |    1 -
 arch/ppc/lib/ticks.S    |    1 -
 commands/bootm.c        |    1 -
 include/watchdog.h      |   92 -----------------------------------------------
 lib/readline_simple.c   |    1 -
 net/net.c               |    1 -
 7 files changed, 0 insertions(+), 98 deletions(-)
 delete mode 100644 include/watchdog.h

diff --git a/arch/ppc/lib/board.c b/arch/ppc/lib/board.c
index 0e839eb..a840c75 100644
--- a/arch/ppc/lib/board.c
+++ b/arch/ppc/lib/board.c
@@ -23,7 +23,6 @@
 
 #include <debug_ll.h>
 #include <common.h>
-#include <watchdog.h>
 #include <command.h>
 #include <malloc.h>
 #include <memory.h>
diff --git a/arch/ppc/lib/ppclinux.c b/arch/ppc/lib/ppclinux.c
index 471b303..025fd27 100644
--- a/arch/ppc/lib/ppclinux.c
+++ b/arch/ppc/lib/ppclinux.c
@@ -2,7 +2,6 @@
 
 #include <common.h>
 #include <command.h>
-#include <watchdog.h>
 #include <image.h>
 #include <init.h>
 #include <environment.h>
diff --git a/arch/ppc/lib/ticks.S b/arch/ppc/lib/ticks.S
index 5d6c427..453d889 100644
--- a/arch/ppc/lib/ticks.S
+++ b/arch/ppc/lib/ticks.S
@@ -26,7 +26,6 @@
 #include <asm/ppc_asm.tmpl>
 #include <asm/ppc_defs.h>
 #include <config.h>
-#include <watchdog.h>
 
 /*
  * unsigned long long get_ticks(void);
diff --git a/commands/bootm.c b/commands/bootm.c
index f97a842..453ae54 100644
--- a/commands/bootm.c
+++ b/commands/bootm.c
@@ -25,7 +25,6 @@
  * Boot support
  */
 #include <common.h>
-#include <watchdog.h>
 #include <driver.h>
 #include <command.h>
 #include <image.h>
diff --git a/include/watchdog.h b/include/watchdog.h
deleted file mode 100644
index 9265be9..0000000
--- a/include/watchdog.h
+++ /dev/null
@@ -1,92 +0,0 @@
-/*
- * (C) Copyright 2001
- * Erik Theisen,  Wave 7 Optics, etheisen@mindspring.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., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-/*
- * Watchdog functions and macros.
- */
-#ifndef _WATCHDOG_H_
-#define _WATCHDOG_H_
-
-#if defined(CONFIG_HW_WATCHDOG) && defined(CONFIG_WATCHDOG)
-#  error "Configuration error: CONFIG_HW_WATCHDOG and CONFIG_WATCHDOG can't be used together."
-#endif
-
-#if defined(__ASSEMBLY__) && defined(__NIOS__)
-#  error "Configuration error: WATCHDOG_RESET inside assembler not supported for Nios platforms."
-#endif
-
-/*
- * Hardware watchdog
- */
-#ifdef CONFIG_HW_WATCHDOG
-	#if defined(__ASSEMBLY__)
-		#define WATCHDOG_RESET bl hw_watchdog_reset
-	#else
-		extern void hw_watchdog_reset(void);
-
-		#define WATCHDOG_RESET hw_watchdog_reset
-	#endif /* __ASSEMBLY__ */
-#else
-	/*
-	 * Maybe a software watchdog?
-	 */
-	#if defined(CONFIG_WATCHDOG)
-		#if defined(__ASSEMBLY__)
-			#define WATCHDOG_RESET bl watchdog_reset
-		#else
-			extern void watchdog_reset(void);
-
-			#define WATCHDOG_RESET watchdog_reset
-		#endif
-	#else
-		/*
-		 * No hardware or software watchdog.
-		 */
-		#if defined(__ASSEMBLY__)
-			#define WATCHDOG_RESET /*XXX DO_NOT_DEL_THIS_COMMENT*/
-		#else
-			#define WATCHDOG_RESET() {}
-		#endif /* __ASSEMBLY__ */
-	#endif /* CONFIG_WATCHDOG && !__ASSEMBLY__ */
-#endif /* CONFIG_HW_WATCHDOG */
-
-/*
- * Prototypes from $(CPU)/cpu.c.
- */
-
-/* MPC 8xx */
-#if (defined(CONFIG_8xx) || defined(CONFIG_MPC860)) && !defined(__ASSEMBLY__)
-	void reset_8xx_watchdog(volatile immap_t *immr);
-#endif
-
-/* MPC 5xx */
-#if defined(CONFIG_5xx) && !defined(__ASSEMBLY__)
-	void reset_5xx_watchdog(volatile immap_t *immr);
-#endif
-
-/* AMCC 4xx */
-#if defined(CONFIG_4xx) && !defined(__ASSEMBLY__)
-	void reset_4xx_watchdog(void);
-#endif
-
-#endif /* _WATCHDOG_H_ */
diff --git a/lib/readline_simple.c b/lib/readline_simple.c
index 9dd2154..982e50e 100644
--- a/lib/readline_simple.c
+++ b/lib/readline_simple.c
@@ -1,5 +1,4 @@
 #include <common.h>
-#include <watchdog.h>
 
 static char erase_seq[] = "\b \b";		/* erase sequence	*/
 static char   tab_seq[] = "        ";		/* used to expand TABs	*/
diff --git a/net/net.c b/net/net.c
index 8416861..26ba44e 100644
--- a/net/net.c
+++ b/net/net.c
@@ -28,7 +28,6 @@
  */
 #include <common.h>
 #include <clock.h>
-#include <watchdog.h>
 #include <command.h>
 #include <environment.h>
 #include <param.h>
-- 
1.7.7.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/7] remove unused keyboard.h file

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/keyboard.h |   22 ----------------------
 1 files changed, 0 insertions(+), 22 deletions(-)
 delete mode 100644 include/keyboard.h

diff --git a/include/keyboard.h b/include/keyboard.h
deleted file mode 100644
index 88ae12b..0000000
--- a/include/keyboard.h
+++ /dev/null
@@ -1,22 +0,0 @@
-#ifndef __KEYBOARD_H
-#define __KEYBOARD_H
-
-#ifdef CONFIG_PS2MULT
-#include <ps2mult.h>
-#endif
-
-#if !defined(kbd_request_region) || \
-    !defined(kbd_request_irq) || \
-    !defined(kbd_read_input) || \
-    !defined(kbd_read_status) || \
-    !defined(kbd_write_output) || \
-    !defined(kbd_write_command)
-#error PS/2 low level routines not defined
-#endif
-
-extern int kbd_init (void);
-extern void handle_scancode(unsigned char scancode);
-extern int kbd_init_hw(void);
-extern void pckbd_leds(unsigned char leds);
-
-#endif /* __KEYBOARD_H */
-- 
1.7.7.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/7] ARM cpuinfo: decode more bits, use ARRAY_SIZE

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/cpu/cpuinfo.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/cpu/cpuinfo.c b/arch/arm/cpu/cpuinfo.c
index 05971b3..7be1671 100644
--- a/arch/arm/cpu/cpuinfo.c
+++ b/arch/arm/cpu/cpuinfo.c
@@ -47,8 +47,8 @@ static void decode_cache(unsigned long size)
 }
 
 static char *crbits[] = {"M", "A", "C", "W", "P", "D", "L", "B", "S", "R",
-	"F", "Z", "I", "V", "RR", "L4", "", "", "", "", "", "FI", "U", "XP",
-	"VE", "EE", "L2"};
+	"F", "Z", "I", "V", "RR", "L4", "DT", "", "IT", "ST", "", "FI", "U", "XP",
+	"VE", "EE", "L2", "", "TRE", "AFE", "TE"};
 
 static int do_cpuinfo(struct command *cmdtp, int argc, char *argv[])
 {
@@ -170,7 +170,7 @@ static int do_cpuinfo(struct command *cmdtp, int argc, char *argv[])
 	}
 
 	printf("Control register: ");
-	for (i = 0; i < 27; i++)
+	for (i = 0; i < ARRAY_SIZE(crbits); i++)
 		if (cr & (1 << i))
 			printf("%s ", crbits[i]);
 	printf("\n");
-- 
1.7.7.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/7] ARM: remove unused icache command

[Sascha Hauer]

The icache command is unused. Instead of adding it to compilation
again, remove it as the cpuinfo command provides the same information.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/cpu/cpu.c |   34 ----------------------------------
 1 files changed, 0 insertions(+), 34 deletions(-)

diff --git a/arch/arm/cpu/cpu.c b/arch/arm/cpu/cpu.c
index d4a3b14..14bb6d1 100644
--- a/arch/arm/cpu/cpu.c
+++ b/arch/arm/cpu/cpu.c
@@ -89,37 +89,3 @@ void arch_shutdown(void)
 	);
 #endif
 }
-
-/**
- * @page arm_boot_preparation Linux Preparation on ARM
- *
- * For ARM we never enable data cache so we do not need to disable it again.
- * Linux can be called with instruction cache enabled. As this is the
- * default setting we are running in barebox, there's no special preparation
- * required.
- */
-#ifdef CONFIG_COMMAND
-static int do_icache(struct command *cmdtp, int argc, char *argv[])
-{
-	if (argc == 1) {
-		printf("icache is %sabled\n", icache_status() ? "en" : "dis");
-		return 0;
-	}
-
-	if (simple_strtoul(argv[1], NULL, 0) > 0)
-		icache_enable();
-	else
-		icache_disable();
-
-	return 0;
-}
-
-static const __maybe_unused char cmd_icache_help[] =
-"Usage: icache [0|1]\n";
-
-BAREBOX_CMD_START(icache)
-	.cmd		= do_icache,
-	.usage		= "show/change icache status",
-	BAREBOX_CMD_HELP(cmd_icache_help)
-BAREBOX_CMD_END
-#endif
-- 
1.7.7.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/7] common.h: remove unused function declarations

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/common.h |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/include/common.h b/include/common.h
index d594e63..2f37dd8 100644
--- a/include/common.h
+++ b/include/common.h
@@ -81,8 +81,6 @@
 })
 #endif
 
-typedef void (interrupt_handler_t)(void *);
-
 #include <asm/barebox.h> /* boot information for Linux kernel */
 
 /*
@@ -93,8 +91,6 @@ void reginfo(void);
 void __noreturn hang (void);
 void __noreturn panic(const char *fmt, ...);
 
-/* */
-void initdram (int);
 char *size_human_readable(ulong size);
 
 /* common/main.c */
@@ -133,7 +129,6 @@ struct memarea_info {
         unsigned long flags;
 };
 
-int spec_str_to_info(const char *str, struct memarea_info *info);
 int parse_area_spec(const char *str, ulong *start, ulong *size);
 
 /* Just like simple_strtoul(), but this one honors a K/M/G suffix */
-- 
1.7.7.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 6/7] cdev: pass flags to open function

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/nand-bb.c |    2 +-
 drivers/mtd/ubi/cdev.c     |    2 +-
 fs/devfs-core.c            |    2 +-
 fs/devfs.c                 |    2 +-
 include/driver.h           |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/nand-bb.c b/drivers/mtd/nand/nand-bb.c
index dbfb8e3..ae84ce1 100644
--- a/drivers/mtd/nand/nand-bb.c
+++ b/drivers/mtd/nand/nand-bb.c
@@ -164,7 +164,7 @@ static int nand_bb_erase(struct cdev *cdev, size_t count, unsigned long offset)
 }
 #endif
 
-static int nand_bb_open(struct cdev *cdev)
+static int nand_bb_open(struct cdev *cdev, unsigned long flags)
 {
 	struct nand_bb *bb = cdev->priv;
 
diff --git a/drivers/mtd/ubi/cdev.c b/drivers/mtd/ubi/cdev.c
index 96ae16e..95bef1f 100644
--- a/drivers/mtd/ubi/cdev.c
+++ b/drivers/mtd/ubi/cdev.c
@@ -81,7 +81,7 @@ static ssize_t ubi_volume_cdev_write(struct cdev* cdev, const void *buf,
 	return err;
 }
 
-static int ubi_volume_cdev_open(struct cdev *cdev)
+static int ubi_volume_cdev_open(struct cdev *cdev, unsigned long flags)
 {
 	struct ubi_volume_cdev_priv *priv = cdev->priv;
 
diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 9bc3126..5f22ce7 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -63,7 +63,7 @@ struct cdev *cdev_open(const char *name, unsigned long flags)
 		return NULL;
 
 	if (cdev->ops->open) {
-		ret = cdev->ops->open(cdev);
+		ret = cdev->ops->open(cdev, flags);
 		if (ret)
 			return NULL;
 	}
diff --git a/fs/devfs.c b/fs/devfs.c
index 66f7ca4..2e70cc5 100644
--- a/fs/devfs.c
+++ b/fs/devfs.c
@@ -116,7 +116,7 @@ static int devfs_open(struct device_d *_dev, FILE *f, const char *filename)
 	f->inode = cdev;
 
 	if (cdev->ops->open) {
-		ret = cdev->ops->open(cdev);
+		ret = cdev->ops->open(cdev, f->flags);
 		if (ret)
 			return ret;
 	}
diff --git a/include/driver.h b/include/driver.h
index bbe7248..bc60bdc 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -366,7 +366,7 @@ struct file_operations {
 
 	int (*ioctl)(struct cdev*, int, void *);
 	off_t (*lseek)(struct cdev*, off_t);
-	int (*open)(struct cdev*);
+	int (*open)(struct cdev*, unsigned long flags);
 	int (*close)(struct cdev*);
 	int (*flush)(struct cdev*);
 	int (*erase)(struct cdev*, size_t count, unsigned long offset);
-- 
1.7.7.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 7/7] nand-bb: implement lseek in readonly mode

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/nand-bb.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/nand-bb.c b/drivers/mtd/nand/nand-bb.c
index ae84ce1..1482374 100644
--- a/drivers/mtd/nand/nand-bb.c
+++ b/drivers/mtd/nand/nand-bb.c
@@ -45,6 +45,7 @@ struct nand_bb {
 	size_t raw_size;
 	size_t size;
 	off_t offset;
+	unsigned long flags;
 	void *writebuf;
 
 	struct cdev cdev;
@@ -171,6 +172,7 @@ static int nand_bb_open(struct cdev *cdev, unsigned long flags)
 	if (bb->open)
 		return -EBUSY;
 
+	bb->flags = flags;
 	bb->open = 1;
 	bb->offset = 0;
 	bb->needs_write = 0;
@@ -211,10 +213,40 @@ static int nand_bb_calc_size(struct nand_bb *bb)
 	return 0;
 }
 
+static off_t nand_bb_lseek(struct cdev *cdev, off_t __offset)
+{
+	struct nand_bb *bb = cdev->priv;
+	unsigned long raw_pos = 0;
+	uint32_t offset = __offset;
+	int ret;
+
+	/* lseek only in readonly mode */
+	if (bb->flags & O_ACCMODE)
+		return -ENOSYS;
+
+	while (raw_pos < bb->raw_size) {
+		off_t now = min(offset, bb->info.erasesize);
+
+		ret = cdev_ioctl(bb->cdev_parent, MEMGETBADBLOCK, (void *)raw_pos);
+		if (ret < 0)
+			return ret;
+		if (!ret)
+			offset -= now;
+		raw_pos += now;
+		if (!offset) {
+			bb->offset = raw_pos;
+			return __offset;
+		}
+	}
+
+	return -EINVAL;
+}
+
 static struct file_operations nand_bb_ops = {
 	.open   = nand_bb_open,
 	.close  = nand_bb_close,
 	.read  	= nand_bb_read,
+	.lseek	= nand_bb_lseek,
 #ifdef CONFIG_NAND_WRITE
 	.write 	= nand_bb_write,
 	.erase	= nand_bb_erase,
-- 
1.7.7.3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 7/7] nand-bb: implement lseek in readonly mode

[Robert Jarzmik]

Sascha Hauer <s.hauer@pengutronix.de> writes:

> +static off_t nand_bb_lseek(struct cdev *cdev, off_t __offset)
> +{
> +	struct nand_bb *bb = cdev->priv;
> +	unsigned long raw_pos = 0;
> +	uint32_t offset = __offset;
> +	int ret;
> +
> +	/* lseek only in readonly mode */
> +	if (bb->flags & O_ACCMODE)
> +		return -ENOSYS;
> +
> +	while (raw_pos < bb->raw_size) {
> +		off_t now = min(offset, bb->info.erasesize);
> +
> +		ret = cdev_ioctl(bb->cdev_parent, MEMGETBADBLOCK, (void *)raw_pos);
> +		if (ret < 0)
> +			return ret;
> +		if (!ret)
> +			offset -= now;
> +		raw_pos += now;
> +		if (!offset) {
> +			bb->offset = raw_pos;
> +			return __offset;
> +		}
> +	}
Are you sure of this algorithm ?
I tried to check it with:
 - erasesize=16 (silly I know, but it's simpler for my mind)
 - bb->raw_size = +oo
 - offset = 34

Let's assume we have eraseblock B0, B1, B2 and B3 of 16 bytes.
B0, B1 and B3 are good, B2 is a bad block.

+--------+--------+--------+--------+
|   B0   |   B1   |xxxB2xxx|   B3   |
+--------+--------+--------+--------+

If I unroll the while loop:
 - loop1:
     now=16
     ret=0 (B0 good)
     offset = 18
     raw_pos = 16
 - loop2:
     now=16
     ret=0 (B1 good)
     offset = 2
     raw_pos = 32
 - loop3:
     now=2
     ret=1 (B2 bad)
     offset = 2
     raw_pos = 34
 - loop4:
     now=2
     ret=0 (B3 good)
     offset = 0
     raw_pos = 36
     bb->offset = 36
     return 34

So we end up with bb->offset = 36, which seems incorrect to me. I would have
understood a value of 50, but 36 ... I don't

Cheers.

-- 
Robert

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 7/7] nand-bb: implement lseek in readonly mode

[Sascha Hauer]

On Thu, Dec 15, 2011 at 02:51:40PM +0100, Robert Jarzmik wrote:
> Sascha Hauer <s.hauer@pengutronix.de> writes:
> 
> > +static off_t nand_bb_lseek(struct cdev *cdev, off_t __offset)
> > +{
> > +	struct nand_bb *bb = cdev->priv;
> > +	unsigned long raw_pos = 0;
> > +	uint32_t offset = __offset;
> > +	int ret;
> > +
> > +	/* lseek only in readonly mode */
> > +	if (bb->flags & O_ACCMODE)
> > +		return -ENOSYS;
> > +
> > +	while (raw_pos < bb->raw_size) {
> > +		off_t now = min(offset, bb->info.erasesize);
> > +
> > +		ret = cdev_ioctl(bb->cdev_parent, MEMGETBADBLOCK, (void *)raw_pos);
> > +		if (ret < 0)
> > +			return ret;
> > +		if (!ret)
> > +			offset -= now;
> > +		raw_pos += now;
> > +		if (!offset) {
> > +			bb->offset = raw_pos;
> > +			return __offset;
> > +		}
> > +	}
> Are you sure of this algorithm ?
> I tried to check it with:
>  - erasesize=16 (silly I know, but it's simpler for my mind)
>  - bb->raw_size = +oo
>  - offset = 34
> 
> Let's assume we have eraseblock B0, B1, B2 and B3 of 16 bytes.
> B0, B1 and B3 are good, B2 is a bad block.
> 
> +--------+--------+--------+--------+
> |   B0   |   B1   |xxxB2xxx|   B3   |
> +--------+--------+--------+--------+
> 
> If I unroll the while loop:
>  - loop1:
>      now=16
>      ret=0 (B0 good)
>      offset = 18
>      raw_pos = 16
>  - loop2:
>      now=16
>      ret=0 (B1 good)
>      offset = 2
>      raw_pos = 32
>  - loop3:
>      now=2
>      ret=1 (B2 bad)
>      offset = 2
>      raw_pos = 34

Yup, that's wrong. We have to add 16 to raw_pos here, not 2.

>  - loop4:
>      now=2
>      ret=0 (B3 good)
>      offset = 0
>      raw_pos = 36
>      bb->offset = 36
>      return 34

Funny enough that my code actually works. In loop4 we have ret=1
because we are still on B2 (raw_pos is 34, -> still in block 2).
Now the code loops over Block2 in small steps until it comes to
block3 and returns with the correct raw offset, so I didn't see
this in my tests.

> 
> So we end up with bb->offset = 36, which seems incorrect to me. I would have
> understood a value of 50, but 36 ... I don't

This one should work like expected:

	while (raw_pos < bb->raw_size) {
		off_t now = min(offset, bb->info.erasesize);

		ret = cdev_ioctl(bb->cdev_parent, MEMGETBADBLOCK, (void *)raw_pos);
		if (ret < 0)
			return ret;
		if (!ret) {
			offset -= now;
			raw_pos += now;
		} else {
			raw_pos += bb->info.erasesize
		}
		if (!offset) {
			bb->offset = raw_pos;
			return __offset;
		}
	}

loop1:
	now = 16
	ret = 0
	offset = 18
	raw_pos = 16
loop2:
	now = 16
	ret = 0
	offset = 2
	raw_pos = 32
loop3:
	now = 2
	ret = 1
	offset = 2
	raw_pos = 48
loop4:
	now = 2
	ret = 0
	offset = 0
	raw_pos = 50

Thanks for catching this.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 7/7] nand-bb: implement lseek in readonly mode

[Robert Jarzmik]

Sascha Hauer <s.hauer@pengutronix.de> writes:

> This one should work like expected:
>
> 	while (raw_pos < bb->raw_size) {
> 		off_t now = min(offset, bb->info.erasesize);
>
> 		ret = cdev_ioctl(bb->cdev_parent, MEMGETBADBLOCK, (void *)raw_pos);
> 		if (ret < 0)
> 			return ret;
> 		if (!ret) {
> 			offset -= now;
> 			raw_pos += now;
> 		} else {
> 			raw_pos += bb->info.erasesize
> 		}
> 		if (!offset) {
> 			bb->offset = raw_pos;
> 			return __offset;
> 		}
> 	}
Yup, looks good.
If you wish, you can add my :
Reviewed-by: Robert Jarzmik <robert.jarzmik@free.fr>

Cheers.

--
Robert

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox