| Message ID | 20210925173318.25804-6-kabel@kernel.org |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | Fix `mtd erase` when used with mtdpart | expand |
On 25/09/21 07:33PM, Marek Behún wrote: > From: Marek Behún <marek.behun@nic.cz> > > This check is already done in mtdcore's mtd_erase(), no reason to do > this here as well. But do we always get here via mtd_erase()? What about "sf erase"? I looked at the code and I don't see any checks for 0 length there. > > Signed-off-by: Marek Behún <marek.behun@nic.cz> > Reviewed-by: Simon Glass <sjg@chromium.org> > Tested-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > --- > drivers/mtd/spi/spi-nor-core.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index 9e936cbe1a..211eea22a4 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -912,9 +912,6 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) > dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr, > (long long)instr->len); > > - if (!instr->len) > - return 0; > - > div_u64_rem(instr->len, mtd->erasesize, &rem); > if (rem) > return -EINVAL; > -- > 2.32.0 >
On Tue, 28 Sep 2021 22:29:11 +0530 Pratyush Yadav <p.yadav@ti.com> wrote: > On 25/09/21 07:33PM, Marek Behún wrote: > > From: Marek Behún <marek.behun@nic.cz> > > > > This check is already done in mtdcore's mtd_erase(), no reason to do > > this here as well. > > But do we always get here via mtd_erase()? What about "sf erase"? I > looked at the code and I don't see any checks for 0 length there. Hello Pratyush, you are right. This function is also called from include/spi_flash.h static inline function spi_flash_erase(), when CONFIG_IS_ENABLED(DM_SPI_FLASH) is false. I think I should move this test to the static inline imlpementation, before calling mtd->_erase(). This should be done in the caller at one place, not in all _erase() implementations. Marek
On 01/10/21 11:25AM, Marek Behún wrote: > On Tue, 28 Sep 2021 22:29:11 +0530 > Pratyush Yadav <p.yadav@ti.com> wrote: > > > On 25/09/21 07:33PM, Marek Behún wrote: > > > From: Marek Behún <marek.behun@nic.cz> > > > > > > This check is already done in mtdcore's mtd_erase(), no reason to do > > > this here as well. > > > > But do we always get here via mtd_erase()? What about "sf erase"? I > > looked at the code and I don't see any checks for 0 length there. > > Hello Pratyush, you are right. > > This function is also called from include/spi_flash.h static inline > function spi_flash_erase(), when CONFIG_IS_ENABLED(DM_SPI_FLASH) is > false. > > I think I should move this test to the static inline imlpementation, > before calling mtd->_erase(). This should be done in the caller at one > place, not in all _erase() implementations. We would do that. Or we could just leave it here. I don't see it doing much harm.
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 9e936cbe1a..211eea22a4 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -912,9 +912,6 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr, (long long)instr->len); - if (!instr->len) - return 0; - div_u64_rem(instr->len, mtd->erasesize, &rem); if (rem) return -EINVAL;