Huang Shijie | 1 Jan 16:23 2012
Picon

Re: [PATCH v2] MTD/GPMI bugfix : reset the BCH module when it is not MX23

On Sat, Dec 31, 2011 at 10:43 AM, Wolfram Sang <w.sang <at> pengutronix.de> wrote:
> Hi,
>
> On Sat, Dec 31, 2011 at 10:23:50AM +0800, Huang Shijie wrote:
>
>> > On Fri, Dec 30, 2011 at 04:27:26PM +0800, Huang Shijie wrote:
>> >> In MX28, if we do not reset the BCH module. The BCH module may
>> >> becomes unstable when the board reboots for several thousands times.
>> > Do you have more details when and why this happens? What happens on MX23 then?
>> In one customer's 3G router which uses the MX28:
>> [0] NAND boot mode, mount the UBIFS in the NAND partition.
>> [1] We used the gpmi_reset_block(r->bch_regs, true) to init the BCH module.
>> [2] The board will automatically reboot again after it booted by NAND
>> boot mode.
>> [3] After reboot more then thousands times(cost nearly one day), the BCH
>> mode became UNSTABLE,
>> the data read out was not right, so the system could not mount the UBIFS.
>>
>> After we use gpmi_reset_block(r->bch_regs, false) to init the BCH
>> module, the bug never happens.
>
> Yes, I undestood that. I was curious _why_ this happens, for example, because
> of a bug in the rom-code or such. This would then lead to the question if this

This happens because we do NOT follow the right init procedure. The IC
guy told me.
If the BCH was not initialized correctly, it can not guarantees the
data is right.
This is why the mx28 failed.

(Continue reading)

Wolfram Sang | 1 Jan 23:33 2012
Picon

Re: [PATCH v2] MTD/GPMI bugfix : reset the BCH module when it is not MX23

Hi,

> This happens because we do NOT follow the right init procedure. The IC
> guy told me.
> If the BCH was not initialized correctly, it can not guarantees the
> data is right.
> This is why the mx28 failed.
...
> For mx23, it should not reset the BCH, this is the right init
> procedure for mx23.

Okay, this was the information I was looking for. So, the MX23 cannot be reset
due to bug #2847. But it also does not NEED to be reset, which is unlike the
MX28 which needs the reset. Correct? Before that, it was only clear that MX23
cannot be reset. It was not clear (at least to me) if it still could then run
into the same problems as the MX28 after 10000+x boot cycles. That would be a
really bad situation: being in need of the reset which we can't do due to 2847.

> But I did not ever boot 10000 times on mx23. I am not sure if it can
> fail or not.

Hmm, can't your IC guy tell you? I was hoping he knows if the same problem which
happens on the MX28 can also happen on the MX23? I don't want to make your life
unnecessarily hard, but I'd really like to know if I need to warn customers
when using MX23 and NAND (and if so, we have to update the comment in the code).

> > bug 2847, we have a serious problem, because NAND won't work until the next
> > power-cycle? I am curious if my assumptions are true and we have a serious
> > problem on the MX23.
> You can test it if you have time. :)
(Continue reading)

Artem Bityutskiy | 2 Jan 13:20 2012
Picon

[PATCH 04/17] romfs: do not use mtd->get_unmapped_area directly

From: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>

Remove direct usage of mtd->get_unmapped_area. Instead, just call
'mtd_get_unmapped_area()' which will return -EOPNOTSUPP if the function
is not implemented, and then test for this code.

We also translate -EOPNOTSUPP to -ENOSYS because this return code is
probably part of the kernel ABI which we do not want to break.

Cc: linux-fsdevel <at> vger.kernel.org
Signed-off-by: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>
---
 fs/romfs/mmap-nommu.c |   28 +++++++++++++---------------
 1 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/fs/romfs/mmap-nommu.c b/fs/romfs/mmap-nommu.c
index d5168e8..e1a7779 100644
--- a/fs/romfs/mmap-nommu.c
+++ b/fs/romfs/mmap-nommu.c
 <at>  <at>  -28,9 +28,10  <at>  <at>  static unsigned long romfs_get_unmapped_area(struct file *file,
 	struct inode *inode = file->f_mapping->host;
 	struct mtd_info *mtd = inode->i_sb->s_mtd;
 	unsigned long isize, offset, maxpages, lpages;
+	int ret;

 	if (!mtd)
-		goto cant_map_directly;
+		return (unsigned long) -ENOSYS;

 	/* the mapping mustn't extend beyond the EOF */
(Continue reading)

Artem Bityutskiy | 2 Jan 13:20 2012
Picon

[PATCH 00/17] mtd: get rid of direct mtd->func use

Hi,

this patch-set goes on top of the "MTD API rework" patches which I sent
recently:

http://lists.infradead.org/pipermail/linux-mtd/2011-December/039031.html

This patch-set further removes direct mtd function pointers accesses like:

if (mtd->block_isbad)
	blah

This is a part of the big MTD API re-work we have started. The next steps will
be:

1. Rename all the function pointers to make sure we catch all direct uses:
   (s/mtd->func/mtd->_func/g)
2. Move common input parameters check from implementations to the wrapper
   function to get rid of duplication.

Artem.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Artem Bityutskiy | 2 Jan 13:20 2012
Picon

[PATCH 02/17] mtd: do use mtd->point directly

From: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>

Remove direct usage of the "mtd->point" function pointer. Instead,
test the mtd_point() return code for '-EOPNOTSUPP'.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>
---
 fs/jffs2/erase.c        |    9 ++++-----
 fs/jffs2/readinode.c    |   18 ++++++++----------
 fs/jffs2/scan.c         |    2 +-
 include/linux/mtd/mtd.h |    2 ++
 4 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
index c59d642..a01cdad 100644
--- a/fs/jffs2/erase.c
+++ b/fs/jffs2/erase.c
 <at>  <at>  -336,12 +336,11  <at>  <at>  static int jffs2_block_check_erase(struct jffs2_sb_info *c, struct jffs2_erasebl
 	uint32_t ofs;
 	size_t retlen;
 	int ret = -EIO;
+	unsigned long *wordebuf;

-	if (c->mtd->point) {
-		unsigned long *wordebuf;
-
-		ret = mtd_point(c->mtd, jeb->offset, c->sector_size, &retlen,
-				&ebuf, NULL);
+	ret = mtd_point(c->mtd, jeb->offset, c->sector_size, &retlen,
+			&ebuf, NULL);
(Continue reading)

Artem Bityutskiy | 2 Jan 13:20 2012
Picon

[PATCH 01/17] mtd: introduce mtd_has_oob helper

From: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>

We are working in the direction of making sure that MTD clients to not
use 'mtd->func' pointers directly. In some places we want to know if
OOB operations are supported by an MTD device. Introduce 'mtd_has_oob()'
helper for these purposes.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>
---
 drivers/mtd/mtdchar.c   |    2 +-
 drivers/mtd/sm_ftl.c    |    4 ++--
 include/linux/mtd/mtd.h |    5 +++++
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 83b0c82..c501eec 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
 <at>  <at>  -1004,7 +1004,7  <at>  <at>  static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
 			break;

 		case MTD_FILE_MODE_RAW:
-			if (!mtd->read_oob || !mtd->write_oob)
+			if (!mtd_has_oob(mtd))
 				return -EOPNOTSUPP;
 			mfi->mode = arg;

diff --git a/drivers/mtd/sm_ftl.c b/drivers/mtd/sm_ftl.c
index 4ec2af7..072ed59 100644
--- a/drivers/mtd/sm_ftl.c
(Continue reading)

Artem Bityutskiy | 2 Jan 13:20 2012
Picon

[PATCH 05/17] mtd: mtdoops: do not use mtd->panic_write directly

From: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>

Instead of checking if 'mtd->panic_write' is defined, call 'mtd_panic_write()'
and check the error code - '-EOPNOTSUPP' will be returned if the function is
not defined.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>
---
 drivers/mtd/mtdoops.c   |   17 ++++++++---------
 include/linux/mtd/mtd.h |    2 ++
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 69532a3..c8540b8 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
 <at>  <at>  -221,10 +221,14  <at>  <at>  static void mtdoops_write(struct mtdoops_context *cxt, int panic)
 	hdr[0] = cxt->nextcount;
 	hdr[1] = MTDOOPS_KERNMSG_MAGIC;

-	if (panic)
+	if (panic) {
 		ret = mtd_panic_write(mtd, cxt->nextpage * record_size,
 				      record_size, &retlen, cxt->oops_buf);
-	else
+		if (ret == -EOPNOTSUPP) {
+			printk(KERN_ERR "mtdoops: Cannot write from panic without panic_write\n");
+			return;
+		}
+	} else
(Continue reading)

Artem Bityutskiy | 2 Jan 13:20 2012
Picon

[PATCH 06/17] mtd: do not use mtd->read_oob directly

From: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>

Instead of checking whether 'mtd->read_oob' is defined, just call
'mtd_read_oob()' and handle the '-EOPNOTSUPP' error which will be returned
if the function is undefined.

Additionally, make 'mtd_write_oob()' return '-EOPNOTSUPP' if the function
is undefined.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>
---
 drivers/mtd/mtdchar.c   |    9 ++-------
 include/linux/mtd/mtd.h |    4 ++++
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 55f0961..287ff0d 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
 <at>  <at>  -452,13 +452,8  <at>  <at>  static int mtdchar_readoob(struct file *file, struct mtd_info *mtd,
 	if (length > 4096)
 		return -EINVAL;

-	if (!mtd->read_oob)
-		ret = -EOPNOTSUPP;
-	else
-		ret = access_ok(VERIFY_WRITE, ptr,
-				length) ? 0 : -EFAULT;
-	if (ret)
-		return ret;
(Continue reading)

Artem Bityutskiy | 2 Jan 13:20 2012
Picon

[PATCH 03/17] mtd: do not use mtd->get_unmapped_area directly

From: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>

Remove direct usage of mtd->get_unmapped_area. Instead, just call
'mtd_get_unmapped_area()' which will return -EOPNOTSUPP if the function
is not implemented and test for this error code.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>
---
 drivers/mtd/mtdchar.c   |   26 +++++++++++---------------
 drivers/mtd/mtdconcat.c |    6 +-----
 include/linux/mtd/mtd.h |    2 ++
 3 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index c501eec..55f0961 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
 <at>  <at>  -1124,25 +1124,21  <at>  <at>  static unsigned long mtdchar_get_unmapped_area(struct file *file,
 {
 	struct mtd_file_info *mfi = file->private_data;
 	struct mtd_info *mtd = mfi->mtd;
+	unsigned long offset;
+	int ret;

-	if (mtd->get_unmapped_area) {
-		unsigned long offset;
-
-		if (addr != 0)
-			return (unsigned long) -EINVAL;
-
(Continue reading)

Artem Bityutskiy | 2 Jan 13:20 2012
Picon

[PATCH 07/17] mtd: do not use mtd->get_*_prot_info directly

From: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>

Instead, call 'mtd_get_*_prot_info()' and check for '-EOPNOTSUPP'. While
on it, fix the return code from '-EOPNOTSUPP' to '-EINVAL' for the case
when the mode parameter is invalid.

Signed-off-by: Artem Bityutskiy <artem.bityutskiy <at> linux.intel.com>
---
 drivers/mtd/mtdchar.c   |    8 +++-----
 include/linux/mtd/mtd.h |    4 ++++
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 287ff0d..49340dc 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
 <at>  <at>  -919,17 +919,15  <at>  <at>  static int mtdchar_ioctl(struct file *file, u_int cmd, u_long arg)
 		struct otp_info *buf = kmalloc(4096, GFP_KERNEL);
 		if (!buf)
 			return -ENOMEM;
-		ret = -EOPNOTSUPP;
 		switch (mfi->mode) {
 		case MTD_FILE_MODE_OTP_FACTORY:
-			if (mtd->get_fact_prot_info)
-				ret = mtd_get_fact_prot_info(mtd, buf, 4096);
+			ret = mtd_get_fact_prot_info(mtd, buf, 4096);
 			break;
 		case MTD_FILE_MODE_OTP_USER:
-			if (mtd->get_user_prot_info)
-				ret = mtd_get_user_prot_info(mtd, buf, 4096);
(Continue reading)


Gmane