diff mbox

[alsa-devel,3/4] ASoC: mediatek: add BT implementation

Message ID 1467629788-14411-4-git-send-email-garlic.tseng@mediatek.com (mailing list archive)
State New, archived
Headers show

Commit Message

Garlic Tseng July 4, 2016, 10:56 a.m. UTC 
Add BT implementation for mt2701 platform driver.

Signed-off-by: Garlic Tseng <garlic.tseng@mediatek.com>
---
 sound/soc/mediatek/mt2701/mt2701-afe-pcm.c | 139 +++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)

Comments

Mark Brown July 4, 2016, 2:44 p.m. UTC | #1 
On Mon, Jul 04, 2016 at 06:56:27PM +0800, Garlic Tseng wrote:

> +static int mt2701_btmrg_startup(struct snd_pcm_substream *substream,
> +				struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct mtk_base_afe *afe = snd_soc_platform_get_drvdata(rtd->platform);
> +	struct mt2701_afe_private *afe_priv = afe->platform_priv;
> +
> +	regmap_update_bits(afe->regmap, AUDIO_TOP_CON4,
> +			   AUDIO_TOP_CON4_PDN_MRGIF, 0);

We really shouldn't be writing the registers or other internal data of
the device.  Instead we should be getting the driver for the relevant
hardware component to do it.  If we just write to registers that makes
the interoperation with the real driver for the device more fragile than
it should be, people might update the main driver without noticing the
external driver.
Garlic Tseng July 5, 2016, 1:52 a.m. UTC | #2 
On Mon, 2016-07-04 at 16:44 +0200, Mark Brown wrote:
> On Mon, Jul 04, 2016 at 06:56:27PM +0800, Garlic Tseng wrote:
> 
> > +static int mt2701_btmrg_startup(struct snd_pcm_substream *substream,
> > +				struct snd_soc_dai *dai)
> > +{
> > +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > +	struct mtk_base_afe *afe = snd_soc_platform_get_drvdata(rtd->platform);
> > +	struct mt2701_afe_private *afe_priv = afe->platform_priv;
> > +
> > +	regmap_update_bits(afe->regmap, AUDIO_TOP_CON4,
> > +			   AUDIO_TOP_CON4_PDN_MRGIF, 0);
> 
> We really shouldn't be writing the registers or other internal data of
> the device.  Instead we should be getting the driver for the relevant
> hardware component to do it.  If we just write to registers that makes
> the interoperation with the real driver for the device more fragile than
> it should be, people might update the main driver without noticing the
> external driver.

The AUDIO_TOP_CON4 is a reg of the ASoC hw module. All the registers
which are set in the btmrg operator belong to ASoC hardware. The reg
writing you mention above is to power up the MRG interface in the ASoC
module.

When we start BT link, we need to start some hardware component belongs
to ASoC module. Of course we also need to start up the BT module
(including start up the MRG interface hw in BT chip), but it is not in
the ASoC driver.
Mark Brown July 5, 2016, 8:12 a.m. UTC | #3 
On Tue, Jul 05, 2016 at 09:52:13AM +0800, Garlic Tseng wrote:
> On Mon, 2016-07-04 at 16:44 +0200, Mark Brown wrote:

> > We really shouldn't be writing the registers or other internal data of
> > the device.  Instead we should be getting the driver for the relevant
> > hardware component to do it.  If we just write to registers that makes
> > the interoperation with the real driver for the device more fragile than
> > it should be, people might update the main driver without noticing the
> > external driver.

> The AUDIO_TOP_CON4 is a reg of the ASoC hw module. All the registers
> which are set in the btmrg operator belong to ASoC hardware. The reg
> writing you mention above is to power up the MRG interface in the ASoC
> module.

The problem is that this is the machine driver, not a driver for the
chip that's being controlled.
Garlic Tseng July 5, 2016, 8:59 a.m. UTC | #4 
On Tue, 2016-07-05 at 10:12 +0200, Mark Brown wrote:
> On Tue, Jul 05, 2016 at 09:52:13AM +0800, Garlic Tseng wrote:
> > On Mon, 2016-07-04 at 16:44 +0200, Mark Brown wrote:
> 
> > > We really shouldn't be writing the registers or other internal data of
> > > the device.  Instead we should be getting the driver for the relevant
> > > hardware component to do it.  If we just write to registers that makes
> > > the interoperation with the real driver for the device more fragile than
> > > it should be, people might update the main driver without noticing the
> > > external driver.
> 
> > The AUDIO_TOP_CON4 is a reg of the ASoC hw module. All the registers
> > which are set in the btmrg operator belong to ASoC hardware. The reg
> > writing you mention above is to power up the MRG interface in the ASoC
> > module.
> 
> The problem is that this is the machine driver, not a driver for the
> chip that's being controlled.

The patch is for platform driver "mt2701-afe-pcm.c", which contorls the
ASoC module, not for machine driver. Maybe the patch sequence is
confusing and I'm sorry about that. Or did I misunderstand your comment?
diff mbox

Patch

diff --git a/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c b/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
index c865ba1..6c14d68 100644
--- a/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
+++ b/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
@@ -333,6 +333,86 @@  static int mt2701_afe_i2s_set_sysclk(struct snd_soc_dai *dai, int clk_id,
 	return 0;
 }
 
+static int mt2701_btmrg_startup(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct mtk_base_afe *afe = snd_soc_platform_get_drvdata(rtd->platform);
+	struct mt2701_afe_private *afe_priv = afe->platform_priv;
+
+	regmap_update_bits(afe->regmap, AUDIO_TOP_CON4,
+			   AUDIO_TOP_CON4_PDN_MRGIF, 0);
+
+	afe_priv->mrg_enable[substream->stream] = 1;
+	return 0;
+}
+
+static int mt2701_btmrg_hw_params(struct snd_pcm_substream *substream,
+				  struct snd_pcm_hw_params *params,
+				  struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct mtk_base_afe *afe = snd_soc_platform_get_drvdata(rtd->platform);
+	int stream_fs;
+	u32 val, msk;
+
+	stream_fs = params_rate(params);
+
+	if ((stream_fs != 8000) && (stream_fs != 16000)) {
+		dev_err(afe->dev, "%s() btmgr not supprt this stream_fs %d\n",
+			__func__, stream_fs);
+		return -EINVAL;
+	}
+
+	regmap_update_bits(afe->regmap, AFE_MRGIF_CON,
+			   AFE_MRGIF_CON_I2S_MODE_MASK,
+			   AFE_MRGIF_CON_I2S_MODE_32K);
+
+	val = AFE_DAIBT_CON0_BT_FUNC_EN | AFE_DAIBT_CON0_BT_FUNC_RDY
+	      | AFE_DAIBT_CON0_MRG_USE;
+	msk = val;
+
+	if (stream_fs == 16000)
+		val |= AFE_DAIBT_CON0_BT_WIDE_MODE_EN;
+
+	msk |= AFE_DAIBT_CON0_BT_WIDE_MODE_EN;
+
+	regmap_update_bits(afe->regmap, AFE_DAIBT_CON0, msk, val);
+
+	regmap_update_bits(afe->regmap, AFE_DAIBT_CON0,
+			   AFE_DAIBT_CON0_DAIBT_EN,
+			   AFE_DAIBT_CON0_DAIBT_EN);
+	regmap_update_bits(afe->regmap, AFE_MRGIF_CON,
+			   AFE_MRGIF_CON_MRG_I2S_EN,
+			   AFE_MRGIF_CON_MRG_I2S_EN);
+	regmap_update_bits(afe->regmap, AFE_MRGIF_CON,
+			   AFE_MRGIF_CON_MRG_EN,
+			   AFE_MRGIF_CON_MRG_EN);
+	return 0;
+}
+
+static void mt2701_btmrg_shutdown(struct snd_pcm_substream *substream,
+				  struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct mtk_base_afe *afe = snd_soc_platform_get_drvdata(rtd->platform);
+	struct mt2701_afe_private *afe_priv = afe->platform_priv;
+
+	/* if the other direction stream is not occupied */
+	if (!afe_priv->mrg_enable[!substream->stream]) {
+		regmap_update_bits(afe->regmap, AFE_DAIBT_CON0,
+				   AFE_DAIBT_CON0_DAIBT_EN, 0);
+		regmap_update_bits(afe->regmap, AFE_MRGIF_CON,
+				   AFE_MRGIF_CON_MRG_EN, 0);
+		regmap_update_bits(afe->regmap, AFE_MRGIF_CON,
+				   AFE_MRGIF_CON_MRG_I2S_EN, 0);
+		regmap_update_bits(afe->regmap, AUDIO_TOP_CON4,
+				   AUDIO_TOP_CON4_PDN_MRGIF,
+				   AUDIO_TOP_CON4_PDN_MRGIF);
+	}
+	afe_priv->mrg_enable[substream->stream] = 0;
+}
+
 static int mt2701_simple_fe_startup(struct snd_pcm_substream *substream,
 				    struct snd_soc_dai *dai)
 {
@@ -514,6 +594,13 @@  static const struct snd_soc_dai_ops mt2701_afe_i2s_ops = {
 	.set_sysclk	= mt2701_afe_i2s_set_sysclk,
 };
 
+/* MRG BE DAIs */
+static struct snd_soc_dai_ops mt2701_btmrg_ops = {
+	.startup = mt2701_btmrg_startup,
+	.shutdown = mt2701_btmrg_shutdown,
+	.hw_params = mt2701_btmrg_hw_params,
+};
+
 static struct snd_soc_dai_driver mt2701_afe_pcm_dais[] = {
 	/* FE DAIs: memory intefaces to CPU */
 	{
@@ -566,6 +653,36 @@  static struct snd_soc_dai_driver mt2701_afe_pcm_dais[] = {
 		},
 		.ops = &mt2701_single_memif_dai_ops,
 	},
+	{
+		.name = "PCM_BT_DL",
+		.id = MT2701_MEMIF_DLBT,
+		.suspend = mtk_afe_dai_suspend,
+		.resume = mtk_afe_dai_resume,
+		.playback = {
+			.stream_name = "DLBT",
+			.channels_min = 1,
+			.channels_max = 1,
+			.rates = (SNDRV_PCM_RATE_8000
+				| SNDRV_PCM_RATE_16000),
+			.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		},
+		.ops = &mt2701_single_memif_dai_ops,
+	},
+	{
+		.name = "PCM_BT_UL",
+		.id = MT2701_MEMIF_ULBT,
+		.suspend = mtk_afe_dai_suspend,
+		.resume = mtk_afe_dai_resume,
+		.capture = {
+			.stream_name = "ULBT",
+			.channels_min = 1,
+			.channels_max = 1,
+			.rates = (SNDRV_PCM_RATE_8000
+				| SNDRV_PCM_RATE_16000),
+			.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		},
+		.ops = &mt2701_single_memif_dai_ops,
+	},
 	/* BE DAIs */
 	{
 		.name = "I2S0",
@@ -665,6 +782,28 @@  static struct snd_soc_dai_driver mt2701_afe_pcm_dais[] = {
 		.ops = &mt2701_afe_i2s_ops,
 		.symmetric_rates = 1,
 	},
+	{
+		.name = "MRG BT",
+		.id = MT2701_IO_MRG,
+		.playback = {
+			.stream_name = "BT Playback",
+			.channels_min = 1,
+			.channels_max = 1,
+			.rates = (SNDRV_PCM_RATE_8000
+				| SNDRV_PCM_RATE_16000),
+			.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		},
+		.capture = {
+			.stream_name = "BT Capture",
+			.channels_min = 1,
+			.channels_max = 1,
+			.rates = (SNDRV_PCM_RATE_8000
+				| SNDRV_PCM_RATE_16000),
+			.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		},
+		.ops = &mt2701_btmrg_ops,
+		.symmetric_rates = 1,
+	}
 };
 
 static const struct snd_kcontrol_new mt2701_afe_o00_mix[] = {