From mboxrd@z Thu Jan 1 00:00:00 1970 From: Russell King - ARM Linux Subject: Re: [PATCH 2/2] ARM: imx6q: replace clk_register_clkdev with clock DT lookup Date: Tue, 21 Aug 2012 13:27:53 +0100 Message-ID: <20120821122753.GU18957@n2100.arm.linux.org.uk> References: <1345104526-14797-1-git-send-email-shawn.guo@linaro.org> <1345104526-14797-3-git-send-email-shawn.guo@linaro.org> <503232E0.3000007@gmail.com> <20120820151609.GN24242@S2101-09.ap.freescale.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Matt Sealey Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, Mike Turquette , Rob Herring , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: devicetree@vger.kernel.org On Mon, Aug 20, 2012 at 12:22:56PM -0500, Matt Sealey wrote: > You're going to have to define these clocks as a tree with parents and > leaf nodes anyway in the clock subsystem. Why not define these in the > device tree in total and reference them by handle when you build the > entire clock tree from the ground up? Or will it just be all the > clocks defined in Linux, but the lookups (which is what I see here) > moved into the DT? Why not form the lookups as part of the definition > of the clock tree? Well, IMHO the DT conversion of the clk lookup stuff has been done completely wrong. What should have been done is rather than invent a totally new bloody lookup interface that drivers have to use instead of clk_get(), is to embed the OF lookup _inside_ clk_get(). What you do is this: 1. Have property names in the device node like: clock_ = <&provider-node output> In the case of a NULL connection id: clock = <&provider-node output> Remember that the connection ID is _supposed_ to be something that is described by the hardware (like - for the AACI primecell, the clock which runs the functional side is called "AACICLK" by the TRM, and for the MMCI primecell, it's "MMCICLK" - even though these two clocks may be fed by the same source in an implementation.) 2. clkdev's lookup is then modified to look at the struct device, and check for a DT node. If there is a DT node, it formats a property string: if (dev->of_node) { char *propname, *clk_prop = NULL; struct property *p; if (conn_id) { clk_prop = kasprintf("clock_%s", conn_id); propname = clk_prop; } else { propname = "clock"; } p = of_find_property(dev->of_node, propname, NULL); if (clk_prop) kfree(clk_prop); if (p) { clk = clk_get_from_of_property(p); if (clk) return clk; } /* Fallthrough to clkdev table lookup */ } So now, you're not dealing with inventing a whole load of names for clocks on a platform, instead what you're doing is describing _where_ the clock comes from in the system for a particular device by device node and index into it - just like we do for interrupts. This means there's no need for huge tables and such like of clock names. I did mention this idea long ago but got ignored.