Re: [PATCH 4/4] wl1251: spi: add device tree support
From: Kumar Gala
Date: Tue Oct 29 2013 - 04:29:20 EST
On Oct 28, 2013, at 5:38 PM, Tomasz Figa wrote:
> On Monday 28 of October 2013 01:37:34 Kumar Gala wrote:
>> On Oct 27, 2013, at 11:14 AM, Sebastian Reichel wrote:
>>> Add device tree support for the spi variant of wl1251
>>> and document the binding.
>>> Signed-off-by: Sebastian Reichel <sre@xxxxxxxxxx>
>>> .../devicetree/bindings/net/wireless/ti,wl1251.txt | 36
>>> ++++++++++++++++++++++ drivers/net/wireless/ti/wl1251/spi.c
>>> | 23 ++++++++++---- 2 files changed, 53 insertions(+), 6
>>> create mode 100644
>>> diff --git
>>> 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
> 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?)
>>> +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.
>>> +- 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.
>>> +- 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.
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/