Patchwork [4/5] arch/powerpc/boot/dts pcm030 add mpc5200-soc-audio node

login
register
mail settings
Submitter Eric Millbrandt
Date Sept. 12, 2012, 2:14 a.m.
Message ID <1347416089-23393-5-git-send-email-emillbrandt@dekaresearch.com>
Download mbox | patch
Permalink /patch/183235/
State Superseded
Delegated to: Anatolij Gustschin
Headers show

Comments

Eric Millbrandt - Sept. 12, 2012, 2:14 a.m.
Describe the audio codec on the pcm030 baseboard.

Signed-off-by: Eric Millbrandt <emillbrandt@dekaresearch.com>
---
 arch/powerpc/boot/dts/pcm030.dts |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)
Stephen Warren - Sept. 12, 2012, 2:41 a.m.
On 09/11/2012 08:14 PM, Eric Millbrandt wrote:
> Describe the audio codec on the pcm030 baseboard.

> +++ b/arch/powerpc/boot/dts/pcm030.dts

> +	sound {
> +		compatible = "fsl,mpc5200b-soc-audio","fsl,mpc5200-soc-audio";
> +		card-name = "pcm030";
> +		audio-platform = <&audio_platform>;
> +		number-of-dais = <2>;
> +
> +		analog@0 {

Purely from a DT perspective (i.e. I didn't look at the code that parses
this), you don't need to include the "@0" and "@1" in the node names,
because the two node names "analog" and "digital" are already unique.

However, in general I can see that you might want multiple analog DAIs.
There are a couple choices for differentiating the node names in that case:

1) If you want to use the "@0" unit address syntax in the node names to
differentiate them, each child node needs a reg property containing the
same value, and the parent node needs properties #address-cells=<1>,
#size-cells=<0>;

2) Or, since this is within an individual device binding rather than
something within a standardized bus, you can simply choose to make the
node names unique in some other way, such as "analog0", "analog1", i.e.
without the "@"; I believe the "@" syntax would be explicitly reserved
for representing a unit address as in (1).

Of course, I could be wrong about this assertion that the "@n" is
reserved for the unit address even with the privacy of an individual
binding; it'd be best to validate it by posting to the
devicetree-discuss mailing list.

> +			stream-name = "AC97 Analog";
> +			codec-name = "wm9712-codec.0";
> +			codec-dai-name = "wm9712-hifi";
> +			cpu-dai-name = "mpc5200-psc-ac97.0";
> +		};
> +
> +		digital@1 {
> +			stream-name = "AC97 IEC958";
> +			codec-name = "wm9712-codec.0";
> +			codec-dai-name = "wm9712-aux";
> +			cpu-dai-name = "mpc5200-psc-ac97.0";
> +		};
> +	};
>  };
Mark Brown - Sept. 12, 2012, 3:17 a.m.
On Tue, Sep 11, 2012 at 10:14:48PM -0400, Eric Millbrandt wrote:

> +		analog@0 {
> +			stream-name = "AC97 Analog";
> +			codec-name = "wm9712-codec.0";

This name is fairly clearly an internal implementation detail of how
Linux does audio drivers, we shouldn't be using it in device tree
bindings.  We should instead be doing something like referencing a
device tree node for the CODEC.  Look at how drivers like the nVidia
Tegra ones handle this, though for AC'97 we should really be able to
figure out a standard hookup like this automatically at runtime, it's
all enumerable.

Patch

diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts
index 9e35499..e9475e9 100644
--- a/arch/powerpc/boot/dts/pcm030.dts
+++ b/arch/powerpc/boot/dts/pcm030.dts
@@ -59,7 +59,7 @@ 
 			#gpio-cells = <2>;
 		};
 
-		psc@2000 { /* PSC1 in ac97 mode */
+		audio_platform: psc@2000 { /* PSC1 in ac97 mode */
 			compatible = "mpc5200b-psc-ac97","fsl,mpc5200b-psc-ac97";
 			cell-index = <0>;
 		};
@@ -134,4 +134,25 @@ 
 	localbus {
 		status = "disabled";
 	};
+
+	sound {
+		compatible = "fsl,mpc5200b-soc-audio","fsl,mpc5200-soc-audio";
+		card-name = "pcm030";
+		audio-platform = <&audio_platform>;
+		number-of-dais = <2>;
+
+		analog@0 {
+			stream-name = "AC97 Analog";
+			codec-name = "wm9712-codec.0";
+			codec-dai-name = "wm9712-hifi";
+			cpu-dai-name = "mpc5200-psc-ac97.0";
+		};
+
+		digital@1 {
+			stream-name = "AC97 IEC958";
+			codec-name = "wm9712-codec.0";
+			codec-dai-name = "wm9712-aux";
+			cpu-dai-name = "mpc5200-psc-ac97.0";
+		};
+	};
 };