diff mbox

[v8,6/6] ARM: dts: am335x-boneblack: Add HDMI audio support

Message ID dd3484868647610840d4803dd3378014ce1c95b7.1458214526.git.jsarha@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jyri Sarha March 17, 2016, 12:22 p.m. UTC
Add HDMI audio support. Adds mcasp0_pins, clk_mcasp0_fixed,
clk_mcasp0, mcasp0, sound node, and updates the tda19988 node to
follow the new binding.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 arch/arm/boot/dts/am335x-boneblack.dts | 71 ++++++++++++++++++++++++++++++++--
 1 file changed, 67 insertions(+), 4 deletions(-)

Comments

Jean-Francois Moine March 19, 2016, 6:39 a.m. UTC | #1
On Thu, 17 Mar 2016 14:22:34 +0200
Jyri Sarha <jsarha@ti.com> wrote:

> @@ -76,16 +87,22 @@
>  };
>  
>  &i2c0 {
> -	tda19988 {
> +	tda19988: tda19988 {
>  		compatible = "nxp,tda998x";
>  		reg = <0x70>;
> +
>  		pinctrl-names = "default", "off";
>  		pinctrl-0 = <&nxp_hdmi_bonelt_pins>;
>  		pinctrl-1 = <&nxp_hdmi_bonelt_off_pins>;
>  
> -		port {
> -			hdmi_0: endpoint@0 {
> -				remote-endpoint = <&lcdc_0>;
> +		#sound-dai-cells = <0>;
> +		audio-ports = <	AFMT_I2S	0x03>;
> +
> +		ports {
> +			port@0 {
> +				hdmi_0: endpoint@0 {
> +					remote-endpoint = <&lcdc_0>;
> +				};
>  			};
>  		};
>  	};

Why did you add a 'ports' container? As there is only one port,
it is useless.

Also, you don't need '@0' in the 'port' and 'endpoint'.
If you want to keep it, you must add 'reg = 0;'s.
Jyri Sarha March 21, 2016, 7:53 a.m. UTC | #2
On 03/19/16 08:39, Jean-Francois Moine wrote:
> On Thu, 17 Mar 2016 14:22:34 +0200
> Jyri Sarha <jsarha@ti.com> wrote:
> 
>> @@ -76,16 +87,22 @@
>>  };
>>  
>>  &i2c0 {
>> -	tda19988 {
>> +	tda19988: tda19988 {
>>  		compatible = "nxp,tda998x";
>>  		reg = <0x70>;
>> +
>>  		pinctrl-names = "default", "off";
>>  		pinctrl-0 = <&nxp_hdmi_bonelt_pins>;
>>  		pinctrl-1 = <&nxp_hdmi_bonelt_off_pins>;
>>  
>> -		port {
>> -			hdmi_0: endpoint@0 {
>> -				remote-endpoint = <&lcdc_0>;
>> +		#sound-dai-cells = <0>;
>> +		audio-ports = <	AFMT_I2S	0x03>;
>> +
>> +		ports {
>> +			port@0 {
>> +				hdmi_0: endpoint@0 {
>> +					remote-endpoint = <&lcdc_0>;
>> +				};
>>  			};
>>  		};
>>  	};
> 
> Why did you add a 'ports' container? As there is only one port,
> it is useless.
> 

Well, I just left it there. The node parsing code should handle the
container if it follows graph[1] binding. Putting the ports container
node is equally correct as leaving it out, but surely I can leave it out
next time (when ever it comes).

> Also, you don't need '@0' in the 'port' and 'endpoint'.
> If you want to keep it, you must add 'reg = 0;'s.
> 

I am still not sure about the "must" part. From all that I have read[2]
the unit number should be the same as reg property if possible. But then
again if the reg property consists of multiple addresses, how do you put
them in the unit address? To me no unit address at all is an analogue
situation and I see no point in adding an artificial reg property if I
need multiple nodes by same name. This actually happens quite often with
sound-nodes when I have multiple audio devices in the same system.

But still, there is no need for the unit address here and it should be
removed. I'll do that next time.

Best regard,
Jyri

[1] Documentation/devicetree/bindings/graph.txt
[2] http://devicetree.org/Device_Tree_Usage
diff mbox

Patch

diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
index 55c0e95..2bae4d1 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -9,6 +9,7 @@ 
 
 #include "am33xx.dtsi"
 #include "am335x-bone-common.dtsi"
+#include <dt-bindings/display/tda998x.h>
 
 / {
 	model = "TI AM335x BeagleBone Black";
@@ -64,6 +65,16 @@ 
 			AM33XX_IOPAD(0x9b0, PIN_OUTPUT_PULLDOWN | MUX_MODE3)	/* xdma_event_intr0 */
 		>;
 	};
+
+	mcasp0_pins: mcasp0_pins {
+		pinctrl-single,pins = <
+			AM33XX_IOPAD(0x9ac, PIN_INPUT_PULLUP | MUX_MODE0) /* mcasp0_ahcklx.mcasp0_ahclkx */
+			AM33XX_IOPAD(0x99c, PIN_OUTPUT_PULLDOWN | MUX_MODE2) /* mcasp0_ahclkr.mcasp0_axr2*/
+			AM33XX_IOPAD(0x994, PIN_OUTPUT_PULLUP | MUX_MODE0) /* mcasp0_fsx.mcasp0_fsx */
+			AM33XX_IOPAD(0x990, PIN_OUTPUT_PULLDOWN | MUX_MODE0) /* mcasp0_aclkx.mcasp0_aclkx */
+			AM33XX_IOPAD(0x86c, PIN_OUTPUT_PULLDOWN | MUX_MODE7) /* gpmc_a11.GPIO1_27 */
+		>;
+	};
 };
 
 &lcdc {
@@ -76,16 +87,22 @@ 
 };
 
 &i2c0 {
-	tda19988 {
+	tda19988: tda19988 {
 		compatible = "nxp,tda998x";
 		reg = <0x70>;
+
 		pinctrl-names = "default", "off";
 		pinctrl-0 = <&nxp_hdmi_bonelt_pins>;
 		pinctrl-1 = <&nxp_hdmi_bonelt_off_pins>;
 
-		port {
-			hdmi_0: endpoint@0 {
-				remote-endpoint = <&lcdc_0>;
+		#sound-dai-cells = <0>;
+		audio-ports = <	AFMT_I2S	0x03>;
+
+		ports {
+			port@0 {
+				hdmi_0: endpoint@0 {
+					remote-endpoint = <&lcdc_0>;
+				};
 			};
 		};
 	};
@@ -94,3 +111,49 @@ 
 &rtc {
 	system-power-controller;
 };
+
+&mcasp0	{
+	#sound-dai-cells = <0>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&mcasp0_pins>;
+	status = "okay";
+	op-mode = <0>;	/* MCASP_IIS_MODE */
+	tdm-slots = <2>;
+	serial-dir = <	/* 0: INACTIVE, 1: TX, 2: RX */
+			0 0 1 0
+		>;
+	tx-num-evt = <32>;
+	rx-num-evt = <32>;
+};
+
+/ {
+	clk_mcasp0_fixed: clk_mcasp0_fixed {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <24576000>;
+	};
+
+	clk_mcasp0: clk_mcasp0 {
+		#clock-cells = <0>;
+		compatible = "gpio-gate-clock";
+		clocks = <&clk_mcasp0_fixed>;
+		enable-gpios = <&gpio1 27 0>; /* BeagleBone Black Clk enable on GPIO1_27 */
+	};
+
+	sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,name = "TI BeagleBone Black";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,bitclock-master = <&dailink0_master>;
+		simple-audio-card,frame-master = <&dailink0_master>;
+
+		dailink0_master: simple-audio-card,cpu {
+			sound-dai = <&mcasp0>;
+			clocks = <&clk_mcasp0>;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&tda19988>;
+		};
+	};
+};