Patchwork mtd: unify initialization of erase_info->fail_addr

login
register
mail settings
Submitter Shmulik Ladkani
Date Feb. 9, 2012, 1:36 p.m.
Message ID <20120209153629.16bb9b01@pixies.home.jungo.com>
Download mbox | patch
Permalink /patch/140379/
State New
Headers show

Comments

Shmulik Ladkani - Feb. 9, 2012, 1:36 p.m.
From: Shmulik Ladkani <shmulik.ladkani@gmail.com>

Initialization of 'erase_info->fail_addr' to MTD_FAIL_ADDR_UNKNOWN prior
erase operation is duplicated accross several MTD drivers, and also taken
care of by some MTD users as well.

Harmonize it: initialize 'fail_addr' within 'mtd_erase()' interface.

Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
Compile tested only, l2-mtd.git
Artem Bityutskiy - Feb. 13, 2012, 2:02 p.m.
On Thu, 2012-02-09 at 15:36 +0200, Shmulik Ladkani wrote:
> From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> 
> Initialization of 'erase_info->fail_addr' to MTD_FAIL_ADDR_UNKNOWN prior
> erase operation is duplicated accross several MTD drivers, and also taken
> care of by some MTD users as well.
> 
> Harmonize it: initialize 'fail_addr' within 'mtd_erase()' interface.
> 
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>

Could you please re-send this one and

[PATCH] sfc: mtd: Use MTD_FAIL_ADDR_UNKNOWN instead of 0xffffffff

as a single patch or a series of 2 patches?
Artem Bityutskiy - Feb. 13, 2012, 2:03 p.m.
On Mon, 2012-02-13 at 16:02 +0200, Artem Bityutskiy wrote:
> On Thu, 2012-02-09 at 15:36 +0200, Shmulik Ladkani wrote:
> > From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> > 
> > Initialization of 'erase_info->fail_addr' to MTD_FAIL_ADDR_UNKNOWN prior
> > erase operation is duplicated accross several MTD drivers, and also taken
> > care of by some MTD users as well.
> > 
> > Harmonize it: initialize 'fail_addr' within 'mtd_erase()' interface.
> > 
> > Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> 
> Could you please re-send this one and
> 
> [PATCH] sfc: mtd: Use MTD_FAIL_ADDR_UNKNOWN instead of 0xffffffff
> 
> as a single patch or a series of 2 patches?

I mean this patch should also patch 'sfc' or just contain that change,
right?
Shmulik Ladkani - Feb. 13, 2012, 6:56 p.m.
On Mon, 13 Feb 2012 16:03:16 +0200 Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Mon, 2012-02-13 at 16:02 +0200, Artem Bityutskiy wrote:
> > On Thu, 2012-02-09 at 15:36 +0200, Shmulik Ladkani wrote:
> > > From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> > > 
> > > Initialization of 'erase_info->fail_addr' to MTD_FAIL_ADDR_UNKNOWN prior
> > > erase operation is duplicated accross several MTD drivers, and also taken
> > > care of by some MTD users as well.
> > > 
> > > Harmonize it: initialize 'fail_addr' within 'mtd_erase()' interface.
> > > 
> > > Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> > 
> > Could you please re-send this one and
> > 
> > [PATCH] sfc: mtd: Use MTD_FAIL_ADDR_UNKNOWN instead of 0xffffffff
> > 
> > as a single patch or a series of 2 patches?
> 
> I mean this patch should also patch 'sfc' or just contain that change,
> right?
> 

No, these are independent patches.

'mtd: unify initialization of erase_info->fail_addr' deals with the
duplication of 'fail_addr' initialization accross mtd users and drivers.

'sfc: mtd: Use MTD_FAIL_ADDR_UNKNOWN instead of 0xffffffff' is a bug fix
which I accidentally encountered when grepping 'fail_addr' during the
work on the former patch.
Here we're dealing with an 'erase()' implementation, in the case were
the erase has actually failed, however the 'sfc' driver seems not
capable of stating the location of the failure.
In this case 'fail_addr' should be set to MTD_FAIL_ADDR_UNKNOWN
(which was 0xffffffff ages ago).
Note today 'fail_addr' is a uint64_t, and MTD_FAIL_ADDR_UNKNOWN is -1LL,
meaning 'efx_mtd_erase()' was not adhering to the interface.

Regards
Shmulik
Shmulik Ladkani - March 21, 2012, 9:09 a.m.
Hi Artem,

Pinging.

Code harmonization and polish. No semantic changes (hopefully :-)

Regards,
Shmulik

On Thu, 9 Feb 2012 15:36:29 +0200 Shmulik Ladkani <shmulik.ladkani@gmail.com> wrote:
> From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> 
> Initialization of 'erase_info->fail_addr' to MTD_FAIL_ADDR_UNKNOWN prior
> erase operation is duplicated accross several MTD drivers, and also taken
> care of by some MTD users as well.
> 
> Harmonize it: initialize 'fail_addr' within 'mtd_erase()' interface.
> 
> Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
> ---
> Compile tested only, l2-mtd.git
> 
> diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
> index f7a31cc..b900056 100644
> --- a/drivers/mtd/mtdconcat.c
> +++ b/drivers/mtd/mtdconcat.c
> @@ -426,8 +426,6 @@ static int concat_erase(struct mtd_info *mtd, struct erase_info *instr)
>  			return -EINVAL;
>  	}
>  
> -	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
> -
>  	/* make a local copy of instr to avoid modifying the caller's struct */
>  	erase = kmalloc(sizeof (struct erase_info), GFP_KERNEL);
>  
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index b274fdf..c837507 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -695,6 +695,7 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
>  		return -EINVAL;
>  	if (!(mtd->flags & MTD_WRITEABLE))
>  		return -EROFS;
> +	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
>  	if (!instr->len) {
>  		instr->state = MTD_ERASE_DONE;
>  		mtd_erase_callback(instr);
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 5822e3a..5c69204 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2550,8 +2550,6 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
>  	if (check_offs_len(mtd, instr->addr, instr->len))
>  		return -EINVAL;
>  
> -	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
> -
>  	/* Grab the lock and see if the device is available */
>  	nand_get_device(chip, mtd, FL_ERASING);
>  
> diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
> index a1592cf..0e1ff9f 100644
> --- a/drivers/mtd/onenand/onenand_base.c
> +++ b/drivers/mtd/onenand/onenand_base.c
> @@ -2502,8 +2502,6 @@ static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
>  		return -EINVAL;
>  	}
>  
> -	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
> -
>  	/* Grab the lock and see if the device is available */
>  	onenand_get_device(mtd, FL_ERASING);
>  
> diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
> index eafb8d3..095e4b1 100644
> --- a/fs/jffs2/erase.c
> +++ b/fs/jffs2/erase.c
> @@ -69,7 +69,6 @@ static void jffs2_erase_block(struct jffs2_sb_info *c,
>  	instr->len = c->sector_size;
>  	instr->callback = jffs2_erase_callback;
>  	instr->priv = (unsigned long)(&instr[1]);
> -	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
>  
>  	((struct erase_priv_struct *)instr->priv)->jeb = jeb;
>  	((struct erase_priv_struct *)instr->priv)->c = c;
Artem Bityutskiy - March 21, 2012, 2:08 p.m.
On Wed, 2012-03-21 at 11:09 +0200, Shmulik Ladkani wrote:
> Hi Artem,
> 
> Pinging.
> 
> Code harmonization and polish. No semantic changes (hopefully :-)

Sorry, missed it, pushed now.

Patch

diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index f7a31cc..b900056 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -426,8 +426,6 @@  static int concat_erase(struct mtd_info *mtd, struct erase_info *instr)
 			return -EINVAL;
 	}
 
-	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
-
 	/* make a local copy of instr to avoid modifying the caller's struct */
 	erase = kmalloc(sizeof (struct erase_info), GFP_KERNEL);
 
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index b274fdf..c837507 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -695,6 +695,7 @@  int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 		return -EINVAL;
 	if (!(mtd->flags & MTD_WRITEABLE))
 		return -EROFS;
+	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
 	if (!instr->len) {
 		instr->state = MTD_ERASE_DONE;
 		mtd_erase_callback(instr);
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 5822e3a..5c69204 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2550,8 +2550,6 @@  int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
 	if (check_offs_len(mtd, instr->addr, instr->len))
 		return -EINVAL;
 
-	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
-
 	/* Grab the lock and see if the device is available */
 	nand_get_device(chip, mtd, FL_ERASING);
 
diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index a1592cf..0e1ff9f 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -2502,8 +2502,6 @@  static int onenand_erase(struct mtd_info *mtd, struct erase_info *instr)
 		return -EINVAL;
 	}
 
-	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
-
 	/* Grab the lock and see if the device is available */
 	onenand_get_device(mtd, FL_ERASING);
 
diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
index eafb8d3..095e4b1 100644
--- a/fs/jffs2/erase.c
+++ b/fs/jffs2/erase.c
@@ -69,7 +69,6 @@  static void jffs2_erase_block(struct jffs2_sb_info *c,
 	instr->len = c->sector_size;
 	instr->callback = jffs2_erase_callback;
 	instr->priv = (unsigned long)(&instr[1]);
-	instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
 
 	((struct erase_priv_struct *)instr->priv)->jeb = jeb;
 	((struct erase_priv_struct *)instr->priv)->c = c;