| Message ID | 1376879478-22128-8-git-send-email-b32955@freescale.com |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 19, 2013 at 10:31:16AM +0800, Huang Shijie wrote: > If the media is not SLC nand, dataflash, Sibley flash or a > ubi volume, we should return -EINVAL to the caller. > The caller should exit in this case. > > Current code returns 0 in this case which is not proper. > > Signed-off-by: Huang Shijie <b32955@freescale.com> > --- > fs/jffs2/fs.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c > index fe3c052..0452445 100644 > --- a/fs/jffs2/fs.c > +++ b/fs/jffs2/fs.c > @@ -702,7 +702,7 @@ void jffs2_gc_release_page(struct jffs2_sb_info *c, > } > > static int jffs2_flash_setup(struct jffs2_sb_info *c) { > - int ret = 0; > + int ret = -EINVAL; This doesn't look correct to me. This function isn't meant to screen out unsupported flash; it's only for doing special things for certain flash. In fact, I think this will break many NOR flash which currently pass through this function without doing anything. If you really want to make MLC NAND fail, you should do it with an explicit check for MLC, and you probably don't want it in this function. Maybe in jffs2_do_fill_super(), near the other 'if (c->mtd->type == ...)' checks (but outside of the #ifndef, of course). > > if (jffs2_cleanmarker_oob(c)) { > /* NAND flash... do setup accordingly */ Brian
On Fri, Aug 23, 2013 at 11:37:39PM -0700, Brian Norris wrote: > On Mon, Aug 19, 2013 at 10:31:16AM +0800, Huang Shijie wrote: > > If the media is not SLC nand, dataflash, Sibley flash or a > > ubi volume, we should return -EINVAL to the caller. > > The caller should exit in this case. > > > > Current code returns 0 in this case which is not proper. > > > > Signed-off-by: Huang Shijie <b32955@freescale.com> > > --- > > fs/jffs2/fs.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c > > index fe3c052..0452445 100644 > > --- a/fs/jffs2/fs.c > > +++ b/fs/jffs2/fs.c > > @@ -702,7 +702,7 @@ void jffs2_gc_release_page(struct jffs2_sb_info *c, > > } > > > > static int jffs2_flash_setup(struct jffs2_sb_info *c) { > > - int ret = 0; > > + int ret = -EINVAL; > > This doesn't look correct to me. This function isn't meant to screen out > unsupported flash; it's only for doing special things for certain flash. > In fact, I think this will break many NOR flash which currently pass > through this function without doing anything. thanks for pointing this. This patch was created when i was not familiar with the NOR. > > If you really want to make MLC NAND fail, you should do it with an > explicit check for MLC, and you probably don't want it in this function. > Maybe in jffs2_do_fill_super(), near the other 'if (c->mtd->type == ...)' > checks (but outside of the #ifndef, of course). I will create a new patch to fix the MLC issue for the jffs2. Huang Shijie
diff --git a/fs/jffs2/fs.c b/fs/jffs2/fs.c index fe3c052..0452445 100644 --- a/fs/jffs2/fs.c +++ b/fs/jffs2/fs.c @@ -702,7 +702,7 @@ void jffs2_gc_release_page(struct jffs2_sb_info *c, } static int jffs2_flash_setup(struct jffs2_sb_info *c) { - int ret = 0; + int ret = -EINVAL; if (jffs2_cleanmarker_oob(c)) { /* NAND flash... do setup accordingly */
If the media is not SLC nand, dataflash, Sibley flash or a ubi volume, we should return -EINVAL to the caller. The caller should exit in this case. Current code returns 0 in this case which is not proper. Signed-off-by: Huang Shijie <b32955@freescale.com> --- fs/jffs2/fs.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)