[alsa-devel] [RFC][PATCH 2/2 v2] ASoC: simple-card: add Device Tree support

Mark Brown broonie at opensource.wolfsonmicro.com
Sun Jan 27 04:59:55 CET 2013


On Mon, Jan 14, 2013 at 06:40:37PM -0800, Kuninori Morimoto wrote:

Sorry about the delay in getting back to you on this.

> +- simple-audio,cpu,dai,clock-gating		: if needed, see below

A lot of these are listed as "if needed" - this means they should be
listed separately as optional properties rather than in the required
properties section.

> +- simple-audio,codec,controller			: phandle for CODEC DAI

It feels like this should just be simple-audio,codec - the controller is
just redudnant.  Though for idiomatic DT we ought to write something
like

	simple-audio,codec {
		simple-audio,dev = &phandle;
		simple-audio,system-clock-frequency = 122880000;
	};

rather than have these very long prefixes to the individual property
names.

> +- simple-audio,codec,dai,name			: simple-audio CODEC DAI name
> +- simple-audio,codec,dai,format			: see below
> +- simple-audio,codec,dai,clock-gating		: if needed, see below
> +- simple-audio,codec,dai,bitclock-inversion	: if needed
> +- simple-audio,codec,dai,bitclock-master	: if needed
> +- simple-audio,codec,dai,frame-inversion	: if needed
> +- simple-audio,codec,dai,frame-master		: if needed
> +- simple-audio,codec,dai,system-clock-frequency	: system clock rate if needed

I'm also thinking that for some of the above properties which really
should be the same for both ends of the link we should just specify
them at the card levle and copy them over.  The format and inversion 
mainly.

> +
> +simple-audio,xxx,dai,format
> +	"i2s"
> +	"right_j"
> +	"left_j"
> +	"dsp_a"
> +	"dsp_b"
> +	"ac97"
> +	"pdm"
> +	"msb"
> +	"lsb"
> +
> +simple-audio,xxx,dai,clock-gating
> +	"continuous"
> +	"gated"
> +
> +Example:
> +
> +fsi_ak4642 {
> +	compatible = "simple-audio";
> +
> +	simple-audio,card-name = "FSI2A-AK4648";
> +	simple-audio,platform,controller = <&sh_fsi2>;
> +
> +	simple-audio,cpu,dai,name = "fsia-dai";
> +	simple-audio,cpu,dai,format = "left_j";
> +
> +	simple-audio,codec,controller = <&ak4648>;
> +	simple-audio,codec,dai,name = "ak4642-hifi";
> +	simple-audio,codec,dai,format = "left_j";
> +	simple-audio,codec,dai,bitclock-master;
> +	simple-audio,codec,dai,frame-master;
> +	simple-audio,codec,dai,system-clock-frequency = <11289600>;
> +};
> +
> +&i2c0 {
> +	ak4648: ak4648 at 0x12 {
> +		compatible = "asahi-kasei,ak4648";
> +		reg = <0x12>;
> +	};
> +};
> +
> +sh_fsi2: sh_fsi2 at 0xec230000 {
> +	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 6cf8355..72dd8ae 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -9,6 +9,7 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/module.h>
>  #include <sound/simple_card.h>
> @@ -52,11 +53,99 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
>  	return 0;
>  }
>  
> +static struct device_node*
> +__asoc_simple_card_parse_of(struct device_node *np,
> +			    const char *node_name1,
> +			    const char *node_name2,
> +			    const char **interface,
> +			    struct asoc_simple_dai *dai)
> +{
> +	struct device_node *node;
> +	char prop[128];
> +
> +	/* node or name is required */
> +	snprintf(prop, sizeof(prop),
> +		 "simple-audio,%s,controller", node_name1);
> +	node = of_parse_phandle(np, prop, 0);
> +	if (node)
> +		of_node_put(node);
> +
> +	/* get "simple-audio,xxx,yyy,name = xxx" */
> +	snprintf(prop, sizeof(prop),
> +		 "simple-audio,%s%s,name", node_name1, node_name2);
> +	of_property_read_string(np, prop, interface);
> +
> +	if (dai) {
> +		/* get "simple-audio,xxx,yyy,formart = xxx" */
> +		snprintf(prop, sizeof(prop),
> +			 "simple-audio,%s%s,", node_name1, node_name2);
> +		dai->fmt = snd_soc_of_parse_daifmt(np, prop);
> +
> +		/* get "simple-audio,xxx,yyy,system-clock-frequency = <xxx>" */
> +		snprintf(prop, sizeof(prop),
> +			 "simple-audio,%s%s,system-clock-frequency",
> +			 node_name1, node_name2);
> +		of_property_read_u32(np, prop, &dai->sysclk);
> +	}
> +
> +	return node;
> +}
> +
> +static void asoc_simple_card_parse_of(struct device_node *np,
> +				      struct asoc_simple_card_info *info,
> +				      struct device *dev,
> +				      struct device_node **of_cpu,
> +				      struct device_node **of_codec,
> +				      struct device_node **of_platform)
> +{
> +	of_property_read_string(np, "simple-audio,card-name", &info->card);
> +	info->name = info->card;
> +
> +	*of_cpu = __asoc_simple_card_parse_of(
> +		np, "cpu", ",dai", &info->cpu_dai.name, &info->cpu_dai);
> +	*of_codec = __asoc_simple_card_parse_of(
> +		np, "codec", ",dai", &info->codec_dai.name, &info->codec_dai);
> +	*of_platform = __asoc_simple_card_parse_of(
> +		np, "platform", "", &info->platform, NULL);
> +
> +	dev_dbg(dev, "card-name  : %s\n", info->card);
> +	dev_dbg(dev, "cpu info   : %s / %x / %d / %p\n",
> +		info->cpu_dai.name,
> +		info->cpu_dai.fmt,
> +		info->cpu_dai.sysclk,
> +		*of_cpu);
> +	dev_dbg(dev, "codec_info : %s / %x / %d / %p\n",
> +		info->codec_dai.name,
> +		info->codec_dai.fmt,
> +		info->codec_dai.sysclk,
> +		*of_codec);
> +	dev_dbg(dev, "platform_info : %s / %p\n",
> +		info->platform,
> +		*of_platform);
> +}
> +
>  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)
> +			asoc_simple_card_parse_of(np, cinfo, dev,
> +						  &of_cpu,
> +						  &of_codec,
> +						  &of_platform);
> +	} else {
> +		cinfo = pdev->dev.platform_data;
> +	}
> +
>  	if (!cinfo) {
>  		dev_err(dev, "no info for asoc-simple-card\n");
>  		return -EINVAL;
> @@ -64,10 +153,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 +170,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,9 +194,15 @@ 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", },
> +	{},
> +};
> +
>  static struct platform_driver asoc_simple_card = {
>  	.driver = {
>  		.name	= "asoc-simple-card",
> +		.of_match_table = asoc_simple_of_match,
>  	},
>  	.probe		= asoc_simple_card_probe,
>  	.remove		= asoc_simple_card_remove,
> -- 
> 1.7.9.5
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20130127/42b04679/attachment-0001.sig>


More information about the Alsa-devel mailing list