[alsa-devel] [PATCH v2 0/3] Add tdm slot support

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

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; + };

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

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.

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.

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) {

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;
participants (4)
-
Lars-Peter Clausen
-
Li.Xiubo@freescale.com
-
Mark Brown
-
Xiubo Li