From mboxrd@z Thu Jan  1 00:00:00 1970
From: Chin Liang See <clsee@altera.com>
Date: Wed, 12 Mar 2014 08:58:55 -0500
Subject: [U-Boot] [PATCH v5] nand/denali: Adding Denali NAND driver
	support
In-Reply-To: <20140312135520.6F3F.AA925319@jp.panasonic.com>
References: <1394492783-3332-1-git-send-email-clsee@altera.com>
	<20140312135520.6F3F.AA925319@jp.panasonic.com>
Message-ID: <1394632735.3823.2.camel@clsee-VirtualBox.altera.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Masahiro,

On Wed, 2014-03-12 at 13:55 +0900, Masahiro Yamada wrote:
> Hello Chin,
> 
> 
> 
> 
> > +static uint32_t denali_dma_configuration(uint32_t ops, bool raw_xfer,
> > +	uint32_t irq_mask, int oob_required)
> 
> Indentation not fixed yet.
> I mean, only one tab is too small for the second line.
> 

oops, actually I miss this one.
Fixed in next revision


> Rationale:  "Documentation/CodingStyle" of Linux Kernel
> 
> Accoding to it, the following lines should be "placed to substantially
> to the right."
> 
> 
> 
>              Chapter 2: Breaking long lines and strings
> 
> Coding style is all about readability and maintainability using commonly
> available tools.
> 
> The limit on the length of lines is 80 columns and this is a strongly
> preferred limit.
> 
> Statements longer than 80 columns will be broken into sensible chunks, unless
> exceeding 80 columns significantly increases readability and does not hide
> information. Descendants are always substantially shorter than the parent and
> are placed substantially to the right. The same applies to function headers
> with a long argument list. However, never break user-visible strings such as
> printk messages, because that breaks the ability to grep for them.
> 
> 
> 
> 
> 
> 
> > +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;
> 
> 
> denali_nand_init()  is used only inside this file.
> I'd like to suggest to add "static".
> 
> 

Fixed


> 
> 
> > +int board_nand_init(struct nand_chip *chip)
> > +{
> > +	puts("NAND:  Denali NAND controller\n");
> > +	denali_nand_init(chip);
> > +	return 0;
> > +}
> 
> Please delete    
> puts("NAND:  Denali NAND controller\n");
> 

Actually I added this during debugging and forget remove this.
Removed for next patch

Thanks

Chin Liang


> 
> With this patch, the boot messages on my board is like this:
> 
>     U-Boot 2014.04-rc2-00056-g05775a7-dirty (Mar 12 2014 - 13:04:36)
>     
>     CPU:   Peaks LD4 (MN2WS0250) (rev. 3)
>     Board: Peaks LD4 Board
>     SC:    Micro Support Card (DCC CPLD version 3.6.9)
>     DRAM:  512 MiB
>     NAND:  NAND:  Denali NAND controller
>     2048 MiB
>     In:    serial
>     Out:   serial
>     Err:   serial
>     Net:   smc911x-0
> 
> 
> 
> "NAND:  Denali NAND controller" is interrupting
> between "NAND: "  and "2048 Mib"
>     
> 
> I think it is supposed be seen like this:
> 
>     DRAM:  512 MiB
>     NAND:  2048 MiB
>     In:    serial
>     Out:   serial
>     Err:   serial
> 
> 
> 
> 
> Best Regards
> Masahiro Yamada
>