diff mbox

greybus-spec: I2S: Try to add some minor clarifications and notes

Message ID 1422427548-15140-1-git-send-email-john.stultz@linaro.org
State New
Headers show

Commit Message

John Stultz Jan. 28, 2015, 6:45 a.m. UTC
Add some minor clarifications to try to improve the intro,
and add a few notes where I think further improvements
are needed.

Cc: Mark A. Greer <mgreer@animalcreek.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 source/bridged_phy.rst | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)
diff mbox

Patch

diff --git a/source/bridged_phy.rst b/source/bridged_phy.rst
index 91412cc..608c55b 100644
--- a/source/bridged_phy.rst
+++ b/source/bridged_phy.rst
@@ -1447,12 +1447,14 @@  the Module supports other Greybus Protocols.
     document.  This does not preclude the use of other
     data formats.
 
-An I2S Module shall contain one or more *I2S Bundles*.  Each I2S Bundle
-shall contain one *I2S Management CPort*, and may contain zero or
-more *I2S Transmitter CPorts* and zero or more *I2S Receiver CPorts*.
-There shall be at least one I2S Transmitter or Receiver CPort in each
-I2S Bundle.  An I2S Bundle may have no physical low-level I2S or
-similar hardware associated with it.
+An I2S Module shall contain one or more *I2S Bundles*. A bundle is a
+collection of CPorts on a module. Each I2S Bundle shall contain one *I2S
+Management CPort*, and may contain zero or more *I2S Transmitter CPorts*
+and zero or more *I2S Receiver CPorts*, which are controlled and
+configured by the I2S Management CPort in that bundle.  There shall be
+at least one I2S Transmitter or Receiver CPort in each I2S Bundle.  An
+I2S Bundle may have no physical low-level I2S or similar hardware
+associated with it.
 
 I2S Management CPorts, I2S Transmitter CPorts, and I2S Receiver CPorts
 have unique CPort Protocol values in the `protocol` field of the CPort
@@ -1476,6 +1478,15 @@  from the I2S Management CPort used by the AP Module to manage that
 I2S Bundle.  The AP Module shall treat the I2S Bundle in the AP
 Module no differently than an I2S Bundle in any other I2S Module.
 
+..	Personally I find the last few statements in the
+	above quite complicated, and difficult to understand.
+	If an AP module has a transmitter bundle, which includes
+	a management cport, why would it need an additional
+	management port for that bundle? Is the idea that
+	the management cport would be the other end of a control
+	connection (ie, connecting to localhost) to configure
+	the AP bridge transmission settings?  -jstultz
+
 Separate Management and Data Protocols
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
@@ -1611,6 +1622,14 @@  device.  The currently supported Low-level Interface Protocols are:
 Pulse Code Modulation (PCM), Inter-IC Sound (I2S), and Left-Right Stereo
 (LR Stereo).  They are described in more detail below.
 
+..	Personally I think we should be a bit more opinionated
+	in the spec and advise support for one low-level protocol.
+	We may also support other protocols for compatability,but
+	there should probably be some mention here that the
+	PCM or I2S protocol is the preferred/advised protocol
+	to be used. Or even we might only describe one but reserve
+	the bits needed to have the others work. -jstultz
+
 Sometimes Low-level Interface Protocols also specify the format of the
 audio data (e.g., I2S).  For this discussion, the audio data format is
 irrelevant and only the Low-level Interface Protocol is relevant.
@@ -1991,6 +2010,11 @@  the :ref:`i2s-management-protocol`.
 *   Activate the I2S Receiver CPort in the I2S Receiver Bundle.
 *   Activate the I2S Transmitter CPort in the I2S Transmitter Bundle.
 
+..	It might be good to explicitly mention the control
+	mechanism to create unipro connections between I2S
+	transmitter and reciever cports. This seems to gloss
+	over it a bit  -jstultz
+
 The I2S Transmitter Bundle may now stream audio data to the
 I2S Receiver Bundle using :ref:`i2s-send-data-op`\s.