Re: [PATCH v2 5/9] mtd: rawnand: onfi: Define the number of parameter pages

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

On Fri, 24 Apr 2020 18:40:38 +0200
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:

> Use a macro to define the number of parameter page instead of
> hardcoding it everywhere.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> ---
>  drivers/mtd/nand/raw/nand_onfi.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
> index 7d9a3130443a..9fe39adbde4c 100644
> --- a/drivers/mtd/nand/raw/nand_onfi.c
> +++ b/drivers/mtd/nand/raw/nand_onfi.c
> @@ -16,6 +16,8 @@
>  
>  #include "internals.h"
>  
> +#define ONFI_PARAM_PAGES 3
> +
>  u16 onfi_crc16(u16 crc, u8 const *p, size_t len)
>  {
>  	int i;
> @@ -156,7 +158,7 @@ int nand_onfi_detect(struct nand_chip *chip)
>  		return 0;
>  
>  	/* ONFI chip: allocate a buffer to hold its parameter page */
> -	p = kzalloc((sizeof(*p) * 3), GFP_KERNEL);
> +	p = kzalloc((sizeof(*p) * ONFI_PARAM_PAGES), GFP_KERNEL);
>  	if (!p)
>  		return -ENOMEM;
>  
> @@ -166,7 +168,7 @@ int nand_onfi_detect(struct nand_chip *chip)
>  		goto free_onfi_param_page;
>  	}
>  
> -	for (i = 0; i < 3; i++) {
> +	for (i = 0; i < ONFI_PARAM_PAGES; i++) {
>  		ret = nand_read_data_op(chip, &p[i], sizeof(*p), true);
>  		if (ret) {
>  			ret = 0;
> @@ -181,8 +183,8 @@ int nand_onfi_detect(struct nand_chip *chip)
>  		}
>  	}
>  
> -	if (i == 3) {
> -		const void *srcbufs[3] = {p, p + 1, p + 2};
> +	if (i == ONFI_PARAM_PAGES) {
> +		const void *srcbufs[ONFI_PARAM_PAGES] = {p, p + 1, p + 2};
>  

Maybe initialize the srcbufs array using a for loop so you can easily
change ONFI_PARAM_PAGES without having to touch the code. Looks good
otherwise, so

Reviewed-by: Boris Breillon <boris.brezillon@xxxxxxxxxxxxx>

>  		pr_warn("Could not find a valid ONFI parameter page, trying bit-wise majority to recover it\n");
>  		nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), p,


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/