Message ID | 20220210134049.32576-2-ariel.dalessandro@collabora.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2,1/2] ASoC: bindings: fsl-asoc-card: Add compatible for tlv320aic31xx codec | expand |
On Thu, Feb 10, 2022 at 10:40:49AM -0300, Ariel D'Alessandro wrote: > BSH SystemMaster (SMM) S2 PRO board comes with an audio card based on > tlv320aic31xx family codec. > > The audio card exposes two playback devices, one of them using the EASRC > (Enhanced Asynchronous Sample Rate Converter) module. Note that this > would require SDMA and EASRC firmware in order to work. > > Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> > --- > .../dts/freescale/imx8mn-bsh-smm-s2pro.dts | 94 +++++++++++++++++++ > 1 file changed, 94 insertions(+) > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts b/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts > index c6a8ed6745c1..3621354b4a92 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts > +++ b/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts > @@ -7,6 +7,7 @@ > /dts-v1/; > > #include "imx8mn-bsh-smm-s2-common.dtsi" > +#include <dt-bindings/sound/tlv320aic31xx.h> > > / { > model = "BSH SMM S2 PRO"; > @@ -16,6 +17,69 @@ memory@40000000 { > device_type = "memory"; > reg = <0x0 0x40000000 0x0 0x20000000>; > }; > + > + sound-tlv320aic31xx { > + compatible = "fsl,imx-audio-tlv320aic31xx"; > + model = "tlv320aic31xx-hifi"; > + audio-cpu = <&sai3>; > + audio-codec = <&codec>; > + audio-asrc = <&easrc>; > + audio-routing = > + "Ext Spk", "SPL", > + "Ext Spk", "SPR"; > + mclk-id = <PLL_CLKIN_BCLK>; > + }; > + > + vdd_input: vdd_input { > + compatible = "regulator-fixed"; > + regulator-name = "vdd_input"; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + }; > +}; > + > +&easrc { > + fsl,asrc-rate = <48000>; Double spaces before '='. > + fsl,asrc-format = <10>; > + status = "okay"; > +}; > + > +&i2c2 { > + clock-frequency = <400000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_i2c2>; > + status = "okay"; > + > + codec: tlv320dac3101@18 { tlv320dac3101: audio-codec@18 > + #sound-dai-cells = <0>; > + compatible = "ti,tlv320dac3101"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_dac_rst>; > + reg = <0x18>; > + > + ai31xx-micbias-vg = <MICBIAS_AVDDV>; > + > + HPVDD-supply = <&buck4_reg>; > + SPRVDD-supply = <&vdd_input>; > + SPLVDD-supply = <&vdd_input>; > + AVDD-supply = <&buck4_reg>; > + IOVDD-supply = <&buck4_reg>; > + DVDD-supply = <&buck5_reg>; > + reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>; > + > + clocks = <&clk IMX8MN_CLK_SAI3_ROOT>; > + clock-names = "mclk"; > + }; > +}; > + > +&sai3 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_sai3>; > + assigned-clocks = <&clk IMX8MN_CLK_SAI3>; > + assigned-clock-parents = <&clk IMX8MN_AUDIO_PLL1_OUT>; > + assigned-clock-rates = <24576000>; > + fsl,sai-mclk-direction-output; > + status = "okay"; > }; > > /* eMMC */ > @@ -30,6 +94,36 @@ &usdhc1 { > }; > > &iomuxc { > + pinctrl_dac_rst: dac_rst { Name the node more in the same style as other pinctrl nodes. Shawn > + fsl,pins = < > + MX8MN_IOMUXC_GPIO1_IO06_GPIO1_IO6 0x19 /* DAC_RST */ > + >; > + }; > + > + pinctrl_espi2: espi2grp { > + fsl,pins = < > + MX8MN_IOMUXC_ECSPI2_SCLK_ECSPI2_SCLK 0x082 > + MX8MN_IOMUXC_ECSPI2_MOSI_ECSPI2_MOSI 0x082 > + MX8MN_IOMUXC_ECSPI2_MISO_ECSPI2_MISO 0x082 > + MX8MN_IOMUXC_ECSPI2_SS0_ECSPI2_SS0 0x040 > + >; > + }; > + > + pinctrl_i2c2: i2c2grp { > + fsl,pins = < > + MX8MN_IOMUXC_I2C2_SCL_I2C2_SCL 0x400000c3 > + MX8MN_IOMUXC_I2C2_SDA_I2C2_SDA 0x400000c3 > + >; > + }; > + > + pinctrl_sai3: sai3grp { > + fsl,pins = < > + MX8MN_IOMUXC_SAI3_TXFS_SAI3_TX_SYNC 0xd6 > + MX8MN_IOMUXC_SAI3_TXC_SAI3_TX_BCLK 0xd6 > + MX8MN_IOMUXC_SAI3_TXD_SAI3_TX_DATA0 0xd6 > + >; > + }; > + > pinctrl_usdhc1: usdhc1grp { > fsl,pins = < > MX8MN_IOMUXC_SD1_CLK_USDHC1_CLK 0x40000090 > -- > 2.34.1 >
Hi Shawn, Thanks for the review, will address all the below changes in v3. Regards, Ariel On 2/13/22 01:18, Shawn Guo wrote: > On Thu, Feb 10, 2022 at 10:40:49AM -0300, Ariel D'Alessandro wrote: >> BSH SystemMaster (SMM) S2 PRO board comes with an audio card based on >> tlv320aic31xx family codec. >> >> The audio card exposes two playback devices, one of them using the EASRC >> (Enhanced Asynchronous Sample Rate Converter) module. Note that this >> would require SDMA and EASRC firmware in order to work. >> >> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com> >> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> >> --- >> .../dts/freescale/imx8mn-bsh-smm-s2pro.dts | 94 +++++++++++++++++++ >> 1 file changed, 94 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts b/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts >> index c6a8ed6745c1..3621354b4a92 100644 >> --- a/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts >> +++ b/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts >> @@ -7,6 +7,7 @@ >> /dts-v1/; >> >> #include "imx8mn-bsh-smm-s2-common.dtsi" >> +#include <dt-bindings/sound/tlv320aic31xx.h> >> >> / { >> model = "BSH SMM S2 PRO"; >> @@ -16,6 +17,69 @@ memory@40000000 { >> device_type = "memory"; >> reg = <0x0 0x40000000 0x0 0x20000000>; >> }; >> + >> + sound-tlv320aic31xx { >> + compatible = "fsl,imx-audio-tlv320aic31xx"; >> + model = "tlv320aic31xx-hifi"; >> + audio-cpu = <&sai3>; >> + audio-codec = <&codec>; >> + audio-asrc = <&easrc>; >> + audio-routing = >> + "Ext Spk", "SPL", >> + "Ext Spk", "SPR"; >> + mclk-id = <PLL_CLKIN_BCLK>; >> + }; >> + >> + vdd_input: vdd_input { >> + compatible = "regulator-fixed"; >> + regulator-name = "vdd_input"; >> + regulator-min-microvolt = <5000000>; >> + regulator-max-microvolt = <5000000>; >> + }; >> +}; >> + >> +&easrc { >> + fsl,asrc-rate = <48000>; > > Double spaces before '='. > >> + fsl,asrc-format = <10>; >> + status = "okay"; >> +}; >> + >> +&i2c2 { >> + clock-frequency = <400000>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_i2c2>; >> + status = "okay"; >> + >> + codec: tlv320dac3101@18 { > > tlv320dac3101: audio-codec@18 > >> + #sound-dai-cells = <0>; >> + compatible = "ti,tlv320dac3101"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_dac_rst>; >> + reg = <0x18>; >> + >> + ai31xx-micbias-vg = <MICBIAS_AVDDV>; >> + >> + HPVDD-supply = <&buck4_reg>; >> + SPRVDD-supply = <&vdd_input>; >> + SPLVDD-supply = <&vdd_input>; >> + AVDD-supply = <&buck4_reg>; >> + IOVDD-supply = <&buck4_reg>; >> + DVDD-supply = <&buck5_reg>; >> + reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>; >> + >> + clocks = <&clk IMX8MN_CLK_SAI3_ROOT>; >> + clock-names = "mclk"; >> + }; >> +}; >> + >> +&sai3 { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_sai3>; >> + assigned-clocks = <&clk IMX8MN_CLK_SAI3>; >> + assigned-clock-parents = <&clk IMX8MN_AUDIO_PLL1_OUT>; >> + assigned-clock-rates = <24576000>; >> + fsl,sai-mclk-direction-output; >> + status = "okay"; >> }; >> >> /* eMMC */ >> @@ -30,6 +94,36 @@ &usdhc1 { >> }; >> >> &iomuxc { >> + pinctrl_dac_rst: dac_rst { > > Name the node more in the same style as other pinctrl nodes. > > Shawn > >> + fsl,pins = < >> + MX8MN_IOMUXC_GPIO1_IO06_GPIO1_IO6 0x19 /* DAC_RST */ >> + >; >> + }; >> + >> + pinctrl_espi2: espi2grp { >> + fsl,pins = < >> + MX8MN_IOMUXC_ECSPI2_SCLK_ECSPI2_SCLK 0x082 >> + MX8MN_IOMUXC_ECSPI2_MOSI_ECSPI2_MOSI 0x082 >> + MX8MN_IOMUXC_ECSPI2_MISO_ECSPI2_MISO 0x082 >> + MX8MN_IOMUXC_ECSPI2_SS0_ECSPI2_SS0 0x040 >> + >; >> + }; >> + >> + pinctrl_i2c2: i2c2grp { >> + fsl,pins = < >> + MX8MN_IOMUXC_I2C2_SCL_I2C2_SCL 0x400000c3 >> + MX8MN_IOMUXC_I2C2_SDA_I2C2_SDA 0x400000c3 >> + >; >> + }; >> + >> + pinctrl_sai3: sai3grp { >> + fsl,pins = < >> + MX8MN_IOMUXC_SAI3_TXFS_SAI3_TX_SYNC 0xd6 >> + MX8MN_IOMUXC_SAI3_TXC_SAI3_TX_BCLK 0xd6 >> + MX8MN_IOMUXC_SAI3_TXD_SAI3_TX_DATA0 0xd6 >> + >; >> + }; >> + >> pinctrl_usdhc1: usdhc1grp { >> fsl,pins = < >> MX8MN_IOMUXC_SD1_CLK_USDHC1_CLK 0x40000090 >> -- >> 2.34.1 >>
Hi Ariel, On Thu, Feb 10, 2022 at 10:41 AM Ariel D'Alessandro <ariel.dalessandro@collabora.com> wrote: > +&i2c2 { > + clock-frequency = <400000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_i2c2>; > + status = "okay"; > + > + codec: tlv320dac3101@18 { > + #sound-dai-cells = <0>; > + compatible = "ti,tlv320dac3101"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_dac_rst>; > + reg = <0x18>; > + > + ai31xx-micbias-vg = <MICBIAS_AVDDV>; > + > + HPVDD-supply = <&buck4_reg>; > + SPRVDD-supply = <&vdd_input>; > + SPLVDD-supply = <&vdd_input>; > + AVDD-supply = <&buck4_reg>; > + IOVDD-supply = <&buck4_reg>; > + DVDD-supply = <&buck5_reg>; > + reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>; > + > + clocks = <&clk IMX8MN_CLK_SAI3_ROOT>; > + clock-names = "mclk"; The clocks and clock-names properties are not documented in the codec bindings. Also, the driver does not use call clk_get() on this mclk clock. You should drop the clocks and clock-names properties.
Hi Fabio, On 3/10/22 09:29, Fabio Estevam wrote: > Hi Ariel, > > On Thu, Feb 10, 2022 at 10:41 AM Ariel D'Alessandro > <ariel.dalessandro@collabora.com> wrote: > >> +&i2c2 { >> + clock-frequency = <400000>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_i2c2>; >> + status = "okay"; >> + >> + codec: tlv320dac3101@18 { >> + #sound-dai-cells = <0>; >> + compatible = "ti,tlv320dac3101"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_dac_rst>; >> + reg = <0x18>; >> + >> + ai31xx-micbias-vg = <MICBIAS_AVDDV>; >> + >> + HPVDD-supply = <&buck4_reg>; >> + SPRVDD-supply = <&vdd_input>; >> + SPLVDD-supply = <&vdd_input>; >> + AVDD-supply = <&buck4_reg>; >> + IOVDD-supply = <&buck4_reg>; >> + DVDD-supply = <&buck5_reg>; >> + reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>; >> + >> + clocks = <&clk IMX8MN_CLK_SAI3_ROOT>; >> + clock-names = "mclk"; > > The clocks and clock-names properties are not documented in the codec bindings. > > Also, the driver does not use call clk_get() on this mclk clock. > > You should drop the clocks and clock-names properties. The sound card driver is calling clk_get() on the codec's clock. See sound/soc/fsl/fsl-asoc-card.c: /* Get the MCLK rate only, and leave it controlled by CODEC drivers */ if (codec_dev) { struct clk *codec_clk = clk_get(codec_dev, NULL); if (!IS_ERR(codec_clk)) { priv->codec_priv.mclk_freq = clk_get_rate(codec_clk); clk_put(codec_clk); } } Regards, Ariel
Hi Fabio, On 3/17/22 15:47, Ariel D'Alessandro wrote: > Hi Fabio, > > On 3/10/22 09:29, Fabio Estevam wrote: >> Hi Ariel, >> >> On Thu, Feb 10, 2022 at 10:41 AM Ariel D'Alessandro >> <ariel.dalessandro@collabora.com> wrote: >> >>> +&i2c2 { >>> + clock-frequency = <400000>; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&pinctrl_i2c2>; >>> + status = "okay"; >>> + >>> + codec: tlv320dac3101@18 { >>> + #sound-dai-cells = <0>; >>> + compatible = "ti,tlv320dac3101"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&pinctrl_dac_rst>; >>> + reg = <0x18>; >>> + >>> + ai31xx-micbias-vg = <MICBIAS_AVDDV>; >>> + >>> + HPVDD-supply = <&buck4_reg>; >>> + SPRVDD-supply = <&vdd_input>; >>> + SPLVDD-supply = <&vdd_input>; >>> + AVDD-supply = <&buck4_reg>; >>> + IOVDD-supply = <&buck4_reg>; >>> + DVDD-supply = <&buck5_reg>; >>> + reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>; >>> + >>> + clocks = <&clk IMX8MN_CLK_SAI3_ROOT>; >>> + clock-names = "mclk"; >> >> The clocks and clock-names properties are not documented in the codec bindings. >> >> Also, the driver does not use call clk_get() on this mclk clock. >> >> You should drop the clocks and clock-names properties. > > The sound card driver is calling clk_get() on the codec's clock. See > sound/soc/fsl/fsl-asoc-card.c: > > /* Get the MCLK rate only, and leave it controlled by CODEC drivers */ > if (codec_dev) { > struct clk *codec_clk = clk_get(codec_dev, NULL); > > if (!IS_ERR(codec_clk)) { > priv->codec_priv.mclk_freq = clk_get_rate(codec_clk); > clk_put(codec_clk); > } > } Do you have any other comments on this one? Otherwise, could you perhaps ack this patch? See latest version: [PATCH v4] arm64: dts: imx8mn-bsh-smm-s2pro: Add tlv320aic31xx audio card node Thanks Ariel
diff --git a/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts b/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts index c6a8ed6745c1..3621354b4a92 100644 --- a/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts +++ b/arch/arm64/boot/dts/freescale/imx8mn-bsh-smm-s2pro.dts @@ -7,6 +7,7 @@ /dts-v1/; #include "imx8mn-bsh-smm-s2-common.dtsi" +#include <dt-bindings/sound/tlv320aic31xx.h> / { model = "BSH SMM S2 PRO"; @@ -16,6 +17,69 @@ memory@40000000 { device_type = "memory"; reg = <0x0 0x40000000 0x0 0x20000000>; }; + + sound-tlv320aic31xx { + compatible = "fsl,imx-audio-tlv320aic31xx"; + model = "tlv320aic31xx-hifi"; + audio-cpu = <&sai3>; + audio-codec = <&codec>; + audio-asrc = <&easrc>; + audio-routing = + "Ext Spk", "SPL", + "Ext Spk", "SPR"; + mclk-id = <PLL_CLKIN_BCLK>; + }; + + vdd_input: vdd_input { + compatible = "regulator-fixed"; + regulator-name = "vdd_input"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + }; +}; + +&easrc { + fsl,asrc-rate = <48000>; + fsl,asrc-format = <10>; + status = "okay"; +}; + +&i2c2 { + clock-frequency = <400000>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_i2c2>; + status = "okay"; + + codec: tlv320dac3101@18 { + #sound-dai-cells = <0>; + compatible = "ti,tlv320dac3101"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_dac_rst>; + reg = <0x18>; + + ai31xx-micbias-vg = <MICBIAS_AVDDV>; + + HPVDD-supply = <&buck4_reg>; + SPRVDD-supply = <&vdd_input>; + SPLVDD-supply = <&vdd_input>; + AVDD-supply = <&buck4_reg>; + IOVDD-supply = <&buck4_reg>; + DVDD-supply = <&buck5_reg>; + reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>; + + clocks = <&clk IMX8MN_CLK_SAI3_ROOT>; + clock-names = "mclk"; + }; +}; + +&sai3 { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_sai3>; + assigned-clocks = <&clk IMX8MN_CLK_SAI3>; + assigned-clock-parents = <&clk IMX8MN_AUDIO_PLL1_OUT>; + assigned-clock-rates = <24576000>; + fsl,sai-mclk-direction-output; + status = "okay"; }; /* eMMC */ @@ -30,6 +94,36 @@ &usdhc1 { }; &iomuxc { + pinctrl_dac_rst: dac_rst { + fsl,pins = < + MX8MN_IOMUXC_GPIO1_IO06_GPIO1_IO6 0x19 /* DAC_RST */ + >; + }; + + pinctrl_espi2: espi2grp { + fsl,pins = < + MX8MN_IOMUXC_ECSPI2_SCLK_ECSPI2_SCLK 0x082 + MX8MN_IOMUXC_ECSPI2_MOSI_ECSPI2_MOSI 0x082 + MX8MN_IOMUXC_ECSPI2_MISO_ECSPI2_MISO 0x082 + MX8MN_IOMUXC_ECSPI2_SS0_ECSPI2_SS0 0x040 + >; + }; + + pinctrl_i2c2: i2c2grp { + fsl,pins = < + MX8MN_IOMUXC_I2C2_SCL_I2C2_SCL 0x400000c3 + MX8MN_IOMUXC_I2C2_SDA_I2C2_SDA 0x400000c3 + >; + }; + + pinctrl_sai3: sai3grp { + fsl,pins = < + MX8MN_IOMUXC_SAI3_TXFS_SAI3_TX_SYNC 0xd6 + MX8MN_IOMUXC_SAI3_TXC_SAI3_TX_BCLK 0xd6 + MX8MN_IOMUXC_SAI3_TXD_SAI3_TX_DATA0 0xd6 + >; + }; + pinctrl_usdhc1: usdhc1grp { fsl,pins = < MX8MN_IOMUXC_SD1_CLK_USDHC1_CLK 0x40000090