[alsa-devel] [RFC PATCH] Add combined clock support to Atmel SoC audio

Mark Brown broonie at opensource.wolfsonmicro.com
Wed Nov 24 00:29:44 CET 2010

On Wed, Nov 24, 2010 at 11:05:11AM +1300, Ryan Mallon wrote:

> This patch is posted as RFC since the approach is incomplete and a bit
> hackish. I am mostly interested in knowing if this is a sensible
> approach, and could be cleaned up for mainline inclusion, or if there is
> a better way to do this.

This doesn't look obviously hideous.

It should set the symmetric_rates flag when going into combined clocks
mode.  A few other comments:

> +			if (atomic_dec_and_test(&ssc_p->substreams_running))
> +				ssc_writel(ssc_p->ssc->regs, CR,
> +					   dma_params->mask->ssc_disable);

It'd seem clearer to use a regular lock here; probably safer also as you
could get a race between the test and the write which tries to revert
the change - ie a:


type pattern.

> +	if (ssc_p->combined_clock == ATMEL_SSC_CLOCK_RX_ON_TX) {

How about just calling these defines ATMEL_SSC_CLOCK_TX or _RX?  You're
identifying the clock line to use for both cases, it's probably clearer
to say "use the RX lines" or "use the TX" lines than say "do one on the
other" if you see what I mean.

> +		/* RX clock is sourced from TK pin */
> +		rcmr &= ~SSC_BF(RCMR_CKS, 0x7);
> +		rcmr |=  SSC_BF(RCMR_CKS, SSC_CKS_CLOCK);

Is this done by the driver normally, or is it done by the machine
normally?  If it's normally done by the machine perhaps it should be
moved into the driver in all cases.

> +		if (dir == 1) {
> +			/*
> +			 * Set the TX clock period to the RX clock period
> +			 * FIXME - Is this okay if we are already doing TX?
> +			 */
> +			tcmr &= 0x00ffffff;
> +			tcmr |= rcmr & 0xff000000;

Should probably enforce a constraint to stop users doing something that
forces the change?

More information about the Alsa-devel mailing list