On Tue, Mar 08, 2016 at 02:27:56PM +0800, Zidan Wang wrote:
This is an initial imx-wm8958 device-tree-only machine driver. The sound card driver support three dai link, HIFI, VOICE and BT. We can configure the cpu dai from device tree, if present it will create corresponding sound card subdevice. So at least one cpu dai phandle should be set from device tree.
Signed-off-by: Zidan Wang zidan.wang@freescale.com
v3->v4: remove the dummy cpu dai, support set cpu dai1/2/3 from device tree. When cpu-dai1/2/3 present it will create croresponding sound card subdevice. At least one cpu dai should be configured.
+++ b/Documentation/devicetree/bindings/sound/imx-audio-wm8958.txt +Optional properties:
- cpu-dai1 : The phandle of CPU DAI1 controller. At least one
cpu dai is required.
Should indicate that it's for the aif1 of WM8958.
- cpu-dai2 : The phandle of CPU DAI2 controller. At least one
cpu dai is required.
- cpu-dai3 : The phandle of CPU DAI3 controller. At least one
cpu dai is required.
Ditto
diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig index 978c5de..7ac5856 100644 --- a/arch/arm/configs/imx_v6_v7_defconfig +++ b/arch/arm/configs/imx_v6_v7_defconfig @@ -251,6 +251,7 @@ CONFIG_SND_SOC_FSL_ASRC=y CONFIG_SND_IMX_SOC=y CONFIG_SND_SOC_PHYCORE_AC97=y CONFIG_SND_SOC_EUKREA_TLV320=y +CONFIG_SND_SOC_IMX_WM8958=y CONFIG_SND_SOC_IMX_WM8962=y CONFIG_SND_SOC_IMX_SGTL5000=y CONFIG_SND_SOC_IMX_SPDIF=y
Shouldn't include this here.
diff --git a/sound/soc/fsl/imx-wm8958.c b/sound/soc/fsl/imx-wm8958.c
+#define HIFI_DAI (0) +#define VOICE_DAI (1) +#define BT_DAI (2)
We don't necessarily limit each AIF's role *unless* each AIF can only act as the one listed above -- In general, AIF1_DAI, AIF2_DAI would be more flexible for users IMO.
If you really want each role of the AIF to be clear, I suggest you to add an extra property (eg. dai-link-name) in the DT bindings because it does reflect the hardware connections according to the schematics.
+struct imx_wm8958_data {
- struct snd_soc_dai_link dai_link[DAI_LINK_NUM];
- int num_links;
- struct snd_soc_card card;
- struct clk *mclk[WM8958_MCLK_MAX];
Since the Codec driver handles the mclk now, we don't need to maintain it in the private data here any more.
- u32 mclk_freq[WM8958_MCLK_MAX];
unsigned long
+static struct snd_soc_ops imx_hifi_ops = {
- .hw_params = imx_wm8958_hw_params,
- .hw_free = imx_wm8958_hw_free,
+};
+static struct snd_soc_ops imx_voice_ops = {
- .hw_params = imx_wm8958_hw_params,
- .hw_free = imx_wm8958_hw_free,
+};
What's the difference between these two ops?
+static int imx_wm8958_probe(struct platform_device *pdev) +{
- struct device_node *cpu_np[DAI_LINK_NUM], *codec_np;
- struct device_node *np = pdev->dev.of_node;
- struct platform_device *cpu_pdev[DAI_LINK_NUM];
- struct i2c_client *codec_dev;
- struct imx_wm8958_data *data;
- char tmp[8];
- int ret, i;
- data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
- if (!data)
return -ENOMEM;- for (i = 0; i < DAI_LINK_NUM; i++) {
sprintf(tmp, "cpu-dai%d", i + 1);cpu_np[i] = of_parse_phandle(np, tmp, 0);if (cpu_np[i]) {cpu_pdev[i] = of_find_device_by_node(cpu_np[i]);if (!cpu_pdev[i]) {dev_err(&pdev->dev,"failed to get cpu dai%d platform device\n", i);ret = -EINVAL;goto fail;}data->dai_link[data->num_links] = imx_wm8958_dai_link[i];data->dai_link[data->num_links].cpu_dai_name = dev_name(&cpu_pdev[i]->dev);data->dai_link[data->num_links++].platform_of_node = cpu_np[i];
Some lines here crossed 80-characters. If I were you, I would use: if (!cpu_np[i]) continue;
And we don't necessarily maintain num_links in the private data since it's not being used in any other function and we may still fetch it from data->card.num_links even if we somehow need it later.