[U-Boot] [PATCH 1/6] nand: denali: add Denali NAND driver for SPL

[Masahiro Yamada <yamada.m@jp.panasonic.com>]

Hi Marek,


On Fri, 4 Jul 2014 16:34:11 +0200
Marek Vasut <marex@denx.de> wrote:

> 
> > +static void read_data_from_flash_mem(uint8_t *buf, int len)
> > +{
> > +	int i;
> > +	uint32_t *buf32;
> > +
> > +	/* transfer the data from the flash */
> > +	buf32 = (uint32_t *)buf;
> > +	for (i = 0; i < len / 4; i++)
> > +		*buf32++ = readl(denali_flash_mem + INDEX_DATA_REG);
> 
> Won't this trigger unaligned access if $buf is not aligned to 4-byte boundary ?


Thanks for catching this.

Actually, it would seldom happen because the aligned load address is generally given.

But I will add a sanity check here in v2, just in case.



> > +
> > +	/* setup the pipeline command */
> > +	index_addr(cmd, 0x2000 | page_count);
> 
> Magic value 0x2000 should be fixed here.


Actually, this part is the same as that of the normal Denali driver
by Chin Liang See
http://patchwork.ozlabs.org/patch/357717/

Moreover, it is the same as in Linux Kernel.

I understand what you mean, but I guess we can excuse here.




> > +	cmd = MODE_01 | addr;
> > +	writel(cmd, denali_flash_mem + INDEX_CTRL_REG);
> 
> Somehow, eventually, this should be migrated to "struct" based register access 
> instead of such offset computation. If you feel like doing it, that'd be nice 
> ;-)

Dito.
I am following the coding style of the normal Denali driver and the Linux one.

We might perhaps do that migration as the separated task in future, but not now.



Best Regards
Masahiro Yamada