Re: [alsa-devel] [PATCH v2] ASoC: simple-card: add Device Tree support
On Thu, Oct 03, 2013 at 02:33:55AM -0700, Kuninori Morimoto wrote:
ping ?
Don't send contentless pings. I'm waiting for review from the device tree guys but I see now that you didn't CC them so they're unlikely to respond - please resend CCing them.
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information, and .of_xlate_dai_name support on each component driver.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2
- add common clock support, system-clock-frequency can over-write it - add some comment on code
v2 -> v3
- add devicetree ML address
.../devicetree/bindings/sound/simple-card.txt | 85 ++++++++++++ sound/soc/generic/simple-card.c | 146 +++++++++++++++++++- 2 files changed, 226 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt new file mode 100644 index 0000000..75bbc5a --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -0,0 +1,85 @@ +Simple-Card: + +Required properties: + +- compatible : "simple-audio" +- simple-audio,card-name : simple-audio card name +- simple-audio,cpu : CPU sub-node, see below +- simple-audio,codec : CODEC sub-node, see below + +Optional properties: + +- simple-audio,format : CPU/CODEC common format, see below + +Required cpu/codec subnode properties: + +- sound-dai : phandle and port for CPU/CODEC +- #sound-dai-cells : sound-dai phandle's node + +Optional subnode properties: + +- format : specific format if needed, see below +- frame-master : frame master +- bitclock-master : bitclock master +- bitclock-inversion : clock inversion +- frame-inversion : frame inversion +- clocks : phandle for system clock rate +- system-clock-frequency : system clock rate + it will overwrite clocks's rate + +simple-audio,format + "i2s" + "right_j" + "left_j" + "dsp_a" + "dsp_b" + "ac97" + "pdm" + "msb" + "lsb" + +Example: + +clock { + osc: oscillator { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <11289600>; + }; +}; + +sound { + compatible = "simple-audio"; + + simple-audio,card-name = "FSI2A-AK4648"; + simple-audio,format = "left_j"; + + simple-audio,cpu { + sound-dai = <&sh_fsi2 0>; + }; + + simple-audio,codec { + sound-dai = <&ak4648>; + bitclock-master; + frame-master; + clocks = <&osc>; + /* it can use this instead of clocks + * system-clock-frequency = <11289600>; */ + }; +}; + +&i2c0 { + ak4648: ak4648@0x12 { + #sound-dai-cells = <0>; + compatible = "asahi-kasei,ak4648"; + reg = <0x12>; + }; +}; + +sh_fsi2: sh_fsi2@0xec230000 { + #sound-dai-cells = <1>; + compatible = "renesas,sh_fsi2"; + reg = <0xec230000 0x400>; + interrupt-parent = <&gic>; + interrupts = <0 146 0x4>; +}; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 8c49147..62befbd 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -9,6 +9,8 @@ * published by the Free Software Foundation. */
+#include <linux/clk.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/module.h> #include <sound/simple_card.h> @@ -52,11 +54,135 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd) return 0; }
+static int +__asoc_simple_card_parse_of(struct device_node *np, + struct asoc_simple_dai *dai, + struct device_node **node) +{ + struct clk *clk; + int ret; + + /* + * 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); + if (!*node) + return -ENODEV; + + of_node_put(*node); + + /* get dai->name */ + ret = snd_soc_of_get_dai_name(np, &dai->name); + if (ret < 0) + return ret; + + /* + * bitclock-inversion, frame-inversion + * bitclock-master, frame-master + * and specific "format" if it has + */ + dai->fmt = snd_soc_of_parse_daifmt(np, NULL); + + /* dai->sysclk via "clolks = <xxx>" */ + clk = of_clk_get(np, 0); + if (IS_ERR(clk)) + dai->sysclk = 0; + else + dai->sysclk = clk_get_rate(clk); + + /* + * overwrite dai->sysclk if it has + * "system-clock-frequency = <xxx>" + */ + of_property_read_u32(np, + "system-clock-frequency", + &dai->sysclk); + + return 0; +} + +static int asoc_simple_card_parse_of(struct device_node *node, + struct asoc_simple_card_info *info, + struct device *dev, + struct device_node **of_cpu, + struct device_node **of_codec, + struct device_node **of_platform) +{ + struct device_node *np; + int ret = 0; + + /* get card name */ + of_property_read_string(node, "simple-audio,card-name", &info->card); + info->name = info->card; + + /* get CPU/CODEC common format via simple-audio,format */ + info->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio,") & + (SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK); + + /* CPU/CODEC sub-node */ + for_each_child_of_node(node, np) { + if (0 == strcmp("simple-audio,cpu", np->name)) + ret = __asoc_simple_card_parse_of(np, + &info->cpu_dai, + of_cpu); + if (0 == strcmp("simple-audio,codec", np->name)) + ret = __asoc_simple_card_parse_of(np, + &info->codec_dai, + of_codec); + if (ret < 0) + return ret; + } + + /* simple-card assumes platform == cpu */ + *of_platform = *of_cpu; + + dev_dbg(dev, "card-name : %s\n", info->card); + dev_dbg(dev, "platform : %04x / %p\n", + info->daifmt, + *of_platform); + dev_dbg(dev, "cpu : %s / %04x / %d / %p\n", + info->cpu_dai.name, + info->cpu_dai.fmt, + info->cpu_dai.sysclk, + *of_cpu); + dev_dbg(dev, "codec : %s / %04x / %d / %p\n", + info->codec_dai.name, + info->codec_dai.fmt, + info->codec_dai.sysclk, + *of_codec); + + return 0; +} + static int asoc_simple_card_probe(struct platform_device *pdev) { - struct asoc_simple_card_info *cinfo = pdev->dev.platform_data; + struct asoc_simple_card_info *cinfo; + struct device_node *np = pdev->dev.of_node; + struct device_node *of_cpu, *of_codec, *of_platform; struct device *dev = &pdev->dev;
+ cinfo = NULL; + of_cpu = NULL; + of_codec = NULL; + of_platform = NULL; + if (np && of_device_is_available(np)) { + cinfo = devm_kzalloc(dev, sizeof(*cinfo), GFP_KERNEL); + if (cinfo) { + int ret; + ret = asoc_simple_card_parse_of(np, cinfo, dev, + &of_cpu, + &of_codec, + &of_platform); + if (ret < 0) { + dev_err(dev, "parse error %d\n", ret); + return ret; + } + } + } else { + cinfo = pdev->dev.platform_data; + } + if (!cinfo) { dev_err(dev, "no info for asoc-simple-card\n"); return -EINVAL; @@ -64,10 +190,10 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
if (!cinfo->name || !cinfo->card || - !cinfo->codec || - !cinfo->platform || - !cinfo->cpu_dai.name || - !cinfo->codec_dai.name) { + !cinfo->codec_dai.name || + !(cinfo->codec || of_codec) || + !(cinfo->platform || of_platform) || + !(cinfo->cpu_dai.name || of_cpu)) { dev_err(dev, "insufficient asoc_simple_card_info settings\n"); return -EINVAL; } @@ -81,6 +207,9 @@ static int asoc_simple_card_probe(struct platform_device *pdev) cinfo->snd_link.platform_name = cinfo->platform; cinfo->snd_link.codec_name = cinfo->codec; cinfo->snd_link.codec_dai_name = cinfo->codec_dai.name; + cinfo->snd_link.cpu_of_node = of_cpu; + cinfo->snd_link.codec_of_node = of_codec; + cinfo->snd_link.platform_of_node = of_platform; cinfo->snd_link.init = asoc_simple_card_dai_init;
/* @@ -102,10 +231,17 @@ static int asoc_simple_card_remove(struct platform_device *pdev) return snd_soc_unregister_card(&cinfo->snd_card); }
+static const struct of_device_id asoc_simple_of_match[] = { + { .compatible = "simple-audio", }, + {}, +}; +MODULE_DEVICE_TABLE(of, asoc_simple_of_match); + static struct platform_driver asoc_simple_card = { .driver = { .name = "asoc-simple-card", .owner = THIS_MODULE, + .of_match_table = asoc_simple_of_match, }, .probe = asoc_simple_card_probe, .remove = asoc_simple_card_remove,
On Thu, Oct 03, 2013 at 05:04:41PM -0700, Kuninori Morimoto wrote:
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information, and .of_xlate_dai_name support on each component driver.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
CCing in the DT maintainers directly.
v1 -> v2
- add common clock support, system-clock-frequency can over-write it
- add some comment on code
v2 -> v3
- add devicetree ML address
.../devicetree/bindings/sound/simple-card.txt | 85 ++++++++++++ sound/soc/generic/simple-card.c | 146 +++++++++++++++++++- 2 files changed, 226 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt new file mode 100644 index 0000000..75bbc5a --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -0,0 +1,85 @@ +Simple-Card:
+Required properties:
+- compatible : "simple-audio" +- simple-audio,card-name : simple-audio card name +- simple-audio,cpu : CPU sub-node, see below +- simple-audio,codec : CODEC sub-node, see below
+Optional properties:
+- simple-audio,format : CPU/CODEC common format, see below
+Required cpu/codec subnode properties:
+- sound-dai : phandle and port for CPU/CODEC +- #sound-dai-cells : sound-dai phandle's node
+Optional subnode properties:
+- format : specific format if needed, see below +- frame-master : frame master +- bitclock-master : bitclock master +- bitclock-inversion : clock inversion +- frame-inversion : frame inversion +- clocks : phandle for system clock rate +- system-clock-frequency : system clock rate
it will overwrite clocks's rate+simple-audio,format
- "i2s"
- "right_j"
- "left_j"
- "dsp_a"
- "dsp_b"
- "ac97"
- "pdm"
- "msb"
- "lsb"
+Example:
+clock {
- osc: oscillator {
#clock-cells = <0>;compatible = "fixed-clock";clock-frequency = <11289600>;- };
+};
+sound {
- compatible = "simple-audio";
- simple-audio,card-name = "FSI2A-AK4648";
- simple-audio,format = "left_j";
- simple-audio,cpu {
sound-dai = <&sh_fsi2 0>;- };
- simple-audio,codec {
sound-dai = <&ak4648>;bitclock-master;frame-master;clocks = <&osc>;/* it can use this instead of clocks* system-clock-frequency = <11289600>; */- };
+};
+&i2c0 {
- ak4648: ak4648@0x12 {
#sound-dai-cells = <0>;compatible = "asahi-kasei,ak4648";reg = <0x12>;- };
+};
+sh_fsi2: sh_fsi2@0xec230000 {
- #sound-dai-cells = <1>;
- compatible = "renesas,sh_fsi2";
- reg = <0xec230000 0x400>;
- interrupt-parent = <&gic>;
- interrupts = <0 146 0x4>;
+}; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 8c49147..62befbd 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -9,6 +9,8 @@
- published by the Free Software Foundation.
*/
+#include <linux/clk.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/module.h> #include <sound/simple_card.h> @@ -52,11 +54,135 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd) return 0; }
+static int +__asoc_simple_card_parse_of(struct device_node *np,
struct asoc_simple_dai *dai,struct device_node **node)+{
- struct clk *clk;
- int ret;
- /*
* 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);
- if (!*node)
return -ENODEV;- of_node_put(*node);
- /* get dai->name */
- ret = snd_soc_of_get_dai_name(np, &dai->name);
- if (ret < 0)
return ret;- /*
* bitclock-inversion, frame-inversion* bitclock-master, frame-master* and specific "format" if it has*/- dai->fmt = snd_soc_of_parse_daifmt(np, NULL);
- /* dai->sysclk via "clolks = <xxx>" */
- clk = of_clk_get(np, 0);
- if (IS_ERR(clk))
dai->sysclk = 0;- else
dai->sysclk = clk_get_rate(clk);- /*
* overwrite dai->sysclk if it has* "system-clock-frequency = <xxx>"*/- of_property_read_u32(np,
"system-clock-frequency",&dai->sysclk);- return 0;
+}
+static int asoc_simple_card_parse_of(struct device_node *node,
struct asoc_simple_card_info *info,struct device *dev,struct device_node **of_cpu,struct device_node **of_codec,struct device_node **of_platform)+{
- struct device_node *np;
- int ret = 0;
- /* get card name */
- of_property_read_string(node, "simple-audio,card-name", &info->card);
- info->name = info->card;
- /* get CPU/CODEC common format via simple-audio,format */
- info->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio,") &
(SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);- /* CPU/CODEC sub-node */
- for_each_child_of_node(node, np) {
if (0 == strcmp("simple-audio,cpu", np->name))ret = __asoc_simple_card_parse_of(np,&info->cpu_dai,of_cpu);if (0 == strcmp("simple-audio,codec", np->name))ret = __asoc_simple_card_parse_of(np,&info->codec_dai,of_codec);if (ret < 0)return ret;- }
- /* simple-card assumes platform == cpu */
- *of_platform = *of_cpu;
- dev_dbg(dev, "card-name : %s\n", info->card);
- dev_dbg(dev, "platform : %04x / %p\n",
info->daifmt,*of_platform);- dev_dbg(dev, "cpu : %s / %04x / %d / %p\n",
info->cpu_dai.name,info->cpu_dai.fmt,info->cpu_dai.sysclk,*of_cpu);- dev_dbg(dev, "codec : %s / %04x / %d / %p\n",
info->codec_dai.name,info->codec_dai.fmt,info->codec_dai.sysclk,*of_codec);- return 0;
+}
static int asoc_simple_card_probe(struct platform_device *pdev) {
- struct asoc_simple_card_info *cinfo = pdev->dev.platform_data;
struct asoc_simple_card_info *cinfo;
struct device_node *np = pdev->dev.of_node;
struct device_node *of_cpu, *of_codec, *of_platform; struct device *dev = &pdev->dev;
cinfo = NULL;
of_cpu = NULL;
of_codec = NULL;
of_platform = NULL;
if (np && of_device_is_available(np)) {
cinfo = devm_kzalloc(dev, sizeof(*cinfo), GFP_KERNEL);if (cinfo) {int ret;ret = asoc_simple_card_parse_of(np, cinfo, dev,&of_cpu,&of_codec,&of_platform);if (ret < 0) {dev_err(dev, "parse error %d\n", ret);return ret;}}} else {
cinfo = pdev->dev.platform_data;}
if (!cinfo) { dev_err(dev, "no info for asoc-simple-card\n"); return -EINVAL;
@@ -64,10 +190,10 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
if (!cinfo->name || !cinfo->card ||
!cinfo->codec ||!cinfo->platform ||!cinfo->cpu_dai.name ||!cinfo->codec_dai.name) {
!cinfo->codec_dai.name ||!(cinfo->codec || of_codec) ||!(cinfo->platform || of_platform) ||!(cinfo->cpu_dai.name || of_cpu)) {@@ -81,6 +207,9 @@ static int asoc_simple_card_probe(struct platform_device *pdev) cinfo->snd_link.platform_name = cinfo->platform; cinfo->snd_link.codec_name = cinfo->codec; cinfo->snd_link.codec_dai_name = cinfo->codec_dai.name;
cinfo->snd_link.cpu_of_node = of_cpu;
cinfo->snd_link.codec_of_node = of_codec;
cinfo->snd_link.platform_of_node = of_platform; cinfo->snd_link.init = asoc_simple_card_dai_init;
/*
@@ -102,10 +231,17 @@ static int asoc_simple_card_remove(struct platform_device *pdev) return snd_soc_unregister_card(&cinfo->snd_card); }
+static const struct of_device_id asoc_simple_of_match[] = {
- { .compatible = "simple-audio", },
- {},
+}; +MODULE_DEVICE_TABLE(of, asoc_simple_of_match);
static struct platform_driver asoc_simple_card = { .driver = { .name = "asoc-simple-card", .owner = THIS_MODULE,
.of_match_table = asoc_simple_of_match,-- 1.7.9.5
Hi,
On Fri, Oct 04, 2013 at 01:04:41AM +0100, Kuninori Morimoto wrote:
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information, and .of_xlate_dai_name support on each component driver.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
v1 -> v2
- add common clock support, system-clock-frequency can over-write it
- add some comment on code
v2 -> v3
- add devicetree ML address
.../devicetree/bindings/sound/simple-card.txt | 85 ++++++++++++ sound/soc/generic/simple-card.c | 146 +++++++++++++++++++- 2 files changed, 226 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt new file mode 100644 index 0000000..75bbc5a --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -0,0 +1,85 @@ +Simple-Card:
It's really difficult to review this without a description. Please explain what this binding represents.
What hardware is this applicable to?
Does this inherit from some class of binding?
+Required properties:
+- compatible : "simple-audio" +- simple-audio,card-name : simple-audio card name
What's this used for?
+- simple-audio,cpu : CPU sub-node, see below +- simple-audio,codec : CODEC sub-node, see below
Describe these as required subnodes. Nodes are not properties.
+Optional properties:
+- simple-audio,format : CPU/CODEC common format, see below
+Required cpu/codec subnode properties:
+- sound-dai : phandle and port for CPU/CODEC +- #sound-dai-cells : sound-dai phandle's node
This description makes no sense, the organisation seems structurally wrong.
What does this mean? What does it affect?
+Optional subnode properties:
+- format : specific format if needed, see below +- frame-master : frame master +- bitclock-master : bitclock master +- bitclock-inversion : clock inversion +- frame-inversion : frame inversion
What do these mean? Repeating the name without a dash is completely unhelpful. Describe what these imply.
What type are they?
Which subnode(s) do they apply to?
+- clocks : phandle for system clock rate
Just one clock?
Nit: clocks are specified with a phandle + clock-specifier pair, not just a phandle.
+- system-clock-frequency : system clock rate
it will overwrite clocks's rate
This seems very odd.
Why do you want to overwrite a clock's rate?
+simple-audio,format
- "i2s"
- "right_j"
- "left_j"
- "dsp_a"
- "dsp_b"
- "ac97"
- "pdm"
- "msb"
- "lsb"
What do these mean? Why are they not described when the property was defined above?
Does this also apply for the "format" property?
+Example:
+clock {
- osc: oscillator {
#clock-cells = <0>;compatible = "fixed-clock";clock-frequency = <11289600>;- };
+};
+sound {
- compatible = "simple-audio";
- simple-audio,card-name = "FSI2A-AK4648";
- simple-audio,format = "left_j";
- simple-audio,cpu {
sound-dai = <&sh_fsi2 0>;- };
- simple-audio,codec {
sound-dai = <&ak4648>;bitclock-master;frame-master;clocks = <&osc>;/* it can use this instead of clocks* system-clock-frequency = <11289600>; */
This just confuses matters. If ou want to show this off, have two examples.
- };
+};
+&i2c0 {
- ak4648: ak4648@0x12 {
Nit: remove the 0x on the unit-address
#sound-dai-cells = <0>;compatible = "asahi-kasei,ak4648";reg = <0x12>;- };
+};
+sh_fsi2: sh_fsi2@0xec230000 {
Again, fix the unit-address please.
- #sound-dai-cells = <1>;
- compatible = "renesas,sh_fsi2";
- reg = <0xec230000 0x400>;
- interrupt-parent = <&gic>;
- interrupts = <0 146 0x4>;
+}; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 8c49147..62befbd 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -9,6 +9,8 @@
- published by the Free Software Foundation.
*/
+#include <linux/clk.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/module.h> #include <sound/simple_card.h> @@ -52,11 +54,135 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd) return 0; }
+static int +__asoc_simple_card_parse_of(struct device_node *np,
struct asoc_simple_dai *dai,struct device_node **node)+{
- struct clk *clk;
- int ret;
- /*
* 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);
- if (!*node)
return -ENODEV;- of_node_put(*node);
Why?
You're refrering to it, so why do you not want to have it refcounted?
It could disappear under your feet.
- /* get dai->name */
- ret = snd_soc_of_get_dai_name(np, &dai->name);
- if (ret < 0)
return ret;- /*
* bitclock-inversion, frame-inversion* bitclock-master, frame-master* and specific "format" if it has*/- dai->fmt = snd_soc_of_parse_daifmt(np, NULL);
- /* dai->sysclk via "clolks = <xxx>" */
- clk = of_clk_get(np, 0);
- if (IS_ERR(clk))
dai->sysclk = 0;- else
dai->sysclk = clk_get_rate(clk);
This seems like an odd assumption to make. Why not error?
- /*
* overwrite dai->sysclk if it has* "system-clock-frequency = <xxx>"*/- of_property_read_u32(np,
"system-clock-frequency",&dai->sysclk);
Is dai->sysclk defined as a u32?
- return 0;
+}
+static int asoc_simple_card_parse_of(struct device_node *node,
struct asoc_simple_card_info *info,struct device *dev,struct device_node **of_cpu,struct device_node **of_codec,struct device_node **of_platform)+{
- struct device_node *np;
- int ret = 0;
- /* get card name */
- of_property_read_string(node, "simple-audio,card-name", &info->card);
- info->name = info->card;
What if the string is not in the DT?
Does the core code handle these being NULL?
- /* get CPU/CODEC common format via simple-audio,format */
- info->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio,") &
(SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);- /* CPU/CODEC sub-node */
- for_each_child_of_node(node, np) {
if (0 == strcmp("simple-audio,cpu", np->name))ret = __asoc_simple_card_parse_of(np,&info->cpu_dai,of_cpu);if (0 == strcmp("simple-audio,codec", np->name))ret = __asoc_simple_card_parse_of(np,&info->codec_dai,of_codec);
of_get_child_by_name?
if (ret < 0)return ret;- }
What if there were no children?
- /* simple-card assumes platform == cpu */
- *of_platform = *of_cpu;
- dev_dbg(dev, "card-name : %s\n", info->card);
- dev_dbg(dev, "platform : %04x / %p\n",
info->daifmt,*of_platform);
Why is the pointer helpful, rather than the info it points to?
Thanks, Mark.
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v3 -> v4
- explain detail of each properties on simple-card.txt - fixup odd examples on simple-card.txt - remove "simple-card,card-name". create it from cpu/codec name - use of_get_child_by_name() - remove odd pointer info from dev_dbg() - remove subnode format which are no longer needed
This is based on asoc/topic/simple branch
.../devicetree/bindings/sound/simple-card.txt | 73 +++++++++ sound/soc/generic/simple-card.c | 156 +++++++++++++++++++- 2 files changed, 223 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt new file mode 100644 index 0000000..4871e91 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -0,0 +1,73 @@ +Simple-Card: + +Simple-Card specifies audio DAI connection of SoC <-> codec. + +Required properties: + +- compatible : "simple-audio" + +Optional properties: + +- simple-audio,format : CPU/CODEC common audio format. + "i2s", "right_j", "left_j" , "dsp_a" + "dsp_b", "ac97", "pdm", "msb", "lsb" +Required subnodes: + +- simple-audio,cpu : CPU sub-node +- simple-audio,codec : CODEC sub-node + +Required CPU/CODEC subnodes properties: + +- sound-dai : phandle and port of CPU/CODEC + +Optional CPU/CODEC subnodes properties: + +- frame-master : bool property. add this if subnode was frame master +- bitclock-master : bool property. add this if subnode was bitclock master +- bitclock-inversion : bool property. add this if subnode has clock inversion +- frame-inversion : bool property. add this if subnode has frame inversion +- clocks / system-clock-frequency : specify subnode's clock if needed. + it can be specified via "clocks" if system has clock node, + or "system-clock-frequency" if system doesn't have it. + +Example: + +clock { + osc: oscillator { + #clock-cells = <0>; + compatible = "fixed-clock"; + clock-frequency = <11289600>; + }; +}; + +sound { + compatible = "simple-audio"; + simple-audio,format = "left_j"; + + simple-audio,cpu { + sound-dai = <&sh_fsi2 0>; + }; + + simple-audio,codec { + sound-dai = <&ak4648>; + bitclock-master; + frame-master; + clocks = <&osc>; + }; +}; + +&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>; +}; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index b2fbb70..c0fb635 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -8,7 +8,8 @@ * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ - +#include <linux/clk.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/module.h> #include <sound/simple_card.h> @@ -57,11 +58,144 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd) return 0; }
+static int +asoc_simple_card_sub_parse_of(struct device_node *np, + struct asoc_simple_dai *dai, + struct device_node **node) +{ + struct clk *clk; + int ret; + + /* + * 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); + if (!*node) + return -ENODEV; + + /* get dai->name */ + ret = snd_soc_of_get_dai_name(np, &dai->name); + if (ret < 0) + goto parse_error; + + /* + * bitclock-inversion, frame-inversion + * bitclock-master, frame-master + * and specific "format" if it has + */ + dai->fmt = snd_soc_of_parse_daifmt(np, NULL); + + /* + * dai->sysclk come from + * "clolks = <&xxx>" or "system-clock-frequency = <xxx>" + */ + clk = of_clk_get(np, 0); + if (IS_ERR(clk)) + of_property_read_u32(np, + "system-clock-frequency", + &dai->sysclk); + else + dai->sysclk = clk_get_rate(clk); + + ret = 0; + +parse_error: + of_node_put(*node); + + return ret; +} + +static int asoc_simple_card_parse_of(struct device_node *node, + struct asoc_simple_card_info *info, + struct device *dev, + struct device_node **of_cpu, + struct device_node **of_codec, + struct device_node **of_platform) +{ + struct device_node *np; + char *name; + int ret = 0; + + /* get CPU/CODEC common format via simple-audio,format */ + info->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio,") & + (SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK); + + /* CPU sub-node */ + ret = -EINVAL; + np = of_get_child_by_name(node, "simple-audio,cpu"); + if (np) + ret = asoc_simple_card_sub_parse_of(np, + &info->cpu_dai, + of_cpu); + if (ret < 0) + return ret; + + /* CODEC sub-node */ + ret = -EINVAL; + np = of_get_child_by_name(node, "simple-audio,codec"); + if (np) + ret = asoc_simple_card_sub_parse_of(np, + &info->codec_dai, + of_codec); + if (ret < 0) + return ret; + + /* card name is created from CPU/CODEC dai name */ + of_property_read_string(node, "simple-audio,card-name", &info->card); + name = devm_kzalloc(dev, + strlen(info->cpu_dai.name) + + strlen(info->codec_dai.name) + 2, + GFP_KERNEL); + sprintf(name, "%s-%s", info->cpu_dai.name, info->codec_dai.name); + info->name = info->card = name; + + /* simple-card assumes platform == cpu */ + *of_platform = *of_cpu; + + dev_dbg(dev, "card-name : %s\n", info->card); + dev_dbg(dev, "platform : %04x\n", info->daifmt); + dev_dbg(dev, "cpu : %s / %04x / %d\n", + info->cpu_dai.name, + info->cpu_dai.fmt, + info->cpu_dai.sysclk); + dev_dbg(dev, "codec : %s / %04x / %d\n", + info->codec_dai.name, + info->codec_dai.fmt, + info->codec_dai.sysclk); + + return 0; +} + static int asoc_simple_card_probe(struct platform_device *pdev) { - struct asoc_simple_card_info *cinfo = pdev->dev.platform_data; + struct asoc_simple_card_info *cinfo; + struct device_node *np = pdev->dev.of_node; + struct device_node *of_cpu, *of_codec, *of_platform; struct device *dev = &pdev->dev;
+ cinfo = NULL; + of_cpu = NULL; + of_codec = NULL; + of_platform = NULL; + if (np && of_device_is_available(np)) { + cinfo = devm_kzalloc(dev, sizeof(*cinfo), GFP_KERNEL); + if (cinfo) { + int ret; + ret = asoc_simple_card_parse_of(np, cinfo, dev, + &of_cpu, + &of_codec, + &of_platform); + if (ret < 0) { + if (ret != -EPROBE_DEFER) + dev_err(dev, "parse error %d\n", ret); + return ret; + } + } + } else { + cinfo = pdev->dev.platform_data; + } + if (!cinfo) { dev_err(dev, "no info for asoc-simple-card\n"); return -EINVAL; @@ -69,10 +203,10 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
if (!cinfo->name || !cinfo->card || - !cinfo->codec || - !cinfo->platform || - !cinfo->cpu_dai.name || - !cinfo->codec_dai.name) { + !cinfo->codec_dai.name || + !(cinfo->codec || of_codec) || + !(cinfo->platform || of_platform) || + !(cinfo->cpu_dai.name || of_cpu)) { dev_err(dev, "insufficient asoc_simple_card_info settings\n"); return -EINVAL; } @@ -86,6 +220,9 @@ static int asoc_simple_card_probe(struct platform_device *pdev) cinfo->snd_link.platform_name = cinfo->platform; cinfo->snd_link.codec_name = cinfo->codec; cinfo->snd_link.codec_dai_name = cinfo->codec_dai.name; + cinfo->snd_link.cpu_of_node = of_cpu; + cinfo->snd_link.codec_of_node = of_codec; + cinfo->snd_link.platform_of_node = of_platform; cinfo->snd_link.init = asoc_simple_card_dai_init;
/* @@ -107,10 +244,17 @@ static int asoc_simple_card_remove(struct platform_device *pdev) return snd_soc_unregister_card(&cinfo->snd_card); }
+static const struct of_device_id asoc_simple_of_match[] = { + { .compatible = "simple-audio", }, + {}, +}; +MODULE_DEVICE_TABLE(of, asoc_simple_of_match); + static struct platform_driver asoc_simple_card = { .driver = { .name = "asoc-simple-card", .owner = THIS_MODULE, + .of_match_table = asoc_simple_of_match, }, .probe = asoc_simple_card_probe, .remove = asoc_simple_card_remove,
Hi
I would like to know the current status of this patch
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
v3 -> v4
- explain detail of each properties on simple-card.txt
- fixup odd examples on simple-card.txt
- remove "simple-card,card-name". create it from cpu/codec name
- use of_get_child_by_name()
- remove odd pointer info from dev_dbg()
- remove subnode format which are no longer needed
This is based on asoc/topic/simple branch
.../devicetree/bindings/sound/simple-card.txt | 73 +++++++++ sound/soc/generic/simple-card.c | 156 +++++++++++++++++++- 2 files changed, 223 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt new file mode 100644 index 0000000..4871e91 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -0,0 +1,73 @@ +Simple-Card:
+Simple-Card specifies audio DAI connection of SoC <-> codec.
+Required properties:
+- compatible : "simple-audio"
+Optional properties:
+- simple-audio,format : CPU/CODEC common audio format.
"i2s", "right_j", "left_j" , "dsp_a""dsp_b", "ac97", "pdm", "msb", "lsb"+Required subnodes:
+- simple-audio,cpu : CPU sub-node +- simple-audio,codec : CODEC sub-node
+Required CPU/CODEC subnodes properties:
+- sound-dai : phandle and port of CPU/CODEC
+Optional CPU/CODEC subnodes properties:
+- frame-master : bool property. add this if subnode was frame master +- bitclock-master : bool property. add this if subnode was bitclock master +- bitclock-inversion : bool property. add this if subnode has clock inversion +- frame-inversion : bool property. add this if subnode has frame inversion +- clocks / system-clock-frequency : specify subnode's clock if needed.
it can be specified via "clocks" if system has clock node,or "system-clock-frequency" if system doesn't have it.+Example:
+clock {
- osc: oscillator {
#clock-cells = <0>;compatible = "fixed-clock";clock-frequency = <11289600>;- };
+};
+sound {
- compatible = "simple-audio";
- simple-audio,format = "left_j";
- simple-audio,cpu {
sound-dai = <&sh_fsi2 0>;- };
- simple-audio,codec {
sound-dai = <&ak4648>;bitclock-master;frame-master;clocks = <&osc>;- };
+};
+&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>;
+}; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index b2fbb70..c0fb635 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -8,7 +8,8 @@
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
*/
+#include <linux/clk.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/module.h> #include <sound/simple_card.h> @@ -57,11 +58,144 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd) return 0; }
+static int +asoc_simple_card_sub_parse_of(struct device_node *np,
struct asoc_simple_dai *dai,struct device_node **node)+{
- struct clk *clk;
- int ret;
- /*
* 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);
- if (!*node)
return -ENODEV;- /* get dai->name */
- ret = snd_soc_of_get_dai_name(np, &dai->name);
- if (ret < 0)
goto parse_error;- /*
* bitclock-inversion, frame-inversion* bitclock-master, frame-master* and specific "format" if it has*/- dai->fmt = snd_soc_of_parse_daifmt(np, NULL);
- /*
* dai->sysclk come from* "clolks = <&xxx>" or "system-clock-frequency = <xxx>"*/- clk = of_clk_get(np, 0);
- if (IS_ERR(clk))
of_property_read_u32(np,"system-clock-frequency",&dai->sysclk);- else
dai->sysclk = clk_get_rate(clk);- ret = 0;
+parse_error:
- of_node_put(*node);
- return ret;
+}
+static int asoc_simple_card_parse_of(struct device_node *node,
struct asoc_simple_card_info *info,struct device *dev,struct device_node **of_cpu,struct device_node **of_codec,struct device_node **of_platform)+{
- struct device_node *np;
- char *name;
- int ret = 0;
- /* get CPU/CODEC common format via simple-audio,format */
- info->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio,") &
(SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);- /* CPU sub-node */
- ret = -EINVAL;
- np = of_get_child_by_name(node, "simple-audio,cpu");
- if (np)
ret = asoc_simple_card_sub_parse_of(np,&info->cpu_dai,of_cpu);- if (ret < 0)
return ret;- /* CODEC sub-node */
- ret = -EINVAL;
- np = of_get_child_by_name(node, "simple-audio,codec");
- if (np)
ret = asoc_simple_card_sub_parse_of(np,&info->codec_dai,of_codec);- if (ret < 0)
return ret;- /* card name is created from CPU/CODEC dai name */
- of_property_read_string(node, "simple-audio,card-name", &info->card);
- name = devm_kzalloc(dev,
strlen(info->cpu_dai.name) +strlen(info->codec_dai.name) + 2,GFP_KERNEL);- sprintf(name, "%s-%s", info->cpu_dai.name, info->codec_dai.name);
- info->name = info->card = name;
- /* simple-card assumes platform == cpu */
- *of_platform = *of_cpu;
- dev_dbg(dev, "card-name : %s\n", info->card);
- dev_dbg(dev, "platform : %04x\n", info->daifmt);
- dev_dbg(dev, "cpu : %s / %04x / %d\n",
info->cpu_dai.name,info->cpu_dai.fmt,info->cpu_dai.sysclk);- dev_dbg(dev, "codec : %s / %04x / %d\n",
info->codec_dai.name,info->codec_dai.fmt,info->codec_dai.sysclk);- return 0;
+}
static int asoc_simple_card_probe(struct platform_device *pdev) {
- struct asoc_simple_card_info *cinfo = pdev->dev.platform_data;
struct asoc_simple_card_info *cinfo;
struct device_node *np = pdev->dev.of_node;
struct device_node *of_cpu, *of_codec, *of_platform; struct device *dev = &pdev->dev;
cinfo = NULL;
of_cpu = NULL;
of_codec = NULL;
of_platform = NULL;
if (np && of_device_is_available(np)) {
cinfo = devm_kzalloc(dev, sizeof(*cinfo), GFP_KERNEL);if (cinfo) {int ret;ret = asoc_simple_card_parse_of(np, cinfo, dev,&of_cpu,&of_codec,&of_platform);if (ret < 0) {if (ret != -EPROBE_DEFER)dev_err(dev, "parse error %d\n", ret);return ret;}}} else {
cinfo = pdev->dev.platform_data;}
if (!cinfo) { dev_err(dev, "no info for asoc-simple-card\n"); return -EINVAL;
@@ -69,10 +203,10 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
if (!cinfo->name || !cinfo->card ||
!cinfo->codec ||!cinfo->platform ||!cinfo->cpu_dai.name ||!cinfo->codec_dai.name) {
!cinfo->codec_dai.name ||!(cinfo->codec || of_codec) ||!(cinfo->platform || of_platform) ||!(cinfo->cpu_dai.name || of_cpu)) {@@ -86,6 +220,9 @@ static int asoc_simple_card_probe(struct platform_device *pdev) cinfo->snd_link.platform_name = cinfo->platform; cinfo->snd_link.codec_name = cinfo->codec; cinfo->snd_link.codec_dai_name = cinfo->codec_dai.name;
cinfo->snd_link.cpu_of_node = of_cpu;
cinfo->snd_link.codec_of_node = of_codec;
cinfo->snd_link.platform_of_node = of_platform; cinfo->snd_link.init = asoc_simple_card_dai_init;
/*
@@ -107,10 +244,17 @@ static int asoc_simple_card_remove(struct platform_device *pdev) return snd_soc_unregister_card(&cinfo->snd_card); }
+static const struct of_device_id asoc_simple_of_match[] = {
- { .compatible = "simple-audio", },
- {},
+}; +MODULE_DEVICE_TABLE(of, asoc_simple_of_match);
static struct platform_driver asoc_simple_card = { .driver = { .name = "asoc-simple-card", .owner = THIS_MODULE,
.of_match_table = asoc_simple_of_match,-- 1.7.9.5
On Fri, Oct 25, 2013 at 03:14:20AM +0100, Kuninori Morimoto wrote:
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
v3 -> v4
- explain detail of each properties on simple-card.txt
- fixup odd examples on simple-card.txt
- remove "simple-card,card-name". create it from cpu/codec name
- use of_get_child_by_name()
- remove odd pointer info from dev_dbg()
- remove subnode format which are no longer needed
This is based on asoc/topic/simple branch
.../devicetree/bindings/sound/simple-card.txt | 73 +++++++++ sound/soc/generic/simple-card.c | 156 +++++++++++++++++++- 2 files changed, 223 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt new file mode 100644 index 0000000..4871e91 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -0,0 +1,73 @@ +Simple-Card:
+Simple-Card specifies audio DAI connection of SoC <-> codec.
+Required properties:
+- compatible : "simple-audio"
+Optional properties:
+- simple-audio,format : CPU/CODEC common audio format.
"i2s", "right_j", "left_j" , "dsp_a""dsp_b", "ac97", "pdm", "msb", "lsb"+Required subnodes:
+- simple-audio,cpu : CPU sub-node +- simple-audio,codec : CODEC sub-node
+Required CPU/CODEC subnodes properties:
+- sound-dai : phandle and port of CPU/CODEC
Is there a class binding for audio devices this derives from?
+Optional CPU/CODEC subnodes properties:
Do these all apply to both sub-nodes?
+- frame-master : bool property. add this if subnode was frame master +- bitclock-master : bool property. add this if subnode was bitclock master
s/was/is/
+- bitclock-inversion : bool property. add this if subnode has clock inversion +- frame-inversion : bool property. add this if subnode has frame inversion +- clocks / system-clock-frequency : specify subnode's clock if needed.
it can be specified via "clocks" if system has clock node,or "system-clock-frequency" if system doesn't have it.
What does "if system doesn't have it" mean? If it doesn't have a clock, how does said non-existent clock have a frequency?
It would be possible to use a fixed-clock in place of system-clock-frequency, which would make the binding more consistent and the driver simpler, at the cost of making the dt marginally more complex.
+Example:
+clock {
Why is this container here? It's confusing and unnecessary.
- osc: oscillator {
#clock-cells = <0>;compatible = "fixed-clock";clock-frequency = <11289600>;- };
+};
[...]
+static int +asoc_simple_card_sub_parse_of(struct device_node *np,
struct asoc_simple_dai *dai,struct device_node **node)+{
- struct clk *clk;
- int ret;
- /*
* 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);
- if (!*node)
return -ENODEV;- /* get dai->name */
- ret = snd_soc_of_get_dai_name(np, &dai->name);
- if (ret < 0)
goto parse_error;- /*
* bitclock-inversion, frame-inversion* bitclock-master, frame-master* and specific "format" if it has*/
This comment looks confusing to me. I'm not sure it's all that helpful.
- dai->fmt = snd_soc_of_parse_daifmt(np, NULL);
As a general note, I'm surprised there isn't a helper that does all of the above, from of_parse_phandle to here.
- /*
* dai->sysclk come from* "clolks = <&xxx>" or "system-clock-frequency = <xxx>"
s/clolks/clocks/
*/- clk = of_clk_get(np, 0);
- if (IS_ERR(clk))
of_property_read_u32(np,"system-clock-frequency",&dai->sysclk);
What it this isn't present?
- else
dai->sysclk = clk_get_rate(clk);- ret = 0;
+parse_error:
- of_node_put(*node);
- return ret;
+}
+static int asoc_simple_card_parse_of(struct device_node *node,
struct asoc_simple_card_info *info,struct device *dev,struct device_node **of_cpu,struct device_node **of_codec,struct device_node **of_platform)+{
- struct device_node *np;
- char *name;
- int ret = 0;
- /* get CPU/CODEC common format via simple-audio,format */
- info->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio,") &
(SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);- /* CPU sub-node */
- ret = -EINVAL;
- np = of_get_child_by_name(node, "simple-audio,cpu");
- if (np)
ret = asoc_simple_card_sub_parse_of(np,&info->cpu_dai,of_cpu);- if (ret < 0)
return ret;- /* CODEC sub-node */
- ret = -EINVAL;
- np = of_get_child_by_name(node, "simple-audio,codec");
- if (np)
ret = asoc_simple_card_sub_parse_of(np,&info->codec_dai,of_codec);- if (ret < 0)
return ret;- /* card name is created from CPU/CODEC dai name */
- of_property_read_string(node, "simple-audio,card-name", &info->card);
- name = devm_kzalloc(dev,
strlen(info->cpu_dai.name) +strlen(info->codec_dai.name) + 2,GFP_KERNEL);- sprintf(name, "%s-%s", info->cpu_dai.name, info->codec_dai.name);
- info->name = info->card = name;
- /* simple-card assumes platform == cpu */
- *of_platform = *of_cpu;
Why? What does this imply?
Thanks, Mark.
Hi Mark Rutland
Thank you for your feedback
+- frame-master : bool property. add this if subnode was frame master +- bitclock-master : bool property. add this if subnode was bitclock master
s/was/is/
Oops, I will fix it on v5
+- bitclock-inversion : bool property. add this if subnode has clock inversion +- frame-inversion : bool property. add this if subnode has frame inversion +- clocks / system-clock-frequency : specify subnode's clock if needed.
it can be specified via "clocks" if system has clock node,or "system-clock-frequency" if system doesn't have it.What does "if system doesn't have it" mean? If it doesn't have a clock, how does said non-existent clock have a frequency?
It means "if system doesn't support common clock". I will fix it
+static int +asoc_simple_card_sub_parse_of(struct device_node *np,
struct asoc_simple_dai *dai,struct device_node **node)+{
- struct clk *clk;
- int ret;
- /*
* 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);
- if (!*node)
return -ENODEV;- /* get dai->name */
- ret = snd_soc_of_get_dai_name(np, &dai->name);
- if (ret < 0)
goto parse_error;- /*
* bitclock-inversion, frame-inversion* bitclock-master, frame-master* and specific "format" if it has*/This comment looks confusing to me. I'm not sure it's all that helpful.
- dai->fmt = snd_soc_of_parse_daifmt(np, NULL);
As a general note, I'm surprised there isn't a helper that does all of the above, from of_parse_phandle to here.
snd_soc_of_parse_daifmt() is the helper fucntion, and above comment is explaining it. Or, am I misunderstanding your review ?
- /*
* dai->sysclk come from* "clolks = <&xxx>" or "system-clock-frequency = <xxx>"s/clolks/clocks/
Grr, thank you
- clk = of_clk_get(np, 0);
- if (IS_ERR(clk))
of_property_read_u32(np,"system-clock-frequency",&dai->sysclk);What it this isn't present?
If sysclk doesn't have common clock support
- /* simple-card assumes platform == cpu */
- *of_platform = *of_cpu;
Why? What does this imply?
This is the one of reason why this driver is "simple" card
Best regards --- Kuninori Morimoto
On Mon, Nov 18, 2013 at 12:42:23AM +0000, Kuninori Morimoto wrote:
Hi Mark Rutland
Thank you for your feedback
+- frame-master : bool property. add this if subnode was frame master +- bitclock-master : bool property. add this if subnode was bitclock master
s/was/is/
Oops, I will fix it on v5
+- bitclock-inversion : bool property. add this if subnode has clock inversion +- frame-inversion : bool property. add this if subnode has frame inversion +- clocks / system-clock-frequency : specify subnode's clock if needed.
it can be specified via "clocks" if system has clock node,or "system-clock-frequency" if system doesn't have it.What does "if system doesn't have it" mean? If it doesn't have a clock, how does said non-existent clock have a frequency?
It means "if system doesn't support common clock". I will fix it
When you say "doesn't support common clock", you mean the code for that platform is incompatible with the common clock framework? It seems very messy to allow a Linux-internal implementation detail (which is expected to change) to leak into a binding...
+static int +asoc_simple_card_sub_parse_of(struct device_node *np,
struct asoc_simple_dai *dai,struct device_node **node)+{
- struct clk *clk;
- int ret;
- /*
* 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);
- if (!*node)
return -ENODEV;- /* get dai->name */
- ret = snd_soc_of_get_dai_name(np, &dai->name);
- if (ret < 0)
goto parse_error;- /*
* bitclock-inversion, frame-inversion* bitclock-master, frame-master* and specific "format" if it has*/This comment looks confusing to me. I'm not sure it's all that helpful.
- dai->fmt = snd_soc_of_parse_daifmt(np, NULL);
As a general note, I'm surprised there isn't a helper that does all of the above, from of_parse_phandle to here.
snd_soc_of_parse_daifmt() is the helper fucntion, and above comment is explaining it. Or, am I misunderstanding your review ?
What I meant was that I am surprised there isn't a helper doing all of: * of_parse_phandle * snd_soc_of_get_dai_name * snd_soc_of_parse_daifmt
It looks like a common pattern that could be factored out.
- /*
* dai->sysclk come from* "clolks = <&xxx>" or "system-clock-frequency = <xxx>"s/clolks/clocks/
Grr, thank you
- clk = of_clk_get(np, 0);
- if (IS_ERR(clk))
of_property_read_u32(np,"system-clock-frequency",&dai->sysclk);What it this isn't present?
If sysclk doesn't have common clock support
Huh? That's not what I asked.
What if the dt has neither a clock or a system-clock-frequency property?
- /* simple-card assumes platform == cpu */
- *of_platform = *of_cpu;
Why? What does this imply?
This is the one of reason why this driver is "simple" card
The issue here is that I'm almost totally unfamiliar with audio hardware, nor the ASoC bindings. If this makes sense to you and Mark Brown then I'm happy to continue not understanding either. :)
Thanks, Mark.
On Mon, Nov 18, 2013 at 11:36:18AM +0000, Mark Rutland wrote:
What I meant was that I am surprised there isn't a helper doing all of:
- of_parse_phandle
- snd_soc_of_get_dai_name
- snd_soc_of_parse_daifmt
It looks like a common pattern that could be factored out.
This is the factoring out of the common pattern, other drivers will only need the compatible string to work out the format.
Hi Mark Rutland
Thank you for your feedback
It means "if system doesn't support common clock". I will fix it
When you say "doesn't support common clock", you mean the code for that platform is incompatible with the common clock framework? It seems very messy to allow a Linux-internal implementation detail (which is expected to change) to leak into a binding...
Some CPU doesn't support common clock, like PowerPC (?) This is Mark (Brown) comment
-------------------- So, ideally. However we have to consider the fact that the clock API isn't reliably available makes this harder than it should be. Even among the DT using platforms at least PowerPC still uses a custom clock API. We could just use this as a carrot to push people to convert though. ---------------------
of_property_read_u32(np,"system-clock-frequency",&dai->sysclk);What it this isn't present?
If sysclk doesn't have common clock support
Huh? That's not what I asked.
What if the dt has neither a clock or a system-clock-frequency property?
OK, sorry for my English
This happen if cpu/codec was slave, not strange things. Please check "Example". in there, "simple-audio,codec" has clocks but, "simple-audio,cpu" doesn't have it, and "simple-audio,codec" has "bitclock-master" and "frame-master"; This case, codec chip creates audio play/capture clock from system clock, and "cpu" use it. This is the reason why the name is "system-clock-frequency" instead of "clock-frequency" The image is like this
clocks / +- simple card --+ system clock | |-> playback -------------+-[codec]--[cpu] | | |<- capture +----------------+
Best regards --- Kuninori Morimoto
On Tue, Nov 19, 2013 at 02:03:21AM +0000, Kuninori Morimoto wrote:
Hi Mark Rutland
Thank you for your feedback
It means "if system doesn't support common clock". I will fix it
When you say "doesn't support common clock", you mean the code for that platform is incompatible with the common clock framework? It seems very messy to allow a Linux-internal implementation detail (which is expected to change) to leak into a binding...
Some CPU doesn't support common clock, like PowerPC (?) This is Mark (Brown) comment
So, ideally. However we have to consider the fact that the clock API isn't reliably available makes this harder than it should be. Even among the DT using platforms at least PowerPC still uses a custom clock API. We could just use this as a carrot to push people to convert though.
I would be happier if we could unify the common clock infrastructure, it would make this kind of thing a lot lessy messy. However, I'm not against the system-clock-frequency property for now.
of_property_read_u32(np,"system-clock-frequency",&dai->sysclk);What it this isn't present?
If sysclk doesn't have common clock support
Huh? That's not what I asked.
What if the dt has neither a clock or a system-clock-frequency property?
OK, sorry for my English
Sorry for the confusion, I'll try to be less ambiguous in future :)
What I was trying to get at here is that if there is neither a clock or a system-clock-frequency property in the device tree, dai->sysclk will not have been initialised in this path. Is this a valid case, and will dai->sysclk have a well-defined, sane value?
Thanks, Mark.
Hi Mark Rutland
So, ideally. However we have to consider the fact that the clock API isn't reliably available makes this harder than it should be. Even among the DT using platforms at least PowerPC still uses a custom clock API. We could just use this as a carrot to push people to convert though.
I would be happier if we could unify the common clock infrastructure, it would make this kind of thing a lot lessy messy. However, I'm not against the system-clock-frequency property for now.
Thank you
OK, sorry for my English
Sorry for the confusion, I'll try to be less ambiguous in future :)
What I was trying to get at here is that if there is neither a clock or a system-clock-frequency property in the device tree, dai->sysclk will not have been initialised in this path. Is this a valid case, and will dai->sysclk have a well-defined, sane value?
My understanding, this "dai" itself is created by devm_kzalloc() So, default dai->sysclk is 0. And, if there is no clocks, no system-clock-frequency property, it try of_property_read_u32() side. but it will do nothing to dai->sysclk in such case. so dai->sysclk is still 0, and it is very sane on this driver. Is this good answer ?
+ clk = of_clk_get(np, 0); + if (IS_ERR(clk)) + of_property_read_u32(np, + "system-clock-frequency", + &dai->sysclk); + else + dai->sysclk = clk_get_rate(clk);
Best regards --- Kuninori Morimoto
On Thu, Nov 21, 2013 at 12:12:13AM +0000, Kuninori Morimoto wrote:
Hi Mark Rutland
So, ideally. However we have to consider the fact that the clock API isn't reliably available makes this harder than it should be. Even among the DT using platforms at least PowerPC still uses a custom clock API. We could just use this as a carrot to push people to convert though.
I would be happier if we could unify the common clock infrastructure, it would make this kind of thing a lot lessy messy. However, I'm not against the system-clock-frequency property for now.
Thank you
OK, sorry for my English
Sorry for the confusion, I'll try to be less ambiguous in future :)
What I was trying to get at here is that if there is neither a clock or a system-clock-frequency property in the device tree, dai->sysclk will not have been initialised in this path. Is this a valid case, and will dai->sysclk have a well-defined, sane value?
My understanding, this "dai" itself is created by devm_kzalloc() So, default dai->sysclk is 0. And, if there is no clocks, no system-clock-frequency property, it try of_property_read_u32() side. but it will do nothing to dai->sysclk in such case. so dai->sysclk is still 0, and it is very sane on this driver. Is this good answer ?
That sounds fine to me. Just wanted to make sure. :)
Thanks, Mark.
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v4 -> v5
- fixup spell miss - removed un-needed "clock node" example from simple-card.txt - add explain that clocks can be used if system has "common clock"
.../devicetree/bindings/sound/simple-card.txt | 66 ++++++++ sound/soc/generic/simple-card.c | 157 +++++++++++++++++++- 2 files changed, 217 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt new file mode 100644 index 0000000..615a655 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -0,0 +1,66 @@ +Simple-Card: + +Simple-Card specifies audio DAI connection of SoC <-> codec. + +Required properties: + +- compatible : "simple-audio" + +Optional properties: + +- simple-audio,format : CPU/CODEC common audio format. + "i2s", "right_j", "left_j" , "dsp_a" + "dsp_b", "ac97", "pdm", "msb", "lsb" +Required subnodes: + +- simple-audio,cpu : CPU sub-node +- simple-audio,codec : CODEC sub-node + +Required CPU/CODEC subnodes properties: + +- sound-dai : phandle and port of CPU/CODEC + +Optional CPU/CODEC subnodes properties: + +- frame-master : bool property. add this if subnode is frame master +- bitclock-master : bool property. add this if subnode is bitclock master +- bitclock-inversion : bool property. add this if subnode has clock inversion +- frame-inversion : bool property. add this if subnode has frame inversion +- 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 can't use it. + +Example: + +sound { + compatible = "simple-audio"; + simple-audio,format = "left_j"; + + simple-audio,cpu { + sound-dai = <&sh_fsi2 0>; + }; + + simple-audio,codec { + sound-dai = <&ak4648>; + bitclock-master; + frame-master; + clocks = <&osc>; + }; +}; + +&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>; +}; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index b2fbb70..da1fd7e 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -8,7 +8,8 @@ * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ - +#include <linux/clk.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/module.h> #include <sound/simple_card.h> @@ -57,11 +58,145 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd) return 0; }
+static int +asoc_simple_card_sub_parse_of(struct device_node *np, + struct asoc_simple_dai *dai, + struct device_node **node) +{ + struct clk *clk; + int ret; + + /* + * 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); + if (!*node) + return -ENODEV; + + /* get dai->name */ + ret = snd_soc_of_get_dai_name(np, &dai->name); + if (ret < 0) + goto parse_error; + + /* + * bitclock-inversion, frame-inversion + * bitclock-master, frame-master + * and specific "format" if it has + */ + dai->fmt = snd_soc_of_parse_daifmt(np, NULL); + + /* + * dai->sysclk come from + * "clocks = <&xxx>" (if system has common clock) + * or "system-clock-frequency = <xxx>" + */ + clk = of_clk_get(np, 0); + if (IS_ERR(clk)) + of_property_read_u32(np, + "system-clock-frequency", + &dai->sysclk); + else + dai->sysclk = clk_get_rate(clk); + + ret = 0; + +parse_error: + of_node_put(*node); + + return ret; +} + +static int asoc_simple_card_parse_of(struct device_node *node, + struct asoc_simple_card_info *info, + struct device *dev, + struct device_node **of_cpu, + struct device_node **of_codec, + struct device_node **of_platform) +{ + struct device_node *np; + char *name; + int ret = 0; + + /* get CPU/CODEC common format via simple-audio,format */ + info->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio,") & + (SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK); + + /* CPU sub-node */ + ret = -EINVAL; + np = of_get_child_by_name(node, "simple-audio,cpu"); + if (np) + ret = asoc_simple_card_sub_parse_of(np, + &info->cpu_dai, + of_cpu); + if (ret < 0) + return ret; + + /* CODEC sub-node */ + ret = -EINVAL; + np = of_get_child_by_name(node, "simple-audio,codec"); + if (np) + ret = asoc_simple_card_sub_parse_of(np, + &info->codec_dai, + of_codec); + if (ret < 0) + return ret; + + /* card name is created from CPU/CODEC dai name */ + of_property_read_string(node, "simple-audio,card-name", &info->card); + name = devm_kzalloc(dev, + strlen(info->cpu_dai.name) + + strlen(info->codec_dai.name) + 2, + GFP_KERNEL); + sprintf(name, "%s-%s", info->cpu_dai.name, info->codec_dai.name); + info->name = info->card = name; + + /* simple-card assumes platform == cpu */ + *of_platform = *of_cpu; + + dev_dbg(dev, "card-name : %s\n", info->card); + dev_dbg(dev, "platform : %04x\n", info->daifmt); + dev_dbg(dev, "cpu : %s / %04x / %d\n", + info->cpu_dai.name, + info->cpu_dai.fmt, + info->cpu_dai.sysclk); + dev_dbg(dev, "codec : %s / %04x / %d\n", + info->codec_dai.name, + info->codec_dai.fmt, + info->codec_dai.sysclk); + + return 0; +} + static int asoc_simple_card_probe(struct platform_device *pdev) { - struct asoc_simple_card_info *cinfo = pdev->dev.platform_data; + struct asoc_simple_card_info *cinfo; + struct device_node *np = pdev->dev.of_node; + struct device_node *of_cpu, *of_codec, *of_platform; struct device *dev = &pdev->dev;
+ cinfo = NULL; + of_cpu = NULL; + of_codec = NULL; + of_platform = NULL; + if (np && of_device_is_available(np)) { + cinfo = devm_kzalloc(dev, sizeof(*cinfo), GFP_KERNEL); + if (cinfo) { + int ret; + ret = asoc_simple_card_parse_of(np, cinfo, dev, + &of_cpu, + &of_codec, + &of_platform); + if (ret < 0) { + if (ret != -EPROBE_DEFER) + dev_err(dev, "parse error %d\n", ret); + return ret; + } + } + } else { + cinfo = pdev->dev.platform_data; + } + if (!cinfo) { dev_err(dev, "no info for asoc-simple-card\n"); return -EINVAL; @@ -69,10 +204,10 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
if (!cinfo->name || !cinfo->card || - !cinfo->codec || - !cinfo->platform || - !cinfo->cpu_dai.name || - !cinfo->codec_dai.name) { + !cinfo->codec_dai.name || + !(cinfo->codec || of_codec) || + !(cinfo->platform || of_platform) || + !(cinfo->cpu_dai.name || of_cpu)) { dev_err(dev, "insufficient asoc_simple_card_info settings\n"); return -EINVAL; } @@ -86,6 +221,9 @@ static int asoc_simple_card_probe(struct platform_device *pdev) cinfo->snd_link.platform_name = cinfo->platform; cinfo->snd_link.codec_name = cinfo->codec; cinfo->snd_link.codec_dai_name = cinfo->codec_dai.name; + cinfo->snd_link.cpu_of_node = of_cpu; + cinfo->snd_link.codec_of_node = of_codec; + cinfo->snd_link.platform_of_node = of_platform; cinfo->snd_link.init = asoc_simple_card_dai_init;
/* @@ -107,10 +245,17 @@ static int asoc_simple_card_remove(struct platform_device *pdev) return snd_soc_unregister_card(&cinfo->snd_card); }
+static const struct of_device_id asoc_simple_of_match[] = { + { .compatible = "simple-audio", }, + {}, +}; +MODULE_DEVICE_TABLE(of, asoc_simple_of_match); + static struct platform_driver asoc_simple_card = { .driver = { .name = "asoc-simple-card", .owner = THIS_MODULE, + .of_match_table = asoc_simple_of_match, }, .probe = asoc_simple_card_probe, .remove = asoc_simple_card_remove,
Hi
I would like to know current status of this patch
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
v4 -> v5
- fixup spell miss
- removed un-needed "clock node" example from simple-card.txt
- add explain that clocks can be used if system has "common clock"
.../devicetree/bindings/sound/simple-card.txt | 66 ++++++++ sound/soc/generic/simple-card.c | 157 +++++++++++++++++++- 2 files changed, 217 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt new file mode 100644 index 0000000..615a655 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -0,0 +1,66 @@ +Simple-Card:
+Simple-Card specifies audio DAI connection of SoC <-> codec.
+Required properties:
+- compatible : "simple-audio"
+Optional properties:
+- simple-audio,format : CPU/CODEC common audio format.
"i2s", "right_j", "left_j" , "dsp_a""dsp_b", "ac97", "pdm", "msb", "lsb"+Required subnodes:
+- simple-audio,cpu : CPU sub-node +- simple-audio,codec : CODEC sub-node
+Required CPU/CODEC subnodes properties:
+- sound-dai : phandle and port of CPU/CODEC
+Optional CPU/CODEC subnodes properties:
+- frame-master : bool property. add this if subnode is frame master +- bitclock-master : bool property. add this if subnode is bitclock master +- bitclock-inversion : bool property. add this if subnode has clock inversion +- frame-inversion : bool property. add this if subnode has frame inversion +- clocks / system-clock-frequency : specify subnode's clock if needed.
it can be specified via "clocks" if system hasclock node (= common clock),or "system-clock-frequency" if system can't use it.+Example:
+sound {
- compatible = "simple-audio";
- simple-audio,format = "left_j";
- simple-audio,cpu {
sound-dai = <&sh_fsi2 0>;- };
- simple-audio,codec {
sound-dai = <&ak4648>;bitclock-master;frame-master;clocks = <&osc>;- };
+};
+&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>;
+}; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index b2fbb70..da1fd7e 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -8,7 +8,8 @@
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
*/
+#include <linux/clk.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/module.h> #include <sound/simple_card.h> @@ -57,11 +58,145 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd) return 0; }
+static int +asoc_simple_card_sub_parse_of(struct device_node *np,
struct asoc_simple_dai *dai,struct device_node **node)+{
- struct clk *clk;
- int ret;
- /*
* 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);
- if (!*node)
return -ENODEV;- /* get dai->name */
- ret = snd_soc_of_get_dai_name(np, &dai->name);
- if (ret < 0)
goto parse_error;- /*
* bitclock-inversion, frame-inversion* bitclock-master, frame-master* and specific "format" if it has*/- dai->fmt = snd_soc_of_parse_daifmt(np, NULL);
- /*
* dai->sysclk come from* "clocks = <&xxx>" (if system has common clock)* or "system-clock-frequency = <xxx>"*/- clk = of_clk_get(np, 0);
- if (IS_ERR(clk))
of_property_read_u32(np,"system-clock-frequency",&dai->sysclk);- else
dai->sysclk = clk_get_rate(clk);- ret = 0;
+parse_error:
- of_node_put(*node);
- return ret;
+}
+static int asoc_simple_card_parse_of(struct device_node *node,
struct asoc_simple_card_info *info,struct device *dev,struct device_node **of_cpu,struct device_node **of_codec,struct device_node **of_platform)+{
- struct device_node *np;
- char *name;
- int ret = 0;
- /* get CPU/CODEC common format via simple-audio,format */
- info->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio,") &
(SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);- /* CPU sub-node */
- ret = -EINVAL;
- np = of_get_child_by_name(node, "simple-audio,cpu");
- if (np)
ret = asoc_simple_card_sub_parse_of(np,&info->cpu_dai,of_cpu);- if (ret < 0)
return ret;- /* CODEC sub-node */
- ret = -EINVAL;
- np = of_get_child_by_name(node, "simple-audio,codec");
- if (np)
ret = asoc_simple_card_sub_parse_of(np,&info->codec_dai,of_codec);- if (ret < 0)
return ret;- /* card name is created from CPU/CODEC dai name */
- of_property_read_string(node, "simple-audio,card-name", &info->card);
- name = devm_kzalloc(dev,
strlen(info->cpu_dai.name) +strlen(info->codec_dai.name) + 2,GFP_KERNEL);- sprintf(name, "%s-%s", info->cpu_dai.name, info->codec_dai.name);
- info->name = info->card = name;
- /* simple-card assumes platform == cpu */
- *of_platform = *of_cpu;
- dev_dbg(dev, "card-name : %s\n", info->card);
- dev_dbg(dev, "platform : %04x\n", info->daifmt);
- dev_dbg(dev, "cpu : %s / %04x / %d\n",
info->cpu_dai.name,info->cpu_dai.fmt,info->cpu_dai.sysclk);- dev_dbg(dev, "codec : %s / %04x / %d\n",
info->codec_dai.name,info->codec_dai.fmt,info->codec_dai.sysclk);- return 0;
+}
static int asoc_simple_card_probe(struct platform_device *pdev) {
- struct asoc_simple_card_info *cinfo = pdev->dev.platform_data;
struct asoc_simple_card_info *cinfo;
struct device_node *np = pdev->dev.of_node;
struct device_node *of_cpu, *of_codec, *of_platform; struct device *dev = &pdev->dev;
cinfo = NULL;
of_cpu = NULL;
of_codec = NULL;
of_platform = NULL;
if (np && of_device_is_available(np)) {
cinfo = devm_kzalloc(dev, sizeof(*cinfo), GFP_KERNEL);if (cinfo) {int ret;ret = asoc_simple_card_parse_of(np, cinfo, dev,&of_cpu,&of_codec,&of_platform);if (ret < 0) {if (ret != -EPROBE_DEFER)dev_err(dev, "parse error %d\n", ret);return ret;}}} else {
cinfo = pdev->dev.platform_data;}
if (!cinfo) { dev_err(dev, "no info for asoc-simple-card\n"); return -EINVAL;
@@ -69,10 +204,10 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
if (!cinfo->name || !cinfo->card ||
!cinfo->codec ||!cinfo->platform ||!cinfo->cpu_dai.name ||!cinfo->codec_dai.name) {
!cinfo->codec_dai.name ||!(cinfo->codec || of_codec) ||!(cinfo->platform || of_platform) ||!(cinfo->cpu_dai.name || of_cpu)) {@@ -86,6 +221,9 @@ static int asoc_simple_card_probe(struct platform_device *pdev) cinfo->snd_link.platform_name = cinfo->platform; cinfo->snd_link.codec_name = cinfo->codec; cinfo->snd_link.codec_dai_name = cinfo->codec_dai.name;
cinfo->snd_link.cpu_of_node = of_cpu;
cinfo->snd_link.codec_of_node = of_codec;
cinfo->snd_link.platform_of_node = of_platform; cinfo->snd_link.init = asoc_simple_card_dai_init;
/*
@@ -107,10 +245,17 @@ static int asoc_simple_card_remove(struct platform_device *pdev) return snd_soc_unregister_card(&cinfo->snd_card); }
+static const struct of_device_id asoc_simple_of_match[] = {
- { .compatible = "simple-audio", },
- {},
+}; +MODULE_DEVICE_TABLE(of, asoc_simple_of_match);
static struct platform_driver asoc_simple_card = { .driver = { .name = "asoc-simple-card", .owner = THIS_MODULE,
.of_match_table = asoc_simple_of_match,-- 1.7.9.5
Best regards --- Kuninori Morimoto
On Fri, Nov 15, 2013 at 9:50 AM, Mark Rutland mark.rutland@arm.com wrote:
On Fri, Oct 25, 2013 at 03:14:20AM +0100, Kuninori Morimoto wrote:
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
v3 -> v4
- explain detail of each properties on simple-card.txt
- fixup odd examples on simple-card.txt
- remove "simple-card,card-name". create it from cpu/codec name
- use of_get_child_by_name()
- remove odd pointer info from dev_dbg()
- remove subnode format which are no longer needed
This is based on asoc/topic/simple branch
.../devicetree/bindings/sound/simple-card.txt | 73 +++++++++ sound/soc/generic/simple-card.c | 156 +++++++++++++++++++- 2 files changed, 223 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt new file mode 100644 index 0000000..4871e91 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -0,0 +1,73 @@ +Simple-Card:
+Simple-Card specifies audio DAI connection of SoC <-> codec.
+Required properties:
+- compatible : "simple-audio"
Still not really a fan of this generic name. Can we define in the description above what simple means.
+Optional properties:
+- simple-audio,format : CPU/CODEC common audio format.
"i2s", "right_j", "left_j" , "dsp_a""dsp_b", "ac97", "pdm", "msb", "lsb"+Required subnodes:
+- simple-audio,cpu : CPU sub-node +- simple-audio,codec : CODEC sub-node
+Required CPU/CODEC subnodes properties:
+- sound-dai : phandle and port of CPU/CODEC
Is there a class binding for audio devices this derives from?
+Optional CPU/CODEC subnodes properties:
Do these all apply to both sub-nodes?
+- frame-master : bool property. add this if subnode was frame master +- bitclock-master : bool property. add this if subnode was bitclock master
s/was/is/
+- bitclock-inversion : bool property. add this if subnode has clock inversion +- frame-inversion : bool property. add this if subnode has frame inversion +- clocks / system-clock-frequency : specify subnode's clock if needed.
it can be specified via "clocks" if system has clock node,or "system-clock-frequency" if system doesn't have it.What does "if system doesn't have it" mean? If it doesn't have a clock, how does said non-existent clock have a frequency?
It would be possible to use a fixed-clock in place of system-clock-frequency, which would make the binding more consistent and the driver simpler, at the cost of making the dt marginally more complex.
Just plain "clock-frequency" is fairly standard, so please use that instead. Unless there is need for a fixed-clock to be routed to several nodes, then I think using the more simple clock-frequency here is fine.
Can both sub-nodes really have different clocks? Seems like that would exceed the definition of simple.
Rob
On Mon, Nov 18, 2013 at 08:12:06AM -0600, Rob Herring wrote:
Can both sub-nodes really have different clocks? Seems like that would exceed the definition of simple.
In theory, though it would be unusual. What is more normal that the clock is only connected to one of the nodes and we need to know which it is.
Hi Rob, Mark
+Required properties:
+- compatible : "simple-audio"
Still not really a fan of this generic name. Can we define in the description above what simple means.
So, how about "simple-audio-card" ?
Best regards --- Kuninori Morimoto
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v4 -> v5
- fixup spell miss - removed un-needed "clock node" example from simple-card.txt - add explain that clocks can be used if system has "common clock"
v5 -> v6
- simple-audio => simple-audio-card
.../devicetree/bindings/sound/simple-card.txt | 68 +++++++++ sound/soc/generic/simple-card.c | 156 +++++++++++++++++++- 2 files changed, 218 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-card.txt
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt new file mode 100644 index 0000000..769a346 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -0,0 +1,68 @@ +Simple-Card: + +Simple-Card specifies audio DAI connection of SoC <-> codec. + +Required properties: + +- compatible : "simple-audio-card" + +Optional properties: + +- simple-audio-card,format : CPU/CODEC common audio format. + "i2s", "right_j", "left_j" , "dsp_a" + "dsp_b", "ac97", "pdm", "msb", "lsb" +Required subnodes: + +- simple-audio-card,cpu : CPU sub-node +- simple-audio-card,codec : CODEC sub-node + +Required CPU/CODEC subnodes properties: + +- sound-dai : phandle and port of CPU/CODEC + +Optional CPU/CODEC subnodes properties: + +- format : CPU/CODEC specific audio format if needed. + see simple-audio-card,format +- frame-master : bool property. add this if subnode is frame master +- bitclock-master : bool property. add this if subnode is bitclock master +- bitclock-inversion : bool property. add this if subnode has clock inversion +- frame-inversion : bool property. add this if subnode has frame inversion +- 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) + +Example: + +sound { + compatible = "simple-audio-card"; + simple-audio-card,format = "left_j"; + + simple-audio-card,cpu { + sound-dai = <&sh_fsi2 0>; + }; + + simple-audio-card,codec { + sound-dai = <&ak4648>; + bitclock-master; + frame-master; + clocks = <&osc>; + }; +}; + +&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>; +}; diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index b2fbb70..7a9b6b4 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -8,7 +8,8 @@ * it under the terms of the GNU General Public License version 2 as * published by the Free Software Foundation. */ - +#include <linux/clk.h> +#include <linux/of.h> #include <linux/platform_device.h> #include <linux/module.h> #include <sound/simple_card.h> @@ -57,11 +58,144 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd) return 0; }
+static int +asoc_simple_card_sub_parse_of(struct device_node *np, + struct asoc_simple_dai *dai, + struct device_node **node) +{ + struct clk *clk; + int ret; + + /* + * 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); + if (!*node) + return -ENODEV; + + /* get dai->name */ + ret = snd_soc_of_get_dai_name(np, &dai->name); + if (ret < 0) + goto parse_error; + + /* + * bitclock-inversion, frame-inversion + * bitclock-master, frame-master + * and specific "format" if it has + */ + dai->fmt = snd_soc_of_parse_daifmt(np, NULL); + + /* + * dai->sysclk come from + * "clocks = <&xxx>" (if system has common clock) + * or "system-clock-frequency = <xxx>" + */ + clk = of_clk_get(np, 0); + if (IS_ERR(clk)) + of_property_read_u32(np, + "system-clock-frequency", + &dai->sysclk); + else + dai->sysclk = clk_get_rate(clk); + + ret = 0; + +parse_error: + of_node_put(*node); + + return ret; +} + +static int asoc_simple_card_parse_of(struct device_node *node, + struct asoc_simple_card_info *info, + struct device *dev, + struct device_node **of_cpu, + struct device_node **of_codec, + struct device_node **of_platform) +{ + struct device_node *np; + char *name; + int ret = 0; + + /* get CPU/CODEC common format via simple-audio-card,format */ + info->daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,") & + (SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK); + + /* CPU sub-node */ + ret = -EINVAL; + np = of_get_child_by_name(node, "simple-audio-card,cpu"); + if (np) + ret = asoc_simple_card_sub_parse_of(np, + &info->cpu_dai, + of_cpu); + if (ret < 0) + return ret; + + /* CODEC sub-node */ + ret = -EINVAL; + np = of_get_child_by_name(node, "simple-audio-card,codec"); + if (np) + ret = asoc_simple_card_sub_parse_of(np, + &info->codec_dai, + of_codec); + if (ret < 0) + return ret; + + /* card name is created from CPU/CODEC dai name */ + name = devm_kzalloc(dev, + strlen(info->cpu_dai.name) + + strlen(info->codec_dai.name) + 2, + GFP_KERNEL); + sprintf(name, "%s-%s", info->cpu_dai.name, info->codec_dai.name); + info->name = info->card = name; + + /* simple-card assumes platform == cpu */ + *of_platform = *of_cpu; + + dev_dbg(dev, "card-name : %s\n", info->card); + dev_dbg(dev, "platform : %04x\n", info->daifmt); + dev_dbg(dev, "cpu : %s / %04x / %d\n", + info->cpu_dai.name, + info->cpu_dai.fmt, + info->cpu_dai.sysclk); + dev_dbg(dev, "codec : %s / %04x / %d\n", + info->codec_dai.name, + info->codec_dai.fmt, + info->codec_dai.sysclk); + + return 0; +} + static int asoc_simple_card_probe(struct platform_device *pdev) { - struct asoc_simple_card_info *cinfo = pdev->dev.platform_data; + struct asoc_simple_card_info *cinfo; + struct device_node *np = pdev->dev.of_node; + struct device_node *of_cpu, *of_codec, *of_platform; struct device *dev = &pdev->dev;
+ cinfo = NULL; + of_cpu = NULL; + of_codec = NULL; + of_platform = NULL; + if (np && of_device_is_available(np)) { + cinfo = devm_kzalloc(dev, sizeof(*cinfo), GFP_KERNEL); + if (cinfo) { + int ret; + ret = asoc_simple_card_parse_of(np, cinfo, dev, + &of_cpu, + &of_codec, + &of_platform); + if (ret < 0) { + if (ret != -EPROBE_DEFER) + dev_err(dev, "parse error %d\n", ret); + return ret; + } + } + } else { + cinfo = pdev->dev.platform_data; + } + if (!cinfo) { dev_err(dev, "no info for asoc-simple-card\n"); return -EINVAL; @@ -69,10 +203,10 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
if (!cinfo->name || !cinfo->card || - !cinfo->codec || - !cinfo->platform || - !cinfo->cpu_dai.name || - !cinfo->codec_dai.name) { + !cinfo->codec_dai.name || + !(cinfo->codec || of_codec) || + !(cinfo->platform || of_platform) || + !(cinfo->cpu_dai.name || of_cpu)) { dev_err(dev, "insufficient asoc_simple_card_info settings\n"); return -EINVAL; } @@ -86,6 +220,9 @@ static int asoc_simple_card_probe(struct platform_device *pdev) cinfo->snd_link.platform_name = cinfo->platform; cinfo->snd_link.codec_name = cinfo->codec; cinfo->snd_link.codec_dai_name = cinfo->codec_dai.name; + cinfo->snd_link.cpu_of_node = of_cpu; + cinfo->snd_link.codec_of_node = of_codec; + cinfo->snd_link.platform_of_node = of_platform; cinfo->snd_link.init = asoc_simple_card_dai_init;
/* @@ -107,10 +244,17 @@ static int asoc_simple_card_remove(struct platform_device *pdev) return snd_soc_unregister_card(&cinfo->snd_card); }
+static const struct of_device_id asoc_simple_of_match[] = { + { .compatible = "simple-audio-card", }, + {}, +}; +MODULE_DEVICE_TABLE(of, asoc_simple_of_match); + static struct platform_driver asoc_simple_card = { .driver = { .name = "asoc-simple-card", .owner = THIS_MODULE, + .of_match_table = asoc_simple_of_match, }, .probe = asoc_simple_card_probe, .remove = asoc_simple_card_remove,
On Wed, Nov 20, 2013 at 03:25:02PM +0900, Kuninori Morimoto wrote:
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information.
I had a look at the recent DT reviews and it seems like the review here is both getting slower and focusing on more and more minor issues so I think that really everything is OK here and I've applied this.
Thanks for your hard work and perseverence with this.
Hi Mark
Support for loading the simple-card module via DeviceTree. It requests CPU/CODEC information.
I had a look at the recent DT reviews and it seems like the review here is both getting slower and focusing on more and more minor issues so I think that really everything is OK here and I've applied this.
Thanks for your hard work and perseverence with this.
Thank you !! Phew, we spent 1 year for this support ;P
Best regards --- Kuninori Morimoto
On Mon, Nov 18, 2013 at 8:36 PM, Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:
Hi Rob, Mark
+Required properties:
+- compatible : "simple-audio"
Still not really a fan of this generic name. Can we define in the description above what simple means.
So, how about "simple-audio-card" ?
That's missing my point. First, I think you should be defining the actual h/w in the DT and doing the mapping of that to a simple audio driver in the kernel. Otherwise how do you fix some quirk on a particular platform later on without updating the DTB? I'm fine with this being the default compatible string, but you should also require a more specific name. Perhaps it is just <soc>-simple-audio or <board>-simple-audio.
Second, you need to define in this binding document what simple means. What properties of the audio subsystem make it simple? The h/w has and doesn't have what? How do I decide if my platform can or should use this binding?
Rob
On Wed, Nov 20, 2013 at 08:20:19AM -0600, Rob Herring wrote:
Second, you need to define in this binding document what simple means. What properties of the audio subsystem make it simple? The h/w has and doesn't have what? How do I decide if my platform can or should use this binding?
I think it's reasonable to just say that a simple device is one that can use this binding and that if that is not possible we need to make a taste based decision at the time between extending this binding or using a new one.
Hi Rob
Still not really a fan of this generic name. Can we define in the description above what simple means.
So, how about "simple-audio-card" ?
That's missing my point. First, I think you should be defining the actual h/w in the DT and doing the mapping of that to a simple audio driver in the kernel. Otherwise how do you fix some quirk on a particular platform later on without updating the DTB? I'm fine with this being the default compatible string, but you should also require a more specific name. Perhaps it is just <soc>-simple-audio or <board>-simple-audio.
Second, you need to define in this binding document what simple means. What properties of the audio subsystem make it simple? The h/w has and doesn't have what? How do I decide if my platform can or should use this binding?
Basically, on ASoC case, SoC/board needs <soc>-<codec>-audio-card (= not simple card) for matching each other, and this is start point. This means we need many <soc>-<codec>-audio-card.c driver. But, in some case, the difference between <socA>-<codecA> <-> <socA>-<codecB> <-> <socB>-<codecA> <-> <socB>-<codecB> was just "name". creating too many such driver was not sane for me. This simple-audio is used in such case. Of course we can update simple-audio feature (if it is very simple/common feature) but, if you need <soc/board>-audio-card which needs special feature, you need to create such driver without using simple-card. This is very normal approach on ASoC and there are many such driver.
Best regards --- Kuninori Morimoto
Hi Rob
ping ?
Still not really a fan of this generic name. Can we define in the description above what simple means.
So, how about "simple-audio-card" ?
That's missing my point. First, I think you should be defining the actual h/w in the DT and doing the mapping of that to a simple audio driver in the kernel. Otherwise how do you fix some quirk on a particular platform later on without updating the DTB? I'm fine with this being the default compatible string, but you should also require a more specific name. Perhaps it is just <soc>-simple-audio or <board>-simple-audio.
Second, you need to define in this binding document what simple means. What properties of the audio subsystem make it simple? The h/w has and doesn't have what? How do I decide if my platform can or should use this binding?
Basically, on ASoC case, SoC/board needs <soc>-<codec>-audio-card (= not simple card) for matching each other, and this is start point. This means we need many <soc>-<codec>-audio-card.c driver. But, in some case, the difference between <socA>-<codecA> <-> <socA>-<codecB> <-> <socB>-<codecA> <-> <socB>-<codecB> was just "name". creating too many such driver was not sane for me. This simple-audio is used in such case. Of course we can update simple-audio feature (if it is very simple/common feature) but, if you need <soc/board>-audio-card which needs special feature, you need to create such driver without using simple-card. This is very normal approach on ASoC and there are many such driver.
Best regards
Kuninori Morimoto
On Thu, Oct 24, 2013 at 06:17:59PM +0100, Mark Rutland wrote:
On Fri, Oct 04, 2013 at 01:04:41AM +0100, Kuninori Morimoto wrote:
+- simple-audio,card-name : simple-audio card name
What's this used for?
This serves two useful functions. One is that this is used for display to users so they have a friendly name for the sound card (it is relatively common to have multiple sound cards in the system). The other is that it is essentially a compatibility string for configuration - you get a lot of sound devices that are electrically identical and hence look the same from a driver point of view but due different physical form factors should be configured differently.
+- format : specific format if needed, see below +- frame-master : frame master +- bitclock-master : bitclock master +- bitclock-inversion : clock inversion +- frame-inversion : frame inversion
What do these mean? Repeating the name without a dash is completely unhelpful. Describe what these imply.
These are all boolean propeties. The meanings should be obvious or at least very easily discoverable to anyone with any familiarity with audio hardware; if you can understand what to do with them they should be OK.
+- clocks : phandle for system clock rate
Just one clock?
This is a limitation from the simple card, anything that needs more complex clocking should be using a different binding.
+- system-clock-frequency : system clock rate
it will overwrite clocks's rate
This seems very odd.
Why do you want to overwrite a clock's rate?
It's relatively common to derive the audio clock from a general purpose programmable clock which needs to be configured appropriately for use.
+simple-audio,format
- "i2s"
- "right_j"
- "left_j"
- "dsp_a"
- "dsp_b"
- "ac97"
- "pdm"
- "msb"
- "lsb"
What do these mean? Why are they not described when the property was defined above?
This is another one where the names should be clear for people familiar with the hardware, they're well known terms.
Hi Mark (Brown), Mark (Rutland)
I had sent v4 patch, but I removed simple-card,card-name on it. But, as Mark (Brown) explained, I think simple-card,card-name is very helpful for users. I can send v5 patch (with it), or incremental patch if we can have it. So, I need your comment
[1 <multipart/signed (7bit)>] [1.1 <text/plain; us-ascii (7bit)>] On Thu, Oct 24, 2013 at 06:17:59PM +0100, Mark Rutland wrote:
On Fri, Oct 04, 2013 at 01:04:41AM +0100, Kuninori Morimoto wrote:
+- simple-audio,card-name : simple-audio card name
What's this used for?
This serves two useful functions. One is that this is used for display to users so they have a friendly name for the sound card (it is relatively common to have multiple sound cards in the system). The other is that it is essentially a compatibility string for configuration
- you get a lot of sound devices that are electrically identical and
hence look the same from a driver point of view but due different physical form factors should be configured differently.
+- format : specific format if needed, see below +- frame-master : frame master +- bitclock-master : bitclock master +- bitclock-inversion : clock inversion +- frame-inversion : frame inversion
What do these mean? Repeating the name without a dash is completely unhelpful. Describe what these imply.
These are all boolean propeties. The meanings should be obvious or at least very easily discoverable to anyone with any familiarity with audio hardware; if you can understand what to do with them they should be OK.
+- clocks : phandle for system clock rate
Just one clock?
This is a limitation from the simple card, anything that needs more complex clocking should be using a different binding.
+- system-clock-frequency : system clock rate
it will overwrite clocks's rateThis seems very odd.
Why do you want to overwrite a clock's rate?
It's relatively common to derive the audio clock from a general purpose programmable clock which needs to be configured appropriately for use.
+simple-audio,format
- "i2s"
- "right_j"
- "left_j"
- "dsp_a"
- "dsp_b"
- "ac97"
- "pdm"
- "msb"
- "lsb"
What do these mean? Why are they not described when the property was defined above?
This is another one where the names should be clear for people familiar with the hardware, they're well known terms. [1.2 Digital signature <application/pgp-signature (7bit)>]
[2 <text/plain; us-ascii (7bit)>] _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Best regards --- Kuninori Morimoto
On Tue, Oct 29, 2013 at 05:39:17PM -0700, Kuninori Morimoto wrote:
I had sent v4 patch, but I removed simple-card,card-name on it. But, as Mark (Brown) explained, I think simple-card,card-name is very helpful for users. I can send v5 patch (with it), or incremental patch if we can have it. So, I need your comment
Let's review the existing bindings as-is unless there's a need to revise for some other reason, it's easy enough to add the property back later.
Hi Mark
I had sent v4 patch, but I removed simple-card,card-name on it. But, as Mark (Brown) explained, I think simple-card,card-name is very helpful for users. I can send v5 patch (with it), or incremental patch if we can have it. So, I need your comment
Let's review the existing bindings as-is unless there's a need to revise for some other reason, it's easy enough to add the property back later.
OK, I see Thank you.
Best regards --- Kuninori Morimoto
participants (4)
-
Kuninori Morimoto -
Mark Brown -
Mark Rutland -
Rob Herring