Re: sound/pci/hda/intel_hda: small cleanups

[Takashi Iwai <tiwai@suse.de>]

At Thu, 31 Aug 2006 15:39:29 +0200,
Pavel Machek wrote:
> 
> Hi!
> 
> > > @@ -271,8 +272,8 @@ struct azx_dev {
> > >  	/* for sanity check of position buffer */
> > >  	unsigned int period_intr;
> > >  
> > > -	unsigned int opened: 1;
> > > -	unsigned int running: 1;
> > > +	unsigned int opened :1;
> > > +	unsigned int running :1;
> > >  };
> > >  
> > >  /* CORB/RIRB */
> > > @@ -330,8 +331,8 @@ struct azx {
> > >  
> > >  	/* flags */
> > >  	int position_fix;
> > > -	unsigned int initialized: 1;
> > > -	unsigned int single_cmd: 1;
> > > +	unsigned int initialized :1;
> > > +	unsigned int single_cmd :1;
> > >  };
> > 
> > Any official standard reference for bit-field expressions?
> 
> Well, logically : belongs to the 1, and include/linux understands it
> like that...

OK, "xxx :1;" looks major, too :)

> > >  /* driver types */
> > > @@ -642,14 +643,14 @@ static int azx_reset(struct azx *chip)
> > >  	azx_writeb(chip, GCTL, azx_readb(chip, GCTL) | ICH6_GCTL_RESET);
> > >  
> > >  	count = 50;
> > > -	while (! azx_readb(chip, GCTL) && --count)
> > > +	while (!azx_readb(chip, GCTL) && --count)
> > >  		msleep(1);
> > 
> > Hm, it looks rather like a personal preference.
> > IMHO, it's harder to read without space...
> 
> Well, core parts (sched.c?) use it without the space, and I'd say (!
> expression) is unusual in kernel, but no, could not find it codified.

I don't mind much to change it now, but hopefully people won't be too
strict about this rule...

> > I'll fix the volatile things separately.

The access is really over RAM, not MMIO.  So it should be OK to access
in that way.  But volatile seems superfluous.  I'll get rid of it.

Also, msleep() in the removal should be synchronize_irq().

I'll fix these changes and commit with your space fixes to ALSA tree.


Thanks,


Takashi