2014-01-23 06:01:54

by Xiubo Li

Subject: [PATCH 0/8] New and Resend the old patches

New and Resend the old patches basing the newest code version.

And this patch series has been tested based the VF610 Tower board.

2014-01-23 06:01:56

by Xiubo Li

Subject: [PATCH 1/8] ASoC: fsl: Add VF610 soc audio card Kconfig

Signed-off-by: Xiubo Li <[email protected]>
---
sound/soc/fsl/Kconfig | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 324988d..bc1fa9c 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -219,3 +219,27 @@ config SND_SOC_IMX_MC13783
select SND_SOC_IMX_PCM_DMA

endif # SND_IMX_SOC
+
+menuconfig SND_VF610_SOC
+ tristate "SoC Audio for Freescale VF610 CPUs"
+ select DMA_ENGINE
+ help
+ Say Y or M if you want to add support for codecs attached to
+ the VF610 CPUs.
+
+ This will enable Freeacale SAI and SGTL5000 codec, and an extra
+ TWR-AUDIO-SGTL sub-board is needed for SGTL5000.
+
+if SND_VF610_SOC
+
+config SND_SOC_VF610_SGTL5000
+ tristate "SoC Audio support for VF610 boards with SGTL5000"
+ depends on OF && I2C
+ select SND_SOC_FSL_SAI
+ select SND_SOC_SGTL5000
+ select SND_SIMPLE_CARD
+ help
+ Say Y if you want to add support for SoC audio on an VF610 board with
+ a SGTL5000 codec and a SAI.
+
+endif #SND_VF610_SOC
--
1.8.4

2014-01-23 06:02:00

by Xiubo Li

Subject: [PATCH Resend 3/8] ASoC: simple-card: simplify the daifmt code

In the asoc_simple_card_parse_of() will parse the device node's CPU/CODEC
DAI commone fmts, and then in asoc_simple_card_sub_parse_of() will parse
the CPU/CODEC DAI's sub-node fmts, so we can combine the info->daifmt and
info->set.fmt in asoc_simple_card_sub_parse_of() not while just before
_set_fmt().

And this will be more easy to add new functions, such as supporting
_set_tdm_slot(), etc.

Signed-off-by: Xiubo Li <[email protected]>
---
sound/soc/generic/simple-card.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 3b8c9a2..f38e56e 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -24,15 +24,12 @@ struct simple_card_data {
};

static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
- struct asoc_simple_dai *set,
- unsigned int daifmt)
+ struct asoc_simple_dai *set)
{
int ret = 0;

- daifmt |= set->fmt;
-
- if (daifmt) {
- ret = snd_soc_dai_set_fmt(dai, daifmt);
+ if (set->fmt) {
+ ret = snd_soc_dai_set_fmt(dai, set->fmt);
if (ret && ret != -ENOTSUPP) {
dev_err(dai->dev, "simple-card: set_fmt error\n");
return ret;
@@ -56,14 +53,13 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
snd_soc_card_get_drvdata(rtd->card);
struct snd_soc_dai *codec = rtd->codec_dai;
struct snd_soc_dai *cpu = rtd->cpu_dai;
- unsigned int daifmt = priv->daifmt;
int ret;

- ret = __asoc_simple_card_dai_init(codec, &priv->codec_dai, daifmt);
+ ret = __asoc_simple_card_dai_init(codec, &priv->codec_dai);
if (ret < 0)
return ret;

- ret = __asoc_simple_card_dai_init(cpu, &priv->cpu_dai, daifmt);
+ ret = __asoc_simple_card_dai_init(cpu, &priv->cpu_dai);
if (ret < 0)
return ret;

@@ -72,6 +68,7 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)

static int
asoc_simple_card_sub_parse_of(struct device_node *np,
+ unsigned int daifmt,
struct asoc_simple_dai *dai,
const struct device_node **p_node,
const char **name)
@@ -100,6 +97,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
* and specific "format" if it has
*/
dai->fmt = snd_soc_of_parse_daifmt(np, NULL);
+ dai->fmt |= daifmt;

/*
* dai->sysclk come from
@@ -158,7 +156,7 @@ static int asoc_simple_card_parse_of(struct device_node *node,
ret = -EINVAL;
np = of_get_child_by_name(node, "simple-audio-card,cpu");
if (np)
- ret = asoc_simple_card_sub_parse_of(np,
+ ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
&priv->cpu_dai,
&dai_link->cpu_of_node,
&dai_link->cpu_dai_name);
@@ -169,7 +167,7 @@ static int asoc_simple_card_parse_of(struct device_node *node,
ret = -EINVAL;
np = of_get_child_by_name(node, "simple-audio-card,codec");
if (np)
- ret = asoc_simple_card_sub_parse_of(np,
+ ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
&priv->codec_dai,
&dai_link->codec_of_node,
&dai_link->codec_dai_name);
--
1.8.4

2014-01-23 06:02:06

by Xiubo Li

Subject: [PATCH Resend 4/8] ASoC: simple-card: Add snd_card's name parsing from DT node support

If the DT is used and the CPU DAI device has only one DAI, the card
name will be like :

ALSA device list:
0: 40031000.sai-sgtl5000

And this name maybe a little ugly to some customers, so here the
card name parsing from DT node is supported.

Signed-off-by: Xiubo Li <[email protected]>
---
sound/soc/generic/simple-card.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index f38e56e..546b93d 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -140,6 +140,9 @@ static int asoc_simple_card_parse_of(struct device_node *node,
char *name;
int ret;

+ /* parsing the card name from DT */
+ snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
+
/* 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);
@@ -184,7 +187,8 @@ static int asoc_simple_card_parse_of(struct device_node *node,
GFP_KERNEL);
sprintf(name, "%s-%s", dai_link->cpu_dai_name,
dai_link->codec_dai_name);
- priv->snd_card.name = name;
+ if (!priv->snd_card.name)
+ priv->snd_card.name = name;
dai_link->name = dai_link->stream_name = name;

/* simple-card assumes platform == cpu */
--
1.8.4

2014-01-23 06:02:22

by Xiubo Li

Subject: [PATCH 6/8] ASoC: simple-card: add tdm slot supports

For some CPU/CODEC DAI devices the tdm slot maybe needed. This patch
adds the tdm slot supporting for simple-card driver.

The style of the tdm slot in DT:

For instance:

simple-tdm-slot = <0xffffffc 0xffffffc 2 0>;

Signed-off-by: Xiubo Li <[email protected]>
---

The VF610 Tower and VF610 LS1 platforms' SPDIF will depend on this patch.


include/sound/simple_card.h | 8 ++++++
sound/soc/generic/simple-card.c | 61 +++++++++++++++++++++++++++++++++++++----
2 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/include/sound/simple_card.h b/include/sound/simple_card.h
index e1ac996..cfc5b66 100644
--- a/include/sound/simple_card.h
+++ b/include/sound/simple_card.h
@@ -14,10 +14,18 @@

#include <sound/soc.h>

+struct asoc_simple_tdm_slot {
+ unsigned int tx_mask;
+ unsigned int rx_mask;
+ int slots;
+ int slot_width;
+};
+
struct asoc_simple_dai {
const char *name;
unsigned int fmt;
unsigned int sysclk;
+ struct asoc_simple_tdm_slot *tdm;
};

struct asoc_simple_card_info {
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 546b93d..d067e0a 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -32,7 +32,7 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
ret = snd_soc_dai_set_fmt(dai, set->fmt);
if (ret && ret != -ENOTSUPP) {
dev_err(dai->dev, "simple-card: set_fmt error\n");
- return ret;
+ goto err;
}
}

@@ -40,10 +40,22 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
ret = snd_soc_dai_set_sysclk(dai, 0, set->sysclk, 0);
if (ret && ret != -ENOTSUPP) {
dev_err(dai->dev, "simple-card: set_sysclk error\n");
- return ret;
+ goto err;
+ }
+ }
+
+ if (set->tdm) {
+ ret = snd_soc_dai_set_tdm_slot(dai, set->tdm->tx_mask,
+ set->tdm->rx_mask,
+ set->tdm->slots,
+ set->tdm->slot_width);
+ if (ret && ret != -ENOTSUPP) {
+ dev_err(dai->dev, "simple-card: set_tdm_slot error\n");
+ goto err;
}
}

+err:
return ret;
}

@@ -67,11 +79,43 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
}

static int
+asoc_simple_card_of_parse_tdm_slot(struct device_node *np,
+ struct device *dev,
+ struct asoc_simple_dai *dai,
+ const char *propname)
+{
+ struct asoc_simple_tdm_slot *tdm;
+ u32 out_value[4];
+ int ret;
+
+ if (!of_property_read_bool(np, propname))
+ return 0;
+
+ tdm = devm_kzalloc(dev, sizeof(*tdm), GFP_KERNEL);
+ if (!tdm)
+ return -ENOMEM;
+
+ ret = of_property_read_u32_array(np, propname, out_value, 4);
+ if (ret)
+ return ret;
+
+ tdm->tx_mask = out_value[0];
+ tdm->rx_mask = out_value[1];
+ tdm->slots = out_value[2];
+ tdm->slot_width = out_value[3];
+
+ dai->tdm = tdm;
+
+ return 0;
+}
+
+static int
asoc_simple_card_sub_parse_of(struct device_node *np,
unsigned int daifmt,
struct asoc_simple_dai *dai,
const struct device_node **p_node,
- const char **name)
+ const char **name,
+ struct device *dev)
{
struct device_node *node;
struct clk *clk;
@@ -91,6 +135,11 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
if (ret < 0)
goto parse_error;

+ /* parse tdm_slot */
+ ret = asoc_simple_card_of_parse_tdm_slot(np, dev, dai, "tdm-slot");
+ if (ret < 0)
+ goto parse_error;
+
/*
* bitclock-inversion, frame-inversion
* bitclock-master, frame-master
@@ -162,7 +211,8 @@ static int asoc_simple_card_parse_of(struct device_node *node,
ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
&priv->cpu_dai,
&dai_link->cpu_of_node,
- &dai_link->cpu_dai_name);
+ &dai_link->cpu_dai_name,
+ dev);
if (ret < 0)
return ret;

@@ -173,7 +223,8 @@ static int asoc_simple_card_parse_of(struct device_node *node,
ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
&priv->codec_dai,
&dai_link->codec_of_node,
- &dai_link->codec_dai_name);
+ &dai_link->codec_dai_name,
+ dev);
if (ret < 0)
return ret;

--
1.8.4

2014-01-23 06:02:31

by Xiubo Li

Subject: [PATCH 8/8] ASoC: simple-card: add off-codec widgets supports.

Signed-off-by: Xiubo Li <[email protected]>
---
sound/soc/generic/simple-card.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index d067e0a..4b7ef4d 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -196,6 +196,14 @@ static int asoc_simple_card_parse_of(struct device_node *node,
priv->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,") &
(SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);

+ /* off-codec widgets */
+ if (of_property_read_bool(node, "simple-audio-card,widgets")) {
+ ret = snd_soc_of_parse_audio_simple_widgets(&priv->snd_card,
+ "simple-audio-card,widgets");
+ if (ret)
+ return ret;
+ }
+
/* DAPM routes */
if (of_property_read_bool(node, "simple-audio-card,routing")) {
ret = snd_soc_of_parse_audio_routing(&priv->snd_card,
--
1.8.4

2014-01-23 06:02:19

by Xiubo Li

Subject: [PATCH Resend 7/8] ASoC: add snd_soc_of_parse_audio_simple_widgets for DeviceTree

This patch adds snd_soc_of_parse_audio_simple_widgets() and supports
below style of widgets name on DT.

"wname-prefix[ individual name]"
"wname-prefix" includes: "Mic", "Line", "Hp", "Spk"...

For instance:
simple-audio-widgets =
"Mic Jack", "Line In Jack",
"Hp Jack", "Spk Ext",
"Line Out Jack";

Signed-off-by: Xiubo Li <[email protected]>
---

Add :
widgets[i].name = wname;

And this will use the widget name from DT node.




include/sound/soc.h | 2 ++
sound/soc/soc-core.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 9a00147..465dc6e 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1173,6 +1173,8 @@ void snd_soc_util_exit(void);

int snd_soc_of_parse_card_name(struct snd_soc_card *card,
const char *propname);
+int snd_soc_of_parse_audio_simple_widgets(struct snd_soc_card *card,
+ const char *propname);
int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
const char *propname);
unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 393ff06..9c8a686 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -4417,6 +4417,69 @@ int snd_soc_of_parse_card_name(struct snd_soc_card *card,
}
EXPORT_SYMBOL_GPL(snd_soc_of_parse_card_name);

+static struct snd_soc_dapm_widget simple_widgets[] = {
+ SND_SOC_DAPM_MIC("Mic", NULL),
+ SND_SOC_DAPM_LINE("Line", NULL),
+ SND_SOC_DAPM_HP("Hp", NULL),
+ SND_SOC_DAPM_SPK("Spk", NULL),
+};
+
+int snd_soc_of_parse_audio_simple_widgets(struct snd_soc_card *card,
+ const char *propname)
+{
+ struct device_node *np = card->dev->of_node;
+ struct snd_soc_dapm_widget *widgets;
+ const char *wname;
+ int i, j, cnt, ret;
+
+ cnt = of_property_count_strings(np, propname);
+ if (cnt <= 0) {
+ dev_err(card->dev,
+ "ASoC: Property '%s' does not exist or "
+ "length is zero\n", propname);
+ return -EINVAL;
+ }
+
+ widgets = devm_kcalloc(card->dev, cnt, sizeof(*widgets), GFP_KERNEL);
+ if (!widgets) {
+ dev_err(card->dev,
+ "ASoC: Could not allocate DAPM widgets table\n");
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < cnt; i++) {
+ ret = of_property_read_string_index(np, propname, i, &wname);
+ if (ret) {
+ dev_err(card->dev,
+ "ASoC: Property '%s' index %d could not be "
+ "read: %d\n", propname, i, ret);
+ return -EINVAL;
+ }
+
+ for (j = 0; j < ARRAY_SIZE(simple_widgets); j++) {
+ if (!strncmp(wname, simple_widgets[j].name,
+ strlen(simple_widgets[j].name))) {
+ widgets[i] = simple_widgets[j];
+ widgets[i].name = wname;
+ break;
+ }
+ }
+
+ if (j >= ARRAY_SIZE(simple_widgets)) {
+ dev_err(card->dev,
+ "ASoC: DAPM widget : '%s' is not supported\n",
+ wname);
+ return -EINVAL;
+ }
+ }
+
+ card->dapm_widgets = widgets;
+ card->num_dapm_widgets = cnt;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_of_parse_audio_simple_widgets);
+
int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
const char *propname)
{
--
1.8.4

2014-01-23 06:01:58

by Xiubo Li

Subject: [PATCH Resend 2/8] ASoC: simple-card: fix __asoc_simple_card_dai_init

If the CPU/CODEC DAI set_sysclk() is not support, the -ENOTSUPP will returnd.
Here do the check like set_fmt().

Signed-off-by: Xiubo Li <[email protected]>
---



The VF610 Tower and VF610 LS1 platforms' ESAI and SPDIF will depend on this
patch too.




sound/soc/generic/simple-card.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 6443c87..3b8c9a2 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -31,16 +31,21 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,

daifmt |= set->fmt;

- if (daifmt)
+ if (daifmt) {
ret = snd_soc_dai_set_fmt(dai, daifmt);
-
- if (ret == -ENOTSUPP) {
- dev_dbg(dai->dev, "ASoC: set_fmt is not supported\n");
- ret = 0;
+ if (ret && ret != -ENOTSUPP) {
+ dev_err(dai->dev, "simple-card: set_fmt error\n");
+ return ret;
+ }
}

- if (!ret && set->sysclk)
+ if (set->sysclk) {
ret = snd_soc_dai_set_sysclk(dai, 0, set->sysclk, 0);
+ if (ret && ret != -ENOTSUPP) {
+ dev_err(dai->dev, "simple-card: set_sysclk error\n");
+ return ret;
+ }
+ }

return ret;
}
--
1.8.4

2014-01-23 06:11:09

by Xiubo Li

Subject: [PATCH 5/8] ASoC: core: set_tdm_slot() will return -ENOTSUPP if no operation provided

Make it easier for generic code to work with set_tdm_slot() by distinguishing
between the operation not being supported and an error as is done.

Signed-off-by: Xiubo Li <[email protected]>
---
sound/soc/soc-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index fe1df50..393ff06 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3626,7 +3626,7 @@ int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
return dai->driver->ops->set_tdm_slot(dai, tx_mask, rx_mask,
slots, slot_width);
else
- return -EINVAL;
+ return -ENOTSUPP;
}
EXPORT_SYMBOL_GPL(snd_soc_dai_set_tdm_slot);

--
1.8.4

2014-01-23 06:11:29

by David Rientjes

Subject: Re: [PATCH 6/8] ASoC: simple-card: add tdm slot supports

On Thu, 23 Jan 2014, Xiubo Li wrote:

> diff --git a/include/sound/simple_card.h b/include/sound/simple_card.h
> index e1ac996..cfc5b66 100644
> --- a/include/sound/simple_card.h
> +++ b/include/sound/simple_card.h
> @@ -14,10 +14,18 @@
>
> #include <sound/soc.h>
>
> +struct asoc_simple_tdm_slot {
> + unsigned int tx_mask;
> + unsigned int rx_mask;
> + int slots;
> + int slot_width;
> +};
> +
> struct asoc_simple_dai {
> const char *name;
> unsigned int fmt;
> unsigned int sysclk;
> + struct asoc_simple_tdm_slot *tdm;
> };
>
> struct asoc_simple_card_info {
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 546b93d..d067e0a 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -32,7 +32,7 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
> ret = snd_soc_dai_set_fmt(dai, set->fmt);
> if (ret && ret != -ENOTSUPP) {
> dev_err(dai->dev, "simple-card: set_fmt error\n");
> - return ret;
> + goto err;
> }
> }
>
> @@ -40,10 +40,22 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
> ret = snd_soc_dai_set_sysclk(dai, 0, set->sysclk, 0);
> if (ret && ret != -ENOTSUPP) {
> dev_err(dai->dev, "simple-card: set_sysclk error\n");
> - return ret;
> + goto err;
> + }
> + }
> +
> + if (set->tdm) {
> + ret = snd_soc_dai_set_tdm_slot(dai, set->tdm->tx_mask,
> + set->tdm->rx_mask,
> + set->tdm->slots,
> + set->tdm->slot_width);
> + if (ret && ret != -ENOTSUPP) {
> + dev_err(dai->dev, "simple-card: set_tdm_slot error\n");
> + goto err;
> }
> }
>
> +err:
> return ret;
> }
>
> @@ -67,11 +79,43 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
> }
>
> static int
> +asoc_simple_card_of_parse_tdm_slot(struct device_node *np,
> + struct device *dev,
> + struct asoc_simple_dai *dai,
> + const char *propname)
> +{
> + struct asoc_simple_tdm_slot *tdm;
> + u32 out_value[4];
> + int ret;
> +
> + if (!of_property_read_bool(np, propname))
> + return 0;
> +
> + tdm = devm_kzalloc(dev, sizeof(*tdm), GFP_KERNEL);
> + if (!tdm)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32_array(np, propname, out_value, 4);
> + if (ret)
> + return ret;

Looks like a memory leak?

> +
> + tdm->tx_mask = out_value[0];
> + tdm->rx_mask = out_value[1];
> + tdm->slots = out_value[2];
> + tdm->slot_width = out_value[3];
> +
> + dai->tdm = tdm;
> +
> + return 0;
> +}
> +
> +static int
> asoc_simple_card_sub_parse_of(struct device_node *np,
> unsigned int daifmt,
> struct asoc_simple_dai *dai,
> const struct device_node **p_node,
> - const char **name)
> + const char **name,
> + struct device *dev)
> {
> struct device_node *node;
> struct clk *clk;
> @@ -91,6 +135,11 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
> if (ret < 0)
> goto parse_error;
>
> + /* parse tdm_slot */
> + ret = asoc_simple_card_of_parse_tdm_slot(np, dev, dai, "tdm-slot");
> + if (ret < 0)
> + goto parse_error;
> +
> /*
> * bitclock-inversion, frame-inversion
> * bitclock-master, frame-master
> @@ -162,7 +211,8 @@ static int asoc_simple_card_parse_of(struct device_node *node,
> ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
> &priv->cpu_dai,
> &dai_link->cpu_of_node,
> - &dai_link->cpu_dai_name);
> + &dai_link->cpu_dai_name,
> + dev);
> if (ret < 0)
> return ret;
>
> @@ -173,7 +223,8 @@ static int asoc_simple_card_parse_of(struct device_node *node,
> ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
> &priv->codec_dai,
> &dai_link->codec_of_node,
> - &dai_link->codec_dai_name);
> + &dai_link->codec_dai_name,
> + dev);
> if (ret < 0)
> return ret;
>

2014-01-23 06:35:41

by Xiubo Li

Subject: RE: [PATCH 6/8] ASoC: simple-card: add tdm slot supports

Hi David,

Firstly thanks for your comment.

> > +asoc_simple_card_of_parse_tdm_slot(struct device_node *np,
> > + struct device *dev,
> > + struct asoc_simple_dai *dai,
> > + const char *propname)
> > +{
> > + struct asoc_simple_tdm_slot *tdm;
> > + u32 out_value[4];
> > + int ret;
> > +
> > + if (!of_property_read_bool(np, propname))
> > + return 0;
> > +
> > + tdm = devm_kzalloc(dev, sizeof(*tdm), GFP_KERNEL);
> > + if (!tdm)
> > + return -ENOMEM;
> > +
> > + ret = of_property_read_u32_array(np, propname, out_value, 4);
> > + if (ret)
> > + return ret;
>
> Looks like a memory leak?
>

Using devm_ and this will be called by _probe().

Please see the "Documentation/driver-model/devres.txt" for detail about
devres.


> > +
> > + tdm->tx_mask = out_value[0];
> > + tdm->rx_mask = out_value[1];
> > + tdm->slots = out_value[2];
> > + tdm->slot_width = out_value[3];
> > +
> > + dai->tdm = tdm;
> > +
> > + return 0;
> > +}
> > +

Thanks,

Best Regards,
Xiubo

2014-01-23 07:12:05

by Jean-Francois Moine

Subject: Re: [PATCH Resend 2/8] ASoC: simple-card: fix __asoc_simple_card_dai_init

On Thu, 23 Jan 2014 13:02:44 +0800
Xiubo Li <[email protected]> wrote:

> If the CPU/CODEC DAI set_sysclk() is not support, the -ENOTSUPP will returnd.
> Here do the check like set_fmt().
>
> Signed-off-by: Xiubo Li <[email protected]>
> ---
>
>
>
> The VF610 Tower and VF610 LS1 platforms' ESAI and SPDIF will depend on this
> patch too.
>
>
>
>
> sound/soc/generic/simple-card.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 6443c87..3b8c9a2 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -31,16 +31,21 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
>
> daifmt |= set->fmt;
>
> - if (daifmt)
> + if (daifmt) {
> ret = snd_soc_dai_set_fmt(dai, daifmt);
> -
> - if (ret == -ENOTSUPP) {
> - dev_dbg(dai->dev, "ASoC: set_fmt is not supported\n");
> - ret = 0;
> + if (ret && ret != -ENOTSUPP) {
> + dev_err(dai->dev, "simple-card: set_fmt error\n");
> + return ret;
> + }
> }
>
> - if (!ret && set->sysclk)
> + if (set->sysclk) {
> ret = snd_soc_dai_set_sysclk(dai, 0, set->sysclk, 0);
> + if (ret && ret != -ENOTSUPP) {
> + dev_err(dai->dev, "simple-card: set_sysclk error\n");
> + return ret;
> + }
> + }
>
> return ret;

Sorry: you must return 0 here

> }

--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/

2014-01-23 07:22:52

by Xiubo Li

Subject: RE: [PATCH Resend 2/8] ASoC: simple-card: fix __asoc_simple_card_dai_init

> > diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-
> card.c
> > index 6443c87..3b8c9a2 100644
> > --- a/sound/soc/generic/simple-card.c
> > +++ b/sound/soc/generic/simple-card.c
> > @@ -31,16 +31,21 @@ static int __asoc_simple_card_dai_init(struct
> snd_soc_dai *dai,
> >
> > daifmt |= set->fmt;
> >
> > - if (daifmt)
> > + if (daifmt) {
> > ret = snd_soc_dai_set_fmt(dai, daifmt);
> > -
> > - if (ret == -ENOTSUPP) {
> > - dev_dbg(dai->dev, "ASoC: set_fmt is not supported\n");
> > - ret = 0;
> > + if (ret && ret != -ENOTSUPP) {
> > + dev_err(dai->dev, "simple-card: set_fmt error\n");
> > + return ret;
> > + }
> > }
> >
> > - if (!ret && set->sysclk)
> > + if (set->sysclk) {
> > ret = snd_soc_dai_set_sysclk(dai, 0, set->sysclk, 0);
> > + if (ret && ret != -ENOTSUPP) {
> > + dev_err(dai->dev, "simple-card: set_sysclk error\n");
> > + return ret;
> > + }
> > + }
> >
> > return ret;
>
> Sorry: you must return 0 here
>

Yes, the ret maybe -ENOTSUPP.

I will revise it.

Thanks,

Best Regards,
Xiubo

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-01-23 18:38:47

by Mark Brown

Subject: Re: [PATCH 1/8] ASoC: fsl: Add VF610 soc audio card Kconfig

On Thu, Jan 23, 2014 at 01:02:43PM +0800, Xiubo Li wrote:

> +menuconfig SND_VF610_SOC
> + tristate "SoC Audio for Freescale VF610 CPUs"
> + select DMA_ENGINE
> + help
> + Say Y or M if you want to add support for codecs attached to
> + the VF610 CPUs.
> +
> + This will enable Freeacale SAI and SGTL5000 codec, and an extra
> + TWR-AUDIO-SGTL sub-board is needed for SGTL5000.

This bit is OK but...

> +config SND_SOC_VF610_SGTL5000
> + tristate "SoC Audio support for VF610 boards with SGTL5000"
> + depends on OF && I2C
> + select SND_SOC_FSL_SAI
> + select SND_SOC_SGTL5000
> + select SND_SIMPLE_CARD

...for simple card since the whole idea is to support any CODEC with the
same driver we should probably just make sure that all the individual
drivers can be enabled in Kconfig, that way we don't have to have
specific Kconfig entries for boards and loose some of the benefit of the
generic card. I sent a patch earlier exposing the OF supporting CODEC
drivers, one for at least some of the Freescale CPU drivers was sent to
the list recently too but there were some review comments.

Attachments:

2014-01-23 19:20:27

by Mark Brown

Subject: Re: [PATCH Resend 3/8] ASoC: simple-card: simplify the daifmt code

On Thu, Jan 23, 2014 at 01:02:45PM +0800, Xiubo Li wrote:
> In the asoc_simple_card_parse_of() will parse the device node's CPU/CODEC
> DAI commone fmts, and then in asoc_simple_card_sub_parse_of() will parse
> the CPU/CODEC DAI's sub-node fmts, so we can combine the info->daifmt and
> info->set.fmt in asoc_simple_card_sub_parse_of() not while just before
> _set_fmt().

This doesn't apply against the current topic/simple, can you please
check and resend?

Attachments:

2014-01-23 19:24:49

by Mark Brown

Subject: Re: [PATCH Resend 4/8] ASoC: simple-card: Add snd_card's name parsing from DT node support

On Thu, Jan 23, 2014 at 01:02:46PM +0800, Xiubo Li wrote:

> + /* parsing the card name from DT */
> + snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
> +

This is OK but new properties need documenting in the binding.

Attachments:

2014-01-23 19:30:00

by Mark Brown

Subject: Re: [PATCH 5/8] ASoC: core: set_tdm_slot() will return -ENOTSUPP if no operation provided

On Thu, Jan 23, 2014 at 01:02:47PM +0800, Xiubo Li wrote:
> Make it easier for generic code to work with set_tdm_slot() by distinguishing
> between the operation not being supported and an error as is done.

Applied, thanks.

Attachments:

2014-01-23 19:40:01

by Mark Brown

Subject: Re: [PATCH Resend 7/8] ASoC: add snd_soc_of_parse_audio_simple_widgets for DeviceTree

On Thu, Jan 23, 2014 at 01:02:49PM +0800, Xiubo Li wrote:

> +static struct snd_soc_dapm_widget simple_widgets[] = {
> + SND_SOC_DAPM_MIC("Mic", NULL),
> + SND_SOC_DAPM_LINE("Line", NULL),
> + SND_SOC_DAPM_HP("Hp", NULL),
> + SND_SOC_DAPM_SPK("Spk", NULL),
> +};

Does this mean we're restricted to a particular set of names? That
seems sad and won't work if there's a desire for more than one of a
given widget - the main use case I can see is multiple microphones with
separate microphone biases. How about having some templates that we
copy and then replace the name with the one the user supplied?

> + if (cnt <= 0) {
> + dev_err(card->dev,
> + "ASoC: Property '%s' does not exist or "
> + "length is zero\n", propname);

Don't split error messages over lines, this makes it harder to grep for
them.

Attachments:

2014-01-23 19:40:53

by Mark Brown

Subject: Re: [PATCH 8/8] ASoC: simple-card: add off-codec widgets supports.

On Thu, Jan 23, 2014 at 01:02:50PM +0800, Xiubo Li wrote:

> + /* off-codec widgets */
> + if (of_property_read_bool(node, "simple-audio-card,widgets")) {
> + ret = snd_soc_of_parse_audio_simple_widgets(&priv->snd_card,
> + "simple-audio-card,widgets");
> + if (ret)
> + return ret;
> + }

Again, needs documenting.

Attachments:

2014-01-23 19:49:23

by Mark Brown

Subject: Re: [PATCH 6/8] ASoC: simple-card: add tdm slot supports

On Thu, Jan 23, 2014 at 01:02:48PM +0800, Xiubo Li wrote:
> For some CPU/CODEC DAI devices the tdm slot maybe needed. This patch
> adds the tdm slot supporting for simple-card driver.

This needs binding documentation.

Attachments:

2014-01-24 02:46:49

by Xiubo Li

Subject: RE: [PATCH Resend 7/8] ASoC: add snd_soc_of_parse_audio_simple_widgets for DeviceTree

> > +static struct snd_soc_dapm_widget simple_widgets[] = {
> > + SND_SOC_DAPM_MIC("Mic", NULL),
> > + SND_SOC_DAPM_LINE("Line", NULL),
> > + SND_SOC_DAPM_HP("Hp", NULL),
> > + SND_SOC_DAPM_SPK("Spk", NULL),
> > +};
>

This is the templates for "Microphone XXX", "Line XXX", "Headphone XXX"
and "Spk XXX" .



> Does this mean we're restricted to a particular set of names? That
> seems sad and won't work if there's a desire for more than one of a
> given widget - the main use case I can see is multiple microphones with
> separate microphone biases. How about having some templates that we
> copy and then replace the name with the one the user supplied?
>

This code has already supplied this I think.

Using this API, the only limitation is that, for example if the user has
multiple Microphones, it should set the name in DT file like:

"Mic XXX", "Mic YYY", "Mic ZZZ"...

And these widgets will use the template of : SND_SOC_DAPM_MIC("Mic", NULL),
And the widgets' names will be replaced with "Mic XXX", "Mic YYY",
"Mic ZZZ"... from the user's DT node.



Or your suggestion is ??

The templates is:
+static struct snd_soc_dapm_widget simple_widgets[] = {
+ SND_SOC_DAPM_MIC("Mic", NULL),
+ SND_SOC_DAPM_LINE("Line", NULL),
+ SND_SOC_DAPM_HP("Hp", NULL),
+ SND_SOC_DAPM_SPK("Spk", NULL),
+};

And in the DT node:
simple-off-codec-widgets =
/* template-name user-supplied-name */
"Mic", "Microphone XXX",
"Mic", "UUU Microphone YYY",
"Hp", "TTT Headphone XXX",
"Hp", "Headphone YYY";

And then just replace the template's name with the user supplied name?


Thanks,

Best Regards,
Xiubo
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?

2014-01-24 07:28:22

by Xiubo Li

Subject: RE: [PATCH Resend 3/8] ASoC: simple-card: simplify the daifmt code

> Subject: Re: [PATCH Resend 3/8] ASoC: simple-card: simplify the daifmt code
>
> On Thu, Jan 23, 2014 at 01:02:45PM +0800, Xiubo Li wrote:
> > In the asoc_simple_card_parse_of() will parse the device node's CPU/CODEC
> > DAI commone fmts, and then in asoc_simple_card_sub_parse_of() will parse
> > the CPU/CODEC DAI's sub-node fmts, so we can combine the info->daifmt and
> > info->set.fmt in asoc_simple_card_sub_parse_of() not while just before
> > _set_fmt().
>
> This doesn't apply against the current topic/simple, can you please
> check and resend?

Yes, it's caused by the dependency of PATCH 2/8, and the PATCH 2/8 needs to
be revised.

I will resend them, which hasn't been applied, and also adding one binding
patch.

Thanks,

--
Best Regards,
Xiubo

2014-01-24 13:05:25

by Mark Brown

Subject: Re: [PATCH Resend 7/8] ASoC: add snd_soc_of_parse_audio_simple_widgets for DeviceTree

On Fri, Jan 24, 2014 at 02:46:33AM +0000, [email protected] wrote:

> > > +static struct snd_soc_dapm_widget simple_widgets[] = {
> > > + SND_SOC_DAPM_MIC("Mic", NULL),
> > > + SND_SOC_DAPM_LINE("Line", NULL),
> > > + SND_SOC_DAPM_HP("Hp", NULL),
> > > + SND_SOC_DAPM_SPK("Spk", NULL),
> > > +};

> This is the templates for "Microphone XXX", "Line XXX", "Headphone XXX"
> and "Spk XXX" .

> > Does this mean we're restricted to a particular set of names? That
> > seems sad and won't work if there's a desire for more than one of a
> > given widget - the main use case I can see is multiple microphones with
> > separate microphone biases. How about having some templates that we
> > copy and then replace the name with the one the user supplied?

> This code has already supplied this I think.

OK, that's fine - this is one of the things that the lack of binding
documentation doesn't help with, it says what the code is supposed to
do. When reviewing DT code without bindings I tend to just skim the
code.

Attachments:

2014-01-26 05:15:11

by Xiubo Li

Subject: RE: [PATCH 1/8] ASoC: fsl: Add VF610 soc audio card Kconfig

Hi Mark,

> > +config SND_SOC_VF610_SGTL5000
> > + tristate "SoC Audio support for VF610 boards with SGTL5000"
> > + depends on OF && I2C
> > + select SND_SOC_FSL_SAI
> > + select SND_SOC_SGTL5000
> > + select SND_SIMPLE_CARD
>
> ...for simple card since the whole idea is to support any CODEC with the
> same driver we should probably just make sure that all the individual
> drivers can be enabled in Kconfig, that way we don't have to have
> specific Kconfig entries for boards and loose some of the benefit of the
> generic card. I sent a patch earlier exposing the OF supporting CODEC
> drivers, one for at least some of the Freescale CPU drivers was sent to
> the list recently too but there were some review comments.

I hadn't found the patches, could you tell me the titles of them please?

Thanks very much,

--
* Happy New Year *

Best Regards,
Xiubo

2014-01-26 12:43:28

by Mark Brown

Subject: Re: [PATCH 1/8] ASoC: fsl: Add VF610 soc audio card Kconfig

On Sun, Jan 26, 2014 at 05:15:05AM +0000, [email protected] wrote:

> > ...for simple card since the whole idea is to support any CODEC with the
> > same driver we should probably just make sure that all the individual
> > drivers can be enabled in Kconfig, that way we don't have to have
> > specific Kconfig entries for boards and loose some of the benefit of the
> > generic card. I sent a patch earlier exposing the OF supporting CODEC
> > drivers, one for at least some of the Freescale CPU drivers was sent to
> > the list recently too but there were some review comments.

> I hadn't found the patches, could you tell me the titles of them please?

My patch was "ASoC: codecs: Make OF supported CODECs visible in
Kconfig". I can't remember offhand what the patch for i.MX was called.

Attachments:

2014-02-10 04:12:23

by Xiubo Li

Subject: RE: [PATCH 1/8] ASoC: fsl: Add VF610 soc audio card Kconfig

> Subject: Re: [PATCH 1/8] ASoC: fsl: Add VF610 soc audio card Kconfig
>
> On Sun, Jan 26, 2014 at 05:15:05AM +0000, [email protected] wrote:
>
> > > ...for simple card since the whole idea is to support any CODEC with the
> > > same driver we should probably just make sure that all the individual
> > > drivers can be enabled in Kconfig, that way we don't have to have
> > > specific Kconfig entries for boards and loose some of the benefit of the
> > > generic card. I sent a patch earlier exposing the OF supporting CODEC
> > > drivers, one for at least some of the Freescale CPU drivers was sent to
> > > the list recently too but there were some review comments.
>
> > I hadn't found the patches, could you tell me the titles of them please?
>
> My patch was "ASoC: codecs: Make OF supported CODECs visible in
> Kconfig". I can't remember offhand what the patch for i.MX was called.

Yes, found it.

If so, shouldn't the CPU DAIs(such as I2S) also be invisible in Kconfig like
CODECs ?


Thanks very much.
--
Best Regards,
Xiubo

2014-02-10 12:51:12

by Mark Brown

Subject: Re: [PATCH 1/8] ASoC: fsl: Add VF610 soc audio card Kconfig

On Mon, Feb 10, 2014 at 04:12:19AM +0000, [email protected] wrote:

> > My patch was "ASoC: codecs: Make OF supported CODECs visible in
> > Kconfig". I can't remember offhand what the patch for i.MX was called.

> Yes, found it.

> If so, shouldn't the CPU DAIs(such as I2S) also be invisible in Kconfig like
> CODECs ?

Yes, they should be getting made visible too when they can work with
simple-card - that was what the i.MX patch was doing.

Attachments: