| Message ID | 1649157167-29106-2-git-send-email-quic_srivasam@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add lpass pin control support for audio on sc7280 based targets | expand |
On Tue, Apr 05, 2022 at 04:42:46PM +0530, Srinivasa Rao Mandadapu wrote: > Add AMP enable node and pinmux for primary and secondary I2S > for SC7280 based platforms. > > Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> > Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com> > Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 34 +++++++++++++++++++++ > arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 20 +++++++++++++ > arch/arm64/boot/dts/qcom/sc7280.dtsi | 41 ++++++++++++++++++++++++++ > 3 files changed, 95 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > index dc17f20..de646d9 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi > @@ -530,6 +530,26 @@ ap_ec_spi: &spi10 { > drive-strength = <2>; > }; > > +&pri_mi2s_data0 { > + drive-strength = <6>; Isn't this pin used as an input (HP_DIN)? Is specifying the drive strength really needed? > +}; > + > +&pri_mi2s_data1 { > + drive-strength = <6>; > +}; > + > +&pri_mi2s_mclk { > + drive-strength = <6>; > +}; > + > +&pri_mi2s_sclk { > + drive-strength = <6>; > +}; > + > +&pri_mi2s_ws { > + drive-strength = <6>; > +}; > + > &qspi_cs0 { > bias-disable; > drive-strength = <8>; > @@ -610,6 +630,20 @@ ap_ec_spi: &spi10 { > drive-strength = <10>; > }; > > +&sec_mi2s_data0 { > + drive-strength = <6>; > + bias-disable; > +}; > + > +&sec_mi2s_sclk { > + drive-strength = <6>; > + bias-disable; > +}; > + > +&sec_mi2s_ws { > + drive-strength = <6>; > +}; Actually there are several sound configs for herobrine boards. For now I think it's ok to specify the config for herobrine -rev1 (as this patch does) and we can sort out later how to best support the different configs. > /* PINCTRL - board-specific pinctrl */ > > &pm7325_gpios { > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > index ecbf2b8..2afbbe3 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > @@ -462,7 +462,27 @@ > drive-strength = <10>; > }; > > +&sec_mi2s_data0 { > + drive-strength = <6>; > + bias-disable; > +}; > + > +&sec_mi2s_sclk { > + drive-strength = <6>; > + bias-disable; > +}; > + > +&sec_mi2s_ws { > + drive-strength = <6>; > +}; > + > &tlmm { > + amp_en: amp-en { > + pins = "gpio63"; > + bias-pull-down; > + drive-strength = <2>; > + }; nit: all the other pins are i2s related, it might make sense to add amp_en in a separate patch. > + > bt_en: bt-en { > pins = "gpio85"; > function = "gpio"; > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index f0b64be..8d8cec5 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -3527,6 +3527,31 @@ > function = "pcie1_clkreqn"; > }; > > + pri_mi2s_data0: pri-mi2s-data0 { > + pins = "gpio98"; > + function = "mi2s0_data0"; > + }; > + > + pri_mi2s_data1: pri-mi2s-data1 { > + pins = "gpio99"; > + function = "mi2s0_data1"; > + }; > + > + pri_mi2s_mclk: pri-mi2s-mclk { > + pins = "gpio96"; > + function = "pri_mi2s"; > + }; > + > + pri_mi2s_sclk: pri-mi2s-sclk { > + pins = "gpio97"; > + function = "mi2s0_sck"; > + }; > + > + pri_mi2s_ws: pri-mi2s-ws { > + pins = "gpio100"; > + function = "mi2s0_ws"; > + }; > + > qspi_clk: qspi-clk { > pins = "gpio14"; > function = "qspi_clk"; > @@ -4261,6 +4286,22 @@ > drive-strength = <2>; > bias-bus-hold; > }; > + > + sec_mi2s_data0: sec-mi2s-data0 { > + pins = "gpio107"; > + function = "mi2s1_data0"; > + }; > + > + sec_mi2s_sclk: sec-mi2s-sclk { > + pins = "gpio106"; > + function = "mi2s1_sck"; > + }; > + > + sec_mi2s_ws: sec-mi2s-ws { > + pins = "gpio108"; > + function = "mi2s1_ws"; > + }; Is there a particular reason for the pri/sec nomenclature? The datasheet and schematics call the pin mi2sN_xyz, it seems it would be clearer to follow that naming. Primary/secondary seems to imply a 'master/slave' topology, but these are independent controllers IIUC. The datasheet refers to pin 96 as PRI_MI2S_MCLK and pin 105 SEC_MI2S_MCLK, I guess the naming was derived from that. My suggestion would be to follow the naming in the datasheet/schematic, i.e. mi2sN_data0, mi2sN_data1, pri/sec_mi2s_mclk, mi2sN_sck, mi2sN_ws.
On 4/6/2022 12:04 AM, Matthias Kaehlcke wrote: Thanks for your time Matthias!!! > On Tue, Apr 05, 2022 at 04:42:46PM +0530, Srinivasa Rao Mandadapu wrote: >> Add AMP enable node and pinmux for primary and secondary I2S >> for SC7280 based platforms. >> >> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> >> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com> >> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 34 +++++++++++++++++++++ >> arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 20 +++++++++++++ >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 41 ++++++++++++++++++++++++++ >> 3 files changed, 95 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi >> index dc17f20..de646d9 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi >> @@ -530,6 +530,26 @@ ap_ec_spi: &spi10 { >> drive-strength = <2>; >> }; >> >> +&pri_mi2s_data0 { >> + drive-strength = <6>; > Isn't this pin used as an input (HP_DIN)? Is specifying the drive strength > really needed? Okay. will remove this. > >> +}; >> + >> +&pri_mi2s_data1 { >> + drive-strength = <6>; >> +}; >> + >> +&pri_mi2s_mclk { >> + drive-strength = <6>; >> +}; >> + >> +&pri_mi2s_sclk { >> + drive-strength = <6>; >> +}; >> + >> +&pri_mi2s_ws { >> + drive-strength = <6>; >> +}; >> + >> &qspi_cs0 { >> bias-disable; >> drive-strength = <8>; >> @@ -610,6 +630,20 @@ ap_ec_spi: &spi10 { >> drive-strength = <10>; >> }; >> >> +&sec_mi2s_data0 { >> + drive-strength = <6>; >> + bias-disable; >> +}; >> + >> +&sec_mi2s_sclk { >> + drive-strength = <6>; >> + bias-disable; >> +}; >> + >> +&sec_mi2s_ws { >> + drive-strength = <6>; >> +}; > Actually there are several sound configs for herobrine boards. For now I > think it's ok to specify the config for herobrine -rev1 (as this patch > does) and we can sort out later how to best support the different configs. Okay. Will skip this for now. > >> /* PINCTRL - board-specific pinctrl */ >> >> &pm7325_gpios { >> diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >> index ecbf2b8..2afbbe3 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi >> @@ -462,7 +462,27 @@ >> drive-strength = <10>; >> }; >> >> +&sec_mi2s_data0 { >> + drive-strength = <6>; >> + bias-disable; >> +}; >> + >> +&sec_mi2s_sclk { >> + drive-strength = <6>; >> + bias-disable; >> +}; >> + >> +&sec_mi2s_ws { >> + drive-strength = <6>; >> +}; >> + >> &tlmm { >> + amp_en: amp-en { >> + pins = "gpio63"; >> + bias-pull-down; >> + drive-strength = <2>; >> + }; > nit: all the other pins are i2s related, it might make sense to add amp_en > in a separate patch. Okay. will add it in corresponding consumer patch. > >> + >> bt_en: bt-en { >> pins = "gpio85"; >> function = "gpio"; >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> index f0b64be..8d8cec5 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> @@ -3527,6 +3527,31 @@ >> function = "pcie1_clkreqn"; >> }; >> >> + pri_mi2s_data0: pri-mi2s-data0 { >> + pins = "gpio98"; >> + function = "mi2s0_data0"; >> + }; >> + >> + pri_mi2s_data1: pri-mi2s-data1 { >> + pins = "gpio99"; >> + function = "mi2s0_data1"; >> + }; >> + >> + pri_mi2s_mclk: pri-mi2s-mclk { >> + pins = "gpio96"; >> + function = "pri_mi2s"; >> + }; >> + >> + pri_mi2s_sclk: pri-mi2s-sclk { >> + pins = "gpio97"; >> + function = "mi2s0_sck"; >> + }; >> + >> + pri_mi2s_ws: pri-mi2s-ws { >> + pins = "gpio100"; >> + function = "mi2s0_ws"; >> + }; >> + >> qspi_clk: qspi-clk { >> pins = "gpio14"; >> function = "qspi_clk"; >> @@ -4261,6 +4286,22 @@ >> drive-strength = <2>; >> bias-bus-hold; >> }; >> + >> + sec_mi2s_data0: sec-mi2s-data0 { >> + pins = "gpio107"; >> + function = "mi2s1_data0"; >> + }; >> + >> + sec_mi2s_sclk: sec-mi2s-sclk { >> + pins = "gpio106"; >> + function = "mi2s1_sck"; >> + }; >> + >> + sec_mi2s_ws: sec-mi2s-ws { >> + pins = "gpio108"; >> + function = "mi2s1_ws"; >> + }; > Is there a particular reason for the pri/sec nomenclature? The datasheet and > schematics call the pin mi2sN_xyz, it seems it would be clearer to follow > that naming. Primary/secondary seems to imply a 'master/slave' topology, but > these are independent controllers IIUC. The datasheet refers to pin 96 as > PRI_MI2S_MCLK and pin 105 SEC_MI2S_MCLK, I guess the naming was derived from > that. > > My suggestion would be to follow the naming in the datasheet/schematic, i.e. > mi2sN_data0, mi2sN_data1, pri/sec_mi2s_mclk, mi2sN_sck, mi2sN_ws. Okay. Actually we followed same in Rennel architecture. Will change accordingly.
diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi index dc17f20..de646d9 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi @@ -530,6 +530,26 @@ ap_ec_spi: &spi10 { drive-strength = <2>; }; +&pri_mi2s_data0 { + drive-strength = <6>; +}; + +&pri_mi2s_data1 { + drive-strength = <6>; +}; + +&pri_mi2s_mclk { + drive-strength = <6>; +}; + +&pri_mi2s_sclk { + drive-strength = <6>; +}; + +&pri_mi2s_ws { + drive-strength = <6>; +}; + &qspi_cs0 { bias-disable; drive-strength = <8>; @@ -610,6 +630,20 @@ ap_ec_spi: &spi10 { drive-strength = <10>; }; +&sec_mi2s_data0 { + drive-strength = <6>; + bias-disable; +}; + +&sec_mi2s_sclk { + drive-strength = <6>; + bias-disable; +}; + +&sec_mi2s_ws { + drive-strength = <6>; +}; + /* PINCTRL - board-specific pinctrl */ &pm7325_gpios { diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi index ecbf2b8..2afbbe3 100644 --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi @@ -462,7 +462,27 @@ drive-strength = <10>; }; +&sec_mi2s_data0 { + drive-strength = <6>; + bias-disable; +}; + +&sec_mi2s_sclk { + drive-strength = <6>; + bias-disable; +}; + +&sec_mi2s_ws { + drive-strength = <6>; +}; + &tlmm { + amp_en: amp-en { + pins = "gpio63"; + bias-pull-down; + drive-strength = <2>; + }; + bt_en: bt-en { pins = "gpio85"; function = "gpio"; diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi index f0b64be..8d8cec5 100644 --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi @@ -3527,6 +3527,31 @@ function = "pcie1_clkreqn"; }; + pri_mi2s_data0: pri-mi2s-data0 { + pins = "gpio98"; + function = "mi2s0_data0"; + }; + + pri_mi2s_data1: pri-mi2s-data1 { + pins = "gpio99"; + function = "mi2s0_data1"; + }; + + pri_mi2s_mclk: pri-mi2s-mclk { + pins = "gpio96"; + function = "pri_mi2s"; + }; + + pri_mi2s_sclk: pri-mi2s-sclk { + pins = "gpio97"; + function = "mi2s0_sck"; + }; + + pri_mi2s_ws: pri-mi2s-ws { + pins = "gpio100"; + function = "mi2s0_ws"; + }; + qspi_clk: qspi-clk { pins = "gpio14"; function = "qspi_clk"; @@ -4261,6 +4286,22 @@ drive-strength = <2>; bias-bus-hold; }; + + sec_mi2s_data0: sec-mi2s-data0 { + pins = "gpio107"; + function = "mi2s1_data0"; + }; + + sec_mi2s_sclk: sec-mi2s-sclk { + pins = "gpio106"; + function = "mi2s1_sck"; + }; + + sec_mi2s_ws: sec-mi2s-ws { + pins = "gpio108"; + function = "mi2s1_ws"; + }; + }; imem@146a5000 {