Re: [PATCH v4 3/4] mtd: nand: mediatek: add support for different MTK NAND FLASH Controller IP

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

Le Wed, 31 May 2017 11:37:56 +0800,
Xiaolei Li <xiaolei.li@mediatek.com> a écrit :

>  
> -static void mtk_nfc_set_spare_per_sector(u32 *sps, struct mtd_info *mtd)
> +static int mtk_nfc_set_spare_per_sector(u32 *sps, struct mtd_info *mtd)

Why do you change the prototype here? You seem to always return 0
anyway.

>  {
>  	struct nand_chip *nand = mtd_to_nand(mtd);
> -	u32 spare[] = {16, 26, 27, 28, 32, 36, 40, 44,
> -			48, 49, 50, 51, 52, 62, 63, 64};
> -	u32 eccsteps, i;
> +	struct mtk_nfc *nfc = nand_get_controller_data(nand);
> +	const u8 *spare = nfc->caps->spare_size;
> +	u32 eccsteps, i, j = 0;

Can we rename 'j' into 'closest_spare'?

>  
>  	eccsteps = mtd->writesize / nand->ecc.size;
>  	*sps = mtd->oobsize / eccsteps;
> @@ -1144,28 +1102,28 @@ static void mtk_nfc_set_spare_per_sector(u32 *sps, struct mtd_info *mtd)
>  	if (nand->ecc.size == 1024)
>  		*sps >>= 1;
>  
> -	for (i = 0; i < ARRAY_SIZE(spare); i++) {
> -		if (*sps <= spare[i]) {
> -			if (!i)
> -				*sps = spare[i];
> -			else if (*sps != spare[i])
> -				*sps = spare[i - 1];
> -			break;
> +	for (i = 0; i < nfc->caps->num_spare_size; i++) {
> +		if ((*sps >= spare[i]) && (spare[i] >= spare[j])) {

Parenthesis around the 'a >= b' tests are unneeded:

		if (*sps >= spare[i] && spare[i] >= spare[j]) {

> +			j = i;
> +			if (*sps == spare[i])
> +				break;
>  		}
>  	}
>  
> -	if (i >= ARRAY_SIZE(spare))
> -		*sps = spare[ARRAY_SIZE(spare) - 1];

Maybe you could return an error if you didn't find any entry that is
less that *sps in the table, but I'm not sure this can really happen,
and the minimum spare size seems to be the same for all IPs, this is
something you can check before iterating over the array:

	if (*sps < MTK_NFC_MIN_SPARE)
		return -EINVAL;

> +	*sps = spare[j];
>  
>  	if (nand->ecc.size == 1024)
>  		*sps <<= 1;
> +
> +	return 0;
>  }