[alsa-devel] [PATCHv3 0/3] ASoC: simple-card: simplify the code.
Any comment and advice are welcome.
Change in v3: - Fix binding from Jean's comment. - Remove 'ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.' and will send it separately later.
Change in v2: - Maintian compatibility with the old DTs.
Change in v1: - Add simple-card dts node patches. - Fix format parsing bug from Jean-Francois's comment. - Rebase to Kuninori-san's newest changes in next branch.
Xiubo Li (3): ASoC: simple-card: Merge single and muti DAI link(s) code. ASoC: simple-card: Adjust the comments of simple card. ASoC: simple-card: binding: update binding to support the new style.
.../devicetree/bindings/sound/simple-card.txt | 183 ++++++++++++++------- sound/soc/generic/simple-card.c | 70 ++++---- 2 files changed, 161 insertions(+), 92 deletions(-)
This patch will split the DT node into old style and new style: The new style will merge the single DAI link and muti DAI links code together, the new style will be easier to add muti DAI links from old single DAI link DTs.
This patch will maintian compatibility with the old DTs.
Signed-off-by: Xiubo Li Li.Xiubo@freescale.com --- sound/soc/generic/simple-card.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 986d2c7..1597a24 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -178,6 +178,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, char *prefix = ""; int ret;
+ /* For single DAI link & old style of DT node */ if (is_top_level_node) prefix = "simple-audio-card,";
@@ -309,14 +310,16 @@ dai_link_of_err:
static int asoc_simple_card_parse_of(struct device_node *node, struct simple_card_data *priv, - struct device *dev, - int multi) + struct device *dev) { struct snd_soc_dai_link *dai_link = priv->snd_card.dai_link; struct simple_dai_props *dai_props = priv->dai_props; u32 val; int ret;
+ if (!node) + return -EINVAL; + /* parsing the card name from DT */ snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
@@ -344,7 +347,8 @@ static int asoc_simple_card_parse_of(struct device_node *node, dev_dbg(dev, "New simple-card: %s\n", priv->snd_card.name ? priv->snd_card.name : "");
- if (multi) { + /* Single/Muti DAI link(s) & New style of DT node */ + if (of_get_child_by_name(node, "simple-audio-card,dai-link")) { struct device_node *np = NULL; int i = 0;
@@ -361,6 +365,7 @@ static int asoc_simple_card_parse_of(struct device_node *node, i++; } } else { + /* For single DAI link & old style of DT node */ ret = asoc_simple_card_dai_link_of(node, dev, dai_link, dai_props, true); if (ret < 0) @@ -400,16 +405,13 @@ static int asoc_simple_card_probe(struct platform_device *pdev) struct snd_soc_dai_link *dai_link; struct device_node *np = pdev->dev.of_node; struct device *dev = &pdev->dev; - int num_links, multi, ret; + int num_links, ret;
/* get the number of DAI links */ - if (np && of_get_child_by_name(np, "simple-audio-card,dai-link")) { + if (np && of_get_child_by_name(np, "simple-audio-card,dai-link")) num_links = of_get_child_count(np); - multi = 1; - } else { + else num_links = 1; - multi = 0; - }
/* allocate the private data and the DAI link array */ priv = devm_kzalloc(dev, @@ -436,7 +438,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
if (np && of_device_is_available(np)) {
- ret = asoc_simple_card_parse_of(np, priv, dev, multi); + ret = asoc_simple_card_parse_of(np, priv, dev); if (ret < 0) { if (ret != -EPROBE_DEFER) dev_err(dev, "parse error %d\n", ret);
On Wed, Sep 03, 2014 at 10:23:39AM +0800, Xiubo Li wrote:
This patch will split the DT node into old style and new style: The new style will merge the single DAI link and muti DAI links code together, the new style will be easier to add muti DAI links from old single DAI link DTs.
Applied, thanks.
Signed-off-by: Xiubo Li Li.Xiubo@freescale.com --- sound/soc/generic/simple-card.c | 48 ++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 1597a24..07f7fb6 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -120,7 +120,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np, int ret;
/* - * get node via "sound-dai = <&phandle port>" + * Get node via "sound-dai = <&phandle port>" * it will be used as xxx_of_node on soc_bind_dai_link() */ node = of_parse_phandle(np, "sound-dai", 0); @@ -128,19 +128,19 @@ asoc_simple_card_sub_parse_of(struct device_node *np, return -ENODEV; *p_node = node;
- /* get dai->name */ + /* Get dai->name */ ret = snd_soc_of_get_dai_name(np, name); if (ret < 0) return ret;
- /* parse TDM slot */ + /* Parse TDM slot */ ret = snd_soc_of_parse_tdm_slot(np, &dai->slots, &dai->slot_width); if (ret) return ret;
/* - * dai->sysclk come from - * "clocks = <&xxx>" (if system has common clock) + * Parse dai->sysclk come from "clocks = <&xxx>" + * (if system has common clock) * or "system-clock-frequency = <xxx>" * or device's module clock. */ @@ -232,9 +232,11 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, 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. */ + /* + * No DAI link level and master setting was 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 = @@ -263,10 +265,10 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, goto dai_link_of_err; }
- /* simple-card assumes platform == cpu */ + /* Simple Card assumes platform == cpu */ dai_link->platform_of_node = dai_link->cpu_of_node;
- /* Link name is created from CPU/CODEC dai name */ + /* DAI link name is created from CPU/CODEC dai name */ name = devm_kzalloc(dev, strlen(dai_link->cpu_dai_name) + strlen(dai_link->codec_dai_name) + 2, @@ -288,11 +290,11 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, dai_props->codec_dai.sysclk);
/* - * soc_bind_dai_link() will check cpu name - * after of_node matching if dai_link has cpu_dai_name. - * but, it will never match if name was created by fmt_single_name() - * remove cpu_dai_name to escape name matching. - * see + * In soc_bind_dai_link() will check cpu name after + * of_node matching if dai_link has cpu_dai_name. + * but, it will never match if name was created by + * fmt_single_name() remove cpu_dai_name to escape + * name matching. Please see: * fmt_single_name() * fmt_multiple_name() */ @@ -320,10 +322,10 @@ static int asoc_simple_card_parse_of(struct device_node *node, if (!node) return -EINVAL;
- /* parsing the card name from DT */ + /* Parse the card name from DT */ snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
- /* off-codec widgets */ + /* The 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"); @@ -378,7 +380,7 @@ static int asoc_simple_card_parse_of(struct device_node *node, return 0; }
-/* update the reference count of the devices nodes at end of probe */ +/* Decrease the reference count of the device nodes */ static int asoc_simple_card_unref(struct platform_device *pdev) { struct snd_soc_card *card = platform_get_drvdata(pdev); @@ -407,29 +409,27 @@ static int asoc_simple_card_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; int num_links, ret;
- /* get the number of DAI links */ + /* Get the number of DAI links */ if (np && of_get_child_by_name(np, "simple-audio-card,dai-link")) num_links = of_get_child_count(np); else num_links = 1;
- /* allocate the private data and the DAI link array */ + /* Allocate the private data and the DAI link array */ priv = devm_kzalloc(dev, sizeof(*priv) + sizeof(*dai_link) * num_links, GFP_KERNEL); if (!priv) return -ENOMEM;
- /* - * init snd_soc_card - */ + /* Init snd_soc_card */ priv->snd_card.owner = THIS_MODULE; priv->snd_card.dev = dev; dai_link = priv->dai_link; priv->snd_card.dai_link = dai_link; priv->snd_card.num_links = num_links;
- /* get room for the other properties */ + /* Get room for the other properties */ priv->dai_props = devm_kzalloc(dev, sizeof(*priv->dai_props) * num_links, GFP_KERNEL);
On Wed, Sep 03, 2014 at 10:23:40AM +0800, Xiubo Li wrote:
Signed-off-by: Xiubo Li Li.Xiubo@freescale.com
This doesn't apply against current code.
Subject: Re: [PATCHv3 2/3] ASoC: simple-card: Adjust the comments of simple card.
On Wed, Sep 03, 2014 at 10:23:40AM +0800, Xiubo Li wrote:
Signed-off-by: Xiubo Li Li.Xiubo@freescale.com
This doesn't apply against current code.
I will send one new version about this.
Thanks, BRs Xiubo
This update patch will split the DT node into old style and new style: The new style will will be easier to add muti DAI links from old single DAI link DTs.
This patch will maintian compatibility with the old DTs.
Signed-off-by: Xiubo Li Li.Xiubo@freescale.com --- .../devicetree/bindings/sound/simple-card.txt | 183 ++++++++++++++------- 1 file changed, 125 insertions(+), 58 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index c2e9841..8c1bd30 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -1,15 +1,19 @@ -Simple-Card: +Device-Tree bindings for Simple Card
Simple-Card specifies audio DAI connections of SoC <-> codec.
-Required properties: +=== Top level's properties and subnodes ===
+*** Required properties *** - compatible : "simple-audio-card"
-Optional properties: - +*** Optional properties *** - simple-audio-card,name : User specified audio sound card name, one string property. +- simple-audio-card,format : CPU/CODEC common audio format. + "i2s", "right_j", "left_j" , "dsp_a" + "dsp_b", "ac97", "pdm", "msb", "lsb" + (This is used for single DAI link & old style.) - simple-audio-card,widgets : Please refer to widgets.txt. - simple-audio-card,routing : A list of the connections between audio components. Each entry is a pair of strings, the first being the @@ -17,63 +21,85 @@ Optional properties: source. - simple-audio-card,mclk-fs : Multiplication factor between stream rate and codec mclk. - -Optional subnodes: - -- simple-audio-card,dai-link : Container for dai-link level - properties and the CPU and CODEC - sub-nodes. This container may be - omitted when the card has only one - DAI link. See the examples and the - section bellow. - -Dai-link subnode properties and subnodes: - -If dai-link subnode is omitted and the subnode properties are directly -under "sound"-node the subnode property and subnode names have to be -prefixed with "simple-audio-card,"-prefix. - -Required dai-link subnodes: - -- cpu : CPU sub-node -- codec : CODEC sub-node - -Optional dai-link subnode properties: - +- simple-audio-card,frame-master : Indicates DAI link frame master. One phandle to a cpu + or codec subnode. + (This is used for single DAI link & old style.) +- simple-audio-card,bitclock-master : Indicates DAI link bit clock master. One phandle to a + cpu or codec subnode. + (This is used for single DAI link & old style.) + +*** Optional subnodes *** +- simple-audio-card,dai-link : Container for DAI link level properties and the CPU + and CODEC sub-nodes. This container may be omitted + when the card has only one DAI link and using the old + style. See the examples and the section bellow. +- simple-audio-card,cpu : CPU DAI sub-node. + (This is used for single DAI link & old style.) +- simple-audio-card,codec : CODEC DAI sub-node. + (This is used for single DAI link & old style.) + +=== DAI link node's properties and its subnodes === + +*** Required subnodes *** +- cpu : CPU DAI sub-node +- codec : CODEC DAI sub-node + +*** Optional properties *** - format : CPU/CODEC common audio format. "i2s", "right_j", "left_j" , "dsp_a" "dsp_b", "ac97", "pdm", "msb", "lsb" -- frame-master : Indicates dai-link frame master. - phandle to a cpu or codec subnode. -- bitclock-master : Indicates dai-link bit clock master. - phandle to a cpu or codec subnode. -- bitclock-inversion : bool property. Add this if the - dai-link uses bit clock inversion. -- frame-inversion : bool property. Add this if the - dai-link uses frame clock inversion. +- frame-master : Indicates DAI link frame master. One phandle to a cpu + or codec subnode. + (This is One boolean property for old style.) +- bitclock-master : Indicates DAI link bit clock master. One phandle to a + cpu or codec subnode. + (This is one boolean property for old style.)
For backward compatibility the frame-master and bitclock-master properties can be used as booleans in codec subnode to indicate if the -codec is the dai-link frame or bit clock master. In this case there -should be no dai-link node, the same properties should not be present +codec is the DAI link frame or bit clock master. In this case there +should be no DAI link node, the same properties should not be present at sound-node level, and the bitclock-inversion and frame-inversion properties should also be placed in the codec node if needed.
-Required CPU/CODEC subnodes properties:
-- sound-dai : phandle and port of CPU/CODEC +=== CPU/CODEC DAI node's properties and its subnodes ===
-Optional CPU/CODEC subnodes properties: +*** Required properties *** +- sound-dai : One phandle and port of CPU/CODEC
+*** Optional properties *** +- bitclock-inversion : Boolean property. Add this if the DAI device uses bit + clock inversion. +- frame-inversion : Boolean property. Add this if the DAI device uses frame + clock inversion. - dai-tdm-slot-num : Please refer to tdm-slot.txt. - dai-tdm-slot-width : Please refer to tdm-slot.txt. -- clocks / system-clock-frequency : specify subnode's clock if needed. - it can be specified via "clocks" if system has - clock node (= common clock), or "system-clock-frequency" - (if system doens't support common clock) +- clocks / system-clock-frequency : specify CPU/CODEC DAI node's clock if needed. It can be + specified via "clocks" if system has clock node + (= common clock), or "system-clock-frequency"(if system + doens't support common clock)
-Example 1 - single DAI link: +=== Examples === +*** CPU & CODEC DAI DT nodes *** +&i2c0 { + ak4648: ak4648@12 { + #sound-dai-cells = <0>; + compatible = "asahi-kasei,ak4648"; + reg = <0x12>; + }; +};
+sh_fsi2: sh_fsi2@ec230000 { + #sound-dai-cells = <1>; + compatible = "renesas,sh_fsi2"; + reg = <0xec230000 0x400>; + interrupt-parent = <&gic>; + interrupts = <0 146 0x4>; +}; + +Example 1 - single DAI link & old style: +bitclock-master and frame-master as phandles. sound { compatible = "simple-audio-card"; simple-audio-card,name = "VF610-Tower-Sound-Card"; @@ -91,6 +117,7 @@ sound {
simple-audio-card,cpu { sound-dai = <&sh_fsi2 0>; + bitclock-inversion; };
dailink0_master: simple-audio-card,codec { @@ -99,24 +126,64 @@ sound { }; };
-&i2c0 { - ak4648: ak4648@12 { - #sound-dai-cells = <0>; - compatible = "asahi-kasei,ak4648"; - reg = <0x12>; +Example 2 - single DAI link & old style: +bitclock-master and frame-master as boolean properties. +sound { + compatible = "simple-audio-card"; + simple-audio-card,name = "VF610-Tower-Sound-Card"; + simple-audio-card,format = "left_j"; + simple-audio-card,widgets = + "Microphone", "Microphone Jack", + "Headphone", "Headphone Jack", + "Speaker", "External Speaker"; + simple-audio-card,routing = + "MIC_IN", "Microphone Jack", + "Headphone Jack", "HP_OUT", + "External Speaker", "LINE_OUT"; + + simple-audio-card,cpu { + sound-dai = <&sh_fsi2 0>; }; -};
-sh_fsi2: sh_fsi2@ec230000 { - #sound-dai-cells = <1>; - compatible = "renesas,sh_fsi2"; - reg = <0xec230000 0x400>; - interrupt-parent = <&gic>; - interrupts = <0 146 0x4>; + simple-audio-card,codec { + sound-dai = <&ak4648>; + clocks = <&osc>; + bitclock-master; + frame-master; + bitclock-inversion; + }; };
-Example 2 - many DAI links: +Example 3 - single DAI link & new style: +sound { + compatible = "simple-audio-card"; + simple-audio-card,name = "VF610-Tower-Sound-Card"; + simple-audio-card,widgets = + "Microphone", "Microphone Jack", + "Headphone", "Headphone Jack", + "Speaker", "External Speaker"; + simple-audio-card,routing = + "MIC_IN", "Microphone Jack", + "Headphone Jack", "HP_OUT", + "External Speaker", "LINE_OUT"; + + simple-audio-card,dai-link { + format = "i2s"; + bitclock-master = <&dailink0_master>; + frame-master = <&dailink0_master>; + cpu { + sound-dai = <&sh_fsi2 0>; + frame-inversion; + }; + dailink0_master: codec { + sound-dai = <&ak4648>; + clocks = <&osc>; + frame-inversion; + }; + }; +};
+Example 4 - many DAI links: sound { compatible = "simple-audio-card"; simple-audio-card,name = "Cubox Audio";
On Wed, Sep 03, 2014 at 10:23:41AM +0800, Xiubo Li wrote:
This update patch will split the DT node into old style and new style: The new style will will be easier to add muti DAI links from old single DAI link DTs.
This patch will maintian compatibility with the old DTs.
Is everyone happy with this new style binding?
On 09/09/2014 03:09 PM, Mark Brown wrote:
On Wed, Sep 03, 2014 at 10:23:41AM +0800, Xiubo Li wrote:
This update patch will split the DT node into old style and new style: The new style will will be easier to add muti DAI links from old single DAI link DTs.
This patch will maintian compatibility with the old DTs.
Is everyone happy with this new style binding?
If I read the diff correctly the only change in the syntax is moving the bitclock-inversion and frame-inversion properties to DAI nodes also for the new style bindings.
I am Ok with that.
The document does not indicate that the same properties could be also placed to DAI link level like they used to, from where they would apply to both DAI nodes.
I am Ok with this too, but this breaks the backwards compatibility, which I guess is generally considered a problem. However, I doubt anyone but me has yet used this part of the new syntax and I am happy to fix my dts files.
Best regards, Jyri
ps. I could not find the corresponding code changes in the previous patch, so I assume they are coming later.
Hi,
Subject: Re: [PATCHv3 3/3] ASoC: simple-card: binding: update binding to support the new style.
On 09/09/2014 03:09 PM, Mark Brown wrote:
On Wed, Sep 03, 2014 at 10:23:41AM +0800, Xiubo Li wrote:
This update patch will split the DT node into old style and new style: The new style will will be easier to add muti DAI links from old single DAI link DTs.
This patch will maintian compatibility with the old DTs.
Is everyone happy with this new style binding?
If I read the diff correctly the only change in the syntax is moving the bitclock-inversion and frame-inversion properties to DAI nodes also for the new style bindings.
I am Ok with that.
The document does not indicate that the same properties could be also placed to DAI link level like they used to, from where they would apply to both DAI nodes.
I will post one new version of this patch with some very small fix, and I will also add this.
I am Ok with this too, but this breaks the backwards compatibility, which I guess is generally considered a problem. However, I doubt anyone but me has yet used this part of the new syntax and I am happy to fix my dts files.
Best regards, Jyri
ps. I could not find the corresponding code changes in the previous patch, so I assume they are coming later.
Yes, I have one patch about the new style in my local machine, for some reason I will post it later.
Thanks,
BRs Xiubo
participants (4)
-
Jyri Sarha -
Li.Xiubo＠freescale.com -
Mark Brown -
Xiubo Li