All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Tomer Maimon <tmaimon77@gmail.com>
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 <gregkh@linuxfoundation.org>,
	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 <jirislaby@kernel.org>,
	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-kernel@vger.kernel.org>,
	linux-clk@vger.kernel.org,
	linux-serial <linux-serial@vger.kernel.org>,
	linux-watchdog@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1 08/19] clk: npcm8xx: add clock controller
Date: Mon, 23 May 2022 10:07:07 +0300 (EEST)	[thread overview]
Message-ID: <6fa3d94c-294d-1c6c-5738-6d15b2e17e90@linux.intel.com> (raw)
In-Reply-To: <20220522155046.260146-9-tmaimon77@gmail.com>

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 <tmaimon77@gmail.com>

> +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.


WARNING: multiple messages have this Message-ID (diff)
From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Tomer Maimon <tmaimon77@gmail.com>
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 <gregkh@linuxfoundation.org>,
	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 <jirislaby@kernel.org>,
	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-kernel@vger.kernel.org>,
	 linux-clk@vger.kernel.org,
	linux-serial <linux-serial@vger.kernel.org>,
	 linux-watchdog@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1 08/19] clk: npcm8xx: add clock controller
Date: Mon, 23 May 2022 10:07:07 +0300 (EEST)	[thread overview]
Message-ID: <6fa3d94c-294d-1c6c-5738-6d15b2e17e90@linux.intel.com> (raw)
In-Reply-To: <20220522155046.260146-9-tmaimon77@gmail.com>

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 <tmaimon77@gmail.com>

> +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

  reply	other threads:[~2022-05-23  7:30 UTC|newest]

Thread overview: 101+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-22 15:50 [PATCH v1 00/19] Introduce Nuvoton Arbel NPCM8XX BMC SoC Tomer Maimon
2022-05-22 15:50 ` [PATCH v1 01/19] dt-bindings: timer: npcm: Add npcm845 compatible string Tomer Maimon
2022-05-23  7:31   ` Krzysztof Kozlowski
2022-05-23  7:31     ` Krzysztof Kozlowski
2022-05-22 15:50 ` [PATCH v1 02/19] clocksource: timer-npcm7xx: Add NPCM845 timer support Tomer Maimon
2022-05-22 15:50 ` [PATCH v1 03/19] dt-bindings: serial: 8250: Add npcm845 compatible string Tomer Maimon
2022-05-23  7:32   ` Krzysztof Kozlowski
2022-05-23  7:32     ` Krzysztof Kozlowski
2022-05-22 15:50 ` [PATCH v1 04/19] tty: serial: 8250: Add NPCM845 UART support Tomer Maimon
2022-05-23  9:56   ` Arnd Bergmann
2022-05-23  9:56     ` Arnd Bergmann
2022-05-23 12:58     ` Tomer Maimon
2022-05-23 13:06       ` Krzysztof Kozlowski
2022-05-23 13:06         ` Krzysztof Kozlowski
2022-05-23 13:14         ` Tomer Maimon
2022-05-22 15:50 ` [PATCH v1 05/19] dt-bindings: watchdog: npcm: Add npcm845 compatible string Tomer Maimon
2022-05-23  7:32   ` Krzysztof Kozlowski
2022-05-23  7:32     ` Krzysztof Kozlowski
2022-05-22 15:50 ` [PATCH v1 06/19] watchdog: npcm_wdt: Add NPCM845 watchdog support Tomer Maimon
2022-05-22 16:45   ` Guenter Roeck
2022-05-22 16:45     ` Guenter Roeck
2022-05-22 15:50 ` [PATCH v1 07/19] dt-binding: clk: npcm845: Add binding for Nuvoton NPCM8XX Clock Tomer Maimon
2022-05-23  7:35   ` Krzysztof Kozlowski
2022-05-23  7:35     ` Krzysztof Kozlowski
2022-05-23 13:35     ` Tomer Maimon
2022-05-26 19:24   ` Stephen Boyd
2022-05-26 19:24     ` Stephen Boyd
2022-05-30 14:39     ` Tomer Maimon
2022-05-30 14:39       ` Tomer Maimon
2022-05-22 15:50 ` [PATCH v1 08/19] clk: npcm8xx: add clock controller Tomer Maimon
2022-05-23  7:07   ` Ilpo Järvinen [this message]
2022-05-23  7:07     ` Ilpo Järvinen
2022-05-23 12:48     ` Tomer Maimon
2022-05-26 19:36   ` Stephen Boyd
2022-05-26 19:36     ` Stephen Boyd
2022-05-30 14:36     ` Tomer Maimon
2022-05-30 14:36       ` Tomer Maimon
2022-05-22 15:50 ` [PATCH v1 09/19] dt-bindings: reset: add syscon property Tomer Maimon
2022-05-23  7:39   ` Krzysztof Kozlowski
2022-05-23  7:39     ` Krzysztof Kozlowski
2022-05-23 13:44     ` Tomer Maimon
2022-05-23 13:45       ` Krzysztof Kozlowski
2022-05-23 13:45         ` Krzysztof Kozlowski
2022-05-22 15:50 ` [PATCH v1 10/19] reset: npcm: using syscon instead of device data Tomer Maimon
2022-05-23  8:54   ` Krzysztof Kozlowski
2022-05-23  8:54     ` Krzysztof Kozlowski
2022-05-23 13:53     ` Tomer Maimon
2022-05-22 15:50 ` [PATCH v1 11/19] dt-bindings: reset: npcm: Add support for NPCM8XX Tomer Maimon
2022-05-23  9:01   ` Krzysztof Kozlowski
2022-05-23  9:01     ` Krzysztof Kozlowski
2022-05-23 14:03     ` Tomer Maimon
2022-05-23 14:22       ` Geert Uytterhoeven
2022-05-23 14:22         ` Geert Uytterhoeven
2022-05-23 14:26         ` Krzysztof Kozlowski
2022-05-23 14:26           ` Krzysztof Kozlowski
2022-05-23 15:11           ` Geert Uytterhoeven
2022-05-23 15:11             ` Geert Uytterhoeven
2022-05-23 15:22             ` Krzysztof Kozlowski
2022-05-23 15:22               ` Krzysztof Kozlowski
2022-05-23 15:24               ` Krzysztof Kozlowski
2022-05-23 15:24                 ` Krzysztof Kozlowski
2022-05-24  7:26               ` Tomer Maimon
2022-05-24  7:26                 ` Tomer Maimon
2022-05-23 14:23       ` Krzysztof Kozlowski
2022-05-23 14:23         ` Krzysztof Kozlowski
2022-05-22 15:50 ` [PATCH v1 12/19] reset: npcm: Add NPCM8XX support Tomer Maimon
2022-05-23 10:44   ` Arnd Bergmann
2022-05-23 10:44     ` Arnd Bergmann
2022-05-22 15:50 ` [PATCH v1 13/19] dt-bindings: arm: npcm: Add maintainer Tomer Maimon
2022-06-02 12:58   ` Rob Herring
2022-06-02 12:58     ` Rob Herring
2022-05-22 15:50 ` [PATCH v1 14/19] dt-bindings: arm: npcm: Add nuvoton,npcm845 compatible string Tomer Maimon
2022-05-23  9:02   ` Krzysztof Kozlowski
2022-05-23  9:02     ` Krzysztof Kozlowski
2022-05-22 15:50 ` [PATCH v1 15/19] dt-bindings: arm: npcm: Add nuvoton,npcm845 GCR " Tomer Maimon
2022-05-23  9:02   ` Krzysztof Kozlowski
2022-05-23  9:02     ` [PATCH v1 15/19] dt-bindings: arm: npcm: Add nuvoton, npcm845 " Krzysztof Kozlowski
2022-05-23  9:02   ` [PATCH v1 15/19] dt-bindings: arm: npcm: Add nuvoton,npcm845 " Krzysztof Kozlowski
2022-05-23  9:02     ` [PATCH v1 15/19] dt-bindings: arm: npcm: Add nuvoton, npcm845 " Krzysztof Kozlowski
2022-05-22 15:50 ` [PATCH v1 16/19] arm64: npcm: Add support for Nuvoton NPCM8XX BMC SoC Tomer Maimon
2022-05-22 15:50 ` [PATCH v1 17/19] arm64: dts: nuvoton: Add initial NPCM8XX device tree Tomer Maimon
2022-05-23  9:08   ` Krzysztof Kozlowski
2022-05-23  9:08     ` Krzysztof Kozlowski
2022-05-23 13:58     ` Geert Uytterhoeven
2022-05-23 13:58       ` Geert Uytterhoeven
2022-05-23 14:16       ` Krzysztof Kozlowski
2022-05-23 14:16         ` Krzysztof Kozlowski
2022-05-22 15:50 ` [PATCH v1 18/19] arm64: dts: nuvoton: Add initial NPCM845 EVB " Tomer Maimon
2022-05-23  9:26   ` Krzysztof Kozlowski
2022-05-23  9:26     ` Krzysztof Kozlowski
2022-05-23  9:39   ` Arnd Bergmann
2022-05-23  9:39     ` Arnd Bergmann
2022-05-23 14:17     ` Tomer Maimon
2022-05-23 15:37       ` Krzysztof Kozlowski
2022-05-23 15:37         ` Krzysztof Kozlowski
2022-05-22 15:50 ` [PATCH v1 19/19] arm64: defconfig: Add Nuvoton NPCM family support Tomer Maimon
2022-05-23  9:52 ` [PATCH v1 00/19] Introduce Nuvoton Arbel NPCM8XX BMC SoC Arnd Bergmann
2022-05-23  9:52   ` Arnd Bergmann
2022-05-23 12:20   ` Tomer Maimon
2022-05-30 12:24   ` Andy Shevchenko
2022-05-30 12:24     ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6fa3d94c-294d-1c6c-5738-6d15b2e17e90@linux.intel.com \
    --to=ilpo.jarvinen@linux.intel.com \
    --cc=arnd@arndb.de \
    --cc=avifishman70@gmail.com \
    --cc=benjaminfair@google.com \
    --cc=biju.das.jz@bp.renesas.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=gregkh@linuxfoundation.org \
    --cc=j.neuschaefer@gmx.net \
    --cc=jirislaby@kernel.org \
    --cc=joel@jms.id.au \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=lkundrak@v3.sk \
    --cc=marcel.ziswiler@toradex.com \
    --cc=mturquette@baylibre.com \
    --cc=nobuhiro1.iwamatsu@toshiba.co.jp \
    --cc=olof@lixom.net \
    --cc=p.zabel@pengutronix.de \
    --cc=robert.hancock@calian.com \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=soc@kernel.org \
    --cc=tali.perry1@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=tmaimon77@gmail.com \
    --cc=venture@google.com \
    --cc=vkoul@kernel.org \
    --cc=will@kernel.org \
    --cc=wim@linux-watchdog.org \
    --cc=yuenn@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.