mbox series

[v6,0/4] Enable wm8524 on i.MX8MQ-EVK

Message ID 20190308120141.3698-1-daniel.baluta@nxp.com
Headers show
Series Enable wm8524 on i.MX8MQ-EVK | expand

Message

Daniel Baluta March 8, 2019, 12:02 p.m. UTC
On i.MX8MQ EVK we can start the party using the wm8524 codec
which gets it's data through the SAI2 interface.

In order to make it work this patch series enables the SDMA nodes,
sets the correct pinctrl configuration and uses the simple card
machine driver to put everything together.

Changes since v5:
	- remove clocks and clock-names properties from wm8524
	code node as they are not used.

Changes since v4 (after Fabio's review)
	- removed mclk0 from sai2 node because it is not yet supported
	  by SAI driver
	- s/audio-codec-0/audio-codec
	- squashed patches 4 and 5

Changes since v3:
	- use "arm64: dts: imx8mq-evk" prefix for patches specific to
	  8MQ-EVK
	- squash together patches for setting up SAI (previous 3/5 and
	  4/5)
	- add back and document "fsl, imx8mq-sdma" compatbile string

Changes since v2:
        - s/QM/MQ after Chris comments

Changes since v1:
        - added cover letter
        - remove "fsl,imx8mq-sdma" compatible for sdma.

Daniel Baluta (4):
  arm64: dts: imx8mq: Add SDMA nodes
  bindings: fsl-imx-sdma: Document fsl,imx8mq-sdma compatbile string
  arm64: dts: imx8mq: Add SAI2 node
  arm64: dts: imx8mq-evk: Enable audio codec wm8952

 .../devicetree/bindings/dma/fsl-imx-sdma.txt  |  1 +
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts  | 46 +++++++++++++++++++
 arch/arm64/boot/dts/freescale/imx8mq.dtsi     | 37 +++++++++++++++
 3 files changed, 84 insertions(+)

Comments

Fabio Estevam March 8, 2019, 12:06 p.m. UTC | #1
On Fri, Mar 8, 2019 at 9:02 AM Daniel Baluta <daniel.baluta@nxp.com> wrote:
>
> SAI2 is part of AIPS-3 memory region.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

Reviewed-by: Fabio Estevam <festevam@gmail.com>
Fabio Estevam March 8, 2019, 12:06 p.m. UTC | #2
On Fri, Mar 8, 2019 at 9:02 AM Daniel Baluta <daniel.baluta@nxp.com> wrote:
>
> The main Audio DAC used on the EVK board is wm8952.
> The EVK provides the MCLK to wm8952.
>
> Digital interface is SAI2 which includes three signals:
> SYNC_CLK, BCLK and DACDAT.
>
> This patch sets:
>         * SAI2 pinctrl configuration
>         * clock hierarchy
>         * wm8960 codec
>
> Then uses simple-card machine driver to connect them
> into a sound card.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

Reviewed-by: Fabio Estevam <festevam@gmail.com>
Shawn Guo March 19, 2019, 2:31 p.m. UTC | #3
On Fri, Mar 08, 2019 at 12:02:22PM +0000, Daniel Baluta wrote:
> SDMA1 is part of AIPS-3 region and SDMA2 is part
> of AIPS-1 region.
> 
> Reviewed-by: Fabio Estevam <festevam@gmail.com>

Reviewed-by tag should go after your SoB, since that's the sequence of
how they come. 

> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> [initial submit in i.MX internal tree]
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> [adaptation for linux-next]

I would see these notes in commit log or even just cover letter.

Shawn

> ---
>  arch/arm64/boot/dts/freescale/imx8mq.dtsi | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index 9155bd4784eb..9d48450453fb 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -234,6 +234,17 @@
>  				status = "disabled";
>  			};
>  
> +			sdma2: sdma@302c0000 {
> +				compatible = "fsl,imx8mq-sdma","fsl,imx7d-sdma";
> +				reg = <0x302c0000 0x10000>;
> +				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MQ_CLK_SDMA2_ROOT>,
> +					 <&clk IMX8MQ_CLK_SDMA2_ROOT>;
> +				clock-names = "ipg", "ahb";
> +				#dma-cells = <3>;
> +				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
> +			};
> +
>  			iomuxc: iomuxc@30330000 {
>  				compatible = "fsl,imx8mq-iomuxc";
>  				reg = <0x30330000 0x10000>;
> @@ -575,6 +586,17 @@
>  				status = "disabled";
>  			};
>  
> +			sdma1: sdma@30bd0000 {
> +				compatible = "fsl, imx8mq-sdma","fsl,imx7d-sdma";
> +				reg = <0x30bd0000 0x10000>;
> +				interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&clk IMX8MQ_CLK_SDMA1_ROOT>,
> +					 <&clk IMX8MQ_CLK_SDMA1_ROOT>;
> +				clock-names = "ipg", "ahb";
> +				#dma-cells = <3>;
> +				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
> +			};
> +
>  			fec1: ethernet@30be0000 {
>  				compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec";
>  				reg = <0x30be0000 0x10000>;
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Shawn Guo March 19, 2019, 2:41 p.m. UTC | #4
On Fri, Mar 08, 2019 at 12:02:28PM +0000, Daniel Baluta wrote:
> The main Audio DAC used on the EVK board is wm8952.
> The EVK provides the MCLK to wm8952.
> 
> Digital interface is SAI2 which includes three signals:
> SYNC_CLK, BCLK and DACDAT.
> 
> This patch sets:
> 	* SAI2 pinctrl configuration
> 	* clock hierarchy
> 	* wm8960 codec
> 
> Then uses simple-card machine driver to connect them
> into a sound card.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>

Please rebase to my for-next branch.

> ---
>  arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 46 ++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> index 54737bf1772f..09000ee34757 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
> @@ -31,6 +31,33 @@
>  		gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
>  		enable-active-high;
>  	};
> +
> +	wm8524: audio-codec {
> +		#sound-dai-cells = <0>;
> +		compatible = "wlf,wm8524";
> +		wlf,mute-gpios = <&gpio1 8 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	sound-wm8524 {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,name = "wm8524-audio";
> +		simple-audio-card,format = "i2s";
> +		simple-audio-card,frame-master = <&cpudai>;
> +		simple-audio-card,bitclock-master = <&cpudai>;
> +		simple-audio-card,widgets =
> +			"Line", "Left Line Out Jack",
> +			"Line", "Right Line Out Jack";
> +		simple-audio-card,routing =
> +			"Left Line Out Jack", "LINEVOUTL",
> +			"Right Line Out Jack", "LINEVOUTR";

Please have a newline between properties and child node ...

> +		cpudai: simple-audio-card,cpu {
> +			sound-dai = <&sai2>;
> +		};

... and between nodes as well.

> +		link_codec: simple-audio-card,codec {
> +			sound-dai = <&wm8524>;
> +			clocks = <&clk IMX8MQ_CLK_SAI2_ROOT>;
> +		};
> +	};
>  };
>  
>  &fec1 {
> @@ -52,6 +79,15 @@
>  	};
>  };
>  
> +&sai2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_sai2>;
> +	assigned-clocks = <&clk IMX8MQ_CLK_SAI2>;
> +	assigned-clock-parents = <&clk IMX8MQ_AUDIO_PLL1_OUT>;
> +	assigned-clock-rates = <24576000>;
> +	status = "okay";
> +};
> +
>  &i2c1 {
>  	clock-frequency = <100000>;
>  	pinctrl-names = "default";
> @@ -223,6 +259,16 @@
>  		>;
>  	};
>  
> +	pinctrl_sai2: sai2grp {

Please try to keep pinctrl nodes sort alphabetically.

Shawn

> +		fsl,pins = <
> +			MX8MQ_IOMUXC_SAI2_TXFS_SAI2_TX_SYNC     0xd6
> +			MX8MQ_IOMUXC_SAI2_TXC_SAI2_TX_BCLK      0xd6
> +			MX8MQ_IOMUXC_SAI2_MCLK_SAI2_MCLK        0xd6
> +			MX8MQ_IOMUXC_SAI2_TXD0_SAI2_TX_DATA0    0xd6
> +			MX8MQ_IOMUXC_GPIO1_IO08_GPIO1_IO8       0xd6
> +		>;
> +	};
> +
>  	pinctrl_i2c1: i2c1grp {
>  		fsl,pins = <
>  			MX8MQ_IOMUXC_I2C1_SCL_I2C1_SCL			0x4000007f
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Angus Ainslie March 26, 2019, 9:44 p.m. UTC | #5
On 2019-03-19 07:31, Shawn Guo wrote:
> On Fri, Mar 08, 2019 at 12:02:22PM +0000, Daniel Baluta wrote:
>> SDMA1 is part of AIPS-3 region and SDMA2 is part
>> of AIPS-1 region.
>> 
>> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> 
> Reviewed-by tag should go after your SoB, since that's the sequence of
> how they come.
> 
>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>> [initial submit in i.MX internal tree]
>> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
>> [adaptation for linux-next]
> 
> I would see these notes in commit log or even just cover letter.
> 
> Shawn
> 
>> ---
>>  arch/arm64/boot/dts/freescale/imx8mq.dtsi | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
>> b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
>> index 9155bd4784eb..9d48450453fb 100644
>> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
>> @@ -234,6 +234,17 @@
>>  				status = "disabled";
>>  			};
>> 
>> +			sdma2: sdma@302c0000 {
>> +				compatible = "fsl,imx8mq-sdma","fsl,imx7d-sdma";
>> +				reg = <0x302c0000 0x10000>;
>> +				interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
>> +				clocks = <&clk IMX8MQ_CLK_SDMA2_ROOT>,
>> +					 <&clk IMX8MQ_CLK_SDMA2_ROOT>;
>> +				clock-names = "ipg", "ahb";
>> +				#dma-cells = <3>;
>> +				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
>> +			};
>> +
>>  			iomuxc: iomuxc@30330000 {
>>  				compatible = "fsl,imx8mq-iomuxc";
>>  				reg = <0x30330000 0x10000>;
>> @@ -575,6 +586,17 @@
>>  				status = "disabled";
>>  			};
>> 
>> +			sdma1: sdma@30bd0000 {
>> +				compatible = "fsl, imx8mq-sdma","fsl,imx7d-sdma";

There shouldn't be a space in the imx8mq compatible string.

Angus

>> +				reg = <0x30bd0000 0x10000>;
>> +				interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
>> +				clocks = <&clk IMX8MQ_CLK_SDMA1_ROOT>,
>> +					 <&clk IMX8MQ_CLK_SDMA1_ROOT>;
>> +				clock-names = "ipg", "ahb";
>> +				#dma-cells = <3>;
>> +				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
>> +			};
>> +
>>  			fec1: ethernet@30be0000 {
>>  				compatible = "fsl,imx8mq-fec", "fsl,imx6sx-fec";
>>  				reg = <0x30be0000 0x10000>;
>> --
>> 2.17.1
>> 
>> 
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Daniel Baluta March 27, 2019, 9:51 a.m. UTC | #6
On Tue, Mar 26, 2019 at 11:44 PM Angus Ainslie <angus@akkea.ca> wrote:
>
> On 2019-03-19 07:31, Shawn Guo wrote:
> > On Fri, Mar 08, 2019 at 12:02:22PM +0000, Daniel Baluta wrote:
> >> SDMA1 is part of AIPS-3 region and SDMA2 is part
> >> of AIPS-1 region.
> >>
> >> Reviewed-by: Fabio Estevam <festevam@gmail.com>
> >
> > Reviewed-by tag should go after your SoB, since that's the sequence of
> > how they come.
> >
> >> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> >> [initial submit in i.MX internal tree]
> >> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> >> [adaptation for linux-next]
> >
> > I would see these notes in commit log or even just cover letter.
> >
> > Shawn
> >
> >> ---
> >>  arch/arm64/boot/dts/freescale/imx8mq.dtsi | 22 ++++++++++++++++++++++
> >>  1 file changed, 22 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> >> b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> >> index 9155bd4784eb..9d48450453fb 100644
> >> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> >> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> >> @@ -234,6 +234,17 @@
> >>                              status = "disabled";
> >>                      };
> >>
> >> +                    sdma2: sdma@302c0000 {
> >> +                            compatible = "fsl,imx8mq-sdma","fsl,imx7d-sdma";
> >> +                            reg = <0x302c0000 0x10000>;
> >> +                            interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> >> +                            clocks = <&clk IMX8MQ_CLK_SDMA2_ROOT>,
> >> +                                     <&clk IMX8MQ_CLK_SDMA2_ROOT>;
> >> +                            clock-names = "ipg", "ahb";
> >> +                            #dma-cells = <3>;
> >> +                            fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
> >> +                    };
> >> +
> >>                      iomuxc: iomuxc@30330000 {
> >>                              compatible = "fsl,imx8mq-iomuxc";
> >>                              reg = <0x30330000 0x10000>;
> >> @@ -575,6 +586,17 @@
> >>                              status = "disabled";
> >>                      };
> >>
> >> +                    sdma1: sdma@30bd0000 {
> >> +                            compatible = "fsl, imx8mq-sdma","fsl,imx7d-sdma";
>
> There shouldn't be a space in the imx8mq compatible string.

Oh, nice catch. This patch series is already in so care to send a fix?

Otherwise, I will take care of it.

thanks,
Daniel.
Angus Ainslie March 27, 2019, 1:25 p.m. UTC | #7
On 2019-03-27 02:51, Daniel Baluta wrote:
> On Tue, Mar 26, 2019 at 11:44 PM Angus Ainslie <angus@akkea.ca> wrote:
>> 
>> On 2019-03-19 07:31, Shawn Guo wrote:
>> > On Fri, Mar 08, 2019 at 12:02:22PM +0000, Daniel Baluta wrote:
>> >> SDMA1 is part of AIPS-3 region and SDMA2 is part
>> >> of AIPS-1 region.
>> >>
>> >> Reviewed-by: Fabio Estevam <festevam@gmail.com>
>> >
>> > Reviewed-by tag should go after your SoB, since that's the sequence of
>> > how they come.
>> >
>> >> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>> >> [initial submit in i.MX internal tree]
>> >> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
>> >> [adaptation for linux-next]
>> >
>> > I would see these notes in commit log or even just cover letter.
>> >
>> > Shawn
>> >
>> >> ---
>> >>  arch/arm64/boot/dts/freescale/imx8mq.dtsi | 22 ++++++++++++++++++++++
>> >>  1 file changed, 22 insertions(+)
>> >>
>> >> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
>> >> b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
>> >> index 9155bd4784eb..9d48450453fb 100644
>> >> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
>> >> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
>> >> @@ -234,6 +234,17 @@
>> >>                              status = "disabled";
>> >>                      };
>> >>
>> >> +                    sdma2: sdma@302c0000 {
>> >> +                            compatible = "fsl,imx8mq-sdma","fsl,imx7d-sdma";
>> >> +                            reg = <0x302c0000 0x10000>;
>> >> +                            interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
>> >> +                            clocks = <&clk IMX8MQ_CLK_SDMA2_ROOT>,
>> >> +                                     <&clk IMX8MQ_CLK_SDMA2_ROOT>;
>> >> +                            clock-names = "ipg", "ahb";
>> >> +                            #dma-cells = <3>;
>> >> +                            fsl,sdma-ram-script-name = "imx/sdma/sdma-imx7d.bin";
>> >> +                    };
>> >> +
>> >>                      iomuxc: iomuxc@30330000 {
>> >>                              compatible = "fsl,imx8mq-iomuxc";
>> >>                              reg = <0x30330000 0x10000>;
>> >> @@ -575,6 +586,17 @@
>> >>                              status = "disabled";
>> >>                      };
>> >>
>> >> +                    sdma1: sdma@30bd0000 {
>> >> +                            compatible = "fsl, imx8mq-sdma","fsl,imx7d-sdma";
>> 
>> There shouldn't be a space in the imx8mq compatible string.
> 
> Oh, nice catch. This patch series is already in so care to send a fix?
> 
> Otherwise, I will take care of it.

I'm preparing some other fixes so I can add it.

Angus

> 
> thanks,
> Daniel.