Re: [PATCH] dt: bindings: add bindings for ipq4019 wifi block

From: Raja Mani
Date: Wed Jan 13 2016 - 01:00:45 EST

On Wednesday 13 January 2016 07:53 AM, Rob Herring wrote:
On Wed, Dec 30, 2015 at 11:41 PM, Raja Mani <rmani@xxxxxxxxxxxxxxxx> wrote:

On Wednesday 30 December 2015 10:05 PM, Rob Herring wrote:

On Wed, Dec 23, 2015 at 11:05:15AM +0530, Raja Mani wrote:

Add device tree binding documentation details for wifi block present
in Qualcomm IPQ4019 SoC into qcom,ath10k.txt.

Signed-off-by: Raja Mani <rmani@xxxxxxxxxxxxxxxx>
.../bindings/net/wireless/qcom,ath10k.txt | 87
1 file changed, 82 insertions(+), 5 deletions(-)

diff --git
index edefc26..ffd0742 100644
--- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
@@ -1,17 +1,42 @@
* Qualcomm Atheros ath10k wireless devices

-For ath10k devices the calibration data can be provided through Device
-Tree. The node is a child node of the PCI controller.

So it is now not a PCI device?

Right now, ath10k wireless driver has support for PCI devices. There is
a plan to extend ath10k driver to support wifi devices which are connected
over AHB as well (enumeration will happen via device tree node).

Qualcomm IPQ4019 SoC has two inbuilt wifi block which are connected over AHB
bus (not connected over PCI bus) and also has one external PCI
slot where we can connect standalone wifi PCI devices.

In future, ath10k would support both PCI and AHB. For PCI based
devices, only calibration data is supplied via device tree. For AHB
based devices (ie, ipq4019), all wifi properties are supplied
via device tree (including irq, reg addr, cal data,etc).

Put this information in the binding. It is not clear and you seem to
be removing PCI information.

Okay, I'll fix it in next version.

Required properties:
--compatible : Should be "qcom,ath10k"
+- compatible: Should be one of the following:
+ * "qcom,ath10k"
+ * "qcom,ipq4019-wifi"

One is a standalone PCI device and one is embedded block in an SOC?

Yes, it's possible to have this combination (one in SoC + one in
standalone PCI device in the same platform).

These should be more separated as all these new properties would only
apply in the latter case.

Sorry, i didn't get this point. I mentioned it under optional
properties. Whichever properties applies to particular wifi, only
those parameters are needed. For example, for PCI based devices,
only calibration data is needed, for AHB based devices most the
properties are needed.

Is it fine if add below text some thing like this ?

"only "qcom,ath10k-calibration-data" is applicable for PCI based
devices, rest of the members are applicable only for AHB based

Yes, but you need to explain in terms of compatible strings. Which
compatible strings correspond to PCI devices.

Sure, i'll add this detail.

Optional properties:
+- reg: Address and length of the register set for the device.
+- core-id: Core number associated to the device.

This needs a better explanation.

Sure, Let me add below text in next version.

"core-id is numeric number associated to the wifi block.
For example, 0 means first block, 1 means second wifi block,etc."

Are the blocks the same? If not, then use different compatible
strings. Otherwise doesn't the reg property identify which block is
which? Or some other property? Different freq bands? We generally
don't allow indexes like this in the DT, so I need to understand how
you use this.


I understand the confusion and the limitation, i'll remove core-id in next version and manage this in driver itself.

Could you please consider v3 of this patch for further review? I hope,
i addressed all your review comments.