diff mbox

[2/2,v6] ASoC: mediatek: Add second I2S on mt8173-rt5650 machine driver

Message ID 1461134447-42001-3-git-send-email-pc.liao@mediatek.com (mailing list archive)
State New, archived
Headers show

Commit Message

PC Liao April 20, 2016, 6:40 a.m. UTC 
This patch adds second I2S connection to rt5650 codec for capture path on
mt8173-rt5650 machine driver.

Signed-off-by: PC Liao <pc.liao@mediatek.com>
---
 .../devicetree/bindings/sound/mt8173-rt5650.txt    |    6 +++
 sound/soc/mediatek/mt8173-rt5650.c                 |   44 ++++++++++++++++++--
 2 files changed, 47 insertions(+), 3 deletions(-)

Comments

Mark Brown April 20, 2016, 8:29 a.m. UTC | #1 
On Wed, Apr 20, 2016 at 02:40:47PM +0800, PC Liao wrote:

> +Optional properties:
> +- capture-dai: audio codec dai on capture path
> +  <&rt5650 0> : Default setting. Connect rt5650 I2S1 for capture. (dai_name = rt5645-aif1)
> +  <&rt5650 1> : Connect rt5650 I2S2 for capture. (dai_name = rt5645-aif2)

I sugggested changing this to use a subnode but you've ignored my
suggestion...
PC Liao April 20, 2016, 9:39 a.m. UTC | #2 
Hi Mark,

On Wed, 2016-04-20 at 16:29 +0800, Mark Brown wrote:
> On Wed, Apr 20, 2016 at 02:40:47PM +0800, PC Liao wrote:
> 
> > +Optional properties:
> > +- capture-dai: audio codec dai on capture path
> > +  <&rt5650 0> : Default setting. Connect rt5650 I2S1 for capture. (dai_name = rt5645-aif1)
> > +  <&rt5650 1> : Connect rt5650 I2S2 for capture. (dai_name = rt5645-aif2)
> 
> I sugggested changing this to use a subnode but you've ignored my
> suggestion...

Sorry.... It's my mistake.

You mean that using a subnode.
This is my example:

  codec-capture {
	  sound-dai = <&rt5650 1>;
  };

Does this is your suggestion?
Thanks!
Matthias Brugger April 20, 2016, 1:30 p.m. UTC | #3 
On 20/04/16 08:40, PC Liao wrote:
> This patch adds second I2S connection to rt5650 codec for capture path on
> mt8173-rt5650 machine driver.
>
> Signed-off-by: PC Liao <pc.liao@mediatek.com>
> ---
>   .../devicetree/bindings/sound/mt8173-rt5650.txt    |    6 +++
>   sound/soc/mediatek/mt8173-rt5650.c                 |   44 ++++++++++++++++++--
>   2 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/mt8173-rt5650.txt b/Documentation/devicetree/bindings/sound/mt8173-rt5650.txt
> index fe5a5ef..802dd1b 100644
> --- a/Documentation/devicetree/bindings/sound/mt8173-rt5650.txt
> +++ b/Documentation/devicetree/bindings/sound/mt8173-rt5650.txt
> @@ -5,11 +5,17 @@ Required properties:
>   - mediatek,audio-codec: the phandles of rt5650 codecs
>   - mediatek,platform: the phandle of MT8173 ASoC platform
>
> +Optional properties:
> +- capture-dai: audio codec dai on capture path
> +  <&rt5650 0> : Default setting. Connect rt5650 I2S1 for capture. (dai_name = rt5645-aif1)
> +  <&rt5650 1> : Connect rt5650 I2S2 for capture. (dai_name = rt5645-aif2)
> +
>   Example:
>
>   	sound {
>   		compatible = "mediatek,mt8173-rt5650";
>   		mediatek,audio-codec = <&rt5650>;
>   		mediatek,platform = <&afe>;
> +		capture-dai = <&rt5650 1>;
>   	};
>
> diff --git a/sound/soc/mediatek/mt8173-rt5650.c b/sound/soc/mediatek/mt8173-rt5650.c
> index bb09bb1..45dea22 100644
> --- a/sound/soc/mediatek/mt8173-rt5650.c
> +++ b/sound/soc/mediatek/mt8173-rt5650.c
> @@ -85,12 +85,26 @@ static int mt8173_rt5650_init(struct snd_soc_pcm_runtime *runtime)
>   {
>   	struct snd_soc_card *card = runtime->card;
>   	struct snd_soc_codec *codec = runtime->codec_dais[0]->codec;
> +	const char *codec_dai_capture = runtime->codec_dais[1]->name;
>   	int ret;
>
>   	rt5645_sel_asrc_clk_src(codec,
> -				RT5645_DA_STEREO_FILTER |
> -				RT5645_AD_STEREO_FILTER,
> +				RT5645_DA_STEREO_FILTER,
>   				RT5645_CLK_SEL_I2S1_ASRC);
> +
> +	if (!strcmp(codec_dai_capture, "rt5645-aif1")) {
> +		rt5645_sel_asrc_clk_src(codec,
> +					RT5645_AD_STEREO_FILTER,
> +					RT5645_CLK_SEL_I2S1_ASRC);
> +	} else if (!strcmp(codec_dai_capture, "rt5645-aif2")) {
> +		rt5645_sel_asrc_clk_src(codec,
> +					RT5645_AD_STEREO_FILTER,
> +					RT5645_CLK_SEL_I2S2_ASRC);
> +	} else {

For compatibility you should change this to something like:

    dev_warn(card->dev, "Only one dai codec found in DTS, enabled rt5645
                         AD filter\n");
    rt5645_sel_asrc_clk_src(codec, RT5645_AD_STEREO_FILTER,
                                   RT5645_CLK_SEL_I2S1_ASRC);
}

Regards,
Matthias
Mark Brown April 20, 2016, 4:11 p.m. UTC | #4 
On Wed, Apr 20, 2016 at 05:39:50PM +0800, PC Liao wrote:
> On Wed, 2016-04-20 at 16:29 +0800, Mark Brown wrote:
> > On Wed, Apr 20, 2016 at 02:40:47PM +0800, PC Liao wrote:

> > > +Optional properties:
> > > +- capture-dai: audio codec dai on capture path
> > > +  <&rt5650 0> : Default setting. Connect rt5650 I2S1 for capture. (dai_name = rt5645-aif1)
> > > +  <&rt5650 1> : Connect rt5650 I2S2 for capture. (dai_name = rt5645-aif2)

> > I sugggested changing this to use a subnode but you've ignored my
> > suggestion...

> Sorry.... It's my mistake.

> You mean that using a subnode.
> This is my example:

>   codec-capture {
> 	  sound-dai = <&rt5650 1>;
>   };

> Does this is your suggestion?

Yeah.  Having refreshed my mind about the APIs a bit the idea is that if
you do things this way then any additional parameters that are needed
for the DAI can be added within the subnode.
PC Liao April 20, 2016, 4:27 p.m. UTC | #5 
Hi Matthias,

On Wed, 2016-04-20 at 21:30 +0800, Matthias Brugger wrote:
> 
> For compatibility you should change this to something like:
> 
>     dev_warn(card->dev, "Only one dai codec found in DTS, enabled rt5645
>                          AD filter\n");
>     rt5645_sel_asrc_clk_src(codec, RT5645_AD_STEREO_FILTER,
>                                    RT5645_CLK_SEL_I2S1_ASRC);
> }
> 

I will use this in next version.
Thanks!
Matthias Brugger April 21, 2016, 9:22 a.m. UTC | #6 
On 20/04/16 18:27, PC Liao wrote:
> Hi Matthias,
>
> On Wed, 2016-04-20 at 21:30 +0800, Matthias Brugger wrote:
>>
>> For compatibility you should change this to something like:
>>
>>      dev_warn(card->dev, "Only one dai codec found in DTS, enabled rt5645
>>                           AD filter\n");
>>      rt5645_sel_asrc_clk_src(codec, RT5645_AD_STEREO_FILTER,
>>                                     RT5645_CLK_SEL_I2S1_ASRC);
>> }
>>
>
> I will use this in next version.
> Thanks!
>

I think you forgot to add this to v7 of the patches.

Cheers,
Matthias
PC Liao April 21, 2016, 9:38 a.m. UTC | #7 
Hi Matthias,

On Thu, 2016-04-21 at 17:22 +0800, Matthias Brugger wrote:
> 
> On 20/04/16 18:27, PC Liao wrote:
> > Hi Matthias,
> >
> > On Wed, 2016-04-20 at 21:30 +0800, Matthias Brugger wrote:
> >>
> >> For compatibility you should change this to something like:
> >>
> >>      dev_warn(card->dev, "Only one dai codec found in DTS, enabled rt5645
> >>                           AD filter\n");
> >>      rt5645_sel_asrc_clk_src(codec, RT5645_AD_STEREO_FILTER,
> >>                                     RT5645_CLK_SEL_I2S1_ASRC);
> >> }
> >>
> >
> > I will use this in next version.
> > Thanks!
> >
> 
> I think you forgot to add this to v7 of the patches.
> 
> Cheers,
> Matthias

Oh, sorry.
I misunderstand your suggestion.
I will upload.
Thanks!
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/sound/mt8173-rt5650.txt b/Documentation/devicetree/bindings/sound/mt8173-rt5650.txt
index fe5a5ef..802dd1b 100644
--- a/Documentation/devicetree/bindings/sound/mt8173-rt5650.txt
+++ b/Documentation/devicetree/bindings/sound/mt8173-rt5650.txt
@@ -5,11 +5,17 @@  Required properties:
 - mediatek,audio-codec: the phandles of rt5650 codecs
 - mediatek,platform: the phandle of MT8173 ASoC platform
 
+Optional properties:
+- capture-dai: audio codec dai on capture path
+  <&rt5650 0> : Default setting. Connect rt5650 I2S1 for capture. (dai_name = rt5645-aif1)
+  <&rt5650 1> : Connect rt5650 I2S2 for capture. (dai_name = rt5645-aif2)
+
 Example:
 
 	sound {
 		compatible = "mediatek,mt8173-rt5650";
 		mediatek,audio-codec = <&rt5650>;
 		mediatek,platform = <&afe>;
+		capture-dai = <&rt5650 1>;
 	};
 
diff --git a/sound/soc/mediatek/mt8173-rt5650.c b/sound/soc/mediatek/mt8173-rt5650.c
index bb09bb1..45dea22 100644
--- a/sound/soc/mediatek/mt8173-rt5650.c
+++ b/sound/soc/mediatek/mt8173-rt5650.c
@@ -85,12 +85,26 @@  static int mt8173_rt5650_init(struct snd_soc_pcm_runtime *runtime)
 {
 	struct snd_soc_card *card = runtime->card;
 	struct snd_soc_codec *codec = runtime->codec_dais[0]->codec;
+	const char *codec_dai_capture = runtime->codec_dais[1]->name;
 	int ret;
 
 	rt5645_sel_asrc_clk_src(codec,
-				RT5645_DA_STEREO_FILTER |
-				RT5645_AD_STEREO_FILTER,
+				RT5645_DA_STEREO_FILTER,
 				RT5645_CLK_SEL_I2S1_ASRC);
+
+	if (!strcmp(codec_dai_capture, "rt5645-aif1")) {
+		rt5645_sel_asrc_clk_src(codec,
+					RT5645_AD_STEREO_FILTER,
+					RT5645_CLK_SEL_I2S1_ASRC);
+	} else if (!strcmp(codec_dai_capture, "rt5645-aif2")) {
+		rt5645_sel_asrc_clk_src(codec,
+					RT5645_AD_STEREO_FILTER,
+					RT5645_CLK_SEL_I2S2_ASRC);
+	} else {
+		dev_err(card->dev, "Can't get the right codec dai\n");
+		return -EINVAL;
+	}
+
 	/* enable jack detection */
 	ret = snd_soc_card_jack_new(card, "Headset Jack",
 				    SND_JACK_HEADPHONE | SND_JACK_MICROPHONE |
@@ -110,6 +124,11 @@  static int mt8173_rt5650_init(struct snd_soc_pcm_runtime *runtime)
 
 static struct snd_soc_dai_link_component mt8173_rt5650_codecs[] = {
 	{
+		/* Playback */
+		.dai_name = "rt5645-aif1",
+	},
+	{
+		/* Capture */
 		.dai_name = "rt5645-aif1",
 	},
 };
@@ -149,7 +168,7 @@  static struct snd_soc_dai_link mt8173_rt5650_dais[] = {
 		.cpu_dai_name = "I2S",
 		.no_pcm = 1,
 		.codecs = mt8173_rt5650_codecs,
-		.num_codecs = 1,
+		.num_codecs = 2,
 		.init = mt8173_rt5650_init,
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
 			   SND_SOC_DAIFMT_CBS_CFS,
@@ -177,6 +196,8 @@  static int mt8173_rt5650_dev_probe(struct platform_device *pdev)
 {
 	struct snd_soc_card *card = &mt8173_rt5650_card;
 	struct device_node *platform_node;
+	const char *codec_dai_capture;
+	struct of_phandle_args args;
 	int i, ret;
 
 	platform_node = of_parse_phandle(pdev->dev.of_node,
@@ -199,6 +220,23 @@  static int mt8173_rt5650_dev_probe(struct platform_device *pdev)
 			"Property 'audio-codec' missing or invalid\n");
 		return -EINVAL;
 	}
+	mt8173_rt5650_codecs[1].of_node = mt8173_rt5650_codecs[0].of_node;
+
+	if (device_property_present(&pdev->dev, "capture-dai")) {
+		ret = of_parse_phandle_with_args(pdev->dev.of_node,
+						 "capture-dai",
+						 "#sound-dai-cells", 0, &args);
+		if (ret < 0) {
+			dev_err(&pdev->dev,
+				"%s capture-dai name fail %d\n",
+				__func__, ret);
+			return ret;
+		}
+
+		ret = snd_soc_get_dai_name(&args, &codec_dai_capture);
+		mt8173_rt5650_codecs[1].dai_name = codec_dai_capture;
+	}
+
 	card->dev = &pdev->dev;
 	platform_set_drvdata(pdev, card);