Message ID | 20120209153629.16bb9b01@pixies.home.jungo.com |
---|---|
State | Accepted |
Commit | 3b27dac03972c10980ec5480ad8425fc95aae9ad |
Headers | show |
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?
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?
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
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;
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.
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;