diff mbox

ASoC: mediatek: Add second I2S on mt8173-rt5650 machine driver

Message ID 1458877325-33980-1-git-send-email-pc.liao@mediatek.com (mailing list archive)
State New, archived
Headers show

Commit Message

PC Liao March 25, 2016, 3:42 a.m. UTC
This patch adds second I2S for rt5650 codec 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                 |   55 ++++++++++++++++++--
 2 files changed, 56 insertions(+), 5 deletions(-)

Comments

Mark Brown March 28, 2016, 7:11 p.m. UTC | #1
On Fri, Mar 25, 2016 at 11:42:05AM +0800, PC Liao wrote:

> +Optional properties:
> +- mediatek,rt5650_i2s: I2S mode of rt5650
> +  0: using I2S1 on rt5650 for record
> +  1: using I2S2 on rt5650 for record
> +

I would expect the machine driver to have a reference to the DAI that is
in use rather than a property like this.  This is the way pretty much
all DT based machine drivers work...
PC Liao March 29, 2016, 1:28 p.m. UTC | #2
Hi Mark,

On Tue, 2016-03-29 at 03:11 +0800, Mark Brown wrote:
> On Fri, Mar 25, 2016 at 11:42:05AM +0800, PC Liao wrote:
> 
> > +Optional properties:
> > +- mediatek,rt5650_i2s: I2S mode of rt5650
> > +  0: using I2S1 on rt5650 for record
> > +  1: using I2S2 on rt5650 for record
> > +
> 
> I would expect the machine driver to have a reference to the DAI that is
> in use rather than a property like this.  This is the way pretty much
> all DT based machine drivers work...

Thanks for comment.
I think I need to comment more clearly.
This setting is different connection from MT8173 to rt5650 on hardware.

- mediatek,rt5650_i2s: I2S mode of rt5650
0: Default setting. Playback and record path use same set of I2S.
Playback/Record path using same I2S clock connect from MT8173 I2S1 to
rt5650 I2S1.
1: Playback and record path use different set of I2S. Playback path
connects from MT8173 I2S1 to rt5650 I2S1 and record path connects from
MT8173 I2S2 to rt5650 I2S2.

Because we use codec ASRC function and need to initialize on different
setting about this parts:
+	switch (mt8173_rt5650_priv.rt5650_i2s) {
+	case MT8173_RT5650_I2S1:
+		rt5645_sel_asrc_clk_src(codec,
+					RT5645_DA_STEREO_FILTER |
+					RT5645_AD_STEREO_FILTER,
+					RT5645_CLK_SEL_I2S1_ASRC);
+		break;
+	case MT8173_RT5650_I2S2:
+		rt5645_sel_asrc_clk_src(codec,
+					RT5645_DA_STEREO_FILTER,
+					RT5645_CLK_SEL_I2S1_ASRC);
+		rt5645_sel_asrc_clk_src(codec,
+					RT5645_AD_STEREO_FILTER,
+					RT5645_CLK_SEL_I2S2_ASRC);
+		break;
+	default:
+		break;
+	}
, we use device tree to determine which connection we choose.
Does this explain can be accepted?
Thanks!
Mark Brown March 29, 2016, 4:07 p.m. UTC | #3
On Tue, Mar 29, 2016 at 09:28:23PM +0800, PC Liao wrote:

> - mediatek,rt5650_i2s: I2S mode of rt5650
> 0: Default setting. Playback and record path use same set of I2S.
> Playback/Record path using same I2S clock connect from MT8173 I2S1 to
> rt5650 I2S1.
> 1: Playback and record path use different set of I2S. Playback path
> connects from MT8173 I2S1 to rt5650 I2S1 and record path connects from
> MT8173 I2S2 to rt5650 I2S2.

> Because we use codec ASRC function and need to initialize on different
> setting about this parts:

This sort of arrangement is very common - it's often needed to get
different sample rates for playback and capture.  Normally it'd be
represented in the DT by having the two DAI links specified normally and
then having the driver look at the DT to see what's connected to work
out what mode to use.
PC Liao April 1, 2016, 2:50 a.m. UTC | #4
Hi Mark,

On Wed, 2016-03-30 at 00:07 +0800, Mark Brown wrote:
> On Tue, Mar 29, 2016 at 09:28:23PM +0800, PC Liao wrote:
> 
> > - mediatek,rt5650_i2s: I2S mode of rt5650
> > 0: Default setting. Playback and record path use same set of I2S.
> > Playback/Record path using same I2S clock connect from MT8173 I2S1 to
> > rt5650 I2S1.
> > 1: Playback and record path use different set of I2S. Playback path
> > connects from MT8173 I2S1 to rt5650 I2S1 and record path connects from
> > MT8173 I2S2 to rt5650 I2S2.
> 
> > Because we use codec ASRC function and need to initialize on different
> > setting about this parts:
> 
> This sort of arrangement is very common - it's often needed to get
> different sample rates for playback and capture.  Normally it'd be
> represented in the DT by having the two DAI links specified normally and
> then having the driver look at the DT to see what's connected to work
> out what mode to use.

Could you please suggest the reference driver about this?
Thanks!
Mark Brown April 2, 2016, 5:04 p.m. UTC | #5
On Fri, Apr 01, 2016 at 10:50:52AM +0800, PC Liao wrote:
> On Wed, 2016-03-30 at 00:07 +0800, Mark Brown wrote:

> > This sort of arrangement is very common - it's often needed to get
> > different sample rates for playback and capture.  Normally it'd be
> > represented in the DT by having the two DAI links specified normally and
> > then having the driver look at the DT to see what's connected to work
> > out what mode to use.

> Could you please suggest the reference driver about this?

Any driver that can identify an individual DAI within a device by using
sound-dai references, even simple-card does this.
PC Liao April 6, 2016, 1:58 p.m. UTC | #6
Hi Mark,

On Sun, 2016-04-03 at 01:04 +0800, Mark Brown wrote:
> On Fri, Apr 01, 2016 at 10:50:52AM +0800, PC Liao wrote:
> > On Wed, 2016-03-30 at 00:07 +0800, Mark Brown wrote:
> 
> > > This sort of arrangement is very common - it's often needed to get
> > > different sample rates for playback and capture.  Normally it'd be
> > > represented in the DT by having the two DAI links specified normally and
> > > then having the driver look at the DT to see what's connected to work
> > > out what mode to use.
> 
> > Could you please suggest the reference driver about this?
> 
> Any driver that can identify an individual DAI within a device by using
> sound-dai references, even simple-card does this.

Thanks for suggestion.
This machine driver is based on DPCM.
Should I add BE DAI and use DAPM, then using ucm-config(mixer control)
to control the path?
Or, do I think the wrong direction?
Thanks!
Mark Brown April 6, 2016, 5:17 p.m. UTC | #7
On Wed, Apr 06, 2016 at 09:58:25PM +0800, PC Liao wrote:
> On Sun, 2016-04-03 at 01:04 +0800, Mark Brown wrote:

> > Any driver that can identify an individual DAI within a device by using
> > sound-dai references, even simple-card does this.

> Thanks for suggestion.
> This machine driver is based on DPCM.
> Should I add BE DAI and use DAPM, then using ucm-config(mixer control)
> to control the path?
> Or, do I think the wrong direction?
> Thanks!

It sounds like the configuration is fixed in these systems and can't
be usefully varied at runtime?  If that's the case then there's no need
for a userspace control.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/sound/mt8173-rt5650.txt b/Documentation/devicetree/bindings/sound/mt8173-rt5650.txt
index fe5a5ef..0046926 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:
+- mediatek,rt5650_i2s: I2S mode of rt5650
+  0: using I2S1 on rt5650 for record
+  1: using I2S2 on rt5650 for record
+
 Example:
 
 	sound {
 		compatible = "mediatek,mt8173-rt5650";
 		mediatek,audio-codec = <&rt5650>;
 		mediatek,platform = <&afe>;
+		mediatek,rt5650_i2s = <0>;
 	};
 
diff --git a/sound/soc/mediatek/mt8173-rt5650.c b/sound/soc/mediatek/mt8173-rt5650.c
index bb09bb1..ab4c806 100644
--- a/sound/soc/mediatek/mt8173-rt5650.c
+++ b/sound/soc/mediatek/mt8173-rt5650.c
@@ -23,6 +23,20 @@ 
 
 #define MCLK_FOR_CODECS		12288000
 
+struct mt8173_rt5650_platform_data {
+	unsigned int rt5650_i2s;
+	/* 0 = I2S1; 1 = I2S2 */
+};
+
+static struct mt8173_rt5650_platform_data mt8173_rt5650_priv = {
+	.rt5650_i2s = 0,
+};
+
+enum mt8173_rt5650_i2s {
+	MT8173_RT5650_I2S1 = 0,
+	MT8173_RT5650_I2S2,
+};
+
 static const struct snd_soc_dapm_widget mt8173_rt5650_widgets[] = {
 	SND_SOC_DAPM_SPK("Speaker", NULL),
 	SND_SOC_DAPM_MIC("Int Mic", NULL),
@@ -87,10 +101,25 @@  static int mt8173_rt5650_init(struct snd_soc_pcm_runtime *runtime)
 	struct snd_soc_codec *codec = runtime->codec_dais[0]->codec;
 	int ret;
 
-	rt5645_sel_asrc_clk_src(codec,
-				RT5645_DA_STEREO_FILTER |
-				RT5645_AD_STEREO_FILTER,
-				RT5645_CLK_SEL_I2S1_ASRC);
+	switch (mt8173_rt5650_priv.rt5650_i2s) {
+	case MT8173_RT5650_I2S1:
+		rt5645_sel_asrc_clk_src(codec,
+					RT5645_DA_STEREO_FILTER |
+					RT5645_AD_STEREO_FILTER,
+					RT5645_CLK_SEL_I2S1_ASRC);
+		break;
+	case MT8173_RT5650_I2S2:
+		rt5645_sel_asrc_clk_src(codec,
+					RT5645_DA_STEREO_FILTER,
+					RT5645_CLK_SEL_I2S1_ASRC);
+		rt5645_sel_asrc_clk_src(codec,
+					RT5645_AD_STEREO_FILTER,
+					RT5645_CLK_SEL_I2S2_ASRC);
+		break;
+	default:
+		break;
+	}
+
 	/* enable jack detection */
 	ret = snd_soc_card_jack_new(card, "Headset Jack",
 				    SND_JACK_HEADPHONE | SND_JACK_MICROPHONE |
@@ -112,6 +141,9 @@  static struct snd_soc_dai_link_component mt8173_rt5650_codecs[] = {
 	{
 		.dai_name = "rt5645-aif1",
 	},
+	{
+		.dai_name = "rt5645-aif2",
+	},
 };
 
 enum {
@@ -149,7 +181,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,
@@ -199,6 +231,19 @@  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, "mediatek,rt5650_i2s")) {
+		ret = device_property_read_u32(&pdev->dev,
+					       "mediatek,rt5650_i2s",
+					       &mt8173_rt5650_priv.rt5650_i2s);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"%s snd_soc_register_card fail %d\n",
+				__func__, ret);
+		}
+	}
+
 	card->dev = &pdev->dev;
 	platform_set_drvdata(pdev, card);