[PATCH] MTD: mtdconcat NAND/Sibley support (revised)

[PATCH] MTD: mtdconcat NAND/Sibley support (revised)

[Belyakov, Alexander]

My previous message did not appear in mailing list. So I repost it.

This patch adds concat_writev(), concat_writev_ecc(),
concat_block_isbad(), concat_block_markbad() functions to make
concatenation layer compatible with Sibley and NAND chips.

The patch has been changed to use writev() functions of underlying
devices.

It has been verified on both Sibley and NAND before posting.


Signed-off-by: Alexander Belyakov <alexander.belyakov@intel.com>

-------

diff -uNr a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
--- a/drivers/mtd/mtdconcat.c	2006-04-07 20:56:47.000000000 +0400
+++ b/drivers/mtd/mtdconcat.c	2006-05-04 12:41:19.000000000 +0400
@@ -45,6 +45,13 @@
 #define CONCAT(x)  ((struct mtd_concat *)(x))
 
 /*
+ * Forward function declaration
+ */
+static int
+concat_writev_ecc (struct mtd_info *mtd, const struct kvec *vecs,
unsigned long count, 
+		loff_t to, size_t * retlen, u_char *eccbuf, struct
nand_oobinfo *oobsel);
+
+/*
  * MTD methods which look up the relevant subdevice, translate the
  * effective address and pass through to the subdevice.
  */
@@ -140,6 +147,14 @@
 	return err;
 }
 
+
+static int
+concat_writev (struct mtd_info *mtd, const struct kvec *vecs, unsigned
long count, 
+		loff_t to, size_t * retlen)
+{
+	return (concat_writev_ecc(mtd, vecs, count, to, retlen, NULL,
NULL));
+}
+
 static int
 concat_read_ecc(struct mtd_info *mtd, loff_t from, size_t len,
 		size_t * retlen, u_char * buf, u_char * eccbuf,
@@ -251,6 +266,107 @@
 }
 
 static int
+concat_writev_ecc (struct mtd_info *mtd, const struct kvec *vecs,
unsigned long count, 
+		loff_t to, size_t * retlen, u_char *eccbuf, struct
nand_oobinfo *oobsel)
+{
+	struct mtd_concat *concat = CONCAT(mtd);
+	struct kvec *vecs_copy;
+	unsigned long entry_low, entry_high;
+	size_t total_len = 0;
+	int i;
+	int err = -EINVAL;
+	
+	if (!(mtd->flags & MTD_WRITEABLE))
+		return -EROFS;
+
+	*retlen = 0;
+
+	/* Calculate total length of data */
+	for (i = 0; i < count; i++)
+		total_len += vecs[i].iov_len;
+		
+	/* Do not allow write past end of page */
+	if ((to + total_len) > mtd->size) {
+		DEBUG (MTD_DEBUG_LEVEL0, "concat_writev_ecc(): Attempted
write past end of device\n");
+		return -EINVAL;
+	}
+    
+	/* Check alignment */
+	if (concat->mtd.type == MTD_NANDFLASH) {
+    		if ((to & (mtd->oobblock - 1)) || (total_len &
(mtd->oobblock - 1))) {
+			DEBUG (MTD_DEBUG_LEVEL0, "concat_writev_ecc():
Attempted write not aligned data!\n");
+			return -EINVAL;
+		}
+	}
+	
+	/* make a copy of vecs */
+	vecs_copy = kmalloc(sizeof(struct kvec) * count, GFP_KERNEL);
+	if (!vecs_copy)
+		return -ENOMEM;
+	memcpy(vecs_copy, vecs, sizeof(struct kvec) * count);
+
+	entry_low = 0;
+	for (i = 0; i < concat->num_subdev; i++) {
+		struct mtd_info *subdev = concat->subdev[i];
+		size_t size, wsize, retsize, old_iov_len;
+
+		if (to >= subdev->size) {
+			size = 0;
+			to -= subdev->size;
+			continue;
+		}
+		
+		if (to + total_len > subdev->size)
+			size = subdev->size - to;
+		else
+			size = total_len;
+		wsize = size; /* store for future use */
+
+		entry_high = entry_low;
+		while (entry_high < count) {
+			if (size > vecs_copy[entry_high].iov_len)
+				size -= vecs_copy[entry_high++].iov_len;
+			else
+				break;
+		}
+		
+		old_iov_len = vecs_copy[entry_high].iov_len;
+		vecs_copy[entry_high].iov_len = size;
+		
+		if (!(subdev->flags & MTD_WRITEABLE))
+			err = -EROFS;
+		else if (eccbuf)
+			err = subdev->writev_ecc(subdev,
&vecs_copy[entry_low],
+				entry_high - entry_low + 1, to,
&retsize, eccbuf, oobsel);
+		else
+			err = subdev->writev(subdev,
&vecs_copy[entry_low],
+				entry_high - entry_low + 1, to,
&retsize);
+
+		vecs_copy[entry_high].iov_len = old_iov_len - size;
+		vecs_copy[entry_high].iov_base = (void *)((u_char
*)vecs_copy[entry_high].iov_base + size);
+		
+		entry_low = entry_high;
+			
+		if (err)
+			break;
+
+		*retlen += retsize;
+		total_len -= wsize;
+		if (concat->mtd.type == MTD_NANDFLASH && eccbuf)
+			eccbuf += mtd->oobavail * (wsize /
mtd->oobblock);
+
+		if (total_len == 0)
+			break;
+
+		err = -EINVAL;
+		to = 0;
+	}
+	
+	kfree(vecs_copy);
+	return err;
+}
+
+static int
 concat_read_oob(struct mtd_info *mtd, loff_t from, size_t len,
 		size_t * retlen, u_char * buf)
 {
@@ -638,6 +754,60 @@
 	}
 }
 
+static int concat_block_isbad(struct mtd_info *mtd, loff_t ofs)
+{
+	struct mtd_concat *concat = CONCAT(mtd);
+	int i, res = 0;
+	
+	if (!concat->subdev[0]->block_isbad)
+		return res;
+
+	if (ofs > mtd->size)
+		return -EINVAL;
+		
+	for (i = 0; i < concat->num_subdev; i++) {
+		struct mtd_info *subdev = concat->subdev[i];
+
+		if (ofs >= subdev->size) {
+			ofs -= subdev->size;
+			continue;
+		}
+		
+		res = subdev->block_isbad(subdev, ofs);
+
+		break;
+	}
+
+	return res;
+}
+
+static int concat_block_markbad(struct mtd_info *mtd, loff_t ofs)
+{
+	struct mtd_concat *concat = CONCAT(mtd);
+	int i, err = -EINVAL;
+	
+	if (!concat->subdev[0]->block_markbad)
+		return 0;
+
+	if (ofs > mtd->size)
+		return -EINVAL;
+		
+	for (i = 0; i < concat->num_subdev; i++) {
+		struct mtd_info *subdev = concat->subdev[i];
+
+		if (ofs >= subdev->size) {
+			ofs -= subdev->size;
+			continue;
+		}
+		
+		err = subdev->block_markbad(subdev, ofs);
+
+		break;
+	}
+
+	return err;
+}
+
 /*
  * This function constructs a virtual MTD device by concatenating
  * num_devs MTD devices. A pointer to the new device object is
@@ -687,10 +857,18 @@
 		concat->mtd.read_ecc = concat_read_ecc;
 	if (subdev[0]->write_ecc)
 		concat->mtd.write_ecc = concat_write_ecc;
+	if (subdev[0]->writev)
+		concat->mtd.writev = concat_writev;
+	if (subdev[0]->writev_ecc)
+		concat->mtd.writev_ecc = concat_writev_ecc;
 	if (subdev[0]->read_oob)
 		concat->mtd.read_oob = concat_read_oob;
 	if (subdev[0]->write_oob)
 		concat->mtd.write_oob = concat_write_oob;
+	if (subdev[0]->block_isbad)
+		concat->mtd.block_isbad = concat_block_isbad;
+	if (subdev[0]->block_markbad)
+		concat->mtd.block_markbad = concat_block_markbad;
 
 	concat->subdev[0] = subdev[0];
 
@@ -736,14 +914,12 @@
 
 	}
 
+	if(concat->mtd.type == MTD_NANDFLASH)
+
memcpy(&concat->mtd.oobinfo,&subdev[0]->oobinfo,sizeof(struct
nand_oobinfo));
+
 	concat->num_subdev = num_devs;
 	concat->mtd.name = name;
 
-	/*
-	 * NOTE: for now, we do not provide any readv()/writev() methods
-	 *       because they are messy to implement and they are not
-	 *       used to a great extent anyway.
-	 */
 	concat->mtd.erase = concat_erase;
 	concat->mtd.read = concat_read;
 	concat->mtd.write = concat_write;



-------
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here
in no way represent Intel's position on the issue

Re: [PATCH] MTD: mtdconcat NAND/Sibley support (revised)

[Nicolas Pitre]

On Fri, 5 May 2006, Belyakov, Alexander wrote:

> This patch adds concat_writev(), concat_writev_ecc(),
> concat_block_isbad(), concat_block_markbad() functions to make
> concatenation layer compatible with Sibley and NAND chips.
> 
> The patch has been changed to use writev() functions of underlying
> devices.

>From a quick glance at the writev part it looks much better than your 
former version.


Nicolas

Re: [PATCH] MTD: mtdconcat NAND/Sibley support (revised)

[Alexander Belyakov]

Nicolas Pitre wrote:
>
> On Fri, 5 May 2006, Belyakov, Alexander wrote:
>
> > This patch adds concat_writev(), concat_writev_ecc(),
> > concat_block_isbad(), concat_block_markbad() functions to make
> > concatenation layer compatible with Sibley and NAND chips.
> >
> > The patch has been changed to use writev() functions of underlying
> > devices.
>
> From a quick glance at the writev part it looks much better than your
> former version.
>

Is there any chance to see this patch committed?

Thanks,
Alexander

Re: [PATCH] MTD: mtdconcat NAND/Sibley support (revised)

[Jörn Engel]

On Fri, 5 May 2006 16:08:21 +0400, Belyakov, Alexander wrote:
> 
> +concat_writev_ecc (struct mtd_info *mtd, const struct kvec *vecs,
> unsigned long count, 

Your mailer is eating this patch.  Can you either fix your mailer and
resend, resend with a different mailer or resend as attachment?
Inline patch is preferred.

Also, the lines were >80 columns before your mailer mangled them.

> +	return (concat_writev_ecc(mtd, vecs, count, to, retlen, NULL,
> NULL));

Remove redundant brackets.

> +concat_writev_ecc (struct mtd_info *mtd, const struct kvec *vecs,
                    ^
Remove space

> unsigned long count, 
> +		loff_t to, size_t * retlen, u_char *eccbuf, struct
                                   ^
and here.

> +	/* Calculate total length of data */
> +	for (i = 0; i < count; i++)
> +		total_len += vecs[i].iov_len;
> +		
   ^^^^^^^^^^^^^
Remove whitespace damage

> +		DEBUG (MTD_DEBUG_LEVEL0, "concat_writev_ecc(): Attempted
                     ^
> +    
   ^^^^

etc.

For some things, I'd like to apply the patch and use tkdiff to see
more than three lines of context.  So this is not a final review, but
just a "resend with sane mailer" plus a few cosmetics, while we're at
it.

Jörn

-- 
Beware of bugs in the above code; I have only proved it correct, but
not tried it.
-- Donald Knuth

Re: [PATCH] MTD: mtdconcat NAND/Sibley support (revised)

[Alexander Belyakov]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

Jorn Engel wrote:
>
> On Fri, 5 May 2006 16:08:21 +0400, Belyakov, Alexander wrote:
> >
> > +concat_writev_ecc (struct mtd_info *mtd, const struct kvec *vecs,
> > unsigned long count,
>
> Your mailer is eating this patch.  Can you either fix your mailer and
> resend, resend with a different mailer or resend as attachment?
> Inline patch is preferred.
>

One of my mailers corrupts headers, so messages are rejected by MTD list 
admin. Another one deals with headers, but converts tabs to whitespaces. 
And I don't know how to fix them at the moment. Sorry.

So I'm attaching patch file to this message to be sure it will not get 
corrupted.

Thanks,
Alexander

[-- Attachment #2: mtd-concat-nandfix.patch --]
[-- Type: text/plain, Size: 5830 bytes --]

diff -uNr a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
--- a/drivers/mtd/mtdconcat.c	2006-04-07 20:56:47.000000000 +0400
+++ b/drivers/mtd/mtdconcat.c	2006-05-15 17:07:05.000000000 +0400
@@ -45,6 +45,14 @@
 #define CONCAT(x)  ((struct mtd_concat *)(x))
 
 /*
+ * Forward function declaration
+ */
+static int
+concat_writev_ecc(struct mtd_info *mtd, const struct kvec *vecs,
+		unsigned long count, loff_t to, size_t * retlen,
+		u_char *eccbuf, struct nand_oobinfo *oobsel);
+
+/*
  * MTD methods which look up the relevant subdevice, translate the
  * effective address and pass through to the subdevice.
  */
@@ -140,6 +148,14 @@
 	return err;
 }
 
+
+static int
+concat_writev(struct mtd_info *mtd, const struct kvec *vecs,
+		unsigned long count, loff_t to, size_t * retlen)
+{
+	return concat_writev_ecc(mtd, vecs, count, to, retlen, NULL, NULL);
+}
+
 static int
 concat_read_ecc(struct mtd_info *mtd, loff_t from, size_t len,
 		size_t * retlen, u_char * buf, u_char * eccbuf,
@@ -251,6 +267,107 @@
 }
 
 static int
+concat_writev_ecc(struct mtd_info *mtd, const struct kvec *vecs,
+		unsigned long count, loff_t to, size_t * retlen,
+		u_char *eccbuf, struct nand_oobinfo *oobsel)
+{
+	struct mtd_concat *concat = CONCAT(mtd);
+	struct kvec *vecs_copy;
+	unsigned long entry_low, entry_high;
+	size_t total_len = 0;
+	int i;
+	int err = -EINVAL;
+
+	if (!(mtd->flags & MTD_WRITEABLE))
+		return -EROFS;
+
+	*retlen = 0;
+
+	/* Calculate total length of data */
+	for (i = 0; i < count; i++)
+		total_len += vecs[i].iov_len;
+
+	/* Do not allow write past end of page */
+	if ((to + total_len) > mtd->size)
+		return -EINVAL;
+
+	/* Check alignment */
+	if (concat->mtd.type == MTD_NANDFLASH) {
+    		if ((to & (mtd->oobblock - 1)) ||
+		    (total_len & (mtd->oobblock - 1)))
+			return -EINVAL;
+	}
+
+	/* make a copy of vecs */
+	vecs_copy = kmalloc(sizeof(struct kvec) * count, GFP_KERNEL);
+	if (!vecs_copy)
+		return -ENOMEM;
+	memcpy(vecs_copy, vecs, sizeof(struct kvec) * count);
+
+	entry_low = 0;
+	for (i = 0; i < concat->num_subdev; i++) {
+		struct mtd_info *subdev = concat->subdev[i];
+		size_t size, wsize, retsize, old_iov_len;
+
+		if (to >= subdev->size) {
+			size = 0;
+			to -= subdev->size;
+			continue;
+		}
+
+		if (to + total_len > subdev->size)
+			size = subdev->size - to;
+		else
+			size = total_len;
+		wsize = size; /* store for future use */
+
+		entry_high = entry_low;
+		while (entry_high < count) {
+			if (size > vecs_copy[entry_high].iov_len)
+				size -= vecs_copy[entry_high++].iov_len;
+			else
+				break;
+		}
+
+		old_iov_len = vecs_copy[entry_high].iov_len;
+		vecs_copy[entry_high].iov_len = size;
+
+		if (!(subdev->flags & MTD_WRITEABLE))
+			err = -EROFS;
+		else if (eccbuf)
+			err = subdev->writev_ecc(subdev, &vecs_copy[entry_low],
+				entry_high - entry_low + 1, to, &retsize,
+				eccbuf, oobsel);
+		else
+			err = subdev->writev(subdev, &vecs_copy[entry_low],
+				entry_high - entry_low + 1, to, &retsize);
+
+		vecs_copy[entry_high].iov_len = old_iov_len - size;
+		vecs_copy[entry_high].iov_base = vecs_copy[entry_high].iov_base
+						+ size;
+
+		entry_low = entry_high;
+			
+		if (err)
+			break;
+
+		*retlen += retsize;
+		total_len -= wsize;
+		if (concat->mtd.type == MTD_NANDFLASH && eccbuf)
+			eccbuf += mtd->oobavail * (wsize / mtd->oobblock);
+
+		if (total_len == 0)
+			break;
+
+		err = -EINVAL;
+		to = 0;
+	}
+
+	kfree(vecs_copy);
+	return err;
+}
+
+static int
 concat_read_oob(struct mtd_info *mtd, loff_t from, size_t len,
 		size_t * retlen, u_char * buf)
 {
@@ -638,6 +755,58 @@
 	}
 }
 
+static int concat_block_isbad(struct mtd_info *mtd, loff_t ofs)
+{
+	struct mtd_concat *concat = CONCAT(mtd);
+	int i, res = 0;
+
+	if (!concat->subdev[0]->block_isbad)
+		return res;
+
+	if (ofs > mtd->size)
+		return -EINVAL;
+		
+	for (i = 0; i < concat->num_subdev; i++) {
+		struct mtd_info *subdev = concat->subdev[i];
+
+		if (ofs >= subdev->size) {
+			ofs -= subdev->size;
+			continue;
+		}
+
+		res = subdev->block_isbad(subdev, ofs);
+		break;
+	}
+
+	return res;
+}
+
+static int concat_block_markbad(struct mtd_info *mtd, loff_t ofs)
+{
+	struct mtd_concat *concat = CONCAT(mtd);
+	int i, err = -EINVAL;
+
+	if (!concat->subdev[0]->block_markbad)
+		return 0;
+
+	if (ofs > mtd->size)
+		return -EINVAL;
+
+	for (i = 0; i < concat->num_subdev; i++) {
+		struct mtd_info *subdev = concat->subdev[i];
+
+		if (ofs >= subdev->size) {
+			ofs -= subdev->size;
+			continue;
+		}
+
+		err = subdev->block_markbad(subdev, ofs);
+		break;
+	}
+
+	return err;
+}
+
 /*
  * This function constructs a virtual MTD device by concatenating
  * num_devs MTD devices. A pointer to the new device object is
@@ -687,10 +856,18 @@
 		concat->mtd.read_ecc = concat_read_ecc;
 	if (subdev[0]->write_ecc)
 		concat->mtd.write_ecc = concat_write_ecc;
+	if (subdev[0]->writev)
+		concat->mtd.writev = concat_writev;
+	if (subdev[0]->writev_ecc)
+		concat->mtd.writev_ecc = concat_writev_ecc;
 	if (subdev[0]->read_oob)
 		concat->mtd.read_oob = concat_read_oob;
 	if (subdev[0]->write_oob)
 		concat->mtd.write_oob = concat_write_oob;
+	if (subdev[0]->block_isbad)
+		concat->mtd.block_isbad = concat_block_isbad;
+	if (subdev[0]->block_markbad)
+		concat->mtd.block_markbad = concat_block_markbad;
 
 	concat->subdev[0] = subdev[0];
 
@@ -736,14 +913,13 @@
 
 	}
 
+	if(concat->mtd.type == MTD_NANDFLASH)
+		memcpy(&concat->mtd.oobinfo,
+			&subdev[0]->oobinfo,sizeof(struct nand_oobinfo));
+
 	concat->num_subdev = num_devs;
 	concat->mtd.name = name;
 
-	/*
-	 * NOTE: for now, we do not provide any readv()/writev() methods
-	 *       because they are messy to implement and they are not
-	 *       used to a great extent anyway.
-	 */
 	concat->mtd.erase = concat_erase;
 	concat->mtd.read = concat_read;
 	concat->mtd.write = concat_write;

Re: [PATCH] MTD: mtdconcat NAND/Sibley support (revised)

[Jörn Engel]

On Tue, 16 May 2006 10:34:12 +0400, Alexander Belyakov wrote:
> 
> One of my mailers corrupts headers, so messages are rejected by MTD list 
> admin. Another one deals with headers, but converts tabs to whitespaces. 
> And I don't know how to fix them at the moment. Sorry.
> 
> So I'm attaching patch file to this message to be sure it will not get 
> corrupted.

I heard a lot of people are fairly happy with their gmail account for
mailing list usage.  If you want to try it, you can have an invitation
from me.  My personal setup is to use mutt.  Text mailers are not
everyones cup of tea, but they tend to just work.

There are three cases of whitespace corruption left.  If you're using
vim, you could add this to your .vimrc:

" Private macros
highlight RedundantSpaces ctermbg=red guibg=red
match RedundantSpaces /\s\+$\| \+\ze\t/
syntax on

It will highlight them to you.

Jörn

-- 
Optimizations always bust things, because all optimizations are, in
the long haul, a form of cheating, and cheaters eventually get caught.
-- Larry Wall 

Re: [PATCH] MTD: mtdconcat NAND/Sibley support (revised)

[Jörn Engel]

On Tue, 16 May 2006 10:34:12 +0400, Alexander Belyakov wrote:
>  
>  /*
> + * Forward function declaration
> + */
> +static int
> +concat_writev_ecc(struct mtd_info *mtd, const struct kvec *vecs,
> +		unsigned long count, loff_t to, size_t * retlen,
> +		u_char *eccbuf, struct nand_oobinfo *oobsel);
> +

This hunk can be removed...

> +static int
> +concat_writev(struct mtd_info *mtd, const struct kvec *vecs,
> +		unsigned long count, loff_t to, size_t * retlen)
> +{
> +	return concat_writev_ecc(mtd, vecs, count, to, retlen, NULL, NULL);
> +}

...if this is moved down a little.

>  static int
> +concat_writev_ecc(struct mtd_info *mtd, const struct kvec *vecs,
> +		unsigned long count, loff_t to, size_t * retlen,
                                                        ^

> +	/* Do not allow write past end of page */
> +	if ((to + total_len) > mtd->size)
> +		return -EINVAL;

s/page/device/ ?

> +	if (concat->mtd.type == MTD_NANDFLASH) {
> +    		if ((to & (mtd->oobblock - 1)) ||
   ^^^^

> +		    (total_len & (mtd->oobblock - 1)))
> +			return -EINVAL;
> +	}

Not generic enough.  You would need a similar, but not identical check
for dataflash, another one for ecc nor, one for sibley.  And if
someone merges yet another weird chip...

What I'm currently doing in my tree (that dwmw2 still didn't look at)
is remove some of the special cases in existing code.  Basically, I
start with the lowest common denominator of all devices:
1. Device has one erasesize.
2. Device has one writesize.
3. Erasesize is a multiple of writesize.
4. At least <writesize> aligned bytes must be written in one go.
5. Writes to any eraseblock must happen in order, from lowest offset to
   highest.
6. Any number of eraseblocks can be written to in any order, providing
   that rule 5 is followed.
7. Neither writesize nor erasesize must be a power of 2.

Among other things mtd->oobblock is replaced with mtd->writesize, as
that is a generic attribute of any flash type.  Once that is in - and
it should go in before this patch - you can exchange above code with
something like this:

	if (mtd->writesize > 1)
		if ((to % mtd->writesize) || (total_len % mtd->writesize))
			return -EINVAL;

> +	/* make a copy of vecs */
> +	vecs_copy = kmalloc(sizeof(struct kvec) * count, GFP_KERNEL);
> +	if (!vecs_copy)
> +		return -ENOMEM;
> +	memcpy(vecs_copy, vecs, sizeof(struct kvec) * count);

Ok for now.  In the future, we might run into problems when memory
pressure causes dirty pages to get written to flash.  Then this
allocation will fail because of memory pressure and we have a
livelock.

With jffs2 this can't happen, so don't worry about it yet.

> +	entry_low = 0;
> +	for (i = 0; i < concat->num_subdev; i++) {
> +		struct mtd_info *subdev = concat->subdev[i];
> +		size_t size, wsize, retsize, old_iov_len;
> +
> +		if (to >= subdev->size) {
> +			size = 0;

Is this line necessary?

> +			to -= subdev->size;
> +			continue;
> +		}
> +
> +		if (to + total_len > subdev->size)
> +			size = subdev->size - to;
> +		else
> +			size = total_len;

		size = min(total_len, subdev->size - to);

> +		while (entry_high < count) {
> +			if (size > vecs_copy[entry_high].iov_len)
> +				size -= vecs_copy[entry_high++].iov_len;
> +			else
> +				break;
> +		}

		while (entry_high < count) {
			if (size <= vecs_copy[entry_high].iov_len)
				break;
			size -= vecs_copy[entry_high++].iov_len;
		}

> +		vecs_copy[entry_high].iov_base = vecs_copy[entry_high].iov_base
> +						+ size;

		vecs_copy[entry_high].iov_base += size;

> +	if(concat->mtd.type == MTD_NANDFLASH)
> +		memcpy(&concat->mtd.oobinfo,
> +			&subdev[0]->oobinfo,sizeof(struct nand_oobinfo));
                                            ^

Rest looks fine.

Jörn

-- 
Rules of Optimization:
Rule 1: Don't do it.
Rule 2 (for experts only): Don't do it yet.
-- M.A. Jackson 

Re: [PATCH] MTD: mtdconcat NAND/Sibley support (revised)

[Artem B. Bityutskiy]

On Tue, 2006-05-16 at 10:33 +0200, Jörn Engel wrote:

> Not generic enough.  You would need a similar, but not identical check
> for dataflash, another one for ecc nor, one for sibley.  And if
> someone merges yet another weird chip...
> 
> What I'm currently doing in my tree (that dwmw2 still didn't look at)
> is remove some of the special cases in existing code.  Basically, I
> start with the lowest common denominator of all devices:
> 1. Device has one erasesize.
> 2. Device has one writesize.
> 3. Erasesize is a multiple of writesize.
> 4. At least <writesize> aligned bytes must be written in one go.
> 5. Writes to any eraseblock must happen in order, from lowest offset to
>    highest.
> 6. Any number of eraseblocks can be written to in any order, providing
>    that rule 5 is followed.
> 7. Neither writesize nor erasesize must be a power of 2.

Good list. I have a comment for 5. For NOR flash it is possible to clear
individual bits and JFFS2 exploits this feature (to mark nodes
obsolete). So, it may sometimes write out-of-order. Thus, I'd
re-formulate item 5 to take this into account.

-- 
Best Regards,
Artem B. Bityutskiy,
St.-Petersburg, Russia.

Re: [PATCH] MTD: mtdconcat NAND/Sibley support (revised)

[Jörn Engel]

On Tue, 16 May 2006 15:54:05 +0400, Artem B. Bityutskiy wrote:
> On Tue, 2006-05-16 at 10:33 +0200, Jörn Engel wrote:
> 
> > Not generic enough.  You would need a similar, but not identical check
> > for dataflash, another one for ecc nor, one for sibley.  And if
> > someone merges yet another weird chip...
> > 
> > What I'm currently doing in my tree (that dwmw2 still didn't look at)
> > is remove some of the special cases in existing code.  Basically, I
> > start with the lowest common denominator of all devices:
> > 1. Device has one erasesize.
> > 2. Device has one writesize.
> > 3. Erasesize is a multiple of writesize.
> > 4. At least <writesize> aligned bytes must be written in one go.
> > 5. Writes to any eraseblock must happen in order, from lowest offset to
> >    highest.
> > 6. Any number of eraseblocks can be written to in any order, providing
> >    that rule 5 is followed.
> > 7. Neither writesize nor erasesize must be a power of 2.
> 
> Good list. I have a comment for 5. For NOR flash it is possible to clear
> individual bits and JFFS2 exploits this feature (to mark nodes
> obsolete). So, it may sometimes write out-of-order. Thus, I'd
> re-formulate item 5 to take this into account.

The list is the lowest common denominator.  Any flash chip will work
for you as long as you stick to the list.

We could have a second list of optional additional features.

Jörn

-- 
Linux [...] existed just for discussion between people who wanted
to show off how geeky they were.
-- Rob Enderle