| Message ID | 20200619193831.12528-2-pantelis.antoniou@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | ASoC: qcom: add apq8039 sound card and bindings | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 370 lines checked |
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
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
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.
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.
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
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.
> 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.
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.
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"; + }; + }; + };
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