diff mbox

[v2,1/2] dt: binding: Add Qualcomm WCNSS control binding

Message ID 1463095068-32225-1-git-send-email-bjorn.andersson@linaro.org
State New
Headers show

Commit Message

Bjorn Andersson May 12, 2016, 11:17 p.m. UTC
This binding describes the control interface for the Qualcomm WCNSS.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

---

Changes since v1:
- Introduce reference to wcnss block node for register block definition
- Use wcnss block node compatible for hw version detection (riva vs pronto)
- Clean up compatible name for bt node

 .../devicetree/bindings/soc/qcom/qcom,wcnss.txt    | 115 +++++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt

-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Bjorn Andersson May 27, 2016, 4:28 a.m. UTC | #1
On Thu 26 May 20:58 PDT 2016, Rob Herring wrote:

> +dt list

> 

> On Thu, May 12, 2016 at 6:17 PM, Bjorn Andersson

> <bjorn.andersson@linaro.org> wrote:

[..]
> > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt

> > new file mode 100644

> > index 000000000000..5413098a1e1a

> > --- /dev/null

> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt

> > @@ -0,0 +1,115 @@

> > +Qualcomm WCNSS Binding

> > +

> > +This binding describes the Qualcomm WCNSS hardware. It consists of control

> > +block and a BT, WiFi and FM radio block, all useing SMD as command channels.

> 

> s/useing/using/

> 


Right.

> > +

> > +- compatible:

> > +       Usage: required

> > +       Value type: <string>

> > +       Definition: must be: "qcom,wcnss",

> > +

> > +- qcom,smd-channel:

> > +       Usage: required

> > +       Value type: <string>

> > +       Definition: standard SMD property specifying the SMD channel used for

> > +                   communication with the WiFi firmware

> 

> Is the string value defined somewhere?

> 


It's never been anything else than WCNSS_CTRL so I guess I could
document that.

> > +

> > +- qcom,mmio:

> > +       Usage: required

> > +       Value type: <prop-encoded-array>

> > +       Definition: reference to a node specifying the wcnss "ccu" and "dxe"

> > +                   register blocks. The node must be compatible with one of

> > +                   the following:

> > +                   "qcom,riva",

> > +                   "qcom,pronto"

> > +

[..]
> > +

> > +- qcom,state:

> 

> This is kind of generic.

> 


We can rename it qcom,smem-state to make it less likely to collide in
the future. I believe this is the first binding referencing one of
those.

> > +       Usage: required

> > +       Value type: <prop-encoded-array>

> > +       Definition: should reference the tx-enable and tx-rings-empty SMEM states

> > +

> > +- qcom,state-names:


And then qcom,smem-state-names here

> > +       Usage: required

> > +       Value type: <stringlist>

> > +       Definition: must contain "tx-enable" and "tx-rings-empty"

> > +

> > += EXAMPLE

> > +The following example represents a SMD node, with one edge representing the

> > +"pronto" subsystem, with the wcnss device and its wcn3680 BT and WiFi blocks

> > +described; as found on the 8974 platform.

> 

> So somewhere in here I'd expect to see 8974 in a compatible string.

> 


The "Wireless CoNnectivity Sub-System" version is "pronto" and this is
the only thing that has a hardware dependency. So the only part that
makes this 8974 and not 8016 is the placing of those blocks in the
memory space (and the interrupts).

And the compatible qcom,iris would make this a 8064 node.

> > +

> > +smd {

> > +       compatible = "qcom,smd";

> > +

> > +       pronto {

> > +               interrupts = <0 142 1>;

> > +

> > +               qcom,ipc = <&apcs 8 17>;

> > +               qcom,smd-edge = <6>;

> > +

> > +               wcnss {

> > +                       compatible = "qcom,wcnss";

> > +                       qcom,smd-channels = "WCNSS_CTRL";

> > +

> > +                       #address-cells = <1>;

> > +                       #size-cells = <1>;

> > +

> > +                       qcom,wcnss = <&pronto>;

> 

> Not documented and poorly named. The property name should reflect what

> this is for or points too.

> 


It's documented as qcom,mmio, so the example is wrong.

> > +

> > +                       bt {

> > +                               compatible = "qcom,wcnss-bt";

> > +                       };

> > +

> > +                       wlan {

> > +                               compatible = "qcom,wcnss-wlan";

> > +

> > +                               interrupts = <0 145 0>, <0 146 0>;

> > +                               interrupt-names = "tx", "rx";

> > +

> > +                               qcom,state = <&apps_smsm 10>, <&apps_smsm 9>;

> > +                               qcom,state-names = "tx-enable", "tx-rings-empty";

> > +                       };

> > +               };

> > +       };

> > +};

> > +

> > +soc {

> > +       pronto: pronto {

> 

> 2 pronto nodes? That's confusing...

> 


This one denotes the pronto register space, the one above denotes the
communication channel to the firmware running on the cpu in the wcnss.

But in SMD terms the one above is the "pronto edge", so I can change
that to pronto-edge.

> > +               compatible = "qcom,pronto";

> > +

> > +               reg = <0xfb204000 0x2000>, <0xfb202000 0x1000>, <0xfb21b000 0x3000>;

> > +               reg-names = "ccu", "dxe", "pmu";

> > +       };

> > +};


Thanks for your review.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
new file mode 100644
index 000000000000..5413098a1e1a
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
@@ -0,0 +1,115 @@ 
+Qualcomm WCNSS Binding
+
+This binding describes the Qualcomm WCNSS hardware. It consists of control
+block and a BT, WiFi and FM radio block, all useing SMD as command channels.
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be: "qcom,wcnss",
+
+- qcom,smd-channel:
+	Usage: required
+	Value type: <string>
+	Definition: standard SMD property specifying the SMD channel used for
+		    communication with the WiFi firmware
+
+- qcom,mmio:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: reference to a node specifying the wcnss "ccu" and "dxe"
+		    register blocks. The node must be compatible with one of
+		    the following:
+		    "qcom,riva",
+		    "qcom,pronto"
+
+= SUBNODES
+The subnodes of the wcnss node are optional and describe the individual blocks in
+the WCNSS.
+
+== Bluetooth
+The following properties are defined to the bluetooth node:
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be:
+		    "qcom,wcnss-bt"
+
+== WiFi
+The following properties are defined to the WiFi node:
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,wcnss-wlan",
+
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: should specify the "rx" and "tx" interrupts
+
+- interrupt-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: must contain "rx" and "tx"
+
+- qcom,state:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: should reference the tx-enable and tx-rings-empty SMEM states
+
+- qcom,state-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: must contain "tx-enable" and "tx-rings-empty"
+
+= EXAMPLE
+The following example represents a SMD node, with one edge representing the
+"pronto" subsystem, with the wcnss device and its wcn3680 BT and WiFi blocks
+described; as found on the 8974 platform.
+
+smd {
+	compatible = "qcom,smd";
+
+	pronto {
+		interrupts = <0 142 1>;
+
+		qcom,ipc = <&apcs 8 17>;
+		qcom,smd-edge = <6>;
+
+		wcnss {
+			compatible = "qcom,wcnss";
+			qcom,smd-channels = "WCNSS_CTRL";
+
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			qcom,wcnss = <&pronto>;
+
+			bt {
+				compatible = "qcom,wcnss-bt";
+			};
+
+			wlan {
+				compatible = "qcom,wcnss-wlan";
+
+				interrupts = <0 145 0>, <0 146 0>;
+				interrupt-names = "tx", "rx";
+
+				qcom,state = <&apps_smsm 10>, <&apps_smsm 9>;
+				qcom,state-names = "tx-enable", "tx-rings-empty";
+			};
+		};
+	};
+};
+
+soc {
+	pronto: pronto {
+		compatible = "qcom,pronto";
+
+		reg = <0xfb204000 0x2000>, <0xfb202000 0x1000>, <0xfb21b000 0x3000>;
+		reg-names = "ccu", "dxe", "pmu";
+	};
+};