* [PATCH v2] ASoC: simple-card: overwrite cpu_dai->fmt with codec_dai->fmt
@ 2014-03-12 3:02 Nicolin Chen
2014-03-12 9:25 ` Jyri Sarha
2014-03-18 20:18 ` Mark Brown
0 siblings, 2 replies; 5+ messages in thread
From: Nicolin Chen @ 2014-03-12 3:02 UTC (permalink / raw)
To: broonie
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, rob,
lgirdwood, kuninori.morimoto.gx, jsarha, devicetree, Li.Xiubo,
moinejf, linux-doc, linux-kernel, alsa-devel
The current simple-card driver separates the daimft for cpu_dai and codec_dai.
So we might get different values for them (0x4003 and 0x1003 for example):
asoc-simple-card sound-cs42888.12: cpu : 2024000.esai / 4003 / 132000000
asoc-simple-card sound-cs42888.12: codec : cs42888 / 1003 / 24576000
asoc-simple-card sound-cs42888.12: cs42888 <-> 2024000.esai mapping ok
This is not allowed at all as we need to keep the DAIFMT settings identical
for both the ends of the link.
Thus this patch fixes it by overwriting the cpu_dai->fmt with codec_dai->fmt
since we defined the DAIFMT_MASTER basing on CODEC at the first place while
the other bits are same.
Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
---
Changelog
v2:
* Keep the fmt identical for both CPU and CODEC sides.
* Appended warning to binding doc.
.../devicetree/bindings/sound/simple-card.txt | 6 ++++++
sound/soc/generic/simple-card.c | 20 ++++++++++++++------
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index b30c222..881914b1 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -43,6 +43,12 @@ Optional CPU/CODEC subnodes properties:
clock node (= common clock), or "system-clock-frequency"
(if system doens't support common clock)
+Note:
+ * For 'format', 'frame-master', 'bitclock-master', 'bitclock-inversion' and
+ 'frame-inversion', the simple card will use the settings of CODEC for both
+ CPU and CODEC sides as we need to keep the settings identical for both ends
+ of the link.
+
Example:
sound {
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 5dd4769..09591ab 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -157,6 +157,8 @@ static int asoc_simple_card_parse_of(struct device_node *node,
struct device *dev)
{
struct snd_soc_dai_link *dai_link = priv->snd_card.dai_link;
+ struct asoc_simple_dai *codec_dai = &priv->codec_dai;
+ struct asoc_simple_dai *cpu_dai = &priv->cpu_dai;
struct device_node *np;
char *name;
int ret;
@@ -189,7 +191,7 @@ static int asoc_simple_card_parse_of(struct device_node *node,
np = of_get_child_by_name(node, "simple-audio-card,cpu");
if (np)
ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
- &priv->cpu_dai,
+ cpu_dai,
&dai_link->cpu_of_node,
&dai_link->cpu_dai_name);
if (ret < 0)
@@ -200,12 +202,18 @@ static int asoc_simple_card_parse_of(struct device_node *node,
np = of_get_child_by_name(node, "simple-audio-card,codec");
if (np)
ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
- &priv->codec_dai,
+ codec_dai,
&dai_link->codec_of_node,
&dai_link->codec_dai_name);
if (ret < 0)
return ret;
+ /*
+ * overwrite cpu_dai->fmt as its DAIFMT_MASTER bit is based on CODEC
+ * while the other bits should be identical unless buggy SW/HW design.
+ */
+ cpu_dai->fmt = codec_dai->fmt;
+
if (!dai_link->cpu_dai_name || !dai_link->codec_dai_name)
return -EINVAL;
@@ -227,12 +235,12 @@ static int asoc_simple_card_parse_of(struct device_node *node,
dev_dbg(dev, "platform : %04x\n", priv->daifmt);
dev_dbg(dev, "cpu : %s / %04x / %d\n",
dai_link->cpu_dai_name,
- priv->cpu_dai.fmt,
- priv->cpu_dai.sysclk);
+ cpu_dai->fmt,
+ cpu_dai->sysclk);
dev_dbg(dev, "codec : %s / %04x / %d\n",
dai_link->codec_dai_name,
- priv->codec_dai.fmt,
- priv->codec_dai.sysclk);
+ codec_dai->fmt,
+ codec_dai->sysclk);
/*
* soc_bind_dai_link() will check cpu name
--
1.8.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] ASoC: simple-card: overwrite cpu_dai->fmt with codec_dai->fmt
2014-03-12 3:02 [PATCH v2] ASoC: simple-card: overwrite cpu_dai->fmt with codec_dai->fmt Nicolin Chen
@ 2014-03-12 9:25 ` Jyri Sarha
[not found] ` <5320280C.6080004-l0cyMroinI0@public.gmane.org>
2014-03-18 20:18 ` Mark Brown
1 sibling, 1 reply; 5+ messages in thread
From: Jyri Sarha @ 2014-03-12 9:25 UTC (permalink / raw)
To: Nicolin Chen, broonie
Cc: mark.rutland, devicetree, alsa-devel, kuninori.morimoto.gx,
pawel.moll, ijc+devicetree, Li.Xiubo, linux-doc, lgirdwood,
linux-kernel, robh+dt, rob, galak, moinejf
On 03/12/2014 05:02 AM, Nicolin Chen wrote:
> The current simple-card driver separates the daimft for cpu_dai and codec_dai.
> So we might get different values for them (0x4003 and 0x1003 for example):
>
> asoc-simple-card sound-cs42888.12: cpu : 2024000.esai / 4003 / 132000000
> asoc-simple-card sound-cs42888.12: codec : cs42888 / 1003 / 24576000
> asoc-simple-card sound-cs42888.12: cs42888 <-> 2024000.esai mapping ok
>
> This is not allowed at all as we need to keep the DAIFMT settings identical
> for both the ends of the link.
>
> Thus this patch fixes it by overwriting the cpu_dai->fmt with codec_dai->fmt
> since we defined the DAIFMT_MASTER basing on CODEC at the first place while
> the other bits are same.
>
> Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
> ---
Hi Nicolin,
This patch is an improvement, but in my opinion the binding is still a
bit confusing.
How about changing 'frame-master' and 'bitclock-master' to
'codec-frame-master' and 'codec-bitclock-master'. We could possibly keep
the old names as aliases until all the .dts files out there have been fixed.
At the same go we could add SND_SOC_DAIFMT_MASTER_MASK here:
> /* get CPU/CODEC common format via simple-audio-card,format */
> priv->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,") &
> (SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);
or leave the masking out all together. Can't see why
SND_SOC_DAIFMT_CONT/GATED could not be defined at dai-link level too.
This way the norm would be defining the daifmt at link level. We could
still keep the possibility to overwrite the setting at dai level if
there is need for that.
Best regards,
Jyri
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] ASoC: simple-card: overwrite cpu_dai->fmt with codec_dai->fmt
2014-03-12 3:02 [PATCH v2] ASoC: simple-card: overwrite cpu_dai->fmt with codec_dai->fmt Nicolin Chen
2014-03-12 9:25 ` Jyri Sarha
@ 2014-03-18 20:18 ` Mark Brown
1 sibling, 0 replies; 5+ messages in thread
From: Mark Brown @ 2014-03-18 20:18 UTC (permalink / raw)
To: Nicolin Chen
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, rob,
lgirdwood, kuninori.morimoto.gx, jsarha, devicetree, Li.Xiubo,
moinejf, linux-doc, linux-kernel, alsa-devel
[-- Attachment #1: Type: text/plain, Size: 464 bytes --]
On Wed, Mar 12, 2014 at 11:02:11AM +0800, Nicolin Chen wrote:
> The current simple-card driver separates the daimft for cpu_dai and codec_dai.
> So we might get different values for them (0x4003 and 0x1003 for example):
Since I really want to see some sort of fix in v3.15 that allows
sensible looking DTs to be written I've gone ahead and applied this.
I'd expect we'll change it again but this at least lets us only set the
master flag on one end of the link.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-03-18 20:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-12 3:02 [PATCH v2] ASoC: simple-card: overwrite cpu_dai->fmt with codec_dai->fmt Nicolin Chen
2014-03-12 9:25 ` Jyri Sarha
[not found] ` <5320280C.6080004-l0cyMroinI0@public.gmane.org>
2014-03-12 9:33 ` [alsa-devel] " Nicolin Chen
2014-03-13 21:20 ` Mark Brown
2014-03-18 20:18 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).