[PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Bjorn Andersson]

This binding describes the control interface for the Qualcomm WCNSS.

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

Got a reviewed-by from Andy and acked-by from Rob on the WiFi part of this
binding. But during futher testing I spotted a timing issue, where the
wcnss_ctrl driver must finish the uploading of NV to the core os of the wcnss
before the wifi driver can initiate any communication with the wifi part
(although available from the get-go).

The sanest way I could figure to model this is to describe the core part as
parent of the wifi and bt pieces.

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

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..5488904b6185
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
@@ -0,0 +1,104 @@
+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
+
+= 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,btqcomsmd"
+
+== WiFi
+The following properties are defined to the WiFi node:
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,wcn3620-wlan",
+		    "qcom,wcn3660-wlan",
+		    "qcom,wcn3680-wlan"
+
+- qcom,wcnss-mmio:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: should specify base address and size of the WiFi related
+		    registers of WCNSS
+
+- 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>;
+
+			bt {
+				compatible = "qcom,btqcomsmd";
+			};
+
+			wifi {
+				compatible = "qcom,wcn3680-wlan";
+
+				qcom,wcnss-mmio = <0xfb000000 0x21b000>;
+
+				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";
+			};
+		};
+	};
+};
-- 
2.5.0

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Rob Herring]

On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson wrote:
> This binding describes the control interface for the Qualcomm WCNSS.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> 
> Got a reviewed-by from Andy and acked-by from Rob on the WiFi part of this
> binding. But during futher testing I spotted a timing issue, where the
> wcnss_ctrl driver must finish the uploading of NV to the core os of the wcnss
> before the wifi driver can initiate any communication with the wifi part
> (although available from the get-go).
> 
> The sanest way I could figure to model this is to describe the core part as
> parent of the wifi and bt pieces.
> 
>  .../devicetree/bindings/soc/qcom/qcom,wcnss.txt    | 104 +++++++++++++++++++++
>  1 file changed, 104 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
> 
> 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..5488904b6185
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
> @@ -0,0 +1,104 @@
> +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",

This should be more specific.


> +- qcom,smd-channel:
> +	Usage: required
> +	Value type: <string>
> +	Definition: standard SMD property specifying the SMD channel used for
> +		    communication with the WiFi firmware
> +
> += 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,btqcomsmd"

This should be more specific to the chip and there's no need to have 
qcom twice.

> +
> +== WiFi
> +The following properties are defined to the WiFi node:
> +
> +- compatible:
> +	Usage: required
> +	Value type: <string>
> +	Definition: must be one of:
> +		    "qcom,wcn3620-wlan",
> +		    "qcom,wcn3660-wlan",
> +		    "qcom,wcn3680-wlan"
> +
> +- qcom,wcnss-mmio:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: should specify base address and size of the WiFi related
> +		    registers of WCNSS

This is an address visible to the cpu?

> +
> +- 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>;
> +
> +			bt {
> +				compatible = "qcom,btqcomsmd";
> +			};
> +
> +			wifi {
> +				compatible = "qcom,wcn3680-wlan";
> +
> +				qcom,wcnss-mmio = <0xfb000000 0x21b000>;
> +
> +				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";
> +			};
> +		};
> +	};
> +};
> -- 
> 2.5.0
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Bjorn Andersson]

On Thu 31 Mar 07:28 PDT 2016, Rob Herring wrote:

> On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson 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..5488904b6185
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,wcnss.txt
> > @@ -0,0 +1,104 @@
> > +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",
> 
> This should be more specific.
> 

Okay, will have to go back to Qualcomm and try to figure out what kind
of version there actually is on this component.

[..]
> > += 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,btqcomsmd"
> 
> This should be more specific to the chip and there's no need to have 
> qcom twice.
> 

There's only a single implementation of this downstream, so same answer
as above...

> > +
> > +== WiFi
> > +The following properties are defined to the WiFi node:
> > +
> > +- compatible:
> > +	Usage: required
> > +	Value type: <string>
> > +	Definition: must be one of:
> > +		    "qcom,wcn3620-wlan",
> > +		    "qcom,wcn3660-wlan",
> > +		    "qcom,wcn3680-wlan"

Digging through documentation and trying to answer the questions above
made me realize that these numbers are for the external rf component,
not the variants of the logic inside the SoC; and as such wrong.

> > +
> > +- qcom,wcnss-mmio:
> > +	Usage: required
> > +	Value type: <prop-encoded-array>
> > +	Definition: should specify base address and size of the WiFi related
> > +		    registers of WCNSS
> 
> This is an address visible to the cpu?
> 

Yes it is; the device is controlled both through SMD and mmio accessible
registers, where the SMD interface is the primary interface.

SMD being the primary "bus" I believe I can't use reg to denote this
register range. Should I describe this in some other form?

Regards,
Bjorn

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Rob Herring]

On Thu, Mar 31, 2016 at 12:16 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Thu 31 Mar 07:28 PDT 2016, Rob Herring wrote:
>
>> On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson wrote:
> [..]

[...]

>> > +
>> > +== WiFi
>> > +The following properties are defined to the WiFi node:
>> > +
>> > +- compatible:
>> > +   Usage: required
>> > +   Value type: <string>
>> > +   Definition: must be one of:
>> > +               "qcom,wcn3620-wlan",
>> > +               "qcom,wcn3660-wlan",
>> > +               "qcom,wcn3680-wlan"
>
> Digging through documentation and trying to answer the questions above
> made me realize that these numbers are for the external rf component,
> not the variants of the logic inside the SoC; and as such wrong.

Do you need to know both? Or only the firmware image needs to know?

>> > +
>> > +- qcom,wcnss-mmio:
>> > +   Usage: required
>> > +   Value type: <prop-encoded-array>
>> > +   Definition: should specify base address and size of the WiFi related
>> > +               registers of WCNSS
>>
>> This is an address visible to the cpu?
>>
>
> Yes it is; the device is controlled both through SMD and mmio accessible
> registers, where the SMD interface is the primary interface.
>
> SMD being the primary "bus" I believe I can't use reg to denote this
> register range. Should I describe this in some other form?

That's a tricky one. I would create a node for the memory-mapped
portion with proper compatible and reg properties, and then make this
a phandle to that node. Something similar to how we do phandles to
syscon's.

Rob

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Bjorn Andersson]

On Thu 31 Mar 10:38 PDT 2016, Rob Herring wrote:

> On Thu, Mar 31, 2016 at 12:16 PM, Bjorn Andersson
> <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Thu 31 Mar 07:28 PDT 2016, Rob Herring wrote:
> >
> >> On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson wrote:
> > [..]
> 
> [...]
> 
> >> > +
> >> > +== WiFi
> >> > +The following properties are defined to the WiFi node:
> >> > +
> >> > +- compatible:
> >> > +   Usage: required
> >> > +   Value type: <string>
> >> > +   Definition: must be one of:
> >> > +               "qcom,wcn3620-wlan",
> >> > +               "qcom,wcn3660-wlan",
> >> > +               "qcom,wcn3680-wlan"
> >
> > Digging through documentation and trying to answer the questions above
> > made me realize that these numbers are for the external rf component,
> > not the variants of the logic inside the SoC; and as such wrong.
> 
> Do you need to know both? Or only the firmware image needs to know?
> 

So far I've only found cases where we need to know the register map for
the DMA engine shuffling packets, so this is related to the SoC-internal
part only.

The differences in RF capabilities - at least for WiFi - seems to be
acquired in runtime from the firmware.

The other piece that depend on the RF part seems to be the availability
of e.g. ANT support, so if anything that needs to go into the wcnss
node, in some way (either compatible or the set of subnodes).

> >> > +
> >> > +- qcom,wcnss-mmio:
> >> > +   Usage: required
> >> > +   Value type: <prop-encoded-array>
> >> > +   Definition: should specify base address and size of the WiFi related
> >> > +               registers of WCNSS
> >>
> >> This is an address visible to the cpu?
> >>
> >
> > Yes it is; the device is controlled both through SMD and mmio accessible
> > registers, where the SMD interface is the primary interface.
> >
> > SMD being the primary "bus" I believe I can't use reg to denote this
> > register range. Should I describe this in some other form?
> 
> That's a tricky one. I would create a node for the memory-mapped
> portion with proper compatible and reg properties, and then make this
> a phandle to that node. Something similar to how we do phandles to
> syscon's.
> 

Okay, sounds reasonable. I don't see a need for a specific
implementation, so I'll just back it with the generic syscon
implementation (and a specific compatible).

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Rob Herring]

On Thu, Mar 31, 2016 at 1:24 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Thu 31 Mar 10:38 PDT 2016, Rob Herring wrote:
>
>> On Thu, Mar 31, 2016 at 12:16 PM, Bjorn Andersson
>> <bjorn.andersson@linaro.org> wrote:
>> > On Thu 31 Mar 07:28 PDT 2016, Rob Herring wrote:
>> >
>> >> On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson wrote:
>> > [..]
>>
>> [...]
>>
>> >> > +
>> >> > +== WiFi
>> >> > +The following properties are defined to the WiFi node:
>> >> > +
>> >> > +- compatible:
>> >> > +   Usage: required
>> >> > +   Value type: <string>
>> >> > +   Definition: must be one of:
>> >> > +               "qcom,wcn3620-wlan",
>> >> > +               "qcom,wcn3660-wlan",
>> >> > +               "qcom,wcn3680-wlan"
>> >
>> > Digging through documentation and trying to answer the questions above
>> > made me realize that these numbers are for the external rf component,
>> > not the variants of the logic inside the SoC; and as such wrong.
>>
>> Do you need to know both? Or only the firmware image needs to know?
>>
>
> So far I've only found cases where we need to know the register map for
> the DMA engine shuffling packets, so this is related to the SoC-internal
> part only.
>
> The differences in RF capabilities - at least for WiFi - seems to be
> acquired in runtime from the firmware.
>
> The other piece that depend on the RF part seems to be the availability
> of e.g. ANT support, so if anything that needs to go into the wcnss
> node, in some way (either compatible or the set of subnodes).
>
>> >> > +
>> >> > +- qcom,wcnss-mmio:
>> >> > +   Usage: required
>> >> > +   Value type: <prop-encoded-array>
>> >> > +   Definition: should specify base address and size of the WiFi related
>> >> > +               registers of WCNSS
>> >>
>> >> This is an address visible to the cpu?
>> >>
>> >
>> > Yes it is; the device is controlled both through SMD and mmio accessible
>> > registers, where the SMD interface is the primary interface.
>> >
>> > SMD being the primary "bus" I believe I can't use reg to denote this
>> > register range. Should I describe this in some other form?
>>
>> That's a tricky one. I would create a node for the memory-mapped
>> portion with proper compatible and reg properties, and then make this
>> a phandle to that node. Something similar to how we do phandles to
>> syscon's.
>>
>
> Okay, sounds reasonable. I don't see a need for a specific
> implementation, so I'll just back it with the generic syscon
> implementation (and a specific compatible).

I don't think I'd do syscon here as it is mainly designed to have
multiple users. You just need to look-up the phandle, perhaps check
the compatible, and call of_address_to_resource to get the address.
Actually, you could skip the phandle entirely and just find the node
by compatible (assuming there is only one).

Rob

Re: [PATCH 3/5] dt-binding: Add Qualcomm WCNSS control binding

[Bjorn Andersson]

On Thu 31 Mar 13:42 PDT 2016, Rob Herring wrote:

> On Thu, Mar 31, 2016 at 1:24 PM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > On Thu 31 Mar 10:38 PDT 2016, Rob Herring wrote:
> >
> >> On Thu, Mar 31, 2016 at 12:16 PM, Bjorn Andersson
> >> <bjorn.andersson@linaro.org> wrote:
> >> > On Thu 31 Mar 07:28 PDT 2016, Rob Herring wrote:
> >> >
> >> >> On Mon, Mar 28, 2016 at 09:35:24PM -0700, Bjorn Andersson wrote:
> >> > [..]
> >>
> >> [...]
> >>
> >> >> > +
> >> >> > +== WiFi
> >> >> > +The following properties are defined to the WiFi node:
> >> >> > +
> >> >> > +- compatible:
> >> >> > +   Usage: required
> >> >> > +   Value type: <string>
> >> >> > +   Definition: must be one of:
> >> >> > +               "qcom,wcn3620-wlan",
> >> >> > +               "qcom,wcn3660-wlan",
> >> >> > +               "qcom,wcn3680-wlan"
> >> >
> >> > Digging through documentation and trying to answer the questions above
> >> > made me realize that these numbers are for the external rf component,
> >> > not the variants of the logic inside the SoC; and as such wrong.
> >>
> >> Do you need to know both? Or only the firmware image needs to know?
> >>
> >
> > So far I've only found cases where we need to know the register map for
> > the DMA engine shuffling packets, so this is related to the SoC-internal
> > part only.
> >
> > The differences in RF capabilities - at least for WiFi - seems to be
> > acquired in runtime from the firmware.
> >
> > The other piece that depend on the RF part seems to be the availability
> > of e.g. ANT support, so if anything that needs to go into the wcnss
> > node, in some way (either compatible or the set of subnodes).
> >
> >> >> > +
> >> >> > +- qcom,wcnss-mmio:
> >> >> > +   Usage: required
> >> >> > +   Value type: <prop-encoded-array>
> >> >> > +   Definition: should specify base address and size of the WiFi related
> >> >> > +               registers of WCNSS
> >> >>
> >> >> This is an address visible to the cpu?
> >> >>
> >> >
> >> > Yes it is; the device is controlled both through SMD and mmio accessible
> >> > registers, where the SMD interface is the primary interface.
> >> >
> >> > SMD being the primary "bus" I believe I can't use reg to denote this
> >> > register range. Should I describe this in some other form?
> >>
> >> That's a tricky one. I would create a node for the memory-mapped
> >> portion with proper compatible and reg properties, and then make this
> >> a phandle to that node. Something similar to how we do phandles to
> >> syscon's.
> >>
> >
> > Okay, sounds reasonable. I don't see a need for a specific
> > implementation, so I'll just back it with the generic syscon
> > implementation (and a specific compatible).
> 
> I don't think I'd do syscon here as it is mainly designed to have
> multiple users. You just need to look-up the phandle, perhaps check
> the compatible, and call of_address_to_resource to get the address.
> Actually, you could skip the phandle entirely and just find the node
> by compatible (assuming there is only one).
> 

Ahh, right. Thanks for the suggestion.

Regards,
Bjorn