[PATCH v2 0/3] Add tdm slot support

[PATCH v2 0/3] Add tdm slot support

[Xiubo Li]

Xiubo Li (3):
  ASoC: binding: add tdm-slot.txt
  ASoC: core: add slot information parsing supports
  ASoC: simple-card: add slot information parsing supports

 .../devicetree/bindings/sound/simple-card.txt      |  1 +
 .../devicetree/bindings/sound/tdm-slot.txt         | 17 +++++++++++
 include/sound/simple_card.h                        |  1 +
 include/sound/soc.h                                | 10 +++++++
 sound/soc/generic/simple-card.c                    | 35 ++++++++++++++++++++--
 sound/soc/soc-core.c                               | 23 ++++++++++++++
 6 files changed, 84 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/tdm-slot.txt

-- 
1.8.4

[PATCH v2 1/3] ASoC: binding: add tdm-slot.txt

[Xiubo Li]

TDM slot:

This specifies audio DAI's TDM slot.

Each entry is has four non-negative integer values in DT:
       <tx_mask, rx_mask, slots, slot_width>

For instance:
       simple-slot-info = <0xffffffc 0xffffffc 2 0>;

And this will be parsed into the following format:
       struct soc_slot_info {
              unsigned int tx_mask;
              unsigned int rx_mask;
              int slots;
              int slot_width;
       };

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 Documentation/devicetree/bindings/sound/tdm-slot.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/tdm-slot.txt

diff --git a/Documentation/devicetree/bindings/sound/tdm-slot.txt b/Documentation/devicetree/bindings/sound/tdm-slot.txt
new file mode 100644
index 0000000..33dfb15
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/tdm-slot.txt
@@ -0,0 +1,17 @@
+TDM slot:
+
+This specifies audio DAI's TDM slot.
+
+Each entry is has four non-negative integer values in DT:
+	<tx_mask, rx_mask, slots, slot_width>
+
+For instance:
+	simple-slot-info = <0xffffffc 0xffffffc 2 0>;
+
+And this will be parsed into the following format:
+	struct soc_slot_info {
+	       unsigned int tx_mask;
+	       unsigned int rx_mask;
+	       int slots;
+	       int slot_width;
+	};
-- 
1.8.4

Re: [alsa-devel] [PATCH v2 1/3] ASoC: binding: add tdm-slot.txt

[Lars-Peter Clausen]

On 02/12/2014 08:45 AM, Xiubo Li wrote:
> TDM slot:
>
> This specifies audio DAI's TDM slot.
>
> Each entry is has four non-negative integer values in DT:
>         <tx_mask, rx_mask, slots, slot_width>
>
> For instance:
>         simple-slot-info = <0xffffffc 0xffffffc 2 0>;


The current internal API for TDM is very poor, I don't think we want to 
expose that 1 to 1 to the devicetree. Since this means we'd have to support 
that forever. The first thing is that the semantics of 
snd_soc_dai_set_tdm_slot() are very unclear. E.g. some drivers use a zero 
bit for a active slot, some drivers use a 1 bit for a active slot. The 
second thing is that we are not able to specify which channel should be 
mapped to which slot. You can merely specify from/to which slots the CODEC 
should read/write and then it is up to the driver to guess which channel 
should go to which slot. In my opinion a binding that allows to specify a 
explicit mapping of which channel goes to which slot would be much better.

Also those are four different settings. In my opinion they should not be 
expressed in one property, but rather in four. E.g. specifying a tx_mask for 
a rx only device does not make much sense.

- Lars

Re: [alsa-devel] [PATCH v2 1/3] ASoC: binding: add tdm-slot.txt

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1209 bytes --]

On Wed, Feb 12, 2014 at 10:26:52AM +0100, Lars-Peter Clausen wrote:

> The current internal API for TDM is very poor, I don't think we want
> to expose that 1 to 1 to the devicetree. Since this means we'd have
> to support that forever. The first thing is that the semantics of
> snd_soc_dai_set_tdm_slot() are very unclear. E.g. some drivers use a
> zero bit for a active slot, some drivers use a 1 bit for a active

Yes, and if we do end up using masks we need to nail down what's going
on in the DT.

> slot. The second thing is that we are not able to specify which
> channel should be mapped to which slot. You can merely specify
> from/to which slots the CODEC should read/write and then it is up to
> the driver to guess which channel should go to which slot. In my
> opinion a binding that allows to specify a explicit mapping of which
> channel goes to which slot would be much better.

It'd certainly be good to be able to do that, though having a default
would make life easier.

> Also those are four different settings. In my opinion they should
> not be expressed in one property, but rather in four. E.g.
> specifying a tx_mask for a rx only device does not make much sense.

That makes sense.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

RE: [alsa-devel] [PATCH v2 1/3] ASoC: binding: add tdm-slot.txt

[Li.Xiubo]

> > The current internal API for TDM is very poor, I don't think we want
> > to expose that 1 to 1 to the devicetree. Since this means we'd have
> > to support that forever. The first thing is that the semantics of
> > snd_soc_dai_set_tdm_slot() are very unclear. E.g. some drivers use a
> > zero bit for a active slot, some drivers use a 1 bit for a active
> 
> Yes, and if we do end up using masks we need to nail down what's going
> on in the DT.
>

Yes, certainly.

> > slot. The second thing is that we are not able to specify which
> > channel should be mapped to which slot. You can merely specify
> > from/to which slots the CODEC should read/write and then it is up to
> > the driver to guess which channel should go to which slot. In my
> > opinion a binding that allows to specify a explicit mapping of which
> > channel goes to which slot would be much better.
> 
> It'd certainly be good to be able to do that, though having a default
> would make life easier.
> 

@Lars, @Mark,

Yes, then will that means that we can just end up parsing masks from DT
and the masks will be generated by the driver specified
dai->driver->ops->of_xlate_tdm_slot_mask(slots, &tx_mask, &rx_mask)
callback or one default
snd_soc_of_xlate_tdm_slot_mask(slots, &tx_mask, &rx_mask), and the
'slots' is the number of the slot parsed from the DT node ?

Or other better ways ?


Thanks,

--
Best Regards,
Xiubo





> > Also those are four different settings. In my opinion they should
> > not be expressed in one property, but rather in four. E.g.
> > specifying a tx_mask for a rx only device does not make much sense.
> 
> That makes sense.

[PATCH v2 2/3] ASoC: core: add slot information parsing supports

[Xiubo Li]

For some CPU/CODEC DAI devices the slot infomation maybe needed. This
patch adds the slot parsing from DT supports.

The style of the slot information in DT should be:
	<tx_mask, rx_mask, slots, slot_width>

For instance:
	simple-slot-info = <0xffffffc 0xffffffc 2 0>;

Please refer to tdm-slot.txt for more detail.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 include/sound/soc.h  | 10 ++++++++++
 sound/soc/soc-core.c | 23 +++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 21d025e..1aa85d0 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1110,6 +1110,13 @@ struct soc_enum {
 	const unsigned int *values;
 };
 
+struct soc_slot_info {
+	unsigned int tx_mask;
+	unsigned int rx_mask;
+	int slots;
+	int slot_width;
+};
+
 /* codec IO */
 unsigned int snd_soc_read(struct snd_soc_codec *codec, unsigned int reg);
 unsigned int snd_soc_write(struct snd_soc_codec *codec,
@@ -1190,6 +1197,9 @@ 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_slot_info(struct device_node *np,
+			      struct soc_slot_info *info,
+			      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 abee5f4..3ad4b88 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -4558,6 +4558,29 @@ int snd_soc_of_parse_audio_simple_widgets(struct snd_soc_card *card,
 }
 EXPORT_SYMBOL_GPL(snd_soc_of_parse_audio_simple_widgets);
 
+int snd_soc_of_parse_slot_info(struct device_node *np,
+			       struct soc_slot_info *info,
+			       const char *propname)
+{
+	u32 out_value[4];
+	int ret;
+
+	if (!info)
+		return -EINVAL;
+
+	ret = of_property_read_u32_array(np, propname, out_value, 4);
+	if (ret)
+		return ret;
+
+	info->tx_mask = out_value[0];
+	info->rx_mask = out_value[1];
+	info->slots = out_value[2];
+	info->slot_width = out_value[3];
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_of_parse_slot_info);
+
 int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
 				   const char *propname)
 {
-- 
1.8.4

[PATCH v2 3/3] ASoC: simple-card: add slot information parsing supports

[Xiubo Li]

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

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 .../devicetree/bindings/sound/simple-card.txt      |  1 +
 include/sound/simple_card.h                        |  1 +
 sound/soc/generic/simple-card.c                    | 35 ++++++++++++++++++++--
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index 0527358..7abf8f3 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -18,6 +18,7 @@ Optional properties:
 					  Each entry is a pair of strings, the first being the
 					  connection's sink, the second being the connection's
 					  source.
+- simple-audio-card,slot-info		: Please refer to tdm-slot.txt.
 
 Required subnodes:
 
diff --git a/include/sound/simple_card.h b/include/sound/simple_card.h
index e1ac996..d645241 100644
--- a/include/sound/simple_card.h
+++ b/include/sound/simple_card.h
@@ -18,6 +18,7 @@ struct asoc_simple_dai {
 	const char *name;
 	unsigned int fmt;
 	unsigned int sysclk;
+	struct soc_slot_info *slot;
 };
 
 struct asoc_simple_card_info {
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 4fe8abc..fd230c7 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -9,11 +9,14 @@
  * published by the Free Software Foundation.
  */
 #include <linux/clk.h>
+#include <linux/device.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/string.h>
 #include <sound/simple_card.h>
+#include <sound/soc-dai.h>
+#include <sound/soc.h>
 
 struct simple_card_data {
 	struct snd_soc_card snd_card;
@@ -44,6 +47,17 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
 		}
 	}
 
+	if (set->slot) {
+		ret = snd_soc_dai_set_tdm_slot(dai, set->slot->tx_mask,
+						set->slot->rx_mask,
+						set->slot->slots,
+						set->slot->slot_width);
+		if (ret && ret != -ENOTSUPP) {
+			dev_err(dai->dev, "simple-card: set_tdm_slot error\n");
+			goto err;
+		}
+	}
+
 	ret = 0;
 
 err:
@@ -74,7 +88,8 @@ 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;
@@ -94,6 +109,18 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 	if (ret < 0)
 		goto parse_error;
 
+	/* parse TDM slot information */
+	if (of_property_read_bool(np, "simple-audio-card,slot-info")) {
+		dai->slot = devm_kzalloc(dev, sizeof(*dai->slot), GFP_KERNEL);
+		if (!dai->slot)
+			return -ENOMEM;
+
+		ret = snd_soc_of_parse_slot_info(np, dai->slot,
+				"simple-audio-card,slot-info");
+		if (ret < 0)
+			goto parse_error;
+	}
+
 	/*
 	 * bitclock-inversion, frame-inversion
 	 * bitclock-master,    frame-master
@@ -173,7 +200,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;
 
@@ -184,7 +212,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