From patchwork Fri May 1 20:52:47 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 6313051 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 1E3B5BEEE1 for ; Fri, 1 May 2015 20:55:52 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 170CE203ED for ; Fri, 1 May 2015 20:55:51 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id EAEAB203C3 for ; Fri, 1 May 2015 20:55:49 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YoHvv-0000yI-ED; Fri, 01 May 2015 20:53:15 +0000 Received: from smtp.codeaurora.org ([198.145.29.96]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YoHvs-0000ub-Em for linux-arm-kernel@lists.infradead.org; Fri, 01 May 2015 20:53:13 +0000 Received: from smtp.codeaurora.org (localhost [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id D4D9F140564; Fri, 1 May 2015 20:52:49 +0000 (UTC) Received: by smtp.codeaurora.org (Postfix, from userid 486) id BFF5F1405FF; Fri, 1 May 2015 20:52:49 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from [10.134.64.202] (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: sboyd@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 3AA1C140564; Fri, 1 May 2015 20:52:48 +0000 (UTC) Message-ID: <5543E79F.2080400@codeaurora.org> Date: Fri, 01 May 2015 13:52:47 -0700 From: Stephen Boyd User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: =?windows-1252?Q?Heiko_St=FCbner?= Subject: Re: [PATCH v3 0/2] clk: improve handling of orphan clocks References: <1429735986-18592-1-git-send-email-heiko@sntech.de> <20150501001901.GC32407@codeaurora.org> <1981330.kGUrTurMy5@diego> In-Reply-To: <1981330.kGUrTurMy5@diego> X-Virus-Scanned: ClamAV using ClamSMTP X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150501_135312_602256_FFD62E4A X-CRM114-Status: GOOD ( 25.84 ) X-Spam-Score: -0.0 (/) Cc: Ulf Hansson , Geert Uytterhoeven , Andrew Bresticker , dianders@chromium.org, Max Filippov , Alexandre Belloni , Sylwester Nawrocki , Robert Jarzmik , linux-clk@vger.kernel.org, Stefan Wahren , Boris Brezillon , mturquette@linaro.org, emilio@elopez.com.ar, Michal Simek , Alex Elder , Zhangfei Gao , Jason Cooper , Pawel Moll , Stephen Warren , Santosh Shilimkar , Chao Xie , linux-arm-kernel@lists.infradead.org, Barry Song , Peter De Sc hrijver , linux-kernel@vger.kernel.org, Gabriel FERNANDEZ , Tero Kristo , Viresh Kumar , kernel@pengutronix.de, Dinh Nguyen , Georgi Djakov X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On 05/01/15 12:59, Heiko Stübner wrote: > Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd: >> On 04/22, Heiko Stuebner wrote: >>> Using orphan clocks can introduce strange behaviour as they don't have >>> rate information at all and also of course don't track >>> >>> This v2/v3 takes into account suggestions from Stephen Boyd to not try to >>> walk the clock tree at runtime but instead keep track of orphan states >>> on clock tree changes and making it mandatory for everybody from the >>> start as orphaned clocks should not be used at all. >>> >>> >>> This fixes an issue on most rk3288 platforms, where some soc-clocks >>> are supplied by a 32khz clock from an external i2c-chip which often >>> is only probed later in the boot process and maybe even after the >>> drivers using these soc-clocks like the tsadc temperature sensor. >>> In this case the driver using the clock should of course defer probing >>> until the clock is actually usable. >>> >>> >>> As this changes the behaviour for orphan clocks, it would of course >>> benefit from more testing than on my Rockchip boards. To keep the >>> recipent-list reasonable and not spam to much I selected one (the topmost) >>> from the get_maintainer output of each drivers/clk entry. >>> Hopefully some will provide Tested-by-tags :-) >> I don't see any Tested-by: tags yet . I've >> put these two patches on a separate branch "defer-orphans" and >> pushed it to clk-next so we can give it some more exposure. >> >> Unfortunately this doesn't solve the orphan problem for non-OF >> providers. What if we did the orphan check in __clk_create_clk() >> instead and returned an error pointer for orphans? I suspect that >> will solve all cases, right? > hmm, clk_register also uses __clk_create_clk, which in turn would prevent > registering orphan-clocks at all, I'd think. > As on my platform I'm dependant on orphan clocks (the soc-level clock gets > registerted as part of the big clock controller way before the i2c-based > supplying clock), I'd rather not touch this :-) . Have no fear! We should just change clk_register() to call a __clk_create_clk() type function that doesn't check for orphan status. > > Instead I guess we could hook it less deep into clk_get_sys, like in the > following patch? It looks like it will work at least, but still I'd prefer to keep the orphan check contained to clk.c. How about this compile tested only patch? This also brings up an existing problem with clk_unregister() where orphaned clocks are sitting out there useable by drivers when their parent is unregistered. That code could use some work to atomically switch all the orphaned clocks over to use the nodrv_ops. ----8<----- Tested-by: Heiko Stuebner Reviewed-by: Heiko Stuebner Tested-by: Tero Kristo diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 30d45c657a07..1d23daa42dd2 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -2221,14 +2221,6 @@ static inline void clk_debug_unregister(struct clk_core *core) } #endif -static bool clk_is_orphan(const struct clk *clk) -{ - if (!clk) - return false; - - return clk->core->orphan; -} - /** * __clk_init - initialize the data structures in a struct clk * @dev: device initializing this clk, placeholder for now @@ -2420,15 +2412,11 @@ out: return ret; } -struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id, - const char *con_id) +static struct clk *clk_hw_create_clk(struct clk_hw *hw, const char *dev_id, + const char *con_id) { struct clk *clk; - /* This is to allow this function to be chained to others */ - if (!hw || IS_ERR(hw)) - return (struct clk *) hw; - clk = kzalloc(sizeof(*clk), GFP_KERNEL); if (!clk) return ERR_PTR(-ENOMEM); @@ -2445,6 +2433,19 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id, return clk; } +struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id, + const char *con_id) +{ + /* This is to allow this function to be chained to others */ + if (!hw || IS_ERR(hw)) + return (struct clk *) hw; + + if (hw->core->orphan) + return ERR_PTR(-EPROBE_DEFER); + + return clk_hw_create_clk(hw, dev_id, con_id); +} + void __clk_free_clk(struct clk *clk) { clk_prepare_lock(); @@ -2511,7 +2512,7 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw) INIT_HLIST_HEAD(&core->clks); - hw->clk = __clk_create_clk(hw, NULL, NULL); + hw->clk = clk_hw_create_clk(hw, NULL, NULL); if (IS_ERR(hw->clk)) { ret = PTR_ERR(hw->clk); goto fail_parent_names_copy; @@ -2958,10 +2959,6 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec, if (provider->node == clkspec->np) clk = provider->get(clkspec, provider->data); if (!IS_ERR(clk)) { - if (clk_is_orphan(clk)) { - clk = ERR_PTR(-EPROBE_DEFER); - break; - } clk = __clk_create_clk(__clk_get_hw(clk), dev_id, con_id);