[alsa-devel] [PATCH] ASoC: OMAP: mcbsp: ensure that CLKX and CLKR are not used as ouput pins when they are used as input clock for the SRG.

[Thomas Niederprüm]

Am Freitag, den 16.01.2015, 14:42 +0200 schrieb Peter Ujfalusi:
> On 01/16/2015 12:15 PM, Thomas Niederprüm wrote:
> > Am Freitag, den 16.01.2015, 11:15 +0200 schrieb Peter Ujfalusi:
> >> On 01/16/2015 12:28 AM, Thomas Niederprüm wrote:
> >>> This patch fixes faulty behaviour in a setup where the input clock for
> >>> the SRG is fed through the CLKR pin but the McBSP is configured to be
> >>> master (SND_SOC_DAIFMT_CBS_CFS). In that case of course CLKR must not be
> >>> configured as output pin. Otherwise the input clock is messed up
> >>> horribly. The same reasoning applies if CLKX is configured as input for
> >>> the SRG.
> >>
> >> If CLKX/CLKR is used as input clock for McBSP then the CBS_CFS is not valid.
> >> In this case you need to use CBM_CFS.
> >>
> > The setup I am using is the following: McBSP is driving an external i2s
> > DAC as master by supplying CLKX and FSX and DX. The DAC only supports
> > i2s slave mode. For synchronization the DAC and the McBSP should share
> > the same master/reference clock (CLKM). Since I don't need the receive
> > path of the McBSP anyway for the DAC I can use the CLKR pin to insert
> > the CLKM as input to the SRG. I think in this scenario CBS_CFS is valid
> > since the McBSP acts as master (for the transmit path)
> 
> Unfortunately the omap-mcbsp driver only supports synchronous configuration
> for tx/rx (since almost all McBSP instance can only be used this way). The
> first stream will configure both tx and rx to have the same properties. Even
> if you are using McBSP1 which has separate FSX and FSR lines, the driver does
> not have support for this.
> From HW point of view this configuration is valid (not something I would
> expect in any product). I don't think there are or will be other designs than
> your using this mode... But, if you add some comment around the masking of
> CLKXM and CLKRM bits that would be great.

Do you mean adding it to the commit message or to the code? or both?

> Just shortly why and also note that the set_dai_sysclk() _need_ to be called
> after set_dai_fmt() to get the configuration you expect to see.
> 
> > It might be more common to use the CKLS pin to inject the reference
> > clock, but the beagleboard-xm I am using already hard wired this to the
> > built-in twl4030 codec which makes it unusable.
> 
> If you don't use the twl4030 the 256FS line should be floating, we are using
> CBM_CFM for the twl4030 card which means that the 256FS is not enabled -> the
> line is floating.
> It should be safe to use the CLKS, but it is not rooted to any pin :(
> Or remove the resistor form the line and connect your sync clock instead.

I considered that for a moment as well, but it seemed hack-ish to me. I
have to live with whatever can be muxed to the expansion headers of the
beagle-xm and with that boundary condition using the CLKR seemed to be
the most elegant solution. Also I gain the benefit that the twl4030
remains usable. 

> 
> >>>
> >>> Signed-off-by: Thomas Niederprüm <niederp at physik.uni-kl.de>
> >>> ---
> >>>  sound/soc/omap/omap-mcbsp.c | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
> >>> index bd3ef2a..c89f562 100644
> >>> --- a/sound/soc/omap/omap-mcbsp.c
> >>> +++ b/sound/soc/omap/omap-mcbsp.c
> >>> @@ -530,8 +530,12 @@ static int omap_mcbsp_dai_set_dai_sysclk(struct snd_soc_dai *cpu_dai,
> >>>  
> >>>  	case OMAP_MCBSP_SYSCLK_CLKX_EXT:
> >>>  		regs->srgr2	|= CLKSM;
> >>> +		regs->pcr0	|= SCLKME;
> >>> +		regs->pcr0	&= ~CLKXM;
> >>> +		break;
> >>>  	case OMAP_MCBSP_SYSCLK_CLKR_EXT:
> >>>  		regs->pcr0	|= SCLKME;
> >>> +		regs->pcr0	&= ~CLKRM;
> >>>  		break;
> >>>  	default:
> >>>  		err = -ENODEV;
> >>>
> >>
> >>
> > 
> 
> 

-- 
Thomas Niederprüm
TU Kaiserslautern
FB Physik (AG Ott)
Erwin-Schrödinger-Str. 46/431
67663 Kaiserslautern
Tel.: 0631 205 2387
Fax: 0631 205 3906