[alsa-devel] [PATCH] ASoC: qcom: add sdm845 sound card support
This patch adds sdm845 audio machine driver support.
Signed-off-by: Rohit kumar rohitkr@codeaurora.org --- .../devicetree/bindings/sound/qcom,sdm845.txt | 87 ++++ sound/soc/qcom/Kconfig | 9 + sound/soc/qcom/Makefile | 2 + sound/soc/qcom/sdm845.c | 539 +++++++++++++++++++++ 4 files changed, 637 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/qcom,sdm845.txt create mode 100644 sound/soc/qcom/sdm845.c
diff --git a/Documentation/devicetree/bindings/sound/qcom,sdm845.txt b/Documentation/devicetree/bindings/sound/qcom,sdm845.txt new file mode 100644 index 0000000..fc0e98c --- /dev/null +++ b/Documentation/devicetree/bindings/sound/qcom,sdm845.txt @@ -0,0 +1,87 @@ +* Qualcomm Technologies Inc. SDM845 ASoC sound card driver + +This binding describes the SDM845 sound card, which uses qdsp for audio. + +- compatible: + Usage: required + Value type: <stringlist> + Definition: must be "qcom,sdm845-sndcard" + +- qcom,audio-routing: + Usage: Optional + Value type: <stringlist> + Definition: A list of the connections between audio components. + Each entry is a pair of strings, the first being the + connection's sink, the second being the connection's + source. Valid names could be power supplies, MicBias + of codec and the jacks on the board. + +- cdc-vdd-supply: + Usage: Optional + Value type: <phandle> + Definition: phandle of regulator supply required for codec vdd. + += dailinks +Each subnode of sndcard represents either a dailink, and subnodes of each +dailinks would be cpu/codec/platform dais. + +- link-name: + Usage: required + Value type: <string> + Definition: User friendly name for dai link + += CPU, PLATFORM, CODEC dais subnodes +- cpu: + Usage: required + Value type: <subnode> + Definition: cpu dai sub-node + +- codec: + Usage: required + Value type: <subnode> + Definition: codec dai sub-node + +- platform: + Usage: opional + Value type: <subnode> + Definition: platform dai sub-node + +- sound-dai: + Usage: required + Value type: <phandle> + Definition: dai phandle/s and port of CPU/CODEC/PLATFORM node. + +Example: + +audio { + compatible = "qcom,sdm845-sndcard"; + qcom,model = "sdm845-snd-card"; + pinctrl-names = "pri_active", "pri_sleep"; + pinctrl-0 = <&pri_mi2s_active &pri_mi2s_ws_active>; + pinctrl-1 = <&pri_mi2s_sleep &pri_mi2s_ws_sleep>; + + qcom,audio-routing = "PRI_MI2S_RX Audio Mixer", "Pri-mi2s-gpio"; + + cdc-vdd-supply = <&pm8998_l14>; + + fe@1 { + link-name = "MultiMedia1"; + cpu { + sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>; + }; + platform { + sound-dai = <&q6asmdai>; + }; + }; + + be@1 { + link-name = "PRI MI2S Playback"; + cpu { + sound-dai = <&q6afedai PRIMARY_MI2S_RX>; + }; + + platform { + sound-dai = <&q6routing>; + }; + }; +}; diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig index 87838fa..09de50d 100644 --- a/sound/soc/qcom/Kconfig +++ b/sound/soc/qcom/Kconfig @@ -90,3 +90,12 @@ config SND_SOC_MSM8996 Support for Qualcomm Technologies LPASS audio block in APQ8096 SoC-based systems. Say Y if you want to use audio device on this SoCs + +config SND_SOC_SDM845 + tristate "SoC Machine driver for SDM845 boards" + depends on QCOM_APR + select SND_SOC_QDSP6 + help + To add support for audio on Qualcomm Technologies Inc. + SDM845 SoC-based systems. + Say Y if you want to use audio device on this SoCs diff --git a/sound/soc/qcom/Makefile b/sound/soc/qcom/Makefile index 206945b..ac9609e 100644 --- a/sound/soc/qcom/Makefile +++ b/sound/soc/qcom/Makefile @@ -14,10 +14,12 @@ obj-$(CONFIG_SND_SOC_LPASS_APQ8016) += snd-soc-lpass-apq8016.o snd-soc-storm-objs := storm.o snd-soc-apq8016-sbc-objs := apq8016_sbc.o snd-soc-apq8096-objs := apq8096.o +snd-soc-sdm845-objs := sdm845.o
obj-$(CONFIG_SND_SOC_STORM) += snd-soc-storm.o obj-$(CONFIG_SND_SOC_APQ8016_SBC) += snd-soc-apq8016-sbc.o obj-$(CONFIG_SND_SOC_MSM8996) += snd-soc-apq8096.o ++obj-$(CONFIG_SND_SOC_SDM845) += snd-soc-sdm845.o
#DSP lib obj-$(CONFIG_SND_SOC_QDSP6) += qdsp6/ diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c new file mode 100644 index 0000000..70d2611 --- /dev/null +++ b/sound/soc/qcom/sdm845.c @@ -0,0 +1,539 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + */ + +#include <linux/module.h> +#include <linux/component.h> +#include <linux/platform_device.h> +#include <linux/regulator/consumer.h> +#include <linux/atomic.h> +#include <linux/of_device.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <linux/soc/qcom/apr.h> +#include "qdsp6/q6afe.h" + +#define DEFAULT_SAMPLE_RATE_48K 48000 +#define DEFAULT_MCLK_RATE 24576000 +#define DEFAULT_BCLK_RATE 1536000 + +struct sdm845_snd_data { + struct snd_soc_card *card; + struct regulator *vdd_supply; + struct snd_soc_dai_link dai_link[]; +}; + +static struct mutex pri_mi2s_res_lock; +static struct mutex quat_tdm_res_lock; +static atomic_t pri_mi2s_clk_count; +static atomic_t quat_tdm_clk_count; + +static unsigned int tdm_slot_offset[8] = {0, 4, 8, 12, 16, 20, 24, 28}; + +static int sdm845_tdm_snd_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + int ret = 0; + int channels, slot_width; + + channels = params_channels(params); + if (channels < 1 || channels > 8) { + pr_err("%s: invalid param channels %d\n", + __func__, channels); + return -EINVAL; + } + + switch (params_format(params)) { + case SNDRV_PCM_FORMAT_S32_LE: + case SNDRV_PCM_FORMAT_S24_LE: + case SNDRV_PCM_FORMAT_S16_LE: + slot_width = 32; + break; + default: + pr_err("%s: invalid param format 0x%x\n", + __func__, params_format(params)); + return -EINVAL; + } + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0, 0x3, + channels, slot_width); + if (ret < 0) { + pr_err("%s: failed to set tdm slot, err:%d\n", + __func__, ret); + goto end; + } + + ret = snd_soc_dai_set_channel_map(cpu_dai, 0, NULL, + channels, tdm_slot_offset); + if (ret < 0) { + pr_err("%s: failed to set channel map, err:%d\n", + __func__, ret); + goto end; + } + } else { + ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0xf, 0, + channels, slot_width); + if (ret < 0) { + pr_err("%s: failed to set tdm slot, err:%d\n", + __func__, ret); + goto end; + } + + ret = snd_soc_dai_set_channel_map(cpu_dai, channels, + tdm_slot_offset, 0, NULL); + if (ret < 0) { + pr_err("%s: failed to set channel map, err:%d\n", + __func__, ret); + goto end; + } + } +end: + return ret; +} + +static int sdm845_snd_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + int ret = 0; + + switch (cpu_dai->id) { + case QUATERNARY_TDM_RX_0: + case QUATERNARY_TDM_TX_0: + ret = sdm845_tdm_snd_hw_params(substream, params); + break; + default: + pr_err("%s: invalid dai id 0x%x\n", __func__, cpu_dai->id); + break; + } + return ret; +} + +static int sdm845_snd_startup(struct snd_pcm_substream *substream) +{ + unsigned int fmt = SND_SOC_DAIFMT_CBS_CFS; + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + + pr_debug("%s: dai_id: 0x%x\n", __func__, cpu_dai->id); + switch (cpu_dai->id) { + case PRIMARY_MI2S_RX: + case PRIMARY_MI2S_TX: + mutex_lock(&pri_mi2s_res_lock); + if (atomic_inc_return(&pri_mi2s_clk_count) == 1) { + snd_soc_dai_set_sysclk(cpu_dai, + Q6AFE_LPASS_CLK_ID_MCLK_1, + DEFAULT_MCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK); + snd_soc_dai_set_sysclk(cpu_dai, + Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT, + DEFAULT_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK); + } + mutex_unlock(&pri_mi2s_res_lock); + snd_soc_dai_set_fmt(cpu_dai, fmt); + break; + case QUATERNARY_TDM_RX_0: + case QUATERNARY_TDM_TX_0: + mutex_lock(&quat_tdm_res_lock); + if (atomic_inc_return(&quat_tdm_clk_count) == 1) { + snd_soc_dai_set_sysclk(cpu_dai, + Q6AFE_LPASS_CLK_ID_QUAD_TDM_IBIT, + DEFAULT_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK); + } + mutex_unlock(&quat_tdm_res_lock); + break; + default: + pr_err("%s: invalid dai id 0x%x\n", __func__, cpu_dai->id); + break; + } + return 0; +} + +static void sdm845_snd_shutdown(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + + pr_debug("%s: dai_id: 0x%x\n", __func__, cpu_dai->id); + switch (cpu_dai->id) { + case PRIMARY_MI2S_RX: + case PRIMARY_MI2S_TX: + mutex_lock(&pri_mi2s_res_lock); + if (!atomic_dec_return(&pri_mi2s_clk_count)) { + snd_soc_dai_set_sysclk(cpu_dai, + Q6AFE_LPASS_CLK_ID_MCLK_1, + 0, SNDRV_PCM_STREAM_PLAYBACK); + snd_soc_dai_set_sysclk(cpu_dai, + Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT, + 0, SNDRV_PCM_STREAM_PLAYBACK); + }; + mutex_unlock(&pri_mi2s_res_lock); + break; + case QUATERNARY_TDM_RX_0: + case QUATERNARY_TDM_TX_0: + mutex_lock(&quat_tdm_res_lock); + if (!atomic_dec_return(&quat_tdm_clk_count)) { + snd_soc_dai_set_sysclk(cpu_dai, + Q6AFE_LPASS_CLK_ID_QUAD_TDM_IBIT, + 0, SNDRV_PCM_STREAM_PLAYBACK); + } + mutex_unlock(&quat_tdm_res_lock); + break; + default: + pr_err("%s: invalid dai id 0x%x\n", __func__, cpu_dai->id); + break; + } +} + +static struct snd_soc_ops sdm845_be_ops = { + .hw_params = sdm845_snd_hw_params, + .startup = sdm845_snd_startup, + .shutdown = sdm845_snd_shutdown, +}; + +static int sdm845_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd, + struct snd_pcm_hw_params *params) +{ + struct snd_interval *rate = hw_param_interval(params, + SNDRV_PCM_HW_PARAM_RATE); + struct snd_interval *channels = hw_param_interval(params, + SNDRV_PCM_HW_PARAM_CHANNELS); + + rate->min = rate->max = DEFAULT_SAMPLE_RATE_48K; + channels->min = channels->max = 2; + + return 0; +} + +static const struct snd_soc_dapm_widget sdm845_widgets[] = { + SND_SOC_DAPM_PINCTRL("Pri-mi2s-gpio", "pri_active", "pri_sleep"), + SND_SOC_DAPM_PINCTRL("Quat-tdm-gpio", "quat_active", "quat_sleep"), +}; + +static int sdm845_sbc_parse_of(struct snd_soc_card *card) +{ + struct device *dev = card->dev; + struct snd_soc_dai_link *link; + struct device_node *np, *codec, *platform, *cpu, *node; + int ret, num_links; + struct sdm845_snd_data *data; + + ret = snd_soc_of_parse_card_name(card, "qcom,model"); + if (ret) { + dev_err(dev, "Error parsing card name: %d\n", ret); + return ret; + } + + node = dev->of_node; + + /* DAPM routes */ + if (of_property_read_bool(node, "qcom,audio-routing")) { + ret = snd_soc_of_parse_audio_routing(card, + "qcom,audio-routing"); + if (ret) + return ret; + } + + /* Populate links */ + num_links = of_get_child_count(node); + + dev_info(dev, "Found %d child audio dai links..\n", num_links); + /* Allocate the private data and the DAI link array */ + data = kzalloc(sizeof(*data) + sizeof(*link) * num_links, + GFP_KERNEL); + if (!data) + return -ENOMEM; + + card->dai_link = &data->dai_link[0]; + card->num_links = num_links; + card->dapm_widgets = sdm845_widgets; + card->num_dapm_widgets = ARRAY_SIZE(sdm845_widgets); + + link = data->dai_link; + data->card = card; + + for_each_child_of_node(node, np) { + cpu = of_get_child_by_name(np, "cpu"); + platform = of_get_child_by_name(np, "platform"); + codec = of_get_child_by_name(np, "codec"); + + if (!cpu) { + dev_err(dev, "Can't find cpu DT node\n"); + ret = -EINVAL; + goto fail; + } + + link->cpu_of_node = of_parse_phandle(cpu, "sound-dai", 0); + if (!link->cpu_of_node) { + dev_err(card->dev, "error getting cpu phandle\n"); + ret = -EINVAL; + goto fail; + } + + link->platform_of_node = of_parse_phandle(platform, + "sound-dai", 0); + if (!link->platform_of_node) { + dev_err(card->dev, "error getting platform phandle\n"); + ret = -EINVAL; + goto fail; + } + + ret = snd_soc_of_get_dai_name(cpu, &link->cpu_dai_name); + if (ret) { + dev_err(card->dev, "error getting cpu dai name\n"); + goto fail; + } + + if (codec) { + ret = snd_soc_of_get_dai_link_codecs(dev, codec, link); + if (ret < 0) { + dev_err(card->dev, "error getting codec dai name\n"); + goto fail; + } + link->no_pcm = 1; + link->ignore_suspend = 1; + link->ignore_pmdown_time = 1; + link->ops = &sdm845_be_ops; + link->be_hw_params_fixup = sdm845_be_hw_params_fixup; + } else { + link->dynamic = 1; + link->codec_dai_name = "snd-soc-dummy-dai"; + link->codec_name = "snd-soc-dummy"; + } + + ret = of_property_read_string(np, "link-name", &link->name); + if (ret) { + dev_err(card->dev, + "error getting codec dai_link name\n"); + goto fail; + } + + link->dpcm_playback = 1; + link->dpcm_capture = 1; + link->stream_name = link->name; + link++; + } + dev_set_drvdata(dev, card); + snd_soc_card_set_drvdata(card, data); + + return ret; +fail: + kfree(data); + return ret; +} + +static void sdm845_init_supplies(struct device *dev) +{ + struct snd_soc_card *card = dev_get_drvdata(dev); + struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card); + + data->vdd_supply = regulator_get(dev, "cdc-vdd"); + if (IS_ERR(data->vdd_supply)) { + dev_err(dev, "Unable to get regulator supplies\n"); + data->vdd_supply = NULL; + return; + } + + if (regulator_enable(data->vdd_supply)) + dev_err(dev, "Unable to enable vdd supply\n"); +} + +static void sdm845_deinit_supplies(struct device *dev) +{ + struct snd_soc_card *card = dev_get_drvdata(dev); + struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card); + + if (!data->vdd_supply) + return; + + regulator_disable(data->vdd_supply); + regulator_put(data->vdd_supply); +} + +static int sdm845_bind(struct device *dev) +{ + struct snd_soc_card *card; + int ret; + + card = kzalloc(sizeof(*card), GFP_KERNEL); + if (!card) + return -ENOMEM; + + ret = component_bind_all(dev, card); + if (ret) { + dev_err(dev, "Audio components bind failed: %d\n", ret); + goto bind_fail; + } + + card->dev = dev; + ret = sdm845_sbc_parse_of(card); + if (ret) { + dev_err(dev, "Error parsing OF data\n"); + goto parse_dt_fail; + } + sdm845_init_supplies(dev); + + mutex_init(&pri_mi2s_res_lock); + mutex_init(&quat_tdm_res_lock); + atomic_set(&pri_mi2s_clk_count, 0); + atomic_set(&quat_tdm_clk_count, 0); + ret = snd_soc_register_card(card); + if (ret) { + dev_err(dev, "Sound card registration failed\n"); + goto register_card_fail; + } + return ret; + +register_card_fail: + mutex_destroy(&pri_mi2s_res_lock); + mutex_destroy(&quat_tdm_res_lock); + sdm845_deinit_supplies(dev); + kfree(snd_soc_card_get_drvdata(card)); +parse_dt_fail: + component_unbind_all(dev, card); +bind_fail: + kfree(card); + return ret; +} + +static void sdm845_unbind(struct device *dev) +{ + struct snd_soc_card *card = dev_get_drvdata(dev); + struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card); + + mutex_destroy(&pri_mi2s_res_lock); + mutex_destroy(&quat_tdm_res_lock); + if (data->vdd_supply) + regulator_put(data->vdd_supply); + component_unbind_all(dev, card); + snd_soc_unregister_card(card); + kfree(data); + kfree(card); +} + +static const struct component_master_ops sdm845_ops = { + .bind = sdm845_bind, + .unbind = sdm845_unbind, +}; + +static int sdm845_runtime_resume(struct device *dev) +{ + struct snd_soc_card *card = dev_get_drvdata(dev); + struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card); + + if (!data->vdd_supply) { + dev_dbg(dev, "no supplies defined\n"); + return 0; + } + + if (regulator_enable(data->vdd_supply)) + dev_err(dev, "Enable regulator supply failed\n"); + + return 0; +} + +static int sdm845_runtime_suspend(struct device *dev) +{ + struct snd_soc_card *card = dev_get_drvdata(dev); + struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card); + + if (!data->vdd_supply) { + dev_dbg(dev, "no supplies defined\n"); + return 0; + } + + if (regulator_disable(data->vdd_supply)) + dev_err(dev, "Disable regulator supply failed\n"); + + return 0; +} + +static const struct dev_pm_ops sdm845_pm_ops = { + SET_RUNTIME_PM_OPS(sdm845_runtime_suspend, + sdm845_runtime_resume, NULL) +}; + +static int sdm845_compare_of(struct device *dev, void *data) +{ + return dev->of_node == data; +} + +static void sdm845_release_of(struct device *dev, void *data) +{ + of_node_put(data); +} + +static int add_audio_components(struct device *dev, + struct component_match **matchptr) +{ + struct device_node *np, *platform, *cpu, *node, *dai_node; + + node = dev->of_node; + + for_each_child_of_node(node, np) { + cpu = of_get_child_by_name(np, "cpu"); + if (cpu) { + dai_node = of_parse_phandle(cpu, "sound-dai", 0); + of_node_get(dai_node); + component_match_add_release(dev, matchptr, + sdm845_release_of, + sdm845_compare_of, + dai_node); + } + + platform = of_get_child_by_name(np, "platform"); + if (platform) { + dai_node = of_parse_phandle(platform, "sound-dai", 0); + component_match_add_release(dev, matchptr, + sdm845_release_of, + sdm845_compare_of, + dai_node); + } + } + + return 0; +} + +static int sdm845_snd_platform_probe(struct platform_device *pdev) +{ + struct component_match *match = NULL; + int ret; + + ret = add_audio_components(&pdev->dev, &match); + if (ret) + return ret; + + return component_master_add_with_match(&pdev->dev, &sdm845_ops, match); +} + +static int sdm845_snd_platform_remove(struct platform_device *pdev) +{ + component_master_del(&pdev->dev, &sdm845_ops); + return 0; +} + +static const struct of_device_id sdm845_snd_device_id[] = { + { .compatible = "qcom,sdm845-sndcard" }, + {}, +}; +MODULE_DEVICE_TABLE(of, sdm845_snd_device_id); + +static struct platform_driver sdm845_snd_driver = { + .probe = sdm845_snd_platform_probe, + .remove = sdm845_snd_platform_remove, + .driver = { + .name = "msm-snd-sdm845", + .pm = &sdm845_pm_ops, + .owner = THIS_MODULE, + .of_match_table = of_match_ptr(sdm845_snd_device_id), + }, +}; +module_platform_driver(sdm845_snd_driver); + +MODULE_DESCRIPTION("sdm845 ASoC Machine Driver"); +MODULE_LICENSE("GPL v2");
On 18-06-18, 16:46, Rohit kumar wrote:
+struct sdm845_snd_data {
- struct snd_soc_card *card;
- struct regulator *vdd_supply;
- struct snd_soc_dai_link dai_link[];
+};
+static struct mutex pri_mi2s_res_lock; +static struct mutex quat_tdm_res_lock;
any reason why the locks can't be part of sdm845_snd_data? Also why do we need two locks ?
+static atomic_t pri_mi2s_clk_count; +static atomic_t quat_tdm_clk_count;
Any specific reason for using atomic variables?
+static unsigned int tdm_slot_offset[8] = {0, 4, 8, 12, 16, 20, 24, 28};
+static int sdm845_tdm_snd_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- int ret = 0;
- int channels, slot_width;
- channels = params_channels(params);
- if (channels < 1 || channels > 8) {
I though ch = 0 would be caught by framework and IIRC ASoC doesn't support more than 8 channels
pr_err("%s: invalid param channels %d\n",
__func__, channels);
return -EINVAL;
- }
- switch (params_format(params)) {
- case SNDRV_PCM_FORMAT_S32_LE:
- case SNDRV_PCM_FORMAT_S24_LE:
- case SNDRV_PCM_FORMAT_S16_LE:
slot_width = 32;
break;
- default:
pr_err("%s: invalid param format 0x%x\n",
__func__, params_format(params));
why not use dev_err, bonus you get device name printer with the logs :)
+static int sdm845_snd_startup(struct snd_pcm_substream *substream) +{
- unsigned int fmt = SND_SOC_DAIFMT_CBS_CFS;
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- pr_debug("%s: dai_id: 0x%x\n", __func__, cpu_dai->id);
It is good for debug but not very useful here, so removing it would be good
- switch (cpu_dai->id) {
- case PRIMARY_MI2S_RX:
- case PRIMARY_MI2S_TX:
mutex_lock(&pri_mi2s_res_lock);
if (atomic_inc_return(&pri_mi2s_clk_count) == 1) {
snd_soc_dai_set_sysclk(cpu_dai,
Q6AFE_LPASS_CLK_ID_MCLK_1,
DEFAULT_MCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
snd_soc_dai_set_sysclk(cpu_dai,
Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT,
DEFAULT_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
}
mutex_unlock(&pri_mi2s_res_lock);
why do we need locking here? Can you please explain that.
snd_soc_dai_set_fmt(cpu_dai, fmt);
break;
empty line after break helps in readability
+static int sdm845_sbc_parse_of(struct snd_soc_card *card) +{
- struct device *dev = card->dev;
- struct snd_soc_dai_link *link;
- struct device_node *np, *codec, *platform, *cpu, *node;
- int ret, num_links;
- struct sdm845_snd_data *data;
- ret = snd_soc_of_parse_card_name(card, "qcom,model");
- if (ret) {
dev_err(dev, "Error parsing card name: %d\n", ret);
return ret;
- }
- node = dev->of_node;
- /* DAPM routes */
- if (of_property_read_bool(node, "qcom,audio-routing")) {
ret = snd_soc_of_parse_audio_routing(card,
"qcom,audio-routing");
if (ret)
return ret;
- }
so if we dont find audio-routing, then? we seems to continue..
Thanks Vinod for reviewing.
On 6/19/2018 10:35 AM, Vinod wrote:
On 18-06-18, 16:46, Rohit kumar wrote:
+struct sdm845_snd_data {
- struct snd_soc_card *card;
- struct regulator *vdd_supply;
- struct snd_soc_dai_link dai_link[];
+};
+static struct mutex pri_mi2s_res_lock; +static struct mutex quat_tdm_res_lock;
any reason why the locks can't be part of sdm845_snd_data? Also why do we need two locks ?
No specific reason, I will move it to sdm845_snd_data. These locks are used to protect enable/disable of bit clocks. We have Primary MI2S RX/TX and Quaternary TDM RX/TX interfaces. For primary mi2s rx/tx, we have single clock which is synchronized with pri_mi2s_res_lock. For Quat TDM RX/TX, we are using quat_tdm_res_lock. We need two locks as we are protecting two different resources.
+static atomic_t pri_mi2s_clk_count; +static atomic_t quat_tdm_clk_count;
Any specific reason for using atomic variables?
Nothing as such. As we are using mutex to synchronize, we can make it non- atomic. Will do it in next-spin.
+static unsigned int tdm_slot_offset[8] = {0, 4, 8, 12, 16, 20, 24, 28};
+static int sdm845_tdm_snd_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- int ret = 0;
- int channels, slot_width;
- channels = params_channels(params);
- if (channels < 1 || channels > 8) {
I though ch = 0 would be caught by framework and IIRC ASoC doesn't support more than 8 channels
OK. Will check and remove.
pr_err("%s: invalid param channels %d\n",
__func__, channels);
return -EINVAL;
- }
- switch (params_format(params)) {
- case SNDRV_PCM_FORMAT_S32_LE:
- case SNDRV_PCM_FORMAT_S24_LE:
- case SNDRV_PCM_FORMAT_S16_LE:
slot_width = 32;
break;
- default:
pr_err("%s: invalid param format 0x%x\n",
__func__, params_format(params));
why not use dev_err, bonus you get device name printer with the logs :)
Sure. Will change it.
+static int sdm845_snd_startup(struct snd_pcm_substream *substream) +{
- unsigned int fmt = SND_SOC_DAIFMT_CBS_CFS;
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- pr_debug("%s: dai_id: 0x%x\n", __func__, cpu_dai->id);
It is good for debug but not very useful here, so removing it would be good
OK
- switch (cpu_dai->id) {
- case PRIMARY_MI2S_RX:
- case PRIMARY_MI2S_TX:
mutex_lock(&pri_mi2s_res_lock);
if (atomic_inc_return(&pri_mi2s_clk_count) == 1) {
snd_soc_dai_set_sysclk(cpu_dai,
Q6AFE_LPASS_CLK_ID_MCLK_1,
DEFAULT_MCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
snd_soc_dai_set_sysclk(cpu_dai,
Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT,
DEFAULT_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
}
mutex_unlock(&pri_mi2s_res_lock);
why do we need locking here? Can you please explain that.
So, we can have two usecases: one with primary mi2s rx and other with primary mi2s tx. Lock is required to increment pri_mi2s_clk_count and enable clock so that disable of one usecase does not disable the clock.
snd_soc_dai_set_fmt(cpu_dai, fmt);
break;
empty line after break helps in readability
Sure. Will add that change.
+static int sdm845_sbc_parse_of(struct snd_soc_card *card) +{
- struct device *dev = card->dev;
- struct snd_soc_dai_link *link;
- struct device_node *np, *codec, *platform, *cpu, *node;
- int ret, num_links;
- struct sdm845_snd_data *data;
- ret = snd_soc_of_parse_card_name(card, "qcom,model");
- if (ret) {
dev_err(dev, "Error parsing card name: %d\n", ret);
return ret;
- }
- node = dev->of_node;
- /* DAPM routes */
- if (of_property_read_bool(node, "qcom,audio-routing")) {
ret = snd_soc_of_parse_audio_routing(card,
"qcom,audio-routing");
if (ret)
return ret;
- }
so if we dont find audio-routing, then? we seems to continue..
Right. Its not mandatory to have qcom,audio-routing in device tree.
Regards, Rohit
Hi Rohit,
On 19-06-18, 19:20, Rohit Kumar wrote:
On 6/19/2018 10:35 AM, Vinod wrote:
On 18-06-18, 16:46, Rohit kumar wrote:
+struct sdm845_snd_data {
- struct snd_soc_card *card;
- struct regulator *vdd_supply;
- struct snd_soc_dai_link dai_link[];
+};
+static struct mutex pri_mi2s_res_lock; +static struct mutex quat_tdm_res_lock;
any reason why the locks can't be part of sdm845_snd_data? Also why do we need two locks ?
No specific reason, I will move it to sdm845_snd_data. These locks are used to protect enable/disable of bit clocks. We have Primary MI2S RX/TX and Quaternary TDM RX/TX interfaces. For primary mi2s rx/tx, we have single clock which is synchronized with pri_mi2s_res_lock. For Quat TDM RX/TX, we are using quat_tdm_res_lock. We need two locks as we are protecting two different resources.
I think bigger question is why do you need any locks? What is the race scenario you envision which needs protection
Hello Vinod,
On 6/19/2018 9:52 PM, Vinod wrote:
Hi Rohit,
On 19-06-18, 19:20, Rohit Kumar wrote:
On 6/19/2018 10:35 AM, Vinod wrote:
On 18-06-18, 16:46, Rohit kumar wrote:
+struct sdm845_snd_data {
- struct snd_soc_card *card;
- struct regulator *vdd_supply;
- struct snd_soc_dai_link dai_link[];
+};
+static struct mutex pri_mi2s_res_lock; +static struct mutex quat_tdm_res_lock;
any reason why the locks can't be part of sdm845_snd_data? Also why do we need two locks ?
No specific reason, I will move it to sdm845_snd_data. These locks are used to protect enable/disable of bit clocks. We have Primary MI2S RX/TX and Quaternary TDM RX/TX interfaces. For primary mi2s rx/tx, we have single clock which is synchronized with pri_mi2s_res_lock. For Quat TDM RX/TX, we are using quat_tdm_res_lock. We need two locks as we are protecting two different resources.
I think bigger question is why do you need any locks? What is the race scenario you envision which needs protection
Below is one of the race condition:
Thread1 | Thread2 ---------------------------------------------------------- startup() | count++; | startup() read count (count = 1) | enable_clock() | count++; //count = 2 shutdown() | count--;// count = 1 | | read count (count = 1) | enable_clock()
Here clock will be enabled twice but disable will be called only once when count = 0.
This will make the clock always enabled. So, I think we should keep either mutex lock or atomic variable to synchronize this.
Regards, Rohit
Hi Rohit,
On 20-06-18, 13:07, Rohit Kumar wrote:
On 19-06-18, 19:20, Rohit Kumar wrote:
On 6/19/2018 10:35 AM, Vinod wrote:
On 18-06-18, 16:46, Rohit kumar wrote:
+struct sdm845_snd_data {
- struct snd_soc_card *card;
- struct regulator *vdd_supply;
- struct snd_soc_dai_link dai_link[];
+};
+static struct mutex pri_mi2s_res_lock; +static struct mutex quat_tdm_res_lock;
any reason why the locks can't be part of sdm845_snd_data? Also why do we need two locks ?
No specific reason, I will move it to sdm845_snd_data. These locks are used to protect enable/disable of bit clocks. We have Primary MI2S RX/TX and Quaternary TDM RX/TX interfaces. For primary mi2s rx/tx, we have single clock which is synchronized with pri_mi2s_res_lock. For Quat TDM RX/TX, we are using quat_tdm_res_lock. We need two locks as we are protecting two different resources.
I think bigger question is why do you need any locks? What is the race scenario you envision which needs protection
Below is one of the race condition:
Thread1 | Thread2
startup() | count++; | startup() read count (count = 1) | enable_clock() | count++; //count = 2 shutdown() | count--;// count = 1 | | read count (count = 1) | enable_clock()
Here clock will be enabled twice but disable will be called only once when count = 0.
This will make the clock always enabled. So, I think we should keep either mutex lock or atomic variable to synchronize this.
we are using DPCM here right?
On 20/06/18 10:31, Vinod wrote:
Here clock will be enabled twice but disable will be called only once when count = 0.
This will make the clock always enabled. So, I think we should keep either mutex lock or atomic variable to synchronize this.
we are using DPCM here right?
We should probably model this clk as DAPM widget so we do not need to handle this in machine code.
--srini
On 20-06-18, 10:53, Srinivas Kandagatla wrote:
On 20/06/18 10:31, Vinod wrote:
Here clock will be enabled twice but disable will be called only once when count = 0.
This will make the clock always enabled. So, I think we should keep either mutex lock or atomic variable to synchronize this.
we are using DPCM here right?
We should probably model this clk as DAPM widget so we do not need to handle this in machine code.
Sure that would be even better, but my point was that we need not worry about .startup racing in case of DPCM as it holds a card mutex before it calls PCM ops.. so it is already serialized..
Thanks Rohit for the patch!
On 18/06/18 12:16, Rohit kumar wrote:
This patch adds sdm845 audio machine driver support.
Signed-off-by: Rohit kumar rohitkr@codeaurora.org
.../devicetree/bindings/sound/qcom,sdm845.txt | 87 ++++ sound/soc/qcom/Kconfig | 9 + sound/soc/qcom/Makefile | 2 + sound/soc/qcom/sdm845.c | 539 +++++++++++++++++++++ 4 files changed, 637 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/qcom,sdm845.txt
Split the bindings into a separate patch!
create mode 100644 sound/soc/qcom/sdm845.c
diff --git a/Documentation/devicetree/bindings/sound/qcom,sdm845.txt b/Documentation/devicetree/bindings/sound/qcom,sdm845.txt new file mode 100644 index 0000000..fc0e98c --- /dev/null +++ b/Documentation/devicetree/bindings/sound/qcom,sdm845.txt @@ -0,0 +1,87 @@ +* Qualcomm Technologies Inc. SDM845 ASoC sound card driver
+This binding describes the SDM845 sound card, which uses qdsp for audio.
+- compatible:
- Usage: required
- Value type: <stringlist>
- Definition: must be "qcom,sdm845-sndcard"
+- qcom,audio-routing:
- Usage: Optional
- Value type: <stringlist>
- Definition: A list of the connections between audio components.
Each entry is a pair of strings, the first being the
connection's sink, the second being the connection's
source. Valid names could be power supplies, MicBias
of codec and the jacks on the board.
+- cdc-vdd-supply:
- Usage: Optional
- Value type: <phandle>
- Definition: phandle of regulator supply required for codec vdd.
+= dailinks +Each subnode of sndcard represents either a dailink, and subnodes of each +dailinks would be cpu/codec/platform dais.
+- link-name:
- Usage: required
- Value type: <string>
- Definition: User friendly name for dai link
+= CPU, PLATFORM, CODEC dais subnodes +- cpu:
- Usage: required
- Value type: <subnode>
- Definition: cpu dai sub-node
+- codec:
- Usage: required
- Value type: <subnode>
- Definition: codec dai sub-node
+- platform:
- Usage: opional
Optional
- Value type: <subnode>
- Definition: platform dai sub-node
+- sound-dai:
- Usage: required
- Value type: <phandle>
- Definition: dai phandle/s and port of CPU/CODEC/PLATFORM node.
+Example:
+audio {
- compatible = "qcom,sdm845-sndcard";
- qcom,model = "sdm845-snd-card";
- pinctrl-names = "pri_active", "pri_sleep";
- pinctrl-0 = <&pri_mi2s_active &pri_mi2s_ws_active>;
- pinctrl-1 = <&pri_mi2s_sleep &pri_mi2s_ws_sleep>;
- qcom,audio-routing = "PRI_MI2S_RX Audio Mixer", "Pri-mi2s-gpio";
- cdc-vdd-supply = <&pm8998_l14>;
- fe@1 {
Lets not use fe or be reference here, its just a link.
link-name = "MultiMedia1";
cpu {
sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>;
};
platform {
sound-dai = <&q6asmdai>;
};
asmdai has sound-cell specifier as 1, so this will dtc will throw warning for this.
have a look at how 8996 is done.
- };
- be@1 {
link-name = "PRI MI2S Playback";
cpu {
sound-dai = <&q6afedai PRIMARY_MI2S_RX>;
};
platform {
sound-dai = <&q6routing>;
};
- };
+}; diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig index 87838fa..09de50d 100644 --- a/sound/soc/qcom/Kconfig +++ b/sound/soc/qcom/Kconfig @@ -90,3 +90,12 @@ config SND_SOC_MSM8996 Support for Qualcomm Technologies LPASS audio block in APQ8096 SoC-based systems. Say Y if you want to use audio device on this SoCs
+config SND_SOC_SDM845
- tristate "SoC Machine driver for SDM845 boards"
- depends on QCOM_APR
- select SND_SOC_QDSP6
- help
To add support for audio on Qualcomm Technologies Inc.
SDM845 SoC-based systems.
Say Y if you want to use audio device on this SoCs
diff --git a/sound/soc/qcom/Makefile b/sound/soc/qcom/Makefile index 206945b..ac9609e 100644 --- a/sound/soc/qcom/Makefile +++ b/sound/soc/qcom/Makefile @@ -14,10 +14,12 @@ obj-$(CONFIG_SND_SOC_LPASS_APQ8016) += snd-soc-lpass-apq8016.o snd-soc-storm-objs := storm.o snd-soc-apq8016-sbc-objs := apq8016_sbc.o snd-soc-apq8096-objs := apq8096.o +snd-soc-sdm845-objs := sdm845.o
obj-$(CONFIG_SND_SOC_STORM) += snd-soc-storm.o obj-$(CONFIG_SND_SOC_APQ8016_SBC) += snd-soc-apq8016-sbc.o obj-$(CONFIG_SND_SOC_MSM8996) += snd-soc-apq8096.o ++obj-$(CONFIG_SND_SOC_SDM845) += snd-soc-sdm845.o
?? looks like typo here.
#DSP lib obj-$(CONFIG_SND_SOC_QDSP6) += qdsp6/ diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c new file mode 100644 index 0000000..70d2611 --- /dev/null +++ b/sound/soc/qcom/sdm845.c @@ -0,0 +1,539 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (c) 2018, The Linux Foundation. All rights reserved.
- */
+#include <linux/module.h> +#include <linux/component.h> +#include <linux/platform_device.h> +#include <linux/regulator/consumer.h> +#include <linux/atomic.h>
+#include <linux/of_device.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <linux/soc/qcom/apr.h> +#include "qdsp6/q6afe.h"
+#define DEFAULT_SAMPLE_RATE_48K 48000 +#define DEFAULT_MCLK_RATE 24576000 +#define DEFAULT_BCLK_RATE 1536000
+struct sdm845_snd_data {
- struct snd_soc_card *card;
- struct regulator *vdd_supply;
- struct snd_soc_dai_link dai_link[];
+};
+static struct mutex pri_mi2s_res_lock; +static struct mutex quat_tdm_res_lock; +static atomic_t pri_mi2s_clk_count; +static atomic_t quat_tdm_clk_count;
+static unsigned int tdm_slot_offset[8] = {0, 4, 8, 12, 16, 20, 24, 28}; +static int sdm845_snd_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- int ret = 0;
- switch (cpu_dai->id) {
- case QUATERNARY_TDM_RX_0:
- case QUATERNARY_TDM_TX_0:
ret = sdm845_tdm_snd_hw_params(substream, params);
break;
- default:
pr_err("%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
break;
- }
- return ret;
+}
+static int sdm845_snd_startup(struct snd_pcm_substream *substream) +{
- unsigned int fmt = SND_SOC_DAIFMT_CBS_CFS;
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
- pr_debug("%s: dai_id: 0x%x\n", __func__, cpu_dai->id);
- switch (cpu_dai->id) {
- case PRIMARY_MI2S_RX:
- case PRIMARY_MI2S_TX:
mutex_lock(&pri_mi2s_res_lock);
Mutex and atomic variables looks redundant here. Can you explain why do you need both?
if (atomic_inc_return(&pri_mi2s_clk_count) == 1) { > + snd_soc_dai_set_sysclk(cpu_dai,
Q6AFE_LPASS_CLK_ID_MCLK_1,
DEFAULT_MCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
snd_soc_dai_set_sysclk(cpu_dai,
Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT,
DEFAULT_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
}
mutex_unlock(&pri_mi2s_res_lock);
snd_soc_dai_set_fmt(cpu_dai, fmt);
break;
- case QUATERNARY_TDM_RX_0:
- case QUATERNARY_TDM_TX_0:
mutex_lock(&quat_tdm_res_lock);
if (atomic_inc_return(&quat_tdm_clk_count) == 1) {
snd_soc_dai_set_sysclk(cpu_dai,
Q6AFE_LPASS_CLK_ID_QUAD_TDM_IBIT,
DEFAULT_BCLK_RATE, SNDRV_PCM_STREAM_PLAYBACK);
}
mutex_unlock(&quat_tdm_res_lock);
break;
- default:
pr_err("%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
break;
- }
- return 0;
+}
...
+static int sdm845_sbc_parse_of(struct snd_soc_card *card) +{
- struct device *dev = card->dev;
- struct snd_soc_dai_link *link;
- struct device_node *np, *codec, *platform, *cpu, *node;
- int ret, num_links;
- struct sdm845_snd_data *data;
- ret = snd_soc_of_parse_card_name(card, "qcom,model");
- if (ret) {
dev_err(dev, "Error parsing card name: %d\n", ret);
return ret;
- }
- node = dev->of_node;
- /* DAPM routes */
- if (of_property_read_bool(node, "qcom,audio-routing")) {
ret = snd_soc_of_parse_audio_routing(card,
"qcom,audio-routing");
if (ret)
return ret;
- }
- /* Populate links */
- num_links = of_get_child_count(node);
- dev_info(dev, "Found %d child audio dai links..\n", num_links);
Looks unnessary!
- /* Allocate the private data and the DAI link array */
- data = kzalloc(sizeof(*data) + sizeof(*link) * num_links,
GFP_KERNEL);
- if (!data)
return -ENOMEM;
- card->dai_link = &data->dai_link[0];
- card->num_links = num_links;
- card->dapm_widgets = sdm845_widgets;
- card->num_dapm_widgets = ARRAY_SIZE(sdm845_widgets);
- link = data->dai_link;
- data->card = card;
- for_each_child_of_node(node, np) {
cpu = of_get_child_by_name(np, "cpu");
platform = of_get_child_by_name(np, "platform");
codec = of_get_child_by_name(np, "codec");
if (!cpu) {
dev_err(dev, "Can't find cpu DT node\n");
ret = -EINVAL;
goto fail;
}
link->cpu_of_node = of_parse_phandle(cpu, "sound-dai", 0);
if (!link->cpu_of_node) {
dev_err(card->dev, "error getting cpu phandle\n");
ret = -EINVAL;
goto fail;
}
link->platform_of_node = of_parse_phandle(platform,
"sound-dai", 0);
if (!link->platform_of_node) {
dev_err(card->dev, "error getting platform phandle\n");
ret = -EINVAL;
goto fail;
}
ret = snd_soc_of_get_dai_name(cpu, &link->cpu_dai_name);
if (ret) {
dev_err(card->dev, "error getting cpu dai name\n");
goto fail;
}
if (codec) {
ret = snd_soc_of_get_dai_link_codecs(dev, codec, link);
if (ret < 0) {
dev_err(card->dev, "error getting codec dai name\n");
goto fail;
}
link->no_pcm = 1;
link->ignore_suspend = 1;
link->ignore_pmdown_time = 1;
link->ops = &sdm845_be_ops;
link->be_hw_params_fixup = sdm845_be_hw_params_fixup;
} else {
link->dynamic = 1;
link->codec_dai_name = "snd-soc-dummy-dai";
link->codec_name = "snd-soc-dummy";
}
You could optimize this code, have a look at apq8096.c which does exactly same thing.
ret = of_property_read_string(np, "link-name", &link->name);
if (ret) {
dev_err(card->dev,
"error getting codec dai_link name\n");
goto fail;
}
link->dpcm_playback = 1;
link->dpcm_capture = 1;
link->stream_name = link->name;
link++;
- }
- dev_set_drvdata(dev, card);
- snd_soc_card_set_drvdata(card, data);
- return ret;
+fail:
- kfree(data);
- return ret;
+}
...
+static void sdm845_unbind(struct device *dev) +{
- struct snd_soc_card *card = dev_get_drvdata(dev);
- struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card);
- mutex_destroy(&pri_mi2s_res_lock);
- mutex_destroy(&quat_tdm_res_lock);
- if (data->vdd_supply)
regulator_put(data->vdd_supply);
- component_unbind_all(dev, card);
- snd_soc_unregister_card(card);
- kfree(data);
- kfree(card);
+}
+static const struct component_master_ops sdm845_ops = {
- .bind = sdm845_bind,
- .unbind = sdm845_unbind,
+};
+static int sdm845_runtime_resume(struct device *dev) +{
- struct snd_soc_card *card = dev_get_drvdata(dev);
- struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card);
- if (!data->vdd_supply) {
dev_dbg(dev, "no supplies defined\n");
return 0;
- }
- if (regulator_enable(data->vdd_supply))
dev_err(dev, "Enable regulator supply failed\n");
- return 0;
+}
+static struct platform_driver sdm845_snd_driver = {
- .probe = sdm845_snd_platform_probe,
- .remove = sdm845_snd_platform_remove,
- .driver = {
.name = "msm-snd-sdm845",
.pm = &sdm845_pm_ops,
.owner = THIS_MODULE,
not necessary!
.of_match_table = of_match_ptr(sdm845_snd_device_id),
- },
+}; +module_platform_driver(sdm845_snd_driver);
+MODULE_DESCRIPTION("sdm845 ASoC Machine Driver"); +MODULE_LICENSE("GPL v2");
Thanks Srinivas for reviewing.
On 6/19/2018 2:16 PM, Srinivas Kandagatla wrote:
Thanks Rohit for the patch!
On 18/06/18 12:16, Rohit kumar wrote:
This patch adds sdm845 audio machine driver support.
Signed-off-by: Rohit kumar rohitkr@codeaurora.org
.../devicetree/bindings/sound/qcom,sdm845.txt | 87 ++++ sound/soc/qcom/Kconfig | 9 + sound/soc/qcom/Makefile | 2 + sound/soc/qcom/sdm845.c | 539 +++++++++++++++++++++ 4 files changed, 637 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/qcom,sdm845.txt
Split the bindings into a separate patch!
Sure, will do it in next spin.
create mode 100644 sound/soc/qcom/sdm845.c
diff --git a/Documentation/devicetree/bindings/sound/qcom,sdm845.txt b/Documentation/devicetree/bindings/sound/qcom,sdm845.txt new file mode 100644 index 0000000..fc0e98c --- /dev/null +++ b/Documentation/devicetree/bindings/sound/qcom,sdm845.txt @@ -0,0 +1,87 @@ +* Qualcomm Technologies Inc. SDM845 ASoC sound card driver
+This binding describes the SDM845 sound card, which uses qdsp for audio.
[..]
+- codec: + Usage: required + Value type: <subnode> + Definition: codec dai sub-node
+- platform: + Usage: opional
Optional
okay
+ Value type: <subnode> + Definition: platform dai sub-node
+- sound-dai: + Usage: required + Value type: <phandle> + Definition: dai phandle/s and port of CPU/CODEC/PLATFORM node.
+Example:
+audio { + compatible = "qcom,sdm845-sndcard"; + qcom,model = "sdm845-snd-card"; + pinctrl-names = "pri_active", "pri_sleep"; + pinctrl-0 = <&pri_mi2s_active &pri_mi2s_ws_active>; + pinctrl-1 = <&pri_mi2s_sleep &pri_mi2s_ws_sleep>;
+ qcom,audio-routing = "PRI_MI2S_RX Audio Mixer", "Pri-mi2s-gpio";
+ cdc-vdd-supply = <&pm8998_l14>;
+ fe@1 {
Lets not use fe or be reference here, its just a link.
okay
+ link-name = "MultiMedia1"; + cpu { + sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>; + }; + platform { + sound-dai = <&q6asmdai>; + };
asmdai has sound-cell specifier as 1, so this will dtc will throw warning for this.
have a look at how 8996 is done.
ok, sure
+ };
+ be@1 {
[..]
diff --git a/sound/soc/qcom/Makefile b/sound/soc/qcom/Makefile index 206945b..ac9609e 100644 --- a/sound/soc/qcom/Makefile +++ b/sound/soc/qcom/Makefile @@ -14,10 +14,12 @@ obj-$(CONFIG_SND_SOC_LPASS_APQ8016) += snd-soc-lpass-apq8016.o snd-soc-storm-objs := storm.o snd-soc-apq8016-sbc-objs := apq8016_sbc.o snd-soc-apq8096-objs := apq8096.o +snd-soc-sdm845-objs := sdm845.o obj-$(CONFIG_SND_SOC_STORM) += snd-soc-storm.o obj-$(CONFIG_SND_SOC_APQ8016_SBC) += snd-soc-apq8016-sbc.o obj-$(CONFIG_SND_SOC_MSM8996) += snd-soc-apq8096.o ++obj-$(CONFIG_SND_SOC_SDM845) += snd-soc-sdm845.o
?? looks like typo here.
Right. Will update.
#DSP lib obj-$(CONFIG_SND_SOC_QDSP6) += qdsp6/ diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c new file mode 100644 index 0000000..70d2611 --- /dev/null +++ b/sound/soc/qcom/sdm845.c @@ -0,0 +1,539 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (c) 2018, The Linux Foundation. All rights reserved.
- */
+#include <linux/module.h> +#include <linux/component.h>
[..]
+}
+static int sdm845_snd_startup(struct snd_pcm_substream *substream) +{ + unsigned int fmt = SND_SOC_DAIFMT_CBS_CFS; + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ pr_debug("%s: dai_id: 0x%x\n", __func__, cpu_dai->id); + switch (cpu_dai->id) { + case PRIMARY_MI2S_RX: + case PRIMARY_MI2S_TX: + mutex_lock(&pri_mi2s_res_lock);
Mutex and atomic variables looks redundant here. Can you explain why do you need both?
Right. Only mutex is required here. I will make count as non-atomic.
...
+static int sdm845_sbc_parse_of(struct snd_soc_card *card) +{ + struct device *dev = card->dev; + struct snd_soc_dai_link *link; + struct device_node *np, *codec, *platform, *cpu, *node; + int ret, num_links; + struct sdm845_snd_data *data;
+ ret = snd_soc_of_parse_card_name(card, "qcom,model"); + if (ret) { + dev_err(dev, "Error parsing card name: %d\n", ret); + return ret; + }
+ node = dev->of_node;
+ /* DAPM routes */ + if (of_property_read_bool(node, "qcom,audio-routing")) { + ret = snd_soc_of_parse_audio_routing(card, + "qcom,audio-routing"); + if (ret) + return ret; + }
+ /* Populate links */ + num_links = of_get_child_count(node);
+ dev_info(dev, "Found %d child audio dai links..\n", num_links);
Looks unnessary!
Ok . Will remove it in next patchset.
+ link->platform_of_node = of_parse_phandle(platform, + "sound-dai", 0); + if (!link->platform_of_node) { + dev_err(card->dev, "error getting platform phandle\n"); + ret = -EINVAL; + goto fail; + }
+ ret = snd_soc_of_get_dai_name(cpu, &link->cpu_dai_name); + if (ret) { + dev_err(card->dev, "error getting cpu dai name\n"); + goto fail; + }
+ if (codec) { + ret = snd_soc_of_get_dai_link_codecs(dev, codec, link); + if (ret < 0) { + dev_err(card->dev, "error getting codec dai name\n"); + goto fail; + } + link->no_pcm = 1; + link->ignore_suspend = 1; + link->ignore_pmdown_time = 1; + link->ops = &sdm845_be_ops; + link->be_hw_params_fixup = sdm845_be_hw_params_fixup; + } else { + link->dynamic = 1; + link->codec_dai_name = "snd-soc-dummy-dai"; + link->codec_name = "snd-soc-dummy"; + }
You could optimize this code, have a look at apq8096.c which does exactly same thing.
Okay. will check and update.
...
+static void sdm845_unbind(struct device *dev) +{ + struct snd_soc_card *card = dev_get_drvdata(dev); + struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card);
+ mutex_destroy(&pri_mi2s_res_lock); + mutex_destroy(&quat_tdm_res_lock); + if (data->vdd_supply) + regulator_put(data->vdd_supply); + component_unbind_all(dev, card); + snd_soc_unregister_card(card); + kfree(data); + kfree(card); +}
+static const struct component_master_ops sdm845_ops = { + .bind = sdm845_bind, + .unbind = sdm845_unbind, +};
+static int sdm845_runtime_resume(struct device *dev) +{ + struct snd_soc_card *card = dev_get_drvdata(dev); + struct sdm845_snd_data *data = snd_soc_card_get_drvdata(card);
+ if (!data->vdd_supply) { + dev_dbg(dev, "no supplies defined\n"); + return 0; + }
+ if (regulator_enable(data->vdd_supply)) + dev_err(dev, "Enable regulator supply failed\n");
+ return 0; +}
+static struct platform_driver sdm845_snd_driver = { + .probe = sdm845_snd_platform_probe, + .remove = sdm845_snd_platform_remove, + .driver = { + .name = "msm-snd-sdm845", + .pm = &sdm845_pm_ops, + .owner = THIS_MODULE,
not necessary!
Okay. Will remove it.
+ .of_match_table = of_match_ptr(sdm845_snd_device_id), + }, +}; +module_platform_driver(sdm845_snd_driver);
+MODULE_DESCRIPTION("sdm845 ASoC Machine Driver"); +MODULE_LICENSE("GPL v2");
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Regards, Rohit
participants (3)
-
Rohit kumar
-
Srinivas Kandagatla
-
Vinod