Hi Masahiro,
On Wed, 2014-03-12 at 20:25 +0900, Masahiro Yamada wrote:
> Hello Chin,
>
>
> Here is a little more comments about v5.
>
> > +static void get_toshiba_nand_para(void)
> > +{
> > + uint32_t tmp;
> > +
> > + /* Workaround to fix a controller bug which reports a wrong */
> > + /* spare area size for some kind of Toshiba NAND device */
> > + if ((readl(denali.flash_reg + DEVICE_MAIN_AREA_SIZE) == 4096) &&
> > + (readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE)
> > + == 64)){
> > + writel(216, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
> > + tmp = readl(denali.flash_reg + DEVICES_CONNECTED) *
> > + readl(denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
> > + writel(tmp, denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
> > +#if SUPPORT_15BITECC
> > + writel(15, denali.flash_reg + ECC_CORRECTION);
> > +#elif SUPPORT_8BITECC
> > + writel(8, denali.flash_reg + ECC_CORRECTION);
> > +#endif
>
> I can't understand why ECC_CORRECTION must be set here.
> Can you?
> If not, could you delete it?
>
Yup, this is not necessary as we setup this at init function.
Removed
>
>
> > +static void get_hynix_nand_para(uint8_t device_id)
> > +{
> > + uint32_t main_size, spare_size;
> > +
> > + switch (device_id) {
> > + case 0xD5: /* Hynix H27UAG8T2A, H27UBG8U5A or H27UCG8VFA */
> > + case 0xD7: /* Hynix H27UDG8VEM, H27UCG8UDM or H27UCG8V5A */
> > + writel(128, denali.flash_reg + PAGES_PER_BLOCK);
> > + writel(4096, denali.flash_reg + DEVICE_MAIN_AREA_SIZE);
> > + writel(224, denali.flash_reg + DEVICE_SPARE_AREA_SIZE);
> > + main_size = 4096 *
> > + readl(denali.flash_reg + DEVICES_CONNECTED);
> > + spare_size = 224 *
> > + readl(denali.flash_reg + DEVICES_CONNECTED);
> > + writel(main_size, denali.flash_reg + LOGICAL_PAGE_DATA_SIZE);
> > + writel(spare_size, denali.flash_reg + LOGICAL_PAGE_SPARE_SIZE);
> > + writel(0, denali.flash_reg + DEVICE_WIDTH);
> > +#if SUPPORT_15BITECC
> > + writel(15, denali.flash_reg + ECC_CORRECTION);
> > +#elif SUPPORT_8BITECC
> > + writel(8, denali.flash_reg + ECC_CORRECTION);
> > +#endif
>
> Ditto.
>
Removed
>
>
> > +void denali_nand_init(struct nand_chip *nand)
> > +{
> > + denali.flash_reg = (void __iomem *)CONFIG_SYS_NAND_REGS_BASE;
> > + denali.flash_mem = (void __iomem *)CONFIG_SYS_NAND_DATA_BASE;
> > +
> > + nand->chip_delay = 0;
> > +#ifdef CONFIG_SYS_NAND_USE_FLASH_BBT
> > + /* check whether flash got BBT table (located at end of flash). As we
> > + * use NAND_BBT_NO_OOB, the BBT page will start with
>
>
>
> nand->chip_delay = 0;
> is not necessary.
> Please delete.
>
>
> if chip_delay is zero, chip_delay is set to 20
> in nand_set_defaults() function.
>
> Anyway it is never used in Denali NAND driver,
> because we are overriding cmdfunc with denali_cmdfunc().
>
Good catch, this is not required.
Initially I would let user to use other delay value.
As we are using own cmdfunc, this is not relevant.
Thanks
Chin Liang
>
>
>
>
> Best Regards
> Masahiro Yamada
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
