[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.

[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]

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.

Signed-off-by: Thomas Niederprüm <niederp@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;
-- 
2.1.1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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.

[Jarkko Nikula]

Hi

On Thu, Jan 15, 2015 at 11:28:21PM +0100, 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.
> 
> Signed-off-by: Thomas Niederprüm <niederp@physik.uni-kl.de>
> ---
>  sound/soc/omap/omap-mcbsp.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
I cannot check at the moment but is this actually a contradictory
configuration if McBSP is set to bit clock master but at the same want
to use it as an input also? Should you use SND_SOC_DAIFMT_CBM_CFS
instead?

Peter: care to check?

-- 
Jarkko

Re: [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.

[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.

> 
> Signed-off-by: Thomas Niederprüm <niederp@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;
> 


-- 
Péter

Re: [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, 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)

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.

> > 
> > Signed-off-by: Thomas Niederprüm <niederp@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

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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.

[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.
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.

>>>
>>> Signed-off-by: Thomas Niederprüm <niederp@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;
>>>
>>
>>
> 


-- 
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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@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

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [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.

[Peter Ujfalusi]

On 01/16/2015 05:06 PM, Thomas Niederprüm wrote:
>> 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?

Since this is something that the driver has not meant to support officially, I
prefer both. I just hope that HW designers will not get ideas based on this ;)

-- 
Péter
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel