[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87egvt7dmd.wl%kuninori.morimoto.gx@gmail.com>
Date: Tue, 02 Sep 2014 17:26:53 -0700 (PDT)
From: Kuninori Morimoto <kuninori.morimoto.gx@...il.com>
To: Xiubo Li <Li.Xiubo@...escale.com>
Cc: <broonie@...nel.org>, <perex@...ex.cz>, <lgirdwood@...il.com>,
<tiwai@...e.de>, <moinejf@...e.fr>, <andrew@...n.ch>,
<kuninori.morimoto.gx@...esas.com>, <jsarha@...com>,
<devicetree@...r.kernel.org>, <alsa-devel@...a-project.org>,
<robh+dt@...nel.org>, <pawel.moll@....com>, <mark.rutland@....com>,
<ijc+devicetree@...lion.org.uk>, <galak@...eaurora.org>,
linux-kernel@...r.kernel.org
Subject: Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Hi Xiubo
I was very surprised about this patch
because the idea is same as my local patch
(I was planned to send it to ML :)
I attached my local patch to sharing idea.
> +static inline unsigned int
> +asoc_simple_card_fmt_master(struct device_node *np,
> + struct device_node *bitclkmaster,
> + struct device_node *framemaster)
> +{
> + switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
> + case 0x11:
> + return SND_SOC_DAIFMT_CBS_CFS;
> + case 0x10:
> + return SND_SOC_DAIFMT_CBS_CFM;
> + case 0x01:
> + return SND_SOC_DAIFMT_CBM_CFS;
> + default:
> + return SND_SOC_DAIFMT_CBM_CFM;
> + }
> +
> + /* Shouldn't be here */
> + return -EINVAL;
> +}
I think this concept is nice,
but setting all fmt in this function is good for me
see my local patch
----------
>From 85562eb1587e5c184e4f4e0b183bd7063aaa81b7 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
Date: Thu, 28 Aug 2014 19:20:14 +0900
Subject: [PATCH] ASoC: simple-card: add asoc_simple_card_parse_daifmt()
Current daifmt setting method in simple-card driver is
placed to many places, and using un-readable/confusable method.
This patch adds new asoc_simple_card_parse_daifmt()
and tidyup code.
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index bea5901..c932103 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -167,6 +167,64 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static int asoc_simple_card_parse_daifmt(struct device_node *node,
+ struct simple_card_data *priv,
+ struct device_node *cpu,
+ struct device_node *codec,
+ char *prefix, int idx)
+{
+ struct device *dev = simple_priv_to_dev(priv);
+ struct device_node *bitclkmaster = NULL;
+ struct device_node *framemaster = NULL;
+ struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
+ struct asoc_simple_dai *cpu_dai = &dai_props->cpu_dai;
+ struct asoc_simple_dai *codec_dai = &dai_props->codec_dai;
+ unsigned int daifmt;
+
+ daifmt = snd_soc_of_parse_daifmt(node, prefix,
+ &bitclkmaster, &framemaster);
+ daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
+
+ if (strlen(prefix) && !bitclkmaster && !framemaster) {
+ /*
+ * No dai-link level and master setting was not found from
+ * sound node level, revert back to legacy DT parsing and
+ * take the settings from codec node.
+ */
+ dev_dbg(dev, "Revert to legacy daifmt parsing\n");
+
+ cpu_dai->fmt = codec_dai->fmt =
+ snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
+ (daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK);
+ } else {
+
+ switch (((codec == bitclkmaster) << 4) | (codec == framemaster)) {
+ case 0x11:
+ daifmt |= SND_SOC_DAIFMT_CBM_CFM;
+ break;
+ case 0x10:
+ daifmt |= SND_SOC_DAIFMT_CBM_CFS;
+ break;
+ case 0x01:
+ daifmt |= SND_SOC_DAIFMT_CBS_CFM;
+ break;
+ default:
+ daifmt |= SND_SOC_DAIFMT_CBS_CFS;
+ break;
+ }
+
+ cpu_dai->fmt = daifmt;
+ codec_dai->fmt = daifmt;
+ }
+
+ if (bitclkmaster)
+ of_node_put(bitclkmaster);
+ if (framemaster)
+ of_node_put(framemaster);
+
+ return 0;
+}
+
static int asoc_simple_card_dai_link_of(struct device_node *node,
struct simple_card_data *priv,
int idx,
@@ -175,10 +233,8 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
struct device *dev = simple_priv_to_dev(priv);
struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, idx);
struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
- struct device_node *np = NULL;
- struct device_node *bitclkmaster = NULL;
- struct device_node *framemaster = NULL;
- unsigned int daifmt;
+ struct device_node *cpu = NULL;
+ struct device_node *codec = NULL;
char *name;
char prop[128];
char *prefix = "";
@@ -187,82 +243,35 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
if (is_top_level_node)
prefix = "simple-audio-card,";
- daifmt = snd_soc_of_parse_daifmt(node, prefix,
- &bitclkmaster, &framemaster);
- daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
-
snprintf(prop, sizeof(prop), "%scpu", prefix);
- np = of_get_child_by_name(node, prop);
- if (!np) {
+ cpu = of_get_child_by_name(node, prop);
+
+ snprintf(prop, sizeof(prop), "%scodec", prefix);
+ codec = of_get_child_by_name(node, prop);
+
+ if (!cpu || !codec) {
ret = -EINVAL;
dev_err(dev, "%s: Can't find %s DT node\n", __func__, prop);
goto dai_link_of_err;
}
- ret = asoc_simple_card_sub_parse_of(np, &dai_props->cpu_dai,
- &dai_link->cpu_of_node,
- &dai_link->cpu_dai_name);
+ ret = asoc_simple_card_parse_daifmt(node, priv,
+ cpu, codec, prefix, idx);
if (ret < 0)
goto dai_link_of_err;
- dai_props->cpu_dai.fmt = daifmt;
- switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
- case 0x11:
- dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBS_CFS;
- break;
- case 0x10:
- dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBS_CFM;
- break;
- case 0x01:
- dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBM_CFS;
- break;
- default:
- dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBM_CFM;
- break;
- }
-
- of_node_put(np);
- snprintf(prop, sizeof(prop), "%scodec", prefix);
- np = of_get_child_by_name(node, prop);
- if (!np) {
- ret = -EINVAL;
- dev_err(dev, "%s: Can't find %s DT node\n", __func__, prop);
+ ret = asoc_simple_card_sub_parse_of(cpu, &dai_props->cpu_dai,
+ &dai_link->cpu_of_node,
+ &dai_link->cpu_dai_name);
+ if (ret < 0)
goto dai_link_of_err;
- }
- ret = asoc_simple_card_sub_parse_of(np, &dai_props->codec_dai,
+ ret = asoc_simple_card_sub_parse_of(codec, &dai_props->codec_dai,
&dai_link->codec_of_node,
&dai_link->codec_dai_name);
if (ret < 0)
goto dai_link_of_err;
- if (strlen(prefix) && !bitclkmaster && !framemaster) {
- /* No dai-link level and master setting was not found from
- sound node level, revert back to legacy DT parsing and
- take the settings from codec node. */
- dev_dbg(dev, "%s: Revert to legacy daifmt parsing\n",
- __func__);
- dai_props->cpu_dai.fmt = dai_props->codec_dai.fmt =
- snd_soc_of_parse_daifmt(np, NULL, NULL, NULL) |
- (daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK);
- } else {
- dai_props->codec_dai.fmt = daifmt;
- switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
- case 0x11:
- dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBM_CFM;
- break;
- case 0x10:
- dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBM_CFS;
- break;
- case 0x01:
- dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBS_CFM;
- break;
- default:
- dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBS_CFS;
- break;
- }
- }
-
if (!dai_link->cpu_dai_name || !dai_link->codec_dai_name) {
ret = -EINVAL;
goto dai_link_of_err;
@@ -304,12 +313,10 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
dai_link->cpu_dai_name = NULL;
dai_link_of_err:
- if (np)
- of_node_put(np);
- if (bitclkmaster)
- of_node_put(bitclkmaster);
- if (framemaster)
- of_node_put(framemaster);
+ if (cpu)
+ of_node_put(cpu);
+ if (codec)
+ of_node_put(codec);
return ret;
}
---------
Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists