[U-Boot] [U-Boot, v8] nand/denali: Adding Denali NAND driver support

[Masahiro Yamada]

Hi Chin,



On Tue, 19 Aug 2014 04:18:44 -0500
Chin Liang See <clsee at altera.com> wrote:

> On Tue, 2014-08-19 at 16:28 +0800, Chin Liang See wrote:
> > From: Scott Wood [mailto:scottwood at freescale.com] 
> > Sent: Friday, June 20, 2014 9:39 AM
> > To: Chin Liang See
> > Cc: ZY - u-boot
> > Subject: Re: [U-Boot,v8] nand/denali: Adding Denali NAND driver support
> > 
> > On Tue, Jun 10, 2014 at 12:42:19AM -0500, Chin Liang See wrote:
> > > To add the Denali NAND driver support into U-Boot. It required
> > > information such as register base address from configuration
> > > header file  within include/configs folder.
> > 
> > This is hard to parse.  What exactly is required from include/configs and
> > where is it documented?
> > 
> > I see that this driver exists in Linux...  Is this patch related to a
> > particular Linux SHA1?
> > 
> 
> Yup, this driver is leveraged from Linux. I will update the commit
> message to mention about this.
> 

I know this driver is leveraged from Linux.
Is it difficult to describe the particular SHA1 or tag you took it from?




> 
> > 
> > > +/* Reset the flash controller */
> > > +static uint32_t denali_nand_reset(struct denali_nand_info *denali)
> > > +{
> > > +	uint32_t i;
> > > +
> > > +	for (i = 0; i < denali->max_banks; i++)
> > > +		writel(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT,
> > > +		       denali->flash_reg + INTR_STATUS(i));
> > > +
> > > +	for (i = 0; i < denali->max_banks; i++) {
> > > +		writel(1 << i, denali->flash_reg + DEVICE_RESET);
> > > +		while (!(readl(denali->flash_reg + INTR_STATUS(i)) &
> > > +			(INTR_STATUS__RST_COMP | INTR_STATUS__TIME_OUT)))
> > > +			if (readl(denali->flash_reg + INTR_STATUS(i)) &
> > > +				INTR_STATUS__TIME_OUT)
> > > +				debug(KERN_DEBUG "NAND Reset operation "
> > > +					"timed out on bank %d\n", i);
> > > +	}
> > 
> > WARNING: quoted string split across lines
> > #283: FILE: drivers/mtd/nand/denali.c:203:
> > +                               debug(KERN_DEBUG "NAND Reset operation "
> > +                                       "timed out on bank %d\n", i);
> > 
> > (likewise elsewhere)
> > 
> > This instance is not even from Linux -- and where does KERN_DEBUG come
> > from?  The Linux driver has never used it.
> 
> Its defined as empty. I will remove them.


I pointed out that you could still use dev_err() and dev_warn as Linux
and I don't see the necessity to diverge from Linux here.

Anyway your way work enough...



Best Regards
Masahiro Yamada