* [PATCH 0/2] Machine driver to support LPASS SC7280 sound card registration
@ 2021-09-08 18:00 Srinivasa Rao Mandadapu
2021-09-08 18:00 ` [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings Srinivasa Rao Mandadapu
2021-09-08 18:00 ` [PATCH 2/2] ASoC: qcom: SC7280: Add machine driver Srinivasa Rao Mandadapu
0 siblings, 2 replies; 14+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-09-08 18:00 UTC (permalink / raw)
To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
judyhsiao
Cc: Srinivasa Rao Mandadapu
This patch set is to add support for SC7280 sound card registration and
to add dt-bindings documentation file.
These patches depends on the dt-bindings header patch
-- https://patchwork.kernel.org/project/alsa-devel/list/?series=543829
Srinivasa Rao Mandadapu (2):
ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings
ASoC: qcom: SC7280: Add machine driver
.../bindings/sound/google,sc7280-herobrine.yaml | 169 ++++++++++
sound/soc/qcom/Kconfig | 12 +
sound/soc/qcom/Makefile | 2 +
sound/soc/qcom/sc7280.c | 347 +++++++++++++++++++++
4 files changed, 530 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
create mode 100644 sound/soc/qcom/sc7280.c
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings
2021-09-08 18:00 [PATCH 0/2] Machine driver to support LPASS SC7280 sound card registration Srinivasa Rao Mandadapu
@ 2021-09-08 18:00 ` Srinivasa Rao Mandadapu
2021-09-09 12:32 ` Rob Herring
2021-09-08 18:00 ` [PATCH 2/2] ASoC: qcom: SC7280: Add machine driver Srinivasa Rao Mandadapu
1 sibling, 1 reply; 14+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-09-08 18:00 UTC (permalink / raw)
To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
judyhsiao
Cc: Srinivasa Rao Mandadapu
Add devicetree bindings documentation file for sc7280 sound card
registration.
Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
---
.../bindings/sound/google,sc7280-herobrine.yaml | 169 +++++++++++++++++++++
1 file changed, 169 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
diff --git a/Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml b/Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
new file mode 100644
index 0000000..e0d705f
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
@@ -0,0 +1,169 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/google,sc7280-herobrine.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Google SC7280-Herobrine ASoC sound card driver
+
+maintainers:
+ - Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
+ - Judy Hsiao <judyhsiao@chromium.org>
+
+description:
+ This binding describes the SC7280 sound card which uses LPASS for audio.
+
+properties:
+ compatible:
+ enum:
+ - google,sc7280-herobrine
+
+ audio-routing:
+ $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+ description:
+ 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.
+
+ model:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: User specified audio sound card name
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^dai-link@[0-9a-f]$":
+ description:
+ Each subnode represents a dai link. Subnodes of each dai links would be
+ cpu/codec dais.
+
+ type: object
+
+ properties:
+ link-name:
+ description: Indicates dai-link name and PCM stream name.
+ $ref: /schemas/types.yaml#/definitions/string
+ maxItems: 1
+
+ reg:
+ maxItems: 1
+ description: dai link address.
+
+ cpu:
+ description: Holds subnode which indicates cpu dai.
+ type: object
+ properties:
+ sound-dai: true
+
+ codec:
+ description: Holds subnode which indicates codec dai.
+ type: object
+ properties:
+ sound-dai: true
+
+ required:
+ - link-name
+ - cpu
+ - codec
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - model
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
+
+examples:
+
+ - |
+ #include <dt-bindings/sound/qcom,lpass.h>
+ sound {
+ compatible = "google,sc7280-herobrine";
+ model = "sc7280-wcd938x-max98360a-4dmic";
+
+ audio-routing =
+ "IN1_HPHL", "HPHL_OUT",
+ "IN2_HPHR", "HPHR_OUT",
+ "AMIC1", "MIC BIAS1",
+ "AMIC2", "MIC BIAS2",
+ "VA DMIC0", "MIC BIAS3",
+ "VA DMIC1", "MIC BIAS3",
+ "VA DMIC2", "MIC BIAS4",
+ "VA DMIC3", "MIC BIAS4",
+ "TX SWR_ADC0", "ADC1_OUTPUT",
+ "TX SWR_ADC1", "ADC2_OUTPUT",
+ "TX SWR_ADC2", "ADC3_OUTPUT",
+ "TX SWR_DMIC0", "DMIC1_OUTPUT",
+ "TX SWR_DMIC1", "DMIC2_OUTPUT",
+ "TX SWR_DMIC2", "DMIC3_OUTPUT",
+ "TX SWR_DMIC3", "DMIC4_OUTPUT";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dai-link@0 {
+ link-name = "WCD Playback";
+ reg = <LPASS_CDC_DMA_RX0>;
+ cpu {
+ sound-dai = <&lpass_cpu LPASS_CDC_DMA_RX0>;
+ };
+
+ codec {
+ sound-dai = <&wcd938x 0>, <&swr0 0>, <&rxmacro 0>;
+ };
+ };
+ dai-link@1 {
+ link-name = "WCD Capture";
+ reg = <LPASS_CDC_DMA_TX3>;
+ cpu {
+ sound-dai = <&lpass_cpu LPASS_CDC_DMA_TX3>;
+ };
+
+ codec {
+ sound-dai = <&wcd938x 1>, <&swr1 0>, <&txmacro 0>;
+ };
+ };
+
+ dai-link@2 {
+ link-name = "MI2S Playback";
+ reg = <MI2S_SECONDARY>;
+ cpu {
+ sound-dai = <&lpass_cpu MI2S_SECONDARY>;
+ };
+
+ codec {
+ sound-dai = <&max98360a>;
+ };
+ };
+
+ dai-link@3 {
+ link-name = "DMIC Capture";
+ reg = <LPASS_CDC_DMA_VA_TX0>;
+ cpu {
+ sound-dai = <&lpass_cpu LPASS_CDC_DMA_VA_TX0>;
+ };
+
+ codec {
+ sound-dai = <&vamacro 0>;
+ };
+ };
+
+ dai-link@5 {
+ link-name = "DP Playback";
+ reg = <LPASS_DP_RX>;
+ cpu {
+ sound-dai = <&lpass_cpu LPASS_DP_RX>;
+ };
+
+ codec {
+ sound-dai = <&mdss_dp>;
+ };
+ };
+ };
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] ASoC: qcom: SC7280: Add machine driver
2021-09-08 18:00 [PATCH 0/2] Machine driver to support LPASS SC7280 sound card registration Srinivasa Rao Mandadapu
2021-09-08 18:00 ` [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings Srinivasa Rao Mandadapu
@ 2021-09-08 18:00 ` Srinivasa Rao Mandadapu
2021-09-08 22:42 ` Stephen Boyd
1 sibling, 1 reply; 14+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-09-08 18:00 UTC (permalink / raw)
To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
judyhsiao
Cc: Srinivasa Rao Mandadapu
Add new machine driver to register sound card on sc7280 based targets and
do the required configuration for lpass cpu dai and external codecs
connected over MI2S and soundwire interfaces.
Add support for audio jack detection, soundwire init and MBHC.
Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
---
sound/soc/qcom/Kconfig | 12 ++
sound/soc/qcom/Makefile | 2 +
sound/soc/qcom/sc7280.c | 347 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 361 insertions(+)
create mode 100644 sound/soc/qcom/sc7280.c
diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index cc7c1de..d9ffcb7 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -152,4 +152,16 @@ config SND_SOC_SC7180
SC7180 SoC-based systems.
Say Y if you want to use audio device on this SoCs.
+config SND_SOC_SC7280
+ tristate "SoC Machine driver for SC7280 boards"
+ depends on I2C && SOUNDWIRE
+ select SND_SOC_QCOM_COMMON
+ select SND_SOC_MAX98357A
+ select SND_SOC_LPASS_RX_MACRO
+ select SND_SOC_LPASS_TX_MACRO
+ help
+ To add support for audio on Qualcomm Technologies Inc.
+ SC7280 SoC-based systems.
+ Say Y if you want to use audio device on this SoCs.
+
endif #SND_SOC_QCOM
diff --git a/sound/soc/qcom/Makefile b/sound/soc/qcom/Makefile
index 1600ae5..625aec6 100644
--- a/sound/soc/qcom/Makefile
+++ b/sound/soc/qcom/Makefile
@@ -19,6 +19,7 @@ snd-soc-storm-objs := storm.o
snd-soc-apq8016-sbc-objs := apq8016_sbc.o
snd-soc-apq8096-objs := apq8096.o
snd-soc-sc7180-objs := sc7180.o
+snd-soc-sc7280-objs := sc7280.o
snd-soc-sdm845-objs := sdm845.o
snd-soc-sm8250-objs := sm8250.o
snd-soc-qcom-common-objs := common.o
@@ -27,6 +28,7 @@ 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_SC7180) += snd-soc-sc7180.o
+obj-$(CONFIG_SND_SOC_SC7280) += snd-soc-sc7280.o
obj-$(CONFIG_SND_SOC_SDM845) += snd-soc-sdm845.o
obj-$(CONFIG_SND_SOC_SM8250) += snd-soc-sm8250.o
obj-$(CONFIG_SND_SOC_QCOM_COMMON) += snd-soc-qcom-common.o
diff --git a/sound/soc/qcom/sc7280.c b/sound/soc/qcom/sc7280.c
new file mode 100644
index 0000000..1ab29f6
--- /dev/null
+++ b/sound/soc/qcom/sc7280.c
@@ -0,0 +1,347 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
+//
+// sc7280.c -- ALSA SoC Machine driver for sc7280
+
+#include <dt-bindings/sound/sc7180-lpass.h>
+#include <dt-bindings/sound/qcom,q6afe.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <sound/core.h>
+#include <sound/jack.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <uapi/linux/input-event-codes.h>
+
+#include "../codecs/wcd938x.h"
+#include "common.h"
+#include "lpass.h"
+
+#define DRIVER_NAME "SC7280"
+#define LPASS_MAX_PORTS (LPASS_CDC_DMA_VA_TX8 + 1)
+
+
+struct sc7280_snd_data {
+ bool stream_prepared[LPASS_MAX_PORTS];
+ struct snd_soc_card card;
+ struct sdw_stream_runtime *sruntime[LPASS_MAX_PORTS];
+ struct snd_soc_jack hs_jack;
+ struct snd_soc_jack hdmi_jack;
+ bool jack_setup;
+};
+
+static void sc7280_jack_free(struct snd_jack *jack)
+{
+ struct snd_soc_component *component = jack->private_data;
+
+ snd_soc_component_set_jack(component, NULL, NULL);
+}
+
+static int sc7280_headset_init(struct snd_soc_pcm_runtime *rtd)
+{
+ struct snd_soc_card *card = rtd->card;
+ struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(card);
+ struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
+ struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+ struct snd_soc_component *component = codec_dai->component;
+ struct snd_jack *jack;
+ int rval, i;
+
+ if (!pdata->jack_setup) {
+ rval = snd_soc_card_jack_new(card, "Headset Jack",
+ SND_JACK_HEADSET | SND_JACK_LINEOUT |
+ SND_JACK_MECHANICAL |
+ SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+ SND_JACK_BTN_2 | SND_JACK_BTN_3 |
+ SND_JACK_BTN_4 | SND_JACK_BTN_5,
+ &pdata->hs_jack, NULL, 0);
+
+ if (rval < 0) {
+ dev_err(card->dev, "Unable to add Headset Jack\n");
+ return rval;
+ }
+
+ jack = pdata->hs_jack.jack;
+
+ snd_jack_set_key(jack, SND_JACK_BTN_0, KEY_MEDIA);
+ snd_jack_set_key(jack, SND_JACK_BTN_1, KEY_VOICECOMMAND);
+ snd_jack_set_key(jack, SND_JACK_BTN_2, KEY_VOLUMEUP);
+ snd_jack_set_key(jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN);
+
+ jack->private_data = component;
+ jack->private_free = sc7280_jack_free;
+ pdata->jack_setup = true;
+ }
+ switch (cpu_dai->id) {
+ case LPASS_CDC_DMA_RX0:
+ case LPASS_CDC_DMA_TX3:
+ for_each_rtd_codec_dais(rtd, i, codec_dai) {
+ rval = snd_soc_component_set_jack(component, &pdata->hs_jack, NULL);
+ if (rval != 0 && rval != -EOPNOTSUPP) {
+ dev_warn(card->dev, "Failed to set jack: %d\n", rval);
+ return rval;
+ }
+ }
+
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int sc7280_hdmi_init(struct snd_soc_pcm_runtime *rtd)
+{
+ struct snd_soc_card *card = rtd->card;
+ struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(card);
+ struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
+ struct snd_soc_component *component = codec_dai->component;
+ struct snd_jack *jack;
+ int rval;
+
+ rval = snd_soc_card_jack_new(
+ card, "HDMI Jack",
+ SND_JACK_LINEOUT,
+ &pdata->hdmi_jack, NULL, 0);
+
+ if (rval < 0) {
+ dev_err(card->dev, "Unable to add HDMI Jack\n");
+ return rval;
+ }
+
+ jack = pdata->hdmi_jack.jack;
+ jack->private_data = component;
+ jack->private_free = sc7280_jack_free;
+
+ return snd_soc_component_set_jack(component, &pdata->hdmi_jack, NULL);
+}
+
+static int sc7280_init(struct snd_soc_pcm_runtime *rtd)
+{
+ struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+
+ switch (cpu_dai->id) {
+ case LPASS_CDC_DMA_TX3:
+ return sc7280_headset_init(rtd);
+ case LPASS_CDC_DMA_RX0:
+ case LPASS_CDC_DMA_VA_TX0:
+ case MI2S_SECONDARY:
+ return 0;
+ case LPASS_DP_RX:
+ return sc7280_hdmi_init(rtd);
+ default:
+ dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int sc7280_snd_startup(struct snd_pcm_substream *substream)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+
+ switch (cpu_dai->id) {
+ case LPASS_CDC_DMA_RX0:
+ case LPASS_CDC_DMA_TX3:
+ case LPASS_CDC_DMA_VA_TX0:
+ break;
+ case MI2S_SECONDARY:
+ break;
+ case LPASS_DP_RX:
+ break;
+ default:
+ dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int sc7280_snd_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params)
+{
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *codec_dai;
+ struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+ struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card);
+ struct sdw_stream_runtime *sruntime;
+ int i;
+
+ snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
+ snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_RATE, 48000, 48000);
+
+ switch (cpu_dai->id) {
+ case LPASS_CDC_DMA_TX3:
+ case LPASS_CDC_DMA_RX0:
+ for_each_rtd_codec_dais(rtd, i, codec_dai) {
+ sruntime = snd_soc_dai_get_sdw_stream(codec_dai, substream->stream);
+ if (sruntime != ERR_PTR(-EOPNOTSUPP))
+ pdata->sruntime[cpu_dai->id] = sruntime;
+ }
+ break;
+ }
+
+ return 0;
+
+}
+
+static int sc7280_snd_swr_prepare(struct snd_pcm_substream *substream)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+ struct sc7280_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
+ struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
+ int ret;
+
+ if (!sruntime)
+ return 0;
+
+ if (data->stream_prepared[cpu_dai->id]) {
+ sdw_disable_stream(sruntime);
+ sdw_deprepare_stream(sruntime);
+ data->stream_prepared[cpu_dai->id] = false;
+ }
+
+ ret = sdw_prepare_stream(sruntime);
+ if (ret)
+ return ret;
+
+ ret = sdw_enable_stream(sruntime);
+ if (ret) {
+ sdw_deprepare_stream(sruntime);
+ return ret;
+ }
+ data->stream_prepared[cpu_dai->id] = true;
+
+ return ret;
+}
+
+
+static int sc7280_snd_prepare(struct snd_pcm_substream *substream)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+
+ switch (cpu_dai->id) {
+ case LPASS_CDC_DMA_RX0:
+ case LPASS_CDC_DMA_TX3:
+ return sc7280_snd_swr_prepare(substream);
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static int sc7280_snd_hw_free(struct snd_pcm_substream *substream)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct sc7280_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
+ struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+ struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
+
+ switch (cpu_dai->id) {
+ case LPASS_CDC_DMA_RX0:
+ case LPASS_CDC_DMA_TX3:
+ if (sruntime && data->stream_prepared[cpu_dai->id]) {
+ sdw_disable_stream(sruntime);
+ sdw_deprepare_stream(sruntime);
+ data->stream_prepared[cpu_dai->id] = false;
+ }
+ break;
+ default:
+ break;
+ }
+ return 0;
+}
+
+static void sc7280_snd_shutdown(struct snd_pcm_substream *substream)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+
+ switch (cpu_dai->id) {
+ case LPASS_CDC_DMA_RX0:
+ case LPASS_CDC_DMA_TX3:
+ case LPASS_CDC_DMA_VA_TX0:
+ break;
+ case MI2S_SECONDARY:
+ break;
+ case LPASS_DP_RX:
+ break;
+ default:
+ dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
+ break;
+ }
+}
+
+static const struct snd_soc_ops sc7280_ops = {
+ .startup = sc7280_snd_startup,
+ .shutdown = sc7280_snd_shutdown,
+ .hw_params = sc7280_snd_hw_params,
+ .hw_free = sc7280_snd_hw_free,
+ .prepare = sc7280_snd_prepare,
+};
+
+static const struct snd_soc_dapm_widget sc7280_snd_widgets[] = {
+ SND_SOC_DAPM_HP("Headphone Jack", NULL),
+ SND_SOC_DAPM_MIC("Headset Mic", NULL),
+};
+
+static int sc7280_snd_platform_probe(struct platform_device *pdev)
+{
+ struct snd_soc_card *card;
+ struct sc7280_snd_data *data;
+ struct device *dev = &pdev->dev;
+ struct snd_soc_dai_link *link;
+ int ret, i;
+
+ /* Allocate the private data */
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ card = &data->card;
+ data->jack_setup = false;
+ snd_soc_card_set_drvdata(card, data);
+
+ card->owner = THIS_MODULE;
+ card->driver_name = DRIVER_NAME;
+ card->dev = dev;
+
+ ret = qcom_snd_parse_of(card);
+ if (ret)
+ return ret;
+
+ for_each_card_prelinks(card, i, link) {
+ link->init = sc7280_init;
+ link->ops = &sc7280_ops;
+ }
+ ret = devm_snd_soc_register_card(dev, card);
+ return ret;
+}
+
+static const struct of_device_id sc7280_snd_device_id[] = {
+ {.compatible = "google,sc7280-herobrine"},
+ {},
+};
+MODULE_DEVICE_TABLE(of, sc7280_snd_device_id);
+
+static struct platform_driver sc7280_snd_driver = {
+ .probe = sc7280_snd_platform_probe,
+ .driver = {
+ .name = "msm-snd-sc7280",
+ .of_match_table = sc7280_snd_device_id,
+ .pm = &snd_soc_pm_ops,
+ },
+};
+module_platform_driver(sc7280_snd_driver);
+
+MODULE_DESCRIPTION("sc7280 ASoC Machine Driver");
+MODULE_LICENSE("GPL v2");
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] ASoC: qcom: SC7280: Add machine driver
2021-09-08 18:00 ` [PATCH 2/2] ASoC: qcom: SC7280: Add machine driver Srinivasa Rao Mandadapu
@ 2021-09-08 22:42 ` Stephen Boyd
0 siblings, 0 replies; 14+ messages in thread
From: Stephen Boyd @ 2021-09-08 22:42 UTC (permalink / raw)
To: Srinivasa Rao Mandadapu, agross, alsa-devel, bgoswami,
bjorn.andersson, broonie, devicetree, judyhsiao, lgirdwood,
linux-arm-msm, linux-kernel, perex, plai, robh+dt, rohitkr,
srinivas.kandagatla, tiwai
Quoting Srinivasa Rao Mandadapu (2021-09-08 11:00:57)
> diff --git a/sound/soc/qcom/sc7280.c b/sound/soc/qcom/sc7280.c
> new file mode 100644
> index 0000000..1ab29f6
> --- /dev/null
> +++ b/sound/soc/qcom/sc7280.c
> @@ -0,0 +1,347 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
> +//
> +// sc7280.c -- ALSA SoC Machine driver for sc7280
> +
> +#include <dt-bindings/sound/sc7180-lpass.h>
> +#include <dt-bindings/sound/qcom,q6afe.h>
Can these come after the linux/sound includes?
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <sound/core.h>
> +#include <sound/jack.h>
> +#include <sound/pcm.h>
> +#include <sound/soc.h>
> +#include <uapi/linux/input-event-codes.h>
Is this include used?
> +
> +#include "../codecs/wcd938x.h"
> +#include "common.h"
> +#include "lpass.h"
> +
> +#define DRIVER_NAME "SC7280"
Is this useful? Why not just inline it in the one place it is used so we
don't have to jump to the define to figure out what it is?
> +#define LPASS_MAX_PORTS (LPASS_CDC_DMA_VA_TX8 + 1)
> +
> +
> +struct sc7280_snd_data {
> + bool stream_prepared[LPASS_MAX_PORTS];
> + struct snd_soc_card card;
> + struct sdw_stream_runtime *sruntime[LPASS_MAX_PORTS];
> + struct snd_soc_jack hs_jack;
> + struct snd_soc_jack hdmi_jack;
> + bool jack_setup;
> +};
> +
> +static void sc7280_jack_free(struct snd_jack *jack)
> +{
> + struct snd_soc_component *component = jack->private_data;
> +
> + snd_soc_component_set_jack(component, NULL, NULL);
> +}
> +
> +static int sc7280_headset_init(struct snd_soc_pcm_runtime *rtd)
> +{
> + struct snd_soc_card *card = rtd->card;
> + struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(card);
> + struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> + struct snd_soc_component *component = codec_dai->component;
> + struct snd_jack *jack;
> + int rval, i;
> +
> + if (!pdata->jack_setup) {
> + rval = snd_soc_card_jack_new(card, "Headset Jack",
> + SND_JACK_HEADSET | SND_JACK_LINEOUT |
> + SND_JACK_MECHANICAL |
> + SND_JACK_BTN_0 | SND_JACK_BTN_1 |
> + SND_JACK_BTN_2 | SND_JACK_BTN_3 |
> + SND_JACK_BTN_4 | SND_JACK_BTN_5,
> + &pdata->hs_jack, NULL, 0);
> +
> + if (rval < 0) {
> + dev_err(card->dev, "Unable to add Headset Jack\n");
> + return rval;
> + }
> +
> + jack = pdata->hs_jack.jack;
> +
> + snd_jack_set_key(jack, SND_JACK_BTN_0, KEY_MEDIA);
> + snd_jack_set_key(jack, SND_JACK_BTN_1, KEY_VOICECOMMAND);
> + snd_jack_set_key(jack, SND_JACK_BTN_2, KEY_VOLUMEUP);
> + snd_jack_set_key(jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN);
> +
> + jack->private_data = component;
> + jack->private_free = sc7280_jack_free;
> + pdata->jack_setup = true;
> + }
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_TX3:
> + for_each_rtd_codec_dais(rtd, i, codec_dai) {
> + rval = snd_soc_component_set_jack(component, &pdata->hs_jack, NULL);
> + if (rval != 0 && rval != -EOPNOTSUPP) {
> + dev_warn(card->dev, "Failed to set jack: %d\n", rval);
Why not dev_err?
> + return rval;
> + }
> + }
> +
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int sc7280_hdmi_init(struct snd_soc_pcm_runtime *rtd)
> +{
> + struct snd_soc_card *card = rtd->card;
> + struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(card);
> + struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
> + struct snd_soc_component *component = codec_dai->component;
> + struct snd_jack *jack;
> + int rval;
> +
> + rval = snd_soc_card_jack_new(
> + card, "HDMI Jack",
> + SND_JACK_LINEOUT,
> + &pdata->hdmi_jack, NULL, 0);
> +
> + if (rval < 0) {
> + dev_err(card->dev, "Unable to add HDMI Jack\n");
> + return rval;
> + }
> +
> + jack = pdata->hdmi_jack.jack;
> + jack->private_data = component;
> + jack->private_free = sc7280_jack_free;
> +
> + return snd_soc_component_set_jack(component, &pdata->hdmi_jack, NULL);
> +}
> +
> +static int sc7280_init(struct snd_soc_pcm_runtime *rtd)
> +{
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_TX3:
> + return sc7280_headset_init(rtd);
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_VA_TX0:
> + case MI2S_SECONDARY:
> + return 0;
> + case LPASS_DP_RX:
> + return sc7280_hdmi_init(rtd);
> + default:
> + dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
> + return -EINVAL;
> + }
Nitpick: Add newline.
> + return 0;
Can we even get here? Maybe remove return from default above and make
this a return -EINVAL.
> +}
> +
> +static int sc7280_snd_startup(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_TX3:
> + case LPASS_CDC_DMA_VA_TX0:
> + break;
> + case MI2S_SECONDARY:
> + break;
> + case LPASS_DP_RX:
> + break;
> + default:
> + dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
> + return -EINVAL;
> + }
Nitpick: Add newline.
> + return 0;
> +}
> +
> +static int sc7280_snd_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params)
> +{
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *codec_dai;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
const?
> + struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card);
> + struct sdw_stream_runtime *sruntime;
> + int i;
> +
> + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
> + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_RATE, 48000, 48000);
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_TX3:
> + case LPASS_CDC_DMA_RX0:
> + for_each_rtd_codec_dais(rtd, i, codec_dai) {
> + sruntime = snd_soc_dai_get_sdw_stream(codec_dai, substream->stream);
> + if (sruntime != ERR_PTR(-EOPNOTSUPP))
> + pdata->sruntime[cpu_dai->id] = sruntime;
> + }
> + break;
> + }
> +
> + return 0;
> +
Nitpick: Drop newline.
> +}
> +
> +static int sc7280_snd_swr_prepare(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
const?
> + struct sc7280_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
> + struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
> + int ret;
> +
> + if (!sruntime)
> + return 0;
> +
> + if (data->stream_prepared[cpu_dai->id]) {
> + sdw_disable_stream(sruntime);
> + sdw_deprepare_stream(sruntime);
> + data->stream_prepared[cpu_dai->id] = false;
> + }
> +
> + ret = sdw_prepare_stream(sruntime);
> + if (ret)
> + return ret;
> +
> + ret = sdw_enable_stream(sruntime);
> + if (ret) {
> + sdw_deprepare_stream(sruntime);
> + return ret;
> + }
> + data->stream_prepared[cpu_dai->id] = true;
Why two spaces after ]?
> +
> + return ret;
> +}
> +
> +
> +static int sc7280_snd_prepare(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
const?
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_TX3:
> + return sc7280_snd_swr_prepare(substream);
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int sc7280_snd_hw_free(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct sc7280_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
const?
> + struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_TX3:
> + if (sruntime && data->stream_prepared[cpu_dai->id]) {
> + sdw_disable_stream(sruntime);
> + sdw_deprepare_stream(sruntime);
> + data->stream_prepared[cpu_dai->id] = false;
> + }
> + break;
> + default:
> + break;
> + }
> + return 0;
> +}
> +
> +static void sc7280_snd_shutdown(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_TX3:
> + case LPASS_CDC_DMA_VA_TX0:
> + break;
> + case MI2S_SECONDARY:
> + break;
> + case LPASS_DP_RX:
> + break;
> + default:
> + dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
Can use %#x to skip the 0x part.
> + break;
> + }
> +}
> +
> +static const struct snd_soc_ops sc7280_ops = {
> + .startup = sc7280_snd_startup,
> + .shutdown = sc7280_snd_shutdown,
> + .hw_params = sc7280_snd_hw_params,
> + .hw_free = sc7280_snd_hw_free,
> + .prepare = sc7280_snd_prepare,
> +};
> +
> +static const struct snd_soc_dapm_widget sc7280_snd_widgets[] = {
> + SND_SOC_DAPM_HP("Headphone Jack", NULL),
> + SND_SOC_DAPM_MIC("Headset Mic", NULL),
> +};
> +
> +static int sc7280_snd_platform_probe(struct platform_device *pdev)
> +{
> + struct snd_soc_card *card;
> + struct sc7280_snd_data *data;
> + struct device *dev = &pdev->dev;
> + struct snd_soc_dai_link *link;
> + int ret, i;
> +
> + /* Allocate the private data */
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + card = &data->card;
> + data->jack_setup = false;
Isn't that implicit via kzalloc above? This can be dropped.
> + snd_soc_card_set_drvdata(card, data);
> +
> + card->owner = THIS_MODULE;
> + card->driver_name = DRIVER_NAME;
> + card->dev = dev;
> +
> + ret = qcom_snd_parse_of(card);
> + if (ret)
> + return ret;
> +
> + for_each_card_prelinks(card, i, link) {
> + link->init = sc7280_init;
> + link->ops = &sc7280_ops;
> + }
> + ret = devm_snd_soc_register_card(dev, card);
> + return ret;
Nitpick:
return devm_snd_soc_register_card(dev, card)
> +}
> +
> +static const struct of_device_id sc7280_snd_device_id[] = {
> + {.compatible = "google,sc7280-herobrine"},
> + {},
Nitpick: Drop comma here so nothing can come after without causing a
compile error.
> +};
> +MODULE_DEVICE_TABLE(of, sc7280_snd_device_id);
> +
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] ASoC: qcom: SC7280: Add machine driver
@ 2021-09-08 22:42 ` Stephen Boyd
0 siblings, 0 replies; 14+ messages in thread
From: Stephen Boyd @ 2021-09-08 22:42 UTC (permalink / raw)
To: Srinivasa Rao Mandadapu, agross, alsa-devel, bgoswami,
bjorn.andersson, broonie, devicetree, judyhsiao, lgirdwood,
linux-arm-msm, linux-kernel, perex, plai, robh+dt, rohitkr,
srinivas.kandagatla, tiwai
Quoting Srinivasa Rao Mandadapu (2021-09-08 11:00:57)
> diff --git a/sound/soc/qcom/sc7280.c b/sound/soc/qcom/sc7280.c
> new file mode 100644
> index 0000000..1ab29f6
> --- /dev/null
> +++ b/sound/soc/qcom/sc7280.c
> @@ -0,0 +1,347 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
> +//
> +// sc7280.c -- ALSA SoC Machine driver for sc7280
> +
> +#include <dt-bindings/sound/sc7180-lpass.h>
> +#include <dt-bindings/sound/qcom,q6afe.h>
Can these come after the linux/sound includes?
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <sound/core.h>
> +#include <sound/jack.h>
> +#include <sound/pcm.h>
> +#include <sound/soc.h>
> +#include <uapi/linux/input-event-codes.h>
Is this include used?
> +
> +#include "../codecs/wcd938x.h"
> +#include "common.h"
> +#include "lpass.h"
> +
> +#define DRIVER_NAME "SC7280"
Is this useful? Why not just inline it in the one place it is used so we
don't have to jump to the define to figure out what it is?
> +#define LPASS_MAX_PORTS (LPASS_CDC_DMA_VA_TX8 + 1)
> +
> +
> +struct sc7280_snd_data {
> + bool stream_prepared[LPASS_MAX_PORTS];
> + struct snd_soc_card card;
> + struct sdw_stream_runtime *sruntime[LPASS_MAX_PORTS];
> + struct snd_soc_jack hs_jack;
> + struct snd_soc_jack hdmi_jack;
> + bool jack_setup;
> +};
> +
> +static void sc7280_jack_free(struct snd_jack *jack)
> +{
> + struct snd_soc_component *component = jack->private_data;
> +
> + snd_soc_component_set_jack(component, NULL, NULL);
> +}
> +
> +static int sc7280_headset_init(struct snd_soc_pcm_runtime *rtd)
> +{
> + struct snd_soc_card *card = rtd->card;
> + struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(card);
> + struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> + struct snd_soc_component *component = codec_dai->component;
> + struct snd_jack *jack;
> + int rval, i;
> +
> + if (!pdata->jack_setup) {
> + rval = snd_soc_card_jack_new(card, "Headset Jack",
> + SND_JACK_HEADSET | SND_JACK_LINEOUT |
> + SND_JACK_MECHANICAL |
> + SND_JACK_BTN_0 | SND_JACK_BTN_1 |
> + SND_JACK_BTN_2 | SND_JACK_BTN_3 |
> + SND_JACK_BTN_4 | SND_JACK_BTN_5,
> + &pdata->hs_jack, NULL, 0);
> +
> + if (rval < 0) {
> + dev_err(card->dev, "Unable to add Headset Jack\n");
> + return rval;
> + }
> +
> + jack = pdata->hs_jack.jack;
> +
> + snd_jack_set_key(jack, SND_JACK_BTN_0, KEY_MEDIA);
> + snd_jack_set_key(jack, SND_JACK_BTN_1, KEY_VOICECOMMAND);
> + snd_jack_set_key(jack, SND_JACK_BTN_2, KEY_VOLUMEUP);
> + snd_jack_set_key(jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN);
> +
> + jack->private_data = component;
> + jack->private_free = sc7280_jack_free;
> + pdata->jack_setup = true;
> + }
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_TX3:
> + for_each_rtd_codec_dais(rtd, i, codec_dai) {
> + rval = snd_soc_component_set_jack(component, &pdata->hs_jack, NULL);
> + if (rval != 0 && rval != -EOPNOTSUPP) {
> + dev_warn(card->dev, "Failed to set jack: %d\n", rval);
Why not dev_err?
> + return rval;
> + }
> + }
> +
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int sc7280_hdmi_init(struct snd_soc_pcm_runtime *rtd)
> +{
> + struct snd_soc_card *card = rtd->card;
> + struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(card);
> + struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
> + struct snd_soc_component *component = codec_dai->component;
> + struct snd_jack *jack;
> + int rval;
> +
> + rval = snd_soc_card_jack_new(
> + card, "HDMI Jack",
> + SND_JACK_LINEOUT,
> + &pdata->hdmi_jack, NULL, 0);
> +
> + if (rval < 0) {
> + dev_err(card->dev, "Unable to add HDMI Jack\n");
> + return rval;
> + }
> +
> + jack = pdata->hdmi_jack.jack;
> + jack->private_data = component;
> + jack->private_free = sc7280_jack_free;
> +
> + return snd_soc_component_set_jack(component, &pdata->hdmi_jack, NULL);
> +}
> +
> +static int sc7280_init(struct snd_soc_pcm_runtime *rtd)
> +{
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_TX3:
> + return sc7280_headset_init(rtd);
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_VA_TX0:
> + case MI2S_SECONDARY:
> + return 0;
> + case LPASS_DP_RX:
> + return sc7280_hdmi_init(rtd);
> + default:
> + dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
> + return -EINVAL;
> + }
Nitpick: Add newline.
> + return 0;
Can we even get here? Maybe remove return from default above and make
this a return -EINVAL.
> +}
> +
> +static int sc7280_snd_startup(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_TX3:
> + case LPASS_CDC_DMA_VA_TX0:
> + break;
> + case MI2S_SECONDARY:
> + break;
> + case LPASS_DP_RX:
> + break;
> + default:
> + dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
> + return -EINVAL;
> + }
Nitpick: Add newline.
> + return 0;
> +}
> +
> +static int sc7280_snd_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params)
> +{
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *codec_dai;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
const?
> + struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card);
> + struct sdw_stream_runtime *sruntime;
> + int i;
> +
> + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
> + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_RATE, 48000, 48000);
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_TX3:
> + case LPASS_CDC_DMA_RX0:
> + for_each_rtd_codec_dais(rtd, i, codec_dai) {
> + sruntime = snd_soc_dai_get_sdw_stream(codec_dai, substream->stream);
> + if (sruntime != ERR_PTR(-EOPNOTSUPP))
> + pdata->sruntime[cpu_dai->id] = sruntime;
> + }
> + break;
> + }
> +
> + return 0;
> +
Nitpick: Drop newline.
> +}
> +
> +static int sc7280_snd_swr_prepare(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
const?
> + struct sc7280_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
> + struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
> + int ret;
> +
> + if (!sruntime)
> + return 0;
> +
> + if (data->stream_prepared[cpu_dai->id]) {
> + sdw_disable_stream(sruntime);
> + sdw_deprepare_stream(sruntime);
> + data->stream_prepared[cpu_dai->id] = false;
> + }
> +
> + ret = sdw_prepare_stream(sruntime);
> + if (ret)
> + return ret;
> +
> + ret = sdw_enable_stream(sruntime);
> + if (ret) {
> + sdw_deprepare_stream(sruntime);
> + return ret;
> + }
> + data->stream_prepared[cpu_dai->id] = true;
Why two spaces after ]?
> +
> + return ret;
> +}
> +
> +
> +static int sc7280_snd_prepare(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
const?
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_TX3:
> + return sc7280_snd_swr_prepare(substream);
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int sc7280_snd_hw_free(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct sc7280_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
const?
> + struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_TX3:
> + if (sruntime && data->stream_prepared[cpu_dai->id]) {
> + sdw_disable_stream(sruntime);
> + sdw_deprepare_stream(sruntime);
> + data->stream_prepared[cpu_dai->id] = false;
> + }
> + break;
> + default:
> + break;
> + }
> + return 0;
> +}
> +
> +static void sc7280_snd_shutdown(struct snd_pcm_substream *substream)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> +
> + switch (cpu_dai->id) {
> + case LPASS_CDC_DMA_RX0:
> + case LPASS_CDC_DMA_TX3:
> + case LPASS_CDC_DMA_VA_TX0:
> + break;
> + case MI2S_SECONDARY:
> + break;
> + case LPASS_DP_RX:
> + break;
> + default:
> + dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
Can use %#x to skip the 0x part.
> + break;
> + }
> +}
> +
> +static const struct snd_soc_ops sc7280_ops = {
> + .startup = sc7280_snd_startup,
> + .shutdown = sc7280_snd_shutdown,
> + .hw_params = sc7280_snd_hw_params,
> + .hw_free = sc7280_snd_hw_free,
> + .prepare = sc7280_snd_prepare,
> +};
> +
> +static const struct snd_soc_dapm_widget sc7280_snd_widgets[] = {
> + SND_SOC_DAPM_HP("Headphone Jack", NULL),
> + SND_SOC_DAPM_MIC("Headset Mic", NULL),
> +};
> +
> +static int sc7280_snd_platform_probe(struct platform_device *pdev)
> +{
> + struct snd_soc_card *card;
> + struct sc7280_snd_data *data;
> + struct device *dev = &pdev->dev;
> + struct snd_soc_dai_link *link;
> + int ret, i;
> +
> + /* Allocate the private data */
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + card = &data->card;
> + data->jack_setup = false;
Isn't that implicit via kzalloc above? This can be dropped.
> + snd_soc_card_set_drvdata(card, data);
> +
> + card->owner = THIS_MODULE;
> + card->driver_name = DRIVER_NAME;
> + card->dev = dev;
> +
> + ret = qcom_snd_parse_of(card);
> + if (ret)
> + return ret;
> +
> + for_each_card_prelinks(card, i, link) {
> + link->init = sc7280_init;
> + link->ops = &sc7280_ops;
> + }
> + ret = devm_snd_soc_register_card(dev, card);
> + return ret;
Nitpick:
return devm_snd_soc_register_card(dev, card)
> +}
> +
> +static const struct of_device_id sc7280_snd_device_id[] = {
> + {.compatible = "google,sc7280-herobrine"},
> + {},
Nitpick: Drop comma here so nothing can come after without causing a
compile error.
> +};
> +MODULE_DEVICE_TABLE(of, sc7280_snd_device_id);
> +
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings
2021-09-08 18:00 ` [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings Srinivasa Rao Mandadapu
@ 2021-09-09 12:32 ` Rob Herring
0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2021-09-09 12:32 UTC (permalink / raw)
To: Srinivasa Rao Mandadapu
Cc: bjorn.andersson, agross, rohitkr, linux-arm-msm,
srinivas.kandagatla, linux-kernel, broonie, alsa-devel, robh+dt,
bgoswami, devicetree, tiwai, judyhsiao, lgirdwood, perex, swboyd,
plai
On Wed, 08 Sep 2021 23:30:56 +0530, Srinivasa Rao Mandadapu wrote:
> Add devicetree bindings documentation file for sc7280 sound card
> registration.
>
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> ---
> .../bindings/sound/google,sc7280-herobrine.yaml | 169 +++++++++++++++++++++
> 1 file changed, 169 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dts:46.24-25 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:379: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1438: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1525980
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings
@ 2021-09-09 12:32 ` Rob Herring
0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2021-09-09 12:32 UTC (permalink / raw)
To: Srinivasa Rao Mandadapu
Cc: devicetree, alsa-devel, bgoswami, lgirdwood, linux-arm-msm,
broonie, tiwai, linux-kernel, robh+dt, bjorn.andersson, agross,
rohitkr, swboyd, judyhsiao, plai
On Wed, 08 Sep 2021 23:30:56 +0530, Srinivasa Rao Mandadapu wrote:
> Add devicetree bindings documentation file for sc7280 sound card
> registration.
>
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> ---
> .../bindings/sound/google,sc7280-herobrine.yaml | 169 +++++++++++++++++++++
> 1 file changed, 169 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dts:46.24-25 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:379: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1438: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1525980
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings
2021-09-09 12:32 ` Rob Herring
@ 2021-09-13 8:20 ` Srinivasa Rao Mandadapu
-1 siblings, 0 replies; 14+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-09-13 8:20 UTC (permalink / raw)
To: Rob Herring
Cc: bjorn.andersson, agross, rohitkr, linux-arm-msm,
srinivas.kandagatla, linux-kernel, broonie, alsa-devel, robh+dt,
bgoswami, devicetree, tiwai, judyhsiao, lgirdwood, perex, swboyd,
plai
Thanks for info Rob!!!.
This error is due to missing dependency
patch.(https://patchwork.kernel.org/project/alsa-devel/list/?series=543829)
Could You Please check with the above patch applied?
On 9/9/2021 6:02 PM, Rob Herring wrote:
> On Wed, 08 Sep 2021 23:30:56 +0530, Srinivasa Rao Mandadapu wrote:
>> Add devicetree bindings documentation file for sc7280 sound card
>> registration.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
>> ---
>> .../bindings/sound/google,sc7280-herobrine.yaml | 169 +++++++++++++++++++++
>> 1 file changed, 169 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
>>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Error: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dts:46.24-25 syntax error
> FATAL ERROR: Unable to parse input tree
> make[1]: *** [scripts/Makefile.lib:379: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dt.yaml] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1438: dt_binding_check] Error 2
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/patch/1525980
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings
@ 2021-09-13 8:20 ` Srinivasa Rao Mandadapu
0 siblings, 0 replies; 14+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-09-13 8:20 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, alsa-devel, bgoswami, lgirdwood, linux-arm-msm,
broonie, tiwai, linux-kernel, robh+dt, bjorn.andersson, agross,
rohitkr, swboyd, judyhsiao, plai
Thanks for info Rob!!!.
This error is due to missing dependency
patch.(https://patchwork.kernel.org/project/alsa-devel/list/?series=543829)
Could You Please check with the above patch applied?
On 9/9/2021 6:02 PM, Rob Herring wrote:
> On Wed, 08 Sep 2021 23:30:56 +0530, Srinivasa Rao Mandadapu wrote:
>> Add devicetree bindings documentation file for sc7280 sound card
>> registration.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
>> ---
>> .../bindings/sound/google,sc7280-herobrine.yaml | 169 +++++++++++++++++++++
>> 1 file changed, 169 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
>>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Error: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dts:46.24-25 syntax error
> FATAL ERROR: Unable to parse input tree
> make[1]: *** [scripts/Makefile.lib:379: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dt.yaml] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1438: dt_binding_check] Error 2
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/patch/1525980
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] ASoC: qcom: SC7280: Add machine driver
2021-09-08 22:42 ` Stephen Boyd
(?)
@ 2021-09-13 12:26 ` Srinivasa Rao Mandadapu
-1 siblings, 0 replies; 14+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-09-13 12:26 UTC (permalink / raw)
To: Stephen Boyd, agross, alsa-devel, bgoswami, bjorn.andersson,
broonie, devicetree, judyhsiao, lgirdwood, linux-arm-msm,
linux-kernel, perex, plai, robh+dt, rohitkr, srinivas.kandagatla,
tiwai
Thanks for Your Time Stephen!!!
On 9/9/2021 4:12 AM, Stephen Boyd wrote:
> Quoting Srinivasa Rao Mandadapu (2021-09-08 11:00:57)
>> diff --git a/sound/soc/qcom/sc7280.c b/sound/soc/qcom/sc7280.c
>> new file mode 100644
>> index 0000000..1ab29f6
>> --- /dev/null
>> +++ b/sound/soc/qcom/sc7280.c
>> @@ -0,0 +1,347 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +//
>> +// Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
>> +//
>> +// sc7280.c -- ALSA SoC Machine driver for sc7280
>> +
>> +#include <dt-bindings/sound/sc7180-lpass.h>
>> +#include <dt-bindings/sound/qcom,q6afe.h>
> Can these come after the linux/sound includes?
yes. it can be moved to below.
>
>> +#include <linux/gpio.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <sound/core.h>
>> +#include <sound/jack.h>
>> +#include <sound/pcm.h>
>> +#include <sound/soc.h>
>> +#include <uapi/linux/input-event-codes.h>
> Is this include used?
This is is required for macros KEY_MEDIA' KEY_'VOICECOMMAND' 'KEY_VOLUMEUP'.
>
>> +
>> +#include "../codecs/wcd938x.h"
>> +#include "common.h"
>> +#include "lpass.h"
>> +
>> +#define DRIVER_NAME "SC7280"
> Is this useful? Why not just inline it in the one place it is used so we
> don't have to jump to the define to figure out what it is?
Yes. will make it inline.
>> +#define LPASS_MAX_PORTS (LPASS_CDC_DMA_VA_TX8 + 1)
>> +
>> +
>> +struct sc7280_snd_data {
>> + bool stream_prepared[LPASS_MAX_PORTS];
>> + struct snd_soc_card card;
>> + struct sdw_stream_runtime *sruntime[LPASS_MAX_PORTS];
>> + struct snd_soc_jack hs_jack;
>> + struct snd_soc_jack hdmi_jack;
>> + bool jack_setup;
>> +};
>> +
>> +static void sc7280_jack_free(struct snd_jack *jack)
>> +{
>> + struct snd_soc_component *component = jack->private_data;
>> +
>> + snd_soc_component_set_jack(component, NULL, NULL);
>> +}
>> +
>> +static int sc7280_headset_init(struct snd_soc_pcm_runtime *rtd)
>> +{
>> + struct snd_soc_card *card = rtd->card;
>> + struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(card);
>> + struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
>> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
>> + struct snd_soc_component *component = codec_dai->component;
>> + struct snd_jack *jack;
>> + int rval, i;
>> +
>> + if (!pdata->jack_setup) {
>> + rval = snd_soc_card_jack_new(card, "Headset Jack",
>> + SND_JACK_HEADSET | SND_JACK_LINEOUT |
>> + SND_JACK_MECHANICAL |
>> + SND_JACK_BTN_0 | SND_JACK_BTN_1 |
>> + SND_JACK_BTN_2 | SND_JACK_BTN_3 |
>> + SND_JACK_BTN_4 | SND_JACK_BTN_5,
>> + &pdata->hs_jack, NULL, 0);
>> +
>> + if (rval < 0) {
>> + dev_err(card->dev, "Unable to add Headset Jack\n");
>> + return rval;
>> + }
>> +
>> + jack = pdata->hs_jack.jack;
>> +
>> + snd_jack_set_key(jack, SND_JACK_BTN_0, KEY_MEDIA);
>> + snd_jack_set_key(jack, SND_JACK_BTN_1, KEY_VOICECOMMAND);
>> + snd_jack_set_key(jack, SND_JACK_BTN_2, KEY_VOLUMEUP);
>> + snd_jack_set_key(jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN);
>> +
>> + jack->private_data = component;
>> + jack->private_free = sc7280_jack_free;
>> + pdata->jack_setup = true;
>> + }
>> + switch (cpu_dai->id) {
>> + case LPASS_CDC_DMA_RX0:
>> + case LPASS_CDC_DMA_TX3:
>> + for_each_rtd_codec_dais(rtd, i, codec_dai) {
>> + rval = snd_soc_component_set_jack(component, &pdata->hs_jack, NULL);
>> + if (rval != 0 && rval != -EOPNOTSUPP) {
>> + dev_warn(card->dev, "Failed to set jack: %d\n", rval);
> Why not dev_err?
Okay. will change to dev_err().
>
>> + return rval;
>> + }
>> + }
>> +
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int sc7280_hdmi_init(struct snd_soc_pcm_runtime *rtd)
>> +{
>> + struct snd_soc_card *card = rtd->card;
>> + struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(card);
>> + struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
>> + struct snd_soc_component *component = codec_dai->component;
>> + struct snd_jack *jack;
>> + int rval;
>> +
>> + rval = snd_soc_card_jack_new(
>> + card, "HDMI Jack",
>> + SND_JACK_LINEOUT,
>> + &pdata->hdmi_jack, NULL, 0);
>> +
>> + if (rval < 0) {
>> + dev_err(card->dev, "Unable to add HDMI Jack\n");
>> + return rval;
>> + }
>> +
>> + jack = pdata->hdmi_jack.jack;
>> + jack->private_data = component;
>> + jack->private_free = sc7280_jack_free;
>> +
>> + return snd_soc_component_set_jack(component, &pdata->hdmi_jack, NULL);
>> +}
>> +
>> +static int sc7280_init(struct snd_soc_pcm_runtime *rtd)
>> +{
>> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
>> +
>> + switch (cpu_dai->id) {
>> + case LPASS_CDC_DMA_TX3:
>> + return sc7280_headset_init(rtd);
>> + case LPASS_CDC_DMA_RX0:
>> + case LPASS_CDC_DMA_VA_TX0:
>> + case MI2S_SECONDARY:
>> + return 0;
>> + case LPASS_DP_RX:
>> + return sc7280_hdmi_init(rtd);
>> + default:
>> + dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
>> + return -EINVAL;
>> + }
> Nitpick: Add newline.
Okay.
>
>> + return 0;
> Can we even get here? Maybe remove return from default above and make
> this a return -EINVAL.
>
>> +}
>> +
>> +static int sc7280_snd_startup(struct snd_pcm_substream *substream)
>> +{
>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
>> +
>> + switch (cpu_dai->id) {
>> + case LPASS_CDC_DMA_RX0:
>> + case LPASS_CDC_DMA_TX3:
>> + case LPASS_CDC_DMA_VA_TX0:
>> + break;
>> + case MI2S_SECONDARY:
>> + break;
>> + case LPASS_DP_RX:
>> + break;
>> + default:
>> + dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
>> + return -EINVAL;
>> + }
> Nitpick: Add newline.
Okay.
>
>> + return 0;
>> +}
>> +
>> +static int sc7280_snd_hw_params(struct snd_pcm_substream *substream,
>> + struct snd_pcm_hw_params *params)
>> +{
>> + struct snd_pcm_runtime *runtime = substream->runtime;
>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> + struct snd_soc_dai *codec_dai;
>> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> const?
Okay.
>
>> + struct sc7280_snd_data *pdata = snd_soc_card_get_drvdata(rtd->card);
>> + struct sdw_stream_runtime *sruntime;
>> + int i;
>> +
>> + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_CHANNELS, 2, 2);
>> + snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_RATE, 48000, 48000);
>> +
>> + switch (cpu_dai->id) {
>> + case LPASS_CDC_DMA_TX3:
>> + case LPASS_CDC_DMA_RX0:
>> + for_each_rtd_codec_dais(rtd, i, codec_dai) {
>> + sruntime = snd_soc_dai_get_sdw_stream(codec_dai, substream->stream);
>> + if (sruntime != ERR_PTR(-EOPNOTSUPP))
>> + pdata->sruntime[cpu_dai->id] = sruntime;
>> + }
>> + break;
>> + }
>> +
>> + return 0;
>> +
> Nitpick: Drop newline.
Okay.
>
>> +}
>> +
>> +static int sc7280_snd_swr_prepare(struct snd_pcm_substream *substream)
>> +{
>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> const?
Okay.
>
>> + struct sc7280_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
>> + struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
>> + int ret;
>> +
>> + if (!sruntime)
>> + return 0;
>> +
>> + if (data->stream_prepared[cpu_dai->id]) {
>> + sdw_disable_stream(sruntime);
>> + sdw_deprepare_stream(sruntime);
>> + data->stream_prepared[cpu_dai->id] = false;
>> + }
>> +
>> + ret = sdw_prepare_stream(sruntime);
>> + if (ret)
>> + return ret;
>> +
>> + ret = sdw_enable_stream(sruntime);
>> + if (ret) {
>> + sdw_deprepare_stream(sruntime);
>> + return ret;
>> + }
>> + data->stream_prepared[cpu_dai->id] = true;
> Why two spaces after ]?
Okay. Will remove it.
>
>> +
>> + return ret;
>> +}
>> +
>> +
>> +static int sc7280_snd_prepare(struct snd_pcm_substream *substream)
>> +{
>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> const?
Okay.
>
>> +
>> + switch (cpu_dai->id) {
>> + case LPASS_CDC_DMA_RX0:
>> + case LPASS_CDC_DMA_TX3:
>> + return sc7280_snd_swr_prepare(substream);
>> + default:
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int sc7280_snd_hw_free(struct snd_pcm_substream *substream)
>> +{
>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> + struct sc7280_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
>> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> const?
Okay.
>
>> + struct sdw_stream_runtime *sruntime = data->sruntime[cpu_dai->id];
>> +
>> + switch (cpu_dai->id) {
>> + case LPASS_CDC_DMA_RX0:
>> + case LPASS_CDC_DMA_TX3:
>> + if (sruntime && data->stream_prepared[cpu_dai->id]) {
>> + sdw_disable_stream(sruntime);
>> + sdw_deprepare_stream(sruntime);
>> + data->stream_prepared[cpu_dai->id] = false;
>> + }
>> + break;
>> + default:
>> + break;
>> + }
>> + return 0;
>> +}
>> +
>> +static void sc7280_snd_shutdown(struct snd_pcm_substream *substream)
>> +{
>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
>> +
>> + switch (cpu_dai->id) {
>> + case LPASS_CDC_DMA_RX0:
>> + case LPASS_CDC_DMA_TX3:
>> + case LPASS_CDC_DMA_VA_TX0:
>> + break;
>> + case MI2S_SECONDARY:
>> + break;
>> + case LPASS_DP_RX:
>> + break;
>> + default:
>> + dev_err(rtd->dev, "%s: invalid dai id 0x%x\n", __func__, cpu_dai->id);
> Can use %#x to skip the 0x part.
Okay. Will Change it to %d.
>
>> + break;
>> + }
>> +}
>> +
>> +static const struct snd_soc_ops sc7280_ops = {
>> + .startup = sc7280_snd_startup,
>> + .shutdown = sc7280_snd_shutdown,
>> + .hw_params = sc7280_snd_hw_params,
>> + .hw_free = sc7280_snd_hw_free,
>> + .prepare = sc7280_snd_prepare,
>> +};
>> +
>> +static const struct snd_soc_dapm_widget sc7280_snd_widgets[] = {
>> + SND_SOC_DAPM_HP("Headphone Jack", NULL),
>> + SND_SOC_DAPM_MIC("Headset Mic", NULL),
>> +};
>> +
>> +static int sc7280_snd_platform_probe(struct platform_device *pdev)
>> +{
>> + struct snd_soc_card *card;
>> + struct sc7280_snd_data *data;
>> + struct device *dev = &pdev->dev;
>> + struct snd_soc_dai_link *link;
>> + int ret, i;
>> +
>> + /* Allocate the private data */
>> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + card = &data->card;
>> + data->jack_setup = false;
> Isn't that implicit via kzalloc above? This can be dropped.
Yes. Will remove it.
>
>> + snd_soc_card_set_drvdata(card, data);
>> +
>> + card->owner = THIS_MODULE;
>> + card->driver_name = DRIVER_NAME;
>> + card->dev = dev;
>> +
>> + ret = qcom_snd_parse_of(card);
>> + if (ret)
>> + return ret;
>> +
>> + for_each_card_prelinks(card, i, link) {
>> + link->init = sc7280_init;
>> + link->ops = &sc7280_ops;
>> + }
>> + ret = devm_snd_soc_register_card(dev, card);
>> + return ret;
> Nitpick:
>
> return devm_snd_soc_register_card(dev, card)
Okay. will change it.
>
>> +}
>> +
>> +static const struct of_device_id sc7280_snd_device_id[] = {
>> + {.compatible = "google,sc7280-herobrine"},
>> + {},
> Nitpick: Drop comma here so nothing can come after without causing a
> compile error.
Okay. Will remove it.
>
>> +};
>> +MODULE_DEVICE_TABLE(of, sc7280_snd_device_id);
>> +
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings
2021-09-13 8:20 ` Srinivasa Rao Mandadapu
@ 2021-09-14 12:53 ` Rob Herring
-1 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2021-09-14 12:53 UTC (permalink / raw)
To: Srinivasa Rao Mandadapu
Cc: Bjorn Andersson, Gross, Andy, Rohit kumar, linux-arm-msm,
Srinivas Kandagatla, linux-kernel, Mark Brown, Linux-ALSA,
Banajit Goswami, devicetree, Takashi Iwai, judyhsiao,
Liam Girdwood, Jaroslav Kysela, Stephen Boyd, Patrick Lai
On Mon, Sep 13, 2021 at 3:20 AM Srinivasa Rao Mandadapu
<srivasam@codeaurora.org> wrote:
>
> Thanks for info Rob!!!.
Don't top-post on maillists.
>
> This error is due to missing dependency
> patch.(https://patchwork.kernel.org/project/alsa-devel/list/?series=543829)
>
> Could You Please check with the above patch applied?
No. How is a script supposed to handle that?
Where have you noted the dependency in the patch? Without that, I
assume you just didn't run 'make dt_binding_check' and the maintainer
doesn't know to check that the dependency is applied.
Rob
>
>
> On 9/9/2021 6:02 PM, Rob Herring wrote:
> > On Wed, 08 Sep 2021 23:30:56 +0530, Srinivasa Rao Mandadapu wrote:
> >> Add devicetree bindings documentation file for sc7280 sound card
> >> registration.
> >>
> >> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> >> ---
> >> .../bindings/sound/google,sc7280-herobrine.yaml | 169 +++++++++++++++++++++
> >> 1 file changed, 169 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
> >>
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > Error: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dts:46.24-25 syntax error
> > FATAL ERROR: Unable to parse input tree
> > make[1]: *** [scripts/Makefile.lib:379: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dt.yaml] Error 1
> > make[1]: *** Waiting for unfinished jobs....
> > make: *** [Makefile:1438: dt_binding_check] Error 2
> >
> > doc reference errors (make refcheckdocs):
> >
> > See https://patchwork.ozlabs.org/patch/1525980
> >
> > This check can fail if there are any dependencies. The base for a patch
> > series is generally the most recent rc1.
> >
> > If you already ran 'make dt_binding_check' and didn't see the above
> > error(s), then make sure 'yamllint' is installed and dt-schema is up to
> > date:
> >
> > pip3 install dtschema --upgrade
> >
> > Please check and re-submit.
> >
> --
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
> is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings
@ 2021-09-14 12:53 ` Rob Herring
0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2021-09-14 12:53 UTC (permalink / raw)
To: Srinivasa Rao Mandadapu
Cc: devicetree, Linux-ALSA, Banajit Goswami, Liam Girdwood,
linux-arm-msm, Mark Brown, Takashi Iwai, linux-kernel,
Stephen Boyd, Bjorn Andersson, Gross, Andy, Rohit kumar,
judyhsiao, Patrick Lai
On Mon, Sep 13, 2021 at 3:20 AM Srinivasa Rao Mandadapu
<srivasam@codeaurora.org> wrote:
>
> Thanks for info Rob!!!.
Don't top-post on maillists.
>
> This error is due to missing dependency
> patch.(https://patchwork.kernel.org/project/alsa-devel/list/?series=543829)
>
> Could You Please check with the above patch applied?
No. How is a script supposed to handle that?
Where have you noted the dependency in the patch? Without that, I
assume you just didn't run 'make dt_binding_check' and the maintainer
doesn't know to check that the dependency is applied.
Rob
>
>
> On 9/9/2021 6:02 PM, Rob Herring wrote:
> > On Wed, 08 Sep 2021 23:30:56 +0530, Srinivasa Rao Mandadapu wrote:
> >> Add devicetree bindings documentation file for sc7280 sound card
> >> registration.
> >>
> >> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> >> ---
> >> .../bindings/sound/google,sc7280-herobrine.yaml | 169 +++++++++++++++++++++
> >> 1 file changed, 169 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
> >>
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > Error: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dts:46.24-25 syntax error
> > FATAL ERROR: Unable to parse input tree
> > make[1]: *** [scripts/Makefile.lib:379: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dt.yaml] Error 1
> > make[1]: *** Waiting for unfinished jobs....
> > make: *** [Makefile:1438: dt_binding_check] Error 2
> >
> > doc reference errors (make refcheckdocs):
> >
> > See https://patchwork.ozlabs.org/patch/1525980
> >
> > This check can fail if there are any dependencies. The base for a patch
> > series is generally the most recent rc1.
> >
> > If you already ran 'make dt_binding_check' and didn't see the above
> > error(s), then make sure 'yamllint' is installed and dt-schema is up to
> > date:
> >
> > pip3 install dtschema --upgrade
> >
> > Please check and re-submit.
> >
> --
> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
> is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings
2021-09-14 12:53 ` Rob Herring
@ 2021-09-15 3:44 ` Srinivasa Rao Mandadapu
-1 siblings, 0 replies; 14+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-09-15 3:44 UTC (permalink / raw)
To: Rob Herring
Cc: Bjorn Andersson, Gross, Andy, Rohit kumar, linux-arm-msm,
Srinivas Kandagatla, linux-kernel, Mark Brown, Linux-ALSA,
Banajit Goswami, devicetree, Takashi Iwai, judyhsiao,
Liam Girdwood, Jaroslav Kysela, Stephen Boyd, Patrick Lai
On 9/14/2021 6:23 PM, Rob Herring wrote:
> On Mon, Sep 13, 2021 at 3:20 AM Srinivasa Rao Mandadapu
> <srivasam@codeaurora.org> wrote:
>> Thanks for info Rob!!!.
> Don't top-post on maillists.
Okay. Will take care from Next time.
>
>> This error is due to missing dependency
>> patch.(https://patchwork.kernel.org/project/alsa-devel/list/?series=543829)
>>
>> Could You Please check with the above patch applied?
> No. How is a script supposed to handle that?
>
> Where have you noted the dependency in the patch? Without that, I
> assume you just didn't run 'make dt_binding_check' and the maintainer
> doesn't know to check that the dependency is applied.
>
> Rob
Included dependency list in cover letter. Will include in this patch
next time.
>>
>> On 9/9/2021 6:02 PM, Rob Herring wrote:
>>> On Wed, 08 Sep 2021 23:30:56 +0530, Srinivasa Rao Mandadapu wrote:
>>>> Add devicetree bindings documentation file for sc7280 sound card
>>>> registration.
>>>>
>>>> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
>>>> ---
>>>> .../bindings/sound/google,sc7280-herobrine.yaml | 169 +++++++++++++++++++++
>>>> 1 file changed, 169 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
>>>>
>>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>>>
>>> yamllint warnings/errors:
>>>
>>> dtschema/dtc warnings/errors:
>>> Error: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dts:46.24-25 syntax error
>>> FATAL ERROR: Unable to parse input tree
>>> make[1]: *** [scripts/Makefile.lib:379: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dt.yaml] Error 1
>>> make[1]: *** Waiting for unfinished jobs....
>>> make: *** [Makefile:1438: dt_binding_check] Error 2
>>>
>>> doc reference errors (make refcheckdocs):
>>>
>>> See https://patchwork.ozlabs.org/patch/1525980
>>>
>>> This check can fail if there are any dependencies. The base for a patch
>>> series is generally the most recent rc1.
>>>
>>> If you already ran 'make dt_binding_check' and didn't see the above
>>> error(s), then make sure 'yamllint' is installed and dt-schema is up to
>>> date:
>>>
>>> pip3 install dtschema --upgrade
>>>
>>> Please check and re-submit.
>>>
>> --
>> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
>> is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>>
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings
@ 2021-09-15 3:44 ` Srinivasa Rao Mandadapu
0 siblings, 0 replies; 14+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-09-15 3:44 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Linux-ALSA, Banajit Goswami, Liam Girdwood,
linux-arm-msm, Mark Brown, Takashi Iwai, linux-kernel,
Stephen Boyd, Bjorn Andersson, Gross, Andy, Rohit kumar,
judyhsiao, Patrick Lai
On 9/14/2021 6:23 PM, Rob Herring wrote:
> On Mon, Sep 13, 2021 at 3:20 AM Srinivasa Rao Mandadapu
> <srivasam@codeaurora.org> wrote:
>> Thanks for info Rob!!!.
> Don't top-post on maillists.
Okay. Will take care from Next time.
>
>> This error is due to missing dependency
>> patch.(https://patchwork.kernel.org/project/alsa-devel/list/?series=543829)
>>
>> Could You Please check with the above patch applied?
> No. How is a script supposed to handle that?
>
> Where have you noted the dependency in the patch? Without that, I
> assume you just didn't run 'make dt_binding_check' and the maintainer
> doesn't know to check that the dependency is applied.
>
> Rob
Included dependency list in cover letter. Will include in this patch
next time.
>>
>> On 9/9/2021 6:02 PM, Rob Herring wrote:
>>> On Wed, 08 Sep 2021 23:30:56 +0530, Srinivasa Rao Mandadapu wrote:
>>>> Add devicetree bindings documentation file for sc7280 sound card
>>>> registration.
>>>>
>>>> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
>>>> ---
>>>> .../bindings/sound/google,sc7280-herobrine.yaml | 169 +++++++++++++++++++++
>>>> 1 file changed, 169 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/sound/google,sc7280-herobrine.yaml
>>>>
>>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
>>> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>>>
>>> yamllint warnings/errors:
>>>
>>> dtschema/dtc warnings/errors:
>>> Error: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dts:46.24-25 syntax error
>>> FATAL ERROR: Unable to parse input tree
>>> make[1]: *** [scripts/Makefile.lib:379: Documentation/devicetree/bindings/sound/google,sc7280-herobrine.example.dt.yaml] Error 1
>>> make[1]: *** Waiting for unfinished jobs....
>>> make: *** [Makefile:1438: dt_binding_check] Error 2
>>>
>>> doc reference errors (make refcheckdocs):
>>>
>>> See https://patchwork.ozlabs.org/patch/1525980
>>>
>>> This check can fail if there are any dependencies. The base for a patch
>>> series is generally the most recent rc1.
>>>
>>> If you already ran 'make dt_binding_check' and didn't see the above
>>> error(s), then make sure 'yamllint' is installed and dt-schema is up to
>>> date:
>>>
>>> pip3 install dtschema --upgrade
>>>
>>> Please check and re-submit.
>>>
>> --
>> Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
>> is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
>>
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-09-15 3:45 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-08 18:00 [PATCH 0/2] Machine driver to support LPASS SC7280 sound card registration Srinivasa Rao Mandadapu
2021-09-08 18:00 ` [PATCH 1/2] ASoC: google: dt-bindings: Add sc7280-herobrine machine bindings Srinivasa Rao Mandadapu
2021-09-09 12:32 ` Rob Herring
2021-09-09 12:32 ` Rob Herring
2021-09-13 8:20 ` Srinivasa Rao Mandadapu
2021-09-13 8:20 ` Srinivasa Rao Mandadapu
2021-09-14 12:53 ` Rob Herring
2021-09-14 12:53 ` Rob Herring
2021-09-15 3:44 ` Srinivasa Rao Mandadapu
2021-09-15 3:44 ` Srinivasa Rao Mandadapu
2021-09-08 18:00 ` [PATCH 2/2] ASoC: qcom: SC7280: Add machine driver Srinivasa Rao Mandadapu
2021-09-08 22:42 ` Stephen Boyd
2021-09-08 22:42 ` Stephen Boyd
2021-09-13 12:26 ` Srinivasa Rao Mandadapu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.