[PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common to elbc devices

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru at gmail.com]
> Sent: Monday, September 06, 2010 16:22 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; Wood Scott-B07421
> Subject: Re: [PATCH 1/3][MTD] P4080/eLBC: Make Freescale elbc interrupt common
> to elbc devices
> 
> On Mon, Sep 06, 2010 at 11:38:09AM +0800, Zang Roy-R61911 wrote:
> [...]
> > > >  	switch (br & BR_MSEL) {
> > > >  	case BR_MS_UPMA:
> > > > -		upm->mxmr = &fsl_lbc_regs->mamr;
> > > > +		upm->mxmr = &fsl_lbc_ctrl_dev->regs->mamr;
> > >
> > > Ditto, a very repetitive stuff, desires a variable for regs?
> > But the fact is that the variable represents different reg
> > address according to the condition. It will be ugly to use
> > the reg address directoly.
> 
> I meant a dedicated var for 'fsl_lbc_ctrl_dev->regs'.
> I.e.
> 
> regs = fsl_lbc_ctrl_dev->regs;
> ...
> mxmr = &regs->mamr;
> ...
> mxmr = &regs->mbmr;
> ..
> mxmr = &regs->mcmr;
> 
> Instead of
> 
> mxmr = &fsl_lbc_ctrl_dev->regs->mamr;
> ...
> mxmr = &fsl_lbc_ctrl_dev->regs->mbmr;
> ..
> mxmr = &fsl_lbc_ctrl_dev->regs->mcmr;
That makes sense.  A global or local variable for fsl_lbc_ctrl_dev->regs? Which one is better?

> 
> [...]
> > > > +static int __devinit fsl_lbc_ctrl_probe(struct of_device *ofdev,
> > > > +					 const struct of_device_id *match)
> > > > +{
> > > > +	int ret = 0;
> > >
> > > no need for the initial value here.
> > Any harm?
> 
> Probably not as gcc will likely optimize it away,
> but it's not needed, so why keep it there?
habit.
Thanks.
Roy