Re: [PATCH v3] ASoC: Add Freescale SGTL5000 codec support

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Thu, Feb 17, 2011 at 02:53:52AM +0800, Zeng Zhaoming wrote:
> On Wed 2011-02-16 19:53:14, Mark Brown wrote:
> > On Wed, Feb 16, 2011 at 06:56:16AM +0800, zhaoming.zeng@freescale.com wrote:

> > > +	l = (reg & SGTL5000_DAC_VOL_LEFT_MASK) >> SGTL5000_DAC_VOL_LEFT_SHIFT;
> > > +	r = (reg & SGTL5000_DAC_VOL_RIGHT_MASK) >> SGTL5000_DAC_VOL_RIGHT_SHIFT;
> > > +	l = l < 0x3c ? 0x3c : l;
> > > +	l = l > 0xfc ? 0xfc : l;
> > > +	r = r < 0x3c ? 0x3c : r;
> > > +	r = r > 0xfc ? 0xfc : r;

> > My previous comments about the lebility of this still stand.

> Sorry, I don't catch you means quite well, you means more comments to make it clearer?

It needs to be clearer and I'm pretty certain that comments aren't
enough to deal with the issue - the stack of ternery operators and magic
numbers just isn't good for comprehensibility.

> > Since there's no macro arguments this could just be defined directly in
> > line.

> you means add it to kcontrol array directly instead of define a macro for it?

Yes.