[PATCH] MTD: mtdconcat NAND/Sibley support

[PATCH] MTD: mtdconcat NAND/Sibley support

[Belyakov, Alexander]

JFFS2 (and not only JFFS2) does not work on top of concatenated MTD
device in case of NAND and Sibley chips due to missing
concat_block_isbad(), concat_block_markbad(), concat_writev() and
concat_writev_ecc() functions. If anyone cares - the patch below fixes
that issue.


---

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-04-26 15:26:21.000000000 +0400
@@ -141,6 +141,97 @@
 }
 
 static int
+concat_writev (struct mtd_info *mtd, const struct kvec *vecs, unsigned
long count, 
+		loff_t to, size_t * retlen)
+{
+	int i, page, len, total_len, ret = 0, written = 0, cnt = 0,
towrite;
+	u_char *bufstart;
+	char* data_poi;
+	char* data_buf;
+	loff_t write_offset;
+	int rl_wr;
+	u_int32_t pagesize;
+
+	if (mtd->flags & MTD_PROGRAM_REGIONS) {
+		pagesize = MTD_PROGREGION_SIZE(mtd);
+	} else {
+		pagesize = mtd->oobblock;
+	}
+    
+	data_buf = kmalloc(pagesize, GFP_KERNEL);
+
+	/* Preset written len for early exit */
+	*retlen = 0;
+
+	/* Calculate total length of data */
+	total_len = 0;
+	for (i = 0; i < count; i++)
+		total_len += (int) vecs[i].iov_len;
+
+	/* Do not allow write past end of page */
+	if ((to + total_len) > mtd->size) {
+		kfree(data_buf);
+		return -EINVAL;
+	}
+
+	/* Setup start page */
+	page = ((int) to) / pagesize;
+	towrite = (page + 1) * pagesize - to;
+	write_offset = to;
+	written = 0;
+	
+	/* Loop until all iovecs' data has been written */
+	len = 0;
+	while (len < total_len) {
+    		bufstart = (u_char *)vecs->iov_base;
+		bufstart += written;
+		data_poi = bufstart;
+
+    		/* If the given tuple is >= reet of page then
+		 * write it out from the iov
+		 */
+		if ( (vecs->iov_len-written) >= towrite) {
+			ret = mtd->write(mtd, write_offset, towrite,
&rl_wr, data_poi);
+			if(ret)
+	    			break;
+			len += towrite;
+			page ++;
+			write_offset = page * pagesize;
+			towrite = pagesize;
+			written += towrite;
+			if (vecs->iov_len  == written) {
+				vecs ++;
+				written = 0;
+			}
+		} else {
+			cnt = 0;
+			while (cnt < towrite ) {
+				data_buf[cnt++] = ((u_char *)
vecs->iov_base)[written++];
+				if (vecs->iov_len == written ) {
+					if((cnt+len) == total_len )
+						break;
+					vecs ++;
+					written = 0;
+				}
+			}
+			data_poi = data_buf;
+			ret = mtd->write(mtd, write_offset, cnt, &rl_wr,
data_poi);
+			if (ret)
+				break;
+			len += cnt;
+			page ++;
+			write_offset = page * pagesize;
+			towrite = pagesize;
+		}
+	}
+
+	if(retlen)
+		*retlen = len;
+	kfree(data_buf);
+	return ret;
+}
+
+static int
 concat_read_ecc(struct mtd_info *mtd, loff_t from, size_t len,
 		size_t * retlen, u_char * buf, u_char * eccbuf,
 		struct nand_oobinfo *oobsel)
@@ -251,6 +342,123 @@
 }
 
 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)
+{
+	int i, page, len, total_len, ret = 0, written = 0, cnt = 0,
towrite;
+	u_char *bufstart;
+	char* data_poi;
+	char* data_buf;
+	loff_t write_offset;
+	int rl_wr;
+	u_int32_t pagesize;
+
+	if (mtd->flags & MTD_PROGRAM_REGIONS) {
+		pagesize = MTD_PROGREGION_SIZE(mtd);
+	} else {
+		pagesize = mtd->oobblock;
+	}
+    
+	data_buf = kmalloc(pagesize, GFP_KERNEL);
+    
+	if (!(mtd->flags & MTD_WRITEABLE))
+		return -EROFS;
+
+	/* Preset written len for early exit */
+	*retlen = 0;
+
+	/* Calculate total length of data */
+	total_len = 0;
+	for (i = 0; i < count; i++)
+		total_len += (int) vecs[i].iov_len;
+
+	/* check if no data is going to be written */
+	if (!total_len) {
+		kfree(data_buf);
+		return 0;
+	}
+
+	/* 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");
+		kfree(data_buf);
+		return -EINVAL;
+	}
+    
+	/* Check alignment */
+	if ((to & (pagesize - 1)) || (total_len & (pagesize - 1))) {
+		DEBUG (MTD_DEBUG_LEVEL0, "concat_writev_ecc(): Attempted
write not aligned data!\n");
+		kfree(data_buf);
+		return -EINVAL;
+	}
+    
+	/* Setup start page. Notaligned data is not allowed for
write_ecc. */
+	page = ((int) to) / pagesize;
+	towrite = (page + 1) * pagesize - to;
+	write_offset = to;
+	written = 0; 
+	/* Loop until all iovecs' data has been written */
+	len = 0;
+	while (len < total_len) {
+		bufstart = (u_char *)vecs->iov_base;
+		bufstart += written;
+		data_poi = bufstart;
+
+	        /* If the given tuple is >= reet of page then
+		* write it out from the iov
+		*/
+		if ( (vecs->iov_len-written) >= towrite) {
+			ret = mtd->write_ecc(mtd, write_offset, towrite,
+						&rl_wr, data_poi,
eccbuf, oobsel);
+			if (ret)
+	    			break;
+			len += towrite;
+			page ++;
+			write_offset = page * pagesize;
+			towrite = pagesize;
+			written += towrite;
+			if (vecs->iov_len  == written) {
+	    			vecs ++;
+	    			written = 0;
+	    		}
+	    
+			if(eccbuf)
+				eccbuf += mtd->oobavail;
+		} else {
+			cnt = 0;
+			while (cnt < towrite ) {
+				data_buf[cnt++] = ((u_char *)
vecs->iov_base)[written++];
+	    			if (vecs->iov_len == written ) {
+					if ((cnt+len) == total_len )
+						break;
+					vecs ++;
+					written = 0;
+				}
+			}
+			
+			data_poi = data_buf;
+			ret = mtd->write_ecc(mtd, write_offset, cnt, 
+						&rl_wr, data_poi,
eccbuf, oobsel);
+			if (ret)
+				break;
+			len += cnt;
+			page ++;
+			write_offset = page * pagesize;
+			towrite = pagesize;
+	    
+			if(eccbuf)
+				eccbuf += mtd->oobavail;
+		}
+	}
+
+	if(retlen)
+		*retlen = len;
+	kfree(data_buf);
+    
+	return ret;
+}
+
+static int
 concat_read_oob(struct mtd_info *mtd, loff_t from, size_t len,
 		size_t * retlen, u_char * buf)
 {
@@ -638,6 +846,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
@@ -685,12 +947,18 @@
 	concat->mtd.eccsize = subdev[0]->eccsize;
 	if (subdev[0]->read_ecc)
 		concat->mtd.read_ecc = concat_read_ecc;
-	if (subdev[0]->write_ecc)
+	if (subdev[0]->write_ecc) {
 		concat->mtd.write_ecc = concat_write_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,17 +1004,16 @@
 
 	}
 
+	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;
+	concat->mtd.writev = concat_writev;
 	concat->mtd.sync = concat_sync;
 	concat->mtd.lock = concat_lock;
 	concat->mtd.unlock = concat_unlock;

--
Thanks,
Alexander Belyakov

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

[Jörn Engel]

On Wed, 26 April 2006 15:36:08 +0400, Belyakov, Alexander wrote:
> 
> JFFS2 (and not only JFFS2) does not work on top of concatenated MTD
> device in case of NAND and Sibley chips due to missing
> concat_block_isbad(), concat_block_markbad(), concat_writev() and
> concat_writev_ecc() functions. If anyone cares - the patch below fixes
> that issue.

concat_writev() is far too complicated.  I have a patch somewhere that
adds a generic writev() function to every device.  Once that is in,
your patch should get a respin and be much simpler.

I didn't closely look at the rest of it but suspect similar things.
Having complicated code in the kernel should not result in adding more
complicated code, but in someone making the existing code simple. ;)

Jörn

-- 
The strong give up and move away, while the weak give up and stay.
-- unknown

Re: [PATCH] MTD: mtdconcat NAND/Sibley support

[Alexander Belyakov]

Jorn Engel wrote:
>
> concat_writev() is far too complicated.  I have a patch somewhere that
> adds a generic writev() function to every device.  Once that is in,
> your patch should get a respin and be much simpler.
>

Having generic writev() still requires concat_writev() to split the data 
into several parts (in general case by number of subdevices). So I do 
not see how concat_writev() could become simpler.

In my patch you split data from downcoming vector into several  _equal_  
page-sized chunks and write them by calling generic write().

In case you think about one have to create  _several_vectors_  of  
_unknown_  size and pass them to the generic writev() which in turn will 
anyway split these new vectors into equal page-sized chunks.

And I really doubt there is a way to simplify concat_block_isbad() and 
concat_block_markbad() functions from the patch.

> I didn't closely look at the rest of it but suspect similar things.
>

Why I'm not surprised here? :)

Patch have nothing complex and have completely nothing new. Just fixes 
some broken stuff.

Is it better to leave concatenation layer without any changes and only 
compatible with NOR devices, despite mtdconcat.c file comments claim 
that it does support NAND?

Alexander

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

[Jörn Engel]

On Wed, 26 April 2006 15:36:08 +0400, Belyakov, Alexander wrote:
> 
> 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-04-26 15:26:21.000000000 +0400
> @@ -141,6 +141,97 @@
>  }
>  
>  static int
> +concat_writev (struct mtd_info *mtd, const struct kvec *vecs, unsigned
> long count, 
> +		loff_t to, size_t * retlen)
> +{
> +	int i, page, len, total_len, ret = 0, written = 0, cnt = 0,
> towrite;
> +	u_char *bufstart;
> +	char* data_poi;
> +	char* data_buf;
> +	loff_t write_offset;
> +	int rl_wr;
> +	u_int32_t pagesize;
> +
> +	if (mtd->flags & MTD_PROGRAM_REGIONS) {
> +		pagesize = MTD_PROGREGION_SIZE(mtd);
> +	} else {
> +		pagesize = mtd->oobblock;
> +	}

Take a look at my tree:
http://git.infradead.org/?p=users/joern/mtd-2.6.git;a=summary

Once writesize is in, this part can just go.

> +	data_buf = kmalloc(pagesize, GFP_KERNEL);

Allocation can fail.

> +	/* Preset written len for early exit */
> +	*retlen = 0;
> +
> +	/* Calculate total length of data */
> +	total_len = 0;
> +	for (i = 0; i < count; i++)
> +		total_len += (int) vecs[i].iov_len;
> +
> +	/* Do not allow write past end of page */
> +	if ((to + total_len) > mtd->size) {
> +		kfree(data_buf);

Move the allocation below this check instead.

> +		return -EINVAL;
> +	}
> +
> +	/* Setup start page */
> +	page = ((int) to) / pagesize;

If you have to cast, at least take uint32_t, as mtd->size has that
type as well.

> +	towrite = (page + 1) * pagesize - to;
> +	write_offset = to;
> +	written = 0;
> +	
> +	/* Loop until all iovecs' data has been written */
> +	len = 0;
> +	while (len < total_len) {
> +    		bufstart = (u_char *)vecs->iov_base;

Why the cast?  Can't bufstart be a void*?

> +		bufstart += written;
> +		data_poi = bufstart;
> +
> +    		/* If the given tuple is >= reet of page then
> +		 * write it out from the iov
> +		 */
> +		if ( (vecs->iov_len-written) >= towrite) {
> +			ret = mtd->write(mtd, write_offset, towrite,
> &rl_wr, data_poi);
> +			if(ret)

Documentation/CodingStyle requires a space here.

> +	    			break;
> +			len += towrite;
> +			page ++;
> +			write_offset = page * pagesize;
> +			towrite = pagesize;
> +			written += towrite;
> +			if (vecs->iov_len  == written) {
> +				vecs ++;
> +				written = 0;
> +			}
> +		} else {
> +			cnt = 0;
> +			while (cnt < towrite ) {
> +				data_buf[cnt++] = ((u_char *)
> vecs->iov_base)[written++];
> +				if (vecs->iov_len == written ) {
> +					if((cnt+len) == total_len )
> +						break;
> +					vecs ++;
> +					written = 0;
> +				}
> +			}
> +			data_poi = data_buf;
> +			ret = mtd->write(mtd, write_offset, cnt, &rl_wr,
> data_poi);
> +			if (ret)
> +				break;
> +			len += cnt;
> +			page ++;
> +			write_offset = page * pagesize;
> +			towrite = pagesize;
> +		}
> +	}
> +
> +	if(retlen)
> +		*retlen = len;
> +	kfree(data_buf);
> +	return ret;
> +}
> +
> +static int
>  concat_read_ecc(struct mtd_info *mtd, loff_t from, size_t len,
>  		size_t * retlen, u_char * buf, u_char * eccbuf,
>  		struct nand_oobinfo *oobsel)
> @@ -251,6 +342,123 @@
>  }
>  
>  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)
> +{
> +	int i, page, len, total_len, ret = 0, written = 0, cnt = 0,
> towrite;
> +	u_char *bufstart;
> +	char* data_poi;
> +	char* data_buf;
> +	loff_t write_offset;
> +	int rl_wr;
> +	u_int32_t pagesize;
> +
> +	if (mtd->flags & MTD_PROGRAM_REGIONS) {
> +		pagesize = MTD_PROGREGION_SIZE(mtd);
> +	} else {
> +		pagesize = mtd->oobblock;
> +	}

Again.

> +	data_buf = kmalloc(pagesize, GFP_KERNEL);

Again.

> +	if (!(mtd->flags & MTD_WRITEABLE))
> +		return -EROFS;
> +
> +	/* Preset written len for early exit */
> +	*retlen = 0;
> +
> +	/* Calculate total length of data */
> +	total_len = 0;
> +	for (i = 0; i < count; i++)
> +		total_len += (int) vecs[i].iov_len;
> +
> +	/* check if no data is going to be written */
> +	if (!total_len) {
> +		kfree(data_buf);

Again.

I'll stop reading here.  These two functions look fairly similar so
there might be some duplicated code you can move into a common helper.

Once you have the above fixed up and checked all the code for similar
problems, I'll take another look.

Jörn

-- 
Simplicity is prerequisite for reliability.
-- Edsger W. Dijkstra

Re: [PATCH] MTD: mtdconcat NAND/Sibley support

[Jörn Engel]

On Wed, 26 April 2006 19:21:38 +0400, Alexander Belyakov wrote:
> Jorn Engel wrote:
> >
> >concat_writev() is far too complicated.  I have a patch somewhere that
> >adds a generic writev() function to every device.  Once that is in,
> >your patch should get a respin and be much simpler.
> >
> 
> Having generic writev() still requires concat_writev() to split the data 
> into several parts (in general case by number of subdevices). So I do 
> not see how concat_writev() could become simpler.

Fair.  Having writev() for the underlying devices doesn't buy you
much.

> In my patch you split data from downcoming vector into several  _equal_  
> page-sized chunks and write them by calling generic write().

Not sure whether the splitting makes much sense.  As long as you don't
cross a boundary, just send the whole vector to the subdevice.  And in
case of a boudary, there is no guarantee for it to reside on a page
boundary, so you currently have a bug anyway.  One that you'll quickly
hit on dataflash.

> >I didn't closely look at the rest of it but suspect similar things.
> 
> Why I'm not surprised here? :)

Either because
a) I'm lazy or
b) your code doesn't look inviting.

And just because I'm lazy doesn't imply anything about your code. :)

> Patch have nothing complex and have completely nothing new. Just fixes 
> some broken stuff.
> 
> Is it better to leave concatenation layer without any changes and only 
> compatible with NOR devices, despite mtdconcat.c file comments claim 
> that it does support NAND?

I'm not sure about your "nothing complex" statement, but fixing concat
is a worthy goal.  Hope you don't mind if I point out the questionable
parts of your patch.

Jörn

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

Re: [PATCH] MTD: mtdconcat NAND/Sibley support

[Nicolas Pitre]

On Wed, 26 Apr 2006, Belyakov, Alexander wrote:

> JFFS2 (and not only JFFS2) does not work on top of concatenated MTD
> device in case of NAND and Sibley chips due to missing
> concat_block_isbad(), concat_block_markbad(), concat_writev() and
> concat_writev_ecc() functions. If anyone cares - the patch below fixes
> that issue.

That won't work with Sibley.  You must call the underlying devices with 
writev() as well, using the appropriate vector for each device, and only 
once per subdevice.  Sibley flash must write everything all at once with 
a single writev call.


Nicolas

Re: [PATCH] MTD: mtdconcat NAND/Sibley support

[Jörn Engel]

On Wed, 26 April 2006 12:04:51 -0400, Nicolas Pitre wrote:
> On Wed, 26 Apr 2006, Belyakov, Alexander wrote:
> 
> > JFFS2 (and not only JFFS2) does not work on top of concatenated MTD
> > device in case of NAND and Sibley chips due to missing
> > concat_block_isbad(), concat_block_markbad(), concat_writev() and
> > concat_writev_ecc() functions. If anyone cares - the patch below fixes
> > that issue.
> 
> That won't work with Sibley.  You must call the underlying devices with 
> writev() as well, using the appropriate vector for each device, and only 
> once per subdevice.  Sibley flash must write everything all at once with 
> a single writev call.

Everything?  Not just in chunks of 1KiB?

Jörn

-- 
And spam is a useful source of entropy for /dev/random too!
-- Jasmine Strong

Re: [PATCH] MTD: mtdconcat NAND/Sibley support

[Nicolas Pitre]

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

On Wed, 26 Apr 2006, Jörn Engel wrote:

> On Wed, 26 April 2006 12:04:51 -0400, Nicolas Pitre wrote:
> > On Wed, 26 Apr 2006, Belyakov, Alexander wrote:
> > 
> > > JFFS2 (and not only JFFS2) does not work on top of concatenated MTD
> > > device in case of NAND and Sibley chips due to missing
> > > concat_block_isbad(), concat_block_markbad(), concat_writev() and
> > > concat_writev_ecc() functions. If anyone cares - the patch below fixes
> > > that issue.
> > 
> > That won't work with Sibley.  You must call the underlying devices with 
> > writev() as well, using the appropriate vector for each device, and only 
> > once per subdevice.  Sibley flash must write everything all at once with 
> > a single writev call.
> 
> Everything?  Not just in chunks of 1KiB?

Well you can do more, obviously, and you can do less too.  But if you 
write less then it's still the whole 1KB that is gone.

The point is why is the patch so complex?

Something like:

	for (each subdevice) {
		determine_number_of_vector_entries_for_this_subdevice;
		limit_that_last_vector_entry_length_if_needed;
		writev(subdevice, vector, number_of_entries);
		update_vector_ptr_to_last_entry_above;
		readjust_offset_and_length_for_that_vector_entry;
		if (no_more_to_write)
			break;
	}

It seems to me that this should take four times less code than the 
proposed patch, and won't have any memory allocation nor copying 
required.


Nicolas

Re: [PATCH] MTD: mtdconcat NAND/Sibley support

[Alexander Belyakov]

Nicolas Pitre wrote:
>
> That won't work with Sibley.
>

It does work for Sibley. I've checked it for both NAND and Sibley before 
posting.

> Sibley flash must write everything all at once with
> a single writev call.
>

Not sure what did you  mean by that. But I should say that 
concat_writev() function does writes by page-sized chunks even if one 
vector entry has size less than one page. Function just "glues" it with 
the next vector entry (or its part) if any. So there are no writes with 
size less than pagesize, except first and last pages if we have 
unaligned data in downcoming vector.


As for too "complex" patch implementation. I'll take a look what can be 
done with usage of writev() and writev_ecc() of underlying devices. 
Thank you for advice. But as I said this change is just design matter 
and will not benefit much, because underlying device writev() function 
have to do the pagesize chunks splitting anyway.

Alexander

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

[Alexander Belyakov]

Jorn Engel wrote:
>
> Once you have the above fixed up and checked all the code for similar
> problems, I'll take another look.
>

Thanks for review, Jorn. I'll fix those coding/documentation guidelines 
and sanity/exception checks you have mentioned. Then post again, hope 
with usage of writev() functions of underlying devices.

Alexander

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

[Nicolas Pitre]

On Thu, 27 Apr 2006, Alexander Belyakov wrote:

> Nicolas Pitre wrote:
> > 
> > That won't work with Sibley.
> > 
> 
> It does work for Sibley. I've checked it for both NAND and Sibley before
> posting.
> 
> > Sibley flash must write everything all at once with
> > a single writev call.
> > 
> 
> Not sure what did you  mean by that. But I should say that concat_writev()
> function does writes by page-sized chunks even if one vector entry has size
> less than one page. Function just "glues" it with the next vector entry (or
> its part) if any. So there are no writes with size less than pagesize, except
> first and last pages if we have unaligned data in downcoming vector.

Which means you have to copy data into your page, which is against the 
point of writev().  If you're copying data into a separate page in order 
to use write() then JFFS2 could simply do it itself and writev() 
eliminated entirely.


Nicolas

Re: [PATCH] MTD: mtdconcat NAND/Sibley support

[Alexander Belyakov]

Nicolas Pitre wrote:
>
> Which means you have to copy data into your page, which is against the
> point of writev().  If you're copying data into a separate page in order
> to use write() then JFFS2 could simply do it itself and writev()
> eliminated entirely.
>

I've got your point.

Thanks,
Alexander

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

[Thomas Gleixner]

On Wed, 2006-04-26 at 17:51 +0200, Jörn Engel wrote:
> > +	if (mtd->flags & MTD_PROGRAM_REGIONS) {
> > +		pagesize = MTD_PROGREGION_SIZE(mtd);
> > +	} else {
> > +		pagesize = mtd->oobblock;
> > +	}
> 
> Take a look at my tree:
> http://git.infradead.org/?p=users/joern/mtd-2.6.git;a=summary
> 
> Once writesize is in, this part can just go.

And we should get rid of the _ecc variants too. Nothing outside of the
flash chip driver itself needs to be aware of them. Hopefully I come
around to clean that up along with the oob mess in the next weeks.

	tglx