Re: [PATCH 2/4][SSB] EXTIF serial port initialization

[Arnaldo Carvalho de Melo <acme@ghostprotocols.net>]

Em Mon, Aug 06, 2007 at 10:51:05AM +0200, Michael Buesch escreveu:
> On Monday 06 August 2007, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Aug 06, 2007 at 01:19:59AM +0200, Aurelien Jarno escreveu:
> > > The patch below against 2.6.23-rc1-mm2 implements EXTIF serial
> > > initialization, currently marked as TODO.
> > > 
> > > It originally comes from the OpenWrt patches.
> > 
> > Comments below
> >  
> > > Cc: Felix Fietkau <nbd@openwrt.org>
> > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > > 
> > > --- a/drivers/ssb/driver_mipscore.c
> > > +++ b/drivers/ssb/driver_mipscore.c
> > > @@ -128,51 +128,52 @@
> > >  	ssb_write32(mdev, SSB_IPSFLAG, irqflag);
> > >  }
> > >  
> > > -/* XXX: leave here or move into separate extif driver? */
> > > -static int ssb_extif_serial_init(struct ssb_device *dev, struct ssb_serial_ports *ports)
> > > +static inline bool serial_exists(u8 *regs)
> > >  {
> > > +	u8 save_mcr, msr = 0;
> > 
> > Why declare save_mcr here...
> > 
> > >  
> > > +	if (regs) {
> > 
> > if it is just used here?
> 
> There is almost never an advantage when declaring variables at the beginning
> of a statement block instead of the function start.
> The only thing you "gain" is that you don't easily see anymore how much
> stackspace is used by the funtions.

Well, I disagree, your argument is flawed because if the function is too
big to have so many variables it should be split in smaller functions
anyway, and when it is not that big, like in the above case one can see
the stack usage just fine on the same screen.

> So I really suggest to declare variables at the beginning of functions,
> except for some rare circumstances maybe. Which is not the case here.

I'd recommend the reverse, but anyway, it is just a suggestion.
 
- Arnaldo