[U-Boot] [PATCH] mtd: Update MTD infrastructure to support 64bit device size
This patch brings the U-Boot MTD infrastructure in sync with the current Linux MTD version (2.6.30-rc3). Biggest change is the 64bit device size support and a resync of the mtdpart.c file which has seen multiple fixes meanwhile.
Signed-off-by: Stefan Roese sr@denx.de Cc: Scott Wood scottwood@freescale.com Cc: Kyungmin Park kmpark@infradead.org --- common/cmd_onenand.c | 6 +- common/env_onenand.c | 2 +- drivers/mtd/cfi_mtd.c | 4 +- drivers/mtd/mtdpart.c | 488 +++++++++++++++++++----------------------- drivers/mtd/nand/nand_util.c | 11 +- include/linux/mtd/mtd.h | 29 ++- 6 files changed, 256 insertions(+), 284 deletions(-)
diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 5832ff8..e8f06d7 100644 --- a/common/cmd_onenand.c +++ b/common/cmd_onenand.c @@ -19,6 +19,10 @@
#include <asm/io.h>
+#if !defined(CONFIG_SYS_64BIT_VSPRINTF) +#warning Please define CONFIG_SYS_64BIT_VSPRINTF for correct output! +#endif + static struct mtd_info *mtd;
static loff_t next_ofs; @@ -53,7 +57,7 @@ static int arg_off_size(int argc, char *argv[], ulong *off, size_t *size) }
if ((*off + *size) > mtd->size) { - printf("total chip size (0x%x) exceeded!\n", mtd->size); + printf("total chip size (0x%llx) exceeded!\n", mtd->size); return -1; }
diff --git a/common/env_onenand.c b/common/env_onenand.c index dbccc79..ed77051 100644 --- a/common/env_onenand.c +++ b/common/env_onenand.c @@ -109,7 +109,7 @@ int saveenv(void)
if (onenand_write(&onenand_mtd, env_addr, onenand_mtd.writesize, &retlen, (u_char *) env_ptr)) { - printf("OneNAND: write failed at 0x%08x\n", instr.addr); + printf("OneNAND: write failed at 0x%llx\n", instr.addr); return 2; }
diff --git a/drivers/mtd/cfi_mtd.c b/drivers/mtd/cfi_mtd.c index 4a76917..a0a15da 100644 --- a/drivers/mtd/cfi_mtd.c +++ b/drivers/mtd/cfi_mtd.c @@ -118,7 +118,7 @@ static void cfi_mtd_sync(struct mtd_info *mtd) */ }
-static int cfi_mtd_lock(struct mtd_info *mtd, loff_t ofs, size_t len) +static int cfi_mtd_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len) { flash_info_t *fi = mtd->priv;
@@ -130,7 +130,7 @@ static int cfi_mtd_lock(struct mtd_info *mtd, loff_t ofs, size_t len) return 0; }
-static int cfi_mtd_unlock(struct mtd_info *mtd, loff_t ofs, size_t len) +static int cfi_mtd_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) { flash_info_t *fi = mtd->priv;
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c index f010f5e..e2e43ea 100644 --- a/drivers/mtd/mtdpart.c +++ b/drivers/mtd/mtdpart.c @@ -26,7 +26,7 @@ struct list_head mtd_partitions; struct mtd_part { struct mtd_info mtd; struct mtd_info *master; - u_int32_t offset; + uint64_t offset; int index; struct list_head list; int registered; @@ -44,50 +44,32 @@ struct mtd_part { * to the _real_ device. */
-static int part_read (struct mtd_info *mtd, loff_t from, size_t len, - size_t *retlen, u_char *buf) +static int part_read(struct mtd_info *mtd, loff_t from, size_t len, + size_t *retlen, u_char *buf) { struct mtd_part *part = PART(mtd); + struct mtd_ecc_stats stats; int res;
+ stats = part->master->ecc_stats; + if (from >= mtd->size) len = 0; else if (from + len > mtd->size) len = mtd->size - from; - res = part->master->read (part->master, from + part->offset, + res = part->master->read(part->master, from + part->offset, len, retlen, buf); if (unlikely(res)) { if (res == -EUCLEAN) - mtd->ecc_stats.corrected++; + mtd->ecc_stats.corrected += part->master->ecc_stats.corrected - stats.corrected; if (res == -EBADMSG) - mtd->ecc_stats.failed++; + mtd->ecc_stats.failed += part->master->ecc_stats.failed - stats.failed; } return res; }
-#ifdef MTD_LINUX -static int part_point (struct mtd_info *mtd, loff_t from, size_t len, - size_t *retlen, void **virt, resource_size_t *phys) -{ - struct mtd_part *part = PART(mtd); - if (from >= mtd->size) - len = 0; - else if (from + len > mtd->size) - len = mtd->size - from; - return part->master->point (part->master, from + part->offset, - len, retlen, virt, phys); -} - -static void part_unpoint(struct mtd_info *mtd, loff_t from, size_t len) -{ - struct mtd_part *part = PART(mtd); - - part->master->unpoint(part->master, from + part->offset, len); -} -#endif - static int part_read_oob(struct mtd_info *mtd, loff_t from, - struct mtd_oob_ops *ops) + struct mtd_oob_ops *ops) { struct mtd_part *part = PART(mtd); int res; @@ -107,38 +89,38 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from, return res; }
-static int part_read_user_prot_reg (struct mtd_info *mtd, loff_t from, size_t len, - size_t *retlen, u_char *buf) +static int part_read_user_prot_reg(struct mtd_info *mtd, loff_t from, + size_t len, size_t *retlen, u_char *buf) { struct mtd_part *part = PART(mtd); - return part->master->read_user_prot_reg (part->master, from, + return part->master->read_user_prot_reg(part->master, from, len, retlen, buf); }
-static int part_get_user_prot_info (struct mtd_info *mtd, - struct otp_info *buf, size_t len) +static int part_get_user_prot_info(struct mtd_info *mtd, + struct otp_info *buf, size_t len) { struct mtd_part *part = PART(mtd); - return part->master->get_user_prot_info (part->master, buf, len); + return part->master->get_user_prot_info(part->master, buf, len); }
-static int part_read_fact_prot_reg (struct mtd_info *mtd, loff_t from, size_t len, - size_t *retlen, u_char *buf) +static int part_read_fact_prot_reg(struct mtd_info *mtd, loff_t from, + size_t len, size_t *retlen, u_char *buf) { struct mtd_part *part = PART(mtd); - return part->master->read_fact_prot_reg (part->master, from, + return part->master->read_fact_prot_reg(part->master, from, len, retlen, buf); }
-static int part_get_fact_prot_info (struct mtd_info *mtd, - struct otp_info *buf, size_t len) +static int part_get_fact_prot_info(struct mtd_info *mtd, struct otp_info *buf, + size_t len) { struct mtd_part *part = PART(mtd); - return part->master->get_fact_prot_info (part->master, buf, len); + return part->master->get_fact_prot_info(part->master, buf, len); }
-static int part_write (struct mtd_info *mtd, loff_t to, size_t len, - size_t *retlen, const u_char *buf) +static int part_write(struct mtd_info *mtd, loff_t to, size_t len, + size_t *retlen, const u_char *buf) { struct mtd_part *part = PART(mtd); if (!(mtd->flags & MTD_WRITEABLE)) @@ -147,13 +129,12 @@ static int part_write (struct mtd_info *mtd, loff_t to, size_t len, len = 0; else if (to + len > mtd->size) len = mtd->size - to; - return part->master->write (part->master, to + part->offset, + return part->master->write(part->master, to + part->offset, len, retlen, buf); }
-#ifdef MTD_LINUX -static int part_panic_write (struct mtd_info *mtd, loff_t to, size_t len, - size_t *retlen, const u_char *buf) +static int part_panic_write(struct mtd_info *mtd, loff_t to, size_t len, + size_t *retlen, const u_char *buf) { struct mtd_part *part = PART(mtd); if (!(mtd->flags & MTD_WRITEABLE)) @@ -162,13 +143,12 @@ static int part_panic_write (struct mtd_info *mtd, loff_t to, size_t len, len = 0; else if (to + len > mtd->size) len = mtd->size - to; - return part->master->panic_write (part->master, to + part->offset, + return part->master->panic_write(part->master, to + part->offset, len, retlen, buf); } -#endif
static int part_write_oob(struct mtd_info *mtd, loff_t to, - struct mtd_oob_ops *ops) + struct mtd_oob_ops *ops) { struct mtd_part *part = PART(mtd);
@@ -182,33 +162,22 @@ static int part_write_oob(struct mtd_info *mtd, loff_t to, return part->master->write_oob(part->master, to + part->offset, ops); }
-static int part_write_user_prot_reg (struct mtd_info *mtd, loff_t from, size_t len, - size_t *retlen, u_char *buf) +static int part_write_user_prot_reg(struct mtd_info *mtd, loff_t from, + size_t len, size_t *retlen, u_char *buf) { struct mtd_part *part = PART(mtd); - return part->master->write_user_prot_reg (part->master, from, + return part->master->write_user_prot_reg(part->master, from, len, retlen, buf); }
-static int part_lock_user_prot_reg (struct mtd_info *mtd, loff_t from, size_t len) -{ - struct mtd_part *part = PART(mtd); - return part->master->lock_user_prot_reg (part->master, from, len); -} - -#ifdef MTD_LINUX -static int part_writev (struct mtd_info *mtd, const struct kvec *vecs, - unsigned long count, loff_t to, size_t *retlen) +static int part_lock_user_prot_reg(struct mtd_info *mtd, loff_t from, + size_t len) { struct mtd_part *part = PART(mtd); - if (!(mtd->flags & MTD_WRITEABLE)) - return -EROFS; - return part->master->writev (part->master, vecs, count, - to + part->offset, retlen); + return part->master->lock_user_prot_reg(part->master, from, len); } -#endif
-static int part_erase (struct mtd_info *mtd, struct erase_info *instr) +static int part_erase(struct mtd_info *mtd, struct erase_info *instr) { struct mtd_part *part = PART(mtd); int ret; @@ -219,7 +188,7 @@ static int part_erase (struct mtd_info *mtd, struct erase_info *instr) instr->addr += part->offset; ret = part->master->erase(part->master, instr); if (ret) { - if (instr->fail_addr != 0xffffffff) + if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN) instr->fail_addr -= part->offset; instr->addr -= part->offset; } @@ -231,19 +200,15 @@ void mtd_erase_callback(struct erase_info *instr) if (instr->mtd->erase == part_erase) { struct mtd_part *part = PART(instr->mtd);
- if (instr->fail_addr != 0xffffffff) + if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN) instr->fail_addr -= part->offset; instr->addr -= part->offset; } if (instr->callback) instr->callback(instr); } -#ifdef MTD_LINUX -EXPORT_SYMBOL_GPL(mtd_erase_callback); -#endif
-#ifdef MTD_LINUX -static int part_lock (struct mtd_info *mtd, loff_t ofs, size_t len) +static int part_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len) { struct mtd_part *part = PART(mtd); if ((len + ofs) > mtd->size) @@ -251,14 +216,13 @@ static int part_lock (struct mtd_info *mtd, loff_t ofs, size_t len) return part->master->lock(part->master, ofs + part->offset, len); }
-static int part_unlock (struct mtd_info *mtd, loff_t ofs, size_t len) +static int part_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len) { struct mtd_part *part = PART(mtd); if ((len + ofs) > mtd->size) return -EINVAL; return part->master->unlock(part->master, ofs + part->offset, len); } -#endif
static void part_sync(struct mtd_info *mtd) { @@ -266,7 +230,6 @@ static void part_sync(struct mtd_info *mtd) part->master->sync(part->master); }
-#ifdef MTD_LINUX static int part_suspend(struct mtd_info *mtd) { struct mtd_part *part = PART(mtd); @@ -278,9 +241,8 @@ static void part_resume(struct mtd_info *mtd) struct mtd_part *part = PART(mtd); part->master->resume(part->master); } -#endif
-static int part_block_isbad (struct mtd_info *mtd, loff_t ofs) +static int part_block_isbad(struct mtd_info *mtd, loff_t ofs) { struct mtd_part *part = PART(mtd); if (ofs >= mtd->size) @@ -289,7 +251,7 @@ static int part_block_isbad (struct mtd_info *mtd, loff_t ofs) return part->master->block_isbad(part->master, ofs); }
-static int part_block_markbad (struct mtd_info *mtd, loff_t ofs) +static int part_block_markbad(struct mtd_info *mtd, loff_t ofs) { struct mtd_part *part = PART(mtd); int res; @@ -300,10 +262,8 @@ static int part_block_markbad (struct mtd_info *mtd, loff_t ofs) return -EINVAL; ofs += part->offset; res = part->master->block_markbad(part->master, ofs); -#ifdef MTD_LINUX if (!res) mtd->ecc_stats.badblocks++; -#endif return res; }
@@ -314,31 +274,193 @@ static int part_block_markbad (struct mtd_info *mtd, loff_t ofs)
int del_mtd_partitions(struct mtd_info *master) { - struct list_head *node; - struct mtd_part *slave; + struct mtd_part *slave, *next;
- for (node = mtd_partitions.next; - node != &mtd_partitions; - node = node->next) { - slave = list_entry(node, struct mtd_part, list); + list_for_each_entry_safe(slave, next, &mtd_partitions, list) if (slave->master == master) { - struct list_head *prev = node->prev; - __list_del(prev, node->next); - if(slave->registered) + list_del(&slave->list); + if (slave->registered) del_mtd_device(&slave->mtd); kfree(slave); - node = prev; } - }
return 0; }
+static struct mtd_part *add_one_partition(struct mtd_info *master, + const struct mtd_partition *part, int partno, + uint64_t cur_offset) +{ + struct mtd_part *slave; + + /* allocate the partition structure */ + slave = kzalloc(sizeof(*slave), GFP_KERNEL); + if (!slave) { + printk(KERN_ERR"memory allocation error while creating partitions for "%s"\n", + master->name); + del_mtd_partitions(master); + return NULL; + } + list_add(&slave->list, &mtd_partitions); + + /* set up the MTD object for this partition */ + slave->mtd.type = master->type; + slave->mtd.flags = master->flags & ~part->mask_flags; + slave->mtd.size = part->size; + slave->mtd.writesize = master->writesize; + slave->mtd.oobsize = master->oobsize; + slave->mtd.oobavail = master->oobavail; + slave->mtd.subpage_sft = master->subpage_sft; + + slave->mtd.name = part->name; + slave->mtd.owner = master->owner; + + slave->mtd.read = part_read; + slave->mtd.write = part_write; + + if (master->panic_write) + slave->mtd.panic_write = part_panic_write; + + if (master->read_oob) + slave->mtd.read_oob = part_read_oob; + if (master->write_oob) + slave->mtd.write_oob = part_write_oob; + if (master->read_user_prot_reg) + slave->mtd.read_user_prot_reg = part_read_user_prot_reg; + if (master->read_fact_prot_reg) + slave->mtd.read_fact_prot_reg = part_read_fact_prot_reg; + if (master->write_user_prot_reg) + slave->mtd.write_user_prot_reg = part_write_user_prot_reg; + if (master->lock_user_prot_reg) + slave->mtd.lock_user_prot_reg = part_lock_user_prot_reg; + if (master->get_user_prot_info) + slave->mtd.get_user_prot_info = part_get_user_prot_info; + if (master->get_fact_prot_info) + slave->mtd.get_fact_prot_info = part_get_fact_prot_info; + if (master->sync) + slave->mtd.sync = part_sync; + if (!partno && master->suspend && master->resume) { + slave->mtd.suspend = part_suspend; + slave->mtd.resume = part_resume; + } + if (master->lock) + slave->mtd.lock = part_lock; + if (master->unlock) + slave->mtd.unlock = part_unlock; + if (master->block_isbad) + slave->mtd.block_isbad = part_block_isbad; + if (master->block_markbad) + slave->mtd.block_markbad = part_block_markbad; + slave->mtd.erase = part_erase; + slave->master = master; + slave->offset = part->offset; + slave->index = partno; + + if (slave->offset == MTDPART_OFS_APPEND) + slave->offset = cur_offset; + if (slave->offset == MTDPART_OFS_NXTBLK) { + slave->offset = cur_offset; + if (mtd_mod_by_eb(cur_offset, master) != 0) { + /* Round up to next erasesize */ + slave->offset = (mtd_div_by_eb(cur_offset, master) + 1) * master->erasesize; + printk(KERN_NOTICE "Moving partition %d: " + "0x%012llx -> 0x%012llx\n", partno, + (unsigned long long)cur_offset, (unsigned long long)slave->offset); + } + } + if (slave->mtd.size == MTDPART_SIZ_FULL) + slave->mtd.size = master->size - slave->offset; + + printk(KERN_NOTICE "0x%012llx-0x%012llx : "%s"\n", (unsigned long long)slave->offset, + (unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name); + + /* let's do some sanity checks */ + if (slave->offset >= master->size) { + /* let's register it anyway to preserve ordering */ + slave->offset = 0; + slave->mtd.size = 0; + printk(KERN_ERR"mtd: partition "%s" is out of reach -- disabled\n", + part->name); + goto out_register; + } + if (slave->offset + slave->mtd.size > master->size) { + slave->mtd.size = master->size - slave->offset; + printk(KERN_WARNING"mtd: partition "%s" extends beyond the end of device "%s" -- size truncated to %#llx\n", + part->name, master->name, (unsigned long long)slave->mtd.size); + } + if (master->numeraseregions > 1) { + /* Deal with variable erase size stuff */ + int i, max = master->numeraseregions; + u64 end = slave->offset + slave->mtd.size; + struct mtd_erase_region_info *regions = master->eraseregions; + + /* Find the first erase regions which is part of this + * partition. */ + for (i = 0; i < max && regions[i].offset <= slave->offset; i++) + ; + /* The loop searched for the region _behind_ the first one */ + i--; + + /* Pick biggest erasesize */ + for (; i < max && regions[i].offset < end; i++) { + if (slave->mtd.erasesize < regions[i].erasesize) { + slave->mtd.erasesize = regions[i].erasesize; + } + } + BUG_ON(slave->mtd.erasesize == 0); + } else { + /* Single erase size */ + slave->mtd.erasesize = master->erasesize; + } + + if ((slave->mtd.flags & MTD_WRITEABLE) && + mtd_mod_by_eb(slave->offset, &slave->mtd)) { + /* Doesn't start on a boundary of major erase size */ + /* FIXME: Let it be writable if it is on a boundary of + * _minor_ erase size though */ + slave->mtd.flags &= ~MTD_WRITEABLE; + printk(KERN_WARNING"mtd: partition "%s" doesn't start on an erase block boundary -- force read-only\n", + part->name); + } + if ((slave->mtd.flags & MTD_WRITEABLE) && + mtd_mod_by_eb(slave->mtd.size, &slave->mtd)) { + slave->mtd.flags &= ~MTD_WRITEABLE; + printk(KERN_WARNING"mtd: partition "%s" doesn't end on an erase block -- force read-only\n", + part->name); + } + + slave->mtd.ecclayout = master->ecclayout; + if (master->block_isbad) { + uint64_t offs = 0; + + while (offs < slave->mtd.size) { + if (master->block_isbad(master, + offs + slave->offset)) + slave->mtd.ecc_stats.badblocks++; + offs += slave->mtd.erasesize; + } + } + +out_register: + if (part->mtdp) { + /* store the object pointer (caller may or may not register it*/ + *part->mtdp = &slave->mtd; + slave->registered = 0; + } else { + /* register our partition */ + add_mtd_device(&slave->mtd); + slave->registered = 1; + } + return slave; +} + /* * This function, given a master MTD object and a partition table, creates * and registers slave MTD objects which are bound to the master according to * the partition definitions. - * (Q: should we register the master MTD object as well?) + * + * We don't register the master, or expect the caller to have done so, + * for reasons of data integrity. */
int add_mtd_partitions(struct mtd_info *master, @@ -346,7 +468,7 @@ int add_mtd_partitions(struct mtd_info *master, int nbparts) { struct mtd_part *slave; - u_int32_t cur_offset = 0; + uint64_t cur_offset = 0; int i;
/* @@ -357,184 +479,14 @@ int add_mtd_partitions(struct mtd_info *master, if (mtd_partitions.next == NULL) INIT_LIST_HEAD(&mtd_partitions);
- printk (KERN_NOTICE "Creating %d MTD partitions on "%s":\n", nbparts, master->name); + printk(KERN_NOTICE "Creating %d MTD partitions on "%s":\n", nbparts, master->name);
for (i = 0; i < nbparts; i++) { - - /* allocate the partition structure */ - slave = kzalloc (sizeof(*slave), GFP_KERNEL); - if (!slave) { - printk ("memory allocation error while creating partitions for "%s"\n", - master->name); - del_mtd_partitions(master); + slave = add_one_partition(master, parts + i, i, cur_offset); + if (!slave) return -ENOMEM; - } - list_add(&slave->list, &mtd_partitions); - - /* set up the MTD object for this partition */ - slave->mtd.type = master->type; - slave->mtd.flags = master->flags & ~parts[i].mask_flags; - slave->mtd.size = parts[i].size; - slave->mtd.writesize = master->writesize; - slave->mtd.oobsize = master->oobsize; - slave->mtd.oobavail = master->oobavail; - slave->mtd.subpage_sft = master->subpage_sft; - - slave->mtd.name = parts[i].name; - slave->mtd.owner = master->owner; - - slave->mtd.read = part_read; - slave->mtd.write = part_write; - -#ifdef MTD_LINUX - if (master->panic_write) - slave->mtd.panic_write = part_panic_write; - - if(master->point && master->unpoint){ - slave->mtd.point = part_point; - slave->mtd.unpoint = part_unpoint; - } -#endif - - if (master->read_oob) - slave->mtd.read_oob = part_read_oob; - if (master->write_oob) - slave->mtd.write_oob = part_write_oob; - if(master->read_user_prot_reg) - slave->mtd.read_user_prot_reg = part_read_user_prot_reg; - if(master->read_fact_prot_reg) - slave->mtd.read_fact_prot_reg = part_read_fact_prot_reg; - if(master->write_user_prot_reg) - slave->mtd.write_user_prot_reg = part_write_user_prot_reg; - if(master->lock_user_prot_reg) - slave->mtd.lock_user_prot_reg = part_lock_user_prot_reg; - if(master->get_user_prot_info) - slave->mtd.get_user_prot_info = part_get_user_prot_info; - if(master->get_fact_prot_info) - slave->mtd.get_fact_prot_info = part_get_fact_prot_info; - if (master->sync) - slave->mtd.sync = part_sync; -#ifdef MTD_LINUX - if (!i && master->suspend && master->resume) { - slave->mtd.suspend = part_suspend; - slave->mtd.resume = part_resume; - } - if (master->writev) - slave->mtd.writev = part_writev; - if (master->lock) - slave->mtd.lock = part_lock; - if (master->unlock) - slave->mtd.unlock = part_unlock; -#endif - if (master->block_isbad) - slave->mtd.block_isbad = part_block_isbad; - if (master->block_markbad) - slave->mtd.block_markbad = part_block_markbad; - slave->mtd.erase = part_erase; - slave->master = master; - slave->offset = parts[i].offset; - slave->index = i; - - if (slave->offset == MTDPART_OFS_APPEND) - slave->offset = cur_offset; - if (slave->offset == MTDPART_OFS_NXTBLK) { - slave->offset = cur_offset; - if ((cur_offset % master->erasesize) != 0) { - /* Round up to next erasesize */ - slave->offset = ((cur_offset / master->erasesize) + 1) * master->erasesize; - printk(KERN_NOTICE "Moving partition %d: " - "0x%08x -> 0x%08x\n", i, - cur_offset, slave->offset); - } - } - if (slave->mtd.size == MTDPART_SIZ_FULL) - slave->mtd.size = master->size - slave->offset; cur_offset = slave->offset + slave->mtd.size; - - printk (KERN_NOTICE "0x%08x-0x%08x : "%s"\n", slave->offset, - slave->offset + slave->mtd.size, slave->mtd.name); - - /* let's do some sanity checks */ - if (slave->offset >= master->size) { - /* let's register it anyway to preserve ordering */ - slave->offset = 0; - slave->mtd.size = 0; - printk ("mtd: partition "%s" is out of reach -- disabled\n", - parts[i].name); - } - if (slave->offset + slave->mtd.size > master->size) { - slave->mtd.size = master->size - slave->offset; - printk ("mtd: partition "%s" extends beyond the end of device "%s" -- size truncated to %#x\n", - parts[i].name, master->name, slave->mtd.size); - } - if (master->numeraseregions>1) { - /* Deal with variable erase size stuff */ - int i; - struct mtd_erase_region_info *regions = master->eraseregions; - - /* Find the first erase regions which is part of this partition. */ - for (i=0; i < master->numeraseregions && slave->offset >= regions[i].offset; i++) - ; - - for (i--; i < master->numeraseregions && slave->offset + slave->mtd.size > regions[i].offset; i++) { - if (slave->mtd.erasesize < regions[i].erasesize) { - slave->mtd.erasesize = regions[i].erasesize; - } - } - } else { - /* Single erase size */ - slave->mtd.erasesize = master->erasesize; - } - - if ((slave->mtd.flags & MTD_WRITEABLE) && - (slave->offset % slave->mtd.erasesize)) { - /* Doesn't start on a boundary of major erase size */ - /* FIXME: Let it be writable if it is on a boundary of _minor_ erase size though */ - slave->mtd.flags &= ~MTD_WRITEABLE; - printk ("mtd: partition "%s" doesn't start on an erase block boundary -- force read-only\n", - parts[i].name); - } - if ((slave->mtd.flags & MTD_WRITEABLE) && - (slave->mtd.size % slave->mtd.erasesize)) { - slave->mtd.flags &= ~MTD_WRITEABLE; - printk ("mtd: partition "%s" doesn't end on an erase block -- force read-only\n", - parts[i].name); - } - - slave->mtd.ecclayout = master->ecclayout; - if (master->block_isbad) { - uint32_t offs = 0; - - while(offs < slave->mtd.size) { - if (master->block_isbad(master, - offs + slave->offset)) - slave->mtd.ecc_stats.badblocks++; - offs += slave->mtd.erasesize; - } - } - -#ifdef MTD_LINUX - if (parts[i].mtdp) { - /* store the object pointer - * (caller may or may not register it */ - *parts[i].mtdp = &slave->mtd; - slave->registered = 0; - } else { - /* register our partition */ - add_mtd_device(&slave->mtd); - slave->registered = 1; - } -#else - /* register our partition */ - add_mtd_device(&slave->mtd); - slave->registered = 1; -#endif }
return 0; } - -#ifdef MTD_LINUX -EXPORT_SYMBOL(add_mtd_partitions); -EXPORT_SYMBOL(del_mtd_partitions); -#endif diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index 6ba52b3..88206d0 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -36,12 +36,15 @@ #include <malloc.h> #include <div64.h>
- #include <asm/errno.h> #include <linux/mtd/mtd.h> #include <nand.h> #include <jffs2/jffs2.h>
+#if !defined(CONFIG_SYS_64BIT_VSPRINTF) +#warning Please define CONFIG_SYS_64BIT_VSPRINTF for correct output! +#endif + typedef struct erase_info erase_info_t; typedef struct mtd_info mtd_info_t;
@@ -127,7 +130,7 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts) if (ret > 0) { if (!opts->quiet) printf("\rSkipping bad block at " - "0x%08x " + "0x%08llx " " \n", erase.addr); continue; @@ -181,11 +184,11 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts) if (percent != percent_complete) { percent_complete = percent;
- printf("\rErasing at 0x%x -- %3d%% complete.", + printf("\rErasing at 0x%llx -- %3d%% complete.", erase.addr, percent);
if (opts->jffs2 && result == 0) - printf(" Cleanmarker written at 0x%x.", + printf(" Cleanmarker written at 0x%llx.", erase.addr); } } diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index 354e3a0..c884567 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -8,6 +8,7 @@ #define __MTD_MTD_H__
#include <linux/types.h> +#include <div64.h> #include <linux/mtd/mtd-abi.h>
#define MTD_CHAR_MAJOR 90 @@ -20,6 +21,8 @@ #define MTD_ERASE_DONE 0x08 #define MTD_ERASE_FAILED 0x10
+#define MTD_FAIL_ADDR_UNKNOWN -1LL + /* * Enumeration for NAND/OneNAND flash chip state */ @@ -37,13 +40,13 @@ enum { };
/* If the erase fails, fail_addr might indicate exactly which block failed. If - fail_addr = 0xffffffff, the failure was not at the device level or was not + fail_addr = MTD_FAIL_ADDR_UNKNOWN, the failure was not at the device level or was not specific to any particular block. */ struct erase_info { struct mtd_info *mtd; - u_int32_t addr; - u_int32_t len; - u_int32_t fail_addr; + uint64_t addr; + uint64_t len; + uint64_t fail_addr; u_long time; u_long retries; u_int dev; @@ -55,7 +58,7 @@ struct erase_info { };
struct mtd_erase_region_info { - u_int32_t offset; /* At which this region starts, from the beginning of the MTD */ + uint64_t offset; /* At which this region starts, from the beginning of the MTD */ u_int32_t erasesize; /* For this region */ u_int32_t numblocks; /* Number of blocks of erasesize in this region */ unsigned long *lockmap; /* If keeping bitmap of locks */ @@ -110,7 +113,7 @@ struct mtd_oob_ops { struct mtd_info { u_char type; u_int32_t flags; - u_int32_t size; /* Total size of the MTD */ + uint64_t size; // Total size of the MTD
/* "Major" erase size for the device. Naïve users may take this * to be the only erase size available, or may use the more detailed @@ -202,8 +205,8 @@ struct mtd_info { void (*sync) (struct mtd_info *mtd);
/* Chip-supported device locking */ - int (*lock) (struct mtd_info *mtd, loff_t ofs, size_t len); - int (*unlock) (struct mtd_info *mtd, loff_t ofs, size_t len); + int (*lock) (struct mtd_info *mtd, loff_t ofs, uint64_t len); + int (*unlock) (struct mtd_info *mtd, loff_t ofs, uint64_t len);
/* Power Management functions */ int (*suspend) (struct mtd_info *mtd); @@ -236,6 +239,16 @@ struct mtd_info { void (*put_device) (struct mtd_info *mtd); };
+static inline uint32_t mtd_div_by_eb(uint64_t sz, struct mtd_info *mtd) +{ + do_div(sz, mtd->erasesize); + return sz; +} + +static inline uint32_t mtd_mod_by_eb(uint64_t sz, struct mtd_info *mtd) +{ + return do_div(sz, mtd->erasesize); +}
/* Kernel-side ioctl definitions */
On Monday 11 May 2009 10:03:55 Stefan Roese wrote:
This patch brings the U-Boot MTD infrastructure in sync with the current Linux MTD version (2.6.30-rc3). Biggest change is the 64bit device size support and a resync of the mtdpart.c file which has seen multiple fixes meanwhile.
really wish there was a define to control this. large devices are fine, but it sucks when majority of people dont hit this limit. -mike
On Monday 11 May 2009 18:27:02 Mike Frysinger wrote:
On Monday 11 May 2009 10:03:55 Stefan Roese wrote:
This patch brings the U-Boot MTD infrastructure in sync with the current Linux MTD version (2.6.30-rc3). Biggest change is the 64bit device size support and a resync of the mtdpart.c file which has seen multiple fixes meanwhile.
really wish there was a define to control this. large devices are fine, but it sucks when majority of people dont hit this limit.
What is the drawback of this code version? I have to admit that I didn't compare code size, but this is the only possible drawback coming to my mind. And the advantages are big enough from my point of view. Being in sync with Linux once again not only brings 64bit device size support but removes quite a lot of bugs from the mtd infrastructure (for example used by UBI).
And frankly, cluttering the files with another set of #ifdef's doesn't really sound promising. Additionally this would make future sync's with the Linux version more troublesome.
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@denx.de =====================================================================
On Monday 11 May 2009 12:38:45 Stefan Roese wrote:
On Monday 11 May 2009 18:27:02 Mike Frysinger wrote:
On Monday 11 May 2009 10:03:55 Stefan Roese wrote:
This patch brings the U-Boot MTD infrastructure in sync with the current Linux MTD version (2.6.30-rc3). Biggest change is the 64bit device size support and a resync of the mtdpart.c file which has seen multiple fixes meanwhile.
really wish there was a define to control this. large devices are fine, but it sucks when majority of people dont hit this limit.
What is the drawback of this code version? I have to admit that I didn't compare code size, but this is the only possible drawback coming to my mind. And the advantages are big enough from my point of view. Being in sync with Linux once again not only brings 64bit device size support but removes quite a lot of bugs from the mtd infrastructure (for example used by UBI).
And frankly, cluttering the files with another set of #ifdef's doesn't really sound promising. Additionally this would make future sync's with the Linux version more troublesome.
i'd see most of the changes be via a typedef and thus there are no ifdefs cluttering up the code -mike
On Mon, May 11, 2009 at 12:57:52PM -0400, Mike Frysinger wrote:
On Monday 11 May 2009 12:38:45 Stefan Roese wrote:
On Monday 11 May 2009 18:27:02 Mike Frysinger wrote:
On Monday 11 May 2009 10:03:55 Stefan Roese wrote:
This patch brings the U-Boot MTD infrastructure in sync with the current Linux MTD version (2.6.30-rc3). Biggest change is the 64bit device size support and a resync of the mtdpart.c file which has seen multiple fixes meanwhile.
ACK
i'd see most of the changes be via a typedef and thus there are no ifdefs cluttering up the code
If you make the size a typedef, then all the printf formats that reference size will need casts (and will still need 64-bit printf), or some #defined format string.
We could do it if makes a big difference (looks like about 500 bytes in the NAND code on powerpc), but I suspect there are better places to save space in mtd, such as not building in default functions that are just going to be overridden (or not selected in the first place).
-Scott
Dear Stefan Roese,
In message 200905111838.45552.sr@denx.de you wrote:
really wish there was a define to control this. large devices are fine, but it sucks when majority of people dont hit this limit.
What is the drawback of this code version? I have to admit that I didn't compare code size, but this is the only possible drawback coming to my mind.
Can you please make such a comparison? I, too, would like to know how much of a difference that makes - I share Mike's concerns.
And the advantages are big enough from my point of view. Being in sync with Linux once again not only brings 64bit device size support but removes quite a lot of bugs from the mtd infrastructure (for example used by UBI).
I do understand this - but then, we're a boot loader, and code size is still a top priority here.
Best regards,
Wolfgang Denk
On Monday 11 May 2009 19:59:10 Wolfgang Denk wrote:
really wish there was a define to control this. large devices are fine, but it sucks when majority of people dont hit this limit.
What is the drawback of this code version? I have to admit that I didn't compare code size, but this is the only possible drawback coming to my mind.
Can you please make such a comparison? I, too, would like to know how much of a difference that makes - I share Mike's concerns.
OK, here we go:
Canyonlands (using NAND) without this patch:
text data bss dec hex filename 339324 20044 336384 695752 a9dc8 ./u-boot
and with this patch:
text data bss dec hex filename 339796 20044 336384 696224 a9fa0 ./u-boot
So it's 472 bytes on this platform.
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@denx.de =====================================================================
On Tuesday 12 May 2009 00:39:24 Stefan Roese wrote:
On Monday 11 May 2009 19:59:10 Wolfgang Denk wrote:
really wish there was a define to control this. large devices are fine, but it sucks when majority of people dont hit this limit.
What is the drawback of this code version? I have to admit that I didn't compare code size, but this is the only possible drawback coming to my mind.
Can you please make such a comparison? I, too, would like to know how much of a difference that makes - I share Mike's concerns.
OK, here we go:
Canyonlands (using NAND) without this patch:
text data bss dec hex filename 339324 20044 336384 695752 a9dc8 ./u-boot
and with this patch:
text data bss dec hex filename 339796 20044 336384 696224 a9fa0 ./u-boot
So it's 472 bytes on this platform.
doesnt sound like it's worth the hassle to me. thanks for testing a bit. -mike
Dear Stefan Roese,
In message 200905120639.24172.sr@denx.de you wrote:
OK, here we go:
Canyonlands (using NAND) without this patch:
text data bss dec hex filename 339324 20044 336384 695752 a9dc8 ./u-boot
and with this patch:
text data bss dec hex filename 339796 20044 336384 696224 a9fa0 ./u-boot
So it's 472 bytes on this platform.
Thanks - that's indeed not much.
Is my understanding correct that the effect on NOR-only systems is even smaller, it at all visible?
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Tuesday 12 May 2009 10:14:38 Wolfgang Denk wrote:
So it's 472 bytes on this platform.
Thanks - that's indeed not much.
Is my understanding correct that the effect on NOR-only systems is even smaller, it at all visible?
Yes, the common NOR-only systems without MTD support (cfi_mtd is needed only for UBI right now) will not see any changes at all.
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@denx.de =====================================================================
Dear Stefan Roese,
In message 1242050635-7152-1-git-send-email-sr@denx.de you wrote:
--===============0052372897==
This patch brings the U-Boot MTD infrastructure in sync with the current Linux MTD version (2.6.30-rc3). Biggest change is the 64bit device size support and a resync of the mtdpart.c file which has seen multiple fixes meanwhile.
Signed-off-by: Stefan Roese sr@denx.de Cc: Scott Wood scottwood@freescale.com Cc: Kyungmin Park kmpark@infradead.org
common/cmd_onenand.c | 6 +- common/env_onenand.c | 2 +- drivers/mtd/cfi_mtd.c | 4 +- drivers/mtd/mtdpart.c | 488 +++++++++++++++++++----------------------- drivers/mtd/nand/nand_util.c | 11 +- include/linux/mtd/mtd.h | 29 ++- 6 files changed, 256 insertions(+), 284 deletions(-)
Applied to next, thanks.
Best regards,
Wolfgang Denk
Dear Stefan,
having this patch in mainline [1] now, I have some issues with it. I use omap3_beagle_config (ARM). There was already a short discussion with Wolfgang on IRC regarding this, he directed my to you ;) I identified three topics, two minor and one major one:
Stefan Roese wrote:
This patch brings the U-Boot MTD infrastructure in sync with the current Linux MTD version (2.6.30-rc3). Biggest change is the 64bit device size support and a resync of the mtdpart.c file which has seen multiple fixes meanwhile.
Signed-off-by: Stefan Roese sr@denx.de Cc: Scott Wood scottwood@freescale.com Cc: Kyungmin Park kmpark@infradead.org
common/cmd_onenand.c | 6 +- common/env_onenand.c | 2 +- drivers/mtd/cfi_mtd.c | 4 +- drivers/mtd/mtdpart.c | 488 +++++++++++++++++++----------------------- drivers/mtd/nand/nand_util.c | 11 +- include/linux/mtd/mtd.h | 29 ++- 6 files changed, 256 insertions(+), 284 deletions(-)
diff --git a/common/cmd_onenand.c b/common/cmd_onenand.c index 5832ff8..e8f06d7 100644 --- a/common/cmd_onenand.c +++ b/common/cmd_onenand.c @@ -19,6 +19,10 @@
#include <asm/io.h>
+#if !defined(CONFIG_SYS_64BIT_VSPRINTF) +#warning Please define CONFIG_SYS_64BIT_VSPRINTF for correct output! +#endif
1. This preprocessor warning is annoying. Why not enable it in all configs where needed and then remove this warning here?
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h index 354e3a0..c884567 100644 --- a/include/linux/mtd/mtd.h +++ b/include/linux/mtd/mtd.h @@ -8,6 +8,7 @@ #define __MTD_MTD_H__
#include <linux/types.h> +#include <div64.h> #include <linux/mtd/mtd-abi.h>
#define MTD_CHAR_MAJOR 90 @@ -20,6 +21,8 @@ #define MTD_ERASE_DONE 0x08 #define MTD_ERASE_FAILED 0x10
+#define MTD_FAIL_ADDR_UNKNOWN -1LL
/*
- Enumeration for NAND/OneNAND flash chip state
*/ @@ -37,13 +40,13 @@ enum { };
/* If the erase fails, fail_addr might indicate exactly which block failed. If
- fail_addr = 0xffffffff, the failure was not at the device level or was not
- fail_addr = MTD_FAIL_ADDR_UNKNOWN, the failure was not at the device level or was not specific to any particular block. */
struct erase_info { struct mtd_info *mtd;
- u_int32_t addr;
- u_int32_t len;
- u_int32_t fail_addr;
- uint64_t addr;
- uint64_t len;
- uint64_t fail_addr; u_long time; u_long retries; u_int dev;
@@ -55,7 +58,7 @@ struct erase_info { };
struct mtd_erase_region_info {
- u_int32_t offset; /* At which this region starts, from the beginning of the MTD */
- uint64_t offset; /* At which this region starts, from the beginning of the MTD */ u_int32_t erasesize; /* For this region */ u_int32_t numblocks; /* Number of blocks of erasesize in this region */ unsigned long *lockmap; /* If keeping bitmap of locks */
@@ -110,7 +113,7 @@ struct mtd_oob_ops { struct mtd_info { u_char type; u_int32_t flags;
- u_int32_t size; /* Total size of the MTD */
- uint64_t size; // Total size of the MTD
2. C++ style comment. Found by Wolfgang ;)
3. The major one: By the new 64bit variables, depending on tool chain, there are now calls to libgcc introduced. Depending on tool chain, this might work, or fail. As it does with one of my tool chains, which worked totally fine until here. It was my understanding that U-Boot shall not rely on libgcc, i.e. proper tool chain libraries.
Having CONFIG_SYS_64BIT_VSPRINTF not defined, the linker needs "_lshrdi3", which comes from nand_base.o and nand_bbt.o. E.g. from "len = mtd->size >> (this->bbt_erase_shift + 2);" from nand_bbt.c.
Having CONFIG_SYS_64BIT_VSPRINTF enabled, the linker requests _lshrdi3, _udivdi3, _umoddi3 and _clz from libgcc.
Looking into U-Boot's lib_arm/ directory, we have already some low level math functions there to avoid libgcc. E.g. nand_bbt.c has references to _ashrdi3, too, but this is resolved by lib_arm's version what is fine.
I'd like that NAND code is modified that only math is used which U-Boot provides and no libgcc is needed.
Best regards
Dirk
[1] http://git.denx.de/?p=u-boot.git;a=commit;h=8d2effea23e938631126a7888008a063...
C++ style comment. Found by Wolfgang ;)
The major one: By the new 64bit variables, depending on tool chain,
there are now calls to libgcc introduced. Depending on tool chain, this might work, or fail. As it does with one of my tool chains, which worked totally fine until here. It was my understanding that U-Boot shall not rely on libgcc, i.e. proper tool chain libraries.
Having CONFIG_SYS_64BIT_VSPRINTF not defined, the linker needs "_lshrdi3", which comes from nand_base.o and nand_bbt.o. E.g. from "len = mtd->size >> (this->bbt_erase_shift + 2);" from nand_bbt.c.
Having CONFIG_SYS_64BIT_VSPRINTF enabled, the linker requests _lshrdi3, _udivdi3, _umoddi3 and _clz from libgcc.
Looking into U-Boot's lib_arm/ directory, we have already some low level math functions there to avoid libgcc. E.g. nand_bbt.c has references to _ashrdi3, too, but this is resolved by lib_arm's version what is fine.
I'd like that NAND code is modified that only math is used which U-Boot provides and no libgcc is needed.
this is a already known problem by Stefan and I and solved
two patchs was send to the ML and now we are independant of the libgcc
please test the testing branch of the u-boot-arm tree
Best Regards, J.
Jean-Christophe PLAGNIOL-VILLARD wrote:
C++ style comment. Found by Wolfgang ;)
The major one: By the new 64bit variables, depending on tool chain,
there are now calls to libgcc introduced. Depending on tool chain, this might work, or fail. As it does with one of my tool chains, which worked totally fine until here. It was my understanding that U-Boot shall not rely on libgcc, i.e. proper tool chain libraries.
Having CONFIG_SYS_64BIT_VSPRINTF not defined, the linker needs "_lshrdi3", which comes from nand_base.o and nand_bbt.o. E.g. from "len = mtd->size >> (this->bbt_erase_shift + 2);" from nand_bbt.c.
Having CONFIG_SYS_64BIT_VSPRINTF enabled, the linker requests _lshrdi3, _udivdi3, _umoddi3 and _clz from libgcc.
Looking into U-Boot's lib_arm/ directory, we have already some low level math functions there to avoid libgcc. E.g. nand_bbt.c has references to _ashrdi3, too, but this is resolved by lib_arm's version what is fine.
I'd like that NAND code is modified that only math is used which U-Boot provides and no libgcc is needed.
this is a already known problem by Stefan and I and solved
two patchs was send to the ML
Thanks for the hint! Do you kindly have the subjects or better links to mailing list archive?
Many thanks
Dirk
and now we are independant of the libgcc
please test the testing branch of the u-boot-arm tree
Best Regards, J.
Hi Dirk,
On Saturday 20 June 2009 19:45:40 Dirk Behme wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
- C++ style comment. Found by Wolfgang ;)
This is because it's a copy from the Linux code. Not sure if we should change the coding style here. I really prefer to be in sync with the Linux version. This makes future updates easier.
- The major one: By the new 64bit variables, depending on tool chain,
there are now calls to libgcc introduced. Depending on tool chain, this might work, or fail. As it does with one of my tool chains, which worked totally fine until here. It was my understanding that U-Boot shall not rely on libgcc, i.e. proper tool chain libraries.
Having CONFIG_SYS_64BIT_VSPRINTF not defined, the linker needs "_lshrdi3", which comes from nand_base.o and nand_bbt.o. E.g. from "len = mtd->size >> (this->bbt_erase_shift + 2);" from nand_bbt.c.
Having CONFIG_SYS_64BIT_VSPRINTF enabled, the linker requests _lshrdi3, _udivdi3, _umoddi3 and _clz from libgcc.
Looking into U-Boot's lib_arm/ directory, we have already some low level math functions there to avoid libgcc. E.g. nand_bbt.c has references to _ashrdi3, too, but this is resolved by lib_arm's version what is fine.
I'd like that NAND code is modified that only math is used which U-Boot provides and no libgcc is needed.
this is a already known problem by Stefan and I and solved
two patchs was send to the ML
Thanks for the hint! Do you kindly have the subjects or better links to mailing list archive?
http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=cf96e690cb9840b4a5dc7...
and
http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=c5ae538c9bd359c137d30...
both available in the "testing" branch of u-boot-arm.
From you comments on the list I see that you found those patches here too. :)
Do these patches fix the building problem for you?
Thanks.
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@denx.de =====================================================================
Hi Stefan,
Stefan Roese wrote:
Hi Dirk,
On Saturday 20 June 2009 19:45:40 Dirk Behme wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
- C++ style comment. Found by Wolfgang ;)
This is because it's a copy from the Linux code. Not sure if we should change the coding style here. I really prefer to be in sync with the Linux version. This makes future updates easier.
You have to discuss this with Wolfgang, but he wasn't happy about it. Enjoy the time while he is on vacation ;)
- The major one: By the new 64bit variables, depending on tool chain,
there are now calls to libgcc introduced. Depending on tool chain, this might work, or fail. As it does with one of my tool chains, which worked totally fine until here. It was my understanding that U-Boot shall not rely on libgcc, i.e. proper tool chain libraries.
Having CONFIG_SYS_64BIT_VSPRINTF not defined, the linker needs "_lshrdi3", which comes from nand_base.o and nand_bbt.o. E.g. from "len = mtd->size >> (this->bbt_erase_shift + 2);" from nand_bbt.c.
Having CONFIG_SYS_64BIT_VSPRINTF enabled, the linker requests _lshrdi3, _udivdi3, _umoddi3 and _clz from libgcc.
Looking into U-Boot's lib_arm/ directory, we have already some low level math functions there to avoid libgcc. E.g. nand_bbt.c has references to _ashrdi3, too, but this is resolved by lib_arm's version what is fine.
I'd like that NAND code is modified that only math is used which U-Boot provides and no libgcc is needed.
this is a already known problem by Stefan and I and solved
two patchs was send to the ML
Thanks for the hint! Do you kindly have the subjects or better links to mailing list archive?
http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=cf96e690cb9840b4a5dc7...
and
http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=c5ae538c9bd359c137d30...
both available in the "testing" branch of u-boot-arm.
Yes, I found them. I was confused because I understood Jean-Christophe that they were posted to the list already in the past, and not while we were discussing about it ;)
From you comments on the list I see that you found those patches here too. :)
Do these patches fix the building problem for you?
"testing" branch of u-boot-arm compiled fine for me, but I'm not sure if the NAND changes are in "testing" branch of u-boot-arm, too.
Anyway, I would vote for moving these two commits as soon as possible into mainline to fix mainline, too. But there was some discussion about how to deal with Makefile changes to the patches Jean-Christophe sent to the list.
Maybe we can find a solution for the Makefile issues of these patches discussed on this list and then move the fixes asap into mainline?
Many thanks and best regards
Dirk
===================================================================== 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@denx.de =====================================================================
On Monday 22 June 2009 20:10:23 Dirk Behme wrote:
This is because it's a copy from the Linux code. Not sure if we should change the coding style here. I really prefer to be in sync with the Linux version. This makes future updates easier.
You have to discuss this with Wolfgang, but he wasn't happy about it. Enjoy the time while he is on vacation ;)
Yes, will do. ;)
- The major one: By the new 64bit variables, depending on tool chain,
there are now calls to libgcc introduced. Depending on tool chain, this might work, or fail. As it does with one of my tool chains, which worked totally fine until here. It was my understanding that U-Boot shall not rely on libgcc, i.e. proper tool chain libraries.
Having CONFIG_SYS_64BIT_VSPRINTF not defined, the linker needs "_lshrdi3", which comes from nand_base.o and nand_bbt.o. E.g. from "len = mtd->size >> (this->bbt_erase_shift + 2);" from nand_bbt.c.
Having CONFIG_SYS_64BIT_VSPRINTF enabled, the linker requests _lshrdi3, _udivdi3, _umoddi3 and _clz from libgcc.
Looking into U-Boot's lib_arm/ directory, we have already some low level math functions there to avoid libgcc. E.g. nand_bbt.c has references to _ashrdi3, too, but this is resolved by lib_arm's version what is fine.
I'd like that NAND code is modified that only math is used which U-Boot provides and no libgcc is needed.
this is a already known problem by Stefan and I and solved
two patchs was send to the ML
Thanks for the hint! Do you kindly have the subjects or better links to mailing list archive?
http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=cf96e690cb9840b4a5 dc7e750be863228550b5ce
and
http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=c5ae538c9bd359c137 d306e431b2589ba91fbc08
both available in the "testing" branch of u-boot-arm.
Yes, I found them. I was confused because I understood Jean-Christophe that they were posted to the list already in the past, and not while we were discussing about it ;)
Jean-Christophe did post those patches a few days ago.
From you comments on the list I see that you found those patches here too. :)
Do these patches fix the building problem for you?
"testing" branch of u-boot-arm compiled fine for me, but I'm not sure if the NAND changes are in "testing" branch of u-boot-arm, too.
I'm pretty sure they are, since Jean-Christophe used these patches exactly to fix this NAND related problem on ARM. But it should be easy to check if the patches are in here. git is your friend... ;)
Anyway, I would vote for moving these two commits as soon as possible into mainline to fix mainline, too. But there was some discussion about how to deal with Makefile changes to the patches Jean-Christophe sent to the list.
Maybe we can find a solution for the Makefile issues of these patches discussed on this list and then move the fixes asap into mainline?
Yes, I would really like to see those changes in mainline soon as well. I have to admit that I'm not a real Makefile "expert", so I don't have a real expertise to get involved in this discussion. But I'll try to take another look at this thread tomorrow.
Thanks.
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@denx.de =====================================================================
Dear Stefan,
Stefan Roese wrote:
On Monday 22 June 2009 20:10:23 Dirk Behme wrote:
This is because it's a copy from the Linux code. Not sure if we should change the coding style here. I really prefer to be in sync with the Linux version. This makes future updates easier.
You have to discuss this with Wolfgang, but he wasn't happy about it. Enjoy the time while he is on vacation ;)
Yes, will do. ;)
- The major one: By the new 64bit variables, depending on tool chain,
there are now calls to libgcc introduced. Depending on tool chain, this might work, or fail. As it does with one of my tool chains, which worked totally fine until here. It was my understanding that U-Boot shall not rely on libgcc, i.e. proper tool chain libraries.
Having CONFIG_SYS_64BIT_VSPRINTF not defined, the linker needs "_lshrdi3", which comes from nand_base.o and nand_bbt.o. E.g. from "len = mtd->size >> (this->bbt_erase_shift + 2);" from nand_bbt.c.
Having CONFIG_SYS_64BIT_VSPRINTF enabled, the linker requests _lshrdi3, _udivdi3, _umoddi3 and _clz from libgcc.
Looking into U-Boot's lib_arm/ directory, we have already some low level math functions there to avoid libgcc. E.g. nand_bbt.c has references to _ashrdi3, too, but this is resolved by lib_arm's version what is fine.
I'd like that NAND code is modified that only math is used which U-Boot provides and no libgcc is needed.
this is a already known problem by Stefan and I and solved
two patchs was send to the ML
Thanks for the hint! Do you kindly have the subjects or better links to mailing list archive?
http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=cf96e690cb9840b4a5 dc7e750be863228550b5ce
and
http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=c5ae538c9bd359c137 d306e431b2589ba91fbc08
both available in the "testing" branch of u-boot-arm.
Yes, I found them. I was confused because I understood Jean-Christophe that they were posted to the list already in the past, and not while we were discussing about it ;)
Jean-Christophe did post those patches a few days ago.
From you comments on the list I see that you found those patches here too. :)
Do these patches fix the building problem for you?
"testing" branch of u-boot-arm compiled fine for me, but I'm not sure if the NAND changes are in "testing" branch of u-boot-arm, too.
I'm pretty sure they are, since Jean-Christophe used these patches exactly to fix this NAND related problem on ARM. But it should be easy to check if the patches are in here. git is your friend... ;)
Anyway, I would vote for moving these two commits as soon as possible into mainline to fix mainline, too. But there was some discussion about how to deal with Makefile changes to the patches Jean-Christophe sent to the list.
Maybe we can find a solution for the Makefile issues of these patches discussed on this list and then move the fixes asap into mainline?
Yes, I would really like to see those changes in mainline soon as well. I have to admit that I'm not a real Makefile "expert", so I don't have a real expertise to get involved in this discussion. But I'll try to take another look at this thread tomorrow.
Sorry, I know, the first week after vacation is always quite busy ;)
But any news on this?
Many thanks
Dirk
Hi Dirk,
On Sunday 28 June 2009 07:47:31 Dirk Behme wrote:
Anyway, I would vote for moving these two commits as soon as possible into mainline to fix mainline, too. But there was some discussion about how to deal with Makefile changes to the patches Jean-Christophe sent to the list.
Maybe we can find a solution for the Makefile issues of these patches discussed on this list and then move the fixes asap into mainline?
Yes, I would really like to see those changes in mainline soon as well. I have to admit that I'm not a real Makefile "expert", so I don't have a real expertise to get involved in this discussion. But I'll try to take another look at this thread tomorrow.
Sorry, I know, the first week after vacation is always quite busy ;)
But any news on this?
Yes. News is that I forgot to review this patch. ;)
I'll put it on my list for this week. Thanks for reminding me.
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@denx.de =====================================================================
Hi Dirk,
I forgot to address your first issue. Please see below:
<snip>
+#if !defined(CONFIG_SYS_64BIT_VSPRINTF) +#warning Please define CONFIG_SYS_64BIT_VSPRINTF for correct output! +#endif
- This preprocessor warning is annoying. Why not enable it in all
configs where needed and then remove this warning here?
I could have done that, but this would not have been so easy. Especially with so many new board ports pending in the u-boot-arch/next branches at that time. And upcoming board ports would probably fallen through the crack as well. So I decided to do it this way.
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@denx.de =====================================================================
Stefan Roese wrote:
Hi Dirk,
I forgot to address your first issue. Please see below:
<snip>
+#if !defined(CONFIG_SYS_64BIT_VSPRINTF) +#warning Please define CONFIG_SYS_64BIT_VSPRINTF for correct output! +#endif
- This preprocessor warning is annoying. Why not enable it in all
configs where needed and then remove this warning here?
I could have done that, but this would not have been so easy. Especially with so many new board ports pending in the u-boot-arch/next branches at that time. And upcoming board ports would probably fallen through the crack as well. So I decided to do it this way.
Is this a long term solution or only waiting for all u-boot-arch/next branches to be merged?
For a long term solution this would be annoying. Maybe we can change this after merge window closed?
Thanks and best regards
Dirk
On Monday 22 June 2009 20:12:11 Dirk Behme wrote:
- This preprocessor warning is annoying. Why not enable it in all
configs where needed and then remove this warning here?
I could have done that, but this would not have been so easy. Especially with so many new board ports pending in the u-boot-arch/next branches at that time. And upcoming board ports would probably fallen through the crack as well. So I decided to do it this way.
Is this a long term solution or only waiting for all u-boot-arch/next branches to be merged?
I don't understand this question. The current code with this check is available in mainline. Or not?
For a long term solution this would be annoying. Maybe we can change this after merge window closed?
Why? There could still be new board ports using this NAND code without defining this 64bit printf format option. I don't think this check really hurts.
We could get rid of it of course if we would enable this 64bit printf format on all platforms and removed this option completely. This would cost a bit of image size but would make things easier/simpler. Just my 0.02$...
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@denx.de =====================================================================
Stefan Roese wrote:
On Monday 22 June 2009 20:12:11 Dirk Behme wrote:
- This preprocessor warning is annoying. Why not enable it in all
configs where needed and then remove this warning here?
I could have done that, but this would not have been so easy. Especially with so many new board ports pending in the u-boot-arch/next branches at that time. And upcoming board ports would probably fallen through the crack as well. So I decided to do it this way.
Is this a long term solution or only waiting for all u-boot-arch/next branches to be merged?
I don't understand this question. The current code with this check is available in mainline. Or not?
Sorry, yes, but you mentioned above pending boards and not merged next branches. So my question was if you plan to change the warning after all pending boards and next branches are merged.
For a long term solution this would be annoying. Maybe we can change this after merge window closed?
Why? There could still be new board ports using this NAND code without defining this 64bit printf format option. I don't think this check really hurts.
Because every non-U-Boot expert who just downloads recent git and compiles it will get the warning and will wonder if something is wrong or not. So yes, the check doesn't hurt, but the warning might confuse the users, which are non-experts.
It's similar to the "WARNING - bad CRC or NAND, using default environment." runtime warning. There are tons on questions on various mailing lists if this is an error or not ;)
We could get rid of it of course if we would enable this 64bit printf format on all platforms and removed this option completely. This would cost a bit of image size but would make things easier/simpler. Just my 0.02$...
For OMAP3 I would enabled this option, independent of image size, just to get rid of the warning.
Best regards
Dirk
Dirk Behme wrote:
Because every non-U-Boot expert who just downloads recent git and compiles it will get the warning and will wonder if something is wrong or not. So yes, the check doesn't hurt, but the warning might confuse the users, which are non-experts.
And the answer to that user is that something *is* wrong, and should be reported to the board maintainer so they can enable 64-bit printf.
We could get rid of it of course if we would enable this 64bit printf format on all platforms and removed this option completely. This would cost a bit of image size but would make things easier/simpler. Just my 0.02$...
Agreed. Or once we get kconfig we can have it be selected by any code that requires it.
-Scott
On Monday 22 June 2009 20:50:30 Scott Wood wrote:
Dirk Behme wrote:
Because every non-U-Boot expert who just downloads recent git and compiles it will get the warning and will wonder if something is wrong or not. So yes, the check doesn't hurt, but the warning might confuse the users, which are non-experts.
And the answer to that user is that something *is* wrong, and should be reported to the board maintainer so they can enable 64-bit printf.
Ack.
We could get rid of it of course if we would enable this 64bit printf format on all platforms and removed this option completely. This would cost a bit of image size but would make things easier/simpler. Just my 0.02$...
Agreed. Or once we get kconfig we can have it be selected by any code that requires it.
Good idea. Thanks Scott.
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@denx.de =====================================================================
Stefan Roese wrote:
On Monday 22 June 2009 20:50:30 Scott Wood wrote:
Dirk Behme wrote:
Because every non-U-Boot expert who just downloads recent git and compiles it will get the warning and will wonder if something is wrong or not. So yes, the check doesn't hurt, but the warning might confuse the users, which are non-experts.
And the answer to that user is that something *is* wrong, and should be reported to the board maintainer so they can enable 64-bit printf.
Ack.
Ok. So you tell me we shall enable 64-bit printf for all OMAP3 boards that show this warning?
What I still miss: If this warning indicates that something *is* wrong, why not run MAKEALL and enable it in config for all boards that show this warning? What's the advantage of having all maintainers sending patches for their boards?
Best regards
Dirk
We could get rid of it of course if we would enable this 64bit printf format on all platforms and removed this option completely. This would cost a bit of image size but would make things easier/simpler. Just my 0.02$...
Agreed. Or once we get kconfig we can have it be selected by any code that requires it.
Good idea. Thanks Scott.
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@denx.de =====================================================================
Dirk Behme wrote:
What I still miss: If this warning indicates that something *is* wrong, why not run MAKEALL and enable it in config for all boards that show this warning? What's the advantage of having all maintainers sending patches for their boards?
1. That requires having all toolchains installed, and 2. That does nothing to prevent future boards from being added that are missing the dependency.
-Scott
Scott Wood wrote:
Dirk Behme wrote:
What I still miss: If this warning indicates that something *is* wrong, why not run MAKEALL and enable it in config for all boards that show this warning? What's the advantage of having all maintainers sending patches for their boards?
- That requires having all toolchains installed, and
Hmm? In nand_util.c we have
#if !defined(CONFIG_SYS_64BIT_VSPRINTF) #warning Please define CONFIG_SYS_64BIT_VSPRINTF for correct output! #endif
and in Makefile we have
ifdef CONFIG_CMD_NAND ifndef CONFIG_NAND_LEGACY ... COBJS-y += nand_util.o endif
So I would assume that this warning appears for all boards having CONFIG_CMD_NAND enabled and no CONFIG_NAND_LEGACY and no CONFIG_SYS_64BIT_VSPRINTF? Independent of tool chain?
- That does nothing to prevent future boards from being added that are
missing the dependency.
Ok, for future boards this warning is fine. But I talk about all the existing boards.
Best regards
Dirk
Dirk Behme wrote:
Scott Wood wrote:
Dirk Behme wrote:
What I still miss: If this warning indicates that something *is* wrong, why not run MAKEALL and enable it in config for all boards that show this warning? What's the advantage of having all maintainers sending patches for their boards?
- That requires having all toolchains installed, and
Hmm? In nand_util.c we have
#if !defined(CONFIG_SYS_64BIT_VSPRINTF) #warning Please define CONFIG_SYS_64BIT_VSPRINTF for correct output! #endif
and in Makefile we have
ifdef CONFIG_CMD_NAND ifndef CONFIG_NAND_LEGACY ... COBJS-y += nand_util.o endif
So I would assume that this warning appears for all boards having CONFIG_CMD_NAND enabled and no CONFIG_NAND_LEGACY and no CONFIG_SYS_64BIT_VSPRINTF? Independent of tool chain?
I don't mean different versions, I mean toolchains that cover all architectures. If I don't have a superh or blackfin compiler installed, I'm not going to see any warnings on those boards because they won't build at all.
I agree though that it would be nice to grep for existing NAND users and try to cover the obvious ones.
-Scott
Scott Wood wrote:
Dirk Behme wrote:
Scott Wood wrote:
Dirk Behme wrote:
What I still miss: If this warning indicates that something *is* wrong, why not run MAKEALL and enable it in config for all boards that show this warning? What's the advantage of having all maintainers sending patches for their boards?
- That requires having all toolchains installed, and
Hmm? In nand_util.c we have
#if !defined(CONFIG_SYS_64BIT_VSPRINTF) #warning Please define CONFIG_SYS_64BIT_VSPRINTF for correct output! #endif
and in Makefile we have
ifdef CONFIG_CMD_NAND ifndef CONFIG_NAND_LEGACY ... COBJS-y += nand_util.o endif
So I would assume that this warning appears for all boards having CONFIG_CMD_NAND enabled and no CONFIG_NAND_LEGACY and no CONFIG_SYS_64BIT_VSPRINTF? Independent of tool chain?
I don't mean different versions, I mean toolchains that cover all architectures. If I don't have a superh or blackfin compiler installed, I'm not going to see any warnings on those boards because they won't build at all.
I agree though that it would be nice to grep for existing NAND users and try to cover the obvious ones.
Ack, this is exactly what I meant :)
Dirk
participants (6)
-
Dirk Behme -
Jean-Christophe PLAGNIOL-VILLARD -
Mike Frysinger -
Scott Wood -
Stefan Roese -
Wolfgang Denk