Re: [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio

From: Philipp Zabel <p.zabel@pengutronix.de>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Andrew Jackson <Andrew.Jackson@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Jyri Sarha <jsarha@ti.com>, Mark Brown <broonie@kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio
Date: Mon, 12 Jan 2015 10:25:28 +0100	[thread overview]
Message-ID: <1421054728.3081.28.camel@pengutronix.de> (raw)
In-Reply-To: <20150109200127.GD12302@n2100.arm.linux.org.uk>

Am Freitag, den 09.01.2015, 20:01 +0000 schrieb Russell King - ARM
Linux:
[...]
> 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.

Jean-Francois' reply already reflects this, but the 'port' nodes should
correspond to physical ports of the device if possible. If you can
configure the device to have dedicated input pins for I2S, SPDIF0, and
SPDIF1 at the same time, they should appear in the device tree as
separate ports:

tda998x: hdmi-encoder {
	port@0 { /* pixel data according to video-ports */
		reg = <0x00>;
	};
	port@1 { /* AP1: SPDIF0 */
		reg = <0x01>;
	};
	port@2 { /* AP2: SPDIF1 */
		reg = <0x02>;
	};
	port@3 { /* AP3: I2S */
		reg = <0x03>;
	};
};

The tda998x binding would define how the ports are numbered, some
correspondence to the AP pin numbers would be good.

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel