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@xxxxxxxxxxxxx>
> ---
> 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.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
