Re: [PATCH] ASoC - Add support for upto 16 channels on OMAP MCBSP

[Liam Girdwood <lrg@slimlogic.co.uk>]

On Thu, 2009-11-05 at 10:26 +0200, Peter Ujfalusi wrote:
> On Wednesday 04 November 2009 19:53:55 ext Liam Girdwood wrote:
> > From: Graeme Gregory <gg@slimlogic.co.uk>
> > 
> > This patch increases the number of supported audio channels from 4
> > to 16 and was sponsored by Shotspotter inc.
> > 
> > Signed-off-by: Graeme Gregory <gg@slimlogic.co.uk>
> > Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
> > ---
> >  sound/soc/omap/omap-mcbsp.c |   71
> >  +++++++++++++++++++++++++++++------------- 1 files changed, 49
> >  insertions(+), 22 deletions(-)
> > 

snip

> 
> I would have done this a bit differently:
> 
> ...
> 	/* Check if the number of channels are valid */
> 	if (channels < 1 || channels > 16) {
> 		/* Unsupported number of channels */
> 		/* Probably would be good to have some error message about it? */
> 		return -EINVAL;
> 	}
> 

Not really required as soc-core takes care of this for us :)


> 	format = mcbsp_data->fmt & SND_SOC_DAIFMT_FORMAT_MASK;
> 	wpf = channels = params_channels(params);
> 
> 	if (channels == 2 && format == SND_SOC_DAIFMT_I2S) {
> 		/* Use dual-phase frames */
> 		regs->rcr2      |= RPHASE;
> 		regs->xcr2      |= XPHASE;
> 		/* Set 1 word per (McBSP) frame for phase1 and phase2 */
> 		wpf--;
> 		regs->rcr2      |= RFRLEN2(wpf - 1);
> 		regs->xcr2      |= XFRLEN2(wpf - 1);
> 	}
> 	/* Set word per (McBSP) frame for phase1 */
> 	regs->rcr1      |= RFRLEN1(wpf - 1);
> 	regs->xcr1      |= XFRLEN1(wpf - 1);
> 

I prefer this too.

Liam