Skip to contentSkip to conversation

clk: npcm8xx: add clock controller

PATCH RESEND Superseded Tomer Maimon 2 participants 2 messages Sep 28, 2023

Subsystems (2)

v19 of 28 Superseded Latest is v28 →

lore
Share & tools
lore

Tomer Maimon

author maintainer
#0 lore raw

PATCH RESEND v19 1/1 clk: npcm8xx: add clock controller

Nuvoton Arbel BMC NPCM8XX contains an integrated clock controller which generates and supplies clocks to all modules within the BMC.

---
Changes since version 18:
- NPCM8XX clock driver did not changed from version 18 only build and tested under kernel 6.6-rc1.

Changes since version 17:
- NPCM8XX clock driver did not changed from version 17 only build and tested under kernel 6.5-rc3.

Changes since version 16:
- NPCM8XX clock driver
- Using devm_kzalloc instead kzalloc.
- Remove unnecessary parenthesis.
- Modify incorrect spelling.

Changes since version 15:
- NPCM8XX clock driver
- Remove unused regs parameter from npcm8xx_pll_data structure.
- Using index and clk_hw parameters to set the clock parent in the clock structures.

Changes since version 14:
- NPCM8XX clock driver
- Remove unnecessary register definitions.
- Remove the internal reference clock, instead use the external DT reference clock.
- rearrange the driver.
- using .names parameter in DT to define clock (refclk).

Changes since version 13:
- NPCM8XX clock driver
- Remove unnecessary definitions and add module.h define
- Use in clk_parent_data struct.fw_name and .name.
- Add module_exit function.
- Add const to divider clock names.
- Add MODULE_DESCRIPTION and MODULE_LICENSE

Changes since version 12:
- NPCM8XX clock driver
- Use clk_parent_data in mux and div clock structure.
- Add const to mux tables.
- Using devm_clk_hw_register_fixed_rate function.
- use only .name clk_parent_data instead .name and .fw_name.
- Modify mask values in mux clocks.

Changes since version 11:
- NPCM8XX clock driver
- Modify Kconfig help.
- Modify loop variable to unsigned int.

Changes since version 11:
- NPCM8XX clock driver
- Modify Kconfig help.
- Modify loop variable to unsigned int.

Changes since version 10:
- NPCM8XX clock driver
- Fix const warning.

Changes since version 9:
- NPCM8XX clock driver
- Move configuration place.
- Using clk_parent_data instead of parent_name
- using devm_ioremap instead of ioremap. deeply sorry, I know we had a long discussion on what should the driver use, from other examples (also in other clock drivers) I see the combination of platform_get_resource and devm_ioremap are commonly used and it answer the reset and clock needs.

Changes since version 8:
- NPCM8XX clock driver
- Move configuration place.
- Add space before and aftre '{' '}'.
- Handle devm_of_clk_add_hw_provider function error.

Changes since version 7:
- NPCM8XX clock driver
- The clock and reset registers using the same memory region, due to it the clock driver should claim the ioremap directly without checking the memory region.

Changes since version 5:
- NPCM8XX clock driver
- Remove refclk if devm_of_clk_add_hw_provider function failed.

Changes since version 4:
- NPCM8XX clock driver
- Use the same quote in the dt-binding file.

Changes since version 3:
- NPCM8XX clock driver
- Rename NPCM8xx clock dt-binding header file.
- Remove unused structures.
- Improve Handling the clocks registration.

Changes since version 2:
- NPCM8XX clock driver
- Add debug new line.
- Add 25M fixed rate clock.
- Remove unused clocks and clock name from dt-binding.

Changes since version 1:
- NPCM8XX clock driver
- Modify dt-binding.
- Remove unsed definition and include.
- Include alphabetically.
- Use clock devm.

 drivers/clk/Kconfig       |   8 +
 drivers/clk/Makefile      |   1 +
 drivers/clk/clk-npcm8xx.c | 565 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 574 insertions(+)
 create mode 100644 drivers/clk/clk-npcm8xx.c
drivers/clk/Kconfig +8 -0
index c30099866174..9bc5f2dfc7e2 100644
@@ -325,6 +325,14 @@ config COMMON_CLK_LOCHNAGAR
 	  This driver supports the clocking features of the Cirrus Logic
 	  Lochnagar audio development board.
 
+config COMMON_CLK_NPCM8XX
+	tristate "Clock driver for the NPCM8XX SoC Family"
+	depends on ARCH_NPCM || COMPILE_TEST
+	help
+	  This driver supports the clocks on the Nuvoton BMC NPCM8XX SoC Family,
+	  all the clocks are initialized by the bootloader, so this driver
+	  allows only reading of current settings directly from the hardware.
+
 config COMMON_CLK_LOONGSON2
 	bool "Clock driver for Loongson-2 SoC"
 	depends on LOONGARCH || COMPILE_TEST
drivers/clk/Makefile +1 -0
index 18969cbd4bb1..e2cbc6cceb8c 100644
@@ -51,6 +51,7 @@ obj-$(CONFIG_ARCH_MILBEAUT_M10V)	+= clk-milbeaut.o
 obj-$(CONFIG_ARCH_MOXART)		+= clk-moxart.o
 obj-$(CONFIG_ARCH_NOMADIK)		+= clk-nomadik.o
 obj-$(CONFIG_ARCH_NPCM7XX)	    	+= clk-npcm7xx.o
+obj-$(CONFIG_COMMON_CLK_NPCM8XX)	+= clk-npcm8xx.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= clk-nspire.o
 obj-$(CONFIG_COMMON_CLK_PALMAS)		+= clk-palmas.o
 obj-$(CONFIG_CLK_LS1028A_PLLDIG)	+= clk-plldig.o
drivers/clk/clk-npcm8xx.c new file +565 -0

565 lines not on this page · raw message

Signature · 1 line
2.33.0
#1 lore raw

Le 28/09/2023 à 21:45, Tomer Maimon a écrit :

Quoted 6 lines

Nuvoton Arbel BMC NPCM8XX contains an integrated clock controller which generates and supplies clocks to all modules within the BMC.

---

...

Quoted 15 lines
drivers/clk/clk-npcm8xx.c new file +15 -0
+static struct clk_hw *
+npcm8xx_clk_register_pll(struct device *dev, void __iomem *pllcon,
+			 const char *name, const struct clk_parent_data *parent,
+			 unsigned long flags)
+{
+	struct npcm8xx_clk_pll *pll;
+	struct clk_init_data init = {};
+	int ret;
+
+	pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &npcm8xx_clk_pll_ops;

drivers/clk/clk-npcm8xx.c line 375

Quoted 10 lines
drivers/clk/clk-npcm8xx.c new file +10 -0
+	init.parent_data = parent;
+	init.num_parents = 1;
+	init.flags = flags;
+
+	pll->pllcon = pllcon;
+	pll->hw.init = &init;
+
+	ret = devm_clk_hw_register(dev, &pll->hw);
+	if (ret) {
+		kfree(pll);

Double free. pll is devm_kzalloc()'ed.

Quoted 35 lines
drivers/clk/clk-npcm8xx.c new file +35 -0
+		return ERR_PTR(ret);
+	}
+
+	return &pll->hw;
+}
+
+static DEFINE_SPINLOCK(npcm8xx_clk_lock);
+
+static int npcm8xx_clk_probe(struct platform_device *pdev)
+{
+	struct clk_hw_onecell_data *npcm8xx_clk_data;
+	struct device *dev = &pdev->dev;
+	void __iomem *clk_base;
+	struct resource *res;
+	struct clk_hw *hw;
+	unsigned int i;
+	int err;
+
+	npcm8xx_clk_data = devm_kzalloc(dev, struct_size(npcm8xx_clk_data, hws,
+							 NPCM8XX_NUM_CLOCKS),
+					GFP_KERNEL);
+	if (!npcm8xx_clk_data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	clk_base = devm_ioremap(dev, res->start, resource_size(res));
+	if (!clk_base) {
+		dev_err(&pdev->dev, "Failed to remap I/O memory\n");
+		return -ENOMEM;
+	}
+
+	npcm8xx_clk_data->num = NPCM8XX_NUM_CLOCKS;
+
+	for (i = 0; i < NPCM8XX_NUM_CLOCKS; i++)
+		npcm8xx_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);

drivers/clk/clk-npcm8xx.c line 420

Quoted 10 lines
drivers/clk/clk-npcm8xx.c new file +10 -0
+
+	/* Register plls */
+	for (i = 0; i < ARRAY_SIZE(npcm8xx_pll_clks); i++) {
+		struct npcm8xx_pll_data *pll_clk = &npcm8xx_pll_clks[i];
+
+		hw = npcm8xx_clk_register_pll(dev, clk_base + pll_clk->reg,
+					      pll_clk->name, &pll_clk->parent,
+					      pll_clk->flags);
+		if (IS_ERR(hw)) {
+			dev_err(dev, "npcm8xx_clk: Can't register pll\n");

Defining pr_fmt() would simplify the error message.

Also dev_err_probe() could save a few LoC and log the error codes.

Quoted 96 lines
drivers/clk/clk-npcm8xx.c new file +96 -0
+			return PTR_ERR(hw);
+		}
+		pll_clk->hw = *hw;
+	}
+
+	/* Register fixed dividers */
+	hw = devm_clk_hw_register_fixed_factor(dev, NPCM8XX_CLK_S_PLL1_DIV2,
+					       NPCM8XX_CLK_S_PLL1, 0, 1, 2);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "npcm8xx_clk: Can't register fixed div\n");
+		return PTR_ERR(hw);
+	}
+	hw_pll1_div2 = *hw;
+
+	hw = devm_clk_hw_register_fixed_factor(dev, NPCM8XX_CLK_S_PLL2_DIV2,
+					       NPCM8XX_CLK_S_PLL2, 0, 1, 2);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "npcm8xx_clk: Can't register pll2 div2\n");
+		return PTR_ERR(hw);
+	}
+	hw_pll2_div2 = *hw;
+
+	hw = devm_clk_hw_register_fixed_factor(dev, NPCM8XX_CLK_S_PLL_GFX_DIV2,
+					       NPCM8XX_CLK_S_PLL_GFX, 0, 1, 2);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "npcm8xx_clk: Can't register gfx div2\n");
+		return PTR_ERR(hw);
+	}
+	hw_gfx_div2 = *hw;
+
+	/* Register muxes */
+	for (i = 0; i < ARRAY_SIZE(npcm8xx_muxes); i++) {
+		struct npcm8xx_clk_mux_data *mux_data = &npcm8xx_muxes[i];
+
+		hw = devm_clk_hw_register_mux_parent_data_table(dev,
+								mux_data->name,
+								mux_data->parent_data,
+								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)) {
+			dev_err(dev, "npcm8xx_clk: Can't register mux\n");
+			return PTR_ERR(hw);
+		}
+		mux_data->hw = *hw;
+	}
+
+	hw = devm_clk_hw_register_fixed_factor(dev, NPCM8XX_CLK_S_PRE_CLK,
+					       NPCM8XX_CLK_S_CPU_MUX, 0, 1, 2);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "npcm8xx_clk: Can't register pre clk div2\n");
+		return PTR_ERR(hw);
+	}
+	hw_pre_clk = *hw;
+
+	hw = devm_clk_hw_register_fixed_factor(dev, NPCM8XX_CLK_S_AXI,
+					       NPCM8XX_CLK_S_TH, 0, 1, 2);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "npcm8xx_clk: Can't register axi div2\n");
+		return PTR_ERR(hw);
+	}
+	npcm8xx_clk_data->hws[NPCM8XX_CLK_AXI] = hw;
+
+	hw = devm_clk_hw_register_fixed_factor(dev, NPCM8XX_CLK_S_ATB,
+					       NPCM8XX_CLK_S_AXI, 0, 1, 2);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "npcm8xx_clk: Can't register atb div2\n");
+		return PTR_ERR(hw);
+	}
+	npcm8xx_clk_data->hws[NPCM8XX_CLK_ATB] = 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_parent_data(dev, div_data->name,
+							 &div_data->parent_data,
+							 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)) {
+			dev_err(dev, "npcm8xx_clk: Can't register div table\n");
+			goto err_div_clk;
+		}
+
+		if (div_data->onecell_idx >= 0)
+			npcm8xx_clk_data->hws[div_data->onecell_idx] = hw;
+	}

drivers/clk/clk-npcm8xx.c line 526

Quoted 10 lines
drivers/clk/clk-npcm8xx.c new file +10 -0
+
+	err = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
+					  npcm8xx_clk_data);
+	if (err) {
+		dev_err(dev, "unable to add clk provider\n");
+		hw = ERR_PTR(err);
+		goto err_div_clk;
+	}
+
+	return err;

return 0 is the same but more explicit.

Quoted 39 lines
drivers/clk/clk-npcm8xx.c new file +39 -0
+
+err_div_clk:
+	while (i--)
+		if (npcm8xx_divs[i].onecell_idx >= 0)
+			clk_hw_unregister_divider(npcm8xx_clk_data->hws[npcm8xx_divs[i].onecell_idx]);
+
+	return PTR_ERR(hw);
+}
+
+static const struct of_device_id npcm8xx_clk_dt_ids[] = {
+	{ .compatible = "nuvoton,npcm845-clk", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, npcm8xx_clk_dt_ids);
+
+static struct platform_driver npcm8xx_clk_driver = {
+	.probe  = npcm8xx_clk_probe,
+	.driver = {
+		.name = "npcm8xx_clk",
+		.of_match_table = npcm8xx_clk_dt_ids,
+	},
+};
+
+static int __init npcm8xx_clk_driver_init(void)
+{
+	return platform_driver_register(&npcm8xx_clk_driver);
+}
+arch_initcall(npcm8xx_clk_driver_init);
+
+static void __exit npcm8xx_clk_exit(void)
+{
+	platform_driver_unregister(&npcm8xx_clk_driver);
+}
+module_exit(npcm8xx_clk_exit);
+
+MODULE_DESCRIPTION("Clock driver for Nuvoton NPCM8XX BMC SoC");
+MODULE_AUTHOR("Tomer Maimon <[email protected]>");
+MODULE_LICENSE("GPL v2");
+