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]
Date:	Wed, 14 Jan 2015 12:12:34 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Jean-Francois Moine <moinejf@...e.fr>
Cc:	Jyri Sarha <jsarha@...com>, Philipp Zabel <p.zabel@...gutronix.de>,
	Andrew Jackson <Andrew.Jackson@....com>,
	Mark Brown <broonie@...nel.org>,
	Dave Airlie <airlied@...il.com>,
	"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v9 1/4] drm/i2c: tda998x: Add DT support for audio

On Wed, Jan 14, 2015 at 08:55:01AM +0100, Jean-Francois Moine wrote:
> On Tue, 13 Jan 2015 19:54:15 +0000
> Russell King - ARM Linux <linux@....linux.org.uk> wrote:
> 
> > On Tue, Jan 13, 2015 at 09:41:01PM +0200, Jyri Sarha wrote:
> > > On 01/13/2015 09:26 PM, Russell King - ARM Linux wrote:
> > > >SCLK: _~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_~_
> > > >   WS: __~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~________________________________~
> > > >I2S1: llmm............................llmm............................llm
> > > >I2S2: llmm............................llmm............................llm
> > > >I2S3: llmm............................llmm............................llm
> > > >I2S4: llmm............................llmm............................llm
> > > >
> > > >So, what I'm saying is that it is_impossible_  to drive the TDA998x using
> > > >multiple I2S streams which are not produced by the same I2S block.
> > > 
> > > This is besides the point, but it is possible that one of the multiple I2S
> > > blocks is the bit-clock and frame-clock master to the i2s bus and the others
> > > are slaves to it (banging their bits according to SCLK and WS of the I2S
> > > master). However, in this situation there really is only one i2s bus with
> > > multiple data pins.
> > > 
> > > Just my 0.02€ to this discussion.
> > 
> > Right, that's about the only way it could work.
> > 
> > To represent that in DT, I would imagine we'd need something like this:
> > 
> > 	#address-cells = <1>;
> > 	#size-cells = <0>;
> > 	...
> >         port@1 {                        /* AP1,2 = I2S */
> > 		#address-cells = <1>;
> > 		#size-cells = <0>;
> >                 port-type = "i2s";
> >                 reg = <0x01>;		/* WS */
> >                 tda998x_i2s1: endpoint@2 {
> > 			reg = <0x02>;	/* AP1 */
> >                         remote-endpoint = <&audio1_i2s>;
> >                 };
> >                 tda998x_i2s2: endpoint@4 {
> > 			reg = <0x04>;	/* AP2 */
> >                         remote-endpoint = <&audio2_i2s>;
> >                 };
> >         };
> > 
> > where audio1_i2s is operating in master mode, and audio2_i2s is
> > operating in slave mode for both WS and SCLK.
> > 
> > If we can agree on that, then I'm happy with the proposed binding.
> > (Remember that #address-cells and #size-cells are required in the
> > parent where we have reg= in the child.)
> 
> #address-cells and #size-cells may be defined in any of the upper
> parent, so, as it is defined in the device, it is not needed in the
> port (see of_n_addr_cells in drivers/of/base.c).

That may be, but the documentation specifies that it should be given.
See Documentation/devicetree/bindings/graph.txt - maybe the docs need
fixing?

> Well, I am a bit lost between (only one source / many channels - I2S
> busses) and (many sources / one I2s bus!).

I think that's a matter of how the problem is viewed. :)

> Anyway, I don't understand your example.
> I'd expect that a port would be a DAI.

I view a DAI as a Linux abstraction, which doesn't really have any place
in DT.  I'm looking at this problem from the electronics point of view.

> If yes, when playing is active, both endpoints receive an audio stream
> from the remote audio devices, and the AP register is always set to
> 0x07 (= 0x01 | 0x02 | 0x04).
> Then, it would have been simpler to have:
> 
>  	#address-cells = <1>;
>  	#size-cells = <0>;
>  	...
>          port@7 {                        /* AP1,2 = I2S */
>                  port-type = "i2s";
>                  reg = <0x07>;		/* WS + AP1 + AP2 */
>                  tda998x_i2s1: endpoint@1 {
>                          remote-endpoint = <&audio1_i2s>;
>                  };
>                  tda998x_i2s2: endpoint@2 {
>                          remote-endpoint = <&audio2_i2s>;
>                  };
>          };
> 
> If no, the two DAIs must be created from the endpoints, permitting
> streaming on i2s1 alone, i2s2 alone or both i2s1+i2s2 at the same time.

How does that work - I mean, if we have i2s1 as the SCLK and WS master,
how can i2s2 run without i2s1?

I suppose I2S2 could be reprogrammed to be the master for both signals,
provided that I2S1 is programmed to be a slave before hand, but that
seems to be making things excessively complex (we'd need some way for
I2S1 and I2S2 to comunicate that state between themselves and negotiate
which to be the master.)

However, I'd suggest that if audio1_i2s always maps to HDMI front
left/right, audio1_i2s must always be enabled when audio playback is
required; there is no CA bit combination provided in HDMI where the
front L/R channels are optional.  Hence, I'd suggest that audio1_i2s
must always be the master.

As we don't know, I'd suggest that we permit flexibility here.  We
can use the reg property as you have below, and we just bitwise-or
the of_endpoint port/id together, along with that result from other
active audio endpoints.  The advantage of that is we can use any of
these representations, and it should result in a working setup - and
when we do have the necessary information to make a properly informed
decision, we can update the DT and binding doc accordingly and we
won't have to update the driver (nor will we break backwards compat.)

Sound sane?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ