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 >>> 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?) > >>> +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. - k -- 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-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
