Re: [PATCH 1/2] dt-bindings: sound: Device tree bindings for the apq8039 sound complex

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Stephan,


> On Jun 20, 2020, at 00:41 , Stephan Gerhold <stephan@xxxxxxxxxxx> 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@xxxxxxxxxx>
>> ---
>> .../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@xxxxxxxxxx>
>> +
>> +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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux