[alsa-devel] Request for comments for SND_SOC_IMX_WM8731
This patch is a continuation of my work on supporting the Utilite devices on the vanilla kernel. I've modified the code slightly to conform to established practice. Namely instead of using src-port and ext-port, we use mux-int-port and mux-ext-port.
This driver has support for putting the codec in either master mode or slave mode. So far I've only had success with using slave mode. For trying to submit for kernel inclusion, should I strip out the code for codec master mode? As a kernel dev noob, any other comments are welcome.
Thanks, Jonathan Bennett
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig index 081e406..09c923f 100644 --- a/sound/soc/fsl/Kconfig +++ b/sound/soc/fsl/Kconfig @@ -229,6 +229,17 @@ config SND_SOC_EUKREA_TLV320 Enable I2S based access to the TLV320AIC23B codec attached to the SSI interface
+config SND_SOC_IMX_WM8731 + tristate "SoC Audio support for i.MX boards with wm8731 + depends on OF && I2C + select SND_SOC_WM8731 + select SND_SOC_IMX_PCM_DMA + select SND_SOC_IMX_AUDMUX + select SND_SOC_FSL_SSI + select SND_SOC_FSL_UTILS + help + Soc Audio support for i.MX with wm8731 + config SND_SOC_IMX_WM8962 tristate "SoC Audio support for i.MX boards with wm8962" depends on OF && I2C && INPUT diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile index d28dc25..4292fac 100644 --- a/sound/soc/fsl/Makefile +++ b/sound/soc/fsl/Makefile @@ -54,6 +54,7 @@ snd-soc-mx27vis-aic32x4-objs := mx27vis-aic32x4.o snd-soc-wm1133-ev1-objs := wm1133-ev1.o snd-soc-imx-es8328-objs := imx-es8328.o snd-soc-imx-sgtl5000-objs := imx-sgtl5000.o +snd-soc-imx-wm8731-objs := imx-wm8731.o snd-soc-imx-wm8962-objs := imx-wm8962.o snd-soc-imx-spdif-objs := imx-spdif.o snd-soc-imx-mc13783-objs := imx-mc13783.o @@ -65,5 +66,6 @@ obj-$(CONFIG_SND_MXC_SOC_WM1133_EV1) += snd-soc-wm1133-ev1.o obj-$(CONFIG_SND_SOC_IMX_ES8328) += snd-soc-imx-es8328.o obj-$(CONFIG_SND_SOC_IMX_SGTL5000) += snd-soc-imx-sgtl5000.o obj-$(CONFIG_SND_SOC_IMX_WM8962) += snd-soc-imx-wm8962.o +obj-$(CONFIG_SND_SOC_IMX_WM8731) += snd-soc-imx-wm8731.o obj-$(CONFIG_SND_SOC_IMX_SPDIF) += snd-soc-imx-spdif.o obj-$(CONFIG_SND_SOC_IMX_MC13783) += snd-soc-imx-mc13783.o diff --git a/sound/soc/fsl/imx-wm8731.c b/sound/soc/fsl/imx-wm8731.c new file mode 100644 index 0000000..8ce4e9f --- /dev/null +++ b/sound/soc/fsl/imx-wm8731.c @@ -0,0 +1,687 @@ +/* + * Copyright (C) 2014 Freescale Semiconductor, Inc. + * + * Based on imx-sgtl5000.c + * Copyright (C) 2012 Freescale Semiconductor, Inc. + * Copyright (C) 2012 Linaro Ltd. + * + * The code contained herein is licensed under the GNU General Public + * License. You may obtain a copy of the GNU General Public License + * Version 2 or later at the following locations: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include <linux/module.h> +#include <linux/of_platform.h> +#include <linux/i2c.h> +#include <linux/of_gpio.h> +#include <linux/slab.h> +#include <linux/gpio.h> +#include <linux/clk.h> +#include <sound/soc.h> +#include <sound/jack.h> +#include <sound/pcm_params.h> +#include <sound/soc-dapm.h> +#include <linux/pinctrl/consumer.h> + +#include "../codecs/wm8731.h" +#include "imx-audmux.h" +#include "imx-ssi.h" + +#define DAI_NAME_SIZE 32 +#define WM8731_MCLK_FREQ (24000000 / 2) + +struct imx_wm8731_data { + struct snd_soc_dai_link dai; + struct snd_soc_card card; + char codec_dai_name[DAI_NAME_SIZE]; + char platform_name[DAI_NAME_SIZE]; + struct i2c_client *codec_dev; + /* audio_clocking_data */ + struct clk *pll; + struct clk *clock_root; + long sysclk; + long current_rate; + /* apis */ + int (*clock_enable)(int enable,struct imx_wm8731_data *data); +}; + +static int imx_wm8731_init(struct snd_soc_pcm_runtime *rtd); +static int imx_hifi_hw_params_slv_mode(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params); +static void imx_hifi_shutdown(struct snd_pcm_substream *substream); + +struct imx_priv { + struct platform_device *pdev; + struct imx_wm8731_data *data; +}; + +static struct imx_priv card_priv; + +static struct snd_soc_ops imx_hifi_ops = { + .shutdown = imx_hifi_shutdown, +}; + +/* imx card dapm widgets */ +static const struct snd_soc_dapm_widget imx_dapm_widgets[] = { + SND_SOC_DAPM_HP("Headphone Jack", NULL), + SND_SOC_DAPM_SPK("Ext Spk", NULL), + SND_SOC_DAPM_LINE("Line Jack", NULL), + SND_SOC_DAPM_MIC("Mic Jack", NULL), +}; + +/* imx machine connections to the codec pins */ +static const struct snd_soc_dapm_route audio_map[] = { + { "Headphone Jack", NULL, "LHPOUT" }, + { "Headphone Jack", NULL, "RHPOUT" }, + + { "Ext Spk", NULL, "LOUT" }, + { "Ext Spk", NULL, "ROUT" }, + + { "LLINEIN", NULL, "Line Jack" }, + { "RLINEIN", NULL, "Line Jack" }, + + { "MICIN", NULL, "Mic Bias" }, + { "Mic Bias", NULL, "Mic Jack"}, +}; + +static int wm8731_slv_mode_init(struct imx_wm8731_data *data) +{ + struct clk *new_parent; + struct clk *ssi_clk; + struct i2c_client *codec_dev = data->codec_dev; + + new_parent = devm_clk_get(&codec_dev->dev, "pll4"); + if (IS_ERR(new_parent)) { + pr_err("Could not get "pll4" clock \n"); + return PTR_ERR(new_parent); + } + + ssi_clk = devm_clk_get(&codec_dev->dev, "imx-ssi.1"); + if (IS_ERR(ssi_clk)) { + pr_err("Could not get "imx-ssi.1" clock \n"); + return PTR_ERR(ssi_clk); + } + + clk_set_parent(ssi_clk, new_parent); + + data->pll = new_parent; + data->clock_root = ssi_clk; + data->current_rate = 0; + + data->sysclk = 0; + + return 0; +} + +static int wm8731_slv_mode_clock_enable(int enable, struct imx_wm8731_data *data) +{ + long pll_rate; + long rate_req; + long rate_avail; + + if (!enable) + return 0; + + if (data->sysclk == data->current_rate) + return 0; + + switch (data->sysclk) { + case 11289600: + pll_rate = 632217600; + break; + + case 12288000: + pll_rate = 688128000; + break; + + default: + return -EINVAL; + } + + rate_req = pll_rate; + rate_avail = clk_round_rate(data->pll, rate_req); + clk_set_rate(data->pll, rate_avail); + + rate_req = data->sysclk; + rate_avail = clk_round_rate(data->clock_root, + rate_req); + clk_set_rate(data->clock_root, rate_avail); + + pr_info("%s: "imx-ssi.1" rate = %ld (= %ld)\n", + __func__, rate_avail, rate_req); + + data->current_rate = data->sysclk; + + return 0; +} + +static int imx_hifi_startup_slv_mode(struct snd_pcm_substream *substream) +{ + /* + * As SSI's sys clock rate depends on sampling rate, + * the clock enabling code is moved to imx_hifi_hw_params(). + */ + return 0; +} + +static int wm8731_mst_mode_init(struct imx_wm8731_data *data) +{ + long rate; + struct clk *new_parent; + struct clk *ssi_clk; + struct i2c_client *codec_dev = data->codec_dev; + + new_parent = devm_clk_get(&codec_dev->dev, "cko2"); + if (IS_ERR(new_parent)) { + pr_err("Could not get "cko2" clock \n"); + return PTR_ERR(new_parent); + } + + ssi_clk = devm_clk_get(&codec_dev->dev, "cko"); + if (IS_ERR(ssi_clk)) { + pr_err("Could not get "cko" clock \n"); + return PTR_ERR(ssi_clk); + } + + rate = clk_round_rate(new_parent, WM8731_MCLK_FREQ); + clk_set_rate(new_parent, rate); + + clk_set_parent(ssi_clk, new_parent); + + rate = clk_round_rate(ssi_clk, WM8731_MCLK_FREQ); + clk_set_rate(ssi_clk, rate); + + pr_info("%s: "CLKO" rate = %ld (= %d)\n", + __func__, rate, WM8731_MCLK_FREQ); + + data->pll = new_parent; + data->clock_root = ssi_clk; + data->sysclk = rate; + + return 0; +} + +static int wm8731_mst_mode_clock_enable(int enable, struct imx_wm8731_data *data) +{ + struct clk *clko = data->clock_root; + + if (enable) + clk_enable(clko); + else + clk_disable(clko); + + return 0; +} + +static int imx_hifi_startup_mst_mode(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *codec_dai = rtd->codec_dai; + struct snd_soc_card *card = codec_dai->card; + struct imx_wm8731_data *data = snd_soc_card_get_drvdata(card); + + if (!codec_dai->active) + data->clock_enable(1,data); + + return 0; +} + + +static int imx_hifi_hw_params_slv_mode(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + struct snd_soc_dai *codec_dai = rtd->codec_dai; + struct snd_soc_card *card = codec_dai->card; + struct imx_wm8731_data *data = snd_soc_card_get_drvdata(card); + + u32 dai_format; + snd_pcm_format_t sample_format; + unsigned int channels; + unsigned int tx_mask, rx_mask; + unsigned int sampling_rate; + unsigned int div_2, div_psr, div_pm; + int ret; + + sampling_rate = params_rate(params); + sample_format = params_format(params); + + channels = params_channels(params); + printk("%s:%s sampling rate = %u channels = %u \n", __FUNCTION__, + (substream->stream == SNDRV_PCM_STREAM_PLAYBACK ? "Playback" : "Capture"), + sampling_rate, channels); + + /* set CPU DAI configuration */ + switch (sampling_rate) { + case 8000: + case 32000: + case 48000: + case 96000: + data->sysclk = 12288000; + break; + + case 44100: + case 88200: + data->sysclk = 11289600; + break; + + default: + return -EINVAL; + } + + wm8731_slv_mode_clock_enable(1,data); + + dai_format = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_IF | + SND_SOC_DAIFMT_CBS_CFS; + + ret = snd_soc_dai_set_fmt(cpu_dai, dai_format); + if (ret < 0) + return ret; + + /* set i.MX active slot mask */ + /* S[TR]CCR:DC */ + tx_mask = ~((1 << channels) - 1); + rx_mask = tx_mask; + snd_soc_dai_set_tdm_slot(cpu_dai, tx_mask, rx_mask, 2, 32); + + /* + * SSI sysclk divider: + * div_2: /1 or /2 + * div_psr: /1 or /8 + * div_pm: /1 .. /256 + */ + div_2 = 0; + div_psr = 0; + switch (sampling_rate) { + case 8000: + // 1x1x12 + div_pm = 11; + break; + case 32000: + // 1x1x3 + div_pm = 2; + break; + case 48000: + // 1x1x2 + div_pm = 1; + break; + case 96000: + // 1x1x1 + div_pm = 0; + break; + case 44100: + // 1x1x2 + div_pm = 1; + break; + case 88200: + // 1x1x1 + div_pm = 0; + break; + default: + return -EINVAL; + } + + /* sync mode: a single clock controls both playback and capture */ + snd_soc_dai_set_clkdiv(cpu_dai, IMX_SSI_TX_DIV_2, (div_2 ? SSI_STCCR_DIV2 : 0)); + snd_soc_dai_set_clkdiv(cpu_dai, IMX_SSI_TX_DIV_PSR, (div_psr ? SSI_STCCR_PSR : 0)); + snd_soc_dai_set_clkdiv(cpu_dai, IMX_SSI_TX_DIV_PM, div_pm); + + /* set codec DAI configuration */ + dai_format = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | + SND_SOC_DAIFMT_CBS_CFS; + + ret = snd_soc_dai_set_fmt(codec_dai, dai_format); + if (ret < 0) + return ret; + + ret = snd_soc_dai_set_sysclk(codec_dai, + WM8731_SYSCLK_MCLK, + data->sysclk, + SND_SOC_CLOCK_IN); + + if (ret < 0) { + pr_err("Failed to set codec master clock to %u: %d \n", + data->sysclk, ret); + return ret; + } + + return 0; +} + +static int imx_hifi_hw_params_mst_mode(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + struct snd_soc_dai *codec_dai = rtd->codec_dai; + struct snd_soc_card *card = codec_dai->card; + struct imx_wm8731_data *data = snd_soc_card_get_drvdata(card); + u32 dai_format; + unsigned int channels; + unsigned int tx_mask, rx_mask; + unsigned int sampling_rate; + int ret; + + + sampling_rate = params_rate(params); + channels = params_channels(params); + pr_debug("%s:%s sampling rate = %u channels = %u \n", __FUNCTION__, + (substream->stream == SNDRV_PCM_STREAM_PLAYBACK ? "Playback" : "Capture"), + sampling_rate, channels); + + /* set cpu DAI configuration */ + dai_format = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_IF | + SND_SOC_DAIFMT_CBM_CFM; + + ret = snd_soc_dai_set_fmt(cpu_dai, dai_format); + if (ret < 0) + return ret; + + /* set i.MX active slot mask */ + /* S[TR]CCR:DC */ + tx_mask = ~((1 << channels) - 1); + rx_mask = tx_mask; + snd_soc_dai_set_tdm_slot(cpu_dai, tx_mask, rx_mask, 2, 32); + + /* set codec DAI configuration */ + dai_format = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | + SND_SOC_DAIFMT_CBM_CFM; + + ret = snd_soc_dai_set_fmt(codec_dai, dai_format); + if (ret < 0) + return ret; + + ret = snd_soc_dai_set_sysclk(codec_dai, + WM8731_SYSCLK_MCLK, + data->sysclk, + SND_SOC_CLOCK_IN); + + if (ret < 0) { + pr_err("Failed to set codec master clock to %u: %d \n", + data->sysclk, ret); + return ret; + } + + return 0; +} + +static void imx_hifi_shutdown(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *codec_dai = rtd->codec_dai; + struct snd_soc_card *card = codec_dai->card; + struct imx_wm8731_data *data = snd_soc_card_get_drvdata(card); + + if (!codec_dai->active) + data->clock_enable(0,data); + + return; +} + +static int imx_wm8731_init(struct snd_soc_pcm_runtime *rtd) +{ + int ret = 0; + struct snd_soc_codec *codec = rtd->codec; + + /* Add imx specific widgets */ + ret = snd_soc_dapm_new_controls(&codec->dapm, imx_dapm_widgets, + ARRAY_SIZE(imx_dapm_widgets)); + if (ret) + goto out_retcode; + + /* Set up imx specific audio path audio_map */ + ret = snd_soc_dapm_add_routes(&codec->dapm, audio_map, ARRAY_SIZE(audio_map)); + if (ret) + goto out_retcode; + + ret = snd_soc_dapm_enable_pin(&codec->dapm, "Headphone Jack"); + if (ret) + goto out_retcode; + + ret = snd_soc_dapm_nc_pin(&codec->dapm, "Ext Spk"); + if (ret) + goto out_retcode; + +out_retcode: + + if (ret) + pr_err("%s: failed with error code: %d \n", __FUNCTION__, ret); + else + pr_info("%s: success \n", __FUNCTION__); + + return ret; +} + +/** + * Configure AUDMUX interconnection between + * _slave (CPU side) and _master (codec size) + * + * When SSI operates in master mode, 5-wire interconnect with + * audio codec is required: + * TXC - BCLK + * TXD - DAC data + * RXD - ADC data + * TXFS - {DAC|ADC}LRC, i.e. word clock + * RXC - MCLK, i.e. oversampling clock + * Audmux is operated in asynchronous mode to enable 6-wire + * interface (as opposed to 4-wire interface in sync mode). + */ +static int imx_audmux_config_slv_mode(int _slave, int _master) +{ + unsigned int ptcr, pdcr; + int slave = _slave - 1; + int master = _master - 1; + + ptcr = IMX_AUDMUX_V2_PTCR_SYN | + IMX_AUDMUX_V2_PTCR_TFSDIR | + IMX_AUDMUX_V2_PTCR_TFSEL(slave) | + IMX_AUDMUX_V2_PTCR_RCLKDIR | + IMX_AUDMUX_V2_PTCR_RCSEL(slave | 0x8) | + IMX_AUDMUX_V2_PTCR_TCLKDIR | + IMX_AUDMUX_V2_PTCR_TCSEL(slave); + + pdcr = IMX_AUDMUX_V2_PDCR_RXDSEL(slave); + imx_audmux_v2_configure_port(master, ptcr, pdcr); + ptcr = ptcr & ~IMX_AUDMUX_V2_PTCR_SYN; + imx_audmux_v2_configure_port(master, ptcr, pdcr); + + ptcr = IMX_AUDMUX_V2_PTCR_SYN | + IMX_AUDMUX_V2_PTCR_RCLKDIR | + IMX_AUDMUX_V2_PTCR_RCSEL(master | 0x8) | + IMX_AUDMUX_V2_PTCR_TCLKDIR | + IMX_AUDMUX_V2_PTCR_TCSEL(master); + + pdcr = IMX_AUDMUX_V2_PDCR_RXDSEL(master); + imx_audmux_v2_configure_port(slave, ptcr, pdcr); + ptcr = ptcr & ~IMX_AUDMUX_V2_PTCR_SYN; + imx_audmux_v2_configure_port(slave, ptcr, pdcr); + + return 0; +} + +static int imx_audmux_config_mst_mode(int _slave, int _master) +{ + unsigned int ptcr, pdcr; + int slave = _slave - 1; + int master = _master - 1; + + ptcr = IMX_AUDMUX_V2_PTCR_SYN; + ptcr |= IMX_AUDMUX_V2_PTCR_TFSDIR | + IMX_AUDMUX_V2_PTCR_TFSEL(master) | + IMX_AUDMUX_V2_PTCR_TCLKDIR | + IMX_AUDMUX_V2_PTCR_TCSEL(master); + pdcr = IMX_AUDMUX_V2_PDCR_RXDSEL(master); + imx_audmux_v2_configure_port(slave, ptcr, pdcr); + + ptcr = IMX_AUDMUX_V2_PTCR_SYN; + pdcr = IMX_AUDMUX_V2_PDCR_RXDSEL(slave); + imx_audmux_v2_configure_port(master, ptcr, pdcr); + + return 0; +} + +static int imx_wm8731_probe(struct platform_device *pdev) +{ + struct device_node *ssi_np, *codec_np; + struct platform_device *ssi_pdev; + struct imx_priv *priv = &card_priv; + struct i2c_client *codec_dev; + struct imx_wm8731_data *data; + unsigned int src_port, ext_port; + unsigned int ssi_mode; + const char *ssi_mode_str; + + int ret; + + priv->pdev = pdev; + + ssi_np = of_parse_phandle(pdev->dev.of_node, "ssi-controller", 0); + codec_np = of_parse_phandle(pdev->dev.of_node, "audio-codec", 0); + if (!ssi_np || !codec_np) { + dev_err(&pdev->dev, "phandle missing or invalid\n"); + ret = -EINVAL; + goto fail; + } + + ssi_pdev = of_find_device_by_node(ssi_np); + if (!ssi_pdev) { + dev_err(&pdev->dev, "failed to find SSI platform device\n"); + ret = -EINVAL; + goto fail; + } + + codec_dev = of_find_i2c_device_by_node(codec_np); + if (!codec_dev || !codec_dev->dev.driver) { + dev_err(&pdev->dev, "failed to find codec platform device\n"); + ret = -EINVAL; + goto fail; + } + + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); + if (!data) { + ret = -ENOMEM; + goto fail; + } + + card_priv.data = data; + + data->codec_dev = codec_dev; + + data->dai.name = "HiFi"; + data->dai.stream_name = "HiFi"; + data->dai.codec_dai_name = "wm8731-hifi"; + data->dai.codec_of_node = codec_np; + data->dai.cpu_dai_name = dev_name(&ssi_pdev->dev); + data->dai.platform_of_node = ssi_np; + data->dai.ops = &imx_hifi_ops; + data->dai.init = &imx_wm8731_init; + + ret = of_property_read_u32(pdev->dev.of_node, "mux-int-port", &src_port); + if (ret) { + dev_err(&pdev->dev, "failed to get "mux-int-port" value\n"); + ret = -EINVAL; + goto fail; + } + + ret = of_property_read_u32(pdev->dev.of_node, "mux-ext-port", &ext_port); + if (ret) { + dev_err(&pdev->dev, "failed to get "mux-ext-port" value\n"); + ret = -EINVAL; + goto fail; + } + + ret = of_property_read_string(ssi_np, "fsl,mode", &ssi_mode_str); + if (ret) { + dev_err(&pdev->dev, "failed to get "fsl,mode" value\n"); + ret = -EINVAL; + goto fail; + } + + ssi_mode = strcmp(ssi_mode_str, "i2s-master"); + + if (ssi_mode) { + /* Master Mode */ + imx_audmux_config_mst_mode(src_port, ext_port); + wm8731_mst_mode_init(data); + data->clock_enable = wm8731_mst_mode_clock_enable; + imx_hifi_ops.hw_params = imx_hifi_hw_params_mst_mode; + imx_hifi_ops.startup = imx_hifi_startup_mst_mode; + } else { + /* Slave Mode */ + imx_audmux_config_slv_mode(src_port, ext_port); + wm8731_slv_mode_init(data); + data->clock_enable = wm8731_slv_mode_clock_enable; + imx_hifi_ops.hw_params = imx_hifi_hw_params_slv_mode; + imx_hifi_ops.startup = imx_hifi_startup_slv_mode; + } + + data->card.dev = &pdev->dev; + ret = snd_soc_of_parse_card_name(&data->card, "model"); + if (ret) + goto fail; + + ret = snd_soc_of_parse_audio_routing(&data->card, "audio-routing"); + if (ret) + goto fail; + + data->card.num_links = 1; + data->card.dai_link = &data->dai; + + data->card.dapm_widgets = imx_dapm_widgets; + data->card.num_dapm_widgets = ARRAY_SIZE(imx_dapm_widgets); + + platform_set_drvdata(pdev, &data->card); + snd_soc_card_set_drvdata(&data->card, data); + + ret = devm_snd_soc_register_card(&pdev->dev, &data->card); + if (ret) { + dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret); + goto fail; + } +/* of_node_put(ssi_np); + of_node_put(codec_np); */ + return 0; +fail: + + if (ssi_np) + of_node_put(ssi_np); + + if (codec_np) + of_node_put(codec_np); + + return ret; +} + +static int imx_wm8731_remove(struct platform_device *pdev) +{ + struct snd_soc_card *card = platform_get_drvdata(pdev); + snd_soc_unregister_card(card); + + return 0; +} + +static const struct of_device_id imx_wm8731_dt_ids[] = { + { .compatible = "fsl,imx-audio-wm8731", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, imx_wm8731_dt_ids); + +static struct platform_driver imx_wm8731_driver = { + .driver = { + .name = "imx-wm8731", + .owner = THIS_MODULE, + .of_match_table = imx_wm8731_dt_ids, + }, + .probe = imx_wm8731_probe, + .remove = imx_wm8731_remove, +}; +module_platform_driver(imx_wm8731_driver); + +MODULE_AUTHOR("Freescale Semiconductor, Inc."); +MODULE_DESCRIPTION("Freescale i.MX WM8731 ASoC machine driver"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:imx-wm8731");
On Fri, Oct 24, 2014 at 2:42 AM, Jonathan Bennett jbscience87@gmail.com wrote:
This patch is a continuation of my work on supporting the Utilite devices on the vanilla kernel. I've modified the code slightly to conform to established practice. Namely instead of using src-port and ext-port, we use mux-int-port and mux-ext-port.
This driver has support for putting the codec in either master mode or slave mode. So far I've only had success with using slave mode. For trying to submit for kernel inclusion, should I strip out the code for codec master
Yes, just add codec slave support then.
mode? As a kernel dev noob, any other comments are welcome.
Some suggestions: - Run ./scripts/checkpatch on your patch and fix all the reported errors/warnings - Add the maintainers on Cc (Mark Brown, Nicolin Chen) - In the subject, use something like: ASoC: fsl: Add support for imx-wm8731 and a proper commit message - Send the patch via git send-email
More comments below:
+config SND_SOC_IMX_WM8731
- tristate "SoC Audio support for i.MX boards with wm8731
- depends on OF && I2C
- select SND_SOC_WM8731
- select SND_SOC_IMX_PCM_DMA
- select SND_SOC_IMX_AUDMUX
- select SND_SOC_FSL_SSI
- select SND_SOC_FSL_UTILS
You can remove this UTILS option.
+static int wm8731_mst_mode_init(struct imx_wm8731_data *data) +{
- long rate;
- struct clk *new_parent;
- struct clk *ssi_clk;
- struct i2c_client *codec_dev = data->codec_dev;
- new_parent = devm_clk_get(&codec_dev->dev, "cko2");
- if (IS_ERR(new_parent)) {
- pr_err("Could not get "cko2" clock \n");
- return PTR_ERR(new_parent);
- }
- ssi_clk = devm_clk_get(&codec_dev->dev, "cko");
- if (IS_ERR(ssi_clk)) {
- pr_err("Could not get "cko" clock \n");
- return PTR_ERR(ssi_clk);
I am not sure that adding the clock handling here is the correct approach.
The dts you use looks like:
codec: wm8731@1a { compatible = "wlf,wm8731"; reg = <0x1a>; clocks = <&clks 173>, <&clks 158>, <&clks 201>, <&clks 200>; clock-names = "pll4", "imx-ssi.1", "cko", "cko2"; AVDD-supply = <®_3p3v>; HPVDD-supply = <®_3p3v>; DCVDD-supply = <®_3p3v>; DBVDD-supply = <®_3p3v>;
,which does not match the current bindings for the wm8731. The wm8731 driver could be extended to handle the clocks if needed, instead of handling them via machine driver.
+static int imx_hifi_hw_params_slv_mode(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- struct snd_soc_dai *codec_dai = rtd->codec_dai;
- struct snd_soc_card *card = codec_dai->card;
- struct imx_wm8731_data *data = snd_soc_card_get_drvdata(card);
- u32 dai_format;
- snd_pcm_format_t sample_format;
- unsigned int channels;
- unsigned int tx_mask, rx_mask;
- unsigned int sampling_rate;
- unsigned int div_2, div_psr, div_pm;
- int ret;
- sampling_rate = params_rate(params);
- sample_format = params_format(params);
- channels = params_channels(params);
- printk("%s:%s sampling rate = %u channels = %u \n", __FUNCTION__,
pr_debug here?
- div_2 = 0;
- div_psr = 0;
- switch (sampling_rate) {
- case 8000:
- // 1x1x12
No // style comments. Comments should use teh /* bla bla bla */ style.
+static int imx_wm8731_probe(struct platform_device *pdev) +{
- struct device_node *ssi_np, *codec_np;
- struct platform_device *ssi_pdev;
- struct imx_priv *priv = &card_priv;
- struct i2c_client *codec_dev;
- struct imx_wm8731_data *data;
- unsigned int src_port, ext_port;
- unsigned int ssi_mode;
- const char *ssi_mode_str;
- int ret;
- priv->pdev = pdev;
- ssi_np = of_parse_phandle(pdev->dev.of_node, "ssi-controller", 0);
- codec_np = of_parse_phandle(pdev->dev.of_node, "audio-codec", 0);
- if (!ssi_np || !codec_np) {
- dev_err(&pdev->dev, "phandle missing or invalid\n");
- ret = -EINVAL;
- goto fail;
No need for jumping to 'fail', just return the error directly. Same on the other locations.
- }
- ssi_pdev = of_find_device_by_node(ssi_np);
- if (!ssi_pdev) {
- dev_err(&pdev->dev, "failed to find SSI platform device\n");
- ret = -EINVAL;
- goto fail;
- }
- codec_dev = of_find_i2c_device_by_node(codec_np);
- if (!codec_dev || !codec_dev->dev.driver) {
- dev_err(&pdev->dev, "failed to find codec platform device\n");
- ret = -EINVAL;
- goto fail;
- }
- data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
- if (!data) {
- ret = -ENOMEM;
- goto fail;
- }
- card_priv.data = data;
- data->codec_dev = codec_dev;
- data->dai.name = "HiFi";
- data->dai.stream_name = "HiFi";
- data->dai.codec_dai_name = "wm8731-hifi";
- data->dai.codec_of_node = codec_np;
- data->dai.cpu_dai_name = dev_name(&ssi_pdev->dev);
- data->dai.platform_of_node = ssi_np;
- data->dai.ops = &imx_hifi_ops;
- data->dai.init = &imx_wm8731_init;
- ret = of_property_read_u32(pdev->dev.of_node, "mux-int-port", &src_port);
- if (ret) {
- dev_err(&pdev->dev, "failed to get "mux-int-port" value\n");
- ret = -EINVAL;
- goto fail;
- }
- ret = of_property_read_u32(pdev->dev.of_node, "mux-ext-port", &ext_port);
- if (ret) {
- dev_err(&pdev->dev, "failed to get "mux-ext-port" value\n");
- ret = -EINVAL;
- goto fail;
- }
- ret = of_property_read_string(ssi_np, "fsl,mode", &ssi_mode_str);
- if (ret) {
- dev_err(&pdev->dev, "failed to get "fsl,mode" value\n");
- ret = -EINVAL;
- goto fail;
- }
- ssi_mode = strcmp(ssi_mode_str, "i2s-master");
- if (ssi_mode) {
- /* Master Mode */
- imx_audmux_config_mst_mode(src_port, ext_port);
- wm8731_mst_mode_init(data);
- data->clock_enable = wm8731_mst_mode_clock_enable;
- imx_hifi_ops.hw_params = imx_hifi_hw_params_mst_mode;
- imx_hifi_ops.startup = imx_hifi_startup_mst_mode;
- } else {
- /* Slave Mode */
- imx_audmux_config_slv_mode(src_port, ext_port);
- wm8731_slv_mode_init(data);
- data->clock_enable = wm8731_slv_mode_clock_enable;
- imx_hifi_ops.hw_params = imx_hifi_hw_params_slv_mode;
- imx_hifi_ops.startup = imx_hifi_startup_slv_mode;
- }
- data->card.dev = &pdev->dev;
- ret = snd_soc_of_parse_card_name(&data->card, "model");
- if (ret)
- goto fail;
- ret = snd_soc_of_parse_audio_routing(&data->card, "audio-routing");
- if (ssi_np)
- of_node_put(ssi_np);
of_node_put can handle NULL, so no need for the if test.
- if (codec_np)
- of_node_put(codec_np);
Same here.
On Fri, Oct 24, 2014 at 10:20:29AM -0200, Fabio Estevam wrote:
On Fri, Oct 24, 2014 at 2:42 AM, Jonathan Bennett jbscience87@gmail.com wrote:
- ssi_clk = devm_clk_get(&codec_dev->dev, "cko");
- if (IS_ERR(ssi_clk)) {
- pr_err("Could not get "cko" clock \n");
- return PTR_ERR(ssi_clk);
I am not sure that adding the clock handling here is the correct approach.
The dts you use looks like:
codec: wm8731@1a { compatible = "wlf,wm8731"; reg = <0x1a>; clocks = <&clks 173>, <&clks 158>, <&clks 201>, <&clks 200>; clock-names = "pll4", "imx-ssi.1", "cko", "cko2";
,which does not match the current bindings for the wm8731. The wm8731 driver could be extended to handle the clocks if needed, instead of handling them via machine driver.
The WM8731 doesn't have anything like that number of clock inputs so hanging them off the WM8731 seems very confused.
On Fri, Oct 24, 2014 at 12:04 PM, Mark Brown broonie@kernel.org wrote:
On Fri, Oct 24, 2014 at 10:20:29AM -0200, Fabio Estevam wrote:
On Fri, Oct 24, 2014 at 2:42 AM, Jonathan Bennett jbscience87@gmail.com wrote:
- ssi_clk = devm_clk_get(&codec_dev->dev, "cko");
- if (IS_ERR(ssi_clk)) {
- pr_err("Could not get "cko" clock \n");
- return PTR_ERR(ssi_clk);
I am not sure that adding the clock handling here is the correct approach.
The dts you use looks like:
codec: wm8731@1a { compatible = "wlf,wm8731"; reg = <0x1a>; clocks = <&clks 173>, <&clks 158>, <&clks 201>, <&clks 200>; clock-names = "pll4", "imx-ssi.1", "cko", "cko2";
,which does not match the current bindings for the wm8731. The wm8731 driver could be extended to handle the clocks if needed, instead of handling them via machine driver.
The WM8731 doesn't have anything like that number of clock inputs so hanging them off the WM8731 seems very confused.
Yes, exactly. If we need to pass a clock to WM8731 it should be MCLK only.
The pll4, ssi clocks are internal mx6 clocks, so the proposed clock handling should be reworked.
On Fri, Oct 24, 2014 at 9:10 AM, Fabio Estevam festevam@gmail.com wrote:
On Fri, Oct 24, 2014 at 12:04 PM, Mark Brown broonie@kernel.org wrote:
On Fri, Oct 24, 2014 at 10:20:29AM -0200, Fabio Estevam wrote:
On Fri, Oct 24, 2014 at 2:42 AM, Jonathan Bennett <
jbscience87@gmail.com> wrote:
- ssi_clk = devm_clk_get(&codec_dev->dev, "cko");
- if (IS_ERR(ssi_clk)) {
- pr_err("Could not get "cko" clock \n");
- return PTR_ERR(ssi_clk);
I am not sure that adding the clock handling here is the correct
approach.
The dts you use looks like:
codec: wm8731@1a { compatible = "wlf,wm8731"; reg = <0x1a>; clocks = <&clks 173>, <&clks 158>, <&clks 201>, <&clks 200>; clock-names = "pll4", "imx-ssi.1", "cko", "cko2";
,which does not match the current bindings for the wm8731. The wm8731 driver could be extended to handle the clocks if needed, instead of handling them via machine driver.
The WM8731 doesn't have anything like that number of clock inputs so hanging them off the WM8731 seems very confused.
Yes, exactly. If we need to pass a clock to WM8731 it should be MCLK only.
The pll4, ssi clocks are internal mx6 clocks, so the proposed clock handling should be reworked.
All 4 clocks were in that dts as a result of tinkering with both slave and master mode. We are actually only using the first two. That said, I think you're right. I think the ones we use should be moved to the sound {} block of the dts. I'll get these changes fixed up and send in the next version.
Oh, also, this patch is a bit big for the mailing list, what is the preferred method for sending in large patches to look at? I can attach, throw it on github, put it on an http server, etc.
On Fri, Oct 24, 2014 at 12:48 PM, Jonathan Bennett jbscience87@gmail.com wrote:
All 4 clocks were in that dts as a result of tinkering with both slave and master mode. We are actually only using the first two. That said, I think you're right. I think the ones we use should be moved to the sound {} block of the dts. I'll get these changes fixed up and send in the next version.
Oh, also, this patch is a bit big for the mailing list, what is the preferred method for sending in large patches to look at? I can attach, throw it on github, put it on an http server, etc.
Like I said previously just post the patch to the list (with maintainers on Cc) using git send-email.
Take a look at Documentation/SubmittingPatches.
On Fri, Oct 24, 2014 at 12:56 PM, Fabio Estevam festevam@gmail.com wrote:
On Fri, Oct 24, 2014 at 12:48 PM, Jonathan Bennett jbscience87@gmail.com wrote:
All 4 clocks were in that dts as a result of tinkering with both slave and master mode. We are actually only using the first two. That said, I think you're right. I think the ones we use should be moved to the sound {} block of the dts. I'll get these changes fixed up and send in the next version.
Oh, also, this patch is a bit big for the mailing list, what is the preferred method for sending in large patches to look at? I can attach, throw it on github, put it on an http server, etc.
Like I said previously just post the patch to the list (with maintainers on Cc) using git send-email.
Take a look at Documentation/SubmittingPatches.
Also, you need to add Documentation/devicetree/bindings/sound/imx-audio-wm8731.txt file.
On Fri, Oct 24, 2014 at 09:48:37AM -0500, Jonathan Bennett wrote:
Oh, also, this patch is a bit big for the mailing list, what is the preferred method for sending in large patches to look at? I can attach, throw it on github, put it on an http server, etc.
You need to follow the procedure in SubmittingPatches, that means you need to post your code to the list.
When I sent the original rfc message to the mailing list, it was denied for being over 60k, hence my question. Looking at the raw email, I suspect that gmail is inflating the size dramatically by sending both plaintext and html versions.
The document, SubmittingPatches, also refers to the large patch issue. On re-reading it, I see it puts the limit at 300k, which this is nowhere near. I'll see about using git send-email for the next attempt at sending it in. On Oct 24, 2014 11:19 AM, "Mark Brown" broonie@kernel.org wrote:
On Fri, Oct 24, 2014 at 09:48:37AM -0500, Jonathan Bennett wrote:
Oh, also, this patch is a bit big for the mailing list, what is the preferred method for sending in large patches to look at? I can attach, throw it on github, put it on an http server, etc.
You need to follow the procedure in SubmittingPatches, that means you need to post your code to the list.
On Fri, Oct 24, 2014 at 11:33:51AM -0500, Jonathan Bennett wrote:
When I sent the original rfc message to the mailing list, it was denied for being over 60k, hence my question. Looking at the raw email, I suspect that gmail is inflating the size dramatically by sending both plaintext and html versions.
The document, SubmittingPatches, also refers to the large patch issue. On re-reading it, I see it puts the limit at 300k, which this is nowhere near. I'll see about using git send-email for the next attempt at sending it in.
Right, *never* send patches in HTML format.
Hi Jonathan,
On Fri, Oct 24, 2014 at 10:20:29AM -0200, Fabio Estevam wrote:
On Fri, Oct 24, 2014 at 2:42 AM, Jonathan Bennett jbscience87@gmail.com wrote:
This patch is a continuation of my work on supporting the Utilite devices on the vanilla kernel. I've modified the code slightly to conform to established practice. Namely instead of using src-port and ext-port, we use mux-int-port and mux-ext-port.
This driver has support for putting the codec in either master mode or slave mode. So far I've only had success with using slave mode. For trying to submit for kernel inclusion, should I strip out the code for codec master
Just took a look at your driver. And I have a suggestion on the top of Fabio's comments.
I think you should try fsl-asoc-card driver instead of adding this one. Your driver doesn't seemly have some specific requirement and it actually contains a few out-of-date code like SSI clock divider settings. SSI driver now has the capability to calculate divisors automatically based on the required sample rate, even if it may not be perfect to apply all the sample rates. But I don't think the machine driver shall take care this job any more.
So I suggest you first to try the fsl-asoc-card with a specific change for wm8731 to make it work -- I think you only need to configure some parameters in its probe() as I did for the other CODECs. If it doesn't work out, you can raise the problem. I'll also help on it.
Thanks, Nicolin
participants (4)
-
Fabio Estevam -
Jonathan Bennett -
Mark Brown -
Nicolin Chen