| Message ID | 20200907100039.1731457-3-cychiang@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add documentation and machine driver for SC7180 sound card | expand |
On Mon, Sep 07, 2020 at 06:00:38PM +0800, Cheng-Yi Chiang wrote: > Add devicetree bindings documentation file for sc7180 sound card. > > Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org> > --- > .../bindings/sound/qcom,sc7180.yaml | 143 ++++++++++++++++++ > 1 file changed, 143 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > diff --git a/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > new file mode 100644 > index 000000000000..ae809346ca80 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > @@ -0,0 +1,143 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/qcom,sc7180.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Technologies Inc. SC7180 ASoC sound card driver > + > +maintainers: > + - Rohit kumar <rohitkr@codeaurora.org> > + - Cheng-Yi Chiang <cychiang@chromium.org> > + > +description: > + This binding describes the SC7180 sound card which uses LPASS for audio. > + > +properties: > + compatible: > + const: qcom,sc7180-sndcard > + > + audio-routing: > + $ref: /schemas/types.yaml#/definitions/non-unique-string-array > + description: > + A list of the connections between audio components. Each entry is a > + pair of strings, the first being the connection's sink, the second > + being the connection's source. > + > + model: > + $ref: /schemas/types.yaml#/definitions/string > + description: User specified audio sound card name > + > + headset-jack: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: phandle of the codec for headset detection > + > + hdmi-jack: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: phandle of the codec for hdmi jack detection You already have links to these devices. Why duplicate it here? What if you had 2 headsets? This doesn't scale. Rob
On Wed, Sep 9, 2020 at 4:34 AM Rob Herring <robh@kernel.org> wrote: > > On Mon, Sep 07, 2020 at 06:00:38PM +0800, Cheng-Yi Chiang wrote: > > Add devicetree bindings documentation file for sc7180 sound card. > > > > Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org> > > --- > > .../bindings/sound/qcom,sc7180.yaml | 143 ++++++++++++++++++ > > 1 file changed, 143 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > > > diff --git a/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > new file mode 100644 > > index 000000000000..ae809346ca80 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > @@ -0,0 +1,143 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/sound/qcom,sc7180.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Qualcomm Technologies Inc. SC7180 ASoC sound card driver > > + > > +maintainers: > > + - Rohit kumar <rohitkr@codeaurora.org> > > + - Cheng-Yi Chiang <cychiang@chromium.org> > > + > > +description: > > + This binding describes the SC7180 sound card which uses LPASS for audio. > > + > > +properties: > > + compatible: > > + const: qcom,sc7180-sndcard > > + > > + audio-routing: > > + $ref: /schemas/types.yaml#/definitions/non-unique-string-array > > + description: > > + A list of the connections between audio components. Each entry is a > > + pair of strings, the first being the connection's sink, the second > > + being the connection's source. > > + > > + model: > > + $ref: /schemas/types.yaml#/definitions/string > > + description: User specified audio sound card name > > + > > + headset-jack: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: phandle of the codec for headset detection > > + > > + hdmi-jack: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: phandle of the codec for hdmi jack detection > > You already have links to these devices. Why duplicate it here? > > What if you had 2 headsets? This doesn't scale. > Hi Rob, thanks for reviewing. There was some discussion in https://patchwork.kernel.org/patch/11737905/#23571643 about how to specify the dailink that has a headset jack. I would like to pass the information of headset jack and hdmi jack to the machine driver so the machine driver can call snd_soc_component_set_jack to set jack when init the corresponding link. Headset jack and hdmi jack will be treated differently for button and event type. Because of this, we can not just set a property "jack" in the link. As for the 2 headsets case (I guess you mean hp jack and mic jack), on this board we will not have this use case. If someone really wants to build hp jack and mic jack on the board based on this machine driver, we can add two more property hp-jack and mic-jack to specify that, as the machine driver will need to know the different jack types anyway. What do you think ? Or could you please suggest a proper way to pass such information ? Thanks! > > Rob
On Wed, Sep 9, 2020 at 5:23 PM Cheng-yi Chiang <cychiang@chromium.org> wrote: > > On Wed, Sep 9, 2020 at 4:34 AM Rob Herring <robh@kernel.org> wrote: > > > > On Mon, Sep 07, 2020 at 06:00:38PM +0800, Cheng-Yi Chiang wrote: > > > Add devicetree bindings documentation file for sc7180 sound card. > > > > > > Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org> > > > --- > > > .../bindings/sound/qcom,sc7180.yaml | 143 ++++++++++++++++++ > > > 1 file changed, 143 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > > new file mode 100644 > > > index 000000000000..ae809346ca80 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > > @@ -0,0 +1,143 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/sound/qcom,sc7180.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Qualcomm Technologies Inc. SC7180 ASoC sound card driver > > > + > > > +maintainers: > > > + - Rohit kumar <rohitkr@codeaurora.org> > > > + - Cheng-Yi Chiang <cychiang@chromium.org> > > > + > > > +description: > > > + This binding describes the SC7180 sound card which uses LPASS for audio. > > > + > > > +properties: > > > + compatible: > > > + const: qcom,sc7180-sndcard > > > + > > > + audio-routing: > > > + $ref: /schemas/types.yaml#/definitions/non-unique-string-array > > > + description: > > > + A list of the connections between audio components. Each entry is a > > > + pair of strings, the first being the connection's sink, the second > > > + being the connection's source. > > > + > > > + model: > > > + $ref: /schemas/types.yaml#/definitions/string > > > + description: User specified audio sound card name > > > + > > > + headset-jack: > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + description: phandle of the codec for headset detection > > > + > > > + hdmi-jack: > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + description: phandle of the codec for hdmi jack detection > > > > You already have links to these devices. Why duplicate it here? > > > > What if you had 2 headsets? This doesn't scale. > > > Hi Rob, thanks for reviewing. > There was some discussion in > https://patchwork.kernel.org/patch/11737905/#23571643 about how to > specify the dailink that has a headset jack. > I would like to pass the information of headset jack and hdmi jack to > the machine driver so the machine driver can call > snd_soc_component_set_jack to set jack when init the corresponding link. > Headset jack and hdmi jack will be treated differently for button and > event type. > Because of this, we can not just set a property "jack" in the link. > > As for the 2 headsets case (I guess you mean hp jack and mic jack), on > this board we will not have this use case. > If someone really wants to build hp jack and mic jack on the board > based on this machine driver, we can add two more property hp-jack and > mic-jack to specify that, > as the machine driver will need to know the different jack types > anyway. What do you think ? > > Or could you please suggest a proper way to pass such information ? > > Thanks! > > Alternatively we can probably do dai-link@0 { link-name = "MultiMedia0"; reg = <0>; cpu { sound-dai = <&lpass_cpu 0>; }; headset_jack; codec { sound-dai = <&alc5682 0>; }; }; dai-link@2 { link-name = "MultiMedia2"; reg = <2>; cpu { sound-dai = <&lpass_hdmi 0>; }; hdmi_jack; codec { sound-dai = <&msm_dp>; }; }; Or even put the flag into codec {}. Please let me know if you feel this is a better way. I think it will make the driver code a little more complicated, but the interface on dts might looks cleaner. Thanks! > > Rob
On Wed, Sep 9, 2020 at 3:24 AM Cheng-yi Chiang <cychiang@chromium.org> wrote: > > On Wed, Sep 9, 2020 at 4:34 AM Rob Herring <robh@kernel.org> wrote: > > > > On Mon, Sep 07, 2020 at 06:00:38PM +0800, Cheng-Yi Chiang wrote: > > > Add devicetree bindings documentation file for sc7180 sound card. > > > > > > Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org> > > > --- > > > .../bindings/sound/qcom,sc7180.yaml | 143 ++++++++++++++++++ > > > 1 file changed, 143 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > > new file mode 100644 > > > index 000000000000..ae809346ca80 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > > @@ -0,0 +1,143 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/sound/qcom,sc7180.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Qualcomm Technologies Inc. SC7180 ASoC sound card driver > > > + > > > +maintainers: > > > + - Rohit kumar <rohitkr@codeaurora.org> > > > + - Cheng-Yi Chiang <cychiang@chromium.org> > > > + > > > +description: > > > + This binding describes the SC7180 sound card which uses LPASS for audio. > > > + > > > +properties: > > > + compatible: > > > + const: qcom,sc7180-sndcard > > > + > > > + audio-routing: > > > + $ref: /schemas/types.yaml#/definitions/non-unique-string-array > > > + description: > > > + A list of the connections between audio components. Each entry is a > > > + pair of strings, the first being the connection's sink, the second > > > + being the connection's source. > > > + > > > + model: > > > + $ref: /schemas/types.yaml#/definitions/string > > > + description: User specified audio sound card name > > > + > > > + headset-jack: > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + description: phandle of the codec for headset detection > > > + > > > + hdmi-jack: > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > + description: phandle of the codec for hdmi jack detection > > > > You already have links to these devices. Why duplicate it here? > > > > What if you had 2 headsets? This doesn't scale. > > > Hi Rob, thanks for reviewing. > There was some discussion in > https://patchwork.kernel.org/patch/11737905/#23571643 about how to > specify the dailink that has a headset jack. > I would like to pass the information of headset jack and hdmi jack to > the machine driver so the machine driver can call > snd_soc_component_set_jack to set jack when init the corresponding link. > Headset jack and hdmi jack will be treated differently for button and > event type. > Because of this, we can not just set a property "jack" in the link. Don't design your binding around some driver architecture. Limitations of ASoC are not reasons for your binding. For DP and HDMI, we assume HPD is supported generally as that's a standard function for HDMI/DP controllers. There's a 'no-hpd' property for cases of broken HPD. That hardware description is part of the device HPD is connected to which is the HDMI/DP controller/bridge or the connector node in the case of a GPIO line. That doesn't belong in the virtual sound card. I would assume a codec to be similar. The codec node (the alc5682 node) should have any jack related properties (or possibly implicitly support it by default). > As for the 2 headsets case (I guess you mean hp jack and mic jack), on > this board we will not have this use case. > If someone really wants to build hp jack and mic jack on the board > based on this machine driver, we can add two more property hp-jack and > mic-jack to specify that, > as the machine driver will need to know the different jack types > anyway. What do you think ? I was thinking more of a case of having 2 of the same thing. Perhaps 2 HDMI outputs. Or if you have DP instead of HDMI? Just going to add 'dp-jack'? Rob
On Thu, Sep 10, 2020 at 1:49 AM Rob Herring <robh@kernel.org> wrote: > > On Wed, Sep 9, 2020 at 3:24 AM Cheng-yi Chiang <cychiang@chromium.org> wrote: > > > > On Wed, Sep 9, 2020 at 4:34 AM Rob Herring <robh@kernel.org> wrote: > > > > > > On Mon, Sep 07, 2020 at 06:00:38PM +0800, Cheng-Yi Chiang wrote: > > > > Add devicetree bindings documentation file for sc7180 sound card. > > > > > > > > Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org> > > > > --- > > > > .../bindings/sound/qcom,sc7180.yaml | 143 ++++++++++++++++++ > > > > 1 file changed, 143 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > > > new file mode 100644 > > > > index 000000000000..ae809346ca80 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml > > > > @@ -0,0 +1,143 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/sound/qcom,sc7180.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Qualcomm Technologies Inc. SC7180 ASoC sound card driver > > > > + > > > > +maintainers: > > > > + - Rohit kumar <rohitkr@codeaurora.org> > > > > + - Cheng-Yi Chiang <cychiang@chromium.org> > > > > + > > > > +description: > > > > + This binding describes the SC7180 sound card which uses LPASS for audio. > > > > + > > > > +properties: > > > > + compatible: > > > > + const: qcom,sc7180-sndcard > > > > + > > > > + audio-routing: > > > > + $ref: /schemas/types.yaml#/definitions/non-unique-string-array > > > > + description: > > > > + A list of the connections between audio components. Each entry is a > > > > + pair of strings, the first being the connection's sink, the second > > > > + being the connection's source. > > > > + > > > > + model: > > > > + $ref: /schemas/types.yaml#/definitions/string > > > > + description: User specified audio sound card name > > > > + > > > > + headset-jack: > > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > > + description: phandle of the codec for headset detection > > > > + > > > > + hdmi-jack: > > > > + $ref: /schemas/types.yaml#/definitions/phandle > > > > + description: phandle of the codec for hdmi jack detection > > > > > > You already have links to these devices. Why duplicate it here? > > > > > > What if you had 2 headsets? This doesn't scale. > > > > > Hi Rob, thanks for reviewing. > > There was some discussion in > > https://patchwork.kernel.org/patch/11737905/#23571643 about how to > > specify the dailink that has a headset jack. > > I would like to pass the information of headset jack and hdmi jack to > > the machine driver so the machine driver can call > > snd_soc_component_set_jack to set jack when init the corresponding link. > > Headset jack and hdmi jack will be treated differently for button and > > event type. > > Because of this, we can not just set a property "jack" in the link. > > Don't design your binding around some driver architecture. Limitations > of ASoC are not reasons for your binding. > > For DP and HDMI, we assume HPD is supported generally as that's a > standard function for HDMI/DP controllers. There's a 'no-hpd' property > for cases of broken HPD. That hardware description is part of the > device HPD is connected to which is the HDMI/DP controller/bridge or > the connector node in the case of a GPIO line. That doesn't belong in > the virtual sound card. > > I would assume a codec to be similar. The codec node (the alc5682 > node) should have any jack related properties (or possibly implicitly > support it by default). Thanks for the explanation. Now I can follow your thoughts. I see that the sound card driver should assume that there is jack/hpd available on HDMI/codec unless it is specified in HDMI/codec node (should not specified in sound card). > > > > As for the 2 headsets case (I guess you mean hp jack and mic jack), on > > this board we will not have this use case. > > If someone really wants to build hp jack and mic jack on the board > > based on this machine driver, we can add two more property hp-jack and > > mic-jack to specify that, > > as the machine driver will need to know the different jack types > > anyway. What do you think ? > > I was thinking more of a case of having 2 of the same thing. Perhaps 2 > HDMI outputs. Or if you have DP instead of HDMI? Just going to add > 'dp-jack'? I agree that this may not scale. It will become an abuse of binding interface. Sound card drivers should have more knowledge/control on this kind of configuration. I think this can be taken care of inside the sound card driver. If a machine wants a different configuration, we can use compatible strings to achieve that. This will be more flexible than the limited binding interface. Thanks for the discussion, I will remove headset-jack and hdmi-jack in v8. > > > Rob
On Wed, Sep 09, 2020 at 11:49:22AM -0600, Rob Herring wrote: > I would assume a codec to be similar. The codec node (the alc5682 > node) should have any jack related properties (or possibly implicitly > support it by default). This isn't always clear - some jack detection is implemented entirely by integrating simple components like GPIOs and ADCs or by integrating limited support in one device with some external components to get more features (like bolting an ADC on the side for button detect). I would not expect to ever see a device with jack detection support that could just be unconditionally enabled, it will always require some board level wiring up to provide the connections required to function.
diff --git a/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml new file mode 100644 index 000000000000..ae809346ca80 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/qcom,sc7180.yaml @@ -0,0 +1,143 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/qcom,sc7180.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Technologies Inc. SC7180 ASoC sound card driver + +maintainers: + - Rohit kumar <rohitkr@codeaurora.org> + - Cheng-Yi Chiang <cychiang@chromium.org> + +description: + This binding describes the SC7180 sound card which uses LPASS for audio. + +properties: + compatible: + const: qcom,sc7180-sndcard + + audio-routing: + $ref: /schemas/types.yaml#/definitions/non-unique-string-array + description: + A list of the connections between audio components. Each entry is a + pair of strings, the first being the connection's sink, the second + being the connection's source. + + model: + $ref: /schemas/types.yaml#/definitions/string + description: User specified audio sound card name + + headset-jack: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle of the codec for headset detection + + hdmi-jack: + $ref: /schemas/types.yaml#/definitions/phandle + description: phandle of the codec for hdmi jack detection + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + +patternProperties: + "^dai-link(@[0-9])?$": + description: + Each subnode represents a dai link. Subnodes of each dai links would be + cpu/codec dais. + + type: object + + properties: + link-name: + description: Indicates dai-link name and PCM stream name. + $ref: /schemas/types.yaml#/definitions/string + maxItems: 1 + + reg: + description: dai link address. + + cpu: + description: Holds subnode which indicates cpu dai. + type: object + properties: + sound-dai: true + + codec: + description: Holds subnode which indicates codec dai. + type: object + properties: + sound-dai: true + + required: + - link-name + - cpu + - codec + + additionalProperties: false + +required: + - compatible + - model + - headset-jack + - hdmi-jack + - "#address-cells" + - "#size-cells" + +additionalProperties: false + +examples: + + - | + sound { + compatible = "qcom,sc7180-sndcard"; + model = "sc7180-snd-card"; + + audio-routing = + "Headphone Jack", "HPOL", + "Headphone Jack", "HPOR"; + + headset-jack = <&alc5682>; + hdmi-jack = <&msm_dp>; + + #address-cells = <1>; + #size-cells = <0>; + + dai-link@0 { + link-name = "MultiMedia0"; + reg = <0>; + cpu { + sound-dai = <&lpass_cpu 0>; + }; + + codec { + sound-dai = <&alc5682 0>; + }; + }; + + dai-link@1 { + link-name = "MultiMedia1"; + reg = <1>; + cpu { + sound-dai = <&lpass_cpu 1>; + }; + + codec { + sound-dai = <&max98357a>; + }; + }; + + dai-link@2 { + link-name = "MultiMedia2"; + reg = <2>; + cpu { + sound-dai = <&lpass_hdmi 0>; + }; + + codec { + sound-dai = <&msm_dp>; + }; + }; + };
Add devicetree bindings documentation file for sc7180 sound card. Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org> --- .../bindings/sound/qcom,sc7180.yaml | 143 ++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/qcom,sc7180.yaml