Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.

Hi Kuninori-san,

Yes, I think it make sense to set all fmt in one function, and will Be more readable.

I agree with you, could you please just wait, because there has many Replications and good Ideas about this patch, and I will revise it. Then you can improve it as your patch blow.

Thanks,

BRs Xiubo

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@renesas.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@renesas.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