[PATCH 0/2] ASoC: Dell IoT Gateway audio support

[PATCH 0/2] ASoC: Dell IoT Gateway audio support

[Shrirang Bagul]

These patches add basic audio support on new Dell Edge IoT Gateways.
RT5660 codec is connected to SSP2 port on Intel Atom Baytrail E3815 SoC.
The machine driver is based on bytcr_rt5640. These devices have line-in and
line-out jacks with line-out mute enable gpio described in ACPI tables.

Shrirang Bagul (2):
  ASoC: rt5660: Add ACPI support
  ASoC: Intel: boards: Add Baytrail RT5660 machine driver

 sound/soc/codecs/rt5660.c             |  26 ++
 sound/soc/intel/Kconfig               |  13 +
 sound/soc/intel/atom/sst/sst_acpi.c   |   2 +
 sound/soc/intel/boards/Makefile       |   2 +
 sound/soc/intel/boards/bytcr_rt5660.c | 573 ++++++++++++++++++++++++++++++++++
 5 files changed, 616 insertions(+)
 create mode 100644 sound/soc/intel/boards/bytcr_rt5660.c

-- 
2.9.3

[PATCH 1/2] ASoC: rt5660: Add ACPI support

[Shrirang Bagul]

On Dell IoT Gateways, RT5660 codec is available with ACPI ID 10EC3277.
Also, GPIO's are only available by index, so we register mappings to allow
machine drivers to access them by name.

Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
---
 sound/soc/codecs/rt5660.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/sound/soc/codecs/rt5660.c b/sound/soc/codecs/rt5660.c
index 76cf76a..c8bdeb3 100644
--- a/sound/soc/codecs/rt5660.c
+++ b/sound/soc/codecs/rt5660.c
@@ -9,6 +9,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/acpi.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/init.h>
@@ -1245,10 +1246,31 @@ MODULE_DEVICE_TABLE(of, rt5660_of_match);
 
 static const struct acpi_device_id rt5660_acpi_match[] = {
 	{ "10EC5660", 0 },
+	{ "10EC3277", 0 },
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, rt5660_acpi_match);
 
+static const struct acpi_gpio_params audio_wake_intr_gpio = { 0, 0, false };
+static const struct acpi_gpio_params lineout_mute_gpio = { 1, 0, true };
+
+static const struct acpi_gpio_mapping byt_rt5660_gpios[] = {
+	{ "audio-wake-intr-gpios", &audio_wake_intr_gpio, 1 },
+	{ "lineout-mute-gpios", &lineout_mute_gpio, 1 },
+	{ NULL },
+};
+
+static void rt5660_read_acpi_properties(struct rt5660_priv *rt5660,
+		struct device *dev)
+{
+	int ret;
+
+	ret = acpi_dev_add_driver_gpios(ACPI_COMPANION(dev),
+			byt_rt5660_gpios);
+	if (ret)
+		dev_warn(dev, "Failed to add driver gpios\n");
+}
+
 static int rt5660_parse_dt(struct rt5660_priv *rt5660, struct device *dev)
 {
 	rt5660->pdata.in1_diff = device_property_read_bool(dev,
@@ -1288,6 +1310,10 @@ static int rt5660_i2c_probe(struct i2c_client *i2c,
 		rt5660->pdata = *pdata;
 	else if (i2c->dev.of_node)
 		rt5660_parse_dt(rt5660, &i2c->dev);
+	else if (ACPI_HANDLE(&i2c->dev))
+		rt5660_read_acpi_properties(rt5660, &i2c->dev);
+	else
+		return -EINVAL;
 
 	rt5660->regmap = devm_regmap_init_i2c(i2c, &rt5660_regmap);
 	if (IS_ERR(rt5660->regmap)) {
-- 
2.9.3

[PATCH 2/2] ASoC: Intel: boards: Add Baytrail RT5660 machine driver

[Shrirang Bagul]

This is used by Dell Edge IoT Gateways.

Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com>
---
 sound/soc/intel/Kconfig               |  13 +
 sound/soc/intel/atom/sst/sst_acpi.c   |   2 +
 sound/soc/intel/boards/Makefile       |   2 +
 sound/soc/intel/boards/bytcr_rt5660.c | 573 ++++++++++++++++++++++++++++++++++
 4 files changed, 590 insertions(+)
 create mode 100644 sound/soc/intel/boards/bytcr_rt5660.c

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index fd5d1e0..04027e6 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -172,6 +172,19 @@ config SND_SOC_INTEL_BYTCR_RT5651_MACH
           Say Y if you have such a device.
           If unsure select "N".
 
+config SND_SOC_INTEL_BYTCR_RT5660_MACH
+	tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with RT5660 codec"
+	depends on X86 && I2C && ACPI
+	select SND_SOC_RT5660
+	select SND_SST_MFLD_PLATFORM
+	select SND_SST_IPC_ACPI
+	select SND_SOC_INTEL_SST_MATCH if ACPI
+	help
+	  This adds support for ASoC machine driver for Intel(R) Baytrail and Baytrail-CR
+	  platforms with RT5660 audio codec.
+	  Say Y if you have such a device.
+	  If unsure select "N".
+
 config SND_SOC_INTEL_CHT_BSW_RT5672_MACH
         tristate "ASoC Audio driver for Intel Cherrytrail & Braswell with RT5672 codec"
         depends on X86_INTEL_LPSS && I2C && ACPI
diff --git a/sound/soc/intel/atom/sst/sst_acpi.c b/sound/soc/intel/atom/sst/sst_acpi.c
index f4d92bb..a7f8400 100644
--- a/sound/soc/intel/atom/sst/sst_acpi.c
+++ b/sound/soc/intel/atom/sst/sst_acpi.c
@@ -445,6 +445,8 @@ static struct sst_acpi_mach sst_acpi_bytcr[] = {
 						&byt_rvp_platform_data },
 	{"10EC5651", "bytcr_rt5651", "intel/fw_sst_0f28.bin", "bytcr_rt5651", NULL,
 						&byt_rvp_platform_data },
+	{"10EC3277", "bytcr_rt5660", "intel/fw_sst_0f28.bin", "bytcr_rt5660", NULL,
+						&byt_rvp_platform_data },
 	{},
 };
 
diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
index 5639f10..32255ee 100644
--- a/sound/soc/intel/boards/Makefile
+++ b/sound/soc/intel/boards/Makefile
@@ -7,6 +7,7 @@ snd-soc-sst-bxt-da7219_max98357a-objs := bxt_da7219_max98357a.o
 snd-soc-sst-bxt-rt298-objs := bxt_rt298.o
 snd-soc-sst-bytcr-rt5640-objs := bytcr_rt5640.o
 snd-soc-sst-bytcr-rt5651-objs := bytcr_rt5651.o
+snd-soc-sst-bytcr-rt5660-objs := bytcr_rt5660.o
 snd-soc-sst-cht-bsw-rt5672-objs := cht_bsw_rt5672.o
 snd-soc-sst-cht-bsw-rt5645-objs := cht_bsw_rt5645.o
 snd-soc-sst-cht-bsw-max98090_ti-objs := cht_bsw_max98090_ti.o
@@ -23,6 +24,7 @@ obj-$(CONFIG_SND_SOC_INTEL_BROADWELL_MACH) += snd-soc-sst-broadwell.o
 obj-$(CONFIG_SND_SOC_INTEL_BDW_RT5677_MACH) += snd-soc-sst-bdw-rt5677-mach.o
 obj-$(CONFIG_SND_SOC_INTEL_BYTCR_RT5640_MACH) += snd-soc-sst-bytcr-rt5640.o
 obj-$(CONFIG_SND_SOC_INTEL_BYTCR_RT5651_MACH) += snd-soc-sst-bytcr-rt5651.o
+obj-$(CONFIG_SND_SOC_INTEL_BYTCR_RT5660_MACH) += snd-soc-sst-bytcr-rt5660.o
 obj-$(CONFIG_SND_SOC_INTEL_CHT_BSW_RT5672_MACH) += snd-soc-sst-cht-bsw-rt5672.o
 obj-$(CONFIG_SND_SOC_INTEL_CHT_BSW_RT5645_MACH) += snd-soc-sst-cht-bsw-rt5645.o
 obj-$(CONFIG_SND_SOC_INTEL_CHT_BSW_MAX98090_TI_MACH) += snd-soc-sst-cht-bsw-max98090_ti.o
diff --git a/sound/soc/intel/boards/bytcr_rt5660.c b/sound/soc/intel/boards/bytcr_rt5660.c
new file mode 100644
index 0000000..0cd989d
--- /dev/null
+++ b/sound/soc/intel/boards/bytcr_rt5660.c
@@ -0,0 +1,573 @@
+/*
+ *  Intel Baytrail SST RT5660 machine driver
+ *  Copyright (C) 2016 Shrirang Bagul <shrirang.bagul@canonical.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ */
+
+#define DEBUG
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/dmi.h>
+#include <linux/slab.h>
+#include <asm/cpu_device_id.h>
+#include <asm/platform_sst_audio.h>
+#include <linux/clk.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/jack.h>
+#include "../../codecs/rt5660.h"
+#include "../atom/sst-atom-controls.h"
+#include "../common/sst-acpi.h"
+#include "../common/sst-dsp.h"
+
+#define BYT_RT5660_MAP(quirk)	((quirk) & 0xff)
+#define BYT_RT5660_SSP0_AIF1	BIT(16)
+#define BYT_RT5660_MCLK_EN	BIT(17)
+#define BYT_RT5660_MCLK_25MHZ	BIT(18)
+
+struct byt_rt5660_private {
+	struct clk *mclk;
+	struct gpio_desc *gpio_lo_mute;
+};
+
+static unsigned long byt_rt5660_quirk = (BYT_RT5660_MCLK_EN |
+		BYT_RT5660_MCLK_25MHZ);
+
+static void log_quirks(struct device *dev)
+{
+	if (byt_rt5660_quirk & BYT_RT5660_MCLK_EN)
+		dev_info(dev, "quirk MCLK_EN enabled");
+	if (byt_rt5660_quirk & BYT_RT5660_MCLK_25MHZ)
+		dev_info(dev, "quirk MCLK_25MHZ enabled");
+	if (byt_rt5660_quirk & BYT_RT5660_SSP0_AIF1)
+		dev_info(dev, "quirk SSP0_AIF1 enabled");
+}
+
+#define BYT_CODEC_DAI1	"rt5660-aif1"
+
+static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card)
+{
+	struct snd_soc_pcm_runtime *rtd;
+
+	list_for_each_entry(rtd, &card->rtd_list, list) {
+		if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI1,
+			     strlen(BYT_CODEC_DAI1)))
+			return rtd->codec_dai;
+	}
+	return NULL;
+}
+
+static int platform_clock_control(struct snd_soc_dapm_widget *w,
+				  struct snd_kcontrol *k, int  event)
+{
+	struct snd_soc_dapm_context *dapm = w->dapm;
+	struct snd_soc_card *card = dapm->card;
+	struct snd_soc_dai *codec_dai;
+	struct byt_rt5660_private *priv = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	codec_dai = byt_get_codec_dai(card);
+	if (!codec_dai) {
+		dev_err(card->dev,
+			"Codec dai not found; Unable to set platform clock\n");
+		return -EIO;
+	}
+
+	if (SND_SOC_DAPM_EVENT_ON(event)) {
+		if ((byt_rt5660_quirk & BYT_RT5660_MCLK_EN) && priv->mclk) {
+			ret = clk_prepare_enable(priv->mclk);
+			if (ret < 0) {
+				dev_err(card->dev,
+					"could not configure MCLK state");
+				return ret;
+			}
+		}
+		ret = snd_soc_dai_set_sysclk(codec_dai, RT5660_SCLK_S_PLL1,
+					     48000 * 512,
+					     SND_SOC_CLOCK_IN);
+	} else {
+		/*
+		 * Set codec clock source to internal clock before
+		 * turning off the platform clock. Codec needs clock
+		 * for Jack detection and button press
+		 */
+		ret = snd_soc_dai_set_sysclk(codec_dai, RT5660_SCLK_S_RCCLK,
+					     0,
+					     SND_SOC_CLOCK_IN);
+		if (!ret) {
+			if ((byt_rt5660_quirk & BYT_RT5660_MCLK_EN) && priv->mclk)
+				clk_disable_unprepare(priv->mclk);
+		}
+	}
+
+	if (ret < 0) {
+		dev_err(card->dev, "can't set codec sysclk: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int byt_rt5660_event_lineout(struct snd_soc_dapm_widget *w,
+			struct snd_kcontrol *k, int event)
+{
+	struct snd_soc_dapm_context *dapm = w->dapm;
+	struct snd_soc_card *card = dapm->card;
+	struct byt_rt5660_private *priv = snd_soc_card_get_drvdata(card);
+
+	gpiod_set_value_cansleep(priv->gpio_lo_mute,
+			!(SND_SOC_DAPM_EVENT_ON(event)));
+
+	return 0;
+}
+
+static const struct snd_soc_dapm_widget byt_rt5660_widgets[] = {
+	SND_SOC_DAPM_MIC("Line In", NULL),
+	SND_SOC_DAPM_LINE("Line Out", byt_rt5660_event_lineout),
+	SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
+			platform_clock_control, SND_SOC_DAPM_PRE_PMU |
+			SND_SOC_DAPM_POST_PMD),
+};
+
+static const struct snd_soc_dapm_route byt_rt5660_audio_map[] = {
+	{"IN1P", NULL, "Line In"},
+	{"IN2P", NULL, "Line In"},
+	{"Line Out", NULL, "LOUTR"},
+	{"Line Out", NULL, "LOUTL"},
+
+	{"Line In", NULL, "Platform Clock"},
+	{"Line Out", NULL, "Platform Clock"},
+};
+
+static const struct snd_soc_dapm_route byt_rt5660_ssp2_aif1_map[] = {
+	{"ssp2 Tx", NULL, "codec_out0"},
+	{"ssp2 Tx", NULL, "codec_out1"},
+	{"codec_in0", NULL, "ssp2 Rx"},
+	{"codec_in1", NULL, "ssp2 Rx"},
+
+	{"AIF1 Playback", NULL, "ssp2 Tx"},
+	{"ssp2 Rx", NULL, "AIF1 Capture"},
+};
+
+static const struct snd_soc_dapm_route byt_rt5660_ssp0_aif1_map[] = {
+	{"ssp0 Tx", NULL, "modem_out"},
+	{"modem_in", NULL, "ssp0 Rx"},
+
+	{"AIF1 Playback", NULL, "ssp0 Tx"},
+	{"ssp0 Rx", NULL, "AIF1 Capture"},
+};
+
+static const struct snd_kcontrol_new byt_rt5660_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Line In"),
+	SOC_DAPM_PIN_SWITCH("Line Out"),
+};
+
+static int byt_rt5660_aif1_hw_params(struct snd_pcm_substream *substream,
+				struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	int ret;
+
+	ret = snd_soc_dai_set_sysclk(codec_dai, RT5660_SCLK_S_PLL1,
+				     params_rate(params) * 256,
+				     SND_SOC_CLOCK_IN);
+	if (ret < 0) {
+		dev_err(codec_dai->dev, "can't set codec clock %d\n", ret);
+		return ret;
+	}
+
+	if (!(byt_rt5660_quirk & BYT_RT5660_MCLK_EN)) {
+		/* use bitclock as PLL input */
+		if ((byt_rt5660_quirk & BYT_RT5660_SSP0_AIF1)) {
+			/* 2x16 bit slots on SSP0 */
+			ret = snd_soc_dai_set_pll(codec_dai, 0,
+						RT5660_PLL1_S_BCLK,
+						params_rate(params) * 32,
+						params_rate(params) * 512);
+		} else {
+			/* 2x15 bit slots on SSP2 */
+			ret = snd_soc_dai_set_pll(codec_dai, 0,
+						RT5660_PLL1_S_BCLK,
+						params_rate(params) * 50,
+						params_rate(params) * 512);
+		}
+	} else {
+		if (byt_rt5660_quirk & BYT_RT5660_MCLK_25MHZ) {
+			ret = snd_soc_dai_set_pll(codec_dai, 0,
+					RT5660_SCLK_S_MCLK,
+					25000000,
+					params_rate(params) * 512);
+		} else {
+			ret = snd_soc_dai_set_pll(codec_dai, 0,
+					RT5660_SCLK_S_MCLK,
+					19200000,
+					params_rate(params) * 512);
+		}
+	}
+
+	if (ret < 0) {
+		dev_err(codec_dai->dev, "can't set codec pll: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int byt_rt5660_quirk_cb(const struct dmi_system_id *id)
+{
+	byt_rt5660_quirk = (unsigned long)id->driver_data;
+	return 1;
+}
+
+static const struct dmi_system_id byt_rt5660_quirk_table[] = {
+	{
+		.callback = byt_rt5660_quirk_cb,
+		.matches = {
+			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Edge Gateway 3003"),
+		},
+		.driver_data = (unsigned long *)(BYT_RT5660_MCLK_EN |
+				BYT_RT5660_MCLK_25MHZ),
+	},
+	{},
+};
+
+static int byt_rt5660_init(struct snd_soc_pcm_runtime *runtime)
+{
+	int ret;
+	struct snd_soc_card *card = runtime->card;
+	struct byt_rt5660_private *priv = snd_soc_card_get_drvdata(card);
+	struct snd_soc_codec *codec = runtime->codec;
+
+	card->dapm.idle_bias_off = true;
+
+	/* Request rt5660 GPIO for lineout mute control */
+	priv->gpio_lo_mute = devm_gpiod_get_index(codec->dev,
+			"lineout-mute", 0, 0);
+	if (IS_ERR(priv->gpio_lo_mute)) {
+		dev_err(card->dev, "Can't find GPIO_MUTE# gpio\n");
+		return PTR_ERR(priv->gpio_lo_mute);
+	}
+	gpiod_direction_output(priv->gpio_lo_mute, 1);
+
+	ret = snd_soc_add_card_controls(card, byt_rt5660_controls,
+					ARRAY_SIZE(byt_rt5660_controls));
+	if (ret) {
+		dev_err(card->dev, "unable to add card controls\n");
+		return ret;
+	}
+
+	if (byt_rt5660_quirk & BYT_RT5660_SSP0_AIF1) {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5660_ssp0_aif1_map,
+					ARRAY_SIZE(byt_rt5660_ssp0_aif1_map));
+	} else {
+		ret = snd_soc_dapm_add_routes(&card->dapm,
+					byt_rt5660_ssp2_aif1_map,
+					ARRAY_SIZE(byt_rt5660_ssp2_aif1_map));
+	}
+	if (ret)
+		return ret;
+
+	snd_soc_dapm_ignore_suspend(&card->dapm, "Line Out");
+	snd_soc_dapm_ignore_suspend(&card->dapm, "Line In");
+	snd_soc_dapm_enable_pin_unlocked(&card->dapm, "Line Out");
+	snd_soc_dapm_enable_pin_unlocked(&card->dapm, "Line In");
+
+	if ((byt_rt5660_quirk & BYT_RT5660_MCLK_EN) && priv->mclk) {
+		/*
+		 * The firmware might enable the clock at
+		 * boot (this information may or may not
+		 * be reflected in the enable clock register).
+		 * To change the rate we must disable the clock
+		 * first to cover these cases. Due to common
+		 * clock framework restrictions that do not allow
+		 * to disable a clock that has not been enabled,
+		 * we need to enable the clock first.
+		 */
+		ret = clk_prepare_enable(priv->mclk);
+		if (!ret)
+			clk_disable_unprepare(priv->mclk);
+
+		if (byt_rt5660_quirk & BYT_RT5660_MCLK_25MHZ)
+			ret = clk_set_rate(priv->mclk, 25000000);
+		else
+			ret = clk_set_rate(priv->mclk, 19200000);
+
+		if (ret)
+			dev_err(card->dev, "unable to set MCLK rate\n");
+	}
+
+	return ret;
+}
+
+static const struct snd_soc_pcm_stream byt_rt5660_dai_params = {
+	.formats = SNDRV_PCM_FMTBIT_S24_LE,
+	.rate_min = 48000,
+	.rate_max = 48000,
+	.channels_min = 2,
+	.channels_max = 2,
+};
+
+static int byt_rt5660_codec_fixup(struct snd_soc_pcm_runtime *rtd,
+			    struct snd_pcm_hw_params *params)
+{
+	struct snd_interval *rate = hw_param_interval(params,
+			SNDRV_PCM_HW_PARAM_RATE);
+	struct snd_interval *channels = hw_param_interval(params,
+						SNDRV_PCM_HW_PARAM_CHANNELS);
+	int ret;
+
+	/* The DSP will covert the FE rate to 48k, stereo */
+	rate->min = rate->max = 48000;
+	channels->min = channels->max = 2;
+
+	if ((byt_rt5660_quirk & BYT_RT5660_SSP0_AIF1)) {
+
+		/* set SSP0 to 16-bit */
+		params_set_format(params, SNDRV_PCM_FORMAT_S16_LE);
+
+		/*
+		 * Default mode for SSP configuration is TDM 4 slot, override config
+		 * with explicit setting to I2S 2ch 16-bit. The word length is set with
+		 * dai_set_tdm_slot() since there is no other API exposed
+		 */
+		ret = snd_soc_dai_set_fmt(rtd->cpu_dai,
+					SND_SOC_DAIFMT_I2S     |
+					SND_SOC_DAIFMT_NB_IF   |
+					SND_SOC_DAIFMT_CBS_CFS
+			);
+		if (ret < 0) {
+			dev_err(rtd->dev, "can't set format to I2S, err %d\n", ret);
+			return ret;
+		}
+
+		ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, 16);
+		if (ret < 0) {
+			dev_err(rtd->dev, "can't set I2S config, err %d\n", ret);
+			return ret;
+		}
+
+	} else {
+
+		/* set SSP2 to 24-bit */
+		params_set_format(params, SNDRV_PCM_FORMAT_S24_LE);
+
+		/*
+		 * Default mode for SSP configuration is TDM 4 slot, override config
+		 * with explicit setting to I2S 2ch 24-bit. The word length is set with
+		 * dai_set_tdm_slot() since there is no other API exposed
+		 */
+		ret = snd_soc_dai_set_fmt(rtd->cpu_dai,
+					SND_SOC_DAIFMT_I2S     |
+					SND_SOC_DAIFMT_NB_IF   |
+					SND_SOC_DAIFMT_CBS_CFS
+			);
+		if (ret < 0) {
+			dev_err(rtd->dev, "can't set format to I2S, err %d\n", ret);
+			return ret;
+		}
+
+		ret = snd_soc_dai_set_tdm_slot(rtd->cpu_dai, 0x3, 0x3, 2, 24);
+		if (ret < 0) {
+			dev_err(rtd->dev, "can't set I2S config, err %d\n", ret);
+			return ret;
+		}
+	}
+	return 0;
+}
+
+static int byt_rt5660_aif1_startup(struct snd_pcm_substream *substream)
+{
+	return snd_pcm_hw_constraint_single(substream->runtime,
+			SNDRV_PCM_HW_PARAM_RATE, 48000);
+}
+
+static struct snd_soc_ops byt_rt5660_aif1_ops = {
+	.startup = byt_rt5660_aif1_startup,
+};
+
+static struct snd_soc_ops byt_rt5660_be_ssp2_ops = {
+	.hw_params = byt_rt5660_aif1_hw_params,
+};
+
+static struct snd_soc_dai_link byt_rt5660_dais[] = {
+	[MERR_DPCM_AUDIO] = {
+		.name = "Baytrail Audio Port",
+		.stream_name = "Baytrail Audio",
+		.cpu_dai_name = "media-cpu-dai",
+		.codec_dai_name = "snd-soc-dummy-dai",
+		.codec_name = "snd-soc-dummy",
+		.platform_name = "sst-mfld-platform",
+		.ignore_suspend = 1,
+		.dynamic = 1,
+		.dpcm_playback = 1,
+		.dpcm_capture = 1,
+		.ops = &byt_rt5660_aif1_ops,
+	},
+	[MERR_DPCM_DEEP_BUFFER] = {
+		.name = "Deep-Buffer Audio Port",
+		.stream_name = "Deep-Buffer Audio",
+		.cpu_dai_name = "deepbuffer-cpu-dai",
+		.codec_dai_name = "snd-soc-dummy-dai",
+		.codec_name = "snd-soc-dummy",
+		.platform_name = "sst-mfld-platform",
+		.ignore_suspend = 1,
+		.nonatomic = true,
+		.dynamic = 1,
+		.dpcm_playback = 1,
+		.ops = &byt_rt5660_aif1_ops,
+	},
+	[MERR_DPCM_COMPR] = {
+		.name = "Baytrail Compressed Port",
+		.stream_name = "Baytrail Compress",
+		.cpu_dai_name = "compress-cpu-dai",
+		.codec_dai_name = "snd-soc-dummy-dai",
+		.codec_name = "snd-soc-dummy",
+		.platform_name = "sst-mfld-platform",
+	},
+		/* back ends */
+	{
+		.name = "SSP2-Codec",
+		.id = 1,
+		.cpu_dai_name = "ssp2-port", /* overwritten for ssp0 routing */
+		.platform_name = "sst-mfld-platform",
+		.no_pcm = 1,
+		.codec_dai_name = "rt5660-aif1",
+		.codec_name = "i2c-10EC5660:00", /* overwritten with HID */
+		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
+						| SND_SOC_DAIFMT_CBS_CFS,
+		.be_hw_params_fixup = byt_rt5660_codec_fixup,
+		.ignore_suspend = 1,
+		.dpcm_playback = 1,
+		.dpcm_capture = 1,
+		.init = byt_rt5660_init,
+		.ops = &byt_rt5660_be_ssp2_ops,
+	},
+};
+
+static struct snd_soc_card byt_rt5660_card = {
+	.name = "byt-rt5660",
+	.owner = THIS_MODULE,
+	.dai_link = byt_rt5660_dais,
+	.num_links = ARRAY_SIZE(byt_rt5660_dais),
+	.dapm_widgets = byt_rt5660_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(byt_rt5660_widgets),
+	.dapm_routes = byt_rt5660_audio_map,
+	.num_dapm_routes = ARRAY_SIZE(byt_rt5660_audio_map),
+	.fully_routed = true,
+};
+
+static char byt_rt5660_codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */
+static char byt_rt5660_cpu_dai_name[10]; /*  = "ssp[0|2]-port" */
+
+static bool is_valleyview(void)
+{
+	static const struct x86_cpu_id cpu_ids[] = {
+		{ X86_VENDOR_INTEL, 6, 55 }, /* Valleyview, Bay Trail */
+		{}
+	};
+
+	if (!x86_match_cpu(cpu_ids))
+		return false;
+	return true;
+}
+
+static int byt_rt5660_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &byt_rt5660_card;
+	struct byt_rt5660_private *priv;
+	struct sst_acpi_mach *mach;
+	const char *i2c_name = NULL;
+	int ret_val = 0;
+	int i, dai_index;
+
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC);
+	if (!priv)
+		return -ENOMEM;
+
+	card->dev = &pdev->dev;
+	mach = byt_rt5660_card.dev->platform_data;
+	snd_soc_card_set_drvdata(card, priv);
+
+	/* fix index of codec dai */
+	dai_index = MERR_DPCM_COMPR + 1;
+	for (i = 0; i < ARRAY_SIZE(byt_rt5660_dais); i++) {
+		if (!strcmp(byt_rt5660_dais[i].codec_name, "i2c-10EC5660:00")) {
+			dai_index = i;
+			break;
+		}
+	}
+
+	/* fixup codec name based on HID */
+	i2c_name = sst_acpi_find_name_from_hid(mach->id);
+	if (i2c_name != NULL) {
+		snprintf(byt_rt5660_codec_name, sizeof(byt_rt5660_codec_name),
+			"%s%s", "i2c-", i2c_name);
+
+		byt_rt5660_dais[dai_index].codec_name = byt_rt5660_codec_name;
+	}
+
+	dmi_check_system(byt_rt5660_quirk_table);
+	log_quirks(&pdev->dev);
+
+	if ((byt_rt5660_quirk & BYT_RT5660_SSP0_AIF1)) {
+		/* fixup cpu dai name name */
+		snprintf(byt_rt5660_cpu_dai_name,
+			sizeof(byt_rt5660_cpu_dai_name),
+			"%s", "ssp0-port");
+
+		byt_rt5660_dais[dai_index].cpu_dai_name =
+			byt_rt5660_cpu_dai_name;
+	}
+
+	if ((byt_rt5660_quirk & BYT_RT5660_MCLK_EN) && (is_valleyview())) {
+		priv->mclk = devm_clk_get(&pdev->dev, "pmc_plt_clk_3");
+		if (IS_ERR(priv->mclk)) {
+			dev_err(&pdev->dev,
+					"Failed to get MCLK from pmc_plt_clk_3 :  % ld\n",
+					PTR_ERR(priv->mclk));
+			return PTR_ERR(priv->mclk);
+		}
+	}
+
+	ret_val = devm_snd_soc_register_card(&pdev->dev, card);
+	if (ret_val) {
+		dev_err(&pdev->dev, "devm_snd_soc_register_card failed % d\n",
+				ret_val);
+		return ret_val;
+	}
+	platform_set_drvdata(pdev, card);
+	return ret_val;
+}
+
+static struct platform_driver byt_rt5660_audio = {
+	.probe = byt_rt5660_probe,
+	.driver = {
+		.name = "bytcr_rt5660",
+		.pm = &snd_soc_pm_ops,
+	},
+};
+module_platform_driver(byt_rt5660_audio)
+
+MODULE_DESCRIPTION("ASoC Intel(R) Baytrail CR Machine driver");
+MODULE_AUTHOR("Shrirang Bagul");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:bytcr_rt5660");
-- 
2.9.3

Re: [PATCH 1/2] ASoC: rt5660: Add ACPI support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

On Mon, Dec 19, 2016 at 09:51:46PM +0800, Shrirang Bagul wrote:

> +static const struct acpi_gpio_params audio_wake_intr_gpio = { 0, 0, false };
> +static const struct acpi_gpio_params lineout_mute_gpio = { 1, 0, true };

Can we please write these in a fashion more idiomatic for the kernel and
useful for human readers with named struct fields?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] ASoC: Intel: boards: Add Baytrail RT5660 machine driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2851 bytes --]

On Mon, Dec 19, 2016 at 09:51:47PM +0800, Shrirang Bagul wrote:

> +
> +#define DEBUG

This should be production code...

> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/dmi.h>
> +#include <linux/slab.h>
> +#include <asm/cpu_device_id.h>
> +#include <asm/platform_sst_audio.h>
> +#include <linux/clk.h>

The ordering of the headers is very random here, and is there a matching
platform patch somewhere registering clocks?

> +		/*
> +		 * Set codec clock source to internal clock before
> +		 * turning off the platform clock. Codec needs clock
> +		 * for Jack detection and button press
> +		 */
> +		ret = snd_soc_dai_set_sysclk(codec_dai, RT5660_SCLK_S_RCCLK,
> +					     0,
> +					     SND_SOC_CLOCK_IN);
> +		if (!ret) {
> +			if ((byt_rt5660_quirk & BYT_RT5660_MCLK_EN) && priv->mclk)
> +				clk_disable_unprepare(priv->mclk);
> +		}

Please write the error handling normally and don't embed the next steps
of the process in where the error handling would usually be.

> +	ret = snd_soc_add_card_controls(card, byt_rt5660_controls,
> +					ARRAY_SIZE(byt_rt5660_controls));
> +	if (ret) {
> +		dev_err(card->dev, "unable to add card controls\n");
> +		return ret;
> +	}

Why not initialize these from the card struct?

> +	if ((byt_rt5660_quirk & BYT_RT5660_MCLK_EN) && priv->mclk) {
> +		/*
> +		 * The firmware might enable the clock at
> +		 * boot (this information may or may not
> +		 * be reflected in the enable clock register).
> +		 * To change the rate we must disable the clock
> +		 * first to cover these cases. Due to common
> +		 * clock framework restrictions that do not allow
> +		 * to disable a clock that has not been enabled,
> +		 * we need to enable the clock first.
> +		 */
> +		ret = clk_prepare_enable(priv->mclk);
> +		if (!ret)
> +			clk_disable_unprepare(priv->mclk);

We don't disable the clock after (apparently) temporarily enabling it
here.

> +		/*
> +		 * Default mode for SSP configuration is TDM 4 slot, override config
> +		 * with explicit setting to I2S 2ch 16-bit. The word length is set with
> +		 * dai_set_tdm_slot() since there is no other API exposed
> +		 */
> +		ret = snd_soc_dai_set_fmt(rtd->cpu_dai,
> +					SND_SOC_DAIFMT_I2S     |
> +					SND_SOC_DAIFMT_NB_IF   |
> +					SND_SOC_DAIFMT_CBS_CFS
> +			);

Please use a normal kernel coding style and please initialize this from
the dai_link too, it's always the same.

> +	/* fixup codec name based on HID */
> +	i2c_name = sst_acpi_find_name_from_hid(mach->id);
> +	if (i2c_name != NULL) {
> +		snprintf(byt_rt5660_codec_name, sizeof(byt_rt5660_codec_name),
> +			"%s%s", "i2c-", i2c_name);

There's no point in formatting a static string in here, just include it
in the string.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [alsa-devel] [PATCH 2/2] ASoC: Intel: boards: Add Baytrail RT5660 machine driver

[Pierre-Louis Bossart]

On 12/19/16 9:55 AM, Mark Brown wrote:
> On Mon, Dec 19, 2016 at 09:51:47PM +0800, Shrirang Bagul wrote:
>
>> +
>> +#define DEBUG
>
> This should be production code...
>
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/acpi.h>
>> +#include <linux/device.h>
>> +#include <linux/dmi.h>
>> +#include <linux/slab.h>
>> +#include <asm/cpu_device_id.h>
>> +#include <asm/platform_sst_audio.h>
>> +#include <linux/clk.h>
>
> The ordering of the headers is very random here, and is there a matching
> platform patch somewhere registering clocks?
>
>> +		/*
>> +		 * Set codec clock source to internal clock before
>> +		 * turning off the platform clock. Codec needs clock
>> +		 * for Jack detection and button press
>> +		 */
>> +		ret = snd_soc_dai_set_sysclk(codec_dai, RT5660_SCLK_S_RCCLK,
>> +					     0,
>> +					     SND_SOC_CLOCK_IN);
>> +		if (!ret) {
>> +			if ((byt_rt5660_quirk & BYT_RT5660_MCLK_EN) && priv->mclk)
>> +				clk_disable_unprepare(priv->mclk);
>> +		}
>
> Please write the error handling normally and don't embed the next steps
> of the process in where the error handling would usually be.
>
>> +	ret = snd_soc_add_card_controls(card, byt_rt5660_controls,
>> +					ARRAY_SIZE(byt_rt5660_controls));
>> +	if (ret) {
>> +		dev_err(card->dev, "unable to add card controls\n");
>> +		return ret;
>> +	}
>
> Why not initialize these from the card struct?
>
>> +	if ((byt_rt5660_quirk & BYT_RT5660_MCLK_EN) && priv->mclk) {
>> +		/*
>> +		 * The firmware might enable the clock at
>> +		 * boot (this information may or may not
>> +		 * be reflected in the enable clock register).
>> +		 * To change the rate we must disable the clock
>> +		 * first to cover these cases. Due to common
>> +		 * clock framework restrictions that do not allow
>> +		 * to disable a clock that has not been enabled,
>> +		 * we need to enable the clock first.
>> +		 */
>> +		ret = clk_prepare_enable(priv->mclk);
>> +		if (!ret)
>> +			clk_disable_unprepare(priv->mclk);
>
> We don't disable the clock after (apparently) temporarily enabling it
> here.
>
>> +		/*
>> +		 * Default mode for SSP configuration is TDM 4 slot, override config
>> +		 * with explicit setting to I2S 2ch 16-bit. The word length is set with
>> +		 * dai_set_tdm_slot() since there is no other API exposed
>> +		 */
>> +		ret = snd_soc_dai_set_fmt(rtd->cpu_dai,
>> +					SND_SOC_DAIFMT_I2S     |
>> +					SND_SOC_DAIFMT_NB_IF   |
>> +					SND_SOC_DAIFMT_CBS_CFS
>> +			);
>
> Please use a normal kernel coding style and please initialize this from
> the dai_link too, it's always the same.
>
>> +	/* fixup codec name based on HID */
>> +	i2c_name = sst_acpi_find_name_from_hid(mach->id);
>> +	if (i2c_name != NULL) {
>> +		snprintf(byt_rt5660_codec_name, sizeof(byt_rt5660_codec_name),
>> +			"%s%s", "i2c-", i2c_name);
>
> There's no point in formatting a static string in here, just include it
> in the string.

All this code seems to be largely a copy-paste of the bytcr_rt5640 
machine driver and the same comments would apply there. This patch did 
miss the last correction merged by Mark to deal with errors "ASoC: 
Intel: bytcr_rt5640: fallback mechanism if MCLK is not enabled" and the 
same error handling would be needed.
The clock registration part isn't in the mainline yet, barring a 
Christmas miracle we may want to wait until it's available before adding 
more machine drivers depending on it (I have 5 related changes for 
rt5651, rt5645, rt5670, max98090 and TI 32xx on my github).

Re: [alsa-devel] [PATCH 2/2] ASoC: Intel: boards: Add Baytrail RT5660 machine driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 631 bytes --]

On Mon, Dec 19, 2016 at 10:30:09AM -0600, Pierre-Louis Bossart wrote:

> All this code seems to be largely a copy-paste of the bytcr_rt5640 machine
> driver and the same comments would apply there. This patch did miss the last

Yes, there's a lot of room for cleanups in the existing code too (and of
course if there's such a large amount of cut'n'paste that implies that
there should be some code reuse going on).

> correction merged by Mark to deal with errors "ASoC: Intel: bytcr_rt5640:
> fallback mechanism if MCLK is not enabled" and the same error handling would
> be needed.

There was a cut back version of it I thought?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [alsa-devel] [PATCH 2/2] ASoC: Intel: boards: Add Baytrail RT5660 machine driver

[Shrirang Bagul]

[-- Attachment #1: Type: text/plain, Size: 972 bytes --]

On Mon, 2016-12-19 at 16:44 +0000, Mark Brown wrote:
> On Mon, Dec 19, 2016 at 10:30:09AM -0600, Pierre-Louis Bossart wrote:
> 
> > All this code seems to be largely a copy-paste of the bytcr_rt5640 machine
> > driver and the same comments would apply there. This patch did miss the last
> 
> Yes, there's a lot of room for cleanups in the existing code too (and of
> course if there's such a large amount of cut'n'paste that implies that
> there should be some code reuse going on).
Thank you for the review and valuable comments. Following the discussion so far,
I feel the proper way would be to adapt bytcr_rt5660  machine driver to manage
RT5660 codec.
> 
> > correction merged by Mark to deal with errors "ASoC: Intel: bytcr_rt5640:
> > fallback mechanism if MCLK is not enabled" and the same error handling would
> > be needed.
> 
> There was a cut back version of it I thought?
Will try and include the MCLK fallback patch in ver. 2 of the patch.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

Re: [PATCH 1/2] ASoC: rt5660: Add ACPI support

[Shrirang Bagul]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

On Mon, 2016-12-19 at 15:44 +0000, Mark Brown wrote:
> On Mon, Dec 19, 2016 at 09:51:46PM +0800, Shrirang Bagul wrote:
> 
> > +static const struct acpi_gpio_params audio_wake_intr_gpio = { 0, 0, false
> > };
> > +static const struct acpi_gpio_params lineout_mute_gpio = { 1, 0, true };
> 
> Can we please write these in a fashion more idiomatic for the kernel and
> useful for human readers with named struct fields?
Okay, will update in ver. 2 of the patch. Thanks.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 841 bytes --]

Re: [alsa-devel] [PATCH 2/2] ASoC: Intel: boards: Add Baytrail RT5660 machine driver

[Shrirang Bagul]

[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]

On Tue, 2016-12-27 at 11:29 +0800, Shrirang Bagul wrote:
> On Mon, 2016-12-19 at 16:44 +0000, Mark Brown wrote:
> > On Mon, Dec 19, 2016 at 10:30:09AM -0600, Pierre-Louis Bossart wrote:
> > 
> > > All this code seems to be largely a copy-paste of the bytcr_rt5640 machine
> > > driver and the same comments would apply there. This patch did miss the
> > > last
> > 
> > Yes, there's a lot of room for cleanups in the existing code too (and of
> > course if there's such a large amount of cut'n'paste that implies that
> > there should be some code reuse going on).
> 
> Thank you for the review and valuable comments. Following the discussion so
> far,
> I feel the proper way would be to adapt bytcr_rt5660  machine driver to manage
> RT5660 codec.
typo, should be bytcr_rt5640
> > 
> > > correction merged by Mark to deal with errors "ASoC: Intel: bytcr_rt5640:
> > > fallback mechanism if MCLK is not enabled" and the same error handling
> > > would
> > > be needed.
> > 
> > There was a cut back version of it I thought?
> 
> Will try and include the MCLK fallback patch in ver. 2 of the patch.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 841 bytes --]