On 6/7/19 3:56 AM, Srinivas Kandagatla wrote:Yes, this version of the controller which am working on does not have DPN_SampleCtrl2 register for SampleIntervalHigh Field and has registers for programming offset2 does indeed indicate that these ports are reduced ports.
This patch adds bindings for Qualcomm soundwire controller.
Qualcomm SoundWire Master controller is present in most Qualcomm SoCs
either integrated as part of WCD audio codecs via slimbus or
as part of SOC I/O.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
---
 .../bindings/soundwire/qcom,swr.txt | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,swr.txt
you seem to use the 'swr' prefix in this patch. Most implementers use 'sdw', and that's the default also used in the MIPI DisCo spec for properties. Can we align on the same naming conventions?
diff --git a/Documentation/devicetree/bindings/soundwire/qcom,swr.txt b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
new file mode 100644
index 000000000000..eb84d0f4f36f
--- /dev/null
+++ b/Documentation/devicetree/bindings/soundwire/qcom,swr.txt
@@ -0,0 +1,62 @@
+Qualcomm SoundWire Controller
+
+This binding describes the Qualcomm SoundWire Controller Bindings.
+
+Required properties:
+
+- compatible:ÂÂÂÂÂÂÂ Must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>",
+ÂÂÂÂÂÂÂÂÂÂÂÂ example:
+ÂÂÂÂÂÂÂÂÂÂÂ "qcom,soundwire-v1.3.0"
+ÂÂÂÂÂÂÂÂÂÂÂ "qcom,soundwire-v1.5.0"
+ÂÂÂÂÂÂÂÂÂÂÂ "qcom,soundwire-v1.6.0"
+- reg:ÂÂÂÂÂÂÂÂÂÂÂ SoundWire controller address space.
+- interrupts:ÂÂÂÂÂÂÂ SoundWire controller interrupt.
+- clock-names:ÂÂÂÂÂÂÂ Must contain "iface".
+- clocks:ÂÂÂÂÂÂÂ Interface clocks needed for controller.
+- #sound-dai-cells:ÂÂÂ Must be 1 for digital audio interfaces on the controllers.
+- #address-cells:ÂÂÂ Must be 1 for SoundWire devices;
+- #size-cells:ÂÂÂÂÂÂÂ Must be <0> as SoundWire addresses have no size component.
+- qcom,dout-ports:ÂÂÂÂ Must be count of data out ports
+- qcom,din-ports:ÂÂÂÂ Must be count of data in ports
+- qcom,ports-offset1:ÂÂÂ Must be frame offset1 of each data port.
+ÂÂÂÂÂÂÂÂÂÂÂ Out followed by In. Used for Block size calculation.
+- qcom,ports-offset2:ÂÂÂÂ Must be frame offset2 of each data port.
+ÂÂÂÂÂÂÂÂÂÂÂ Out followed by In. Used for Block size calculation.
+- qcom,ports-sinterval-low: Must be sample interval low of each data port.
+ÂÂÂÂÂÂÂÂÂÂÂ Out followed by In. Used for Sample Interval calculation.
These definitions are valid only for specific types of ports, I believe here it's a 'reduced' port since offset2 is not required for simpler ports and you don't have Hstart/Hstop.
so if you state that all of these properties are required, you are explicitly ruling out future implementations of simple ports or will have to redefine them later.
Also the definition 'frame offset1/2' is incorrect. the offset is defined within each Payload Transport Window - not each frame - and its definition depends on the packing mode used, which isn't defined or stated here.
And last it looks like you assume a fixed frame shape - likely 50 rows by 8 columns, it might be worth adding a note on the max values for offset1/2 implied by this frame shape.