linux-kernel - Re: [alsa-devel] [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150110164711.637892fc@armhf>
Date:	Sat, 10 Jan 2015 16:47:11 +0100
From:	Jean-Francois Moine <moinejf@...e.fr>
To:	Russell King - ARM Linux <linux@....linux.org.uk>,
	Mark Brown <broonie@...nel.org>
Cc:	Philipp Zabel <p.zabel@...gutronix.de>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
	Andrew Jackson <Andrew.Jackson@....com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Jyri Sarha <jsarha@...com>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	Dave Airlie <airlied@...il.com>
Subject: Re: [alsa-devel] [PATCH v9 1/4] drm/i2c: tda998x: Add DT support
 for audio

On Fri, 9 Jan 2015 20:01:27 +0000
Russell King - ARM Linux <linux@....linux.org.uk> wrote:

> > I proposed:
> > 
> > 	audio-ports = <0x04>, <0x03>;
> > 	audio-port-names = "spdif", "i2s";
> > 
> > Do you better like:
> > 
> > 	audio-spdif-port = <0x04>;
> > 	audio-i2s-port = <0x03>;  
> 
> Neither; we know that there are TDA998x devices which allow SPDIF to be
> input via two separate pins, but only one to be active at any one time.
> Given that the hardware is flexible in that regard, a binding which
> artificially restricts that flexibility would seem unwise.
> 
> If we were to come across a setup which did route two SPDIF streams to
> the TDA998x, and we had to make the decision at run time which to route
> to the HDMI sink, we'd have to rework the binding, and we'd have to
> support the new binding and the old binding in the driver.
> 
> Can you please look at Documentation/devicetree/bindings/graph.txt ?
> 
> I think we may be able to use something like this:
> 
> tda998x: hdmi-encoder {
> 	compatible = "nxp,tda998x";
> 	reg = <0x70>;
> 	video-ports = <0x234501>;
> 
> 	port {
> 		tda998x_video: endpoint {
> 			remote-endpoint = <&lcd0_rgb>;
> 		};
> 	};
> 
> 	port {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		tda998x_spdif0: endpoint@02 {
> 			reg = <0x02>;
> 			remote-endpoint = <&spdif0>;
> 		};
> 
> 		tda998x_spdif1: endpoint@04 {
> 			reg = <0x04>;
> 			remote-endpoint = <&spdif0>;
> 		};
> 
> 		tda998x_i2s: endpoint@03 {
> 			reg = <0x03>;
> 			remote-endpoint = <&i2s>;
> 		};
> 	};
> };
> 
> I'm adding Philipp Zabel for comment.  The issue I see with this is that
> we have two ports here which are not mutually exclusive, and it's not
> obvious how they are dealt with.

Very interesting idea!

That's a detail, but I don't fully agree your endpoint description:

- pin description
	AP1 is the first S/PDIF input - ok
	AP2 is the second S/PDIF input - ok
	then, the I2S input may go only to AP3 or AP4.

- source input nodes
	You declare the same external S/PDIF source on AP1 and AP2.
	I would have better seen a kirkwood-like audio device as one of
	the S/PDIF input, say the second one.

To complexify a bit, I also connect the spdif wire of the kirkwood-like
device to a S/PDIF optical output ;)

spdif_in -------------+
                      v
       + i2s -----> tda998x ---> HDMI
audio1 |              ^
       + spdif -------+--------> spdif_out


Here are the modified ports of
- the HDMI transmitter (I removed the #xxx_cells):

	port@0 { // AP 1
		tda998x_spdif0: endpoint@02 {
			reg = <0x02>;
			remote-endpoint = <&spdif_in_port>;
		};
	};
	port@1 { //AP 2
		tda998x_spdif1: endpoint@04 {
			reg = <0x04>;
			remote-endpoint = <&audio1_spdif0>;
		};
	};
	port@2 { //AP 3
		tda998x_i2s: endpoint@08 {
			reg = <0x09>;
			remote-endpoint = <&audio1_i2s>;
		};
	};

- the (internal) audio device (audio1):

	port@0 {
		audio1_spdif0: endpoint@0 {
			reg = 0;
			remote-endpoint = <&tda998x_spdif1>;
		};
		audio1_spdif1: endpoint@1 {
			reg = 0;
			remote-endpoint = <&spdif_out_port>;
		};
	};
	port@1 {
		audio1_i2s: endpoint@1 {
			reg = 1;
			remote-endpoint = <&tda998x_i2s>;
		};
	};

- and the S/PDIF external input (spdif_in):

	port {
		spdif_in_port: endpoint@0 {
			reg = 0;
			remote-endpoint = <&tda998x_spdif0>;
		};
	};

- and the S/PDIF external output (spdif_out):

	port {
		spdif_out_port: endpoint@0 {
			reg = 0;
			remote-endpoint = <&audio1_spdif1>;
		};
	};

All the hardware is described. There is nothing more to add in the DT.

Especially, there is no 'simple-card' which is pure software and rather
Linux specific.

And, now, from this DT description, ASoC expects a sound card to be
built.

It seems that this creation should be done in the same way the video
cards are built, i.e. from the source devices, i.e. the kirkwood-like
device, and also from the spdif_in (which could be some other internal
audio device outputting s/pdif instead of a simple S/PDIF input
connector)! Is there one or two cards, and if 2 cards, how do they
share the tda998x?

Well, I will have a look at how to get audio out of my machine with
these new DT definitions (hopefully, there is only one audio source!).

Mark, you may forget about my other patch adding multi-codecs in the
simple-card...

Thanks.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/