[PATCH 3/3][MTD] P4080/nand: Fix the freescale lbc issue with 36bit mode

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru at gmail.com]
> Sent: Friday, September 03, 2010 19:36 PM
> To: Zang Roy-R61911
> Cc: linux-mtd at lists.infradead.org; Lan Chunhe-B25806; linuxppc-dev at ozlabs.org;
> akpm at linux-foundation.org; Gala Kumar-B11780
> Subject: Re: [PATCH 3/3][MTD] P4080/nand: Fix the freescale lbc issue with
> 36bit mode
> 
> On Fri, Aug 06, 2010 at 10:51:36AM +0800, Roy Zang wrote:
> [...]
> >  /**
> > + * convert_lbc_address - convert the base address
> > + * @addr_base:	base address of the memory bank
> > + *
> > + * This function converts a base address of lbc into the right format for
> the BR
> > + * registers. If the SOC has eLBC then it returns 32bit physical address
> else
> > + * it returns 34bit physical address for local bus(Example: MPC8641).
> > + */
> > +unsigned int convert_lbc_address(phys_addr_t addr_base)
> > +{
> > +	void *dev;
> > +	int compatible;
> > +
> > +	dev = of_find_node_by_name(NULL, "localbus");
> 
> Nope, you shouldn't do this. Never search by name.
OK. I will search by compatible.
> 
> Also, aren't there already a global dev, which was found by the
> _probe() stuff?
I do not see the global dev can be used in probe.

> 
> > +	if (!dev) {
> > +		printk(KERN_INFO "fsl-lbc: can't find localbus node\n");
> > +		of_node_put(dev);
> > +		return 0;
> > +	}
> > +
> > +	compatible = of_device_is_compatible(dev, "fsl,elbc");
> > +	of_node_put(dev);
> > +	if (compatible)
> > +		return addr_base & 0xffff8000;
> > +	else
> > +		return (addr_base & 0x0ffff8000ull) \
> > +			| ((addr_base & 0x300000000ull) >> 19);
> > +}
> > +EXPORT_SYMBOL(convert_lbc_address);
> > +
> > +/**
> >   * fsl_lbc_find - find Localbus bank
> >   * @addr_base:	base address of the memory bank
> >   *
> > @@ -50,7 +80,8 @@ int fsl_lbc_find(phys_addr_t addr_base)
> >  		__be32 br = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].br);
> >  		__be32 or = in_be32(&fsl_lbc_ctrl_dev->regs->bank[i].or);
> >
> > -		if (br & BR_V && (br & or & BR_BA) == addr_base)
> > +		if (br & BR_V && (br & or & BR_BA) \
> 
> No need for "\" at the end of the line, keep == on the same line.
> 
> > +				== convert_lbc_address(addr_base))
> 
> Would be prettier if you name it fsl_lbc_addr(). Keeps prefix
> the same for the rest of the file, plus makes it shorter (so
> there probably won't be any need for breaking the line).
Agree the new name.
Thanks.
Roy