Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
References: <20200421163935.775935-1-jbrunet@baylibre.com> <20200421163935.775935-3-jbrunet@baylibre.com> <63b05930-4814-f8f9-d1eb-2bd487ed3406@baylibre.com>
User-agent: mu4e 1.3.3; emacs 26.3
From:   Jerome Brunet <jbrunet@baylibre.com>
To:     Neil Armstrong <narmstrong@baylibre.com>,
        Kevin Hilman <khilman@baylibre.com>
Cc:     linux-amlogic@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/6] arm64: dts: meson-gx: add aiu support
In-reply-to: <63b05930-4814-f8f9-d1eb-2bd487ed3406@baylibre.com>
Date:   Wed, 22 Apr 2020 16:38:52 +0200
Message-ID: <1jimhrzitf.fsf@starbuckisacylon.baylibre.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On Wed 22 Apr 2020 at 15:12, Neil Armstrong <narmstrong@baylibre.com> wrote:

> On 21/04/2020 18:39, Jerome Brunet wrote:
>> Add the AIU audio device to the Amlogic GX SoC family DT.
>> ATM, this device provides the i2s and spdif output stages and also
>> the hdmi and internal codec glues.
>> 
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>  arch/arm64/boot/dts/amlogic/meson-gx.dtsi   | 13 ++++++++++++
>>  arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 23 +++++++++++++++++++++
>>  arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 23 +++++++++++++++++++++
>>  3 files changed, 59 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>> index 03f79fe045b7..a8dc8f810253 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
>> @@ -278,6 +278,17 @@ reset: reset-controller@4404 {
>>  				#reset-cells = <1>;
>>  			};
>>  
>> +			aiu: audio-controller@5400 {
>> +				compatible = "amlogic,aiu";
>> +				#sound-dai-cells = <2>;
>> +				sound-name-prefix = "AIU";
>> +				reg = <0x0 0x5400 0x0 0x2ac>;
>> +				interrupts = <GIC_SPI 48 IRQ_TYPE_EDGE_RISING>,
>> +					     <GIC_SPI 50 IRQ_TYPE_EDGE_RISING>;
>> +				interrupt-names = "i2s", "spdif";
>> +				status = "disabled";
>> +			};
>> +
>>  			uart_A: serial@84c0 {
>>  				compatible = "amlogic,meson-gx-uart";
>>  				reg = <0x0 0x84c0 0x0 0x18>;
>> @@ -626,6 +637,8 @@ hdmi_tx: hdmi-tx@c883a000 {
>>  			interrupts = <GIC_SPI 57 IRQ_TYPE_EDGE_RISING>;
>>  			#address-cells = <1>;
>>  			#size-cells = <0>;
>> +			#sound-dai-cells = <0>;
>> +			sound-name-prefix = "HDMITX";
>
> sound-name-prefix is not a valid property in Documentation/devicetree/bindings/display/amlogic,meson-dw-hdmi.yaml
>
> It should be added.

Not sure it should be explicitly added. It should be valid for any
device providing a sound-dai (sound-dai-cell)

>
> AFAIK sound-name-prefix is not present on g12-common, is it missing ?

It might not be strictly required, as long as there is no name colision
in the ASoC widget and Control, it is OK. IMHO, it is a good practice to
set one to avoid those colision.

>
> Neil
>
>>  			status = "disabled";
>>  
>>  			/* VPU VENC Input */
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> index 0cb40326b0d3..234490d3ee68 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
>> @@ -60,6 +60,29 @@ usb1: usb@c9100000 {
>>  	};
>>  };
>>  
>> +&aiu {
>> +	compatible = "amlogic,aiu-gxbb", "amlogic,aiu";
>> +	clocks = <&clkc CLKID_AIU_GLUE>,
>> +		 <&clkc CLKID_I2S_OUT>,
>> +		 <&clkc CLKID_AOCLK_GATE>,
>> +		 <&clkc CLKID_CTS_AMCLK>,
>> +		 <&clkc CLKID_MIXER_IFACE>,
>> +		 <&clkc CLKID_IEC958>,
>> +		 <&clkc CLKID_IEC958_GATE>,
>> +		 <&clkc CLKID_CTS_MCLK_I958>,
>> +		 <&clkc CLKID_CTS_I958>;
>> +	clock-names = "pclk",
>> +		      "i2s_pclk",
>> +		      "i2s_aoclk",
>> +		      "i2s_mclk",
>> +		      "i2s_mixer",
>> +		      "spdif_pclk",
>> +		      "spdif_aoclk",
>> +		      "spdif_mclk",
>> +		      "spdif_mclk_sel";
>> +	resets = <&reset RESET_AIU>;
>> +};
>> +
>>  &aobus {
>>  	pinctrl_aobus: pinctrl@14 {
>>  		compatible = "amlogic,meson-gxbb-aobus-pinctrl";
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> index 259d86399390..d9f00c5a9a5c 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
>> @@ -49,6 +49,29 @@ crypto: crypto@c883e000 {
>>  	};
>>  };
>>  
>> +&aiu {
>> +	compatible = "amlogic,aiu-gxl", "amlogic,aiu";
>> +	clocks = <&clkc CLKID_AIU_GLUE>,
>> +		 <&clkc CLKID_I2S_OUT>,
>> +		 <&clkc CLKID_AOCLK_GATE>,
>> +		 <&clkc CLKID_CTS_AMCLK>,
>> +		 <&clkc CLKID_MIXER_IFACE>,
>> +		 <&clkc CLKID_IEC958>,
>> +		 <&clkc CLKID_IEC958_GATE>,
>> +		 <&clkc CLKID_CTS_MCLK_I958>,
>> +		 <&clkc CLKID_CTS_I958>;
>> +	clock-names = "pclk",
>> +		      "i2s_pclk",
>> +		      "i2s_aoclk",
>> +		      "i2s_mclk",
>> +		      "i2s_mixer",
>> +		      "spdif_pclk",
>> +		      "spdif_aoclk",
>> +		      "spdif_mclk",
>> +		      "spdif_mclk_sel";
>> +	resets = <&reset RESET_AIU>;
>> +};
>> +
>>  &apb {
>>  	usb2_phy0: phy@78000 {
>>  		compatible = "amlogic,meson-gxl-usb2-phy";
>>