* [PATCH 0/2] ASoC: qcom: add apq8039 sound card and bindings
@ 2020-06-19 19:38 Pantelis Antoniou
2020-06-19 19:38 ` [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex Pantelis Antoniou
2020-06-19 19:38 ` [PATCH 2/2] ASoC: qcom: add apq8039 sound card support Pantelis Antoniou
0 siblings, 2 replies; 13+ messages in thread
From: Pantelis Antoniou @ 2020-06-19 19:38 UTC (permalink / raw)
To: alsa-devel
Cc: devicetree, linux-arm-msm, Srinivas Kandagatla, Mark Brown,
Matt Porter, Shawn Guo
Add support for the apq8039 sound card along with device tree
bindings for it.
A notable feature of this patchset is the capability to aggregate
arbitrary ALSA controls sequences into functions that are exposed
via a single enum ALSA control.
Detailed information for this mechanism is presented in the DT binding
document.
Pantelis Antoniou (2):
dt-bindings: sound: Device tree bindings for the apq8039 sound complex
ASoC: qcom: add apq8039 sound card support
.../bindings/sound/qcom,apq8039.yaml | 370 ++++++
sound/soc/qcom/Kconfig | 9 +
sound/soc/qcom/Makefile | 2 +
sound/soc/qcom/apq8039.c | 1126 +++++++++++++++++
4 files changed, 1507 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/qcom,apq8039.yaml
create mode 100644 sound/soc/qcom/apq8039.c
--
2.20.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex
2020-06-19 19:38 [PATCH 0/2] ASoC: qcom: add apq8039 sound card and bindings Pantelis Antoniou
@ 2020-06-19 19:38 ` Pantelis Antoniou
2020-06-19 21:41 ` Stephan Gerhold
2020-06-19 19:38 ` [PATCH 2/2] ASoC: qcom: add apq8039 sound card support Pantelis Antoniou
1 sibling, 1 reply; 13+ messages in thread
From: Pantelis Antoniou @ 2020-06-19 19:38 UTC (permalink / raw)
To: alsa-devel
Cc: devicetree, linux-arm-msm, Srinivas Kandagatla, Mark Brown,
Matt Porter, Shawn Guo
Add a yaml device binding for the QCOM apq8039 sound complex driver.
Signed-off-by: Pantelis Antoniou <pantelis.antoniou@linaro.org>
---
.../bindings/sound/qcom,apq8039.yaml | 370 ++++++++++++++++++
1 file changed, 370 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/qcom,apq8039.yaml
diff --git a/Documentation/devicetree/bindings/sound/qcom,apq8039.yaml b/Documentation/devicetree/bindings/sound/qcom,apq8039.yaml
new file mode 100644
index 000000000000..f1c4fb99ccbb
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/qcom,apq8039.yaml
@@ -0,0 +1,370 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/qcom,apq8039.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies APQ8039 ASoC sound card
+
+maintainers:
+ - Pantelis Antoniou <pantelis.antoniou@linaro.org>
+
+properties:
+ compatible:
+ items:
+ - const: qcom,apq8039-sndcard
+
+ pinctrl-0:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: |
+ Should specify pin control groups used for this controller matching
+ the first entry in pinctrl-names.
+
+ pinctrl-1:
+ description: |
+ Should specify pin control groups used for this controller matching
+ the second entry in pinctrl-names.
+
+ pinctrl-names:
+ minItems: 1
+ items:
+ - const: default
+ - const: sleep
+ description:
+ Names for the pin configuration(s); may be "default" or "sleep",
+ where the "sleep" configuration may describe the state
+ the pins should be in during system suspend.
+
+ reg:
+ description: Must contain an address for each entry in "reg-names".
+ minItems: 2
+ maxItems: 2
+
+ reg-names:
+ items:
+ - const: mic-iomux
+ - const: spkr-iomux
+
+ qcom,model:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: The user-visible name of the sound complex.
+
+ qcom,audio-routing:
+ $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+ description: |
+ 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
+ and MicBias of msm8916-analoc-wcd codec.
+
+ function-definition:
+ type: object
+ description: |
+ Functional configuration for the sound complex via a
+ simple control. allows fixed and dynamically constructed
+ function selection.
+
+ properties:
+ mixer-control:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: |
+ Name of the exported alsa mix control.
+
+ function-list:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: |
+ phandle(s) of the functions which the sound complex
+ exposes via the control.
+
+ system-list:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: |
+ phandle(s) of the default, init and shutdown functions
+ Must be one of the declared ones in the function property.
+ The default function is the one selected by default on
+ startup (after the init function's sequence is executed).
+ On shutdown the shutdown function sequence will be executed.
+ Typically init and shutdown are the same and it's purpose
+ is to initialize the sound complex mixer controls to the
+ all off state, and be ready for a regular function selection.
+
+ patternProperties:
+ "^[A-Za-z_][A-Aa-z0-9_]*$":
+ type: object
+ description:
+ Function description subnodes. The name of the function
+ is simply the name of the subnode, so restrictions apply
+ to the valid node names.
+
+ The function definition of each subnode is either a cooked
+ function (i.e. which is not dependent on state inputs), or
+ a function that is selecting a cooked function based on the
+ state inputs and the generated state vector.
+
+ oneOf:
+ # non-cooked function
+ - properties:
+ enable:
+ $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+ description: |
+ Sequence of alsa mixer controls to apply when this state is to
+ be enabled.
+
+ disable:
+ $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+ description: |
+ Sequence of alsa mixer controls to apply when this state is to
+ be disabled.
+
+ required:
+ - enable
+
+ # cooked function
+ - properties:
+ state-inputs:
+ description: |
+ A list of state inputs to be used in constructing a state
+ vector.
+ type: array
+ uniqueItems: true
+ minItems: 1
+ items:
+ anyOf:
+ - const: JACK_HEADPHONE
+ - const: JACK_MICROPHONE
+
+ state-input-bits:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ Number of bits to use for each state-input in the
+ state vector creation. For now only the value 1 is
+ supported for JACK_HEADPHONE and JACK_MICROPHONE.
+
+ state-input-defaults:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ The default value to use as a state input at startup.
+
+ state-map:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: |
+ The mapping of this function to a cooked function. The
+ format used is a sequence of phandle to a state, the mask
+ to apply to the state vector, and the equality value.
+
+ Take the example's configuration
+
+ state-inputs = "JACK_HEADPHONE", "JACK_MICROPHONE";
+ state-input-bits = <1>, <1>;
+ state-input-defaults = <0>, <0>;
+
+ state-map = <&speaker 0x1 0x0>,
+ <&headphones 0x3 0x1>,
+ <&headset 0x3 0x3>;
+
+ is decoded as follows.
+
+ There are 3 possible cooked functions to be selected.
+ speaker, headphone and headset. The state-inputs are
+ the JACK_HEADPHONE and JACK_MICROPHONE, which are single
+ bit values, being placed at bit 0 and bit 1 of the
+ constructed vector.
+
+ The 4 possible state vectors are:
+ MICROPHONE=0, HEADPHONE=0, 0
+ MICROPHONE=0, HEADPHONE=1, 1
+ MICROPHONE=1, HEADPHONE=0, 2
+ MICROPHONE=1, HEADPHONE=1, 3
+
+ The speaker function is selected when HEADPHONE=0 because
+ both (0 & 1) == (2 & 1) == 0.
+
+ The headphones function is selected when HEADPHONE=1 and
+ MICROPHONE=0 because (1 & 3) == 1.
+
+ The headset function is selected when both HEADPHONE=1 and
+ MICROPHONE=1 because (3 & 3) == 3.
+
+ required:
+ - state-inputs
+ - state-input-bits
+ - state-input-defaults
+ - state-map
+
+patternProperties:
+ "^.*dai-link-[0-9]+$":
+ type: object
+ description: |-
+ cpu and codec child nodes:
+ Container for cpu and codec dai sub-nodes.
+ One cpu and one codec sub-node must exist.
+
+ properties:
+ link-name:
+ description: The link name
+
+ cpu:
+ type: object
+ properties:
+
+ sound-dai:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: phandle(s) of the CPU DAI(s)
+
+ required:
+ - sound-dai
+
+ codec:
+ type: object
+ properties:
+
+ sound-dai:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ description: phandle(s) of the codec DAI(s)
+
+ required:
+ - sound-dai
+
+ required:
+ - link-name
+ - cpu
+ - codec
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - qcom,model
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/sound/apq8016-lpass.h>
+
+ sound: sound@7702000 {
+ compatible = "qcom,apq8039-sndcard";
+ reg = <0x07702000 0x4>, <0x07702004 0x4>;
+ reg-names = "mic-iomux", "spkr-iomux";
+
+ status = "okay";
+ pinctrl-0 = <&cdc_pdm_lines_act>;
+ pinctrl-1 = <&cdc_pdm_lines_sus>;
+ pinctrl-names = "default", "sleep";
+ qcom,model = "APQ8039";
+ qcom,audio-routing = "AMIC2", "MIC BIAS Internal2";
+
+ internal-codec-playback-dai-link-0 {
+ link-name = "WCD";
+ cpu {
+ sound-dai = <&lpass MI2S_PRIMARY>;
+ };
+ codec {
+ sound-dai = <&lpass_codec 0>, <&wcd_codec 0>;
+ };
+ };
+
+ internal-codec-capture-dai-link-0 {
+ link-name = "WCD-Capture";
+ cpu {
+ sound-dai = <&lpass MI2S_TERTIARY>;
+ };
+ codec {
+ sound-dai = <&lpass_codec 1>, <&wcd_codec 1>;
+ };
+ };
+
+ function-definition {
+
+ mixer-control = "Jack Function";
+ function-list = <&auto &headphones &headset &speaker &off>;
+ system-list = <&auto &off &off>; // default, init, shutdown
+
+ auto: Automatic {
+ // Headphone presence bit 0 (1) - H
+ // Microphone presence bit 1 (2) - M
+ state-inputs = "JACK_HEADPHONE", "JACK_MICROPHONE";
+ state-input-bits = <1>, <1>;
+ state-input-defaults = <0>, <0>;
+
+ // HM & MASK
+ state-map =
+ <&speaker 0x1 0x0>, // no headphone -> speaker
+ <&headphones 0x3 0x1>, // headphone but no mic -> headphones
+ <&headset 0x3 0x3>; // headphone & mic -> headset
+ };
+ headphones: Headphones {
+ enable =
+ "RX1 MIX1 INP1", "RX1",
+ "RX2 MIX1 INP1", "RX2",
+ "RDAC2 MUX", "RX2",
+ "RX1 Digital Volume", "128",
+ "RX2 Digital Volume", "128",
+ "HPHL", "Switch",
+ "HPHR", "Switch";
+
+ disable =
+ "RX1 Digital Volume", "0",
+ "RX2 Digital Volume", "0",
+ "HPHL", "ZERO",
+ "HPHR", "ZERO",
+ "RDAC2 MUX", "RX1",
+ "RX1 MIX1 INP1", "ZERO",
+ "RX2 MIX1 INP1", "ZERO";
+ };
+ headset: Headset {
+ enable =
+ "RX1 MIX1 INP1", "RX1",
+ "RX2 MIX1 INP1", "RX2",
+ "RDAC2 MUX", "RX2",
+ "RX1 Digital Volume", "128",
+ "RX2 Digital Volume", "128",
+ "DEC1 MUX", "ADC2",
+ "CIC1 MUX", "AMIC",
+ "ADC2 Volume", "8",
+ "ADC2 MUX", "INP2",
+ "HPHL", "Switch",
+ "HPHR", "Switch";
+
+ disable =
+ "RX1 Digital Volume", "0",
+ "RX2 Digital Volume", "0",
+ "HPHL", "ZERO",
+ "HPHR", "ZERO",
+ "RDAC2 MUX", "RX1",
+ "RX1 MIX1 INP1", "ZERO",
+ "RX2 MIX1 INP1", "ZERO",
+ "ADC2 MUX", "ZERO",
+ "ADC2 Volume", "0",
+ "DEC1 MUX", "ZERO";
+ };
+ speaker: Speaker {
+ enable =
+ "SPK DAC Switch", "1",
+ "RX3 MIX1 INP1", "RX1",
+ "RX3 MIX1 INP2", "RX2",
+ "RX3 Digital Volume", "128";
+
+ disable =
+ "SPK DAC Switch", "0",
+ "RX3 MIX1 INP1", "ZERO",
+ "RX3 MIX1 INP2", "ZERO";
+ };
+ off: Off {
+ enable =
+ "RX1 Digital Volume", "0",
+ "RX2 Digital Volume", "0",
+ "HPHL", "ZERO",
+ "HPHR", "ZERO",
+ "RDAC2 MUX", "RX1",
+ "RX1 MIX1 INP1", "ZERO",
+ "RX2 MIX1 INP1", "ZERO",
+ "ADC2 MUX", "ZERO",
+ "ADC2 Volume", "0",
+ "DEC1 MUX", "ZERO",
+ "SPK DAC Switch", "0",
+ "RX3 MIX1 INP1", "ZERO",
+ "RX3 MIX1 INP2", "ZERO";
+ };
+ };
+ };
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] ASoC: qcom: add apq8039 sound card support
2020-06-19 19:38 [PATCH 0/2] ASoC: qcom: add apq8039 sound card and bindings Pantelis Antoniou
2020-06-19 19:38 ` [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex Pantelis Antoniou
@ 2020-06-19 19:38 ` Pantelis Antoniou
2020-06-19 21:58 ` kernel test robot
2020-06-20 6:39 ` kernel test robot
1 sibling, 2 replies; 13+ messages in thread
From: Pantelis Antoniou @ 2020-06-19 19:38 UTC (permalink / raw)
To: alsa-devel
Cc: devicetree, linux-arm-msm, Srinivas Kandagatla, Mark Brown,
Matt Porter, Shawn Guo
This patch adds apq8039 machine driver support.
It is similar to the apq8016 driver but adds an method of
function configuration via an exposed mixer control, with
everything being configurable via DT.
Signed-off-by: Pantelis Antoniou <pantelis.antoniou@linaro.org>
---
sound/soc/qcom/Kconfig | 9 +
sound/soc/qcom/Makefile | 2 +
sound/soc/qcom/apq8039.c | 1126 ++++++++++++++++++++++++++++++++++++++
3 files changed, 1137 insertions(+)
create mode 100644 sound/soc/qcom/apq8039.c
diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
index f51b28d1b94d..350100781c04 100644
--- a/sound/soc/qcom/Kconfig
+++ b/sound/soc/qcom/Kconfig
@@ -42,6 +42,15 @@ config SND_SOC_APQ8016_SBC
APQ8016 SOC-based systems.
Say Y if you want to use audio devices on MI2S.
+config SND_SOC_APQ8039
+ tristate "SoC Audio support for APQ8039 based platforms"
+ depends on SND_SOC_QCOM
+ select SND_SOC_LPASS_APQ8016
+ help
+ Support for Qualcomm Technologies LPASS audio block in
+ APQ8039 SOC-based systems.
+ Say Y if you want to use audio devices on MI2S.
+
config SND_SOC_QCOM_COMMON
tristate
diff --git a/sound/soc/qcom/Makefile b/sound/soc/qcom/Makefile
index 41b2c7a23a4d..cc67e240744f 100644
--- a/sound/soc/qcom/Makefile
+++ b/sound/soc/qcom/Makefile
@@ -13,12 +13,14 @@ obj-$(CONFIG_SND_SOC_LPASS_APQ8016) += snd-soc-lpass-apq8016.o
# Machine
snd-soc-storm-objs := storm.o
snd-soc-apq8016-sbc-objs := apq8016_sbc.o
+snd-soc-apq8039-objs := apq8039.o
snd-soc-apq8096-objs := apq8096.o
snd-soc-sdm845-objs := sdm845.o
snd-soc-qcom-common-objs := common.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_APQ8039) += snd-soc-apq8039.o
obj-$(CONFIG_SND_SOC_MSM8996) += snd-soc-apq8096.o
obj-$(CONFIG_SND_SOC_SDM845) += snd-soc-sdm845.o
obj-$(CONFIG_SND_SOC_QCOM_COMMON) += snd-soc-qcom-common.o
diff --git a/sound/soc/qcom/apq8039.c b/sound/soc/qcom/apq8039.c
new file mode 100644
index 000000000000..0c413cf215dd
--- /dev/null
+++ b/sound/soc/qcom/apq8039.c
@@ -0,0 +1,1126 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2020, Linaro Limited
+
+// #define DEBUG
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/jack.h>
+#include <sound/soc.h>
+#include <uapi/linux/input-event-codes.h>
+#include <dt-bindings/sound/apq8016-lpass.h>
+
+struct ctl_set_node {
+ struct list_head node;
+ char *name;
+ char *value;
+};
+
+struct state_input_node {
+ struct list_head node;
+ const char *name;
+ int state_input;
+};
+
+struct apq8039_data {
+ void __iomem *mic_iomux;
+ void __iomem *spkr_iomux;
+ struct snd_soc_jack jack;
+ bool jack_setup;
+ int headphone_det;
+ int microphone_det;
+ struct list_head ctl_list;
+ struct list_head state_input_list;
+ int num_functions;
+ struct device_node **function_nodes;
+ const char **function_names;
+ struct soc_enum func_enum[1];
+ struct snd_kcontrol_new func_control[1];
+ struct device_node *function_node;
+ struct device_node *cooked_function_node;
+ struct device_node *init_node;
+ struct device_node *shutdown_node;
+ char *control_name;
+ struct snd_soc_dai_link dai_link[]; /* dynamically allocated */
+};
+
+static struct snd_kcontrol *
+snd_soc_card_ctl_find(struct snd_soc_card *card,
+ const char *name, const char *suffix)
+{
+ struct snd_ctl_elem_id sid;
+
+ memset(&sid, 0, sizeof(sid));
+ if (suffix)
+ snprintf(sid.name, sizeof(sid.name), "%s %s", name, suffix);
+ else
+ snprintf(sid.name, sizeof(sid.name), "%s", name);
+ sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+ return snd_ctl_find_id(card->snd_card, &sid);
+}
+
+static int
+snd_soc_card_ctl_getset(struct snd_soc_card *card,
+ const char *name,
+ char *orig, size_t origsz, const char *value)
+{
+ struct snd_kcontrol *ctl;
+ struct snd_ctl_elem_info *uinfo = NULL;
+ struct snd_ctl_elem_value *uctl = NULL;
+ unsigned int i, count;
+ int ret;
+
+ /* get the control */
+ ctl = snd_soc_card_ctl_find(card, name, NULL);
+ if (!ctl)
+ return -ENOENT;
+
+ /* allocate info and value */
+ uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
+ uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
+ if (!uinfo || !uctl) {
+ ret = -ENOMEM;
+ goto out_free;
+ }
+
+ ret = ctl->info(ctl, uinfo);
+ if (ret)
+ goto out_free;
+ if (uinfo->count != 1) {
+ ret = -ENOTSUPP;
+ goto out_free;
+ }
+
+ ret = ctl->get(ctl, uctl);
+ if (ret)
+ goto out_free;
+
+ ret = 0;
+
+ switch (uinfo->type) {
+
+ case SNDRV_CTL_ELEM_TYPE_NONE:
+ break;
+
+ case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
+ if (orig && origsz > 0)
+ snprintf(orig, origsz, "%s",
+ uctl->value.integer.value[0] ?
+ "true" : "false");
+
+ if (value) {
+ bool bval;
+
+ ret = kstrtobool(value, &bval);
+ if (ret)
+ goto out_free;
+ uctl->value.integer.value[0] = !!bval;
+
+ }
+ break;
+
+ case SNDRV_CTL_ELEM_TYPE_INTEGER:
+ if (orig && origsz > 0)
+ snprintf(orig, origsz, "%ld",
+ uctl->value.integer.value[0]);
+
+ if (value) {
+ ret = kstrtol(value, 10,
+ &uctl->value.integer.value[0]);
+ if (ret)
+ goto out_free;
+ }
+ break;
+
+ case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
+ count = uinfo->value.enumerated.items;
+
+ if (value) { /* set */
+ for (i = 0; i < count; i++) {
+ uinfo->value.enumerated.item = i;
+ ret = ctl->info(ctl, uinfo);
+ if (ret)
+ goto out_free;
+
+ if (!strcmp(value,
+ uinfo->value.enumerated.name))
+ break;
+ }
+
+ /* setting without finding the enum */
+ if (i >= count) {
+ ret = -EINVAL;
+ goto out_free;
+ }
+ uctl->value.enumerated.item[0] = i;
+ } else { /* get */
+ uinfo->value.enumerated.item =
+ uctl->value.enumerated.item[0];
+ ret = ctl->info(ctl, uinfo);
+ if (ret)
+ goto out_free;
+
+ if (orig && origsz) {
+ if (origsz <
+ strlen(uinfo->value.enumerated.name)
+ + 1) {
+ ret = -ENOSPC;
+ goto out_free;
+ }
+ strncpy(orig, uinfo->value.enumerated.name,
+ origsz);
+ }
+ }
+ break;
+
+ case SNDRV_CTL_ELEM_TYPE_INTEGER64:
+ if (orig && origsz > 0)
+ snprintf(orig, origsz, "%lld",
+ uctl->value.integer64.value[0]);
+
+ if (value) {
+ ret = kstrtoll(value, 10,
+ &uctl->value.integer64.value[0]);
+ if (ret)
+ goto out_free;
+ }
+ break;
+
+ case SNDRV_CTL_ELEM_TYPE_BYTES:
+ case SNDRV_CTL_ELEM_TYPE_IEC958:
+ ret = -ENOTSUPP;
+ goto out_free;
+ default:
+ ret = -EINVAL;
+ goto out_free;
+ }
+
+ if (value) {
+ ret = ctl->put(ctl, uctl);
+ if (ret < 0)
+ goto out_free;
+
+ /* if it changed, report change to user space */
+ if (ret > 0)
+ snd_ctl_notify(card->snd_card,
+ SNDRV_CTL_EVENT_MASK_VALUE,
+ &uctl->id);
+ }
+
+out_free:
+ kfree(uctl);
+ kfree(uinfo);
+
+ if (ret < 0)
+ dev_err(card->dev, "%s: %s operation failed for \"%s\"",
+ __func__, value ? "set" : "get", name);
+
+ return ret;
+}
+
+static int apq8039_ctl_set(struct snd_soc_card *card,
+ const char *name, const char *value)
+{
+ struct apq8039_data *pdata = snd_soc_card_get_drvdata(card);
+ struct ctl_set_node *n;
+ char *nvalue;
+
+ list_for_each_entry(n, &pdata->ctl_list, node) {
+ /* if it exists, just update with new value */
+ if (!strcmp(name, n->name)) {
+ nvalue = devm_kstrdup(card->dev, value, GFP_KERNEL);
+ if (!nvalue)
+ return -ENOMEM;
+ devm_kfree(card->dev, n->value);
+ n->value = nvalue;
+ dev_dbg(card->dev, "REP \"%s\" \"%s\"\r\n",
+ n->name, n->value);
+ return 0;
+ }
+ }
+ n = devm_kzalloc(card->dev, sizeof(*n), GFP_KERNEL);
+ if (!n)
+ return -ENOMEM;
+ n->name = devm_kstrdup(card->dev, name, GFP_KERNEL);
+ n->value = devm_kstrdup(card->dev, value, GFP_KERNEL);
+ if (!n->name || !n->value) {
+ /* n is not NULL and kfree can handle NULLs */
+ devm_kfree(card->dev, n->name);
+ devm_kfree(card->dev, n->value);
+ devm_kfree(card->dev, n);
+ return -ENOMEM;
+ }
+ list_add_tail(&n->node, &pdata->ctl_list);
+
+ dev_dbg(card->dev, "NEW \"%s\" \"%s\"\r\n", n->name, n->value);
+
+ return 0;
+}
+
+static int apq8039_ctl_commit(struct snd_soc_card *card)
+{
+ struct apq8039_data *pdata = snd_soc_card_get_drvdata(card);
+ struct ctl_set_node *n, *nn;
+ int res, failed_res;
+ bool modified;
+
+ modified = false;
+ failed_res = 0;
+ list_for_each_entry_safe(n, nn, &pdata->ctl_list, node) {
+
+ list_del(&n->node);
+
+ dev_dbg(card->dev, "CMT \"%s\" \"%s\"\r\n", n->name, n->value);
+ res = snd_soc_card_ctl_getset(card, n->name, NULL, 0, n->value);
+ if (res < 0 && !failed_res)
+ failed_res = res;
+ if (res > 0 && !modified)
+ modified = true;
+
+ devm_kfree(card->dev, n->name);
+ devm_kfree(card->dev, n->value);
+ devm_kfree(card->dev, n);
+ }
+
+ if (failed_res)
+ return failed_res;
+
+ return modified ? 1 : 0;
+}
+
+static bool apq8039_function_is_cooked(struct snd_soc_card *card,
+ struct device_node *funcnp)
+{
+ int res;
+
+ res = of_property_count_strings(funcnp, "state-inputs");
+ return res >= 1;
+}
+
+static int apq8039_state_input_get(struct snd_soc_card *card,
+ const char *name)
+{
+ struct apq8039_data *pdata = snd_soc_card_get_drvdata(card);
+ struct state_input_node *n;
+
+ list_for_each_entry(n, &pdata->state_input_list, node) {
+ if (!strcmp(n->name, name))
+ return n->state_input;
+ }
+
+ /* does not exist */
+ return -ENOENT;
+}
+
+static int apq8039_state_get(struct snd_soc_card *card,
+ struct device_node *funcnp)
+{
+ int res = -EINVAL, i, state, count;
+ u32 bits, def, bitpos;
+ const char *state_input;
+
+ res = of_property_count_strings(funcnp, "state-inputs");
+ if (res <= 0) {
+ /* if there's no events property this is a fixed function */
+ res = 0;
+ goto out;
+ }
+ count = res;
+
+ state = 0;
+ bitpos = 0;
+ for (i = 0; i < count; i++) {
+
+ res = of_property_read_string_index(funcnp, "state-inputs",
+ i, &state_input);
+ if (res < 0) {
+ dev_err(card->dev,
+ "Failed to read \"%s\" #%d of \"%s\"\n",
+ "state-inputs", i, funcnp->name);
+ goto out;
+ }
+
+ res = of_property_read_u32_index(funcnp, "state-input-bits",
+ i, &bits);
+ if (res < 0) {
+ dev_err(card->dev,
+ "Failed to read \"%s\" #%d of \"%s\"\n",
+ "state-input-bits", i, funcnp->name);
+ goto out;
+ }
+
+ res = of_property_read_u32_index(funcnp,
+ "state-input-defaults", i, &def);
+ if (res < 0) {
+ dev_err(card->dev,
+ "Failed to read \"%s\" #%d of \"%s\"\n",
+ "state-input-defaults", i,
+ funcnp->name);
+ goto out;
+ }
+
+ if (bitpos + bits >= 32) {
+ dev_err(card->dev,
+ "Too many events for bitstate \"%s\"\n",
+ funcnp->name);
+ res = -EINVAL;
+ goto out;
+ }
+
+ res = apq8039_state_input_get(card, state_input);
+ if (res < 0)
+ res = (int)def;
+
+ dev_dbg(card->dev, "@%s state_input \"%s\" -> %d\n",
+ __func__, state_input, res);
+
+ state |= (res & ((1 << bits) - 1)) << bitpos;
+
+ bitpos += bits;
+ }
+
+ /* return the state */
+ res = state;
+
+ dev_dbg(card->dev, "@%s state 0x%x\n", __func__, state);
+
+out:
+ return res;
+}
+
+static struct device_node *
+apq8039_get_cooked_function(struct snd_soc_card *card,
+ struct device_node *funcnp)
+{
+ struct device_node *cooked_funcnp = NULL;
+ int res, i, count, state;
+ struct of_phandle_args ofargs;
+
+ /* if this is not a cooked function, return self (with a ref) */
+ if (!apq8039_function_is_cooked(card, funcnp))
+ return of_node_get(funcnp);
+
+ /* get synthetic state from state inputs */
+ state = apq8039_state_get(card, funcnp);
+
+ /* must exist and be a multiple of 3 */
+ res = of_property_count_elems_of_size(funcnp, "state-map",
+ sizeof(u32));
+ if (res <= 0 || (res % 3)) {
+ dev_err(card->dev, "%s %s property of \"%s\" node\n",
+ res < 0 ? "missing" : "invalid",
+ "state-map", funcnp->name);
+ goto out;
+ }
+ count = res / 3;
+
+ memset(&ofargs, 0, sizeof(ofargs));
+
+ /* look for a match for the state in the map */
+ for (i = 0; i < count; i++) {
+ res = of_parse_phandle_with_fixed_args(funcnp, "state-map",
+ 2, i, &ofargs);
+ if (res < 0) {
+ dev_err(card->dev,
+ "failed to parse %s prop \"%s\" #%d\n",
+ "state-map", funcnp->name, i);
+ goto out;
+ }
+ /* match */
+ if (((uint32_t)state & ofargs.args[0]) == ofargs.args[1])
+ break;
+ of_node_put(ofargs.np);
+ }
+ if (i >= count) {
+ dev_err(card->dev,
+ "failed to match state %d %s prop of \"%s\"\n",
+ state, "state-map", funcnp->name);
+ goto out;
+ }
+
+ dev_dbg(card->dev, "@%s function \"%s\" state 0x%x -> \"%s\"\n",
+ __func__, funcnp->name, state, ofargs.np->name);
+ cooked_funcnp = ofargs.np;
+
+out:
+ return cooked_funcnp;
+}
+
+static int apq8039_apply_function_sequence(struct snd_soc_card *card,
+ struct device_node *funcnp,
+ const char *kind)
+{
+ int res = -EINVAL, i, count;
+ const char *ctl, *val;
+
+ /* if the property does not exist, it's no problem */
+ if (!of_get_property(funcnp, kind, NULL)) {
+ res = 0;
+ goto out;
+ }
+
+ /* number of strings (must be pairs) */
+ res = of_property_count_strings(funcnp, kind);
+ if (res < 0 || (res % 2) != 0) {
+ dev_err(card->dev, "Illegal string list prop \"%s\"\n",
+ kind);
+ goto out;
+ }
+
+ for (i = 0, count = res; i < count; i += 2) {
+
+ res = of_property_read_string_index(funcnp, kind, i, &ctl);
+ if (res < 0) {
+ dev_err(card->dev,
+ "Bad string list prop ctl \"%s\" @%d\n",
+ kind, i);
+ goto out;
+ }
+
+ res = of_property_read_string_index(funcnp, kind, i + 1, &val);
+ if (res < 0) {
+ dev_err(card->dev,
+ "Bad string list prop val \"%s\" @%d\n",
+ kind, i + 1);
+ goto out;
+ }
+
+ res = apq8039_ctl_set(card, ctl, val);
+ if (res < 0) {
+ dev_err(card->dev,
+ "ctl_set() failed at \"%s\" = \"%s\"\n",
+ ctl, val);
+ goto out;
+ }
+ }
+
+ res = 0;
+
+out:
+ return res;
+}
+
+static void apq8039_select_function(struct snd_soc_card *card,
+ struct device_node *new_funcnp)
+{
+ struct apq8039_data *pdata = snd_soc_card_get_drvdata(card);
+ struct device_node *old_funcnp, *old_cooked_funcnp, *new_cooked_funcnp;
+
+ if (!new_funcnp)
+ return;
+
+ /* get final old function */
+ old_funcnp = pdata->function_node;
+ old_cooked_funcnp = pdata->cooked_function_node;
+
+ new_cooked_funcnp = apq8039_get_cooked_function(card, new_funcnp);
+
+ dev_dbg(card->dev, "function_node \"%s\" -> \"%s\"\r\n",
+ old_funcnp ? old_funcnp->name : "NULL",
+ new_funcnp->name);
+ dev_dbg(card->dev, "cooked function_node \"%s\" -> \"%s\"\r\n",
+ old_cooked_funcnp ? old_cooked_funcnp->name : "NULL",
+ new_cooked_funcnp->name);
+
+ /* same? don't bother */
+ if (old_cooked_funcnp == new_cooked_funcnp)
+ return;
+
+ if (pdata->cooked_function_node)
+ apq8039_apply_function_sequence(card,
+ pdata->cooked_function_node, "disable");
+
+ of_node_put(pdata->function_node);
+ of_node_put(pdata->cooked_function_node);
+
+ pdata->function_node = of_node_get(new_funcnp);
+ pdata->cooked_function_node = new_cooked_funcnp;
+
+ apq8039_apply_function_sequence(card,
+ pdata->cooked_function_node, "enable");
+
+ apq8039_ctl_commit(card);
+}
+
+static int apq8039_state_input_set(struct snd_soc_card *card,
+ const char *name, int value)
+{
+ struct apq8039_data *pdata = snd_soc_card_get_drvdata(card);
+ struct state_input_node *n;
+
+ list_for_each_entry(n, &pdata->state_input_list, node) {
+ /* if it exists, just update with new value */
+ if (!strcmp(name, n->name)) {
+ /* same state */
+ if (value == n->state_input)
+ return 0;
+ /* return 1 on update, 0, on no change */
+ n->state_input = value;
+ dev_dbg(card->dev, "update state-input \"%s\" %d\r\n",
+ n->name, n->state_input);
+ return 1;
+ }
+ }
+ n = devm_kzalloc(card->dev, sizeof(*n), GFP_KERNEL);
+ if (!n)
+ return -ENOMEM;
+ n->name = devm_kstrdup(card->dev, name, GFP_KERNEL);
+ if (!n->name) {
+ devm_kfree(card->dev, n);
+ return -ENOMEM;
+ }
+ n->state_input = value;
+ list_add_tail(&n->node, &pdata->state_input_list);
+
+ dev_dbg(card->dev, "new state-input \"%s\" %d\r\n",
+ n->name, n->state_input);
+
+ return 1;
+}
+
+static int apq8039_get_function(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol);
+static int apq8039_set_function(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol);
+
+static int apq8039_function_parse_of(struct snd_soc_card *card,
+ struct apq8039_data *pdata)
+{
+ static const char *root_name = "function-definition";
+ struct device_node *rootnp, *np;
+ int res = -EINVAL, i = 0, j, count;
+ const char *str;
+
+ if (!card || !card->dev || !card->dev->of_node)
+ return -EINVAL;
+
+ rootnp = of_get_child_by_name(card->dev->of_node, root_name);
+ if (!rootnp) {
+ dev_err(card->dev, "No \"%s\" child node\n", root_name);
+ return -ENOENT;
+ }
+
+ res = of_property_count_elems_of_size(rootnp, "function-list",
+ sizeof(u32));
+ if (res <= 0) {
+ dev_err(card->dev, "%s %s property of \"%s\" node\n",
+ res < 0 ? "missing" : "invalid",
+ root_name, "function-list");
+ goto err_out;
+ }
+ count = res;
+
+ pdata->num_functions = count;
+ pdata->function_nodes = devm_kzalloc(card->dev,
+ sizeof(*pdata->function_nodes) * count,
+ GFP_KERNEL);
+ if (!pdata->function_nodes) {
+ dev_err(card->dev, "out of memory allocating function_nodes\n");
+ res = -ENOMEM;
+ goto err_out;
+ }
+
+ pdata->function_names = devm_kzalloc(card->dev,
+ sizeof(*pdata->function_names) *
+ pdata->num_functions,
+ GFP_KERNEL);
+ if (!pdata->function_names) {
+ dev_err(card->dev, "out of memory allocating function_names\n");
+ res = -ENOMEM;
+ goto err_out;
+ }
+
+
+ for (i = 0; i < pdata->num_functions; i++) {
+ np = of_parse_phandle(rootnp, "function-list", i);
+ if (!np) {
+ dev_err(card->dev,
+ "failed to parse \"%s\" phandle #%d\n",
+ "function-list", i);
+ res = -ENOENT;
+ goto err_out;
+ }
+ pdata->function_nodes[i] = np;
+ }
+
+ /* get the default, init, shutdown state(s) */
+ for (j = 0; j < 3; j++) {
+ np = of_parse_phandle(rootnp, "system-list", j);
+
+ /* init and shutdown are optional */
+ if (j > 0 && !np)
+ break;
+
+ if (!np) {
+ dev_err(card->dev,
+ "failed to parse \"%s\" phandle #%d\n",
+ "system-list", j);
+ res = -ENOENT;
+ goto err_out;
+ }
+ /* and verify it's one of the ones we declared */
+ for (i = 0; i < pdata->num_functions; i++) {
+ if (np == pdata->function_nodes[i])
+ break;
+ }
+ if (i >= pdata->num_functions) {
+ dev_err(card->dev,
+ "\"%s\" is not part of \"%s\"\n",
+ "system-list", "function-list");
+ res = -EINVAL;
+ of_node_put(np);
+ goto err_out;
+ }
+ switch (j) {
+ case 0:
+ pdata->function_node = np;
+ break;
+ case 1:
+ pdata->init_node = np;
+ break;
+ case 2:
+ pdata->shutdown_node = np;
+ break;
+ }
+ }
+
+ /* collect the function names */
+ for (i = 0; i < pdata->num_functions; i++)
+ pdata->function_names[i] = pdata->function_nodes[i]->name;
+
+ res = of_property_read_string(rootnp, "mixer-control", &str);
+ if (res) {
+ dev_err(card->dev, "failed to read string prop \"%s\"\n",
+ "mixer-control");
+ goto err_out;
+ }
+ pdata->control_name = devm_kstrdup(card->dev, str, GFP_KERNEL);
+ if (!pdata->control_name) {
+ res = -ENOMEM;
+ goto err_out;
+ }
+
+ pdata->func_enum[0] = (const struct soc_enum)
+ SOC_ENUM_SINGLE_EXT(pdata->num_functions,
+ pdata->function_names);
+ pdata->func_control[0] = (const struct snd_kcontrol_new)
+ SOC_ENUM_EXT(pdata->control_name,
+ pdata->func_enum[0],
+ apq8039_get_function,
+ apq8039_set_function);
+
+ /* note! cooked_function_node must be set */
+
+ of_node_put(rootnp);
+ return 0;
+
+err_out:
+ if (pdata->function_nodes) {
+ for (i = pdata->num_functions - 1; i >= 0; i--)
+ of_node_put(pdata->function_nodes[i]);
+ devm_kfree(card->dev, pdata->function_nodes);
+ }
+ pdata->function_nodes = NULL;
+ if (pdata->function_names)
+ devm_kfree(card->dev, pdata->function_names);
+ pdata->function_names = NULL;
+
+ of_node_put(pdata->function_node);
+ of_node_put(pdata->cooked_function_node);
+ of_node_put(pdata->init_node);
+ of_node_put(pdata->shutdown_node);
+
+ of_node_put(rootnp);
+ return res;
+}
+
+static int apq8039_jack_event(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct snd_soc_jack *jack = (struct snd_soc_jack *)data;
+ struct snd_soc_card *card = jack->card;
+ struct apq8039_data *pdata = snd_soc_card_get_drvdata(card);
+ int headphone, microphone;
+ bool modified;
+
+ headphone = (event & SND_JACK_HEADPHONE) == SND_JACK_HEADPHONE;
+ microphone = (event & SND_JACK_MICROPHONE) == SND_JACK_MICROPHONE;
+
+ apq8039_state_input_set(card, "JACK_HEADPHONE", headphone);
+ apq8039_state_input_set(card, "JACK_MICROPHONE", microphone);
+
+ modified = false;
+ if (pdata->headphone_det != headphone) {
+ pdata->headphone_det = headphone;
+ modified = true;
+ }
+ if (pdata->microphone_det != microphone) {
+ pdata->microphone_det = headphone;
+ modified = true;
+ }
+
+ if (!modified)
+ return 0;
+
+ apq8039_select_function(card, pdata->function_node);
+
+ return 0;
+}
+
+static struct notifier_block apq8039_jack_nb = {
+ .notifier_call = apq8039_jack_event,
+};
+
+#define MIC_CTRL_TER_WS_SLAVE_SEL BIT(21)
+#define MIC_CTRL_QUA_WS_SLAVE_SEL_10 BIT(17)
+#define MIC_CTRL_TLMM_SCLK_EN BIT(1)
+#define SPKR_CTL_PRI_WS_SLAVE_SEL_11 (BIT(17) | BIT(16))
+#define DEFAULT_MCLK_RATE 9600000
+
+static int apq8039_dai_init(struct snd_soc_pcm_runtime *rtd)
+{
+ struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+ struct snd_soc_dai *codec_dai;
+ struct snd_soc_component *component;
+ struct snd_soc_card *card = rtd->card;
+ struct apq8039_data *pdata = snd_soc_card_get_drvdata(card);
+ int i, rval;
+
+ switch (cpu_dai->id) {
+ case MI2S_PRIMARY:
+ writel(readl(pdata->spkr_iomux) | SPKR_CTL_PRI_WS_SLAVE_SEL_11,
+ pdata->spkr_iomux);
+ break;
+
+ case MI2S_QUATERNARY:
+ writel(readl(pdata->mic_iomux) | MIC_CTRL_QUA_WS_SLAVE_SEL_10 |
+ MIC_CTRL_TLMM_SCLK_EN,
+ pdata->mic_iomux);
+ break;
+ case MI2S_TERTIARY:
+ writel(readl(pdata->mic_iomux) | MIC_CTRL_TER_WS_SLAVE_SEL |
+ MIC_CTRL_TLMM_SCLK_EN,
+ pdata->mic_iomux);
+
+ break;
+
+ default:
+ dev_err(card->dev, "unsupported cpu dai configuration\n");
+ return -EINVAL;
+
+ }
+
+ if (!pdata->jack_setup) {
+ struct snd_jack *jack;
+
+ rval = snd_soc_card_jack_new(card, "Headset Jack",
+ SND_JACK_HEADSET |
+ SND_JACK_HEADPHONE |
+ SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+ SND_JACK_BTN_2 | SND_JACK_BTN_3 |
+ SND_JACK_BTN_4,
+ &pdata->jack, NULL, 0);
+
+ if (rval < 0) {
+ dev_err(card->dev, "Unable to add Headphone Jack\n");
+ return rval;
+ }
+
+ jack = pdata->jack.jack;
+
+ snd_jack_set_key(jack, SND_JACK_BTN_0, KEY_PLAYPAUSE);
+ 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);
+ pdata->jack_setup = true;
+
+ snd_soc_jack_notifier_register(&pdata->jack,
+ &apq8039_jack_nb);
+ }
+
+ for_each_rtd_codec_dais(rtd, i, codec_dai) {
+
+ component = codec_dai->component;
+ /* Set default mclk for internal codec */
+ rval = snd_soc_component_set_sysclk(component, 0, 0,
+ DEFAULT_MCLK_RATE, SND_SOC_CLOCK_IN);
+ if (rval != 0 && rval != -ENOTSUPP) {
+ dev_warn(card->dev, "Failed to set mclk: %d\n", rval);
+ return rval;
+ }
+ rval = snd_soc_component_set_jack(component,
+ &pdata->jack, NULL);
+ if (rval != 0 && rval != -ENOTSUPP) {
+ dev_warn(card->dev, "Failed to set jack: %d\n", rval);
+ return rval;
+ }
+ }
+
+ return 0;
+}
+
+static struct apq8039_data *apq8039_parse_of(struct snd_soc_card *card)
+{
+ struct device *dev = card->dev;
+ struct snd_soc_dai_link *link;
+ struct device_node *np, *codec, *cpu, *node = dev->of_node;
+ struct apq8039_data *data;
+ struct snd_soc_dai_link_component *dlc;
+ int ret, num_links;
+
+ ret = snd_soc_of_parse_card_name(card, "qcom,model");
+ if (ret) {
+ dev_err(dev, "Error parsing card name: %d\n", ret);
+ return ERR_PTR(ret);
+ }
+
+ /* 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 ERR_PTR(ret);
+ }
+
+ /* count the number of dai links */
+ num_links = 0;
+ for_each_child_of_node(node, np) {
+ cpu = of_get_child_by_name(np, "cpu");
+ codec = of_get_child_by_name(np, "codec");
+
+ /* it's a dai link only if both cpu & codec nodes exist */
+ num_links += cpu && codec;
+
+ /* handles NULLs just fine */
+ of_node_put(cpu);
+ of_node_put(codec);
+ }
+
+ /* Allocate the private data and the DAI link array */
+ data = devm_kzalloc(dev,
+ struct_size(data, dai_link, num_links),
+ GFP_KERNEL);
+ if (!data)
+ return ERR_PTR(-ENOMEM);
+
+ card->dai_link = &data->dai_link[0];
+ card->num_links = num_links;
+
+ link = data->dai_link;
+
+ for_each_child_of_node(node, np) {
+ cpu = of_get_child_by_name(np, "cpu");
+ codec = of_get_child_by_name(np, "codec");
+
+ if (!cpu || !codec) {
+ /* not an error, just not a dai node */
+ of_node_put(cpu);
+ of_node_put(codec);
+ continue;
+ }
+
+ dlc = devm_kzalloc(dev, 2 * sizeof(*dlc), GFP_KERNEL);
+ if (!dlc)
+ return ERR_PTR(-ENOMEM);
+
+ link->cpus = &dlc[0];
+ link->platforms = &dlc[1];
+
+ link->num_cpus = 1;
+ link->num_platforms = 1;
+
+ link->cpus->of_node = of_parse_phandle(cpu, "sound-dai", 0);
+ if (!link->cpus->of_node) {
+ dev_err(card->dev,
+ "error getting cpu phandle (%s)\n",
+ of_node_full_name(cpu));
+ ret = -EINVAL;
+ goto error;
+ }
+
+ ret = snd_soc_of_get_dai_name(cpu, &link->cpus->dai_name);
+ if (ret) {
+ dev_err(card->dev,
+ "error getting cpu dai name (%s)\n",
+ of_node_full_name(cpu));
+ goto error;
+ }
+
+ ret = snd_soc_of_get_dai_link_codecs(dev, codec, link);
+ if (ret < 0) {
+ dev_err(card->dev,
+ "error getting codec dai name (%s)\n",
+ of_node_full_name(codec));
+ goto error;
+ }
+
+ link->platforms->of_node = link->cpus->of_node;
+ ret = of_property_read_string(np, "link-name", &link->name);
+ if (ret) {
+ dev_err(card->dev, "error getting dai_link name\n");
+ goto error;
+ }
+
+ link->stream_name = link->name;
+ link->init = apq8039_dai_init;
+ link++;
+
+ of_node_put(cpu);
+ of_node_put(codec);
+ }
+ cpu = codec = NULL;
+
+ ret = apq8039_function_parse_of(card, data);
+ if (ret != 0)
+ goto error;
+
+ return data;
+
+ error:
+ of_node_put(np);
+ of_node_put(cpu);
+ of_node_put(codec);
+ return ERR_PTR(ret);
+}
+
+static int apq8039_get_function(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_card *card = snd_kcontrol_chip(kcontrol);
+ struct apq8039_data *pdata = snd_soc_card_get_drvdata(card);
+ int i;
+
+ for (i = 0; i < pdata->num_functions; i++) {
+ if (pdata->function_node == pdata->function_nodes[i]) {
+ ucontrol->value.enumerated.item[0] = i;
+ return 0;
+ }
+ }
+ return -EINVAL;
+}
+
+static int apq8039_set_function(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_soc_card *card = snd_kcontrol_chip(kcontrol);
+ struct apq8039_data *pdata = snd_soc_card_get_drvdata(card);
+ unsigned int target;
+
+ if (ucontrol->value.enumerated.item[0] >= pdata->num_functions)
+ return -EINVAL;
+
+ target = ucontrol->value.enumerated.item[0];
+ apq8039_select_function(card, pdata->function_nodes[target]);
+
+ return 0;
+}
+
+static int apq8039_platform_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct snd_soc_card *card;
+ struct apq8039_data *data;
+ struct resource *res;
+ int ret;
+
+ card = devm_kzalloc(dev, sizeof(*card), GFP_KERNEL);
+ if (!card)
+ return -ENOMEM;
+
+ card->dev = dev;
+
+ data = apq8039_parse_of(card);
+ if (IS_ERR(data)) {
+ dev_err(&pdev->dev, "Error resolving dai links: %ld\n",
+ PTR_ERR(data));
+ return PTR_ERR(data);
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mic-iomux");
+ data->mic_iomux = devm_ioremap_resource(dev, res);
+ if (IS_ERR(data->mic_iomux))
+ return PTR_ERR(data->mic_iomux);
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "spkr-iomux");
+ data->spkr_iomux = devm_ioremap_resource(dev, res);
+ if (IS_ERR(data->spkr_iomux))
+ return PTR_ERR(data->spkr_iomux);
+
+ data->headphone_det = -1;
+ data->microphone_det = -1;
+
+ INIT_LIST_HEAD(&data->ctl_list);
+ INIT_LIST_HEAD(&data->state_input_list);
+
+ card->controls = data->func_control;
+ card->num_controls = ARRAY_SIZE(data->func_control);
+
+ snd_soc_card_set_drvdata(card, data);
+
+ ret = devm_snd_soc_register_card(&pdev->dev, card);
+ if (ret)
+ return ret;
+
+ /* initialize first */
+ if (data->init_node)
+ apq8039_select_function(card, data->init_node);
+
+ /* select now */
+ if (data->function_node != data->init_node)
+ apq8039_select_function(card, data->function_node);
+
+ return 0;
+}
+
+static int apq8039_platform_remove(struct platform_device *pdev)
+{
+ struct snd_soc_card *card = dev_get_drvdata(&pdev->dev);
+ struct apq8039_data *pdata = snd_soc_card_get_drvdata(card);
+ int i;
+
+ /* the lists are using managed memory, no need to free them */
+ if (pdata) {
+
+ if (pdata->shutdown_node)
+ apq8039_select_function(card, pdata->shutdown_node);
+
+ of_node_put(pdata->function_node);
+ of_node_put(pdata->cooked_function_node);
+ of_node_put(pdata->init_node);
+ of_node_put(pdata->shutdown_node);
+ if (pdata->function_nodes) {
+ for (i = 0; i < pdata->num_functions; i++)
+ of_node_put(pdata->function_nodes[i]);
+ }
+ }
+
+ snd_soc_unregister_card(card);
+
+ return 0;
+}
+
+static const struct of_device_id apq8039_device_id[] = {
+ { .compatible = "qcom,apq8039-sndcard" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, apq8039_device_id);
+
+static struct platform_driver apq8039_platform_driver = {
+ .driver = {
+ .name = "qcom-apq8039",
+ .of_match_table = of_match_ptr(apq8039_device_id),
+ },
+ .probe = apq8039_platform_probe,
+ .remove = apq8039_platform_remove,
+};
+module_platform_driver(apq8039_platform_driver);
+
+MODULE_AUTHOR("Pantelis Antoniou <pantelis.antoniou@linaro.org");
+MODULE_DESCRIPTION("APQ8039 ASoC Machine Driver");
+MODULE_LICENSE("GPL v2");
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex
2020-06-19 19:38 ` [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex Pantelis Antoniou
@ 2020-06-19 21:41 ` Stephan Gerhold
2020-06-22 11:34 ` Pantelis Antoniou
2020-06-22 11:51 ` Mark Brown
0 siblings, 2 replies; 13+ messages in thread
From: Stephan Gerhold @ 2020-06-19 21:41 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: alsa-devel, devicetree, linux-arm-msm, Srinivas Kandagatla,
Mark Brown, Matt Porter, Shawn Guo
Hi Pantelis,
On Fri, Jun 19, 2020 at 10:38:30PM +0300, Pantelis Antoniou wrote:
> Add a yaml device binding for the QCOM apq8039 sound complex driver.
>
Nice to see some activity to get sound working on another SoC!
Thanks for documenting all these properties.
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@linaro.org>
> ---
> .../bindings/sound/qcom,apq8039.yaml | 370 ++++++++++++++++++
> 1 file changed, 370 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/qcom,apq8039.yaml
>
> diff --git a/Documentation/devicetree/bindings/sound/qcom,apq8039.yaml b/Documentation/devicetree/bindings/sound/qcom,apq8039.yaml
> new file mode 100644
> index 000000000000..f1c4fb99ccbb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,apq8039.yaml
> @@ -0,0 +1,370 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/qcom,apq8039.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies APQ8039 ASoC sound card
> +
> +maintainers:
> + - Pantelis Antoniou <pantelis.antoniou@linaro.org>
> +
> +properties:
> + compatible:
> + items:
> + - const: qcom,apq8039-sndcard
> +
> + pinctrl-0:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: |
> + Should specify pin control groups used for this controller matching
> + the first entry in pinctrl-names.
> +
> + pinctrl-1:
> + description: |
> + Should specify pin control groups used for this controller matching
> + the second entry in pinctrl-names.
> +
> + pinctrl-names:
> + minItems: 1
> + items:
> + - const: default
> + - const: sleep
> + description:
> + Names for the pin configuration(s); may be "default" or "sleep",
> + where the "sleep" configuration may describe the state
> + the pins should be in during system suspend.
> +
> + reg:
> + description: Must contain an address for each entry in "reg-names".
> + minItems: 2
> + maxItems: 2
> +
> + reg-names:
> + items:
> + - const: mic-iomux
> + - const: spkr-iomux
> +
> + qcom,model:
> + $ref: /schemas/types.yaml#/definitions/string
> + description: The user-visible name of the sound complex.
> +
> + qcom,audio-routing:
> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> + description: |
> + 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
> + and MicBias of msm8916-analoc-wcd codec.
> +
> + function-definition:
> + type: object
> + description: |
> + Functional configuration for the sound complex via a
> + simple control. allows fixed and dynamically constructed
> + function selection.
> +
> + properties:
> + mixer-control:
> + $ref: /schemas/types.yaml#/definitions/string
> + description: |
> + Name of the exported alsa mix control.
> +
> + function-list:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: |
> + phandle(s) of the functions which the sound complex
> + exposes via the control.
> +
> + system-list:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: |
> + phandle(s) of the default, init and shutdown functions
> + Must be one of the declared ones in the function property.
> + The default function is the one selected by default on
> + startup (after the init function's sequence is executed).
> + On shutdown the shutdown function sequence will be executed.
> + Typically init and shutdown are the same and it's purpose
> + is to initialize the sound complex mixer controls to the
> + all off state, and be ready for a regular function selection.
> +
> + patternProperties:
> + "^[A-Za-z_][A-Aa-z0-9_]*$":
> + type: object
> + description:
> + Function description subnodes. The name of the function
> + is simply the name of the subnode, so restrictions apply
> + to the valid node names.
> +
> + The function definition of each subnode is either a cooked
> + function (i.e. which is not dependent on state inputs), or
> + a function that is selecting a cooked function based on the
> + state inputs and the generated state vector.
> +
> + oneOf:
> + # non-cooked function
> + - properties:
> + enable:
> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> + description: |
> + Sequence of alsa mixer controls to apply when this state is to
> + be enabled.
> +
> + disable:
> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> + description: |
> + Sequence of alsa mixer controls to apply when this state is to
> + be disabled.
> +
> + required:
> + - enable
> +
> + # cooked function
> + - properties:
> + state-inputs:
> + description: |
> + A list of state inputs to be used in constructing a state
> + vector.
> + type: array
> + uniqueItems: true
> + minItems: 1
> + items:
> + anyOf:
> + - const: JACK_HEADPHONE
> + - const: JACK_MICROPHONE
> +
> + state-input-bits:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + Number of bits to use for each state-input in the
> + state vector creation. For now only the value 1 is
> + supported for JACK_HEADPHONE and JACK_MICROPHONE.
> +
> + state-input-defaults:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + The default value to use as a state input at startup.
> +
> + state-map:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: |
> + The mapping of this function to a cooked function. The
> + format used is a sequence of phandle to a state, the mask
> + to apply to the state vector, and the equality value.
> +
> + Take the example's configuration
> +
> + state-inputs = "JACK_HEADPHONE", "JACK_MICROPHONE";
> + state-input-bits = <1>, <1>;
> + state-input-defaults = <0>, <0>;
> +
> + state-map = <&speaker 0x1 0x0>,
> + <&headphones 0x3 0x1>,
> + <&headset 0x3 0x3>;
> +
> + is decoded as follows.
> +
> + There are 3 possible cooked functions to be selected.
> + speaker, headphone and headset. The state-inputs are
> + the JACK_HEADPHONE and JACK_MICROPHONE, which are single
> + bit values, being placed at bit 0 and bit 1 of the
> + constructed vector.
> +
> + The 4 possible state vectors are:
> + MICROPHONE=0, HEADPHONE=0, 0
> + MICROPHONE=0, HEADPHONE=1, 1
> + MICROPHONE=1, HEADPHONE=0, 2
> + MICROPHONE=1, HEADPHONE=1, 3
> +
> + The speaker function is selected when HEADPHONE=0 because
> + both (0 & 1) == (2 & 1) == 0.
> +
> + The headphones function is selected when HEADPHONE=1 and
> + MICROPHONE=0 because (1 & 3) == 1.
> +
> + The headset function is selected when both HEADPHONE=1 and
> + MICROPHONE=1 because (3 & 3) == 3.
> +
> + required:
> + - state-inputs
> + - state-input-bits
> + - state-input-defaults
> + - state-map
> +
> +patternProperties:
> + "^.*dai-link-[0-9]+$":
> + type: object
> + description: |-
> + cpu and codec child nodes:
> + Container for cpu and codec dai sub-nodes.
> + One cpu and one codec sub-node must exist.
> +
> + properties:
> + link-name:
> + description: The link name
> +
> + cpu:
> + type: object
> + properties:
> +
> + sound-dai:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: phandle(s) of the CPU DAI(s)
> +
> + required:
> + - sound-dai
> +
> + codec:
> + type: object
> + properties:
> +
> + sound-dai:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + description: phandle(s) of the codec DAI(s)
> +
> + required:
> + - sound-dai
> +
> + required:
> + - link-name
> + - cpu
> + - codec
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> + - qcom,model
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/sound/apq8016-lpass.h>
> +
> + sound: sound@7702000 {
> + compatible = "qcom,apq8039-sndcard";
> + reg = <0x07702000 0x4>, <0x07702004 0x4>;
> + reg-names = "mic-iomux", "spkr-iomux";
> +
> + status = "okay";
> + pinctrl-0 = <&cdc_pdm_lines_act>;
> + pinctrl-1 = <&cdc_pdm_lines_sus>;
> + pinctrl-names = "default", "sleep";
> + qcom,model = "APQ8039";
> + qcom,audio-routing = "AMIC2", "MIC BIAS Internal2";
> +
> + internal-codec-playback-dai-link-0 {
> + link-name = "WCD";
> + cpu {
> + sound-dai = <&lpass MI2S_PRIMARY>;
> + };
> + codec {
> + sound-dai = <&lpass_codec 0>, <&wcd_codec 0>;
> + };
> + };
> +
> + internal-codec-capture-dai-link-0 {
> + link-name = "WCD-Capture";
> + cpu {
> + sound-dai = <&lpass MI2S_TERTIARY>;
> + };
> + codec {
> + sound-dai = <&lpass_codec 1>, <&wcd_codec 1>;
> + };
> + };
> +
> + function-definition {
> +
> + mixer-control = "Jack Function";
> + function-list = <&auto &headphones &headset &speaker &off>;
> + system-list = <&auto &off &off>; // default, init, shutdown
> +
> + auto: Automatic {
> + // Headphone presence bit 0 (1) - H
> + // Microphone presence bit 1 (2) - M
> + state-inputs = "JACK_HEADPHONE", "JACK_MICROPHONE";
> + state-input-bits = <1>, <1>;
> + state-input-defaults = <0>, <0>;
> +
> + // HM & MASK
> + state-map =
> + <&speaker 0x1 0x0>, // no headphone -> speaker
> + <&headphones 0x3 0x1>, // headphone but no mic -> headphones
> + <&headset 0x3 0x3>; // headphone & mic -> headset
> + };
> + headphones: Headphones {
> + enable =
> + "RX1 MIX1 INP1", "RX1",
> + "RX2 MIX1 INP1", "RX2",
> + "RDAC2 MUX", "RX2",
> + "RX1 Digital Volume", "128",
> + "RX2 Digital Volume", "128",
> + "HPHL", "Switch",
> + "HPHR", "Switch";
> +
> + disable =
> + "RX1 Digital Volume", "0",
> + "RX2 Digital Volume", "0",
> + "HPHL", "ZERO",
> + "HPHR", "ZERO",
> + "RDAC2 MUX", "RX1",
> + "RX1 MIX1 INP1", "ZERO",
> + "RX2 MIX1 INP1", "ZERO";
> + };
> + headset: Headset {
> + enable =
> + "RX1 MIX1 INP1", "RX1",
> + "RX2 MIX1 INP1", "RX2",
> + "RDAC2 MUX", "RX2",
> + "RX1 Digital Volume", "128",
> + "RX2 Digital Volume", "128",
> + "DEC1 MUX", "ADC2",
> + "CIC1 MUX", "AMIC",
> + "ADC2 Volume", "8",
> + "ADC2 MUX", "INP2",
> + "HPHL", "Switch",
> + "HPHR", "Switch";
> +
> + disable =
> + "RX1 Digital Volume", "0",
> + "RX2 Digital Volume", "0",
> + "HPHL", "ZERO",
> + "HPHR", "ZERO",
> + "RDAC2 MUX", "RX1",
> + "RX1 MIX1 INP1", "ZERO",
> + "RX2 MIX1 INP1", "ZERO",
> + "ADC2 MUX", "ZERO",
> + "ADC2 Volume", "0",
> + "DEC1 MUX", "ZERO";
> + };
> + speaker: Speaker {
> + enable =
> + "SPK DAC Switch", "1",
> + "RX3 MIX1 INP1", "RX1",
> + "RX3 MIX1 INP2", "RX2",
> + "RX3 Digital Volume", "128";
> +
> + disable =
> + "SPK DAC Switch", "0",
> + "RX3 MIX1 INP1", "ZERO",
> + "RX3 MIX1 INP2", "ZERO";
> + };
> + off: Off {
> + enable =
> + "RX1 Digital Volume", "0",
> + "RX2 Digital Volume", "0",
> + "HPHL", "ZERO",
> + "HPHR", "ZERO",
> + "RDAC2 MUX", "RX1",
> + "RX1 MIX1 INP1", "ZERO",
> + "RX2 MIX1 INP1", "ZERO",
> + "ADC2 MUX", "ZERO",
> + "ADC2 Volume", "0",
> + "DEC1 MUX", "ZERO",
> + "SPK DAC Switch", "0",
> + "RX3 MIX1 INP1", "ZERO",
> + "RX3 MIX1 INP2", "ZERO";
> + };
> + };
This looks much like a replacement for ALSA UCM and userspace audio jack
detection coded into the device tree.
While I personally think this is an interesting idea
(We have the device tree to describe the hardware, why can we not also
describe necessary audio routing to enable a particular output?)
this is also not really specific to the APQ8039 hardware, is it?
In fact, without all the code to handle the mixer enable/disable
sequences the machine driver looks almost identical to the existing
apq8016-sbc.
If you want to discuss ways to integrate mixer enable/disable sequences
into the device tree, I suggest that you post your ideas separately as
[RFC] with a more generic subject. That will make it more easy for
everyone interested to share their thoughts.
Right now it's quite hidden in a patch set where the subjects suggest
that it's just a simple machine driver to glue some codecs together.
Thanks,
Stephan
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] ASoC: qcom: add apq8039 sound card support
2020-06-19 19:38 ` [PATCH 2/2] ASoC: qcom: add apq8039 sound card support Pantelis Antoniou
@ 2020-06-19 21:58 ` kernel test robot
2020-06-20 6:39 ` kernel test robot
1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2020-06-19 21:58 UTC (permalink / raw)
To: Pantelis Antoniou, alsa-devel
Cc: kbuild-all, devicetree, linux-arm-msm, Srinivas Kandagatla,
Mark Brown, Matt Porter, Shawn Guo
[-- Attachment #1: Type: text/plain, Size: 5810 bytes --]
Hi Pantelis,
I love your patch! Perhaps something to improve:
[auto build test WARNING on asoc/for-next]
[also build test WARNING on sound/for-next v5.8-rc1 next-20200618]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Pantelis-Antoniou/ASoC-qcom-add-apq8039-sound-card-and-bindings/20200620-034022
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: parisc-allyesconfig (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
sound/soc/qcom/apq8039.c: In function 'snd_soc_card_ctl_getset.constprop':
>> sound/soc/qcom/apq8039.c:176:5: warning: 'strncpy' destination unchanged after copying no bytes [-Wstringop-truncation]
176 | strncpy(orig, uinfo->value.enumerated.name,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
177 | origsz);
| ~~~~~~~
vim +/strncpy +176 sound/soc/qcom/apq8039.c
67
68 static int
69 snd_soc_card_ctl_getset(struct snd_soc_card *card,
70 const char *name,
71 char *orig, size_t origsz, const char *value)
72 {
73 struct snd_kcontrol *ctl;
74 struct snd_ctl_elem_info *uinfo = NULL;
75 struct snd_ctl_elem_value *uctl = NULL;
76 unsigned int i, count;
77 int ret;
78
79 /* get the control */
80 ctl = snd_soc_card_ctl_find(card, name, NULL);
81 if (!ctl)
82 return -ENOENT;
83
84 /* allocate info and value */
85 uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
86 uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
87 if (!uinfo || !uctl) {
88 ret = -ENOMEM;
89 goto out_free;
90 }
91
92 ret = ctl->info(ctl, uinfo);
93 if (ret)
94 goto out_free;
95 if (uinfo->count != 1) {
96 ret = -ENOTSUPP;
97 goto out_free;
98 }
99
100 ret = ctl->get(ctl, uctl);
101 if (ret)
102 goto out_free;
103
104 ret = 0;
105
106 switch (uinfo->type) {
107
108 case SNDRV_CTL_ELEM_TYPE_NONE:
109 break;
110
111 case SNDRV_CTL_ELEM_TYPE_BOOLEAN:
112 if (orig && origsz > 0)
113 snprintf(orig, origsz, "%s",
114 uctl->value.integer.value[0] ?
115 "true" : "false");
116
117 if (value) {
118 bool bval;
119
120 ret = kstrtobool(value, &bval);
121 if (ret)
122 goto out_free;
123 uctl->value.integer.value[0] = !!bval;
124
125 }
126 break;
127
128 case SNDRV_CTL_ELEM_TYPE_INTEGER:
129 if (orig && origsz > 0)
130 snprintf(orig, origsz, "%ld",
131 uctl->value.integer.value[0]);
132
133 if (value) {
134 ret = kstrtol(value, 10,
135 &uctl->value.integer.value[0]);
136 if (ret)
137 goto out_free;
138 }
139 break;
140
141 case SNDRV_CTL_ELEM_TYPE_ENUMERATED:
142 count = uinfo->value.enumerated.items;
143
144 if (value) { /* set */
145 for (i = 0; i < count; i++) {
146 uinfo->value.enumerated.item = i;
147 ret = ctl->info(ctl, uinfo);
148 if (ret)
149 goto out_free;
150
151 if (!strcmp(value,
152 uinfo->value.enumerated.name))
153 break;
154 }
155
156 /* setting without finding the enum */
157 if (i >= count) {
158 ret = -EINVAL;
159 goto out_free;
160 }
161 uctl->value.enumerated.item[0] = i;
162 } else { /* get */
163 uinfo->value.enumerated.item =
164 uctl->value.enumerated.item[0];
165 ret = ctl->info(ctl, uinfo);
166 if (ret)
167 goto out_free;
168
169 if (orig && origsz) {
170 if (origsz <
171 strlen(uinfo->value.enumerated.name)
172 + 1) {
173 ret = -ENOSPC;
174 goto out_free;
175 }
> 176 strncpy(orig, uinfo->value.enumerated.name,
177 origsz);
178 }
179 }
180 break;
181
182 case SNDRV_CTL_ELEM_TYPE_INTEGER64:
183 if (orig && origsz > 0)
184 snprintf(orig, origsz, "%lld",
185 uctl->value.integer64.value[0]);
186
187 if (value) {
188 ret = kstrtoll(value, 10,
189 &uctl->value.integer64.value[0]);
190 if (ret)
191 goto out_free;
192 }
193 break;
194
195 case SNDRV_CTL_ELEM_TYPE_BYTES:
196 case SNDRV_CTL_ELEM_TYPE_IEC958:
197 ret = -ENOTSUPP;
198 goto out_free;
199 default:
200 ret = -EINVAL;
201 goto out_free;
202 }
203
204 if (value) {
205 ret = ctl->put(ctl, uctl);
206 if (ret < 0)
207 goto out_free;
208
209 /* if it changed, report change to user space */
210 if (ret > 0)
211 snd_ctl_notify(card->snd_card,
212 SNDRV_CTL_EVENT_MASK_VALUE,
213 &uctl->id);
214 }
215
216 out_free:
217 kfree(uctl);
218 kfree(uinfo);
219
220 if (ret < 0)
221 dev_err(card->dev, "%s: %s operation failed for \"%s\"",
222 __func__, value ? "set" : "get", name);
223
224 return ret;
225 }
226
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 65331 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] ASoC: qcom: add apq8039 sound card support
2020-06-19 19:38 ` [PATCH 2/2] ASoC: qcom: add apq8039 sound card support Pantelis Antoniou
2020-06-19 21:58 ` kernel test robot
@ 2020-06-20 6:39 ` kernel test robot
1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2020-06-20 6:39 UTC (permalink / raw)
To: Pantelis Antoniou, alsa-devel
Cc: kbuild-all, devicetree, linux-arm-msm, Srinivas Kandagatla,
Mark Brown, Matt Porter, Shawn Guo
[-- Attachment #1: Type: text/plain, Size: 4655 bytes --]
Hi Pantelis,
I love your patch! Perhaps something to improve:
[auto build test WARNING on asoc/for-next]
[also build test WARNING on sound/for-next v5.8-rc1 next-20200618]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Pantelis-Antoniou/ASoC-qcom-add-apq8039-sound-card-and-bindings/20200620-034022
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
In file included from arch/x86/include/asm/page_32.h:35,
from arch/x86/include/asm/page.h:14,
from arch/x86/include/asm/thread_info.h:12,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/rcupdate.h:27,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from sound/soc/qcom/apq8039.c:5:
In function 'strncpy',
inlined from 'snd_soc_card_ctl_getset.constprop' at sound/soc/qcom/apq8039.c:176:5:
>> include/linux/string.h:297:30: warning: '__builtin_strncpy' destination unchanged after copying no bytes [-Wstringop-truncation]
297 | #define __underlying_strncpy __builtin_strncpy
| ^
include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'
307 | return __underlying_strncpy(p, q, size);
| ^~~~~~~~~~~~~~~~~~~~
vim +/__builtin_strncpy +297 include/linux/string.h
47227d27e2fcb0 Daniel Axtens 2020-06-03 275
47227d27e2fcb0 Daniel Axtens 2020-06-03 276 #ifdef CONFIG_KASAN
47227d27e2fcb0 Daniel Axtens 2020-06-03 277 extern void *__underlying_memchr(const void *p, int c, __kernel_size_t size) __RENAME(memchr);
47227d27e2fcb0 Daniel Axtens 2020-06-03 278 extern int __underlying_memcmp(const void *p, const void *q, __kernel_size_t size) __RENAME(memcmp);
47227d27e2fcb0 Daniel Axtens 2020-06-03 279 extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(memcpy);
47227d27e2fcb0 Daniel Axtens 2020-06-03 280 extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(memmove);
47227d27e2fcb0 Daniel Axtens 2020-06-03 281 extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(memset);
47227d27e2fcb0 Daniel Axtens 2020-06-03 282 extern char *__underlying_strcat(char *p, const char *q) __RENAME(strcat);
47227d27e2fcb0 Daniel Axtens 2020-06-03 283 extern char *__underlying_strcpy(char *p, const char *q) __RENAME(strcpy);
47227d27e2fcb0 Daniel Axtens 2020-06-03 284 extern __kernel_size_t __underlying_strlen(const char *p) __RENAME(strlen);
47227d27e2fcb0 Daniel Axtens 2020-06-03 285 extern char *__underlying_strncat(char *p, const char *q, __kernel_size_t count) __RENAME(strncat);
47227d27e2fcb0 Daniel Axtens 2020-06-03 286 extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) __RENAME(strncpy);
47227d27e2fcb0 Daniel Axtens 2020-06-03 287 #else
47227d27e2fcb0 Daniel Axtens 2020-06-03 288 #define __underlying_memchr __builtin_memchr
47227d27e2fcb0 Daniel Axtens 2020-06-03 289 #define __underlying_memcmp __builtin_memcmp
47227d27e2fcb0 Daniel Axtens 2020-06-03 290 #define __underlying_memcpy __builtin_memcpy
47227d27e2fcb0 Daniel Axtens 2020-06-03 291 #define __underlying_memmove __builtin_memmove
47227d27e2fcb0 Daniel Axtens 2020-06-03 292 #define __underlying_memset __builtin_memset
47227d27e2fcb0 Daniel Axtens 2020-06-03 293 #define __underlying_strcat __builtin_strcat
47227d27e2fcb0 Daniel Axtens 2020-06-03 294 #define __underlying_strcpy __builtin_strcpy
47227d27e2fcb0 Daniel Axtens 2020-06-03 295 #define __underlying_strlen __builtin_strlen
47227d27e2fcb0 Daniel Axtens 2020-06-03 296 #define __underlying_strncat __builtin_strncat
47227d27e2fcb0 Daniel Axtens 2020-06-03 @297 #define __underlying_strncpy __builtin_strncpy
47227d27e2fcb0 Daniel Axtens 2020-06-03 298 #endif
47227d27e2fcb0 Daniel Axtens 2020-06-03 299
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 74058 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex
2020-06-19 21:41 ` Stephan Gerhold
@ 2020-06-22 11:34 ` Pantelis Antoniou
2020-06-22 12:04 ` Mark Brown
2020-06-22 11:51 ` Mark Brown
1 sibling, 1 reply; 13+ messages in thread
From: Pantelis Antoniou @ 2020-06-22 11:34 UTC (permalink / raw)
To: Stephan Gerhold
Cc: alsa-devel, devicetree, linux-arm-msm, Srinivas Kandagatla,
Mark Brown, Matthew Porter, Shawn Guo
Hi Stephan,
> On Jun 20, 2020, at 00:41 , Stephan Gerhold <stephan@gerhold.net> wrote:
>
> Hi Pantelis,
>
> On Fri, Jun 19, 2020 at 10:38:30PM +0300, Pantelis Antoniou wrote:
>> Add a yaml device binding for the QCOM apq8039 sound complex driver.
>>
>
> Nice to see some activity to get sound working on another SoC!
> Thanks for documenting all these properties.
>
Thanks (I guess :) )
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@linaro.org>
>> ---
>> .../bindings/sound/qcom,apq8039.yaml | 370 ++++++++++++++++++
>> 1 file changed, 370 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/sound/qcom,apq8039.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,apq8039.yaml b/Documentation/devicetree/bindings/sound/qcom,apq8039.yaml
>> new file mode 100644
>> index 000000000000..f1c4fb99ccbb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/qcom,apq8039.yaml
>> @@ -0,0 +1,370 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/sound/qcom,apq8039.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Technologies APQ8039 ASoC sound card
>> +
>> +maintainers:
>> + - Pantelis Antoniou <pantelis.antoniou@linaro.org>
>> +
>> +properties:
>> + compatible:
>> + items:
>> + - const: qcom,apq8039-sndcard
>> +
>> + pinctrl-0:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: |
>> + Should specify pin control groups used for this controller matching
>> + the first entry in pinctrl-names.
>> +
>> + pinctrl-1:
>> + description: |
>> + Should specify pin control groups used for this controller matching
>> + the second entry in pinctrl-names.
>> +
>> + pinctrl-names:
>> + minItems: 1
>> + items:
>> + - const: default
>> + - const: sleep
>> + description:
>> + Names for the pin configuration(s); may be "default" or "sleep",
>> + where the "sleep" configuration may describe the state
>> + the pins should be in during system suspend.
>> +
>> + reg:
>> + description: Must contain an address for each entry in "reg-names".
>> + minItems: 2
>> + maxItems: 2
>> +
>> + reg-names:
>> + items:
>> + - const: mic-iomux
>> + - const: spkr-iomux
>> +
>> + qcom,model:
>> + $ref: /schemas/types.yaml#/definitions/string
>> + description: The user-visible name of the sound complex.
>> +
>> + qcom,audio-routing:
>> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>> + description: |
>> + 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
>> + and MicBias of msm8916-analoc-wcd codec.
>> +
>> + function-definition:
>> + type: object
>> + description: |
>> + Functional configuration for the sound complex via a
>> + simple control. allows fixed and dynamically constructed
>> + function selection.
>> +
>> + properties:
>> + mixer-control:
>> + $ref: /schemas/types.yaml#/definitions/string
>> + description: |
>> + Name of the exported alsa mix control.
>> +
>> + function-list:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: |
>> + phandle(s) of the functions which the sound complex
>> + exposes via the control.
>> +
>> + system-list:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: |
>> + phandle(s) of the default, init and shutdown functions
>> + Must be one of the declared ones in the function property.
>> + The default function is the one selected by default on
>> + startup (after the init function's sequence is executed).
>> + On shutdown the shutdown function sequence will be executed.
>> + Typically init and shutdown are the same and it's purpose
>> + is to initialize the sound complex mixer controls to the
>> + all off state, and be ready for a regular function selection.
>> +
>> + patternProperties:
>> + "^[A-Za-z_][A-Aa-z0-9_]*$":
>> + type: object
>> + description:
>> + Function description subnodes. The name of the function
>> + is simply the name of the subnode, so restrictions apply
>> + to the valid node names.
>> +
>> + The function definition of each subnode is either a cooked
>> + function (i.e. which is not dependent on state inputs), or
>> + a function that is selecting a cooked function based on the
>> + state inputs and the generated state vector.
>> +
>> + oneOf:
>> + # non-cooked function
>> + - properties:
>> + enable:
>> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>> + description: |
>> + Sequence of alsa mixer controls to apply when this state is to
>> + be enabled.
>> +
>> + disable:
>> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
>> + description: |
>> + Sequence of alsa mixer controls to apply when this state is to
>> + be disabled.
>> +
>> + required:
>> + - enable
>> +
>> + # cooked function
>> + - properties:
>> + state-inputs:
>> + description: |
>> + A list of state inputs to be used in constructing a state
>> + vector.
>> + type: array
>> + uniqueItems: true
>> + minItems: 1
>> + items:
>> + anyOf:
>> + - const: JACK_HEADPHONE
>> + - const: JACK_MICROPHONE
>> +
>> + state-input-bits:
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + description: |
>> + Number of bits to use for each state-input in the
>> + state vector creation. For now only the value 1 is
>> + supported for JACK_HEADPHONE and JACK_MICROPHONE.
>> +
>> + state-input-defaults:
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + description: |
>> + The default value to use as a state input at startup.
>> +
>> + state-map:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: |
>> + The mapping of this function to a cooked function. The
>> + format used is a sequence of phandle to a state, the mask
>> + to apply to the state vector, and the equality value.
>> +
>> + Take the example's configuration
>> +
>> + state-inputs = "JACK_HEADPHONE", "JACK_MICROPHONE";
>> + state-input-bits = <1>, <1>;
>> + state-input-defaults = <0>, <0>;
>> +
>> + state-map = <&speaker 0x1 0x0>,
>> + <&headphones 0x3 0x1>,
>> + <&headset 0x3 0x3>;
>> +
>> + is decoded as follows.
>> +
>> + There are 3 possible cooked functions to be selected.
>> + speaker, headphone and headset. The state-inputs are
>> + the JACK_HEADPHONE and JACK_MICROPHONE, which are single
>> + bit values, being placed at bit 0 and bit 1 of the
>> + constructed vector.
>> +
>> + The 4 possible state vectors are:
>> + MICROPHONE=0, HEADPHONE=0, 0
>> + MICROPHONE=0, HEADPHONE=1, 1
>> + MICROPHONE=1, HEADPHONE=0, 2
>> + MICROPHONE=1, HEADPHONE=1, 3
>> +
>> + The speaker function is selected when HEADPHONE=0 because
>> + both (0 & 1) == (2 & 1) == 0.
>> +
>> + The headphones function is selected when HEADPHONE=1 and
>> + MICROPHONE=0 because (1 & 3) == 1.
>> +
>> + The headset function is selected when both HEADPHONE=1 and
>> + MICROPHONE=1 because (3 & 3) == 3.
>> +
>> + required:
>> + - state-inputs
>> + - state-input-bits
>> + - state-input-defaults
>> + - state-map
>> +
>> +patternProperties:
>> + "^.*dai-link-[0-9]+$":
>> + type: object
>> + description: |-
>> + cpu and codec child nodes:
>> + Container for cpu and codec dai sub-nodes.
>> + One cpu and one codec sub-node must exist.
>> +
>> + properties:
>> + link-name:
>> + description: The link name
>> +
>> + cpu:
>> + type: object
>> + properties:
>> +
>> + sound-dai:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: phandle(s) of the CPU DAI(s)
>> +
>> + required:
>> + - sound-dai
>> +
>> + codec:
>> + type: object
>> + properties:
>> +
>> + sound-dai:
>> + $ref: /schemas/types.yaml#/definitions/phandle-array
>> + description: phandle(s) of the codec DAI(s)
>> +
>> + required:
>> + - sound-dai
>> +
>> + required:
>> + - link-name
>> + - cpu
>> + - codec
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - reg-names
>> + - qcom,model
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/sound/apq8016-lpass.h>
>> +
>> + sound: sound@7702000 {
>> + compatible = "qcom,apq8039-sndcard";
>> + reg = <0x07702000 0x4>, <0x07702004 0x4>;
>> + reg-names = "mic-iomux", "spkr-iomux";
>> +
>> + status = "okay";
>> + pinctrl-0 = <&cdc_pdm_lines_act>;
>> + pinctrl-1 = <&cdc_pdm_lines_sus>;
>> + pinctrl-names = "default", "sleep";
>> + qcom,model = "APQ8039";
>> + qcom,audio-routing = "AMIC2", "MIC BIAS Internal2";
>> +
>> + internal-codec-playback-dai-link-0 {
>> + link-name = "WCD";
>> + cpu {
>> + sound-dai = <&lpass MI2S_PRIMARY>;
>> + };
>> + codec {
>> + sound-dai = <&lpass_codec 0>, <&wcd_codec 0>;
>> + };
>> + };
>> +
>> + internal-codec-capture-dai-link-0 {
>> + link-name = "WCD-Capture";
>> + cpu {
>> + sound-dai = <&lpass MI2S_TERTIARY>;
>> + };
>> + codec {
>> + sound-dai = <&lpass_codec 1>, <&wcd_codec 1>;
>> + };
>> + };
>> +
>> + function-definition {
>> +
>> + mixer-control = "Jack Function";
>> + function-list = <&auto &headphones &headset &speaker &off>;
>> + system-list = <&auto &off &off>; // default, init, shutdown
>> +
>> + auto: Automatic {
>> + // Headphone presence bit 0 (1) - H
>> + // Microphone presence bit 1 (2) - M
>> + state-inputs = "JACK_HEADPHONE", "JACK_MICROPHONE";
>> + state-input-bits = <1>, <1>;
>> + state-input-defaults = <0>, <0>;
>> +
>> + // HM & MASK
>> + state-map =
>> + <&speaker 0x1 0x0>, // no headphone -> speaker
>> + <&headphones 0x3 0x1>, // headphone but no mic -> headphones
>> + <&headset 0x3 0x3>; // headphone & mic -> headset
>> + };
>> + headphones: Headphones {
>> + enable =
>> + "RX1 MIX1 INP1", "RX1",
>> + "RX2 MIX1 INP1", "RX2",
>> + "RDAC2 MUX", "RX2",
>> + "RX1 Digital Volume", "128",
>> + "RX2 Digital Volume", "128",
>> + "HPHL", "Switch",
>> + "HPHR", "Switch";
>> +
>> + disable =
>> + "RX1 Digital Volume", "0",
>> + "RX2 Digital Volume", "0",
>> + "HPHL", "ZERO",
>> + "HPHR", "ZERO",
>> + "RDAC2 MUX", "RX1",
>> + "RX1 MIX1 INP1", "ZERO",
>> + "RX2 MIX1 INP1", "ZERO";
>> + };
>> + headset: Headset {
>> + enable =
>> + "RX1 MIX1 INP1", "RX1",
>> + "RX2 MIX1 INP1", "RX2",
>> + "RDAC2 MUX", "RX2",
>> + "RX1 Digital Volume", "128",
>> + "RX2 Digital Volume", "128",
>> + "DEC1 MUX", "ADC2",
>> + "CIC1 MUX", "AMIC",
>> + "ADC2 Volume", "8",
>> + "ADC2 MUX", "INP2",
>> + "HPHL", "Switch",
>> + "HPHR", "Switch";
>> +
>> + disable =
>> + "RX1 Digital Volume", "0",
>> + "RX2 Digital Volume", "0",
>> + "HPHL", "ZERO",
>> + "HPHR", "ZERO",
>> + "RDAC2 MUX", "RX1",
>> + "RX1 MIX1 INP1", "ZERO",
>> + "RX2 MIX1 INP1", "ZERO",
>> + "ADC2 MUX", "ZERO",
>> + "ADC2 Volume", "0",
>> + "DEC1 MUX", "ZERO";
>> + };
>> + speaker: Speaker {
>> + enable =
>> + "SPK DAC Switch", "1",
>> + "RX3 MIX1 INP1", "RX1",
>> + "RX3 MIX1 INP2", "RX2",
>> + "RX3 Digital Volume", "128";
>> +
>> + disable =
>> + "SPK DAC Switch", "0",
>> + "RX3 MIX1 INP1", "ZERO",
>> + "RX3 MIX1 INP2", "ZERO";
>> + };
>> + off: Off {
>> + enable =
>> + "RX1 Digital Volume", "0",
>> + "RX2 Digital Volume", "0",
>> + "HPHL", "ZERO",
>> + "HPHR", "ZERO",
>> + "RDAC2 MUX", "RX1",
>> + "RX1 MIX1 INP1", "ZERO",
>> + "RX2 MIX1 INP1", "ZERO",
>> + "ADC2 MUX", "ZERO",
>> + "ADC2 Volume", "0",
>> + "DEC1 MUX", "ZERO",
>> + "SPK DAC Switch", "0",
>> + "RX3 MIX1 INP1", "ZERO",
>> + "RX3 MIX1 INP2", "ZERO";
>> + };
>> + };
>
> This looks much like a replacement for ALSA UCM and userspace audio jack
> detection coded into the device tree.
>
I wouldn’t call it a replacement exactly. It’s merely a way to bundle all
of this information about codec glue in the kernel (where it should belong IMO).
> While I personally think this is an interesting idea
> (We have the device tree to describe the hardware, why can we not also
> describe necessary audio routing to enable a particular output?)
> this is also not really specific to the APQ8039 hardware, is it?
>
Not really TBF. However it is considerably simplified but not handling
all the mixer controls types besides the ones that are applicable to
this driver.
> In fact, without all the code to handle the mixer enable/disable
> sequences the machine driver looks almost identical to the existing
> apq8016-sbc.
>
Yep, modulo some device tree handling fixes.
> If you want to discuss ways to integrate mixer enable/disable sequences
> into the device tree, I suggest that you post your ideas separately as
> [RFC] with a more generic subject. That will make it more easy for
> everyone interested to share their thoughts.
>
Well, I can certainly do that. However I’d like to see if this is
something that the community would be interested to, but feedback
against this patch.
As I mentioned earlier it needs some work to be made to something
that’s completely generic (i.e. handling arbitrary control types).
> Right now it's quite hidden in a patch set where the subjects suggest
> that it's just a simple machine driver to glue some codecs together.
>
> Thanks,
> Stephan
Regards
— Pantelis
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex
2020-06-19 21:41 ` Stephan Gerhold
2020-06-22 11:34 ` Pantelis Antoniou
@ 2020-06-22 11:51 ` Mark Brown
1 sibling, 0 replies; 13+ messages in thread
From: Mark Brown @ 2020-06-22 11:51 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Pantelis Antoniou, alsa-devel, devicetree, linux-arm-msm,
Srinivas Kandagatla, Matt Porter, Shawn Guo
[-- Attachment #1: Type: text/plain, Size: 2591 bytes --]
On Fri, Jun 19, 2020 at 11:41:26PM +0200, Stephan Gerhold wrote:
> Hi Pantelis,
>
> On Fri, Jun 19, 2020 at 10:38:30PM +0300, Pantelis Antoniou wrote:
> > Add a yaml device binding for the QCOM apq8039 sound complex driver.
> >
>
> Nice to see some activity to get sound working on another SoC!
> Thanks for documenting all these properties.
Please delete unneeded context from mails when replying. Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.
> > + function-definition:
> > + type: object
> > + description: |
> > + Functional configuration for the sound complex via a
> > + simple control. allows fixed and dynamically constructed
> > + function selection.
> > +
> > + properties:
> > + mixer-control:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + description: |
> > + Name of the exported alsa mix control.
This does *not* look like something that should be in a DT binding, it
is quite clearly OS specific.
> > + system-list:
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > + description: |
> > + phandle(s) of the default, init and shutdown functions
> > + Must be one of the declared ones in the function property.
> > + The default function is the one selected by default on
> > + startup (after the init function's sequence is executed).
> > + On shutdown the shutdown function sequence will be executed.
> > + Typically init and shutdown are the same and it's purpose
> > + is to initialize the sound complex mixer controls to the
> > + all off state, and be ready for a regular function selection.
> This looks much like a replacement for ALSA UCM and userspace audio jack
> detection coded into the device tree.
Very much so. This is use case configuration and completely
inappropriate for DT. DT should describe the hardware, the way the OS
intends to use the hardware is up to the OS.
> If you want to discuss ways to integrate mixer enable/disable sequences
> into the device tree, I suggest that you post your ideas separately as
> [RFC] with a more generic subject. That will make it more easy for
> everyone interested to share their thoughts.
> Right now it's quite hidden in a patch set where the subjects suggest
> that it's just a simple machine driver to glue some codecs together.
Indeed, I agree entirely.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex
2020-06-22 11:34 ` Pantelis Antoniou
@ 2020-06-22 12:04 ` Mark Brown
2020-06-22 13:32 ` Pantelis Antoniou
0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2020-06-22 12:04 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Stephan Gerhold, alsa-devel, devicetree, linux-arm-msm,
Srinivas Kandagatla, Matthew Porter, Shawn Guo
[-- Attachment #1: Type: text/plain, Size: 700 bytes --]
On Mon, Jun 22, 2020 at 02:34:23PM +0300, Pantelis Antoniou wrote:
> > This looks much like a replacement for ALSA UCM and userspace audio jack
> > detection coded into the device tree.
> I wouldn’t call it a replacement exactly. It’s merely a way to bundle all
> of this information about codec glue in the kernel (where it should belong IMO).
No, you're encoding use case decisions into the DT here - for example
your example will break use cases like ring tones and shutter sounds
which should play through both speaker and headphones. It's also
setting volumes which may be inappropriate or may be not and interferes
with userspace using those same physical volume controls.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex
2020-06-22 12:04 ` Mark Brown
@ 2020-06-22 13:32 ` Pantelis Antoniou
2020-06-22 13:41 ` Mark Brown
0 siblings, 1 reply; 13+ messages in thread
From: Pantelis Antoniou @ 2020-06-22 13:32 UTC (permalink / raw)
To: Mark Brown
Cc: Stephan Gerhold, alsa-devel, devicetree, linux-arm-msm,
Srinivas Kandagatla, Matthew Porter, Shawn Guo
Hi Mark,
> On Jun 22, 2020, at 15:04 , Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Jun 22, 2020 at 02:34:23PM +0300, Pantelis Antoniou wrote:
>
>>> This looks much like a replacement for ALSA UCM and userspace audio jack
>>> detection coded into the device tree.
>
>> I wouldn’t call it a replacement exactly. It’s merely a way to bundle all
>> of this information about codec glue in the kernel (where it should belong IMO).
>
> No, you're encoding use case decisions into the DT here - for example
> your example will break use cases like ring tones and shutter sounds
> which should play through both speaker and headphones. It's also
> setting volumes which may be inappropriate or may be not and interferes
> with userspace using those same physical volume controls.
It is completely optional whether you use this functionality or not.
In that case you don’t use the automatic routing you merely set it to off
and everything works as before. Or you merely use the route setup for
the function from userspace.
The device in question is not a mobile phone so there is no requirement
to have speaker and headphone active at the same time. It is possible to
create a function that would be headphone+speaker active at the same time
for that case.
Regards
— Pantelis
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex
2020-06-22 13:32 ` Pantelis Antoniou
@ 2020-06-22 13:41 ` Mark Brown
2020-06-22 14:04 ` Pantelis Antoniou
0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2020-06-22 13:41 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Stephan Gerhold, alsa-devel, devicetree, linux-arm-msm,
Srinivas Kandagatla, Matthew Porter, Shawn Guo
[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]
On Mon, Jun 22, 2020 at 04:32:46PM +0300, Pantelis Antoniou wrote:
> > On Jun 22, 2020, at 15:04 , Mark Brown <broonie@kernel.org> wrote:
> > No, you're encoding use case decisions into the DT here - for example
> > your example will break use cases like ring tones and shutter sounds
> > which should play through both speaker and headphones. It's also
> > setting volumes which may be inappropriate or may be not and interferes
> > with userspace using those same physical volume controls.
> It is completely optional whether you use this functionality or not.
It's optional for whoever writes the DT and flashes it, it is not
optional for whoever's doing the OS configuration - these may not be the
same people.
> In that case you don’t use the automatic routing you merely set it to off
> and everything works as before. Or you merely use the route setup for
> the function from userspace.
Userspace shouldn't have to be fighting with the kernel for control of
the device.
> The device in question is not a mobile phone so there is no requirement
> to have speaker and headphone active at the same time. It is possible to
> create a function that would be headphone+speaker active at the same time
> for that case.
That may be true for your OS configuration but that doesn't mean that
some other user of the same hardware won't want to do something that
needs both simultaneously.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex
2020-06-22 13:41 ` Mark Brown
@ 2020-06-22 14:04 ` Pantelis Antoniou
2020-06-22 16:43 ` Mark Brown
0 siblings, 1 reply; 13+ messages in thread
From: Pantelis Antoniou @ 2020-06-22 14:04 UTC (permalink / raw)
To: Mark Brown
Cc: Stephan Gerhold, alsa-devel, devicetree, linux-arm-msm,
Srinivas Kandagatla, Matthew Porter, Shawn Guo
> On Jun 22, 2020, at 16:41 , Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Jun 22, 2020 at 04:32:46PM +0300, Pantelis Antoniou wrote:
>>> On Jun 22, 2020, at 15:04 , Mark Brown <broonie@kernel.org> wrote:
>
>>> No, you're encoding use case decisions into the DT here - for example
>>> your example will break use cases like ring tones and shutter sounds
>>> which should play through both speaker and headphones. It's also
>>> setting volumes which may be inappropriate or may be not and interferes
>>> with userspace using those same physical volume controls.
>
>> It is completely optional whether you use this functionality or not.
>
> It's optional for whoever writes the DT and flashes it, it is not
> optional for whoever's doing the OS configuration - these may not be the
> same people.
>
>> In that case you don’t use the automatic routing you merely set it to off
>> and everything works as before. Or you merely use the route setup for
>> the function from userspace.
>
> Userspace shouldn't have to be fighting with the kernel for control of
> the device.
>
>> The device in question is not a mobile phone so there is no requirement
>> to have speaker and headphone active at the same time. It is possible to
>> create a function that would be headphone+speaker active at the same time
>> for that case.
>
> That may be true for your OS configuration but that doesn't mean that
> some other user of the same hardware won't want to do something that
> needs both simultaneously.
Let’s step back a bit and let me present the problem and what this is about.
Disregard the automatic function selection using external state inputs.
The problem is that for sound card that is composed of a number of component
like this one a pretty non trivial setting of controls must be done.
Tt is not atypical for a card like this the set of control being a dozen
or so, with some requiring even more.
Someone has to do them, be it the kernel or userspace.
Instead of having userspace do it, bundle everything in DT so that everything
can be set in one go, and without having the user-space engineer read the
a few 10-100 pages of reference manuals.
This is arguably a hardware setting (eg. the set of configuration parameters
that enables routing sound to speaker).
Now this is not going to perfect for all cases; some cases are very complicated
and indeed user-space has to be engaged and perform the configuration.
This mechanism does not preclude it.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex
2020-06-22 14:04 ` Pantelis Antoniou
@ 2020-06-22 16:43 ` Mark Brown
0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2020-06-22 16:43 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Stephan Gerhold, alsa-devel, devicetree, linux-arm-msm,
Srinivas Kandagatla, Matthew Porter, Shawn Guo
[-- Attachment #1: Type: text/plain, Size: 2511 bytes --]
On Mon, Jun 22, 2020 at 05:04:16PM +0300, Pantelis Antoniou wrote:
> The problem is that for sound card that is composed of a number of component
> like this one a pretty non trivial setting of controls must be done.
> Tt is not atypical for a card like this the set of control being a dozen
> or so, with some requiring even more.
> Someone has to do them, be it the kernel or userspace.
This is super standard stuff, it's why UCM (and the Android equivalent)
exist. There is nothing here that's remarkable or new here, *please*
look at existing solutions before proposing new stuff and (as Stephan
suggested) please don't try to sneak major changes in how things work
into otherwise routine patches.
> Instead of having userspace do it, bundle everything in DT so that everything
> can be set in one go, and without having the user-space engineer read the
> a few 10-100 pages of reference manuals.
Very often in embedded systems the people doing the tuning include
hardware and acoustic engineers for whom dealing with the flexibility of
the device is not an issue but having to reflash and reboot the system
to test out changes is a substantial inconvenience. I've seen how happy
they can be with userspace configuration options allowing them to speed
up their workflows. For end users it doesn't really make a huge
difference if the configuration is delivered as part of the firmware or
as part of userspace.
> This is arguably a hardware setting (eg. the set of configuration parameters
> that enables routing sound to speaker).
In all but the simplest systems there are several, frequently many,
options available for even seemingly simple tasks like routing audio to
the speaker. Deciding between these is something that's well within the
bounds of userspace configurability, it's not like there's only one way
to do things and there may be tradeoffs to be made or combinations of
things to be considered (eg, will we have to mix additional streams in
or route the audio to additional outputs later?). Transitions between
use cases are also very much part of this, they can often be worked out
automatically but not always.
> Now this is not going to perfect for all cases; some cases are very complicated
> and indeed user-space has to be engaged and perform the configuration.
> This mechanism does not preclude it.
Having multiple uncoordinated mechanisms for doing the same thing in the
same system makes the system more complicated.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-06-22 16:43 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-19 19:38 [PATCH 0/2] ASoC: qcom: add apq8039 sound card and bindings Pantelis Antoniou
2020-06-19 19:38 ` [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex Pantelis Antoniou
2020-06-19 21:41 ` Stephan Gerhold
2020-06-22 11:34 ` Pantelis Antoniou
2020-06-22 12:04 ` Mark Brown
2020-06-22 13:32 ` Pantelis Antoniou
2020-06-22 13:41 ` Mark Brown
2020-06-22 14:04 ` Pantelis Antoniou
2020-06-22 16:43 ` Mark Brown
2020-06-22 11:51 ` Mark Brown
2020-06-19 19:38 ` [PATCH 2/2] ASoC: qcom: add apq8039 sound card support Pantelis Antoniou
2020-06-19 21:58 ` kernel test robot
2020-06-20 6:39 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).