[U-Boot] [PATCH V2 2/4] mtd/NAND: Add FSMC driver support

[Scott Wood <scottwood@freescale.com>]

On 05/07/2012 02:26 AM, Amit Virdi wrote:
> +	while (num_err--) {
> +		change_bit(0, &err_idx[i]);
> +		change_bit(1, &err_idx[i]);
> +
> +		if (err_idx[i] < 512 * 8) {
> +			change_bit(err_idx[i], dat);
> +			i++;
> +		}
> +	}

Is it normal to not count bit flips in the ECC itself?

> +{
> +	u_int ecc_tmp;
> +
> +	switch (fsmc_version) {
> +	case FSMC_VER8:
> +		/* Busy waiting for ecc computation to finish for 512 bytes */
> +		while (!(readl(&fsmc_regs_p->sts) & FSMC_CODE_RDY))
> +			;

Timeout?

> +	/*
> +	 * ecc_oob is intentionally taken as u16. In 16bit devices, we end up
> +	 * reading 14 bytes (7 words) from oob. The local array is to maintain
> +	 * word alignment
> +	 */
> +	uint16_t ecc_oob[7];
> +	uint8_t *oob = (uint8_t *)&ecc_oob[0];

Please use a union, or better an explicit alignment attribute.

> +enum {
> +	FSMC_VER1 = 1,
> +	FSMC_VER2,
> +	FSMC_VER3,
> +	FSMC_VER4,
> +	FSMC_VER5,
> +	FSMC_VER6,
> +	FSMC_VER7,
> +	FSMC_VER8,
> +};

Is this really necessary?

-Scott