On 2/8/21 1:41 PM, Pratyush Yadav wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On 05/02/21 03:52PM, Tudor Ambarus wrote: >> Wait for the erase cmd to complete and then advance the erase. >> >> Signed-off-by: Tudor Ambarus <[email protected]> >> --- >> drivers/mtd/spi-nor/core.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c >> index 0522304f52fa..bcaa161bc7db 100644 >> --- a/drivers/mtd/spi-nor/core.c >> +++ b/drivers/mtd/spi-nor/core.c >> @@ -1618,12 +1618,12 @@ static int spi_nor_erase_multi_sectors(struct >> spi_nor *nor, u64 addr, u32 len) >> if (ret) >> goto destroy_erase_cmd_list; >> >> - addr += cmd->size; >> - cmd->count--; >> - >> ret = spi_nor_wait_till_ready(nor); >> if (ret) >> goto destroy_erase_cmd_list; >> + >> + addr += cmd->size; >> + cmd->count--; >> } >> list_del(&cmd->list); >> kfree(cmd); >> @@ -1704,12 +1704,12 @@ static int spi_nor_erase(struct mtd_info *mtd, >> struct erase_info *instr) >> if (ret) >> goto erase_err; >> >> - addr += mtd->erasesize; >> - len -= mtd->erasesize; >> - >> ret = spi_nor_wait_till_ready(nor); >> if (ret) >> goto erase_err; >> + >> + addr += mtd->erasesize; >> + len -= mtd->erasesize; > > Do these changes have any practical benefit? IMO they are worth doing > even if there is none but I'm curious what prompted this patch.
I saw these when reviewing Takahiro's patches. Addr and len were gratuitously updated even when the wait failed. We'll avoid 2 extra ops on the error path. Plus, having them updated before the wait can be misleading for someone that tracks them down with some debug messages. I find the code better structured, and the code will make more sense when it is read, if using this patch. > > Reviewed-by: Pratyush Yadav <[email protected]> Thanks, ta > >> } >> >> /* erase multiple sectors */ > > -- > Regards, > Pratyush Yadav > Texas Instruments Inc. >
