From patchwork Tue Jun 28 18:04:38 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 9203767 X-Patchwork-Delegate: sboyd@codeaurora.org Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9EDC060757 for ; Tue, 28 Jun 2016 18:04:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 91E8B28614 for ; Tue, 28 Jun 2016 18:04:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 84A6028616; Tue, 28 Jun 2016 18:04:42 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1917428614 for ; Tue, 28 Jun 2016 18:04:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752165AbcF1SEl (ORCPT ); Tue, 28 Jun 2016 14:04:41 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:44520 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752060AbcF1SEk (ORCPT ); Tue, 28 Jun 2016 14:04:40 -0400 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 0000561388; Tue, 28 Jun 2016 18:04:39 +0000 (UTC) Received: from localhost (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 33D60611E0; Tue, 28 Jun 2016 18:04:39 +0000 (UTC) Date: Tue, 28 Jun 2016 11:04:38 -0700 From: Stephen Boyd To: Ricardo Ribalda Delgado Cc: Michael Turquette , s.hauer@pengutronix.de, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 1/2] clk: fixed-factor: Convert into a module platform driver Message-ID: <20160628180438.GD1521@codeaurora.org> References: <1466499688-28625-1-git-send-email-ricardo.ribalda@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1466499688-28625-1-git-send-email-ricardo.ribalda@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-clk-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-clk@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 06/21, Ricardo Ribalda Delgado wrote: > @@ -145,23 +146,24 @@ EXPORT_SYMBOL_GPL(clk_hw_unregister_fixed_factor); > /** > * of_fixed_factor_clk_setup() - Setup function for simple fixed factor clock > */ > -void __init of_fixed_factor_clk_setup(struct device_node *node) > +struct clk *_of_fixed_factor_clk_setup(struct device_node *node) > { > struct clk *clk; > const char *clk_name = node->name; > const char *parent_name; > u32 div, mult; > + int ret; > > if (of_property_read_u32(node, "clock-div", &div)) { > pr_err("%s Fixed factor clock <%s> must have a clock-div property\n", > __func__, node->name); > - return; > + return ERR_PTR(-EIO); > } > > if (of_property_read_u32(node, "clock-mult", &mult)) { > pr_err("%s Fixed factor clock <%s> must have a clock-mult property\n", > __func__, node->name); > - return; > + return ERR_PTR(-EIO); > } > > of_property_read_string(node, "clock-output-names", &clk_name); > @@ -169,10 +171,71 @@ void __init of_fixed_factor_clk_setup(struct device_node *node) > > clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0, > mult, div); > - if (!IS_ERR(clk)) > - of_clk_add_provider(node, of_clk_src_simple_get, clk); > + if (IS_ERR(clk)) > + return clk; > + > + ret = of_clk_add_provider(node, of_clk_src_simple_get, clk); > + if (ret) { > + clk_unregister(clk); > + return ERR_PTR(ret); > + } > + > + return clk; > +} > + > +void __init of_fixed_factor_clk_setup(struct device_node *node) > +{ > + if (!_of_fixed_factor_clk_setup(node)) > + of_node_set_flag(node, OF_POPULATED); Almost, except that this flag should be set in the common clk framework and not in each driver. The large majority of cases will want that. Only a few will want to clear it, and then we can hide that fact by having a different CLK_OF_DECLARE macro for them that indicates this. I see these potential users right now, please double check. drivers/clk/axis/clk-artpec6.c drivers/clk/nxp/clk-lpc18xx-creg.c drivers/clk/samsung/clk-exynos3250.c drivers/clk/sunxi/clk-mod0.c drivers/clk/sunxi/clk-sun8i-apb0.c drivers/clk/ti/clk-dra7-atl.c This is what I'm talking about. I wonder if there's a better way to avoid making a closure with macros to set the populated bit. It would be nice if we had a return value from the init callback, but we don't. ----8<---- diff --git a/drivers/clk/axis/clk-artpec6.c b/drivers/clk/axis/clk-artpec6.c index ffc988b098e4..5c196ea2478b 100644 --- a/drivers/clk/axis/clk-artpec6.c +++ b/drivers/clk/axis/clk-artpec6.c @@ -113,7 +113,7 @@ static void of_artpec6_clkctrl_setup(struct device_node *np) of_clk_add_provider(np, of_clk_src_onecell_get, &clkdata->clk_data); } -CLK_OF_DECLARE(artpec6_clkctrl, "axis,artpec6-clkctrl", +CLK_OF_DECLARE_DRIVER(artpec6_clkctrl, "axis,artpec6-clkctrl", of_artpec6_clkctrl_setup); static int artpec6_clkctrl_probe(struct platform_device *pdev) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 95b80aeb8c9d..4b808726b25b 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3387,6 +3387,8 @@ void __init of_clk_init(const struct of_device_id *matches) &clk_provider_list, node) { if (force || parent_ready(clk_provider->np)) { + /* Don't populate platform devices */ + of_node_set_flag(clk_provider->np, OF_POPULATED); clk_provider->clk_init_cb(clk_provider->np); of_clk_set_defaults(clk_provider->np, true); diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index fb39d5add173..abf71c46ca25 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -777,6 +777,14 @@ extern struct of_device_id __clk_of_table; #define CLK_OF_DECLARE(name, compat, fn) OF_DECLARE_1(clk, name, compat, fn) +#define CLK_OF_DECLARE_DRIVER(name, compat, fn) \ + static void name##_of_clk_init_driver(struct device_node *np) \ + { \ + of_node_clear_flag(np, OF_POPULATED); \ + fn(np); \ + } \ + OF_DECLARE_1(clk, name, compat, name##_of_clk_init_driver) + #ifdef CONFIG_OF int of_clk_add_provider(struct device_node *np, struct clk *(*clk_src_get)(struct of_phandle_args *args,