On Oct 28, 2013, at 5:38 PM, Tomasz Figa wrote:

On Monday 28 of October 2013 01:37:34 Kumar Gala wrote:

quoted

On Oct 27, 2013, at 11:14 AM, Sebastian Reichel wrote:

quoted

Add device tree support for the spi variant of wl1251
and document the binding.

Signed-off-by: Sebastian Reichel <redacted>
---
.../devicetree/bindings/net/wireless/ti,wl1251.txt | 36
++++++++++++++++++++++ drivers/net/wireless/ti/wl1251/spi.c          
   | 23 ++++++++++---- 2 files changed, 53 insertions(+), 6
deletions(-)
create mode 100644
Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt

diff --git
a/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt new
file mode 100644
index 0000000..5f8a154

--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt

@@ -0,0 +1,36 @@
+* Texas Instruments wl1251 controller
+
+The wl1251 chip can be connected via SPI or via SDIO. The linux
+kernel currently only supports device tree for the SPI variant.
+

From the binding I have no idea what this chip actually does, also we
don't normally reference linux kernel support in bindings specs (so
please remove it).

However, what would expect the SDIO binding to look like?  Or more
specifically, how would you distinguish the SPI vs SDIO
binding/connection?  I'm wondering if the compatible should be
something like "ti,wl1251-spi" and than the sdio can be
"ti,wl1251-sdio"

Well, you can easily distinguish an SDIO device from an SPI device by its 
parent node, but...

The binding for SDIO might require different set of properties (other than 
ones inherited from generic SDIO or SPI bindings) than one for SPI. So 
probably different compatible values might be justified.

Did we already have such case before? (maybe some I2C + SPI devices?)

quoted

quoted

+Required properties:
+- compatible : Should be "ti,wl1251"

reg is not listed as a required prop.

It is implied by SPI bindings, but it might be a good idea to have this 
stated here as well.

quoted

quoted

+- interrupts : Should contain interrupt line
+- interrupt-parent : Should be the phandle for the interrupt
+  controller that services interrupts for this device
+- vio-supply : phandle to regulator providing VIO
+- power-gpio : GPIO connected to chip's PMEN pin

should be vendor prefixed: ti,power-gpio

Hmm, out of curiosity, is it a rule for this kind of properties? I can see 
both cases with and without prefixes when grepping for "-gpios" in 
Documentation/devicetree/bindings. We should really have such things 
written down somewhere.

Agreed, it should be part of the various docs we are suppose to produce for review and binding creation guidelines.

quoted

quoted

+- For additional required properties on SPI, please consult
+  Documentation/devicetree/bindings/spi/spi-bus.txt
+
+Optional properties:
+- ti,use-eeprom : If found, configuration will be loaded from eeprom.

can you be a bit more specific on what cfg will be loaded.  Also, is
this property a boolean, if so how do I know which eeprom the cfg is
loaded from (is it one that is directly connected to the wl1251?

Maybe one from ti,has-eeprom or ti,config-eeprom would be better name for 
this property?

Probably, ti,wl1251-has-eeprom or something like that would be better.  However, I'm not going to get too caught up on names of properties.

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation