[U-Boot] [PATCH v2 10/17] SPEAr : FSMC driver support added

[Vipin KUMAR]

>> + * Vipin Kumar, ST Micoelectronics, vipin.kumar at st.com.
>> + *
>> + * See file CREDITS for list of people who contributed to this
>> + * project.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +
>> +#include <common.h>
>> +#include <nand.h>
>> +#include <asm/io.h>
>> +#include <linux/bitops.h>
>> +#include <linux/err.h>
>> +#include <linux/mtd/nand_ecc.h>
>> +#include <linux/mtd/fsmc_nand.h>
>> +#include <asm/arch/hardware.h>
>> +
>> +static u32 fsmc_version;
>> +static struct fsmc_regs *const fsmc_regs_p =
>> +    (struct fsmc_regs *)CONFIG_SYS_FSMC_BASE;
>> +
> 
> CONFIG_SYS_FSMC_BASE is not defined until 11/17
> This make this change non-bisectable.
> Fix.

Since this driver is implemented independent of the spear platform, and it 
becomes the responsibility of the platform using this driver to #define 
this macro before using it.
Should I do it some other way ?

>> +    i = 0;
>> +    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++;
>> +        }
>> +    }
>> +    return i;
>> +}
>> +
>> +static int fsmc_read_hwecc(struct mtd_info *mtd,
>> +                const u_char *data, u_char *ecc)
>> +{
>> +    u_int ecc_tmp;
>> +
>> +    switch (fsmc_version) {
>> +    case FSMC_VER8:
>> +        while (!(readl(&fsmc_regs_p->sts) & FSMC_CODE_RDY))
>> +            ;
> 
> Add a comment that this is a busy wait.

OK.

>> +    uint16_t ecc_oob[7];
>> +
>> +    for (i = 0, s = 0; s < eccsteps; s++, i += eccbytes, p += eccsize) {
>> +
>> +        chip->cmdfunc(mtd, NAND_CMD_READ0, s * eccsize, page);
>> +        chip->ecc.hwctl(mtd, NAND_ECC_READ);
>> +        chip->read_buf(mtd, p, eccsize);
>> +
>> +        for (j = 0; j < eccbytes;) {
>> +            off = fsmc_eccpl.eccplace[s].offset;
>> +            len = fsmc_eccpl.eccplace[s].length;
>> +
>> +            /*
>> +             * length is intentionally kept a higher multiple of 2
>> +             * to read at least 13 bytes even in case of 16 bit NAND
>> +             * devices
>> +             */
>> +            len = roundup(len, 2);
>> +            chip->cmdfunc(mtd, NAND_CMD_READOOB, off, page);
>> +            chip->read_buf(mtd, (uint8_t *)&ecc_oob[j], len);
>> +            j += len;
>> +        }
>> +
>> +        memcpy(&ecc_code[i], ecc_oob, 13);
>> +        chip->ecc.calculate(mtd, p, &ecc_calc[i]);
>> +
>> +        stat = chip->ecc.correct(mtd, p, &ecc_code[i], &ecc_calc[i]);
>> +        if (stat < 0)
>> +            mtd->ecc_stats.failed++;
>> +        else
>> +            mtd->ecc_stats.corrected += stat;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +int fsmc_nand_init(struct nand_chip *nand)
>> +{
>> +    u32 peripid2 = readl(&fsmc_regs_p->peripid2);
>> +
>> +    fsmc_version = (peripid2 >> FSMC_REVISION_SHFT) &
>> +               FSMC_REVISION_MSK;
>> +#if defined(CONFIG_SYS_FSMC_NAND_16BIT)
> 
> 16BIT is never defined
> This is dead code
> Remove

Since this is a generic driver, any platform using fsmc peripheral can enable 
this feature by #defining CONFIG_SYS_FSMC_NAND_16BIT in respective 
include/configs/xxx.h


All other comments are accepted and would be fixed in v3

Thanks and Regards
Vipin