Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755553AbcK2VGH (ORCPT ); Tue, 29 Nov 2016 16:06:07 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:45218 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751063AbcK2VF6 (ORCPT ); Tue, 29 Nov 2016 16:05:58 -0500 DMARC-Filter: OpenDMARC Filter v1.3.1 smtp.codeaurora.org 30C9D6146A Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=pass smtp.mailfrom=sboyd@codeaurora.org Date: Tue, 29 Nov 2016 13:05:56 -0800 From: Stephen Boyd To: Kuninori Morimoto Cc: Rob Herring , Linux-ALSA , Linux-DT , Michael Turquette , Russell King , Linux-Kernel , Mark Brown , linux-clk@vger.kernel.org, Linux-ARM Subject: Re: [alsa-devel] [PATCH v2] clkdev: add devm_of_clk_get() Message-ID: <20161129210556.GC6095@codeaurora.org> References: <87ziptixv7.wl%kuninori.morimoto.gx@renesas.com> <20160707122636.GP1041@n2100.armlinux.org.uk> <8760shgfzu.wl%kuninori.morimoto.gx@renesas.com> <146794140875.73491.7115209079607438738@resonance> <871t34hlin.wl%kuninori.morimoto.gx@renesas.com> <878twndi54.wl%kuninori.morimoto.gx@renesas.com> <8737isvwc6.wl%kuninori.morimoto.gx@renesas.com> <20161123191037.GE25626@codeaurora.org> <87a8cpejn5.wl%kuninori.morimoto.gx@renesas.com> <87y409cw71.wl%kuninori.morimoto.gx@renesas.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87y409cw71.wl%kuninori.morimoto.gx@renesas.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1863 Lines: 50 On 11/24, Kuninori Morimoto wrote: > > Hi Stephen, again > > > > I've seen bindings that have the 'clocks' property at the top > > > level and the appropriate 'clock-names' property to relate the > > > clocks to a subnode. > > > > > > sound_soc { > > > clocks = <&xxx>, <&xxx>; > > > clock-names = "cpu", "codec"; > > > ... > > > cpu { > > > ... > > > }; > > > codec { > > > ... > > > }; > > > }; > > > > > > Then the subnodes call clk_get() with the top level device and > > > the name of their node and things match up. I suppose this > > > binding is finalized though, so we can't really do that? > > > > > > I see that the gpio framework has a similar design called > > > devm_get_gpiod_from_child(), so how about we add a > > > devm_get_clk_from_child() API? That would more closely match the > > > intent here, which is to restrict the clk_get() operation to > > > child nodes of the device passed as the first argument. > > > > > > struct clk *devm_get_clk_from_child(struct device *dev, > > > const char *con_id, > > > struct device_node *child); > > Thanks, but, my point is that Linux already have "of_clk_get()", > but we don't have its devm_ version. > The point is that of_clk_get() can get clock from "device_node". > Why having devm_ version become so problem ? The problem is that it encourages the use of of_clk_get() when clk_get() is more desirable. Ideally of_clk_get() is never used when a device exists. In this case, it seems like we need to support it though, hence the suggestion of having a devm_get_clk_from_child() API, that explicitly reads as "get a clock from a child node of this device". The distinction is important, because of_clk_get() should rarely be used. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project