| Message ID | 8be5cb694a763d3a74d9c3c7e76dd98b7b1333f6.1732152768.git.Takahiro.Kuwano@infineon.com |
|---|---|
| State | New |
| Headers | show |
| Series | mtd: spinand: Add support for SkyHigh S35ML-3 family | expand |
On 11/21/24 2:08 AM, tkuw584924@gmail.com wrote: > From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > > We don't have to call spinand_write_enable_op() in spinand_markbad() as > it is called in spinand_write_page(). Indeed. The patch is a follow up for: Fixes: b645ad39d568 ("mtd: spinand: Do not erase the block before writing a bad block marker") That commit removed spinand_erase_op(), but failed to remove spinand_write_enable_op() with it. Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org> Miquel, shall the spinand_write_enable_op() call be moved into spinand_erase_op()? > > Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > --- > drivers/mtd/nand/spi/core.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c > index 4d76f9f71a0e..47c369f2925d 100644 > --- a/drivers/mtd/nand/spi/core.c > +++ b/drivers/mtd/nand/spi/core.c > @@ -942,10 +942,6 @@ static int spinand_markbad(struct nand_device *nand, const struct nand_pos *pos) > if (ret) > return ret; > > - ret = spinand_write_enable_op(spinand); > - if (ret) > - return ret; > - > return spinand_write_page(spinand, &req); > } >
Hello, On 22/11/2024 at 09:37:45 GMT, Tudor Ambarus <tudor.ambarus@linaro.org> wrote: > On 11/21/24 2:08 AM, tkuw584924@gmail.com wrote: >> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >> >> We don't have to call spinand_write_enable_op() in spinand_markbad() as >> it is called in spinand_write_page(). > > Indeed. The patch is a follow up for: > Fixes: b645ad39d568 ("mtd: spinand: Do not erase the block before > writing a bad block marker") Correct. Takahiro, would you mind updating the commit log to mention that commit, please? It makes sense to have it mentioned. Otherwise the series looks good, I'll apply it once you send a v3. > That commit removed spinand_erase_op(), but failed to remove > spinand_write_enable_op() with it. > > Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org> > > Miquel, shall the spinand_write_enable_op() call be moved into > spinand_erase_op()? Thanks a lot for the review! spinand_erase_op() is just about the "op", not about handling an "mtd" erase. It is called from spinand_erase() which already does the spinand_write_enable_op() and I believe it makes more sense to keep it there, so when we browse the code we don't need to enter the low-level functions and immediately identify what operations we send to the chip without surprises. Cheers, Miquèl
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c index 4d76f9f71a0e..47c369f2925d 100644 --- a/drivers/mtd/nand/spi/core.c +++ b/drivers/mtd/nand/spi/core.c @@ -942,10 +942,6 @@ static int spinand_markbad(struct nand_device *nand, const struct nand_pos *pos) if (ret) return ret; - ret = spinand_write_enable_op(spinand); - if (ret) - return ret; - return spinand_write_page(spinand, &req); }