On Wed, Apr 13, 2022 at 08:51:17PM +0530, Srinivasa Rao Mandadapu wrote:
> Add dt nodes for sound card support, which is using WCD938x headset
> playback, capture, I2S speaker playback and DMICs via VA macro.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Co-developed-by: Venkata Prasad Potturu <[email protected]>
> Signed-off-by: Venkata Prasad Potturu <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7280-crd.dts | 23 ++++++++
> arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 93 ++++++++++++++++++++++++++++++++
> 2 files changed, 116 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index b944366..1e16854 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
You need to refresh your tree, this file has been renamed to
sc7280-crd-r3.dts. That DT is for the CRD <= 2.x, newer versions
use sc7280-herobrine-crd.dts.
> @@ -90,6 +90,29 @@ ap_ts_pen_1v8: &i2c13 {
> us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
> };
>
> +&sound {
> + audio-routing =
> + "IN1_HPHL", "HPHL_OUT",
> + "IN2_HPHR", "HPHR_OUT",
> + "AMIC1", "MIC BIAS1",
> + "AMIC2", "MIC BIAS2",
> + "VA DMIC0", "MIC BIAS1",
> + "VA DMIC1", "MIC BIAS1",
> + "VA DMIC2", "MIC BIAS3",
> + "VA DMIC3", "MIC BIAS3",
> + "TX SWR_ADC0", "ADC1_OUTPUT",
> + "TX SWR_ADC1", "ADC2_OUTPUT",
> + "TX SWR_ADC2", "ADC3_OUTPUT",
> + "TX SWR_DMIC0", "DMIC1_OUTPUT",
> + "TX SWR_DMIC1", "DMIC2_OUTPUT",
> + "TX SWR_DMIC2", "DMIC3_OUTPUT",
> + "TX SWR_DMIC3", "DMIC4_OUTPUT",
> + "TX SWR_DMIC4", "DMIC5_OUTPUT",
> + "TX SWR_DMIC5", "DMIC6_OUTPUT",
> + "TX SWR_DMIC6", "DMIC7_OUTPUT",
> + "TX SWR_DMIC7", "DMIC8_OUTPUT";
> +};
> +
> &tlmm {
> tp_int_odl: tp-int-odl {
> pins = "gpio7";
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> index cf62d06..a7c884a 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> @@ -84,6 +84,99 @@
> pinctrl-names = "default";
> pinctrl-0 = <&nvme_pwren>;
> };
> +
> + sound: sound {
> + compatible = "google,sc7280-herobrine";
> + model = "sc7280-wcd938x-max98360a-1mic";
> +
> + audio-routing =
> + "IN1_HPHL", "HPHL_OUT",
> + "IN2_HPHR", "HPHR_OUT",
> + "AMIC1", "MIC BIAS1",
> + "AMIC2", "MIC BIAS2",
> + "VA DMIC0", "MIC BIAS3",
> + "VA DMIC1", "MIC BIAS3",
> + "VA DMIC2", "MIC BIAS1",
> + "VA DMIC3", "MIC BIAS1",
> + "TX SWR_ADC0", "ADC1_OUTPUT",
> + "TX SWR_ADC1", "ADC2_OUTPUT",
> + "TX SWR_ADC2", "ADC3_OUTPUT",
> + "TX SWR_DMIC0", "DMIC1_OUTPUT",
> + "TX SWR_DMIC1", "DMIC2_OUTPUT",
> + "TX SWR_DMIC2", "DMIC3_OUTPUT",
> + "TX SWR_DMIC3", "DMIC4_OUTPUT",
> + "TX SWR_DMIC4", "DMIC5_OUTPUT",
> + "TX SWR_DMIC5", "DMIC6_OUTPUT",
> + "TX SWR_DMIC6", "DMIC7_OUTPUT",
> + "TX SWR_DMIC7", "DMIC8_OUTPUT";
> +
> + qcom,msm-mbhc-hphl-swh = <1>;
> + qcom,msm-mbhc-gnd-swh = <1>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #sound-dai-cells = <0>;
> +
> + dai-link@1 {
> + link-name = "MAX98360A";
> + reg = <MI2S_SECONDARY>;
Dumb question: is this value actually used? A quick glance through
qcom_snd_parse_of() suggests it isn't. And the CPU DAI id is already
specified in the 'sound-dai' property below.
In a quick test I replaced the corresponding 'reg' values in
sc7180-trogdor.dtsi with 'random' values and audio playback on
my coachz (sc7180-trogdor-coachz-r3.dts) still works ...
> + cpu {
> + sound-dai = <&lpass_cpu MI2S_SECONDARY>;
> + };
> +
> + codec {
> + sound-dai = <&max98360a>;
> + };
> + };
> +
> + dai-link@5 {
> + link-name = "DisplayPort";
> + reg = <LPASS_DP_RX>;
nit: add an empty line (in all links) to separate the properties from the node
> + cpu {
> + sound-dai = <&lpass_cpu LPASS_DP_RX>;
> + };
> +
> + codec {
> + sound-dai = <&mdss_dp>;
> + };
> + };
> +
> + dai-link@6 {
> + link-name = "WCD9385 Playback";
> + reg = <LPASS_CDC_DMA_RX0>;
> + cpu {
> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_RX0>;
> + };
> +
> + codec {
> + sound-dai = <&wcd938x 0>, <&swr0 0>, <&lpass_rx_macro 0>;
> + };
> + };
> +
> + dai-link@19 {
> + link-name = "WCD9385 Capture";
> + reg = <LPASS_CDC_DMA_TX3>;
> + cpu {
> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_TX3>;
> + };
> +
> + codec {
> + sound-dai = <&wcd938x 1>, <&swr1 0>, <&lpass_tx_macro 0>;
> + };
> + };
> +
> + dai-link@25 {
> + link-name = "DMIC";
> + reg = <LPASS_CDC_DMA_VA_TX0>;
> + cpu {
> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_VA_TX0>;
> + };
> +
> + codec {
> + sound-dai = <&lpass_va_macro 0>;
> + };
> + };
> + };
> };
>
> &apps_rsc {
> --
> 2.7.4
>
On 4/14/2022 2:53 AM, Matthias Kaehlcke wrote:
Thanks for your time Matthias!!!
> On Wed, Apr 13, 2022 at 08:51:17PM +0530, Srinivasa Rao Mandadapu wrote:
>> Add dt nodes for sound card support, which is using WCD938x headset
>> playback, capture, I2S speaker playback and DMICs via VA macro.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>> Co-developed-by: Venkata Prasad Potturu <[email protected]>
>> Signed-off-by: Venkata Prasad Potturu <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/sc7280-crd.dts | 23 ++++++++
>> arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 93 ++++++++++++++++++++++++++++++++
>> 2 files changed, 116 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> index b944366..1e16854 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> You need to refresh your tree, this file has been renamed to
> sc7280-crd-r3.dts. That DT is for the CRD <= 2.x, newer versions
> use sc7280-herobrine-crd.dts.
Okay. will update accordingly.
>
>> @@ -90,6 +90,29 @@ ap_ts_pen_1v8: &i2c13 {
>> us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
>> };
>>
>> +&sound {
>> + audio-routing =
>> + "IN1_HPHL", "HPHL_OUT",
>> + "IN2_HPHR", "HPHR_OUT",
>> + "AMIC1", "MIC BIAS1",
>> + "AMIC2", "MIC BIAS2",
>> + "VA DMIC0", "MIC BIAS1",
>> + "VA DMIC1", "MIC BIAS1",
>> + "VA DMIC2", "MIC BIAS3",
>> + "VA DMIC3", "MIC BIAS3",
>> + "TX SWR_ADC0", "ADC1_OUTPUT",
>> + "TX SWR_ADC1", "ADC2_OUTPUT",
>> + "TX SWR_ADC2", "ADC3_OUTPUT",
>> + "TX SWR_DMIC0", "DMIC1_OUTPUT",
>> + "TX SWR_DMIC1", "DMIC2_OUTPUT",
>> + "TX SWR_DMIC2", "DMIC3_OUTPUT",
>> + "TX SWR_DMIC3", "DMIC4_OUTPUT",
>> + "TX SWR_DMIC4", "DMIC5_OUTPUT",
>> + "TX SWR_DMIC5", "DMIC6_OUTPUT",
>> + "TX SWR_DMIC6", "DMIC7_OUTPUT",
>> + "TX SWR_DMIC7", "DMIC8_OUTPUT";
>> +};
>> +
>> &tlmm {
>> tp_int_odl: tp-int-odl {
>> pins = "gpio7";
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> index cf62d06..a7c884a 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
>> @@ -84,6 +84,99 @@
>> pinctrl-names = "default";
>> pinctrl-0 = <&nvme_pwren>;
>> };
>> +
>> + sound: sound {
>> + compatible = "google,sc7280-herobrine";
>> + model = "sc7280-wcd938x-max98360a-1mic";
>> +
>> + audio-routing =
>> + "IN1_HPHL", "HPHL_OUT",
>> + "IN2_HPHR", "HPHR_OUT",
>> + "AMIC1", "MIC BIAS1",
>> + "AMIC2", "MIC BIAS2",
>> + "VA DMIC0", "MIC BIAS3",
>> + "VA DMIC1", "MIC BIAS3",
>> + "VA DMIC2", "MIC BIAS1",
>> + "VA DMIC3", "MIC BIAS1",
>> + "TX SWR_ADC0", "ADC1_OUTPUT",
>> + "TX SWR_ADC1", "ADC2_OUTPUT",
>> + "TX SWR_ADC2", "ADC3_OUTPUT",
>> + "TX SWR_DMIC0", "DMIC1_OUTPUT",
>> + "TX SWR_DMIC1", "DMIC2_OUTPUT",
>> + "TX SWR_DMIC2", "DMIC3_OUTPUT",
>> + "TX SWR_DMIC3", "DMIC4_OUTPUT",
>> + "TX SWR_DMIC4", "DMIC5_OUTPUT",
>> + "TX SWR_DMIC5", "DMIC6_OUTPUT",
>> + "TX SWR_DMIC6", "DMIC7_OUTPUT",
>> + "TX SWR_DMIC7", "DMIC8_OUTPUT";
>> +
>> + qcom,msm-mbhc-hphl-swh = <1>;
>> + qcom,msm-mbhc-gnd-swh = <1>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + #sound-dai-cells = <0>;
>> +
>> + dai-link@1 {
>> + link-name = "MAX98360A";
>> + reg = <MI2S_SECONDARY>;
> Dumb question: is this value actually used? A quick glance through
> qcom_snd_parse_of() suggests it isn't. And the CPU DAI id is already
> specified in the 'sound-dai' property below.
>
> In a quick test I replaced the corresponding 'reg' values in
> sc7180-trogdor.dtsi with 'random' values and audio playback on
> my coachz (sc7180-trogdor-coachz-r3.dts) still works ...
Yes. agree that it's not being used. But i am not sure of general syntax
followed.
for now will delete it.
>
>> + cpu {
>> + sound-dai = <&lpass_cpu MI2S_SECONDARY>;
>> + };
>> +
>> + codec {
>> + sound-dai = <&max98360a>;
>> + };
>> + };
>> +
>> + dai-link@5 {
>> + link-name = "DisplayPort";
>> + reg = <LPASS_DP_RX>;
> nit: add an empty line (in all links) to separate the properties from the node
Okay.
>
>> + cpu {
>> + sound-dai = <&lpass_cpu LPASS_DP_RX>;
>> + };
>> +
>> + codec {
>> + sound-dai = <&mdss_dp>;
>> + };
>> + };
>> +
>> + dai-link@6 {
>> + link-name = "WCD9385 Playback";
>> + reg = <LPASS_CDC_DMA_RX0>;
>> + cpu {
>> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_RX0>;
>> + };
>> +
>> + codec {
>> + sound-dai = <&wcd938x 0>, <&swr0 0>, <&lpass_rx_macro 0>;
>> + };
>> + };
>> +
>> + dai-link@19 {
>> + link-name = "WCD9385 Capture";
>> + reg = <LPASS_CDC_DMA_TX3>;
>> + cpu {
>> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_TX3>;
>> + };
>> +
>> + codec {
>> + sound-dai = <&wcd938x 1>, <&swr1 0>, <&lpass_tx_macro 0>;
>> + };
>> + };
>> +
>> + dai-link@25 {
>> + link-name = "DMIC";
>> + reg = <LPASS_CDC_DMA_VA_TX0>;
>> + cpu {
>> + sound-dai = <&lpass_cpu LPASS_CDC_DMA_VA_TX0>;
>> + };
>> +
>> + codec {
>> + sound-dai = <&lpass_va_macro 0>;
>> + };
>> + };
>> + };
>> };
>>
>> &apps_rsc {
>> --
>> 2.7.4
>>
On Mon, Apr 18, 2022 at 07:39:44PM +0530, Srinivasa Rao Mandadapu wrote:
>
> On 4/14/2022 2:53 AM, Matthias Kaehlcke wrote:
> Thanks for your time Matthias!!!
> > On Wed, Apr 13, 2022 at 08:51:17PM +0530, Srinivasa Rao Mandadapu wrote:
> > > Add dt nodes for sound card support, which is using WCD938x headset
> > > playback, capture, I2S speaker playback and DMICs via VA macro.
> > >
> > > Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> > > Co-developed-by: Venkata Prasad Potturu <[email protected]>
> > > Signed-off-by: Venkata Prasad Potturu <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/qcom/sc7280-crd.dts | 23 ++++++++
> > > arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 93 ++++++++++++++++++++++++++++++++
> > > 2 files changed, 116 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> > > index b944366..1e16854 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> > > +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> > You need to refresh your tree, this file has been renamed to
> > sc7280-crd-r3.dts. That DT is for the CRD <= 2.x, newer versions
> > use sc7280-herobrine-crd.dts.
> Okay. will update accordingly.
> >
> > > @@ -90,6 +90,29 @@ ap_ts_pen_1v8: &i2c13 {
> > > us-euro-gpios = <&tlmm 81 GPIO_ACTIVE_HIGH>;
> > > };
> > > +&sound {
> > > + audio-routing =
> > > + "IN1_HPHL", "HPHL_OUT",
> > > + "IN2_HPHR", "HPHR_OUT",
> > > + "AMIC1", "MIC BIAS1",
> > > + "AMIC2", "MIC BIAS2",
> > > + "VA DMIC0", "MIC BIAS1",
> > > + "VA DMIC1", "MIC BIAS1",
> > > + "VA DMIC2", "MIC BIAS3",
> > > + "VA DMIC3", "MIC BIAS3",
> > > + "TX SWR_ADC0", "ADC1_OUTPUT",
> > > + "TX SWR_ADC1", "ADC2_OUTPUT",
> > > + "TX SWR_ADC2", "ADC3_OUTPUT",
> > > + "TX SWR_DMIC0", "DMIC1_OUTPUT",
> > > + "TX SWR_DMIC1", "DMIC2_OUTPUT",
> > > + "TX SWR_DMIC2", "DMIC3_OUTPUT",
> > > + "TX SWR_DMIC3", "DMIC4_OUTPUT",
> > > + "TX SWR_DMIC4", "DMIC5_OUTPUT",
> > > + "TX SWR_DMIC5", "DMIC6_OUTPUT",
> > > + "TX SWR_DMIC6", "DMIC7_OUTPUT",
> > > + "TX SWR_DMIC7", "DMIC8_OUTPUT";
> > > +};
> > > +
> > > &tlmm {
> > > tp_int_odl: tp-int-odl {
> > > pins = "gpio7";
> > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > > index cf62d06..a7c884a 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi
> > > @@ -84,6 +84,99 @@
> > > pinctrl-names = "default";
> > > pinctrl-0 = <&nvme_pwren>;
> > > };
> > > +
> > > + sound: sound {
> > > + compatible = "google,sc7280-herobrine";
> > > + model = "sc7280-wcd938x-max98360a-1mic";
> > > +
> > > + audio-routing =
> > > + "IN1_HPHL", "HPHL_OUT",
> > > + "IN2_HPHR", "HPHR_OUT",
> > > + "AMIC1", "MIC BIAS1",
> > > + "AMIC2", "MIC BIAS2",
> > > + "VA DMIC0", "MIC BIAS3",
> > > + "VA DMIC1", "MIC BIAS3",
> > > + "VA DMIC2", "MIC BIAS1",
> > > + "VA DMIC3", "MIC BIAS1",
> > > + "TX SWR_ADC0", "ADC1_OUTPUT",
> > > + "TX SWR_ADC1", "ADC2_OUTPUT",
> > > + "TX SWR_ADC2", "ADC3_OUTPUT",
> > > + "TX SWR_DMIC0", "DMIC1_OUTPUT",
> > > + "TX SWR_DMIC1", "DMIC2_OUTPUT",
> > > + "TX SWR_DMIC2", "DMIC3_OUTPUT",
> > > + "TX SWR_DMIC3", "DMIC4_OUTPUT",
> > > + "TX SWR_DMIC4", "DMIC5_OUTPUT",
> > > + "TX SWR_DMIC5", "DMIC6_OUTPUT",
> > > + "TX SWR_DMIC6", "DMIC7_OUTPUT",
> > > + "TX SWR_DMIC7", "DMIC8_OUTPUT";
> > > +
> > > + qcom,msm-mbhc-hphl-swh = <1>;
> > > + qcom,msm-mbhc-gnd-swh = <1>;
> > > +
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + #sound-dai-cells = <0>;
> > > +
> > > + dai-link@1 {
> > > + link-name = "MAX98360A";
> > > + reg = <MI2S_SECONDARY>;
> > Dumb question: is this value actually used? A quick glance through
> > qcom_snd_parse_of() suggests it isn't. And the CPU DAI id is already
> > specified in the 'sound-dai' property below.
> >
> > In a quick test I replaced the corresponding 'reg' values in
> > sc7180-trogdor.dtsi with 'random' values and audio playback on
> > my coachz (sc7180-trogdor-coachz-r3.dts) still works ...
>
> Yes. agree that it's not being used. But i am not sure of general syntax
> followed.
>
> for now will delete it.
The binding requires it though. I think the correct thing would be to remove it
from the binding and from all device trees currently using it. But that might be
beyond the scope of this series.
Maybe a two step approach would be the best: add 'reg' in this series, get it
landed and then send another series that changes the binding and deletes all
'reg' entries (and adjusts the 'address' in the node name).
I don't think the CPU DAI id should be in the node name either. It isn't even
necessarily a unique identifier if I understand correctly. A SoC could have
multiple IP blocks with audio buses, each with their own enumeration of DAIs.
I suggest to just enumerate the nodes (and 'reg' while it's there) linearly
starting with 0, or maybe even change the node name in the binding (in a
second step) to dai-link-N.