[U-Boot] [PATCH 3/4] mtd: atmel_nand: alloc memory instead of use static array for pmecc data

[Scott Wood]

On 06/14/2013 06:20:41 AM, Josh Wu wrote:
> +static int pmecc_data_alloc(struct atmel_nand_host *host)
> +{
> +	const int cap = host->pmecc_corr_cap;
> +	int size;
> +
> +	size = (2 * cap + 1) * sizeof(int16_t);
> +	host->pmecc_partial_syn = (int16_t *)malloc(size);
> +	host->pmecc_si = (int16_t *)malloc(size);
> +	host->pmecc_lmu = (int16_t *)malloc((cap + 1) *  
> sizeof(int16_t));
> +	host->pmecc_smu = (int16_t *)
> +		malloc((cap + 2) * size);
> +
> +	size = (cap + 1) * sizeof(int);
> +	host->pmecc_mu = (int *)malloc(size);
> +	host->pmecc_dmu = (int *)malloc(size);
> +	host->pmecc_delta = (int *)malloc(size);

Don't cast malloc's return value.  Checkpatch would catch this, except  
that it hasn't been updated to deal with U-Boot using plain "malloc"  
rather than kmalloc and friends.

> +	if (!host->pmecc_partial_syn ||
> +		!host->pmecc_si ||
> +		!host->pmecc_lmu ||
> +		!host->pmecc_smu ||
> +		!host->pmecc_mu ||
> +		!host->pmecc_dmu ||
> +		!host->pmecc_delta)
> +		return -ENOMEM;

Don't line up continuation lines with the body of the "if".  It makes  
it harder to see where the condition ends and the body begins.

Please free any of the allocations that did succeed before returning  
-ENOMEM.

>  static void pmecc_gen_syndrome(struct mtd_info *mtd, int sector)
>  {
>  	struct nand_chip *nand_chip = mtd->priv;
> @@ -710,6 +751,13 @@ static int atmel_pmecc_nand_init_params(struct  
> nand_chip *nand,
>  		return 0;
>  	}
> 
> +	/* Allocate data for PMECC computation */
> +	if (pmecc_data_alloc(host)) {
> +		printk(KERN_ERR "Cannot allocate memory for PMECC  
> computation!\n");
> +		pmecc_data_free(host);
> +		return -ENOMEM;
> +	}

In the Linux driver this is dev_err, so why printk(KERN_ERR here?  Just  
use printf -- or move the dev_err compatibility out of the USB code so  
the rest of U-Boot can use it.

-Scott