JFFS1/MTD bug-fixes

JFFS1/MTD bug-fixes

[Aleksander Sanochkin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 218 bytes --]

Hi!

We've been working with JFFS1 on various boards
and have made what we believe is a number of assorted
bug-fixes and small improvements in the JFFS1/MTD code.

Attached is a diff against the current CVS.

Regards.

[-- Attachment #2: Type: TEXT/PLAIN, Size: 12157 bytes --]

diff -ur cvs/drivers/mtd/chips/cfi_cmdset_0002.c other/drivers/mtd/chips/cfi_cmdset_0002.c
--- cvs/drivers/mtd/chips/cfi_cmdset_0002.c	Wed Oct 24 13:37:30 2001
+++ other/drivers/mtd/chips/cfi_cmdset_0002.c	Wed Nov 21 12:58:47 2001
@@ -162,10 +162,7 @@
 	/* Also select the correct geometry setup too */ 
 	mtd->size = devsize * cfi->numchips;
 	
-	if (cfi->cfiq->NumEraseRegions == 1) {
-		/* No need to muck about with multiple erase sizes */
-		mtd->erasesize = ((cfi->cfiq->EraseRegionInfo[0] >> 8) & ~0xff) * cfi->interleave;
-	} else {
+	{
 		unsigned long offset = 0;
 		int i,j;
 
diff -ur cvs/drivers/mtd/devices/mtdram.c other/drivers/mtd/devices/mtdram.c
--- cvs/drivers/mtd/devices/mtdram.c	Tue Oct  2 19:05:13 2001
+++ other/drivers/mtd/devices/mtdram.c	Wed Nov 21 12:59:45 2001
@@ -123,7 +123,7 @@
    // Allocate some memory
    mtd_info = (struct mtd_info *)kmalloc(sizeof(struct mtd_info), GFP_KERNEL);
    if (!mtd_info)
-      return 0;
+      return -ENOMEM;
    
    memset(mtd_info, 0, sizeof(*mtd_info));
 
diff -ur cvs/drivers/mtd/mtdchar-compat.c other/drivers/mtd/mtdchar-compat.c
--- cvs/drivers/mtd/mtdchar-compat.c	Tue Oct  2 19:05:11 2001
+++ other/drivers/mtd/mtdchar-compat.c	Wed Nov 21 13:03:59 2001
@@ -480,7 +480,7 @@
 		
 	default:
 		DEBUG(MTD_DEBUG_LEVEL0, "Invalid ioctl %x (MEMGETINFO = %x)\n", cmd, MEMGETINFO);
-		ret = -ENOTTY;
+		ret = -ENOIOCTLCMD;
 	}
 
 	return ret;
diff -ur cvs/drivers/mtd/mtdchar.c other/drivers/mtd/mtdchar.c
--- cvs/drivers/mtd/mtdchar.c	Tue Oct  2 19:05:11 2001
+++ other/drivers/mtd/mtdchar.c	Wed Nov 21 13:04:10 2001
@@ -437,7 +437,7 @@
 		
 	default:
 		DEBUG(MTD_DEBUG_LEVEL0, "Invalid ioctl %x (MEMGETINFO = %x)\n", cmd, MEMGETINFO);
-		ret = -ENOTTY;
+		ret = -ENOIOCTLCMD;
 	}
 
 	return ret;
diff -ur cvs/drivers/mtd/mtdcore.c other/drivers/mtd/mtdcore.c
--- cvs/drivers/mtd/mtdcore.c	Tue Oct  2 19:05:11 2001
+++ other/drivers/mtd/mtdcore.c	Wed Nov 21 17:37:04 2001
@@ -51,6 +51,7 @@
 			struct mtd_notifier *not=mtd_notifiers;
 
 			mtd_table[i] = mtd;
+			mtd->usage_counter = 0;
 			mtd->index = i;
 			DEBUG(0, "mtd: Giving out device %d to %s\n",i, mtd->name);
 			while (not)
@@ -88,6 +89,11 @@
 	{
 		if (mtd_table[i] == mtd)
 		{
+			if (mtd->usage_counter)
+			{
+				up(&mtd_table_mutex);
+				return -EBUSY;
+			}
 			while (not)
 			{
 				(*(not->remove))(mtd);
@@ -316,6 +322,8 @@
 
 /*====================================================================*/
 /* Init code */
+
+extern int init_mtd_devices(void);
 
 int __init init_mtd(void)
 {
diff -ur cvs/drivers/mtd/mtdpart.c other/drivers/mtd/mtdpart.c
--- cvs/drivers/mtd/mtdpart.c	Wed Nov  7 04:20:58 2001
+++ other/drivers/mtd/mtdpart.c	Wed Nov 21 17:46:29 2001
@@ -140,6 +140,7 @@
 {
 	struct list_head *node;
 	struct mtd_part *slave;
+	int err;
 
 	for (node = mtd_partitions.next;
 	     node != &mtd_partitions;
@@ -147,8 +148,11 @@
 		slave = list_entry(node, struct mtd_part, list);
 		if (slave->master == master) {
 			struct list_head *prev = node->prev;
+			err = del_mtd_device(&slave->mtd);
+			if (err)
+				return err;
 			__list_del(prev, node->next);
-			del_mtd_device(&slave->mtd);
+			kfree(slave->mtd.eraseregions);
 			kfree(slave);
 			node = prev;
 		}
@@ -171,6 +175,7 @@
 	struct mtd_part *slave;
 	u_int32_t cur_offset = 0;
 	int i;
+	int err;
 
 	printk (KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
 
@@ -272,6 +277,17 @@
 			/* Single erase size */
 			slave->mtd.erasesize = master->erasesize;
 		}
+		slave->mtd.numeraseregions = 1;
+		if (! (slave->mtd.eraseregions = kmalloc(sizeof
+			 (struct mtd_erase_region_info), GFP_KERNEL))) {
+			kfree(slave);
+			del_mtd_partitions(master);
+			return -ENOMEM;
+		}
+		slave->mtd.eraseregions->offset = 0;
+		slave->mtd.eraseregions->numblocks =
+			slave->mtd.size / slave->mtd.erasesize;
+		slave->mtd.eraseregions->erasesize = slave->mtd.erasesize;
 
 		if ((slave->mtd.flags & MTD_WRITEABLE) && 
 		    (slave->offset % slave->mtd.erasesize)) {
@@ -289,7 +305,13 @@
 		}
 
 		/* register our partition */
-		add_mtd_device(&slave->mtd);
+		err = add_mtd_device(&slave->mtd);
+		if (err) {
+			kfree(slave->mtd.eraseregions);
+			kfree(slave);
+			del_mtd_partitions(master);
+			return err;
+		}
 	}
 
 	return 0;
diff -ur cvs/fs/jffs/inode-v23.c other/fs/jffs/inode-v23.c
--- cvs/fs/jffs/inode-v23.c	Tue Oct  2 13:16:02 2001
+++ other/fs/jffs/inode-v23.c	Wed Nov 21 18:36:46 2001
@@ -137,10 +137,22 @@
 	if (c->gc_maxdirty_threshold < c->fmc->sector_size)
 		c->gc_maxdirty_threshold = c->fmc->sector_size;
 
+	init_completion(&c->gc_thread_comp);
+	c->gc_task = 0;
 
 	c->thread_pid = kernel_thread (jffs_garbage_collect_thread, 
 				        (void *) c, 
 				        CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
+	/* Ensure that the garbage collection thread is actually started. */
+	if (c->thread_pid <= 0)
+	{
+		printk(KERN_ERR "Could not start GC thread. Code = %d\n", c->thread_pid);
+		goto jffs_sb_err3;
+	}
+	while (! c->gc_task)
+	{
+		schedule();
+	}
 	D1(printk(KERN_NOTICE "JFFS: GC thread pid=%d.\n", (int) c->thread_pid));
 
 	D1(printk(KERN_NOTICE "JFFS: Successfully mounted device %s.\n",
@@ -391,6 +403,15 @@
 	buf->f_bfree = (jffs_free_size1(fmc) + jffs_free_size2(fmc) +
 		       fmc->dirty_size - fmc->min_free_size)
 			       >> PAGE_CACHE_SHIFT;
+	/* statfs() is prevented from reporting negative sizes. */
+	if (buf->f_bfree < 0)
+	{
+		buf->f_bfree = 0;
+	}
+	if (buf->f_blocks < 0)
+	{
+		buf->f_blocks = 0;
+	}
 	buf->f_bavail = buf->f_bfree;
 
 	/* Find out how many files there are in the filesystem.  */
@@ -1348,6 +1369,22 @@
 		  "filp: 0x%p, buf: 0x%p, count: %d\n",
 		  inode, inode->i_ino, filp, buf, count));
 
+	/* The following two checks are stolen from fs/ext2/file.c
+	 * POSIX: mtime/ctime may not change for 0 count
+	 */
+	if (!count)
+	{
+		return 0;
+	}
+
+	/* This makes the bounds-checking arithmetic later on much more
+	 * sane.
+	 */
+	if (((signed) count) < 0)
+	{
+		return -EINVAL;
+	}
+
 #if 0
 	if (inode->i_sb->s_flags & MS_RDONLY) {
 		D(printk("jffs_file_write(): MS_RDONLY\n"));
@@ -1385,6 +1422,31 @@
 	D3(printk (KERN_NOTICE "file_write(): down biglock\n"));
 	down(&c->fmc->biglock);
 
+	/* Check whether there is enough free space to complete the call
+	 * before writing anything to Flash.
+	 */
+	if (
+	      /* If only a single chunk is about to be written,
+	       * the check performed by jffs_write_node() lately is enough.
+	       */
+	    count != thiscount &&
+	    !JFFS_ENOUGH_SPACE(c, count +
+	        (count / thiscount + 1) * sizeof(raw_inode) +
+	          /* Currently the name is written along with every raw_inode.
+	           */
+	        (count / thiscount + 1) * JFFS_PAD(f->nsize) +
+	          /* Only the last chunk could have unaligned data size.
+	           */
+	        JFFS_GET_PAD_BYTES(count) +
+	          /* One extra max_chunk_size that we leave to be able to
+	           * remove files later.
+	           */
+	        c->fmc->max_chunk_size))
+	{
+		err = -ENOSPC;
+		goto out;
+	}
+
 	/* Urgh. POSIX says we can do short writes if we feel like it. 
 	 * In practice, we can't. Nothing will cope. So we loop until
 	 * we're done.
@@ -1473,6 +1535,8 @@
 	D3(printk (KERN_NOTICE "file_write(): up biglock\n"));
 	up(&c->fmc->biglock);
 
+	*ppos = pos;		
+
 	/* Fix things in the real inode.  */
 	if (pos > inode->i_size) {
 		inode->i_size = pos;
@@ -1483,6 +1547,8 @@
 	invalidate_inode_pages(inode);
 
  out_isem:
+	if (written)
+		return written;
 	return err;
 } /* jffs_file_write()  */
 
@@ -1552,8 +1618,9 @@
 			fst.size = fmc->flash_size;
 			fst.used = fmc->used_size;
 			fst.dirty = fmc->dirty_size;
-			fst.begin = fmc->head->offset;
-			fst.end = fmc->tail->offset + fmc->tail->size;
+			fst.begin = fmc->head ? fmc->head->offset : 0;
+			fst.end = fmc->tail ? fmc->tail->offset +
+			                      fmc->tail->size : 0;
 			printk("size: %d, used: %d, dirty: %d, "
 			       "begin: %d, end: %d\n",
 			       fst.size, fst.used, fst.dirty,
@@ -1614,6 +1681,7 @@
 static struct file_operations jffs_dir_operations =
 {
 	readdir:	jffs_readdir,
+	ioctl:   	jffs_ioctl,
 };
 
 
diff -ur cvs/fs/jffs/intrep.c other/fs/jffs/intrep.c
--- cvs/fs/jffs/intrep.c	Mon Sep 24 03:28:36 2001
+++ other/fs/jffs/intrep.c	Wed Nov 21 20:01:55 2001
@@ -116,7 +116,7 @@
 		int i;
 
 		printk("%ld:", (long) pos);
-		for (j = 0; j < 16; j++) {
+		for (j = 0; j < (size > 16 ? 16 : size); j++) {
 			line[j] = flash_read_u8(mtd, pos++);
 		}
 		for (i = 0; i < j; i++) {
@@ -256,6 +256,7 @@
 {
 	static unsigned char pattern[64];
 	int i;
+ 	int err;
 
 	/* fill up pattern */
 
@@ -265,7 +266,12 @@
 	/* write as many 64-byte chunks as we can */
 
 	while (size >= 64) {
-		flash_safe_write(mtd, to, pattern, 64);
+		err = flash_safe_write(mtd, to, pattern, 64);
+		if (err)
+		{
+			D(printk("flash_safe_write() failed\n"));
+			return err;
+		}
 		size -= 64;
 		to += 64;
 	}
@@ -273,7 +279,14 @@
 	/* and the rest */
 
 	if(size)
-		flash_safe_write(mtd, to, pattern, size);
+ 	{
+		err = flash_safe_write(mtd, to, pattern, size);
+		if (err)
+		{
+			D(printk("2flash_safe_write() failed\n"));
+			return err;
+		}
+	}
 
 	return size;
 }
@@ -1411,7 +1424,11 @@
 		   -- dwmw2
 		*/
 		if (node->data_size || node->removed_size) {
-			jffs_update_file(f, node);
+			int err;
+			if ((err = jffs_update_file(f, node)) < 0)
+			{
+				return err;
+			}
 		}
 		jffs_remove_redundant_nodes(f);
 
@@ -1802,6 +1819,7 @@
 		
 		if (err == -ENOSPC) {
 			/* Just out of space. GC and try again */
+			jffs_fm_write_unlock(fmc);
 			if (fmc->dirty_size < fmc->sector_size) {
 				D(printk("jffs_write_node(): jffs_fmalloc(0x%p, %u) "
 					 "failed, no dirty space to GC\n", fmc,
@@ -1815,7 +1833,6 @@
 				D(printk("jffs_write_node(): jffs_garbage_collect_now() failed\n"));
 				return err;
 			}
-			jffs_fm_write_lock(fmc);
 			continue;
 		} 
 
@@ -1831,8 +1848,6 @@
 			/* The jffs_fm struct that we got is not good enough.
 			   Make that space dirty and try again  */
 			if ((err = jffs_write_dummy_node(c, fm)) < 0) {
-				kfree(fm);
-				DJM(no_jffs_fm--);
 				jffs_fm_write_unlock(fmc);
 				D(printk("jffs_write_node(): "
 					 "jffs_write_dummy_node(): Failed!\n"));
@@ -2141,7 +2156,11 @@
 		  f->ino, (f->name ? f->name : "")));
 
 	for (n = f->version_head; n; n = n->version_next) {
-		jffs_update_file(f, n);
+		int err;
+		if ((err = jffs_update_file(f, n)) < 0)
+		{
+			return err;
+		}
 	}
 	return 0;
 }
@@ -2716,9 +2735,7 @@
 		} else {
 			err = -ENOSPC;
 		}
-		DJM(no_jffs_fm--);
 		jffs_fm_write_unlock(fmc);
-		kfree(fm);
 		
 		return err;
 	}
@@ -3349,7 +3366,6 @@
 
 	current->session = 1;
 	current->pgrp = 1;
-	init_completion(&c->gc_thread_comp); /* barrier */ 
 	spin_lock_irq(&current->sigmask_lock);
 	siginitsetinv (&current->blocked, sigmask(SIGHUP) | sigmask(SIGKILL) | sigmask(SIGSTOP) | sigmask(SIGCONT));
 	recalc_sigpending(current);
diff -ur cvs/include/linux/mtd/mtd.h other/include/linux/mtd/mtd.h
--- cvs/include/linux/mtd/mtd.h	Sat Jun  9 04:08:59 2001
+++ other/include/linux/mtd/mtd.h	Wed Nov 21 19:08:06 2001
@@ -199,6 +199,9 @@
 	int (*suspend) (struct mtd_info *mtd);
 	void (*resume) (struct mtd_info *mtd);
 
+	/* Partition usage counter. */
+	int usage_counter;
+
 	void *priv;
 };
 
@@ -219,13 +222,18 @@
 	if (ret && ret->module && !try_inc_mod_count(ret->module))
 		return NULL;
 
+	if (ret)
+		ret->usage_counter++;
+
 	return ret;
 }
 
 static inline void put_mtd_device(struct mtd_info *mtd)
 {
-       if (mtd->module)
-	       __MOD_DEC_USE_COUNT(mtd->module);
+	if (mtd->module)
+		__MOD_DEC_USE_COUNT(mtd->module);
+	if (mtd->usage_counter)
+		mtd->usage_counter--;
 }
 
 

Re: JFFS1/MTD bug-fixes

[David Woodhouse]

> --- cvs/drivers/mtd/chips/cfi_cmdset_0002.c	Wed Oct 24 13:37:30 2001
> +++ other/drivers/mtd/chips/cfi_cmdset_0002.c	Wed Nov 21 12:58:47 2001
> @@ -162,10 +162,7 @@
>  	/* Also select the correct geometry setup too */ 
>  	mtd->size = devsize * cfi->numchips;
>  	
> -	if (cfi->cfiq->NumEraseRegions == 1) {
> -		/* No need to muck about with multiple erase sizes */
> -		mtd->erasesize = ((cfi->cfiq->EraseRegionInfo[0] >> 8) & ~0xff) * cfi->interleave;
> -	} else {
> +	{
>  		unsigned long offset = 0;
>  		int i,j;

Why do we need this? If NumEraseRegions is one (or zero, for that matter), 
we don't need to allocate the structure containing the region information, 
as the whole device is known to have the same 'major' erasesize.

>     mtd_info = (struct mtd_info *)kmalloc(sizeof(struct mtd_info), GFP_KERNEL);
>     if (!mtd_info)
> -      return 0;
> +      return -ENOMEM;

Applied.

>  	default:
>  		DEBUG(MTD_DEBUG_LEVEL0, "Invalid ioctl %x (MEMGETINFO = %x)\n", cmd, MEMGETINFO);
> -		ret = -ENOTTY;
> +		ret = -ENOIOCTLCMD;

Strange though it may seem, I thought -ENOTTY was correct there. 

       ENOTTY The specified request does not apply to the kind of
              object that the descriptor d references.

> +			if (mtd->usage_counter)
> +			{
> +				up(&mtd_table_mutex);
> +				return -EBUSY;
> +			}

Do we need to provide this in the kernel? I can see it protects against 
user error, but that isn't necessarily the kernel's job. Am I missing 
another need for it?

> --- cvs/fs/jffs/inode-v23.c	Tue Oct  2 13:16:02 2001
> +++ other/fs/jffs/inode-v23.c	Wed Nov 21 18:36:46 2001
> @@ -137,10 +137,22 @@
>  	if (c->gc_maxdirty_threshold < c->fmc->sector_size)
>  		c->gc_maxdirty_threshold = c->fmc->sector_size;
>  
> +	init_completion(&c->gc_thread_comp);
> +	c->gc_task = 0;

Agreed. The existing code is racy. 

> +	while (! c->gc_task)
> +	{
> +		schedule();
> +	}

Better to do this with a semaphore. Start it locked, have the GC thread 
call up(), and this code wait in down(). 

The rest of the JFFS changes look sane. Would you be happy to take over 
maintenance of JFFS1?

--
dwmw2

Re: JFFS1/MTD bug-fixes

[Aleksander Sanochkin]

> > --- cvs/drivers/mtd/chips/cfi_cmdset_0002.c   Wed Oct 24 13:37:30 2001
> > +++ other/drivers/mtd/chips/cfi_cmdset_0002.c Wed Nov 21 12:58:47 2001
> > @@ -162,10 +162,7 @@
> >       /* Also select the correct geometry setup too */
> >       mtd->size = devsize * cfi->numchips;
> >
> > -     if (cfi->cfiq->NumEraseRegions == 1) {
> > -             /* No need to muck about with multiple erase sizes */
> > -             mtd->erasesize = ((cfi->cfiq->EraseRegionInfo[0] >> 8) &
~0xff) * cfi->interleave;
> > -     } else {
> > +     {
> >               unsigned long offset = 0;
> >               int i,j;
>
> Why do we need this? If NumEraseRegions is one (or zero, for that
> matter),
> we don't need to allocate the structure containing the region
> information,
> as the whole device is known to have the same 'major' erasesize.

We did this for two reasons:

 1) To be consistent with cfi_cmdset_002.c which always initializes
    eraseregions.

 2) We have also modified JFFS1 code so as it now works for flash
    partitions with non-uniform sectors, and our code is based on
    the assumption that eraseregions are always initialized.


> >       default:
> >               DEBUG(MTD_DEBUG_LEVEL0, "Invalid ioctl %x (MEMGETINFO =
%x)\n", cmd, MEMGETINFO);
> > -             ret = -ENOTTY;
> > +             ret = -ENOIOCTLCMD;
>
> Strange though it may seem, I thought -ENOTTY was correct there.
>
>        ENOTTY The specified request does not apply to the kind of
>               object that the descriptor d references.

You are right: ENOTTY is the correct value here.

>
> > +                     if (mtd->usage_counter)
> > +                     {
> > +                             up(&mtd_table_mutex);
> > +                             return -EBUSY;
> > +                     }
>
> Do we need to provide this in the kernel? I can see it protects against
> user error, but that isn't necessarily the kernel's job. Am I missing
> another need for it?

We have also added a possibility for run-time partitioning of an MTD
device, and use this usage_counter to prevent deleting of a partition
while it is being used by some process.

> > +     while (! c->gc_task)
> > +     {
> > +             schedule();
> > +     }
>
> Better to do this with a semaphore. Start it locked, have the GC thread
> call up(), and this code wait in down().

You are right.

Re: JFFS1/MTD bug-fixes

[David Woodhouse]

asanochkin@Lnxw.COM said:
> 
>  1) To be consistent with cfi_cmdset_002.c which always initializes
>     eraseregions.
>  2) We have also modified JFFS1 code so as it now works for flash
>     partitions with non-uniform sectors, and our code is based on
>     the assumption that eraseregions are always initialized. 

If possible, I would prefer to keep the individual listing of erase regions
optional for the common case where it's not necessary - how difficult would
it be to make the JFFS1 code deal with that?

>  We have also added a possibility for run-time partitioning of an MTD
> device, and use this usage_counter to prevent deleting of a partition
> while it is being used by some process.

In that context, I agree that it makes sense. Have you seen Jörn Engel's
rewrite of the partitioning code, which I was intending to look at merging
in 2.5? It would be useful to make that does what you require before we
merge it.

--
dwmw2

Re: JFFS1/MTD bug-fixes

[Jörn Engel]

Hi!

> >  We have also added a possibility for run-time partitioning of an MTD
> > device, and use this usage_counter to prevent deleting of a partition
> > while it is being used by some process.
> 
> In that context, I agree that it makes sense. Have you seen Jörn Engel's
> rewrite of the partitioning code, which I was intending to look at merging
> in 2.5? It would be useful to make that does what you require before we
> merge it.

http://wh.fh-wedel.de/~joern/software/kernel/
http://wh.fh-wedel.de/~joern/software/kernel/mtd.patch.core.2.4.15

Not having seen any code, I fear that your patch won't easily merge
with mine. But I would like to have a look - it might get rid of some
dear old problems, commonly known as reboot. :)

Jörn

-- 
When writing gateway software of any kind, take pains to disturb 
the data stream as little as possible -- and *never* throw 
away information unless the recipient forces you to!

Re: JFFS1/MTD bug-fixes

[Aleksander Sanochkin]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2481 bytes --]


> >
> >  1) To be consistent with cfi_cmdset_002.c which always initializes
> >     eraseregions.
> >  2) We have also modified JFFS1 code so as it now works for flash
> >     partitions with non-uniform sectors, and our code is based on
> >     the assumption that eraseregions are always initialized.
>
> If possible, I would prefer to keep the individual listing of erase
> regions
> optional for the common case where it's not necessary - how difficult
> would
> it be to make the JFFS1 code deal with that?

It is not very difficult. A bigger problem testing presents: our code has
been tested as is, and making modfications to it will require some
additional testing, which we are not sure we can do.

Our flash partitioning code assumes that erase_regions are initialized,
too.

Anyway, why do you want to keep erase regions optional? To me, it only
complicates things, doesn't it?

And cfi_cmdset_0002.c always initializes erase_regions.

>
>
> >  We have also added a possibility for run-time partitioning of an MTD
> > device, and use this usage_counter to prevent deleting of a partition
> > while it is being used by some process.
>
> In that context, I agree that it makes sense. Have you seen Jörn Engel's
> rewrite of the partitioning code, which I was intending to look at
> merging in 2.5? It would be useful to make that does what you require
> before we merge it.
>

Yes, I've looked through Jörn Engel's code. Our modifications are less
significant and are based on other goals in mind.

Primary goal was to allow for creating Flash partitions that consist of
non-consequent sectors. For instance, the user may wish to chose sector
2 for storing some binary data and use sectors 1,3,4..... as a single
JFFS partition.

Another goal was a possibility of flexible (re-)configration of the
partitions, without having to change any C code. Supported methods are:
build-time (via a CONFIG_... define), boot-time (via a kernel command
line), and run-time (via an mtdchar IOCTL command). With all the three
methods, the user uses a string notation to specify desired layout of the
partitions. For instance, "0,3-8:1,2:9-25" means three partitions, with
sectors 0 and 3 to 8 belonging to the first partition, 1 and 2 to the
second, and 9 to 25 to the third.

Attached are the following files:

 o) A diff of our partitioning code against the current CVS

 o) An example mapping driver to get an idea of how the generic code is
    supposed to be used.


Best regards.

[-- Attachment #2: Type: TEXT/PLAIN, Size: 27783 bytes --]

Index: drivers/mtd/mtdchar.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/mtdchar.c,v
retrieving revision 1.44
diff -u -r1.44 mtdchar.c
--- drivers/mtd/mtdchar.c	2001/10/02 15:05:11	1.44
+++ drivers/mtd/mtdchar.c	2001/12/04 14:13:15
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/mtd/mtd.h>
 #include <linux/slab.h>
+#include <linux/mtd/partitions.h>
 
 #ifdef CONFIG_DEVFS_FS
 #include <linux/devfs_fs_kernel.h>
@@ -284,7 +285,60 @@
 				 sizeof(struct mtd_info_user)))
 			return -EFAULT;
 		break;
+	case MEMDEFPARTTABLE:
+	{
+		struct mtd_partition_conf new_conf;
+		char *                    conf_str;
+		
+		if (! mtd->modify_part_table)
+		{
+			ret = -ENOSYS;
+			break;
+		}
+
+		if (copy_from_user(&new_conf,
+		                   (struct mtd_partition_conf *)arg,
+		                   sizeof(struct mtd_partition_conf)))
+		{
+			ret = -EFAULT;
+			break;
+		}
+		
+		if (! new_conf.size)
+		{
+			break;
+		}
+		
+		conf_str = kmalloc(new_conf.size + 1, GFP_KERNEL);
+
+		if (! conf_str)
+		{
+			ret = -ENOMEM;
+			break;
+		}
+
+		if (copy_from_user(conf_str, new_conf.conf, new_conf.size))
+		{
+			ret = -EFAULT;
+			break;
+		}
+
+		if (conf_str[new_conf.size - 1])
+		{
+			conf_str[new_conf.size] = 0;
+		}
 
+		if ((ret = mtd->modify_part_table(conf_str)) < 0)
+		{
+#if 1
+			printk(KERN_ERR "Modification of the partition table failed %d\n",
+			       ret);
+#endif
+		};
+
+		kfree(conf_str);
+		break;
+	}
 	case MEMERASE:
 	{
 		struct erase_info *erase=kmalloc(sizeof(struct erase_info),GFP_KERNEL);
Index: drivers/mtd/mtdpart.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/mtdpart.c,v
retrieving revision 1.25
diff -u -r1.25 mtdpart.c
--- drivers/mtd/mtdpart.c	2001/11/27 14:55:11	1.25
+++ drivers/mtd/mtdpart.c	2001/12/04 14:13:17
@@ -17,7 +17,18 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 
+/* Maximim length of the configuration string. */
+#define MAX_PART_CONF_STR 100
 
+/* Maximum number of sectors that device may have. */
+#define MAX_NUM_SECTORS 200
+
+#define MAX_NUM_ERASE_REGIONS 16
+
+#define MAX_NUM_PART_CHUNKS 16
+
+static char * used_sectors = 0;
+
 /* Our partition linked list */
 static LIST_HEAD(mtd_partitions);
 
@@ -25,7 +36,8 @@
 struct mtd_part {
 	struct mtd_info mtd;
 	struct mtd_info *master;
-	u_int32_t offset;
+	u_long chunk_number;
+	struct mtd_part_chunk *chunks;
 	int index;
 	struct list_head list;
 };
@@ -46,71 +58,246 @@
 			size_t *retlen, u_char *buf)
 {
 	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->read (part->master, from + part->offset, 
-					len, retlen, buf);
+	int err = 0;
+	int i;
+
+	*retlen = 0;
+	
+	for (i = 0; i < part->chunk_number && len > 0; i++) {
+		struct mtd_part_chunk *chunk = part->chunks + i;
+		size_t size, retsize;
+	
+		if (from >= chunk->size)
+			size = 0;
+		else if (from + len > chunk->size)
+			size = chunk->size - from;
+		else
+			size = len;
+
+		err = part->master->read (part->master,
+					  from + chunk->offset,
+					  size, &retsize, buf);
+
+		if (err)
+			break;
+
+		*retlen += retsize;
+		from = from > chunk->size ? from - chunk->size : 0;
+		len -= size;
+		buf += size;
+	}
+
+	return err;
 }
 
 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);
+	int err = 0;
+	int i;
+
 	if (!(mtd->flags & MTD_WRITEABLE))
 		return -EROFS;
-	if (to >= mtd->size)
-		len = 0;
-	else if (to + len > mtd->size)
-		len = mtd->size - to;
-	return part->master->write (part->master, to + part->offset, 
-					len, retlen, buf);
+
+	*retlen = 0;
+	
+	for (i = 0; i < part->chunk_number && len > 0; i++) {
+		struct mtd_part_chunk *chunk = part->chunks + i;
+		size_t size, retsize;
+	
+		if (to >= chunk->size)
+			size = 0;
+		else if (to + len > chunk->size)
+			size = chunk->size - to;
+		else
+			size = len;
+
+		err = part->master->write (part->master,
+					   to + chunk->offset,
+					   size, &retsize, buf);
+
+		if (err)
+			break;
+
+		*retlen += retsize;
+		to = to > chunk->size ? to - chunk->size : 0;
+		len -= size;
+		buf += size;
+	}
+
+	return err;
 }
 
 static int part_writev (struct mtd_info *mtd,  const struct iovec *vecs,
 			 unsigned long count, loff_t to, size_t *retlen)
 {
 	struct mtd_part *part = PART(mtd);
+	struct mtd_part_chunk *chunk = NULL;
+	int i, total_len;
+
 	if (!(mtd->flags & MTD_WRITEABLE))
 		return -EROFS;
+
+	for (i = 0; i < part->chunk_number; i++) {
+		chunk = &part->chunks[i];
+
+		if (chunk->size <= to) {
+			to -= chunk->size;
+		} else {
+			break;
+		}
+	}
+
+	if (! (i < part->chunk_number)) {
+		return -EINVAL;
+	}
+
+	total_len = 0;
+
+	for (i = 0; (i < count) && (total_len < chunk->size); i++) {
+		total_len += vecs[i].iov_len;
+	}
+
+	count = i;
+
 	return part->master->writev (part->master, vecs, count,
-					to + part->offset, retlen);
+				     to + chunk->offset, retlen);
 }
 
 static int part_readv (struct mtd_info *mtd,  struct iovec *vecs,
 			 unsigned long count, loff_t from, size_t *retlen)
 {
 	struct mtd_part *part = PART(mtd);
+	struct mtd_part_chunk *chunk = NULL;
+	int i, total_len;
+
+	for (i = 0; i < part->chunk_number; i++) {
+		chunk = &part->chunks[i];
+
+		if (chunk->size <= from) {
+			from   -= chunk->size;
+		} else {
+			break;
+		}
+	}
+
+	if (! (i < part->chunk_number)) {
+		return -EINVAL;
+	}
+
+	total_len = 0;
+
+	for (i = 0; (i < count) && (total_len < chunk->size); i++) {
+		total_len += vecs[i].iov_len;
+	}
+
+	count = i;
+
 	return part->master->readv (part->master, vecs, count,
-					from + part->offset, retlen);
+					from + chunk->offset, retlen);
 }
 
 static int part_erase (struct mtd_info *mtd, struct erase_info *instr)
 {
 	struct mtd_part *part = PART(mtd);
+	struct mtd_part_chunk *chunk = NULL;
+	int i;
+
 	if (!(mtd->flags & MTD_WRITEABLE))
 		return -EROFS;
-	if (instr->addr >= mtd->size)
+
+	for (i = 0; i < part->chunk_number; i++) {
+		chunk = &part->chunks[i];
+
+		if (chunk->size <= instr->addr) {
+			instr->addr -= chunk->size;
+		} else {
+			break;
+		}
+	}
+
+	if (! (i < part->chunk_number)) {
+		return -EINVAL;
+	}
+
+	if (instr->addr + instr->len > chunk->size) {
+		instr->len = chunk->size - instr->addr;
+	}
+
+	if (instr->addr >= chunk->size)
 		return -EINVAL;
-	instr->addr += part->offset;
+	instr->addr += chunk->offset;
+
 	return part->master->erase(part->master, instr);
 }
 
 static int part_lock (struct mtd_info *mtd, loff_t ofs, size_t len)
 {
 	struct mtd_part *part = PART(mtd);
+	int err = 0;
+	int i;
+
 	if ((len + ofs) > mtd->size) 
 		return -EINVAL;
-	return part->master->lock(part->master, ofs + part->offset, len);
+
+	for (i = 0; i < part->chunk_number && len > 0; i++) {
+		struct mtd_part_chunk *chunk = part->chunks + i;
+		size_t size;
+	
+		if (ofs >= chunk->size)
+			size = 0;
+		else if (ofs + len > chunk->size)
+			size = chunk->size - ofs;
+		else
+			size = len;
+
+		err = part->master->lock (part->master,
+					  ofs + chunk->offset,
+					  size);
+
+		if (err)
+			break;
+
+		ofs = ofs > chunk->size ? ofs - chunk->size : 0;
+		len -= size;
+	}
+
+	return err;
 }
 
 static int part_unlock (struct mtd_info *mtd, loff_t ofs, size_t len)
 {
 	struct mtd_part *part = PART(mtd);
+	int err = 0;
+	int i;
+
 	if ((len + ofs) > mtd->size) 
 		return -EINVAL;
-	return part->master->unlock(part->master, ofs + part->offset, len);
+
+	for (i = 0; i < part->chunk_number && len > 0; i++) {
+		struct mtd_part_chunk *chunk = part->chunks + i;
+		size_t size;
+	
+		if (ofs >= chunk->size)
+			size = 0;
+		else if (ofs + len > chunk->size)
+			size = chunk->size - ofs;
+		else
+			size = len;
+
+		err = part->master->unlock (part->master,
+					    ofs + chunk->offset,
+					    size);
+
+		if (err)
+			break;
+
+		ofs = ofs > chunk->size ? ofs - chunk->size : 0;
+		len -= size;
+	}
+
+	return err;
 }
 
 static void part_sync(struct mtd_info *mtd)
@@ -140,6 +327,7 @@
 {
 	struct list_head *node;
 	struct mtd_part *slave;
+	int err;
 
 	for (node = mtd_partitions.next;
 	     node != &mtd_partitions;
@@ -147,8 +335,11 @@
 		slave = list_entry(node, struct mtd_part, list);
 		if (slave->master == master) {
 			struct list_head *prev = node->prev;
+			err = del_mtd_device(&slave->mtd);
+			if (err)
+				return err;
 			__list_del(prev, node->next);
-			del_mtd_device(&slave->mtd);
+			kfree(slave->mtd.eraseregions);
 			kfree(slave);
 			node = prev;
 		}
@@ -171,13 +362,20 @@
 	struct mtd_part *slave;
 	u_int32_t cur_offset = 0;
 	int i;
+	int err;
+
+	struct mtd_erase_region_info tmp_regions[MAX_NUM_ERASE_REGIONS], *region;
+	int j, k, tmp_region_number, tmp_prev_offset, chunk_number;
+	struct mtd_erase_region_info *regions;
+	struct mtd_part_chunk *chunk;
 
 	printk (KERN_NOTICE "Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
 
 	for (i = 0; i < nbparts; i++) {
 
 		/* allocate the partition structure */
-		slave = kmalloc (sizeof(*slave), GFP_KERNEL);
+		chunk_number = parts[i].chunk_number ? parts[i].chunk_number : 1;
+		slave = kmalloc (sizeof(*slave) + chunk_number*sizeof(struct mtd_part_chunk), GFP_KERNEL);
 		if (!slave) {
 			printk ("memory allocation error while creating partitions for \"%s\"\n",
 				master->name);
@@ -190,7 +388,39 @@
 		/* 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->chunk_number = chunk_number;
+		slave->chunks = (struct mtd_part_chunk*) &slave[1];
+		if (parts[i].chunk_number)
+		{
+			for (j=0; j<slave->chunk_number; j++)
+			{
+				slave->chunks[j] = parts[i].chunks[j];
+				slave->mtd.size += slave->chunks[j].size;
+			}
+		}
+		else 
+		{
+			slave->chunks[0].offset = parts[i].offset;
+			slave->chunks[0].size   = parts[i].size;
+
+			if (parts[i].offset == MTDPART_OFS_APPEND)
+				slave->chunks[0].offset = cur_offset;
+			if (slave->offset == MTDPART_OFS_NXTBLK) {
+				u_int32_t emask = master->erasesize-1;
+				slave->offset = (cur_offset + emask) & ~emask;
+				if (slave->offset != cur_offset) {
+					printk(KERN_NOTICE
+					       "Moving partition %d: "
+					       "0x%08x -> 0x%08x\n", i,
+					       cur_offset, slave->offset);
+				}
+			}
+			if (parts[i].size == MTDPART_SIZ_FULL)
+				slave->chunks[0].size = master->size - slave->chunks[0].offset;
+			cur_offset = slave->chunks[0].offset + slave->chunks[0].size;
+
+			slave->mtd.size = slave->chunks[0].size;
+		}
 		slave->mtd.oobblock = master->oobblock;
 		slave->mtd.oobsize = master->oobsize;
 		slave->mtd.ecctype = master->ecctype;
@@ -223,79 +453,408 @@
 			slave->mtd.unlock = part_unlock;
 		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) {
-			u_int32_t emask = master->erasesize-1;
-			slave->offset = (cur_offset + emask) & ~emask;
-			if (slave->offset != cur_offset) {
-				printk(KERN_NOTICE "Moving partition %d: "
-				       "0x%08x -> 0x%08x\n", i,
-				       cur_offset, slave->offset);
-			}
+		printk (KERN_NOTICE);
+		for (j=0; j<slave->chunk_number; j++)
+		{
+			if (j != 0)
+				printk (",");
+			printk ("0x%08x-0x%08x", slave->chunks[j].offset, 
+				slave->chunks[j].offset + slave->chunks[j].size);
 		}
-		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);
+		printk (" : \"%s\"\n", slave->mtd.name);
 
 		/* let's do some sanity checks */
-		if (slave->offset >= master->size) {
+		for (j=0; j<slave->chunk_number; j++)
+		{
+			/* let's do some sanity checks */
+			if (slave->chunks[j].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++)
+				slave->mtd.size = 0;
+				slave->chunk_number = 0;
+				printk ("mtd: partition \"%s\" is out of reach -- disabled\n",
+					parts[i].name);
+			}
+			if (slave->chunks[j].offset + slave->chunks[j].size > master->size) {
+				if (j < slave->chunk_number - 1) {
+					slave->mtd.size = 0;
+					slave->chunk_number = 0;
+					printk ("mtd: partition \"%s\" extends beyond the end of device \"%s\""
+						" -- disabled\n",
+					parts[i].name, master->name);
+
+				} else {
+					slave->mtd.size -= (slave->chunks[j].offset + slave->chunks[j].size) - master->size;
+					slave->chunks[j].size = master->size - slave->chunks[j].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);
+				}
+			}
+		}
+
+		regions = master->eraseregions;
+		tmp_region_number = tmp_prev_offset = 0;
+
+		for (k=0; k<slave->chunk_number; k++)
+		{
+			 /* Find the first erase regions which is part of this partition. */
+			chunk = &slave->chunks[k];
+
+			for (j=0; j < master->numeraseregions && chunk->offset >= regions[j].offset; j++)
 				;
+
+			for (j--; j < master->numeraseregions && chunk->offset + chunk->size > regions[j].offset; j++) {
+				region = &tmp_regions[tmp_region_number++];
+
+				region->numblocks = regions[j].numblocks;
+				if (chunk->offset > regions[j].offset) {
+					region->numblocks -= (chunk->offset - regions[j].offset) / regions[j].erasesize;
+
+					if ((slave->mtd.flags & MTD_WRITEABLE) && 
+					    ((chunk->offset - regions[j].offset) % regions[j].erasesize)) {
+				                    /* Doesn't start on a boundary of major erase size */
+						slave->mtd.flags &= ~MTD_WRITEABLE;
+						printk ("mtd: partition \"%s\" doesn't start on an erase block boundary "
+							"-- force read-only\n",
+							parts[i].name);
+					}
+
+				}
 
-			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;
+				if (chunk->offset + chunk->size < regions[j].offset + regions[j].erasesize * regions[j].numblocks) {
+					region->numblocks -= ((regions[j].offset + regions[j].erasesize * regions[j].numblocks) - (chunk->offset + chunk->size)) / regions[j].erasesize;
+
+					if ((slave->mtd.flags & MTD_WRITEABLE) && 
+					    ((chunk->offset + chunk->size - regions[j].offset) % regions[j].erasesize)) {
+						slave->mtd.flags &= ~MTD_WRITEABLE;
+						printk ("mtd: partition \"%s\" doesn't end on an erase block "
+							"-- force read-only\n",
+							parts[i].name);
+					}
+				}
+				region->offset    = tmp_prev_offset;
+				region->erasesize = regions[j].erasesize;
+
+				tmp_prev_offset += region->numblocks * region->erasesize;
+				if (slave->mtd.erasesize < regions[j].erasesize) {
+					slave->mtd.erasesize = regions[j].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);
+		if (! (slave->mtd.eraseregions = kmalloc(sizeof (struct mtd_erase_region_info) * tmp_region_number, GFP_KERNEL))) {
+			kfree(slave);
+			del_mtd_partitions(master);
+			return -ENOMEM;
 		}
 
+		memmove(slave->mtd.eraseregions, tmp_regions, tmp_region_number * sizeof(struct mtd_erase_region_info));
+
+		slave->mtd.numeraseregions = tmp_region_number;
+
 		/* register our partition */
-		add_mtd_device(&slave->mtd);
+		err = add_mtd_device(&slave->mtd);
+		if (err) {
+			kfree(slave->mtd.eraseregions);
+			kfree(slave);
+			del_mtd_partitions(master);
+			return err;
+		}
 	}
 
 	return 0;
+}
+
+/* This function takes an argument of a string which contains sector numbers
+ * separated by ',' or by '-' and places apropriate list of mtd_partitions
+ * structures into part and apropriate list of mtd_flash_region structures
+ * into regions. Memory is allocated inside the function. Caller's
+ * responsibility is to free the the memory when it no longer needed.
+ */
+
+static int mtd_parse_part(char                    *  sectors,
+                          struct mtd_partition    *  part,
+			  struct mtd_info         *  driver)
+{
+	char                    * start_num;
+	char                    * end_num;
+	struct mtd_part_chunk   * next_part    = 0;
+	struct mtd_part_chunk   * temp;
+	int                       i, res       = -EINVAL;
+	int                       from, to;
+	loff_t                    offset;
+	struct mtd_info         * mymtd        = driver;
+	char                    * conv_err;
+	struct mtd_erase_region_info * region = 0;
+	struct mtd_part_chunk   part_chunks[MAX_NUM_PART_CHUNKS];
+	int part_chunk_number = 0;
+
+	if ((! sectors) || strchr(sectors, ' '))
+	{
+		printk(KERN_WARNING 
+		       "Confiduration string is empty or contains illlegal symbols\n");
+		goto Done;
+	}
+
+	for (start_num = strtok(sectors, ",");
+	     start_num != NULL;
+	     start_num = strtok(NULL, ","))
+	{
+		if ((end_num = strchr(start_num, '-')) != NULL)
+		{
+			*end_num++ = 0;
+			to = simple_strtoul(end_num, &conv_err, 10);
+			if (*conv_err != 0)
+			{
+				printk(KERN_WARNING "Conversion error: %s is invalid.\n", conv_err);
+				goto Done;
+			}
+		}
+		else
+		{
+			to = 0;
+		}
+		from = simple_strtoul(start_num, &conv_err, 10);
+		if (*conv_err != 0)
+		{
+			printk(KERN_WARNING "Conversion error: %s is invalid.\n", conv_err);
+			goto Done;
+		}
+
+
+		if ((to && to < from)       ||
+		    from >= MAX_NUM_SECTORS ||
+		    to   >= MAX_NUM_SECTORS ||
+				from <  0)
+		{
+				goto Done;
+		}
+			
+		i = from;
+		do
+		{
+			if (used_sectors[i])
+			{
+				printk(KERN_WARNING
+				       "mtd_part: Attempt to configure sector #%d to "
+				       "multiple partitions\n", i);
+				goto Done;
+			}
+			used_sectors[i++]++;
+		}
+		while (i < to || (to > 0 && i == to));
+
+		temp = &part_chunks[part_chunk_number++];
+
+		memset(temp, 0, sizeof *temp);
+
+		next_part = temp;
+
+		offset = 0;
+		for (i = 0; i < mymtd->numeraseregions; i++)
+		{
+			region = &mymtd->eraseregions[i];
+					
+			if (region->numblocks <= from)
+			{
+				from       -= region->numblocks;
+				to         -= region->numblocks;
+				offset     += region->numblocks * region->erasesize;
+			}
+			else
+			{
+				break;
+			}
+		}
+
+		if (! (i < mymtd->numeraseregions))
+		{
+			printk(KERN_WARNING "Sector seems to be out of the device\n");
+			goto Done;
+		}
+
+		next_part->offset = offset + from * region->erasesize;
+		
+		if (to <= 0)
+		{
+			next_part->size     = region->erasesize;
+		}
+		else if (to < region->numblocks)
+		{
+			next_part->size     = (to - from + 1) * region->erasesize;
+		}
+		else
+		{
+			next_part->size     =  (region->numblocks - from) * region->erasesize;
+			to                  -= region->numblocks;
+
+			for (i++; i < mymtd->numeraseregions; i++)
+			{
+				region = &mymtd->eraseregions[i];
+
+				if (region->numblocks <= to)
+				{
+					to              -= region->numblocks;
+					next_part->size += region->numblocks * region->erasesize;
+				}
+				else
+				{
+					break;
+				}
+			}
+
+			if (! (i < mymtd->numeraseregions))
+		        {
+				printk(KERN_WARNING "Sector seems to be out of the device\n");
+				goto Done;
+			}
+
+			next_part->size += (to + 1) * region->erasesize;
+		}
+	}
+
+	res = 0;
+
+	part->chunks = kmalloc(sizeof(struct mtd_part_chunk) * 
+			       part_chunk_number, GFP_KERNEL);
+
+	if (!part->chunks)
+	{
+		res = -ENOMEM;
+		goto Done;
+	}
+
+	part->chunk_number = part_chunk_number;
+	part->name         = driver->name;
+
+	memmove(part->chunks, part_chunks, 
+		part_chunk_number * sizeof(struct mtd_part_chunk));
+		
+	Done:
+	return res;
+}
+
+static int find_mtd_partitions(struct mtd_info *master, struct mtd_info **parts)
+{
+	struct list_head *node;
+	struct mtd_part *slave;
+	int num = 0;
+
+	for (node = mtd_partitions.next;
+	     node != &mtd_partitions;
+	     node = node->next) {
+		slave = list_entry(node, struct mtd_part, list);
+		if (slave->master == master) {
+			parts[num++] = &slave->mtd;
+		}
+	}
+
+	return num;
+}
+
+int mtd_create_partitions(const char      *  layout,
+                          struct mtd_info ** part_mtds,
+                          int                max_num,
+                          struct mtd_info *  driver)
+{
+	char                    * next_part;
+	int                       i;
+	struct mtd_partition    parts[max_num];
+	int                       err               = 0;
+	int                       res               = 0;
+	char                    * layout_conf;
+
+	if (! (layout_conf = kmalloc(MAX_PART_CONF_STR, GFP_KERNEL)))
+	{
+		err = -ENOMEM;
+		goto Done;
+	}
+
+	if (! (used_sectors = kmalloc(MAX_NUM_SECTORS, GFP_KERNEL)))
+	{
+		err = -ENOMEM;
+		goto Done;
+	}
+
+	memset(parts, 0, sizeof parts);
+	memset(used_sectors, 0, MAX_NUM_SECTORS);
+
+	strncpy(layout_conf, layout, MAX_PART_CONF_STR);
+	layout_conf[MAX_PART_CONF_STR - 1] = 0;
+
+	for (next_part  = strtok(layout_conf, ":"), i = 0;
+	     next_part != NULL;
+	     next_part  = strtok(NULL, ":"), i++)
+	{
+		char * save_strtok;
+
+		if (*next_part == 0)
+		{
+				/* This is mainly to support the default value of ":" for the
+				 * configuration option.
+				 */
+			continue;
+		}
+
+		
+			/* Here we should save __strtok pointer as inside the parse 
+			 * function strtok() is called again.
+			 */
+		save_strtok = ___strtok;
+		if ((err = mtd_parse_part(next_part, &parts[i], driver)) < 0) 
+		{
+			printk(KERN_WARNING
+			       "Parsing of configuration for part %d failed.\n", i + 1);
+			goto Done;
+		}
+		___strtok = save_strtok;
+
+		res++;
+	}
+
+	if (! res)
+	{
+		goto Done;
+	}
+
+#if 0
+	printk(KERN_INFO "Configured partitions: %d Registering MTDs\n", res);
+#endif
+
+	err = add_mtd_partitions(driver, parts, res);
+
+	Done:
+
+	for (i=0; i<res; i++)
+	{
+		kfree(parts[i].chunks);
+	}
+
+	if (err)
+	{
+		printk(KERN_WARNING 
+		       "Initialization of the partitions failed: err = %d\n", err);
+
+		if (layout_conf)
+		{
+			kfree(layout_conf);
+		}
+
+		if (used_sectors)
+		{
+			kfree(used_sectors);
+			used_sectors = 0;
+		}
+
+		res = err;
+
+	} else
+		find_mtd_partitions(driver, part_mtds);
+	
+	return res;
 }
 
 EXPORT_SYMBOL(add_mtd_partitions);
Index: include/linux/mtd/mtd.h
===================================================================
RCS file: /home/cvs/mtd/include/linux/mtd/mtd.h,v
retrieving revision 1.34
diff -u -r1.34 mtd.h
--- include/linux/mtd/mtd.h	2001/11/27 14:55:12	1.34
+++ include/linux/mtd/mtd.h	2001/12/04 14:13:19
@@ -93,6 +93,7 @@
 #define MEMUNLOCK               _IOW('M', 6, struct erase_info_user)
 #define MEMGETREGIONCOUNT	_IOR('M', 7, int)
 #define MEMGETREGIONINFO	_IOWR('M', 8, struct region_info_user)
+#define MEMDEFPARTTABLE         _IOW('M', 9, struct mtd_partition_conf)
 
 #ifndef __KERNEL__
 
@@ -152,9 +153,7 @@
 	char *name;
 	int index;
 
-	/* Data for variable erase regions. If numeraseregions is zero,
-	 * it means that the whole device has erasesize as given above. 
-	 */
+	/* Data for variable erase regions. */
 	int numeraseregions;
 	struct mtd_erase_region_info *eraseregions; 
 
@@ -211,6 +210,14 @@
 	int (*suspend) (struct mtd_info *mtd);
 	void (*resume) (struct mtd_info *mtd);
 
+	/* A callback to the driver that will cause reconfiguring of the
+	   partitions.
+	 */
+	int (*modify_part_table) (char * new_table);
+
+	/* Partition usage counter. */
+	int usage_counter;
+
 	void *priv;
 };
 
@@ -231,13 +238,18 @@
 	if (ret && ret->module && !try_inc_mod_count(ret->module))
 		return NULL;
 
+	if (ret)
+		ret->usage_counter++;
+
 	return ret;
 }
 
 static inline void put_mtd_device(struct mtd_info *mtd)
 {
-       if (mtd->module)
-	       __MOD_DEC_USE_COUNT(mtd->module);
+	if (mtd->module)
+		__MOD_DEC_USE_COUNT(mtd->module);
+	if (mtd->usage_counter)
+		mtd->usage_counter--;
 }
 
 
Index: include/linux/mtd/partitions.h
===================================================================
RCS file: /home/cvs/mtd/include/linux/mtd/partitions.h,v
retrieving revision 1.7
diff -u -r1.7 partitions.h
--- include/linux/mtd/partitions.h	2001/11/07 01:20:59	1.7
+++ include/linux/mtd/partitions.h	2001/12/04 14:13:19
@@ -36,9 +36,22 @@
  * erasesize aligned (e.g. use MTDPART_OFS_NEXTBLK).
  */ 
 
+struct mtd_part_chunk {
+	u_long size;		/* partition size */
+	u_long offset;		/* offset within the master MTD space */
+};
+
+struct mtd_partition_conf
+{
+	char * conf;
+	int    size;
+};
+
 struct mtd_partition {
 	char *name;		/* identifier string */
 	u_int32_t size;		/* partition size */
+	int chunk_number;
+	struct mtd_part_chunk *chunks;
 	u_int32_t offset;		/* offset within the master MTD space */
 	u_int32_t mask_flags;	/* master MTD flags to mask out for this partition */
 };
@@ -48,6 +61,10 @@
 #define MTDPART_SIZ_FULL	(0)
 
 
+int mtd_create_partitions(const char      *  layout,
+                          struct mtd_info ** part_mtds,
+                          int                max_num,
+                          struct mtd_info *  driver);
 int add_mtd_partitions(struct mtd_info *, struct mtd_partition *, int);
 int del_mtd_partitions(struct mtd_info *);
 

[-- Attachment #3: Type: TEXT/PLAIN, Size: 4865 bytes --]

/*
 * 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.
 * 
 * iq80310.c
 */

#include <linux/module.h>
#include <linux/types.h>
#include <linux/kernel.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/map.h>
#include <linux/config.h>
#include <asm/arch/hardware.h>

#include <linux/mtd/partitions.h>

#ifndef CONFIG_MTD_IQ80310_PART
#define CONFIG_MTD_IQ80310_PART ":"
#endif

static struct mtd_info * mtd_part_mtds[4];
static int               configured_parts;
char                   * iq80310_mtd_part_conf = CONFIG_MTD_IQ80310_PART;
static int               runtime_part_conf(char* str);

static struct mtd_info *mymtd;

__u8 iq80310_read8(struct map_info *map, unsigned long ofs)
{
	return *(__u8 *)(map->map_priv_1 + ofs);
}

__u16 iq80310_read16(struct map_info *map, unsigned long ofs)
{
	return *(__u16 *)(map->map_priv_1 + ofs);
}

__u32 iq80310_read32(struct map_info *map, unsigned long ofs)
{
	return *(__u32 *)(map->map_priv_1 + ofs);
}

void iq80310_copy_from(struct map_info * map,
                       void            * to,
                       unsigned long     from,
                       ssize_t           len)
{
	memcpy(to, (void *)(map->map_priv_1 + from), len);
}

void iq80310_write8(struct map_info *map, __u8 d, unsigned long addr)
{
	__u32 final_addr = map->map_priv_2 + addr;

	*(__u8 *)final_addr = d;
}

void iq80310_write16(struct map_info *map, __u16 d, unsigned long addr)
{
	panic("iq80310_write16: 16-bit write to Flash attempted\n");
}

void iq80310_write32(struct map_info *map, __u32 d, unsigned long addr)
{
	panic("iq80310_write32: 32-bit write to Flash attemped\n");
}

struct map_info iq80310_map = {
	name:		"Flash on the IQ80310 board",
	size:		FLASH_SIZE,
	buswidth:	1,
	read8:		iq80310_read8,
	read16:		iq80310_read16,
	read32:		iq80310_read32,
	copy_from:	iq80310_copy_from,
	write8:		iq80310_write8,
	write16:	iq80310_write16,
	write32:	iq80310_write32,
#if 0
	copy_to:	iq80310_copy_to,
#else
	copy_to:	0,
#endif
	map_priv_1:	FLASH_BASE,
	map_priv_2:	FLASH_BASE
};

#if LINUX_VERSION_CODE < 0x20300
#ifdef MODULE
#define init_iq80310 init_module
#define cleanup_iq80310 cleanup_module
#endif
#endif

int __init init_iq80310(void)
{
	int res = -ENXIO;

	printk("Flash mapping for the IQ80310 board initialized "
	       "starting at address 0x%lx\n", iq80310_map.map_priv_1);

	if (! (mymtd = do_cfi_probe(&iq80310_map)))
	{
		goto Done;
	}

#ifdef MODULE
	mymtd->module = &__this_module;
#endif

	mymtd->modify_part_table = runtime_part_conf;

	add_mtd_device(mymtd);

	printk("IQ80310 Flash MTD driver: Configuration of partitions "
	       "is %s\n", iq80310_mtd_part_conf);
	configured_parts = mtd_create_partitions(iq80310_mtd_part_conf,
	                                         mtd_part_mtds,
	                                         4,
	                                         mymtd);
	if (configured_parts < 0)
	{
		printk("IQ80310 Flash MTD driver: mtd_create_partitions() failed, "
		       "err = %d\n", configured_parts);
		configured_parts = 0;
	}
	else
	{
		printk("IQ80310 Flash MTD driver: Configured %d partitions\n",
		       configured_parts);
	}

	res = 0;
	Done:
	return res;
}

static void __exit cleanup_iq80310(void)
{
	if (mymtd) {
		del_mtd_partitions(mymtd);
		del_mtd_device(mymtd);
		map_destroy(mymtd);
	}
}

static int __init iq80310_mtd_part_conf_setup(char *str)
{
	int n;

	if (! str)
	{
		return 0;
	}
	if (*str == '\'' || *str == '\"')
	{
		str++;
	}
	n = strlen(str);
	if (str[n - 1] == '\'' || str[n - 1] == '\"')
	{
		str[n - 1] = 0;
	}
	iq80310_mtd_part_conf = str;

	return 1;
}

__setup("iq80310_part_conf=", iq80310_mtd_part_conf_setup);

static int runtime_part_conf(char* str)
{
	int res = 0;

	res = del_mtd_partitions(mymtd);
	if (res)
	{
		goto Done;
	}

	printk("IQ80310 Flash MTD driver: Configuration of partitions "
	       "is %s\n", str);
	configured_parts = mtd_create_partitions(str,
	                                         mtd_part_mtds,
	                                         4,
	                                         mymtd);
	if (configured_parts < 0)
	{
		printk("IQ80310 Flash MTD driver: mtd_create_partitions() "
		       "failed, err = %d\n", configured_parts);
		res = configured_parts;
		configured_parts = 0;
	}
	else
	{
		printk("IQ80310 Flash MTD driver: Configured %d partitions\n",
		       configured_parts);
	}

	Done:
	return res;
}	

module_init(init_iq80310);
module_exit(cleanup_iq80310);

	/* End of File.
	 */

Re: JFFS1/MTD bug-fixes

[Aleksander Sanochkin]

Hi!

>
>
> Hi!
>
> > > We have also added a possibility for run-time partitioning of an
> > > MTD device, and use this usage_counter to prevent deleting of a
> > > partition while it is being used by some process.
> >
> > In that context, I agree that it makes sense. Have you seen Jörn
Engel's
> > rewrite of the partitioning code, which I was intending to look at
merging
> > in 2.5? It would be useful to make that does what you require before
we
> > merge it.
>
> http://wh.fh-wedel.de/~joern/software/kernel/
> http://wh.fh-wedel.de/~joern/software/kernel/mtd.patch.core.2.4.15
>
> Not having seen any code, I fear that your patch won't easily merge
> with mine. But I would like to have a look - it might get rid of some
> dear old problems, commonly known as reboot. :)

Yes, it won't be easy to merge. Though it won't be harder than to merge
your patch with the current MTD code.

Re: JFFS1/MTD bug-fixes

[Jörn Engel]

Hi!

> > Not having seen any code, I fear that your patch won't easily merge
> > with mine. But I would like to have a look - it might get rid of some
> > dear old problems, commonly known as reboot. :)
> 
> Yes, it won't be easy to merge. Though it won't be harder than to merge
> your patch with the current MTD code.

As this is the first real feedback I got, did you have problems
merging the patch with the current MTD code?

I am basically creating those blindly and untested, as my companies
codebase has diverged quite far from the official kernels. It's a
mess and noone is willing to fix it. :(

Jörn

-- 
Open your arms to change, but don't let go
of your values.

Re: JFFS1/MTD bug-fixes

[Jörn Engel]

Hi!

> Yes, I've looked through Jörn Engel's code. Our modifications are less
> significant and are based on other goals in mind.
> 
> Primary goal was to allow for creating Flash partitions that consist of
> non-consequent sectors. For instance, the user may wish to chose sector
> 2 for storing some binary data and use sectors 1,3,4..... as a single
> JFFS partition.
> 
> Another goal was a possibility of flexible (re-)configration of the
> partitions, without having to change any C code. Supported methods are:
> build-time (via a CONFIG_... define), boot-time (via a kernel command
> line), and run-time (via an mtdchar IOCTL command). With all the three
> methods, the user uses a string notation to specify desired layout of the
> partitions. For instance, "0,3-8:1,2:9-25" means three partitions, with
> sectors 0 and 3 to 8 belonging to the first partition, 1 and 2 to the
> second, and 9 to 25 to the third.

We already have partitioning that is independent of the kernels normal
partitioning. Now you send in the equivalent of the logical volume
manager. The mtd code is getting there. :)

What are your reasons to use mtdchar ioctl()s for runtime
repartitioning? This should be pretty independent of the char device,
(caching) block device or whatever the future might still bring. Why
not a /proc interface?

Apart from that the design looks pretty sane. I will look through the
code and try to merge it with mine as soon as my workload drops to an
acceptable value again, most likely around christmas.

Joern

Re: JFFS1/MTD bug-fixes

[Aleksander Sanochkin]

> Hi!
>
> > > Not having seen any code, I fear that your patch won't easily merge
> > > with mine. But I would like to have a look - it might get rid of
some
> > > dear old problems, commonly known as reboot. :)
> >
> > Yes, it won't be easy to merge. Though it won't be harder than to
merge
> > your patch with the current MTD code.
>
> As this is the first real feedback I got, did you have problems
> merging the patch with the current MTD code?

No, I didn't apply your patch actually. What I meant was that your changes
to the MTD code are quite significant, and merging them with our updates
to MTD doesn't appear to be harder than doing so with the current MTD
code.

Best regards.

Re: JFFS1/MTD bug-fixes

[Aleksander Sanochkin]

> Hi!
>
> > Yes, I've looked through Jörn Engel's code. Our modifications are less
> > significant and are based on other goals in mind.
> >
> > Primary goal was to allow for creating Flash partitions that consist
of
> > non-consequent sectors. For instance, the user may wish to chose
sector
> > 2 for storing some binary data and use sectors 1,3,4..... as a single
> > JFFS partition.
> >
> > Another goal was a possibility of flexible (re-)configration of the
> > partitions, without having to change any C code. Supported methods
are:
> > build-time (via a CONFIG_... define), boot-time (via a kernel command
> > line), and run-time (via an mtdchar IOCTL command). With all the three
> > methods, the user uses a string notation to specify desired layout of
the
> > partitions. For instance, "0,3-8:1,2:9-25" means three partitions,
with
> > sectors 0 and 3 to 8 belonging to the first partition, 1 and 2 to the
> > second, and 9 to 25 to the third.
>
> We already have partitioning that is independent of the kernels normal
> partitioning. Now you send in the equivalent of the logical volume
> manager. The mtd code is getting there. :)
>
> What are your reasons to use mtdchar ioctl()s for runtime
> repartitioning? This should be pretty independent of the char device,
> (caching) block device or whatever the future might still bring. Why
> not a /proc interface?

One reason was that the user will most likely have mtdchar in the kernel
anyway, if he wants to use a tool for erasing Flash, but he may want
not to include /proc in the kernel for some reasons. So, we found it
more logical to have an IOCTL command for the repartitioning.

Best regards.

Re: JFFS1/MTD bug-fixes

[Jörn Engel]

Hi!

> > What are your reasons to use mtdchar ioctl()s for runtime
> > repartitioning? This should be pretty independent of the char device,
> > (caching) block device or whatever the future might still bring. Why
> > not a /proc interface?

Au contraire, at least in my case. I do use /proc, but not mtdchar,
since mtdblock is more convenient. A more important point is that
Linus generally favors /proc over ioctl()s, iirc.
Just a suggestion, though.

Jörn

-- 
A security system is only as secure as its secret. 
Beware of pseudo-secrets.

Re: JFFS1/MTD bug-fixes

[Jörn Engel]

Hi!

> No, I didn't apply your patch actually. What I meant was that your changes
> to the MTD code are quite significant, and merging them with our updates
> to MTD doesn't appear to be harder than doing so with the current MTD
> code.

Sneaky one, now I understand. But merging my changes with the current
MTD code is finished, whereas merging them with your updates is new
work to be done.
The real question is, which patch will be merged first. If you are
second, the work is on you. ;-)

But as I already said, I will try to do this around Christmas. My
laptop is ordered, better wish me luck that it arrives in time.

Jörn

-- 
Treating your users as co-developers is your least-hassle 
route to rapid code improvement and effective debugging.