From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E3AA4C433F5 for ; Mon, 23 May 2022 07:30:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229864AbiEWHZF (ORCPT ); Mon, 23 May 2022 03:25:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58936 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229746AbiEWHYC (ORCPT ); Mon, 23 May 2022 03:24:02 -0400 Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D1FFB3A1B7; Mon, 23 May 2022 00:17:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1653290258; x=1684826258; h=date:from:to:cc:subject:in-reply-to:message-id: references:mime-version; bh=ev8v7cyguP4Weqk3Bhpwe6YaRfyri/aaDhL9TcGpb8I=; b=OE9+ap3Q9zdtnOJxSTb8kEpeo/ZmSvGahLmXghqMz+2ZBkdM1OlqWPf4 8X+Ub4xTVj3xYi95Y6jO624KN00aRoDkz44RsrN53MLkNcDknmt4fsA+H 8GgzhTNeCaD/LtTjf+uXOhqCYi2y+8k7qXUBcbzBgwNExBBHyTGny5qch rlacEMUZkw8un+JcEFMZyqmOah4Ev0xqF5EtgbbeFkzxC4fuJ+VM/rVFE YIrZZR13c79Pxn7k9GO879Z2cTK+HXafgihEB4iKOMSM/e+YSoF3a33Dq hJacjVpgumve4Bp2ky+QGJ3RBQ3yOuRobZ7YdjK22WWSwXLTch8r/uKON g==; X-IronPort-AV: E=McAfee;i="6400,9594,10355"; a="359523924" X-IronPort-AV: E=Sophos;i="5.91,245,1647327600"; d="scan'208";a="359523924" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 May 2022 00:07:23 -0700 X-IronPort-AV: E=Sophos;i="5.91,245,1647327600"; d="scan'208";a="600484629" Received: from tstralma-mobl1.ger.corp.intel.com ([10.252.55.107]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 May 2022 00:07:13 -0700 Date: Mon, 23 May 2022 10:07:07 +0300 (EEST) From: =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= To: Tomer Maimon cc: avifishman70@gmail.com, tali.perry1@gmail.com, joel@jms.id.au, venture@google.com, yuenn@google.com, benjaminfair@google.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, mturquette@baylibre.com, sboyd@kernel.org, p.zabel@pengutronix.de, Greg Kroah-Hartman , daniel.lezcano@linaro.org, tglx@linutronix.de, wim@linux-watchdog.org, linux@roeck-us.net, catalin.marinas@arm.com, will@kernel.org, arnd@arndb.de, olof@lixom.net, Jiri Slaby , shawnguo@kernel.org, bjorn.andersson@linaro.org, geert+renesas@glider.be, marcel.ziswiler@toradex.com, vkoul@kernel.org, biju.das.jz@bp.renesas.com, nobuhiro1.iwamatsu@toshiba.co.jp, robert.hancock@calian.com, j.neuschaefer@gmx.net, lkundrak@v3.sk, soc@kernel.org, devicetree@vger.kernel.org, LKML , linux-clk@vger.kernel.org, linux-serial , linux-watchdog@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v1 08/19] clk: npcm8xx: add clock controller In-Reply-To: <20220522155046.260146-9-tmaimon77@gmail.com> Message-ID: <6fa3d94c-294d-1c6c-5738-6d15b2e17e90@linux.intel.com> References: <20220522155046.260146-1-tmaimon77@gmail.com> <20220522155046.260146-9-tmaimon77@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 22 May 2022, Tomer Maimon wrote: > Nuvoton Arbel BMC NPCM7XX contains an integrated clock controller, which > generates and supplies clocks to all modules within the BMC. > > Signed-off-by: Tomer Maimon > +static struct clk_hw * > +npcm8xx_clk_register_pll(void __iomem *pllcon, const char *name, > + const char *parent_name, unsigned long flags) > +{ > + struct npcm8xx_clk_pll *pll; > + struct clk_init_data init; > + struct clk_hw *hw; > + int ret; > + > + pll = kzalloc(sizeof(*pll), GFP_KERNEL); > + if (!pll) > + return ERR_PTR(-ENOMEM); > + > + pr_debug("%s reg, name=%s, p=%s\n", __func__, name, parent_name); > + > + init.name = name; > + init.ops = &npcm8xx_clk_pll_ops; > + init.parent_names = &parent_name; > + init.num_parents = 1; > + init.flags = flags; > + > + pll->pllcon = pllcon; > + pll->hw.init = &init; > + > + hw = &pll->hw; > + > + ret = clk_hw_register(NULL, hw); > + if (ret) { > + kfree(pll); > + hw = ERR_PTR(ret); > + } > + > + return hw; > +} > + > +#define NPCM8XX_CLKEN1 (0x00) > +#define NPCM8XX_CLKEN2 (0x28) > +#define NPCM8XX_CLKEN3 (0x30) > +#define NPCM8XX_CLKEN4 (0x70) > +#define NPCM8XX_CLKSEL (0x04) > +#define NPCM8XX_CLKDIV1 (0x08) > +#define NPCM8XX_CLKDIV2 (0x2C) > +#define NPCM8XX_CLKDIV3 (0x58) > +#define NPCM8XX_CLKDIV4 (0x7C) > +#define NPCM8XX_PLLCON0 (0x0C) > +#define NPCM8XX_PLLCON1 (0x10) > +#define NPCM8XX_PLLCON2 (0x54) > +#define NPCM8XX_SWRSTR (0x14) > +#define NPCM8XX_IRQWAKECON (0x18) > +#define NPCM8XX_IRQWAKEFLAG (0x1C) > +#define NPCM8XX_IPSRST1 (0x20) > +#define NPCM8XX_IPSRST2 (0x24) > +#define NPCM8XX_IPSRST3 (0x34) > +#define NPCM8XX_WD0RCR (0x38) > +#define NPCM8XX_WD1RCR (0x3C) > +#define NPCM8XX_WD2RCR (0x40) > +#define NPCM8XX_SWRSTC1 (0x44) > +#define NPCM8XX_SWRSTC2 (0x48) > +#define NPCM8XX_SWRSTC3 (0x4C) > +#define NPCM8XX_SWRSTC4 (0x50) > +#define NPCM8XX_CORSTC (0x5C) > +#define NPCM8XX_PLLCONG (0x60) > +#define NPCM8XX_AHBCKFI (0x64) > +#define NPCM8XX_SECCNT (0x68) > +#define NPCM8XX_CNTR25M (0x6C) > +#define NPCM8XX_THRTL_CNT (0xC0) > + > +struct npcm8xx_clk_gate_data { > + u32 reg; > + u8 bit_idx; > + const char *name; > + const char *parent_name; > + unsigned long flags; > + /* > + * If this clock is exported via DT, set onecell_idx to constant > + * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for > + * this specific clock. Otherwise, set to -1. > + */ > + int onecell_idx; > +}; > + > +struct npcm8xx_clk_mux_data { > + u8 shift; > + u8 mask; > + u32 *table; > + const char *name; > + const char * const *parent_names; > + u8 num_parents; > + unsigned long flags; > + /* > + * If this clock is exported via DT, set onecell_idx to constant > + * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for > + * this specific clock. Otherwise, set to -1. > + */ > + int onecell_idx; > + > +}; > + > +struct npcm8xx_clk_div_fixed_data { > + u8 mult; > + u8 div; > + const char *name; > + const char *parent_name; > + u8 clk_divider_flags; > + /* > + * If this clock is exported via DT, set onecell_idx to constant > + * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for > + * this specific clock. Otherwise, set to -1. > + */ > + int onecell_idx; > +}; > + > +struct npcm8xx_clk_div_data { > + u32 reg; > + u8 shift; > + u8 width; > + const char *name; > + const char *parent_name; > + u8 clk_divider_flags; > + unsigned long flags; > + /* > + * If this clock is exported via DT, set onecell_idx to constant > + * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for > + * this specific clock. Otherwise, set to -1. > + */ > + int onecell_idx; > +}; > + > +struct npcm8xx_clk_pll_data { > + u32 reg; > + const char *name; > + const char *parent_name; > + unsigned long flags; > + /* > + * If this clock is exported via DT, set onecell_idx to constant > + * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for > + * this specific clock. Otherwise, set to -1. > + */ > + int onecell_idx; > +}; > + > +/* > + * Single copy of strings used to refer to clocks within this driver indexed by > + * above enum. > + */ > +#define NPCM8XX_CLK_S_REFCLK "refclk" > +#define NPCM8XX_CLK_S_SYSBYPCK "sysbypck" > +#define NPCM8XX_CLK_S_MCBYPCK "mcbypck" > +#define NPCM8XX_CLK_S_GFXBYPCK "gfxbypck" > +#define NPCM8XX_CLK_S_PLL0 "pll0" > +#define NPCM8XX_CLK_S_PLL1 "pll1" > +#define NPCM8XX_CLK_S_PLL1_DIV2 "pll1_div2" > +#define NPCM8XX_CLK_S_PLL2 "pll2" > +#define NPCM8XX_CLK_S_PLL_GFX "pll_gfx" > +#define NPCM8XX_CLK_S_PLL2_DIV2 "pll2_div2" > +#define NPCM8XX_CLK_S_PIX_MUX "gfx_pixel" > +#define NPCM8XX_CLK_S_GPRFSEL_MUX "gprfsel_mux" > +#define NPCM8XX_CLK_S_MC_MUX "mc_phy" > +#define NPCM8XX_CLK_S_CPU_MUX "cpu" /*AKA system clock.*/ Add spaces around comment. > +#define NPCM8XX_CLK_S_MC "mc" > +#define NPCM8XX_CLK_S_AXI "axi" /*AKA CLK2*/ > +#define NPCM8XX_CLK_S_AHB "ahb" /*AKA CLK4*/ Ditto. > +static void __init npcm8xx_clk_init(struct device_node *clk_np) > +{ > + struct clk_hw_onecell_data *npcm8xx_clk_data; > + void __iomem *clk_base; > + struct resource res; > + struct clk_hw *hw; > + int ret; > + int i; > + > + ret = of_address_to_resource(clk_np, 0, &res); > + if (ret) { > + pr_err("%pOFn: failed to get resource, ret %d\n", clk_np, ret); > + return; > + } > + > + clk_base = ioremap(res.start, resource_size(&res)); > + if (!clk_base) > + goto npcm8xx_init_error; > + > + npcm8xx_clk_data = kzalloc(struct_size(npcm8xx_clk_data, hws, > + NPCM8XX_NUM_CLOCKS), GFP_KERNEL); > + if (!npcm8xx_clk_data) > + goto npcm8xx_init_np_err; > + > + npcm8xx_clk_data->num = NPCM8XX_NUM_CLOCKS; > + > + for (i = 0; i < NPCM8XX_NUM_CLOCKS; i++) > + npcm8xx_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER); > + > + /* Register plls */ > + for (i = 0; i < ARRAY_SIZE(npcm8xx_plls); i++) { > + const struct npcm8xx_clk_pll_data *pll_data = &npcm8xx_plls[i]; > + > + hw = npcm8xx_clk_register_pll(clk_base + pll_data->reg, > + pll_data->name, > + pll_data->parent_name, > + pll_data->flags); > + if (IS_ERR(hw)) { Who deregisters the already registered plls on error paths? You might want to consider devm_ variants in npcm8xx_clk_register_pll() to make the cleanup simpler. Please check the other error path rollbacks from this point onward too. > + pr_err("npcm8xx_clk: Can't register pll\n"); > + goto npcm8xx_init_fail; > + } > + > + if (pll_data->onecell_idx >= 0) > + npcm8xx_clk_data->hws[pll_data->onecell_idx] = hw; > + } > + > + /* Register fixed dividers */ > + hw = clk_hw_register_fixed_factor(NULL, NPCM8XX_CLK_S_PLL1_DIV2, > + NPCM8XX_CLK_S_PLL1, 0, 1, 2); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register fixed div\n"); > + goto npcm8xx_init_fail; > + } > + > + hw = clk_hw_register_fixed_factor(NULL, NPCM8XX_CLK_S_PLL2_DIV2, > + NPCM8XX_CLK_S_PLL2, 0, 1, 2); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register pll div2\n"); > + goto npcm8xx_init_fail; > + } > + > + hw = clk_hw_register_fixed_factor(NULL, NPCM8XX_CLK_S_PRE_CLK, > + NPCM8XX_CLK_S_CPU_MUX, 0, 1, 2); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register ckclk div2\n"); > + goto npcm8xx_init_fail; > + } > + > + hw = clk_hw_register_fixed_factor(NULL, NPCM8XX_CLK_S_AXI, > + NPCM8XX_CLK_S_TH, 0, 1, 2); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register axi div2\n"); > + goto npcm8xx_init_fail; > + } > + > + hw = clk_hw_register_fixed_factor(NULL, NPCM8XX_CLK_S_ATB, > + NPCM8XX_CLK_S_AXI, 0, 1, 2); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register atb div2\n"); > + goto npcm8xx_init_fail; > + } > + > + /* Register muxes */ > + for (i = 0; i < ARRAY_SIZE(npcm8xx_muxes); i++) { > + const struct npcm8xx_clk_mux_data *mux_data = &npcm8xx_muxes[i]; > + > + hw = clk_hw_register_mux_table(NULL, mux_data->name, > + mux_data->parent_names, > + mux_data->num_parents, > + mux_data->flags, > + clk_base + NPCM8XX_CLKSEL, > + mux_data->shift, > + mux_data->mask, 0, > + mux_data->table, > + &npcm8xx_clk_lock); > + > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register mux\n"); > + goto npcm8xx_init_fail; > + } > + > + if (mux_data->onecell_idx >= 0) > + npcm8xx_clk_data->hws[mux_data->onecell_idx] = hw; > + } > + > + /* Register clock dividers specified in npcm8xx_divs */ > + for (i = 0; i < ARRAY_SIZE(npcm8xx_divs); i++) { > + const struct npcm8xx_clk_div_data *div_data = &npcm8xx_divs[i]; > + > + hw = clk_hw_register_divider(NULL, div_data->name, > + div_data->parent_name, > + div_data->flags, > + clk_base + div_data->reg, > + div_data->shift, div_data->width, > + div_data->clk_divider_flags, > + &npcm8xx_clk_lock); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register div table\n"); > + goto npcm8xx_init_fail; > + } > + > + if (div_data->onecell_idx >= 0) > + npcm8xx_clk_data->hws[div_data->onecell_idx] = hw; > + } > + > + ret = of_clk_add_hw_provider(clk_np, of_clk_hw_onecell_get, > + npcm8xx_clk_data); > + if (ret) > + pr_err("failed to add DT provider: %d\n", ret); > + > + of_node_put(clk_np); > + > + return; > + > +npcm8xx_init_fail: > + kfree(npcm8xx_clk_data->hws); > +npcm8xx_init_np_err: > + iounmap(clk_base); > +npcm8xx_init_error: > + of_node_put(clk_np); > +} > + > +CLK_OF_DECLARE(npcm8xx_clk_init, "nuvoton,npcm845-clk", npcm8xx_clk_init); > -- i. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 45946C433F5 for ; Mon, 23 May 2022 07:55:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:Message-ID: In-Reply-To:Subject:cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=rArAVn/XgeaSWXzJRZhCA0M7dN/ZxUY324PIV57j7/E=; b=qHMUP6xA9Tk/l5 DKFLhz+8bHsGjRH9/r05i2x2RVtZecD7frG2+E9tpyk8GGZ60fnQqtT/LKGKegu7Vb4hTOIlbJNSi kSxnipuxJcVD10IPRO2uMVZP2og8PfXiY3yfp6pvZACMOm3yirUR0eZosE7rl7EEpaN/uUVZ8wcmb sPTZ2f56m4oD9xpktezy3yRWqN+Sh6+hOSfioPMqojxa3HnnLVqG0DdzbL7NosOVfBGTmJ+NSY5ku GfusaVvuzzr0XqmUldC8pGi8ft/12Sa2AfGJekx6FnZxr+/gruUUrzhx96RZL4YnvNHS/NLaoup/V WK34BN0uptRum3qV/c9g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nt2st-002ChY-5T; Mon, 23 May 2022 07:53:48 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nt2nB-002AAA-IT for linux-arm-kernel@bombadil.infradead.org; Mon, 23 May 2022 07:47:53 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=Content-Type:MIME-Version:References: Message-ID:In-Reply-To:Subject:cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=DVclFx1/E6sgfu4gUJteWLYCAnXMxDGNG6EU+jE2HGw=; b=PIgo4JxAhiyiFD99WTD++R+ZgX znfei7aH0y8M9tsQbz6zToAfu1g2j8NWM4AxemlRbPO8kiHZhy9DMizbSyvsYxstia1agCnM+KqWO 7XUIdba93DYTqjeUqlXHiJ0G/b7+j3A8bI+FrwEh5KdimcL8jVYbeb60rRdTieFw7jVC0SwOq5Zh9 IuWPlmwE2ZlmDUK/vcuxxDjU5yo7PYGXOxQMAH0GsPop/8e/TfyB/uyv9oPKj1Xa6FCKw3YoXoGya wbWfvaksdScSQiZxKQRfhLjQ7eIlRT0e6zJD9Jqc+AS+jcxw1Wc0HV/Sj2984PToYMLdKv4YRoHpR NZZHQlyw==; Received: from mga17.intel.com ([192.55.52.151]) by desiato.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nt2B5-000vV8-3Q for linux-arm-kernel@lists.infradead.org; Mon, 23 May 2022 07:08:36 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1653289711; x=1684825711; h=date:from:to:cc:subject:in-reply-to:message-id: references:mime-version; bh=ev8v7cyguP4Weqk3Bhpwe6YaRfyri/aaDhL9TcGpb8I=; b=TjsFjaBNyQfMxyY6lTH/wk5Av8j25nihOWewUulo094PdFAv/+Ec9hI7 vmN7tO1IyMuI8krM+oceOfA6fn6iT+Zhpbc6Tfzcxj4q4hm2Y6H6cIXqm hJXXi7+9i3mWdixQjwjqSzH/u3t8UFjivJO8B6IS1XaV2oTcWlec/FUi1 luP5XfUieUular8kaslYeruc8L8MfLJkgXE6TYPJVxdSxfyWBE8L5dQoE vZqQVvlzRLUd5DIunbhQ1iJnX+ufXteJ1g+RTIzWcqN96ATXQC7FGa3ZV xw98EjZe+y8m3GG8D3+WT6jqSiBJHOQ4s0WGH6OKuZbEKAyZtm7FX+QG8 w==; X-IronPort-AV: E=McAfee;i="6400,9594,10355"; a="253634463" X-IronPort-AV: E=Sophos;i="5.91,245,1647327600"; d="scan'208";a="253634463" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 May 2022 00:07:23 -0700 X-IronPort-AV: E=Sophos;i="5.91,245,1647327600"; d="scan'208";a="600484629" Received: from tstralma-mobl1.ger.corp.intel.com ([10.252.55.107]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 May 2022 00:07:13 -0700 Date: Mon, 23 May 2022 10:07:07 +0300 (EEST) From: =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= To: Tomer Maimon cc: avifishman70@gmail.com, tali.perry1@gmail.com, joel@jms.id.au, venture@google.com, yuenn@google.com, benjaminfair@google.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, mturquette@baylibre.com, sboyd@kernel.org, p.zabel@pengutronix.de, Greg Kroah-Hartman , daniel.lezcano@linaro.org, tglx@linutronix.de, wim@linux-watchdog.org, linux@roeck-us.net, catalin.marinas@arm.com, will@kernel.org, arnd@arndb.de, olof@lixom.net, Jiri Slaby , shawnguo@kernel.org, bjorn.andersson@linaro.org, geert+renesas@glider.be, marcel.ziswiler@toradex.com, vkoul@kernel.org, biju.das.jz@bp.renesas.com, nobuhiro1.iwamatsu@toshiba.co.jp, robert.hancock@calian.com, j.neuschaefer@gmx.net, lkundrak@v3.sk, soc@kernel.org, devicetree@vger.kernel.org, LKML , linux-clk@vger.kernel.org, linux-serial , linux-watchdog@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v1 08/19] clk: npcm8xx: add clock controller In-Reply-To: <20220522155046.260146-9-tmaimon77@gmail.com> Message-ID: <6fa3d94c-294d-1c6c-5738-6d15b2e17e90@linux.intel.com> References: <20220522155046.260146-1-tmaimon77@gmail.com> <20220522155046.260146-9-tmaimon77@gmail.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220523_080833_657734_8113A229 X-CRM114-Status: GOOD ( 19.41 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sun, 22 May 2022, Tomer Maimon wrote: > Nuvoton Arbel BMC NPCM7XX contains an integrated clock controller, which > generates and supplies clocks to all modules within the BMC. > > Signed-off-by: Tomer Maimon > +static struct clk_hw * > +npcm8xx_clk_register_pll(void __iomem *pllcon, const char *name, > + const char *parent_name, unsigned long flags) > +{ > + struct npcm8xx_clk_pll *pll; > + struct clk_init_data init; > + struct clk_hw *hw; > + int ret; > + > + pll = kzalloc(sizeof(*pll), GFP_KERNEL); > + if (!pll) > + return ERR_PTR(-ENOMEM); > + > + pr_debug("%s reg, name=%s, p=%s\n", __func__, name, parent_name); > + > + init.name = name; > + init.ops = &npcm8xx_clk_pll_ops; > + init.parent_names = &parent_name; > + init.num_parents = 1; > + init.flags = flags; > + > + pll->pllcon = pllcon; > + pll->hw.init = &init; > + > + hw = &pll->hw; > + > + ret = clk_hw_register(NULL, hw); > + if (ret) { > + kfree(pll); > + hw = ERR_PTR(ret); > + } > + > + return hw; > +} > + > +#define NPCM8XX_CLKEN1 (0x00) > +#define NPCM8XX_CLKEN2 (0x28) > +#define NPCM8XX_CLKEN3 (0x30) > +#define NPCM8XX_CLKEN4 (0x70) > +#define NPCM8XX_CLKSEL (0x04) > +#define NPCM8XX_CLKDIV1 (0x08) > +#define NPCM8XX_CLKDIV2 (0x2C) > +#define NPCM8XX_CLKDIV3 (0x58) > +#define NPCM8XX_CLKDIV4 (0x7C) > +#define NPCM8XX_PLLCON0 (0x0C) > +#define NPCM8XX_PLLCON1 (0x10) > +#define NPCM8XX_PLLCON2 (0x54) > +#define NPCM8XX_SWRSTR (0x14) > +#define NPCM8XX_IRQWAKECON (0x18) > +#define NPCM8XX_IRQWAKEFLAG (0x1C) > +#define NPCM8XX_IPSRST1 (0x20) > +#define NPCM8XX_IPSRST2 (0x24) > +#define NPCM8XX_IPSRST3 (0x34) > +#define NPCM8XX_WD0RCR (0x38) > +#define NPCM8XX_WD1RCR (0x3C) > +#define NPCM8XX_WD2RCR (0x40) > +#define NPCM8XX_SWRSTC1 (0x44) > +#define NPCM8XX_SWRSTC2 (0x48) > +#define NPCM8XX_SWRSTC3 (0x4C) > +#define NPCM8XX_SWRSTC4 (0x50) > +#define NPCM8XX_CORSTC (0x5C) > +#define NPCM8XX_PLLCONG (0x60) > +#define NPCM8XX_AHBCKFI (0x64) > +#define NPCM8XX_SECCNT (0x68) > +#define NPCM8XX_CNTR25M (0x6C) > +#define NPCM8XX_THRTL_CNT (0xC0) > + > +struct npcm8xx_clk_gate_data { > + u32 reg; > + u8 bit_idx; > + const char *name; > + const char *parent_name; > + unsigned long flags; > + /* > + * If this clock is exported via DT, set onecell_idx to constant > + * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for > + * this specific clock. Otherwise, set to -1. > + */ > + int onecell_idx; > +}; > + > +struct npcm8xx_clk_mux_data { > + u8 shift; > + u8 mask; > + u32 *table; > + const char *name; > + const char * const *parent_names; > + u8 num_parents; > + unsigned long flags; > + /* > + * If this clock is exported via DT, set onecell_idx to constant > + * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for > + * this specific clock. Otherwise, set to -1. > + */ > + int onecell_idx; > + > +}; > + > +struct npcm8xx_clk_div_fixed_data { > + u8 mult; > + u8 div; > + const char *name; > + const char *parent_name; > + u8 clk_divider_flags; > + /* > + * If this clock is exported via DT, set onecell_idx to constant > + * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for > + * this specific clock. Otherwise, set to -1. > + */ > + int onecell_idx; > +}; > + > +struct npcm8xx_clk_div_data { > + u32 reg; > + u8 shift; > + u8 width; > + const char *name; > + const char *parent_name; > + u8 clk_divider_flags; > + unsigned long flags; > + /* > + * If this clock is exported via DT, set onecell_idx to constant > + * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for > + * this specific clock. Otherwise, set to -1. > + */ > + int onecell_idx; > +}; > + > +struct npcm8xx_clk_pll_data { > + u32 reg; > + const char *name; > + const char *parent_name; > + unsigned long flags; > + /* > + * If this clock is exported via DT, set onecell_idx to constant > + * defined in include/dt-bindings/clock/nuvoton, NPCM8XX-clock.h for > + * this specific clock. Otherwise, set to -1. > + */ > + int onecell_idx; > +}; > + > +/* > + * Single copy of strings used to refer to clocks within this driver indexed by > + * above enum. > + */ > +#define NPCM8XX_CLK_S_REFCLK "refclk" > +#define NPCM8XX_CLK_S_SYSBYPCK "sysbypck" > +#define NPCM8XX_CLK_S_MCBYPCK "mcbypck" > +#define NPCM8XX_CLK_S_GFXBYPCK "gfxbypck" > +#define NPCM8XX_CLK_S_PLL0 "pll0" > +#define NPCM8XX_CLK_S_PLL1 "pll1" > +#define NPCM8XX_CLK_S_PLL1_DIV2 "pll1_div2" > +#define NPCM8XX_CLK_S_PLL2 "pll2" > +#define NPCM8XX_CLK_S_PLL_GFX "pll_gfx" > +#define NPCM8XX_CLK_S_PLL2_DIV2 "pll2_div2" > +#define NPCM8XX_CLK_S_PIX_MUX "gfx_pixel" > +#define NPCM8XX_CLK_S_GPRFSEL_MUX "gprfsel_mux" > +#define NPCM8XX_CLK_S_MC_MUX "mc_phy" > +#define NPCM8XX_CLK_S_CPU_MUX "cpu" /*AKA system clock.*/ Add spaces around comment. > +#define NPCM8XX_CLK_S_MC "mc" > +#define NPCM8XX_CLK_S_AXI "axi" /*AKA CLK2*/ > +#define NPCM8XX_CLK_S_AHB "ahb" /*AKA CLK4*/ Ditto. > +static void __init npcm8xx_clk_init(struct device_node *clk_np) > +{ > + struct clk_hw_onecell_data *npcm8xx_clk_data; > + void __iomem *clk_base; > + struct resource res; > + struct clk_hw *hw; > + int ret; > + int i; > + > + ret = of_address_to_resource(clk_np, 0, &res); > + if (ret) { > + pr_err("%pOFn: failed to get resource, ret %d\n", clk_np, ret); > + return; > + } > + > + clk_base = ioremap(res.start, resource_size(&res)); > + if (!clk_base) > + goto npcm8xx_init_error; > + > + npcm8xx_clk_data = kzalloc(struct_size(npcm8xx_clk_data, hws, > + NPCM8XX_NUM_CLOCKS), GFP_KERNEL); > + if (!npcm8xx_clk_data) > + goto npcm8xx_init_np_err; > + > + npcm8xx_clk_data->num = NPCM8XX_NUM_CLOCKS; > + > + for (i = 0; i < NPCM8XX_NUM_CLOCKS; i++) > + npcm8xx_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER); > + > + /* Register plls */ > + for (i = 0; i < ARRAY_SIZE(npcm8xx_plls); i++) { > + const struct npcm8xx_clk_pll_data *pll_data = &npcm8xx_plls[i]; > + > + hw = npcm8xx_clk_register_pll(clk_base + pll_data->reg, > + pll_data->name, > + pll_data->parent_name, > + pll_data->flags); > + if (IS_ERR(hw)) { Who deregisters the already registered plls on error paths? You might want to consider devm_ variants in npcm8xx_clk_register_pll() to make the cleanup simpler. Please check the other error path rollbacks from this point onward too. > + pr_err("npcm8xx_clk: Can't register pll\n"); > + goto npcm8xx_init_fail; > + } > + > + if (pll_data->onecell_idx >= 0) > + npcm8xx_clk_data->hws[pll_data->onecell_idx] = hw; > + } > + > + /* Register fixed dividers */ > + hw = clk_hw_register_fixed_factor(NULL, NPCM8XX_CLK_S_PLL1_DIV2, > + NPCM8XX_CLK_S_PLL1, 0, 1, 2); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register fixed div\n"); > + goto npcm8xx_init_fail; > + } > + > + hw = clk_hw_register_fixed_factor(NULL, NPCM8XX_CLK_S_PLL2_DIV2, > + NPCM8XX_CLK_S_PLL2, 0, 1, 2); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register pll div2\n"); > + goto npcm8xx_init_fail; > + } > + > + hw = clk_hw_register_fixed_factor(NULL, NPCM8XX_CLK_S_PRE_CLK, > + NPCM8XX_CLK_S_CPU_MUX, 0, 1, 2); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register ckclk div2\n"); > + goto npcm8xx_init_fail; > + } > + > + hw = clk_hw_register_fixed_factor(NULL, NPCM8XX_CLK_S_AXI, > + NPCM8XX_CLK_S_TH, 0, 1, 2); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register axi div2\n"); > + goto npcm8xx_init_fail; > + } > + > + hw = clk_hw_register_fixed_factor(NULL, NPCM8XX_CLK_S_ATB, > + NPCM8XX_CLK_S_AXI, 0, 1, 2); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register atb div2\n"); > + goto npcm8xx_init_fail; > + } > + > + /* Register muxes */ > + for (i = 0; i < ARRAY_SIZE(npcm8xx_muxes); i++) { > + const struct npcm8xx_clk_mux_data *mux_data = &npcm8xx_muxes[i]; > + > + hw = clk_hw_register_mux_table(NULL, mux_data->name, > + mux_data->parent_names, > + mux_data->num_parents, > + mux_data->flags, > + clk_base + NPCM8XX_CLKSEL, > + mux_data->shift, > + mux_data->mask, 0, > + mux_data->table, > + &npcm8xx_clk_lock); > + > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register mux\n"); > + goto npcm8xx_init_fail; > + } > + > + if (mux_data->onecell_idx >= 0) > + npcm8xx_clk_data->hws[mux_data->onecell_idx] = hw; > + } > + > + /* Register clock dividers specified in npcm8xx_divs */ > + for (i = 0; i < ARRAY_SIZE(npcm8xx_divs); i++) { > + const struct npcm8xx_clk_div_data *div_data = &npcm8xx_divs[i]; > + > + hw = clk_hw_register_divider(NULL, div_data->name, > + div_data->parent_name, > + div_data->flags, > + clk_base + div_data->reg, > + div_data->shift, div_data->width, > + div_data->clk_divider_flags, > + &npcm8xx_clk_lock); > + if (IS_ERR(hw)) { > + pr_err("npcm8xx_clk: Can't register div table\n"); > + goto npcm8xx_init_fail; > + } > + > + if (div_data->onecell_idx >= 0) > + npcm8xx_clk_data->hws[div_data->onecell_idx] = hw; > + } > + > + ret = of_clk_add_hw_provider(clk_np, of_clk_hw_onecell_get, > + npcm8xx_clk_data); > + if (ret) > + pr_err("failed to add DT provider: %d\n", ret); > + > + of_node_put(clk_np); > + > + return; > + > +npcm8xx_init_fail: > + kfree(npcm8xx_clk_data->hws); > +npcm8xx_init_np_err: > + iounmap(clk_base); > +npcm8xx_init_error: > + of_node_put(clk_np); > +} > + > +CLK_OF_DECLARE(npcm8xx_clk_init, "nuvoton,npcm845-clk", npcm8xx_clk_init); > -- i. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel