* [PATCH v2 0/4] ASoC: Dell IoT Gateway audio support
@ 2017-01-12 12:00 Shrirang Bagul
2017-01-12 12:00 ` [PATCH v2 1/4] ASoC: rt5660: Add ACPI support Shrirang Bagul
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Shrirang Bagul @ 2017-01-12 12:00 UTC (permalink / raw)
To: alsa-devel; +Cc: linux-kernel
This work is based on for-next branch of:
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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.
Since v1:
* rt5660: Use enumerations to define GPIO's described ACPI DSDT
* Initialize rt5660 controls from card struct
* Re-use existing bytcr_rt5640 machine driver to support rt5660 codec based
audio
Shrirang Bagul (4):
ASoC: rt5660: Add ACPI support
ASoC: Intel: bytcr_rt5640: move codec clks to card driver data
ASoC: Intel: Support rt5660 codec for Baytrail
ASoC: Intel: bytcr_rt5640: Support line-out mute gpio
sound/soc/codecs/rt5660.c | 32 ++++++
sound/soc/intel/Kconfig | 11 +-
sound/soc/intel/atom/sst/sst_acpi.c | 2 +
sound/soc/intel/boards/bytcr_rt5640.c | 211 ++++++++++++++++++++++++++++++----
4 files changed, 228 insertions(+), 28 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 1/4] ASoC: rt5660: Add ACPI support 2017-01-12 12:00 [PATCH v2 0/4] ASoC: Dell IoT Gateway audio support Shrirang Bagul @ 2017-01-12 12:00 ` Shrirang Bagul 2017-01-13 2:53 ` Bard Liao 2017-01-12 12:00 ` [PATCH v2 2/4] ASoC: Intel: bytcr_rt5640: move codec clks to card driver data Shrirang Bagul ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Shrirang Bagul @ 2017-01-12 12:00 UTC (permalink / raw) To: alsa-devel Cc: linux-kernel, Bard Liao, Oder Chiou, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai 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 | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/sound/soc/codecs/rt5660.c b/sound/soc/codecs/rt5660.c index 76cf76a..06cdcec 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> @@ -40,6 +41,12 @@ #define RT5660_PR_BASE (RT5660_PR_RANGE_BASE + (0 * RT5660_PR_SPACING)) +/* GPIO indexes defined by ACPI */ +enum { + RT5660_GPIO_WAKE_INTR = 0, + RT5660_GPIO_LINEOUT_MUTE = 1, +}; + static const struct regmap_range_cfg rt5660_ranges[] = { { .name = "PR", .range_min = RT5660_PR_BASE, .range_max = RT5660_PR_BASE + 0xf3, @@ -1245,10 +1252,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 = { RT5660_GPIO_WAKE_INTR, 0, false }; +static const struct acpi_gpio_params lineout_mute_gpio = { RT5660_GPIO_LINEOUT_MUTE, 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 +1316,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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH v2 1/4] ASoC: rt5660: Add ACPI support 2017-01-12 12:00 ` [PATCH v2 1/4] ASoC: rt5660: Add ACPI support Shrirang Bagul @ 2017-01-13 2:53 ` Bard Liao 2017-01-13 22:30 ` Andy Shevchenko 0 siblings, 1 reply; 10+ messages in thread From: Bard Liao @ 2017-01-13 2:53 UTC (permalink / raw) To: Shrirang Bagul, alsa-devel Cc: linux-kernel, Oder Chiou, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai > -----Original Message----- > From: Shrirang Bagul [mailto:shrirang.bagul@canonical.com] > Sent: Thursday, January 12, 2017 8:01 PM > To: alsa-devel@alsa-project.org > Cc: linux-kernel@vger.kernel.org; Bard Liao; Oder Chiou; Liam Girdwood; Mark > Brown; Jaroslav Kysela; Takashi Iwai > Subject: [PATCH v2 1/4] ASoC: rt5660: Add ACPI support > > 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> > --- > +static const struct acpi_gpio_params audio_wake_intr_gpio = > { RT5660_GPIO_WAKE_INTR, 0, false }; > +static const struct acpi_gpio_params lineout_mute_gpio = > { RT5660_GPIO_LINEOUT_MUTE, 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 }, > +}; > + I am thinking if it is more suitable to move the gpio params to machine driver? They are not codec's gpios and are only used by machine driver. > -- > 2.9.3 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/4] ASoC: rt5660: Add ACPI support 2017-01-13 2:53 ` Bard Liao @ 2017-01-13 22:30 ` Andy Shevchenko 0 siblings, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2017-01-13 22:30 UTC (permalink / raw) To: Bard Liao Cc: Shrirang Bagul, alsa-devel, linux-kernel, Oder Chiou, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai On Fri, Jan 13, 2017 at 4:53 AM, Bard Liao <bardliao@realtek.com> wrote: >> +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 }, >> +}; >> + > > I am thinking if it is more suitable to move the gpio params to > machine driver? They are not codec's gpios and are only used > by machine driver. Generally speaking those properties should come per ACPI ID. So, they should share the same module which has MODULE_DEVICE_TABLE(acpi, ...) defined. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/4] ASoC: Intel: bytcr_rt5640: move codec clks to card driver data 2017-01-12 12:00 [PATCH v2 0/4] ASoC: Dell IoT Gateway audio support Shrirang Bagul 2017-01-12 12:00 ` [PATCH v2 1/4] ASoC: rt5660: Add ACPI support Shrirang Bagul @ 2017-01-12 12:00 ` Shrirang Bagul 2017-01-12 12:01 ` [PATCH v2 3/4] ASoC: Intel: Support rt5660 codec for Baytrail Shrirang Bagul 2017-01-12 12:01 ` [PATCH v2 4/4] ASoC: Intel: bytcr_rt5640: Support line-out mute gpio Shrirang Bagul 3 siblings, 0 replies; 10+ messages in thread From: Shrirang Bagul @ 2017-01-12 12:00 UTC (permalink / raw) To: alsa-devel Cc: linux-kernel, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Shrirang Bagul, Irina Tirdea This patch moves codec specific clock indices to card driver data. Functions configuring clocks can be re-used for other codec chips with similar features (viz. RT5660) Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> --- sound/soc/intel/boards/bytcr_rt5640.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index 1bd985f..f6fd397 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -53,8 +53,23 @@ enum { #define BYT_RT5640_MCLK_EN BIT(22) #define BYT_RT5640_MCLK_25MHZ BIT(23) +enum { + SCLK_PLL1 = 0, + SCLK_RCCLK, + PLL1_BCLK, + PLL1_MCLK, +}; + struct byt_rt5640_private { struct clk *mclk; + int *clks; +}; + +static int byt_rt5640_clks[] = { + RT5640_SCLK_S_PLL1, + RT5640_SCLK_S_RCCLK, + RT5640_PLL1_S_BCLK1, + RT5640_PLL1_S_MCLK }; static unsigned long byt_rt5640_quirk = BYT_RT5640_MCLK_EN; @@ -132,7 +147,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w, return ret; } } - ret = snd_soc_dai_set_sysclk(codec_dai, RT5640_SCLK_S_PLL1, + ret = snd_soc_dai_set_sysclk(codec_dai, priv->clks[SCLK_PLL1], 48000 * 512, SND_SOC_CLOCK_IN); } else { @@ -141,7 +156,7 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w, * turning off the platform clock. Codec needs clock * for Jack detection and button press */ - ret = snd_soc_dai_set_sysclk(codec_dai, RT5640_SCLK_S_RCCLK, + ret = snd_soc_dai_set_sysclk(codec_dai, priv->clks[SCLK_RCCLK], 48000 * 512, SND_SOC_CLOCK_IN); if (!ret) { @@ -259,6 +274,7 @@ static int byt_rt5640_aif1_hw_params(struct snd_pcm_substream *substream, { struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_soc_dai *codec_dai = rtd->codec_dai; + struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(rtd->card); int ret; ret = snd_soc_dai_set_sysclk(codec_dai, RT5640_SCLK_S_PLL1, @@ -277,25 +293,25 @@ static int byt_rt5640_aif1_hw_params(struct snd_pcm_substream *substream, /* 2x16 bit slots on SSP0 */ ret = snd_soc_dai_set_pll(codec_dai, 0, - RT5640_PLL1_S_BCLK1, + priv->clks[PLL1_BCLK], params_rate(params) * 32, params_rate(params) * 512); } else { /* 2x15 bit slots on SSP2 */ ret = snd_soc_dai_set_pll(codec_dai, 0, - RT5640_PLL1_S_BCLK1, + priv->clks[PLL1_BCLK], params_rate(params) * 50, params_rate(params) * 512); } } else { if (byt_rt5640_quirk & BYT_RT5640_MCLK_25MHZ) { ret = snd_soc_dai_set_pll(codec_dai, 0, - RT5640_PLL1_S_MCLK, + priv->clks[PLL1_MCLK], 25000000, params_rate(params) * 512); } else { ret = snd_soc_dai_set_pll(codec_dai, 0, - RT5640_PLL1_S_MCLK, + priv->clks[PLL1_MCLK], 19200000, params_rate(params) * 512); } @@ -717,6 +733,7 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) return -ENOMEM; /* register the soc card */ + priv->clks = byt_rt5640_clks; byt_rt5640_card.dev = &pdev->dev; mach = byt_rt5640_card.dev->platform_data; snd_soc_card_set_drvdata(&byt_rt5640_card, priv); -- 2.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/4] ASoC: Intel: Support rt5660 codec for Baytrail 2017-01-12 12:00 [PATCH v2 0/4] ASoC: Dell IoT Gateway audio support Shrirang Bagul 2017-01-12 12:00 ` [PATCH v2 1/4] ASoC: rt5660: Add ACPI support Shrirang Bagul 2017-01-12 12:00 ` [PATCH v2 2/4] ASoC: Intel: bytcr_rt5640: move codec clks to card driver data Shrirang Bagul @ 2017-01-12 12:01 ` Shrirang Bagul 2017-01-12 14:40 ` [alsa-devel] " Pierre-Louis Bossart 2017-01-12 12:01 ` [PATCH v2 4/4] ASoC: Intel: bytcr_rt5640: Support line-out mute gpio Shrirang Bagul 3 siblings, 1 reply; 10+ messages in thread From: Shrirang Bagul @ 2017-01-12 12:01 UTC (permalink / raw) To: alsa-devel Cc: linux-kernel, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Vinod Koul, Jeeja KP, Pierre-Louis Bossart, Sathyanarayana Nujella, Shrirang Bagul, Ramesh Babu, John Keeping, Arnd Bergmann, Colin Ian King, Wei Yongjun, Irina Tirdea rt5660 and rt5640 are similar codecs so reuse the bytcr_rt5640 driver. RT5660 codec is used on Dell Edge IoT Gateways with ACPI ID 10EC3277. These devices sport only Line-In and Line-Out jacks. Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> --- sound/soc/intel/Kconfig | 11 +-- sound/soc/intel/atom/sst/sst_acpi.c | 2 + sound/soc/intel/boards/bytcr_rt5640.c | 156 ++++++++++++++++++++++++++++++---- 3 files changed, 147 insertions(+), 22 deletions(-) diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index fd5d1e0..0b43b6a 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -147,17 +147,18 @@ config SND_SOC_INTEL_BROADWELL_MACH If unsure select "N". config SND_SOC_INTEL_BYTCR_RT5640_MACH - tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with RT5640 codec" + tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with RT5640/5660 codec" depends on X86 && I2C && ACPI select SND_SOC_RT5640 + 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 RT5640 audio codec. - Say Y if you have such a device. - If unsure select "N". + This adds support for ASoC machine driver for Intel(R) Baytrail and Baytrail-CR + platforms with RT5640, RT5460 audio codec. + Say Y if you have such a device. + If unsure select "N". config SND_SOC_INTEL_BYTCR_RT5651_MACH tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with RT5651 codec" diff --git a/sound/soc/intel/atom/sst/sst_acpi.c b/sound/soc/intel/atom/sst/sst_acpi.c index f4d92bb..d401457f 100644 --- a/sound/soc/intel/atom/sst/sst_acpi.c +++ b/sound/soc/intel/atom/sst/sst_acpi.c @@ -441,6 +441,8 @@ static struct sst_acpi_mach sst_acpi_bytcr[] = { &byt_rvp_platform_data }, {"10EC5642", "bytcr_rt5640", "intel/fw_sst_0f28.bin", "bytcr_rt5640", NULL, &byt_rvp_platform_data }, + {"10EC3277", "bytcr_rt5640", "intel/fw_sst_0f28.bin", "bytcr_rt5640", NULL, + &byt_rvp_platform_data }, {"INTCCFFD", "bytcr_rt5640", "intel/fw_sst_0f28.bin", "bytcr_rt5640", NULL, &byt_rvp_platform_data }, {"10EC5651", "bytcr_rt5651", "intel/fw_sst_0f28.bin", "bytcr_rt5651", NULL, diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index f6fd397..e8c9a01 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -32,11 +32,17 @@ #include <sound/soc.h> #include <sound/jack.h> #include "../../codecs/rt5640.h" +#include "../../codecs/rt5660.h" #include "../atom/sst-atom-controls.h" #include "../common/sst-acpi.h" #include "../common/sst-dsp.h" enum { + CODEC_TYPE_RT5640, + CODEC_TYPE_RT5660, +}; + +enum { BYT_RT5640_DMIC1_MAP, BYT_RT5640_DMIC2_MAP, BYT_RT5640_IN1_MAP, @@ -60,8 +66,16 @@ enum { PLL1_MCLK, }; +struct byt_acpi_card { + char *codec_id; + int codec_type; + struct snd_soc_card *soc_card; +}; + struct byt_rt5640_private { + struct byt_acpi_card *acpi_card; struct clk *mclk; + char codec_name[16]; int *clks; }; @@ -72,6 +86,13 @@ static int byt_rt5640_clks[] = { RT5640_PLL1_S_MCLK }; +static int byt_rt5660_clks[] = { + RT5660_SCLK_S_PLL1, + RT5660_SCLK_S_RCCLK, + RT5660_PLL1_S_BCLK, + RT5660_PLL1_S_MCLK +}; + static unsigned long byt_rt5640_quirk = BYT_RT5640_MCLK_EN; static void log_quirks(struct device *dev) @@ -105,6 +126,7 @@ static void log_quirks(struct device *dev) #define BYT_CODEC_DAI1 "rt5640-aif1" #define BYT_CODEC_DAI2 "rt5640-aif2" +#define BYT_CODEC_DAI3 "rt5660-aif1" static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card) { @@ -117,6 +139,9 @@ static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card) if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI2, strlen(BYT_CODEC_DAI2))) return rtd->codec_dai; + if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI3, + strlen(BYT_CODEC_DAI3))) + return rtd->codec_dai; } return NULL; @@ -269,6 +294,29 @@ static const struct snd_kcontrol_new byt_rt5640_controls[] = { SOC_DAPM_PIN_SWITCH("Speaker"), }; +static const struct snd_soc_dapm_widget byt_rt5660_widgets[] = { + SND_SOC_DAPM_MIC("Line In", NULL), + SND_SOC_DAPM_LINE("Line Out", NULL), + 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_kcontrol_new byt_rt5660_controls[] = { + SOC_DAPM_PIN_SWITCH("Line In"), + SOC_DAPM_PIN_SWITCH("Line Out"), +}; + static int byt_rt5640_aif1_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -422,11 +470,8 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime) struct snd_soc_codec *codec = runtime->codec; struct snd_soc_card *card = runtime->card; const struct snd_soc_dapm_route *custom_map; - struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card); int num_routes; - card->dapm.idle_bias_off = true; - rt5640_sel_asrc_clk_src(codec, RT5640_DA_STEREO_FILTER | RT5640_DA_MONO_L_FILTER | @@ -511,6 +556,39 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime) snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone"); snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker"); + return ret; +} + +static int byt_rt5660_init(struct snd_soc_pcm_runtime *runtime) +{ + int ret; + struct snd_soc_card *card = runtime->card; + + ret = snd_soc_dapm_add_routes(&card->dapm, + byt_rt5640_ssp2_aif1_map, + ARRAY_SIZE(byt_rt5640_ssp2_aif1_map)); + if (ret) + return ret; + + snd_soc_dapm_enable_pin(&card->dapm, "Line In"); + snd_soc_dapm_enable_pin(&card->dapm, "Line Out"); + + return ret; +} + +static int byt_rt56x0_init(struct snd_soc_pcm_runtime *runtime) +{ + int ret; + struct snd_soc_card *card = runtime->card; + struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card); + + card->dapm.idle_bias_off = true; + + if (priv->acpi_card->codec_type == CODEC_TYPE_RT5640) + ret = byt_rt5640_init(runtime); + else + ret = byt_rt5660_init(runtime); + if ((byt_rt5640_quirk & BYT_RT5640_MCLK_EN) && priv->mclk) { /* * The firmware might enable the clock at @@ -679,7 +757,7 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = { .ignore_suspend = 1, .dpcm_playback = 1, .dpcm_capture = 1, - .init = byt_rt5640_init, + .init = byt_rt56x0_init, .ops = &byt_rt5640_be_ssp2_ops, }, }; @@ -697,6 +775,25 @@ static struct snd_soc_card byt_rt5640_card = { .fully_routed = true, }; +static struct snd_soc_card byt_rt5660_card = { + .name = "bytcr-rt5660", + .owner = THIS_MODULE, + .dai_link = byt_rt5640_dais, + .num_links = ARRAY_SIZE(byt_rt5640_dais), + .controls = byt_rt5660_controls, + .num_controls = ARRAY_SIZE(byt_rt5660_controls), + .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 struct byt_acpi_card snd_soc_cards[] = { + {"10EC5640", CODEC_TYPE_RT5640, &byt_rt5640_card}, + {"10EC3277", CODEC_TYPE_RT5660, &byt_rt5660_card}, +}; + static char byt_rt5640_codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */ static char byt_rt5640_codec_aif_name[12]; /* = "rt5640-aif[1|2]" */ static char byt_rt5640_cpu_dai_name[10]; /* = "ssp[0|2]-port" */ @@ -721,41 +818,51 @@ struct acpi_chan_package { /* ACPICA seems to require 64 bit integers */ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) { int ret_val = 0; - struct sst_acpi_mach *mach; - const char *i2c_name = NULL; int i; - int dai_index; struct byt_rt5640_private *priv; + struct snd_soc_card *card = snd_soc_cards[0].soc_card; + struct sst_acpi_mach *mach; + const char *i2c_name = NULL; + int dai_index = 0; bool is_bytcr = false; priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC); if (!priv) return -ENOMEM; + for (i = 0; i < ARRAY_SIZE(snd_soc_cards); i++) { + if (acpi_dev_found(snd_soc_cards[i].codec_id)) { + dev_dbg(&pdev->dev, + "found codec %s\n", snd_soc_cards[i].codec_id); + card = snd_soc_cards[i].soc_card; + priv->acpi_card = &snd_soc_cards[i]; + break; + } + } + /* register the soc card */ priv->clks = byt_rt5640_clks; - byt_rt5640_card.dev = &pdev->dev; - mach = byt_rt5640_card.dev->platform_data; - snd_soc_card_set_drvdata(&byt_rt5640_card, priv); + card->dev = &pdev->dev; + mach = card->dev->platform_data; + sprintf(priv->codec_name, "i2c-%s:00", priv->acpi_card->codec_id); /* fix index of codec dai */ - dai_index = MERR_DPCM_COMPR + 1; - for (i = 0; i < ARRAY_SIZE(byt_rt5640_dais); i++) { + for (i = 0; i < ARRAY_SIZE(byt_rt5640_dais); i++) if (!strcmp(byt_rt5640_dais[i].codec_name, "i2c-10EC5640:00")) { + card->dai_link[i].codec_name = priv->codec_name; 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_rt5640_codec_name, sizeof(byt_rt5640_codec_name), "%s%s", "i2c-", i2c_name); - byt_rt5640_dais[dai_index].codec_name = byt_rt5640_codec_name; } + snd_soc_card_set_drvdata(card, priv); + /* * swap SSP0 if bytcr is detected * (will be overridden if DMI quirk is detected) @@ -821,6 +928,21 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) BYT_RT5640_DMIC_EN); } + if (priv->acpi_card->codec_type == CODEC_TYPE_RT5660) { + priv->clks = byt_rt5660_clks; + + /* fixup codec aif name for rt5660 */ + snprintf(byt_rt5640_codec_aif_name, + sizeof(byt_rt5640_codec_aif_name), + "%s", "rt5660-aif1"); + + byt_rt5640_dais[dai_index].codec_dai_name = + byt_rt5640_codec_aif_name; + + /* setup codec quirks for rt5660 */ + byt_rt5640_quirk = BYT_RT5640_MCLK_EN; + } + /* check quirks before creating card */ dmi_check_system(byt_rt5640_quirk_table); log_quirks(&pdev->dev); @@ -869,14 +991,14 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) } } - ret_val = devm_snd_soc_register_card(&pdev->dev, &byt_rt5640_card); + 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, &byt_rt5640_card); + platform_set_drvdata(pdev, card); return ret_val; } -- 2.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/4] ASoC: Intel: Support rt5660 codec for Baytrail 2017-01-12 12:01 ` [PATCH v2 3/4] ASoC: Intel: Support rt5660 codec for Baytrail Shrirang Bagul @ 2017-01-12 14:40 ` Pierre-Louis Bossart 2017-01-16 7:45 ` Shrirang Bagul 0 siblings, 1 reply; 10+ messages in thread From: Pierre-Louis Bossart @ 2017-01-12 14:40 UTC (permalink / raw) To: Shrirang Bagul, alsa-devel Cc: Arnd Bergmann, Liam Girdwood, Vinod Koul, linux-kernel, Irina Tirdea, Takashi Iwai, Ramesh Babu, Mark Brown, John Keeping, Sathyanarayana Nujella, Wei Yongjun, Colin Ian King, Jeeja KP On 1/12/17 6:01 AM, Shrirang Bagul wrote: > rt5660 and rt5640 are similar codecs so reuse the bytcr_rt5640 driver. > RT5660 codec is used on Dell Edge IoT Gateways with ACPI ID 10EC3277. > These devices sport only Line-In and Line-Out jacks. While it would be nice to avoid copy/pasting everytime we add a new codec and refactor the code, I am not comfortable with a series of changes below. Also if we do this refactoring then we might as well do it for rt5651 which is similar and only relies on I2S. other machine drivers enable TDM mode when possible. And last this change has a lot of impact on how we deal with UCM files. The name of the card should reflect which codec is used, and the quirks be added to the long name. If you lump everything with a single name then you will make it really hard for userspace to figure out which controls need to be set. So nice idea but too early to merge. NAK. > > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> > --- > sound/soc/intel/Kconfig | 11 +-- > sound/soc/intel/atom/sst/sst_acpi.c | 2 + > sound/soc/intel/boards/bytcr_rt5640.c | 156 ++++++++++++++++++++++++++++++---- > 3 files changed, 147 insertions(+), 22 deletions(-) > > diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig > index fd5d1e0..0b43b6a 100644 > --- a/sound/soc/intel/Kconfig > +++ b/sound/soc/intel/Kconfig > @@ -147,17 +147,18 @@ config SND_SOC_INTEL_BROADWELL_MACH > If unsure select "N". > > config SND_SOC_INTEL_BYTCR_RT5640_MACH > - tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with RT5640 codec" > + tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with RT5640/5660 codec" > depends on X86 && I2C && ACPI > select SND_SOC_RT5640 > + 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 RT5640 audio codec. > - Say Y if you have such a device. > - If unsure select "N". > + This adds support for ASoC machine driver for Intel(R) Baytrail and Baytrail-CR > + platforms with RT5640, RT5460 audio codec. > + Say Y if you have such a device. > + If unsure select "N". > > config SND_SOC_INTEL_BYTCR_RT5651_MACH > tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with RT5651 codec" > diff --git a/sound/soc/intel/atom/sst/sst_acpi.c b/sound/soc/intel/atom/sst/sst_acpi.c > index f4d92bb..d401457f 100644 > --- a/sound/soc/intel/atom/sst/sst_acpi.c > +++ b/sound/soc/intel/atom/sst/sst_acpi.c > @@ -441,6 +441,8 @@ static struct sst_acpi_mach sst_acpi_bytcr[] = { > &byt_rvp_platform_data }, > {"10EC5642", "bytcr_rt5640", "intel/fw_sst_0f28.bin", "bytcr_rt5640", NULL, > &byt_rvp_platform_data }, > + {"10EC3277", "bytcr_rt5640", "intel/fw_sst_0f28.bin", "bytcr_rt5640", NULL, > + &byt_rvp_platform_data }, so right there you add an HID in the platform driver and you need the same in the platform driver to determine which codec type this is... > {"INTCCFFD", "bytcr_rt5640", "intel/fw_sst_0f28.bin", "bytcr_rt5640", NULL, > &byt_rvp_platform_data }, > {"10EC5651", "bytcr_rt5651", "intel/fw_sst_0f28.bin", "bytcr_rt5651", NULL, > diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c > index f6fd397..e8c9a01 100644 > --- a/sound/soc/intel/boards/bytcr_rt5640.c > +++ b/sound/soc/intel/boards/bytcr_rt5640.c > @@ -32,11 +32,17 @@ > #include <sound/soc.h> > #include <sound/jack.h> > #include "../../codecs/rt5640.h" > +#include "../../codecs/rt5660.h" > #include "../atom/sst-atom-controls.h" > #include "../common/sst-acpi.h" > #include "../common/sst-dsp.h" > > enum { > + CODEC_TYPE_RT5640, > + CODEC_TYPE_RT5660, > +}; > + > +enum { > BYT_RT5640_DMIC1_MAP, > BYT_RT5640_DMIC2_MAP, > BYT_RT5640_IN1_MAP, > @@ -60,8 +66,16 @@ enum { > PLL1_MCLK, > }; > > +struct byt_acpi_card { > + char *codec_id; > + int codec_type; > + struct snd_soc_card *soc_card; > +}; > + > struct byt_rt5640_private { > + struct byt_acpi_card *acpi_card; > struct clk *mclk; > + char codec_name[16]; > int *clks; > }; > > @@ -72,6 +86,13 @@ static int byt_rt5640_clks[] = { > RT5640_PLL1_S_MCLK > }; > > +static int byt_rt5660_clks[] = { > + RT5660_SCLK_S_PLL1, > + RT5660_SCLK_S_RCCLK, > + RT5660_PLL1_S_BCLK, > + RT5660_PLL1_S_MCLK > +}; > + > static unsigned long byt_rt5640_quirk = BYT_RT5640_MCLK_EN; the quirks would need to be isolated and made dependent on codec type > > static void log_quirks(struct device *dev) > @@ -105,6 +126,7 @@ static void log_quirks(struct device *dev) > > #define BYT_CODEC_DAI1 "rt5640-aif1" > #define BYT_CODEC_DAI2 "rt5640-aif2" > +#define BYT_CODEC_DAI3 "rt5660-aif1" > > static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card) > { > @@ -117,6 +139,9 @@ static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card *card) > if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI2, > strlen(BYT_CODEC_DAI2))) > return rtd->codec_dai; > + if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI3, > + strlen(BYT_CODEC_DAI3))) > + return rtd->codec_dai; not very good to go look for a DAI that doesn't exist for a specific codec. this would need to be dependent on codec type. > > } > return NULL; > @@ -269,6 +294,29 @@ static const struct snd_kcontrol_new byt_rt5640_controls[] = { > SOC_DAPM_PIN_SWITCH("Speaker"), > }; > > +static const struct snd_soc_dapm_widget byt_rt5660_widgets[] = { > + SND_SOC_DAPM_MIC("Line In", NULL), > + SND_SOC_DAPM_LINE("Line Out", NULL), > + 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_kcontrol_new byt_rt5660_controls[] = { > + SOC_DAPM_PIN_SWITCH("Line In"), > + SOC_DAPM_PIN_SWITCH("Line Out"), > +}; > + > static int byt_rt5640_aif1_hw_params(struct snd_pcm_substream *substream, > struct snd_pcm_hw_params *params) > { > @@ -422,11 +470,8 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime) > struct snd_soc_codec *codec = runtime->codec; > struct snd_soc_card *card = runtime->card; > const struct snd_soc_dapm_route *custom_map; > - struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card); > int num_routes; > > - card->dapm.idle_bias_off = true; > - > rt5640_sel_asrc_clk_src(codec, > RT5640_DA_STEREO_FILTER | > RT5640_DA_MONO_L_FILTER | > @@ -511,6 +556,39 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime) > snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone"); > snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker"); > > + return ret; > +} > + > +static int byt_rt5660_init(struct snd_soc_pcm_runtime *runtime) > +{ > + int ret; > + struct snd_soc_card *card = runtime->card; > + > + ret = snd_soc_dapm_add_routes(&card->dapm, > + byt_rt5640_ssp2_aif1_map, > + ARRAY_SIZE(byt_rt5640_ssp2_aif1_map)); > + if (ret) > + return ret; > + > + snd_soc_dapm_enable_pin(&card->dapm, "Line In"); > + snd_soc_dapm_enable_pin(&card->dapm, "Line Out"); > + > + return ret; > +} > + > +static int byt_rt56x0_init(struct snd_soc_pcm_runtime *runtime) > +{ > + int ret; > + struct snd_soc_card *card = runtime->card; > + struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card); > + > + card->dapm.idle_bias_off = true; > + > + if (priv->acpi_card->codec_type == CODEC_TYPE_RT5640) > + ret = byt_rt5640_init(runtime); > + else > + ret = byt_rt5660_init(runtime); > + > if ((byt_rt5640_quirk & BYT_RT5640_MCLK_EN) && priv->mclk) { > /* > * The firmware might enable the clock at > @@ -679,7 +757,7 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = { > .ignore_suspend = 1, > .dpcm_playback = 1, > .dpcm_capture = 1, > - .init = byt_rt5640_init, > + .init = byt_rt56x0_init, > .ops = &byt_rt5640_be_ssp2_ops, > }, > }; > @@ -697,6 +775,25 @@ static struct snd_soc_card byt_rt5640_card = { > .fully_routed = true, > }; > > +static struct snd_soc_card byt_rt5660_card = { > + .name = "bytcr-rt5660", > + .owner = THIS_MODULE, > + .dai_link = byt_rt5640_dais, > + .num_links = ARRAY_SIZE(byt_rt5640_dais), > + .controls = byt_rt5660_controls, > + .num_controls = ARRAY_SIZE(byt_rt5660_controls), > + .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 struct byt_acpi_card snd_soc_cards[] = { > + {"10EC5640", CODEC_TYPE_RT5640, &byt_rt5640_card}, > + {"10EC3277", CODEC_TYPE_RT5660, &byt_rt5660_card}, > +}; I know this is what's used for rt5645/50 but I don't like it and don't think it should be the baseline for how we deal with codecs. This forces the addition of additional quirks. > + > static char byt_rt5640_codec_name[16]; /* i2c-<HID>:00 with HID being 8 chars */ > static char byt_rt5640_codec_aif_name[12]; /* = "rt5640-aif[1|2]" */ > static char byt_rt5640_cpu_dai_name[10]; /* = "ssp[0|2]-port" */ > @@ -721,41 +818,51 @@ struct acpi_chan_package { /* ACPICA seems to require 64 bit integers */ > static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) > { > int ret_val = 0; > - struct sst_acpi_mach *mach; > - const char *i2c_name = NULL; > int i; > - int dai_index; > struct byt_rt5640_private *priv; > + struct snd_soc_card *card = snd_soc_cards[0].soc_card; > + struct sst_acpi_mach *mach; > + const char *i2c_name = NULL; > + int dai_index = 0; > bool is_bytcr = false; > > priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC); > if (!priv) > return -ENOMEM; > > + for (i = 0; i < ARRAY_SIZE(snd_soc_cards); i++) { > + if (acpi_dev_found(snd_soc_cards[i].codec_id)) { > + dev_dbg(&pdev->dev, > + "found codec %s\n", snd_soc_cards[i].codec_id); > + card = snd_soc_cards[i].soc_card; > + priv->acpi_card = &snd_soc_cards[i]; > + break; > + } > + } > + > /* register the soc card */ > priv->clks = byt_rt5640_clks; > - byt_rt5640_card.dev = &pdev->dev; > - mach = byt_rt5640_card.dev->platform_data; > - snd_soc_card_set_drvdata(&byt_rt5640_card, priv); > + card->dev = &pdev->dev; > + mach = card->dev->platform_data; > + sprintf(priv->codec_name, "i2c-%s:00", priv->acpi_card->codec_id); > > /* fix index of codec dai */ > - dai_index = MERR_DPCM_COMPR + 1; > - for (i = 0; i < ARRAY_SIZE(byt_rt5640_dais); i++) { > + for (i = 0; i < ARRAY_SIZE(byt_rt5640_dais); i++) > if (!strcmp(byt_rt5640_dais[i].codec_name, "i2c-10EC5640:00")) { > + card->dai_link[i].codec_name = priv->codec_name; > 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_rt5640_codec_name, sizeof(byt_rt5640_codec_name), > "%s%s", "i2c-", i2c_name); > - > byt_rt5640_dais[dai_index].codec_name = byt_rt5640_codec_name; > } > > + snd_soc_card_set_drvdata(card, priv); > + > /* > * swap SSP0 if bytcr is detected > * (will be overridden if DMI quirk is detected) > @@ -821,6 +928,21 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) > BYT_RT5640_DMIC_EN); > } > > + if (priv->acpi_card->codec_type == CODEC_TYPE_RT5660) { > + priv->clks = byt_rt5660_clks; > + > + /* fixup codec aif name for rt5660 */ > + snprintf(byt_rt5640_codec_aif_name, > + sizeof(byt_rt5640_codec_aif_name), > + "%s", "rt5660-aif1"); > + > + byt_rt5640_dais[dai_index].codec_dai_name = > + byt_rt5640_codec_aif_name; > + > + /* setup codec quirks for rt5660 */ > + byt_rt5640_quirk = BYT_RT5640_MCLK_EN; > + } > + > /* check quirks before creating card */ > dmi_check_system(byt_rt5640_quirk_table); the quirks have to be separate. > log_quirks(&pdev->dev); > @@ -869,14 +991,14 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) > } > } > > - ret_val = devm_snd_soc_register_card(&pdev->dev, &byt_rt5640_card); > + 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, &byt_rt5640_card); > + platform_set_drvdata(pdev, card); > return ret_val; > } > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/4] ASoC: Intel: Support rt5660 codec for Baytrail 2017-01-12 14:40 ` [alsa-devel] " Pierre-Louis Bossart @ 2017-01-16 7:45 ` Shrirang Bagul 2017-01-16 15:10 ` Pierre-Louis Bossart 0 siblings, 1 reply; 10+ messages in thread From: Shrirang Bagul @ 2017-01-16 7:45 UTC (permalink / raw) To: Pierre-Louis Bossart, alsa-devel Cc: Arnd Bergmann, Liam Girdwood, Vinod Koul, linux-kernel, Irina Tirdea, Takashi Iwai, Ramesh Babu, Mark Brown, John Keeping, Sathyanarayana Nujella, Wei Yongjun, Colin Ian King, Jeeja KP On Thu, 2017-01-12 at 08:40 -0600, Pierre-Louis Bossart wrote: > On 1/12/17 6:01 AM, Shrirang Bagul wrote: > > rt5660 and rt5640 are similar codecs so reuse the bytcr_rt5640 driver. > > RT5660 codec is used on Dell Edge IoT Gateways with ACPI ID 10EC3277. > > These devices sport only Line-In and Line-Out jacks. > > While it would be nice to avoid copy/pasting everytime we add a new > codec and refactor the code, I am not comfortable with a series of > changes below. > Also if we do this refactoring then we might as well do it for rt5651 > which is similar and only relies on I2S. other machine drivers enable > TDM mode when possible. > And last this change has a lot of impact on how we deal with UCM files. > The name of the card should reflect which codec is used, and the quirks > be added to the long name. If you lump everything with a single name > then you will make it really hard for userspace to figure out which > controls need to be set. > > So nice idea but too early to merge. NAK. Thank you for the review, will address these comments in the next version. When you it be appropriate to re-submit? Are we waiting for any patches which are queued to be merged soon? > > > > > Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> > > --- > > sound/soc/intel/Kconfig | 11 +-- > > sound/soc/intel/atom/sst/sst_acpi.c | 2 + > > sound/soc/intel/boards/bytcr_rt5640.c | 156 ++++++++++++++++++++++++++++++- > > --- > > 3 files changed, 147 insertions(+), 22 deletions(-) > > > > diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig > > index fd5d1e0..0b43b6a 100644 > > --- a/sound/soc/intel/Kconfig > > +++ b/sound/soc/intel/Kconfig > > @@ -147,17 +147,18 @@ config SND_SOC_INTEL_BROADWELL_MACH > > If unsure select "N". > > > > config SND_SOC_INTEL_BYTCR_RT5640_MACH > > - tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with > > RT5640 codec" > > + tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with > > RT5640/5660 codec" > > depends on X86 && I2C && ACPI > > select SND_SOC_RT5640 > > + 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 RT5640 audio codec. > > - Say Y if you have such a device. > > - If unsure select "N". > > + This adds support for ASoC machine driver for Intel(R) Baytrail > > and Baytrail-CR > > + platforms with RT5640, RT5460 audio codec. > > + Say Y if you have such a device. > > + If unsure select "N". > > > > config SND_SOC_INTEL_BYTCR_RT5651_MACH > > tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with > > RT5651 codec" > > diff --git a/sound/soc/intel/atom/sst/sst_acpi.c > > b/sound/soc/intel/atom/sst/sst_acpi.c > > index f4d92bb..d401457f 100644 > > --- a/sound/soc/intel/atom/sst/sst_acpi.c > > +++ b/sound/soc/intel/atom/sst/sst_acpi.c > > @@ -441,6 +441,8 @@ static struct sst_acpi_mach sst_acpi_bytcr[] = { > > &byt_rvp_platform_data }, > > {"10EC5642", "bytcr_rt5640", "intel/fw_sst_0f28.bin", > > "bytcr_rt5640", NULL, > > &byt_rvp_platform_data }, > > + {"10EC3277", "bytcr_rt5640", "intel/fw_sst_0f28.bin", > > "bytcr_rt5640", NULL, > > + &byt_rvp_platform_data }, > > so right there you add an HID in the platform driver and you need the > same in the platform driver to determine which codec type this is... > > > {"INTCCFFD", "bytcr_rt5640", "intel/fw_sst_0f28.bin", > > "bytcr_rt5640", NULL, > > &byt_rvp_platform_data }, > > {"10EC5651", "bytcr_rt5651", "intel/fw_sst_0f28.bin", > > "bytcr_rt5651", NULL, > > diff --git a/sound/soc/intel/boards/bytcr_rt5640.c > > b/sound/soc/intel/boards/bytcr_rt5640.c > > index f6fd397..e8c9a01 100644 > > --- a/sound/soc/intel/boards/bytcr_rt5640.c > > +++ b/sound/soc/intel/boards/bytcr_rt5640.c > > @@ -32,11 +32,17 @@ > > #include <sound/soc.h> > > #include <sound/jack.h> > > #include "../../codecs/rt5640.h" > > +#include "../../codecs/rt5660.h" > > #include "../atom/sst-atom-controls.h" > > #include "../common/sst-acpi.h" > > #include "../common/sst-dsp.h" > > > > enum { > > + CODEC_TYPE_RT5640, > > + CODEC_TYPE_RT5660, > > +}; > > + > > +enum { > > BYT_RT5640_DMIC1_MAP, > > BYT_RT5640_DMIC2_MAP, > > BYT_RT5640_IN1_MAP, > > @@ -60,8 +66,16 @@ enum { > > PLL1_MCLK, > > }; > > > > +struct byt_acpi_card { > > + char *codec_id; > > + int codec_type; > > + struct snd_soc_card *soc_card; > > +}; > > + > > struct byt_rt5640_private { > > + struct byt_acpi_card *acpi_card; > > struct clk *mclk; > > + char codec_name[16]; > > int *clks; > > }; > > > > @@ -72,6 +86,13 @@ static int byt_rt5640_clks[] = { > > RT5640_PLL1_S_MCLK > > }; > > > > +static int byt_rt5660_clks[] = { > > + RT5660_SCLK_S_PLL1, > > + RT5660_SCLK_S_RCCLK, > > + RT5660_PLL1_S_BCLK, > > + RT5660_PLL1_S_MCLK > > +}; > > + > > static unsigned long byt_rt5640_quirk = BYT_RT5640_MCLK_EN; > > the quirks would need to be isolated and made dependent on codec type Will fix in next version of the patch. > > > > > static void log_quirks(struct device *dev) > > @@ -105,6 +126,7 @@ static void log_quirks(struct device *dev) > > > > #define BYT_CODEC_DAI1 "rt5640-aif1" > > #define BYT_CODEC_DAI2 "rt5640-aif2" > > +#define BYT_CODEC_DAI3 "rt5660-aif1" > > > > static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card > > *card) > > { > > @@ -117,6 +139,9 @@ static inline struct snd_soc_dai > > *byt_get_codec_dai(struct snd_soc_card *card) > > if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI2, > > strlen(BYT_CODEC_DAI2))) > > return rtd->codec_dai; > > + if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI3, > > + strlen(BYT_CODEC_DAI3))) > > + return rtd->codec_dai; > > not very good to go look for a DAI that doesn't exist for a specific > codec. this would need to be dependent on codec type. Understood, will fix this in next version. > > > > > } > > return NULL; > > @@ -269,6 +294,29 @@ static const struct snd_kcontrol_new > > byt_rt5640_controls[] = { > > SOC_DAPM_PIN_SWITCH("Speaker"), > > }; > > > > +static const struct snd_soc_dapm_widget byt_rt5660_widgets[] = { > > + SND_SOC_DAPM_MIC("Line In", NULL), > > + SND_SOC_DAPM_LINE("Line Out", NULL), > > + 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_kcontrol_new byt_rt5660_controls[] = { > > + SOC_DAPM_PIN_SWITCH("Line In"), > > + SOC_DAPM_PIN_SWITCH("Line Out"), > > +}; > > + > > static int byt_rt5640_aif1_hw_params(struct snd_pcm_substream *substream, > > struct snd_pcm_hw_params *params) > > { > > @@ -422,11 +470,8 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime > > *runtime) > > struct snd_soc_codec *codec = runtime->codec; > > struct snd_soc_card *card = runtime->card; > > const struct snd_soc_dapm_route *custom_map; > > - struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card); > > int num_routes; > > > > - card->dapm.idle_bias_off = true; > > - > > rt5640_sel_asrc_clk_src(codec, > > RT5640_DA_STEREO_FILTER | > > RT5640_DA_MONO_L_FILTER | > > @@ -511,6 +556,39 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime > > *runtime) > > snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone"); > > snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker"); > > > > + return ret; > > +} > > + > > +static int byt_rt5660_init(struct snd_soc_pcm_runtime *runtime) > > +{ > > + int ret; > > + struct snd_soc_card *card = runtime->card; > > + > > + ret = snd_soc_dapm_add_routes(&card->dapm, > > + byt_rt5640_ssp2_aif1_map, > > + ARRAY_SIZE(byt_rt5640_ssp2_aif1_map)); > > + if (ret) > > + return ret; > > + > > + snd_soc_dapm_enable_pin(&card->dapm, "Line In"); > > + snd_soc_dapm_enable_pin(&card->dapm, "Line Out"); > > + > > + return ret; > > +} > > + > > +static int byt_rt56x0_init(struct snd_soc_pcm_runtime *runtime) > > +{ > > + int ret; > > + struct snd_soc_card *card = runtime->card; > > + struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card); > > + > > + card->dapm.idle_bias_off = true; > > + > > + if (priv->acpi_card->codec_type == CODEC_TYPE_RT5640) > > + ret = byt_rt5640_init(runtime); > > + else > > + ret = byt_rt5660_init(runtime); > > + > > if ((byt_rt5640_quirk & BYT_RT5640_MCLK_EN) && priv->mclk) { > > /* > > * The firmware might enable the clock at > > @@ -679,7 +757,7 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = { > > .ignore_suspend = 1, > > .dpcm_playback = 1, > > .dpcm_capture = 1, > > - .init = byt_rt5640_init, > > + .init = byt_rt56x0_init, > > .ops = &byt_rt5640_be_ssp2_ops, > > }, > > }; > > @@ -697,6 +775,25 @@ static struct snd_soc_card byt_rt5640_card = { > > .fully_routed = true, > > }; > > > > +static struct snd_soc_card byt_rt5660_card = { > > + .name = "bytcr-rt5660", > > + .owner = THIS_MODULE, > > + .dai_link = byt_rt5640_dais, > > + .num_links = ARRAY_SIZE(byt_rt5640_dais), > > + .controls = byt_rt5660_controls, > > + .num_controls = ARRAY_SIZE(byt_rt5660_controls), > > + .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 struct byt_acpi_card snd_soc_cards[] = { > > + {"10EC5640", CODEC_TYPE_RT5640, &byt_rt5640_card}, > > + {"10EC3277", CODEC_TYPE_RT5660, &byt_rt5660_card}, > > +}; > > I know this is what's used for rt5645/50 but I don't like it and don't > think it should be the baseline for how we deal with codecs. This forces > the addition of additional quirks. Is there a better baseline to start from or none exists and we ought to come-up with one? > > > + > > static char byt_rt5640_codec_name[16]; /* i2c-<HID>:00 with HID being 8 > > chars */ > > static char byt_rt5640_codec_aif_name[12]; /* = "rt5640-aif[1|2]" */ > > static char byt_rt5640_cpu_dai_name[10]; /* = "ssp[0|2]-port" */ > > @@ -721,41 +818,51 @@ struct acpi_chan_package { /* ACPICA seems to > > require 64 bit integers */ > > static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) > > { > > int ret_val = 0; > > - struct sst_acpi_mach *mach; > > - const char *i2c_name = NULL; > > int i; > > - int dai_index; > > struct byt_rt5640_private *priv; > > + struct snd_soc_card *card = snd_soc_cards[0].soc_card; > > + struct sst_acpi_mach *mach; > > + const char *i2c_name = NULL; > > + int dai_index = 0; > > bool is_bytcr = false; > > > > priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC); > > if (!priv) > > return -ENOMEM; > > > > + for (i = 0; i < ARRAY_SIZE(snd_soc_cards); i++) { > > + if (acpi_dev_found(snd_soc_cards[i].codec_id)) { > > + dev_dbg(&pdev->dev, > > + "found codec %s\n", > > snd_soc_cards[i].codec_id); > > + card = snd_soc_cards[i].soc_card; > > + priv->acpi_card = &snd_soc_cards[i]; > > + break; > > + } > > + } > > + > > /* register the soc card */ > > priv->clks = byt_rt5640_clks; > > - byt_rt5640_card.dev = &pdev->dev; > > - mach = byt_rt5640_card.dev->platform_data; > > - snd_soc_card_set_drvdata(&byt_rt5640_card, priv); > > + card->dev = &pdev->dev; > > + mach = card->dev->platform_data; > > + sprintf(priv->codec_name, "i2c-%s:00", priv->acpi_card->codec_id); > > > > /* fix index of codec dai */ > > - dai_index = MERR_DPCM_COMPR + 1; > > - for (i = 0; i < ARRAY_SIZE(byt_rt5640_dais); i++) { > > + for (i = 0; i < ARRAY_SIZE(byt_rt5640_dais); i++) > > if (!strcmp(byt_rt5640_dais[i].codec_name, "i2c- > > 10EC5640:00")) { > > + card->dai_link[i].codec_name = priv->codec_name; > > 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_rt5640_codec_name, > > sizeof(byt_rt5640_codec_name), > > "%s%s", "i2c-", i2c_name); > > - > > byt_rt5640_dais[dai_index].codec_name = > > byt_rt5640_codec_name; > > } > > > > + snd_soc_card_set_drvdata(card, priv); > > + > > /* > > * swap SSP0 if bytcr is detected > > * (will be overridden if DMI quirk is detected) > > @@ -821,6 +928,21 @@ static int snd_byt_rt5640_mc_probe(struct > > platform_device *pdev) > > BYT_RT5640_DMIC_EN); > > } > > > > + if (priv->acpi_card->codec_type == CODEC_TYPE_RT5660) { > > + priv->clks = byt_rt5660_clks; > > + > > + /* fixup codec aif name for rt5660 */ > > + snprintf(byt_rt5640_codec_aif_name, > > + sizeof(byt_rt5640_codec_aif_name), > > + "%s", "rt5660-aif1"); > > + > > + byt_rt5640_dais[dai_index].codec_dai_name = > > + byt_rt5640_codec_aif_name; > > + > > + /* setup codec quirks for rt5660 */ > > + byt_rt5640_quirk = BYT_RT5640_MCLK_EN; > > + } > > + > > /* check quirks before creating card */ > > dmi_check_system(byt_rt5640_quirk_table); > > the quirks have to be separate. > > > log_quirks(&pdev->dev); > > @@ -869,14 +991,14 @@ static int snd_byt_rt5640_mc_probe(struct > > platform_device *pdev) > > } > > } > > > > - ret_val = devm_snd_soc_register_card(&pdev->dev, &byt_rt5640_card); > > + 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, &byt_rt5640_card); > > + platform_set_drvdata(pdev, card); > > return ret_val; > > } > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/4] ASoC: Intel: Support rt5660 codec for Baytrail 2017-01-16 7:45 ` Shrirang Bagul @ 2017-01-16 15:10 ` Pierre-Louis Bossart 0 siblings, 0 replies; 10+ messages in thread From: Pierre-Louis Bossart @ 2017-01-16 15:10 UTC (permalink / raw) To: Shrirang Bagul, alsa-devel Cc: Jeeja KP, Arnd Bergmann, Irina Tirdea, Vinod Koul, linux-kernel, Takashi Iwai, Ramesh Babu, Liam Girdwood, Mark Brown, John Keeping, Sathyanarayana Nujella, Colin Ian King, Wei Yongjun On 1/16/17 1:45 AM, Shrirang Bagul wrote: > On Thu, 2017-01-12 at 08:40 -0600, Pierre-Louis Bossart wrote: >> On 1/12/17 6:01 AM, Shrirang Bagul wrote: >>> rt5660 and rt5640 are similar codecs so reuse the bytcr_rt5640 driver. >>> RT5660 codec is used on Dell Edge IoT Gateways with ACPI ID 10EC3277. >>> These devices sport only Line-In and Line-Out jacks. >> >> While it would be nice to avoid copy/pasting everytime we add a new >> codec and refactor the code, I am not comfortable with a series of >> changes below. >> Also if we do this refactoring then we might as well do it for rt5651 >> which is similar and only relies on I2S. other machine drivers enable >> TDM mode when possible. >> And last this change has a lot of impact on how we deal with UCM files. >> The name of the card should reflect which codec is used, and the quirks >> be added to the long name. If you lump everything with a single name >> then you will make it really hard for userspace to figure out which >> controls need to be set. >> >> So nice idea but too early to merge. NAK. > Thank you for the review, will address these comments in the next version. When > you it be appropriate to re-submit? Are we waiting for any patches which are > queued to be merged soon? I have a set of 5640/5651 corrections that will be sent out very soon (fixes for existing platforms). I am ok with factorizing some of the common code, however the quirk management can't really be fused and the platform names need to remain separate. The dailinks and dapm routes need to remain separate, same for the platform clock control. In the end, I am not sure if it's better to have a single file with all the codecs, or if we should keep separate files and use a common set of helper functions. The latter might be wiser if we want to add another codec, it'd avoid complicated if/else cases in the middle of the common code. >> >>> >>> Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> >>> --- >>> sound/soc/intel/Kconfig | 11 +-- >>> sound/soc/intel/atom/sst/sst_acpi.c | 2 + >>> sound/soc/intel/boards/bytcr_rt5640.c | 156 ++++++++++++++++++++++++++++++- >>> --- >>> 3 files changed, 147 insertions(+), 22 deletions(-) >>> >>> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig >>> index fd5d1e0..0b43b6a 100644 >>> --- a/sound/soc/intel/Kconfig >>> +++ b/sound/soc/intel/Kconfig >>> @@ -147,17 +147,18 @@ config SND_SOC_INTEL_BROADWELL_MACH >>> If unsure select "N". >>> >>> config SND_SOC_INTEL_BYTCR_RT5640_MACH >>> - tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with >>> RT5640 codec" >>> + tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with >>> RT5640/5660 codec" >>> depends on X86 && I2C && ACPI >>> select SND_SOC_RT5640 >>> + 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 RT5640 audio codec. >>> - Say Y if you have such a device. >>> - If unsure select "N". >>> + This adds support for ASoC machine driver for Intel(R) Baytrail >>> and Baytrail-CR >>> + platforms with RT5640, RT5460 audio codec. >>> + Say Y if you have such a device. >>> + If unsure select "N". >>> >>> config SND_SOC_INTEL_BYTCR_RT5651_MACH >>> tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with >>> RT5651 codec" >>> diff --git a/sound/soc/intel/atom/sst/sst_acpi.c >>> b/sound/soc/intel/atom/sst/sst_acpi.c >>> index f4d92bb..d401457f 100644 >>> --- a/sound/soc/intel/atom/sst/sst_acpi.c >>> +++ b/sound/soc/intel/atom/sst/sst_acpi.c >>> @@ -441,6 +441,8 @@ static struct sst_acpi_mach sst_acpi_bytcr[] = { >>> &byt_rvp_platform_data }, >>> {"10EC5642", "bytcr_rt5640", "intel/fw_sst_0f28.bin", >>> "bytcr_rt5640", NULL, >>> &byt_rvp_platform_data }, >>> + {"10EC3277", "bytcr_rt5640", "intel/fw_sst_0f28.bin", >>> "bytcr_rt5640", NULL, >>> + &byt_rvp_platform_data }, >> >> so right there you add an HID in the platform driver and you need the >> same in the platform driver to determine which codec type this is... >> >>> {"INTCCFFD", "bytcr_rt5640", "intel/fw_sst_0f28.bin", >>> "bytcr_rt5640", NULL, >>> &byt_rvp_platform_data }, >>> {"10EC5651", "bytcr_rt5651", "intel/fw_sst_0f28.bin", >>> "bytcr_rt5651", NULL, >>> diff --git a/sound/soc/intel/boards/bytcr_rt5640.c >>> b/sound/soc/intel/boards/bytcr_rt5640.c >>> index f6fd397..e8c9a01 100644 >>> --- a/sound/soc/intel/boards/bytcr_rt5640.c >>> +++ b/sound/soc/intel/boards/bytcr_rt5640.c >>> @@ -32,11 +32,17 @@ >>> #include <sound/soc.h> >>> #include <sound/jack.h> >>> #include "../../codecs/rt5640.h" >>> +#include "../../codecs/rt5660.h" >>> #include "../atom/sst-atom-controls.h" >>> #include "../common/sst-acpi.h" >>> #include "../common/sst-dsp.h" >>> >>> enum { >>> + CODEC_TYPE_RT5640, >>> + CODEC_TYPE_RT5660, >>> +}; >>> + >>> +enum { >>> BYT_RT5640_DMIC1_MAP, >>> BYT_RT5640_DMIC2_MAP, >>> BYT_RT5640_IN1_MAP, >>> @@ -60,8 +66,16 @@ enum { >>> PLL1_MCLK, >>> }; >>> >>> +struct byt_acpi_card { >>> + char *codec_id; >>> + int codec_type; >>> + struct snd_soc_card *soc_card; >>> +}; >>> + >>> struct byt_rt5640_private { >>> + struct byt_acpi_card *acpi_card; >>> struct clk *mclk; >>> + char codec_name[16]; >>> int *clks; >>> }; >>> >>> @@ -72,6 +86,13 @@ static int byt_rt5640_clks[] = { >>> RT5640_PLL1_S_MCLK >>> }; >>> >>> +static int byt_rt5660_clks[] = { >>> + RT5660_SCLK_S_PLL1, >>> + RT5660_SCLK_S_RCCLK, >>> + RT5660_PLL1_S_BCLK, >>> + RT5660_PLL1_S_MCLK >>> +}; >>> + >>> static unsigned long byt_rt5640_quirk = BYT_RT5640_MCLK_EN; >> >> the quirks would need to be isolated and made dependent on codec type > Will fix in next version of the patch. >> >>> >>> static void log_quirks(struct device *dev) >>> @@ -105,6 +126,7 @@ static void log_quirks(struct device *dev) >>> >>> #define BYT_CODEC_DAI1 "rt5640-aif1" >>> #define BYT_CODEC_DAI2 "rt5640-aif2" >>> +#define BYT_CODEC_DAI3 "rt5660-aif1" >>> >>> static inline struct snd_soc_dai *byt_get_codec_dai(struct snd_soc_card >>> *card) >>> { >>> @@ -117,6 +139,9 @@ static inline struct snd_soc_dai >>> *byt_get_codec_dai(struct snd_soc_card *card) >>> if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI2, >>> strlen(BYT_CODEC_DAI2))) >>> return rtd->codec_dai; >>> + if (!strncmp(rtd->codec_dai->name, BYT_CODEC_DAI3, >>> + strlen(BYT_CODEC_DAI3))) >>> + return rtd->codec_dai; >> >> not very good to go look for a DAI that doesn't exist for a specific >> codec. this would need to be dependent on codec type. > Understood, will fix this in next version. >> >>> >>> } >>> return NULL; >>> @@ -269,6 +294,29 @@ static const struct snd_kcontrol_new >>> byt_rt5640_controls[] = { >>> SOC_DAPM_PIN_SWITCH("Speaker"), >>> }; >>> >>> +static const struct snd_soc_dapm_widget byt_rt5660_widgets[] = { >>> + SND_SOC_DAPM_MIC("Line In", NULL), >>> + SND_SOC_DAPM_LINE("Line Out", NULL), >>> + 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_kcontrol_new byt_rt5660_controls[] = { >>> + SOC_DAPM_PIN_SWITCH("Line In"), >>> + SOC_DAPM_PIN_SWITCH("Line Out"), >>> +}; >>> + >>> static int byt_rt5640_aif1_hw_params(struct snd_pcm_substream *substream, >>> struct snd_pcm_hw_params *params) >>> { >>> @@ -422,11 +470,8 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime >>> *runtime) >>> struct snd_soc_codec *codec = runtime->codec; >>> struct snd_soc_card *card = runtime->card; >>> const struct snd_soc_dapm_route *custom_map; >>> - struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card); >>> int num_routes; >>> >>> - card->dapm.idle_bias_off = true; >>> - >>> rt5640_sel_asrc_clk_src(codec, >>> RT5640_DA_STEREO_FILTER | >>> RT5640_DA_MONO_L_FILTER | >>> @@ -511,6 +556,39 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime >>> *runtime) >>> snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone"); >>> snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker"); >>> >>> + return ret; >>> +} >>> + >>> +static int byt_rt5660_init(struct snd_soc_pcm_runtime *runtime) >>> +{ >>> + int ret; >>> + struct snd_soc_card *card = runtime->card; >>> + >>> + ret = snd_soc_dapm_add_routes(&card->dapm, >>> + byt_rt5640_ssp2_aif1_map, >>> + ARRAY_SIZE(byt_rt5640_ssp2_aif1_map)); >>> + if (ret) >>> + return ret; >>> + >>> + snd_soc_dapm_enable_pin(&card->dapm, "Line In"); >>> + snd_soc_dapm_enable_pin(&card->dapm, "Line Out"); >>> + >>> + return ret; >>> +} >>> + >>> +static int byt_rt56x0_init(struct snd_soc_pcm_runtime *runtime) >>> +{ >>> + int ret; >>> + struct snd_soc_card *card = runtime->card; >>> + struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card); >>> + >>> + card->dapm.idle_bias_off = true; >>> + >>> + if (priv->acpi_card->codec_type == CODEC_TYPE_RT5640) >>> + ret = byt_rt5640_init(runtime); >>> + else >>> + ret = byt_rt5660_init(runtime); >>> + >>> if ((byt_rt5640_quirk & BYT_RT5640_MCLK_EN) && priv->mclk) { >>> /* >>> * The firmware might enable the clock at >>> @@ -679,7 +757,7 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = { >>> .ignore_suspend = 1, >>> .dpcm_playback = 1, >>> .dpcm_capture = 1, >>> - .init = byt_rt5640_init, >>> + .init = byt_rt56x0_init, >>> .ops = &byt_rt5640_be_ssp2_ops, >>> }, >>> }; >>> @@ -697,6 +775,25 @@ static struct snd_soc_card byt_rt5640_card = { >>> .fully_routed = true, >>> }; >>> >>> +static struct snd_soc_card byt_rt5660_card = { >>> + .name = "bytcr-rt5660", >>> + .owner = THIS_MODULE, >>> + .dai_link = byt_rt5640_dais, >>> + .num_links = ARRAY_SIZE(byt_rt5640_dais), >>> + .controls = byt_rt5660_controls, >>> + .num_controls = ARRAY_SIZE(byt_rt5660_controls), >>> + .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 struct byt_acpi_card snd_soc_cards[] = { >>> + {"10EC5640", CODEC_TYPE_RT5640, &byt_rt5640_card}, >>> + {"10EC3277", CODEC_TYPE_RT5660, &byt_rt5660_card}, >>> +}; >> >> I know this is what's used for rt5645/50 but I don't like it and don't >> think it should be the baseline for how we deal with codecs. This forces >> the addition of additional quirks. > Is there a better baseline to start from or none exists and we ought to come-up > with one? >> >>> + >>> static char byt_rt5640_codec_name[16]; /* i2c-<HID>:00 with HID being 8 >>> chars */ >>> static char byt_rt5640_codec_aif_name[12]; /* = "rt5640-aif[1|2]" */ >>> static char byt_rt5640_cpu_dai_name[10]; /* = "ssp[0|2]-port" */ >>> @@ -721,41 +818,51 @@ struct acpi_chan_package { /* ACPICA seems to >>> require 64 bit integers */ >>> static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) >>> { >>> int ret_val = 0; >>> - struct sst_acpi_mach *mach; >>> - const char *i2c_name = NULL; >>> int i; >>> - int dai_index; >>> struct byt_rt5640_private *priv; >>> + struct snd_soc_card *card = snd_soc_cards[0].soc_card; >>> + struct sst_acpi_mach *mach; >>> + const char *i2c_name = NULL; >>> + int dai_index = 0; >>> bool is_bytcr = false; >>> >>> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_ATOMIC); >>> if (!priv) >>> return -ENOMEM; >>> >>> + for (i = 0; i < ARRAY_SIZE(snd_soc_cards); i++) { >>> + if (acpi_dev_found(snd_soc_cards[i].codec_id)) { >>> + dev_dbg(&pdev->dev, >>> + "found codec %s\n", >>> snd_soc_cards[i].codec_id); >>> + card = snd_soc_cards[i].soc_card; >>> + priv->acpi_card = &snd_soc_cards[i]; >>> + break; >>> + } >>> + } >>> + >>> /* register the soc card */ >>> priv->clks = byt_rt5640_clks; >>> - byt_rt5640_card.dev = &pdev->dev; >>> - mach = byt_rt5640_card.dev->platform_data; >>> - snd_soc_card_set_drvdata(&byt_rt5640_card, priv); >>> + card->dev = &pdev->dev; >>> + mach = card->dev->platform_data; >>> + sprintf(priv->codec_name, "i2c-%s:00", priv->acpi_card->codec_id); >>> >>> /* fix index of codec dai */ >>> - dai_index = MERR_DPCM_COMPR + 1; >>> - for (i = 0; i < ARRAY_SIZE(byt_rt5640_dais); i++) { >>> + for (i = 0; i < ARRAY_SIZE(byt_rt5640_dais); i++) >>> if (!strcmp(byt_rt5640_dais[i].codec_name, "i2c- >>> 10EC5640:00")) { >>> + card->dai_link[i].codec_name = priv->codec_name; >>> 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_rt5640_codec_name, >>> sizeof(byt_rt5640_codec_name), >>> "%s%s", "i2c-", i2c_name); >>> - >>> byt_rt5640_dais[dai_index].codec_name = >>> byt_rt5640_codec_name; >>> } >>> >>> + snd_soc_card_set_drvdata(card, priv); >>> + >>> /* >>> * swap SSP0 if bytcr is detected >>> * (will be overridden if DMI quirk is detected) >>> @@ -821,6 +928,21 @@ static int snd_byt_rt5640_mc_probe(struct >>> platform_device *pdev) >>> BYT_RT5640_DMIC_EN); >>> } >>> >>> + if (priv->acpi_card->codec_type == CODEC_TYPE_RT5660) { >>> + priv->clks = byt_rt5660_clks; >>> + >>> + /* fixup codec aif name for rt5660 */ >>> + snprintf(byt_rt5640_codec_aif_name, >>> + sizeof(byt_rt5640_codec_aif_name), >>> + "%s", "rt5660-aif1"); >>> + >>> + byt_rt5640_dais[dai_index].codec_dai_name = >>> + byt_rt5640_codec_aif_name; >>> + >>> + /* setup codec quirks for rt5660 */ >>> + byt_rt5640_quirk = BYT_RT5640_MCLK_EN; >>> + } >>> + >>> /* check quirks before creating card */ >>> dmi_check_system(byt_rt5640_quirk_table); >> >> the quirks have to be separate. >> >>> log_quirks(&pdev->dev); >>> @@ -869,14 +991,14 @@ static int snd_byt_rt5640_mc_probe(struct >>> platform_device *pdev) >>> } >>> } >>> >>> - ret_val = devm_snd_soc_register_card(&pdev->dev, &byt_rt5640_card); >>> + 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, &byt_rt5640_card); >>> + platform_set_drvdata(pdev, card); >>> return ret_val; >>> } >>> >>> >> >> > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 4/4] ASoC: Intel: bytcr_rt5640: Support line-out mute gpio 2017-01-12 12:00 [PATCH v2 0/4] ASoC: Dell IoT Gateway audio support Shrirang Bagul ` (2 preceding siblings ...) 2017-01-12 12:01 ` [PATCH v2 3/4] ASoC: Intel: Support rt5660 codec for Baytrail Shrirang Bagul @ 2017-01-12 12:01 ` Shrirang Bagul 3 siblings, 0 replies; 10+ messages in thread From: Shrirang Bagul @ 2017-01-12 12:01 UTC (permalink / raw) To: alsa-devel Cc: linux-kernel, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Pierre-Louis Bossart, Shrirang Bagul, Irina Tirdea This patch adds support to toggle mute on 'line-out' pin on Dell Edge IoT Gateways. A dedicated GPIO on the SoC is used to control the amplifier. This GPIO is described in the BIOS DSD table. Signed-off-by: Shrirang Bagul <shrirang.bagul@canonical.com> --- sound/soc/intel/boards/bytcr_rt5640.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index e8c9a01..ae53a29 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -20,6 +20,7 @@ #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> @@ -74,6 +75,7 @@ struct byt_acpi_card { struct byt_rt5640_private { struct byt_acpi_card *acpi_card; + struct gpio_desc *gpio_lo_mute; struct clk *mclk; char codec_name[16]; int *clks; @@ -198,6 +200,19 @@ static int platform_clock_control(struct snd_soc_dapm_widget *w, 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_rt5640_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_rt5640_widgets[] = { SND_SOC_DAPM_HP("Headphone", NULL), SND_SOC_DAPM_MIC("Headset Mic", NULL), @@ -296,7 +311,7 @@ static const struct snd_kcontrol_new byt_rt5640_controls[] = { static const struct snd_soc_dapm_widget byt_rt5660_widgets[] = { SND_SOC_DAPM_MIC("Line In", NULL), - SND_SOC_DAPM_LINE("Line Out", 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), @@ -562,7 +577,18 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime) static int byt_rt5660_init(struct snd_soc_pcm_runtime *runtime) { int ret; + struct snd_soc_codec *codec = runtime->codec; struct snd_soc_card *card = runtime->card; + struct byt_rt5640_private *priv = snd_soc_card_get_drvdata(card); + + /* 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_dapm_add_routes(&card->dapm, byt_rt5640_ssp2_aif1_map, -- 2.9.3 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-01-16 15:10 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-12 12:00 [PATCH v2 0/4] ASoC: Dell IoT Gateway audio support Shrirang Bagul 2017-01-12 12:00 ` [PATCH v2 1/4] ASoC: rt5660: Add ACPI support Shrirang Bagul 2017-01-13 2:53 ` Bard Liao 2017-01-13 22:30 ` Andy Shevchenko 2017-01-12 12:00 ` [PATCH v2 2/4] ASoC: Intel: bytcr_rt5640: move codec clks to card driver data Shrirang Bagul 2017-01-12 12:01 ` [PATCH v2 3/4] ASoC: Intel: Support rt5660 codec for Baytrail Shrirang Bagul 2017-01-12 14:40 ` [alsa-devel] " Pierre-Louis Bossart 2017-01-16 7:45 ` Shrirang Bagul 2017-01-16 15:10 ` Pierre-Louis Bossart 2017-01-12 12:01 ` [PATCH v2 4/4] ASoC: Intel: bytcr_rt5640: Support line-out mute gpio Shrirang Bagul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).