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

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru at gmail.com]
> Sent: Thursday, September 09, 2010 19:42 PM
> To: Zang Roy-R61911
> Cc: linux-mtd at lists.infradead.org; dwmw2 at infradead.org; dedekind1 at gmail.com;
> akpm at linux-foundation.org; Lan Chunhe-B25806; Wood Scott-B07421; Gala Kumar-
> B11780; linuxppc-dev at ozlabs.org
> Subject: Re: [PATCH v2 3/3][MTD] P4080/mtd: Fix the freescale lbc issue with
> 36bit mode
> 
> On Thu, Sep 09, 2010 at 06:20:32PM +0800, Roy Zang wrote:
> [...]
> >  /**
> > + * fsl_lbc_addr - 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).
> > + */
> 
> It returns 34bit physical address encoded in a 32 bit word,
> right? 
Yes.

>Because, IIRC, 'unsigned int' is always 32 bit.
> 
> Worth mentioning this fact.
Agree.
The comment is a little bit confusing. Will update.


> 
> > +unsigned int fsl_lbc_addr(phys_addr_t addr_base)
> > +{
> > +	void *dev;
> 
> struct device_node *np;
> 
> > +	int compatible;
> > +
> > +	dev = fsl_lbc_ctrl_dev->dev->of_node;
> > +	compatible = of_device_is_compatible(dev, "fsl,elbc");
> > +
> > +	if (compatible)
> > +		return addr_base & 0xffff8000;
> > +	else
> > +		return (addr_base & 0x0ffff8000ull)
> > +			| ((addr_base & 0x300000000ull) >> 19);
> > +}
> > +EXPORT_SYMBOL(fsl_lbc_addr);
> 
> Almost perfect. I'm not sure if 'unsigned int' is technically
> correct return type for this function though. I guess it should
> be u32.
> 
What is the different for unsigned int and u32? I think they are same.


> Also, the function may be a bit more understandable and shorter:
> 
> u32 fsl_lbc_addr(phys_addr_t addr)
> {
> 	struct device_node *np = fsl_lbc_ctrl_dev->dev->of_node;
> 	u32 addrl = addr & 0xffff8000;
if addr is 34 bit long, is this correct?
Thanks.
Roy