Return-path: Received: from arroyo.ext.ti.com ([192.94.94.40]:45103 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755198Ab3F1KcK (ORCPT ); Fri, 28 Jun 2013 06:32:10 -0400 Message-ID: <1372415514.21065.50.camel@cumari.coelho.fi> (sfid-20130628_123241_148774_59CCE525) Subject: Re: [PATCH] Documentation: dt: bindings: TI WiLink modules From: Luciano Coelho To: Mark Rutland , CC: "grant.likely@linaro.org" , "rob.herring@calxeda.com" , "devicetree-discuss@lists.ozlabs.org" , "linux-doc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-omap@vger.kernel.org" , "linux-wireless@vger.kernel.org" , "linux-arm@vger.kernel.org" Date: Fri, 28 Jun 2013 13:31:54 +0300 In-Reply-To: <20130628102145.GB2139@e106331-lin.cambridge.arm.com> References: <1372149330-24335-1-git-send-email-coelho@ti.com> <20130628093848.GA2139@e106331-lin.cambridge.arm.com> <1372413215.21065.41.camel@cumari.coelho.fi> <20130628102145.GB2139@e106331-lin.cambridge.arm.com> Content-Type: text/plain; charset="UTF-8" MIME-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: (fixed Mike's address) On Fri, 2013-06-28 at 11:21 +0100, Mark Rutland wrote: > On Fri, Jun 28, 2013 at 10:53:35AM +0100, Luciano Coelho wrote: > > On Fri, 2013-06-28 at 10:38 +0100, Mark Rutland wrote: > > > On Tue, Jun 25, 2013 at 09:35:30AM +0100, Luciano Coelho wrote: > > > > +Optional properties: > > > > +-------------------- > > > > + > > > > +- refclock: the internal WLAN reference clock frequency (required for > > > > + WiLink6 and WiLink7; not used for WiLink8). Must be one of the > > > > + following: > > > > + 0 = 19.2 MHz > > > > + 1 = 26.0 MHz > > > > + 2 = 38.4 MHz > > > > + 3 = 52.0 MHz > > > > + 4 = 38.4 MHz, XTAL > > > > + 5 = 26.0 MHz, XTAL > > > > + > > > > +- tcxoclock: the internal WLAN TCXO clock frequency (required for > > > > + WiLink7 not used for WiLink6 and WiLink8). Must be one of the > > > > + following: > > > > + 0 = 19.200 MHz > > > > + 1 = 26.000 MHz > > > > + 2 = 38.400 MHz > > > > + 3 = 52.000 MHz > > > > + 4 = 16.368 MHz > > > > + 5 = 32.736 MHz > > > > + 6 = 16.800 MHz > > > > + 7 = 33.600 MHz > > > > > > This looks suspiciously like what we have the common clock bindings for: > > > > > > refclk { > > > compatible = "fixed-clock"; > > > #clock-cells = <0>; > > > clock-frequency = <19200000>; > > > } > > > > > > wilink { > > > compatible = "ti,wilink7"; > > > interrupt-parent = <&some_interrupt_controller>; > > > interrupts = <0 1 1>; > > > clocks = <&refclk>, <&refclk>; > > > clock-names = "refclk", "txoclk"; > > > }; > > > > > > Could you not use them? > > > > Hmmm... this actually does look good. But these are internal clocks in > > the modules, they cannot be accessed from outside. Does it make sense > > to register them with the clock framework? > > Given we already have a common way of describing clocks, I think it > makes sense to use it -- people already understand the common bindings, > and it's less code to add add to the kernel. I don't think the fact > these clocks are internal should prevent us from describing them as we > would an external clock. Yes, I agree with you. Thanks for the suggestion! I think it will look much better. And now that I dug a bit more into the code, I can see that there are only structs being populated, so there shouldn't be any other side-effects. > Perhaps Mike Turquette [Cc'd] has an opinion on the matter. Experts' opinions are appreciated. :) -- Luca.