Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753898Ab3J2I3O (ORCPT ); Tue, 29 Oct 2013 04:29:14 -0400 Received: from smtp.codeaurora.org ([198.145.11.231]:37292 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753426Ab3J2I2l convert rfc822-to-8bit (ORCPT ); Tue, 29 Oct 2013 04:28:41 -0400 Subject: Re: [PATCH 4/4] wl1251: spi: add device tree support Mime-Version: 1.0 (Apple Message framework v1283) Content-Type: text/plain; charset=us-ascii From: Kumar Gala In-Reply-To: <4893578.SBstXOAcJY@flatron> Date: Tue, 29 Oct 2013 03:28:39 -0500 Cc: linux-arm-kernel@lists.infradead.org, Sebastian Reichel , Mark Rutland , linux-doc@vger.kernel.org, Tony Lindgren , Russell King , Sachin Kamat , Ian Campbell , Sebastian Reichel , Luciano Coelho , devicetree@vger.kernel.org, Pawel Moll , Stephen Warren , "John W. Linville" , Rob Herring , Bill Pemberton , linux-omap@vger.kernel.org, Greg Kroah-Hartman , linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org, Felipe Balbi , Rob Landley , netdev@vger.kernel.org Content-Transfer-Encoding: 8BIT Message-Id: <11FD245D-02A1-4E29-A23A-66A1D64210DC@codeaurora.org> References: <1382890469-25286-1-git-send-email-sre@debian.org> <1382890469-25286-5-git-send-email-sre@debian.org> <4893578.SBstXOAcJY@flatron> To: Tomasz Figa X-Mailer: Apple Mail (2.1283) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3977 Lines: 100 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 >>> --- >>> .../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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/