Re: [PATCH 3/3] mtd: nand: add option to ignore bad blocks when erasing, opt-in through debugfs

[Boris Brezillon <boris.brezillon@free-electrons.com>]

Le Sat, 20 May 2017 12:24:28 -0300,
"Mario J. Rugiero" <mrugiero@gmail.com> a écrit :

And again, pleases add a commit message.

> Signed-off-by: Mario J. Rugiero <mrugiero@gmail.com>
> ---
>  drivers/mtd/nand/nand_base.c | 26 ++++++++++++++++++++++----
>  include/linux/mtd/nand.h     | 12 ++++++++++++
>  2 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 52a257e12026..8cffea38a642 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -47,6 +47,7 @@
>  #include <linux/io.h>
>  #include <linux/mtd/partitions.h>
>  #include <linux/of.h>
> +#include <linux/debugfs.h>
>  
>  static int nand_get_device(struct mtd_info *mtd, int new_state);
>  
> @@ -3209,10 +3210,15 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr,
>  		/* Check if we have a bad block, we do not erase bad blocks! */
>  		if (nand_block_checkbad(mtd, ((loff_t) page) <<
>  					chip->page_shift, allowbbt)) {
> -			pr_warn("%s: attempt to erase a bad block at page 0x%08x\n",
> -				    __func__, page);
> -			instr->state = MTD_ERASE_FAILED;
> -			goto erase_exit;
> +			if (!chip->dbg.scrub_enabled) {
> +				pr_warn("%s: attempt to erase a bad block at page 0x%08x\n",
> +				    	__func__, page);
> +				instr->state = MTD_ERASE_FAILED;
> +				goto erase_exit;
> +			} else {
> +				pr_warn("%s: erasing a bad block at page 0x%08x\n",
> +					__func__, page);
> +			}
>  		}
>  
>  		/*
> @@ -4931,6 +4937,18 @@ int nand_device_register(struct mtd_info *mtd,
>  			 int defnr_parts)
>  {
>  	int ret = nand_device_register(mtd, defparts, defnr_parts);
> +	struct nand_debug_info *dbg;
> +
> +	if (!ret) {
> +		dbg = &mtd_to_nand(mtd)->dbg;
> +		dbg->scrub_enabled = false;
> +		dbg->dfs_scrub_enabled = debugfs_create_bool("scrub_enabled",

Or just "scrub", the enable/disable status is reflected by the value
exposed by the file.
BTW, not sure scrub is clear enough, how about "allow-bad-block-erase"?

> +							     0600,
> +							     mtd->dbg.dfs_dir,
> +							     &dbg->scrub_enabled);
> +		if (IS_ERR(dbg->dfs_scrub_enabled))
> +			dbg->dfs_scrub_enabled = NULL;
> +	}

I prefer:

	struct nand_debug_info *dbg;
	int ret;

	ret = mtd_device_register(mtd, defparts, defnr_parts);
	if (ret)
		return ret;

	dbg = &chip->dbg;
	dbg->scrub_enabled = false;
	dbg->dfs_scrub_enabled = debugfs_create_bool("scrub", 0600, mtd->dbg.dfs_dir,
						     &dbg->scrub_enabled);
	if (IS_ERR(dbg->dfs_scrub_enabled))
		dbg->dfs_scrub_enabled = NULL;
  
  	return 0;


This way the error path is easily identified and you don't have to add
an indentation level for the normal path.

}