[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.
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;
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?
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;regs->pcr0 &= ~CLKRM;
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;regs->pcr0 &= ~CLKRM;
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;regs->pcr0 &= ~CLKRM;
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;regs->pcr0 &= ~CLKRM;
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 ;)
participants (3)
-
Jarkko Nikula -
Peter Ujfalusi -
Thomas Niederprüm