From patchwork Mon Dec 9 21:31:13 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 3312911 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.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 715D7C0D4A for ; Mon, 9 Dec 2013 21:31:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9853120274 for ; Mon, 9 Dec 2013 21:31:56 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (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 2EA8520220 for ; Mon, 9 Dec 2013 21:31:55 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Vq8Qf-0000Dn-Ou; Mon, 09 Dec 2013 21:31:49 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Vq8Qd-0002jC-2C; Mon, 09 Dec 2013 21:31:47 +0000 Received: from moutng.kundenserver.de ([212.227.17.8]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Vq8QZ-0002hE-LF for linux-arm-kernel@lists.infradead.org; Mon, 09 Dec 2013 21:31:44 +0000 Received: from klappe2.localnet (24-181-213-222.dhcp.hckr.nc.charter.com [24.181.213.222]) by mrelayeu.kundenserver.de (node=mreu0) with ESMTP (Nemesis) id 0MhPvC-1WBdJv1iF8-00MMgp; Mon, 09 Dec 2013 22:31:18 +0100 From: Arnd Bergmann To: dinguyen@altera.com Subject: Re: [PATCH] clk: socfpga: Map the clk manager base address in the clock driver Date: Mon, 9 Dec 2013 22:31:13 +0100 User-Agent: KMail/1.12.2 (Linux/3.8.0-22-generic; KDE/4.3.2; x86_64; ; ) References: <1386617447-1260-1-git-send-email-dinguyen@altera.com> In-Reply-To: <1386617447-1260-1-git-send-email-dinguyen@altera.com> MIME-Version: 1.0 Message-Id: <201312092231.13441.arnd@arndb.de> X-Provags-ID: V02:K0:7ZGxFqVngQ6Akv99BFAbaQdHFTsy4uXRhCe6VVkq6Zm b6vFbY22VqrmQfSQTjTSBbHqJ3McxWtAlKfI7EzjU41kyPmix1 uQON6CmIPdEycGq1vP2Bn66oIM5z/z7aGUugnNICcROOZnw2Tb dH7+seo9BnRZnKpeabojDNtWhxi2sgcDm0JHh1Ku0pwb8BxvEJ aHWyuxjURfZw0ToExK+CcTtZyKb0KiNZWzH+EsCclAQuTaTtH5 Zw4jzTepWegEOl0Y04L5M8pqoxevUFShYR+K6sf1dPUN5HabBt OxyMJh/6EGW4hH4eYaSd49N6aOqL+ejSnyU47l5Wzo3OVBzuEE X88XqkB5KlbJlZNsiv/g= X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20131209_163143_898795_205FA70B X-CRM114-Status: GOOD ( 20.18 ) X-Spam-Score: -1.9 (-) Cc: olof@lixom.net, mturquette@linaro.org, dinh.linux@gmail.com, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 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-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Monday 09 December 2013, dinguyen@altera.com wrote: > From: Dinh Nguyen > > The clk manager's base address was being mapped in SOCFPGA's arch code and > being extern'ed out to the clock driver. This method is not correct, and the > arch code was not really doing anything with that clk manager anyways. > > This patch moves the mapping of the clk manager's base address in the clock > driver itself. > > Signed-off-by: Dinh Nguyen Ok, thanks for doing this cleanup. A few comments for further simplification: > -extern void __iomem *clk_mgr_base_addr; > - > struct socfpga_clk { > struct clk_gate hw; > char *parent_name; > char *clk_name; > + void __iomem *clk_mgr_base_addr; > u32 fixed_div; > void __iomem *div_reg; > u32 width; /* only valid if div_reg != 0 */ I think there is no need to make this a per-clock field, it's fine to have a 'static' variable in the place of the 'extern' declaration you have now. > @@ -129,7 +130,11 @@ static __init struct clk *socfpga_clk_init(struct device_node *node, > if (WARN_ON(!socfpga_clk)) > return NULL; > > - socfpga_clk->hw.reg = clk_mgr_base_addr + reg; > + /* Map the clk manager register */ > + clk_mgr_np = of_find_compatible_node(NULL, NULL, "altr,clk-mgr"); > + socfpga_clk->hw.reg = of_iomap(clk_mgr_np, 0); > + BUG_ON(!socfpga_clk->hw.reg); > + socfpga_clk->hw.reg += reg; > > rc = of_property_read_u32(node, "fixed-divider", &fixed_div); > if (rc) Instead of mapping it every time, I suggest something along the lines of this patch: On a related note, I'd also like to see socfpga_init_clocks() disappear. It should be trivial to just add the "mpuclk" into the DT as a separate compatible = "fixed-factor-clock" node. Arnd diff --git a/drivers/clk/socfpga/clk.c b/drivers/clk/socfpga/clk.c index 81dd31a..1a14742 100644 --- a/drivers/clk/socfpga/clk.c +++ b/drivers/clk/socfpga/clk.c @@ -55,7 +55,7 @@ #define div_mask(width) ((1 << (width)) - 1) #define streq(a, b) (strcmp((a), (b)) == 0) -extern void __iomem *clk_mgr_base_addr; +static void __iomem *clk_mgr_base_addr; struct socfpga_clk { struct clk_gate hw; @@ -322,19 +322,30 @@ static void __init socfpga_pll_init(struct device_node *node) { socfpga_clk_init(node, &clk_pll_ops); } -CLK_OF_DECLARE(socfpga_pll, "altr,socfpga-pll-clock", socfpga_pll_init); static void __init socfpga_periph_init(struct device_node *node) { socfpga_clk_init(node, &periclk_ops); } -CLK_OF_DECLARE(socfpga_periph, "altr,socfpga-perip-clk", socfpga_periph_init); static void __init socfpga_gate_init(struct device_node *node) { socfpga_gate_clk_init(node, &gateclk_ops); } -CLK_OF_DECLARE(socfpga_gate, "altr,socfpga-gate-clk", socfpga_gate_init); + +static struct of_device_id socfpga_child_clocks[] = { + { "altr,socfpga-pll-clock", socfpga_pll_init }, + { "altr,socfpga-perip-clk", socfpga_periph_init }, + { "altr,socfpga-gate-clk", socfpga_gate_init }, + {}, +}; + +static void __init socfpga_pll_init(struct device_node *node) +{ + clk_mgr_base_addr = of_iomap(node, 0); + of_clk_init(socfpga_child_clocks); +} +CLK_OF_DECLARE(socfpga_mgr, "altr,clk-mgr", socfpga_clkmgr_init); void __init socfpga_init_clocks(void) {