| Message ID | 20190918013459.15966-1-dinguyen@kernel.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/2] dt-bindings: documentation: add clock bindings information for Agilex | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | "total: 0 errors, 1 warnings, 90 lines checked" |
Quoting Dinh Nguyen (2019-09-17 18:34:58) > From: Dinh Nguyen <dinh.nguyen@intel.com> > > Document the Agilex clock bindings, and add the clock header file. The > clock header is an enumeration of all the different clocks on the Agilex > platform. > > Signed-off-by: Dinh Nguyen <dinguyen@kernel.org> This needs a signed-off-by from your intel address as that's the author. > --- > .../devicetree/bindings/clock/intc_agilex.txt | 20 ++++++ > include/dt-bindings/clock/agilex-clock.h | 70 +++++++++++++++++++ > 2 files changed, 90 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/intc_agilex.txt > create mode 100644 include/dt-bindings/clock/agilex-clock.h > > diff --git a/Documentation/devicetree/bindings/clock/intc_agilex.txt b/Documentation/devicetree/bindings/clock/intc_agilex.txt > new file mode 100644 > index 000000000000..bfec71420511 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/intc_agilex.txt > @@ -0,0 +1,20 @@ > +Device Tree Clock bindings for Intel's SoCFPGA Agilex platform > + > +This binding uses the common clock binding[1]. Can you write this binding in YAML? That way we can verify the simple usage of it easily. > + > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt > + > +Required properties: > +- compatible : shall be > + "intel,agilex-clkmgr" > + > +- reg : shall be the control register offset from CLOCK_MANAGER's base for the clock. I don't quite understand what this means. What is the CLOCK_MANAGER base for the clock? Doesn't this describe an offset to a clock controller which provides more than one clk? > + > +- #clock-cells : from common clock binding, shall be set to 1. > + > +Example: > + clkmgr: clock-controller@ffd10000 { > + compatible = "intel,agilex-clkmgr"; > + reg = <0xffd10000 0x1000>; > + #clock-cells = <1>; > + };
Quoting Dinh Nguyen (2019-09-17 18:34:59) > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index 0cad76021297..ef2c96c0f1e0 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -18,6 +18,7 @@ endif > > # hardware specific clock types > # please keep this section sorted lexicographically by file path name > +obj-$(CONFIG_ARCH_AGILEX) += socfpga/ Sort by filepath, so all files come before all directories, and directories have a different section in this file. > obj-$(CONFIG_MACH_ASM9260) += clk-asm9260.o > obj-$(CONFIG_COMMON_CLK_AXI_CLKGEN) += clk-axi-clkgen.o > obj-$(CONFIG_ARCH_AXXIA) += clk-axm5516.o > diff --git a/drivers/clk/socfpga/Makefile b/drivers/clk/socfpga/Makefile > index ce5aa7802eb8..bf736f8d201a 100644 > --- a/drivers/clk/socfpga/Makefile > +++ b/drivers/clk/socfpga/Makefile > @@ -3,3 +3,5 @@ obj-$(CONFIG_ARCH_SOCFPGA) += clk.o clk-gate.o clk-pll.o clk-periph.o > obj-$(CONFIG_ARCH_SOCFPGA) += clk-pll-a10.o clk-periph-a10.o clk-gate-a10.o > obj-$(CONFIG_ARCH_STRATIX10) += clk-s10.o > obj-$(CONFIG_ARCH_STRATIX10) += clk-pll-s10.o clk-periph-s10.o clk-gate-s10.o > +obj-$(CONFIG_ARCH_AGILEX) += clk-agilex.o > +obj-$(CONFIG_ARCH_AGILEX) += clk-pll-s10.o clk-periph-s10.o clk-gate-s10.o Preferably keep this sorted on Kconfig name. > diff --git a/drivers/clk/socfpga/clk-agilex.c b/drivers/clk/socfpga/clk-agilex.c > new file mode 100644 > index 000000000000..7d5093f0b2c9 > --- /dev/null > +++ b/drivers/clk/socfpga/clk-agilex.c > @@ -0,0 +1,332 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2019, Intel Corporation > + */ > +#include <linux/slab.h> > +#include <linux/clk-provider.h> > +#include <linux/of_device.h> > +#include <linux/of_address.h> > +#include <linux/platform_device.h> > + > +#include <dt-bindings/clock/agilex-clock.h> > + > +#include "stratix10-clk.h" > + > +static const char * const pll_mux[] = { "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk",}; > +static const char * const cntr_mux[] = { "main_pll", "periph_pll", > + "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk"}; > +static const char * const boot_mux[] = { "osc1", "cb-intosc-hs-div2-clk",}; > + > +static const char * const mpu_free_mux[] = {"main_pll_c0", "peri_pll_c0", > + "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk"}; > + > +static const char * const noc_free_mux[] = {"main_pll_c1", "peri_pll_c1", > + "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk"}; > + > +static const char * const emaca_free_mux[] = {"main_pll_c2", "peri_pll_c2", > + "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk"}; > +static const char * const emacb_free_mux[] = {"main_pll_c3", "peri_pll_c3", > + "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk"}; > +static const char * const emac_ptp_free_mux[] = {"main_pll_c3", "peri_pll_c3", > + "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk"}; > +static const char * const gpio_db_free_mux[] = {"main_pll_c3", "peri_pll_c3", > + "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk"}; > +static const char * const psi_ref_free_mux[] = {"main_pll_c3", "peri_pll_c3", > + "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk"}; > +static const char * const sdmmc_free_mux[] = {"main_pll_c3", "peri_pll_c3", > + "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk"}; > +static const char * const s2f_usr0_free_mux[] = {"main_pll_c2", "peri_pll_c2", > + "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk"}; > +static const char * const s2f_usr1_free_mux[] = {"main_pll_c2", "peri_pll_c2", > + "osc1", "cb-intosc-hs-div2-clk", > + "f2s-free-clk"}; > +static const char * const mpu_mux[] = { "mpu_free_clk", "boot_clk",}; > + > +static const char * const s2f_usr0_mux[] = {"f2s-free-clk", "boot_clk"}; > +static const char * const emac_mux[] = {"emaca_free_clk", "emacb_free_clk"}; > +static const char * const noc_mux[] = {"noc_free_clk", "boot_clk"}; > + > +/* clocks in AO (always on) controller */ > +static const struct stratix10_pll_clock agilex_pll_clks[] = { > + { AGILEX_BOOT_CLK, "boot_clk", boot_mux, ARRAY_SIZE(boot_mux), 0, > + 0x0}, > + { AGILEX_MAIN_PLL_CLK, "main_pll", pll_mux, ARRAY_SIZE(pll_mux), > + 0, 0x48}, > + { AGILEX_PERIPH_PLL_CLK, "periph_pll", pll_mux, ARRAY_SIZE(pll_mux), > + 0, 0x9c}, > +}; > + > +static const struct stratix10_perip_c_clock agilex_main_perip_c_clks[] = { > + { AGILEX_MAIN_PLL_C0_CLK, "main_pll_c0", "main_pll", NULL, 1, 0, 0x58}, > + { AGILEX_MAIN_PLL_C1_CLK, "main_pll_c1", "main_pll", NULL, 1, 0, 0x5C}, > + { AGILEX_MAIN_PLL_C2_CLK, "main_pll_c2", "main_pll", NULL, 1, 0, 0x64}, > + { AGILEX_MAIN_PLL_C3_CLK, "main_pll_c3", "main_pll", NULL, 1, 0, 0x68}, > + { AGILEX_PERIPH_PLL_C0_CLK, "peri_pll_c0", "periph_pll", NULL, 1, 0, 0xAC}, > + { AGILEX_PERIPH_PLL_C1_CLK, "peri_pll_c1", "periph_pll", NULL, 1, 0, 0xB0}, > + { AGILEX_PERIPH_PLL_C2_CLK, "peri_pll_c2", "periph_pll", NULL, 1, 0, 0xB8}, > + { AGILEX_PERIPH_PLL_C3_CLK, "peri_pll_c3", "periph_pll", NULL, 1, 0, 0xBC}, > +}; > + > +static const struct stratix10_perip_cnt_clock agilex_main_perip_cnt_clks[] = { > + { AGILEX_MPU_FREE_CLK, "mpu_free_clk", NULL, mpu_free_mux, ARRAY_SIZE(mpu_free_mux), > + 0, 0x3C, 0, 0, 0}, > + { AGILEX_NOC_FREE_CLK, "noc_free_clk", NULL, noc_free_mux, ARRAY_SIZE(noc_free_mux), > + 0, 0x40, 0, 0, 1}, > + { AGILEX_L4_SYS_FREE_CLK, "l4_sys_free_clk", "noc_free_clk", NULL, 1, 0, > + 0, 4, 0, 0}, > + { AGILEX_NOC_CLK, "noc_clk", NULL, noc_mux, ARRAY_SIZE(noc_mux), > + 0, 0, 0, 0x30, 1}, > + { AGILEX_EMAC_A_FREE_CLK, "emaca_free_clk", NULL, emaca_free_mux, ARRAY_SIZE(emaca_free_mux), > + 0, 0xD4, 0, 0x88, 0}, > + { AGILEX_EMAC_B_FREE_CLK, "emacb_free_clk", NULL, emacb_free_mux, ARRAY_SIZE(emacb_free_mux), > + 0, 0xD8, 0, 0x88, 1}, > + { AGILEX_EMAC_PTP_FREE_CLK, "emac_ptp_free_clk", NULL, emac_ptp_free_mux, > + ARRAY_SIZE(emac_ptp_free_mux), 0, 0xDC, 0, 0x88, 2}, > + { AGILEX_GPIO_DB_FREE_CLK, "gpio_db_free_clk", NULL, gpio_db_free_mux, > + ARRAY_SIZE(gpio_db_free_mux), 0, 0xE0, 0, 0x88, 3}, > + { AGILEX_SDMMC_FREE_CLK, "sdmmc_free_clk", NULL, sdmmc_free_mux, > + ARRAY_SIZE(sdmmc_free_mux), 0, 0xE4, 0, 0x88, 4}, > + { AGILEX_S2F_USER0_FREE_CLK, "s2f_user0_free_clk", NULL, s2f_usr0_free_mux, > + ARRAY_SIZE(s2f_usr0_free_mux), 0, 0xE8, 0, 0, 0}, > + { AGILEX_S2F_USER1_FREE_CLK, "s2f_user1_free_clk", NULL, s2f_usr1_free_mux, > + ARRAY_SIZE(s2f_usr1_free_mux), 0, 0xEC, 0, 0x88, 5}, > + { AGILEX_PSI_REF_FREE_CLK, "psi_ref_free_clk", NULL, psi_ref_free_mux, > + ARRAY_SIZE(psi_ref_free_mux), 0, 0xF0, 0, 0x88, 6}, > +}; > + > +static const struct stratix10_gate_clock agilex_gate_clks[] = { > + { AGILEX_MPU_CLK, "mpu_clk", NULL, mpu_mux, ARRAY_SIZE(mpu_mux), 0, 0x24, > + 0, 0, 0, 0, 0x30, 0, 0}, > + { AGILEX_MPU_PERIPH_CLK, "mpu_periph_clk", "mpu_clk", NULL, 1, 0, 0x24, > + 0, 0, 0, 0, 0, 0, 4}, > + { AGILEX_MPU_L2RAM_CLK, "mpu_l2ram_clk", "mpu_clk", NULL, 1, 0, 0x24, > + 0, 0, 0, 0, 0, 0, 2}, > + { AGILEX_L4_MAIN_CLK, "l4_main_clk", "noc_clk", NULL, 1, 0, 0x24, > + 1, 0x44, 0, 2, 0, 0, 0}, > + { AGILEX_L4_MP_CLK, "l4_mp_clk", "noc_clk", NULL, 1, 0, 0x24, > + 2, 0x44, 8, 2, 0, 0, 0}, > + { AGILEX_L4_SP_CLK, "l4_sp_clk", "noc_clk", NULL, 1, CLK_IS_CRITICAL, 0x24, Please document why a clk is critical in the code. > + 3, 0x44, 16, 2, 0, 0, 0}, > + { AGILEX_CS_AT_CLK, "cs_at_clk", "noc_clk", NULL, 1, 0, 0x24, > + 4, 0x44, 24, 2, 0, 0, 0}, > + { AGILEX_CS_TRACE_CLK, "cs_trace_clk", "noc_clk", NULL, 1, 0, 0x24, > + 4, 0x44, 26, 2, 0, 0, 0}, > + { AGILEX_CS_PDBG_CLK, "cs_pdbg_clk", "cs_at_clk", NULL, 1, 0, 0x24, > + 4, 0x44, 28, 1, 0, 0, 0}, > + { AGILEX_CS_TIMER_CLK, "cs_timer_clk", "noc_clk", NULL, 1, 0, 0x24, > + 5, 0, 0, 0, 0, 0, 0}, > + { AGILEX_S2F_USER0_CLK, "s2f_user0_clk", NULL, s2f_usr0_mux, ARRAY_SIZE(s2f_usr0_mux), 0, 0x24, > + 6, 0, 0, 0, 0, 0, 0}, > + { AGILEX_EMAC0_CLK, "emac0_clk", NULL, emac_mux, ARRAY_SIZE(emac_mux), 0, 0x7C, > + 0, 0, 0, 0, 0x94, 26, 0}, > + { AGILEX_EMAC1_CLK, "emac1_clk", NULL, emac_mux, ARRAY_SIZE(emac_mux), 0, 0x7C, > + 1, 0, 0, 0, 0x94, 27, 0}, > + { AGILEX_EMAC2_CLK, "emac2_clk", NULL, emac_mux, ARRAY_SIZE(emac_mux), 0, 0x7C, > + 2, 0, 0, 0, 0x94, 28, 0}, > + { AGILEX_EMAC_PTP_CLK, "emac_ptp_clk", "emac_ptp_free_clk", NULL, 1, 0, 0x7C, > + 3, 0, 0, 0, 0, 0, 0}, > + { AGILEX_GPIO_DB_CLK, "gpio_db_clk", "gpio_db_free_clk", NULL, 1, 0, 0x7C, > + 4, 0x98, 0, 16, 0, 0, 0}, > + { AGILEX_SDMMC_CLK, "sdmmc_clk", "sdmmc_free_clk", NULL, 1, 0, 0x7C, > + 5, 0, 0, 0, 0, 0, 4}, > + { AGILEX_S2F_USER1_CLK, "s2f_user1_clk", "s2f_user1_free_clk", NULL, 1, 0, 0x7C, > + 6, 0, 0, 0, 0, 0, 0}, > + { AGILEX_PSI_REF_CLK, "psi_ref_clk", "psi_ref_free_clk", NULL, 1, 0, 0x7C, > + 7, 0, 0, 0, 0, 0, 0}, > + { AGILEX_USB_CLK, "usb_clk", "l4_mp_clk", NULL, 1, 0, 0x7C, > + 8, 0, 0, 0, 0, 0, 0}, > + { AGILEX_SPI_M_CLK, "spi_m_clk", "l4_mp_clk", NULL, 1, 0, 0x7C, > + 9, 0, 0, 0, 0, 0, 0}, > + { AGILEX_NAND_CLK, "nand_clk", "l4_main_clk", NULL, 1, 0, 0x7C, > + 10, 0, 0, 0, 0, 0, 0}, > +}; > + > +static int agilex_clk_register_c_perip(const struct stratix10_perip_c_clock *clks, > + int nums, struct stratix10_clock_data *data) > +{ > + struct clk *clk; > + void __iomem *base = data->base; > + int i; > + > + for (i = 0; i < nums; i++) { > + clk = s10_register_periph(clks[i].name, clks[i].parent_name, > + clks[i].parent_names, clks[i].num_parents, > + clks[i].flags, base, clks[i].offset); > + if (IS_ERR(clk)) { > + pr_err("%s: failed to register clock %s\n", > + __func__, clks[i].name); > + continue; > + } > + data->clk_data.clks[clks[i].id] = clk; > + } > + return 0; > +} > + > +static int agilex_clk_register_cnt_perip(const struct stratix10_perip_cnt_clock *clks, > + int nums, struct stratix10_clock_data *data) > +{ > + struct clk *clk; > + void __iomem *base = data->base; > + int i; > + > + for (i = 0; i < nums; i++) { > + clk = s10_register_cnt_periph(clks[i].name, clks[i].parent_name, > + clks[i].parent_names, > + clks[i].num_parents, > + clks[i].flags, base, > + clks[i].offset, > + clks[i].fixed_divider, > + clks[i].bypass_reg, > + clks[i].bypass_shift); > + if (IS_ERR(clk)) { > + pr_err("%s: failed to register clock %s\n", > + __func__, clks[i].name); > + continue; > + } > + data->clk_data.clks[clks[i].id] = clk; > + } > + > + return 0; > +} > + > +static int agilex_clk_register_gate(const struct stratix10_gate_clock *clks, int nums, struct stratix10_clock_data *data) > +{ Something weird happened here. > + struct clk *clk; > + void __iomem *base = data->base; > + int i; > + > + for (i = 0; i < nums; i++) { > + clk = s10_register_gate(clks[i].name, clks[i].parent_name, > + clks[i].parent_names, > + clks[i].num_parents, > + clks[i].flags, base, > + clks[i].gate_reg, > + clks[i].gate_idx, clks[i].div_reg, > + clks[i].div_offset, clks[i].div_width, > + clks[i].bypass_reg, > + clks[i].bypass_shift, > + clks[i].fixed_div); Maybe s10_regsiter_gate should take a struct stratix10_gate_clock instead? > + if (IS_ERR(clk)) { > + pr_err("%s: failed to register clock %s\n", > + __func__, clks[i].name); > + continue; > + } > + data->clk_data.clks[clks[i].id] = clk; > + } > + > + return 0; > +} > + > +static int agilex_clk_register_pll(const struct stratix10_pll_clock *clks, > + int nums, struct stratix10_clock_data *data) > +{ > + struct clk *clk; > + void __iomem *base = data->base; > + int i; > + > + for (i = 0; i < nums; i++) { > + clk = agilex_register_pll(clks[i].name, clks[i].parent_names, > + clks[i].num_parents, > + clks[i].flags, base, > + clks[i].offset); > + if (IS_ERR(clk)) { > + pr_err("%s: failed to register clock %s\n", > + __func__, clks[i].name); > + continue; > + } > + data->clk_data.clks[clks[i].id] = clk; Hmm I just reviewed something that looked so similar. Please use some local variables to simplify this line. > + } > + > + return 0; > +} > + > +static struct stratix10_clock_data *__socfpga_agilex_clk_init(struct platform_device *pdev, > + int nr_clks) > +{ > + struct device_node *np = pdev->dev.of_node; > + struct device *dev = &pdev->dev; > + struct stratix10_clock_data *clk_data; > + struct clk **clk_table; > + struct resource *res; > + void __iomem *base; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + base = devm_ioremap_resource(dev, res); > + if (IS_ERR(base)) { > + pr_err("%s: failed to map clock registers\n", __func__); devm_ioremap_resource() already prints this error. Also, there's a helper now that gets the resource and maps it in one call instead of two, please use it. > + return ERR_CAST(base); > + } > + > + clk_data = devm_kzalloc(dev, sizeof(*clk_data), GFP_KERNEL); > + if (!clk_data) > + return ERR_PTR(-ENOMEM); > + > + clk_data->base = base; > + clk_table = devm_kcalloc(dev, nr_clks, sizeof(*clk_table), GFP_KERNEL); > + if (!clk_table) > + return ERR_PTR(-ENOMEM); > + > + clk_data->clk_data.clks = clk_table; > + clk_data->clk_data.clk_num = nr_clks; > + of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data->clk_data); Please add the clk provider after registering all clks, not before. > + return clk_data; > +} > + > +static int agilex_clkmgr_init(struct platform_device *pdev) > +{ > + struct stratix10_clock_data *clk_data; > + > + clk_data = __socfpga_agilex_clk_init(pdev, AGILEX_NUM_CLKS); > + if (IS_ERR(clk_data)) > + return PTR_ERR(clk_data); Please inline this init function here. > + > + agilex_clk_register_pll(agilex_pll_clks, ARRAY_SIZE(agilex_pll_clks), clk_data); > + > + agilex_clk_register_c_perip(agilex_main_perip_c_clks, > + ARRAY_SIZE(agilex_main_perip_c_clks), clk_data); > + > + agilex_clk_register_cnt_perip(agilex_main_perip_cnt_clks, > + ARRAY_SIZE(agilex_main_perip_cnt_clks), > + clk_data); > + > + agilex_clk_register_gate(agilex_gate_clks, ARRAY_SIZE(agilex_gate_clks), > + clk_data); > + return 0; > +} > + > +static int agilex_clkmgr_probe(struct platform_device *pdev) > +{ > + return agilex_clkmgr_init(pdev); Just inline the function into probe. > +} > diff --git a/drivers/clk/socfpga/clk-pll-s10.c b/drivers/clk/socfpga/clk-pll-s10.c > index 4705eb544f01..e6ce0ec39494 100644 > --- a/drivers/clk/socfpga/clk-pll-s10.c > +++ b/drivers/clk/socfpga/clk-pll-s10.c > @@ -18,8 +18,12 @@ > #define SOCFPGA_PLL_RESET_MASK 0x2 > #define SOCFPGA_PLL_REFDIV_MASK 0x00003F00 > #define SOCFPGA_PLL_REFDIV_SHIFT 8 > +#define SOCFPGA_PLL_AREFDIV_MASK 0x00000F00 > +#define SOCFPGA_PLL_DREFDIV_MASK 0x00003000 > +#define SOCFPGA_PLL_DREFDIV_SHIFT 12 > #define SOCFPGA_PLL_MDIV_MASK 0xFF000000 > #define SOCFPGA_PLL_MDIV_SHIFT 24 > +#define SOCFPGA_AGILEX_PLL_MDIV_MASK 0x000003FF > #define SWCTRLBTCLKSEL_MASK 0x200 > #define SWCTRLBTCLKSEL_SHIFT 9 > > @@ -27,6 +31,27 @@ > > #define to_socfpga_clk(p) container_of(p, struct socfpga_pll, hw.hw) > > +static unsigned long agilex_clk_pll_recalc_rate(struct clk_hw *hwclk, > + unsigned long parent_rate) > +{ > + struct socfpga_pll *socfpgaclk = to_socfpga_clk(hwclk); > + unsigned long arefdiv, reg, mdiv; > + unsigned long long vco_freq; > + > + /* read VCO1 reg for numerator and denominator */ > + reg = readl(socfpgaclk->hw.reg); > + arefdiv = (reg & SOCFPGA_PLL_AREFDIV_MASK) >> SOCFPGA_PLL_REFDIV_SHIFT; > + > + vco_freq = (unsigned long long)parent_rate / arefdiv; Is this a 64-bit div? Probably needs a do_div() call instead. > + > + /* Read mdiv and fdiv from the fdbck register */ > + reg = readl(socfpgaclk->hw.reg + 0x24); > + mdiv = (reg & SOCFPGA_AGILEX_PLL_MDIV_MASK); > + > + vco_freq = (unsigned long long)vco_freq * mdiv; > + return (unsigned long)vco_freq; Drop the cast, it's implicit. > +} > + > static unsigned long clk_pll_recalc_rate(struct clk_hw *hwclk, > unsigned long parent_rate) > { > @@ -145,3 +176,42 @@ struct clk *s10_register_pll(const char *name, const char * const *parent_names, > } > return clk; > } > + > +struct clk *agilex_register_pll(const char *name, > + const char * const *parent_names, > + u8 num_parents, unsigned long flags, > + void __iomem *reg, unsigned long offset) > +{ > + struct clk *clk; > + struct socfpga_pll *pll_clk; > + struct clk_init_data init; > + > + pll_clk = kzalloc(sizeof(*pll_clk), GFP_KERNEL); > + if (WARN_ON(!pll_clk)) > + return NULL; > + > + pll_clk->hw.reg = reg + offset; > + > + if (streq(name, SOCFPGA_BOOT_CLK)) > + init.ops = &clk_boot_ops; > + else > + init.ops = &agilex_clk_pll_ops; > + > + init.name = name; > + init.flags = flags; > + > + init.num_parents = num_parents; > + init.parent_names = parent_names; Is it possible to use the new way of specifying clk parents here so that we don't have to keep using strings to describe the clk topology? > + pll_clk->hw.hw.init = &init; > + > + pll_clk->hw.bit_idx = SOCFPGA_PLL_POWER; > + clk_pll_ops.enable = clk_gate_ops.enable; > + clk_pll_ops.disable = clk_gate_ops.disable; > + > + clk = clk_register(NULL, &pll_clk->hw.hw); Any reason clk_hw_register() can't be used? > + if (WARN_ON(IS_ERR(clk))) { > + kfree(pll_clk); > + return NULL; > + } > + return clk; > +}
Hi Stephen, On 9/18/19 12:00 AM, Stephen Boyd wrote: > Quoting Dinh Nguyen (2019-09-17 18:34:59) >> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile >> index 0cad76021297..ef2c96c0f1e0 100644 >> --- a/drivers/clk/Makefile >> +++ b/drivers/clk/Makefile >> @@ -18,6 +18,7 @@ endif >> <snip> >> +struct clk *agilex_register_pll(const char *name, >> + const char * const *parent_names, >> + u8 num_parents, unsigned long flags, >> + void __iomem *reg, unsigned long offset) >> +{ >> + struct clk *clk; >> + struct socfpga_pll *pll_clk; >> + struct clk_init_data init; >> + >> + pll_clk = kzalloc(sizeof(*pll_clk), GFP_KERNEL); >> + if (WARN_ON(!pll_clk)) >> + return NULL; >> + >> + pll_clk->hw.reg = reg + offset; >> + >> + if (streq(name, SOCFPGA_BOOT_CLK)) >> + init.ops = &clk_boot_ops; >> + else >> + init.ops = &agilex_clk_pll_ops; >> + >> + init.name = name; >> + init.flags = flags; >> + >> + init.num_parents = num_parents; >> + init.parent_names = parent_names; > > Is it possible to use the new way of specifying clk parents here so that > we don't have to keep using strings to describe the clk topology? > Can you point me to what you mean here? Perhaps a driver that is using this new way of specifying clk parents? Thanks, Dinh
Quoting Dinh Nguyen (2019-12-03 07:15:21) > Hi Stephen, > > On 9/18/19 12:00 AM, Stephen Boyd wrote: > > Quoting Dinh Nguyen (2019-09-17 18:34:59) > >> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > >> index 0cad76021297..ef2c96c0f1e0 100644 > >> --- a/drivers/clk/Makefile > >> +++ b/drivers/clk/Makefile > >> @@ -18,6 +18,7 @@ endif > >> > > <snip> > > >> +struct clk *agilex_register_pll(const char *name, > >> + const char * const *parent_names, > >> + u8 num_parents, unsigned long flags, > >> + void __iomem *reg, unsigned long offset) > >> +{ > >> + struct clk *clk; > >> + struct socfpga_pll *pll_clk; > >> + struct clk_init_data init; > >> + > >> + pll_clk = kzalloc(sizeof(*pll_clk), GFP_KERNEL); > >> + if (WARN_ON(!pll_clk)) > >> + return NULL; > >> + > >> + pll_clk->hw.reg = reg + offset; > >> + > >> + if (streq(name, SOCFPGA_BOOT_CLK)) > >> + init.ops = &clk_boot_ops; > >> + else > >> + init.ops = &agilex_clk_pll_ops; > >> + > >> + init.name = name; > >> + init.flags = flags; > >> + > >> + init.num_parents = num_parents; > >> + init.parent_names = parent_names; > > > > Is it possible to use the new way of specifying clk parents here so that > > we don't have to keep using strings to describe the clk topology? > > > > Can you point me to what you mean here? Perhaps a driver that is using > this new way of specifying clk parents? > I'm supposed to write some documentation, but you can look for drivers that have 'struct clk_parent_data' until I write the doc up. $ git grep 'struct clk_parent_data' -- drivers/clk/
diff --git a/Documentation/devicetree/bindings/clock/intc_agilex.txt b/Documentation/devicetree/bindings/clock/intc_agilex.txt new file mode 100644 index 000000000000..bfec71420511 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/intc_agilex.txt @@ -0,0 +1,20 @@ +Device Tree Clock bindings for Intel's SoCFPGA Agilex platform + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible : shall be + "intel,agilex-clkmgr" + +- reg : shall be the control register offset from CLOCK_MANAGER's base for the clock. + +- #clock-cells : from common clock binding, shall be set to 1. + +Example: + clkmgr: clock-controller@ffd10000 { + compatible = "intel,agilex-clkmgr"; + reg = <0xffd10000 0x1000>; + #clock-cells = <1>; + }; diff --git a/include/dt-bindings/clock/agilex-clock.h b/include/dt-bindings/clock/agilex-clock.h new file mode 100644 index 000000000000..f19cf8ccbdd2 --- /dev/null +++ b/include/dt-bindings/clock/agilex-clock.h @@ -0,0 +1,70 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2019, Intel Corporation + */ + +#ifndef __AGILEX_CLOCK_H +#define __AGILEX_CLOCK_H + +/* fixed rate clocks */ +#define AGILEX_OSC1 0 +#define AGILEX_CB_INTOSC_HS_DIV2_CLK 1 +#define AGILEX_CB_INTOSC_LS_CLK 2 +#define AGILEX_L4_SYS_FREE_CLK 3 +#define AGILEX_F2S_FREE_CLK 4 + +/* PLL clocks */ +#define AGILEX_MAIN_PLL_CLK 5 +#define AGILEX_MAIN_PLL_C0_CLK 6 +#define AGILEX_MAIN_PLL_C1_CLK 7 +#define AGILEX_MAIN_PLL_C2_CLK 8 +#define AGILEX_MAIN_PLL_C3_CLK 9 +#define AGILEX_PERIPH_PLL_CLK 10 +#define AGILEX_PERIPH_PLL_C0_CLK 11 +#define AGILEX_PERIPH_PLL_C1_CLK 12 +#define AGILEX_PERIPH_PLL_C2_CLK 13 +#define AGILEX_PERIPH_PLL_C3_CLK 14 +#define AGILEX_MPU_FREE_CLK 15 +#define AGILEX_MPU_CCU_CLK 16 +#define AGILEX_BOOT_CLK 17 + +/* fixed factor clocks */ +#define AGILEX_L3_MAIN_FREE_CLK 18 +#define AGILEX_NOC_FREE_CLK 19 +#define AGILEX_S2F_USR0_CLK 20 +#define AGILEX_NOC_CLK 21 +#define AGILEX_EMAC_A_FREE_CLK 22 +#define AGILEX_EMAC_B_FREE_CLK 23 +#define AGILEX_EMAC_PTP_FREE_CLK 24 +#define AGILEX_GPIO_DB_FREE_CLK 25 +#define AGILEX_SDMMC_FREE_CLK 26 +#define AGILEX_S2F_USER0_FREE_CLK 27 +#define AGILEX_S2F_USER1_FREE_CLK 28 +#define AGILEX_PSI_REF_FREE_CLK 29 + +/* Gate clocks */ +#define AGILEX_MPU_CLK 30 +#define AGILEX_MPU_L2RAM_CLK 31 +#define AGILEX_MPU_PERIPH_CLK 32 +#define AGILEX_L4_MAIN_CLK 33 +#define AGILEX_L4_MP_CLK 34 +#define AGILEX_L4_SP_CLK 35 +#define AGILEX_CS_AT_CLK 36 +#define AGILEX_CS_TRACE_CLK 37 +#define AGILEX_CS_PDBG_CLK 38 +#define AGILEX_CS_TIMER_CLK 39 +#define AGILEX_S2F_USER0_CLK 40 +#define AGILEX_EMAC0_CLK 41 +#define AGILEX_EMAC1_CLK 43 +#define AGILEX_EMAC2_CLK 44 +#define AGILEX_EMAC_PTP_CLK 45 +#define AGILEX_GPIO_DB_CLK 46 +#define AGILEX_NAND_CLK 47 +#define AGILEX_PSI_REF_CLK 48 +#define AGILEX_S2F_USER1_CLK 49 +#define AGILEX_SDMMC_CLK 50 +#define AGILEX_SPI_M_CLK 51 +#define AGILEX_USB_CLK 52 +#define AGILEX_NUM_CLKS 53 + +#endif /* __AGILEX_CLOCK_H */