Quoting Sivaprakash Murugesan (2020-04-13 19:55:19) > The CPU on Qualcomm's IPQ platform devices are clocked primarily by a > PLL and xo which are connected to a mux and enable block, This patch adds The comma should be a period? Don't write "This patch" in commit text. > support for the mux and the enable. > > Signed-off-by: Sivaprakash Murugesan <sivaprak@xxxxxxxxxxxxxx> > --- > drivers/clk/qcom/Kconfig | 10 +++++ > drivers/clk/qcom/Makefile | 1 + > drivers/clk/qcom/apss-ipq.c | 107 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 118 insertions(+) > create mode 100644 drivers/clk/qcom/apss-ipq.c > > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > index 15cdcdc..8573f2e 100644 > --- a/drivers/clk/qcom/Kconfig > +++ b/drivers/clk/qcom/Kconfig > @@ -89,6 +89,16 @@ config APQ_MMCC_8084 > Say Y if you want to support multimedia devices such as display, > graphics, video encode/decode, camera, etc. > > +config IPQ_APSS > + tristate "IPQ APSS Clock Controller" > + default N Drop this, it's already the default. > + help > + Support for APSS clock controller on ipq platform devices. The Maybe say "IPQ platforms" and drop the devices part? > + APSS clock controller manages the Mux and enable block that feeds the > + CPUs. > + Say Y if you want to support CPU frequency scaling on > + ipq based devices. > + > config IPQ_GCC_4019 > tristate "IPQ4019 Global Clock Controller" > help > diff --git a/drivers/clk/qcom/apss-ipq.c b/drivers/clk/qcom/apss-ipq.c > new file mode 100644 > index 0000000..a37cd98 > --- /dev/null > +++ b/drivers/clk/qcom/apss-ipq.c > @@ -0,0 +1,107 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018, The Linux Foundation. All rights reserved. > + */ > + > +#include <linux/kernel.h> > +#include <linux/err.h> > +#include <linux/platform_device.h> > +#include <linux/clk-provider.h> > +#include <linux/regmap.h> > +#include <linux/module.h> > + > +#include <dt-bindings/clock/qcom,apss-ipq.h> > + > +#include "common.h" > +#include "clk-regmap.h" > +#include "clk-branch.h" > +#include "clk-alpha-pll.h" > +#include "clk-regmap-mux.h" > + > +enum { > + P_XO, > + P_APSS_PLL_EARLY, > +}; > + > +static const struct clk_parent_data parents_apcs_alias0_clk_src[] = { > + { .fw_name = "xo" }, > + { .fw_name = "pll" }, > +}; > + > +static const struct parent_map parents_apcs_alias0_clk_src_map[] = { > + { P_XO, 0 }, > + { P_APSS_PLL_EARLY, 5 }, > +}; > + > +static struct clk_regmap_mux apcs_alias0_clk_src = { > + .reg = 0x0050, > + .width = 3, > + .shift = 7, > + .parent_map = parents_apcs_alias0_clk_src_map, > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "apcs_alias0_clk_src", > + .parent_data = parents_apcs_alias0_clk_src, > + .num_parents = 2, > + .ops = &clk_regmap_mux_closest_ops, > + .flags = CLK_SET_RATE_PARENT, > + }, > +}; > + > +/*required for cpufreq*/ This comment doesn't help in understanding. > +static struct clk_branch apcs_alias0_core_clk = { > + .halt_reg = 0x0058, > + .clkr = { > + .enable_reg = 0x0058, > + .enable_mask = BIT(0), > + .hw.init = &(struct clk_init_data){ > + .name = "apcs_alias0_core_clk", > + .parent_hws = (const struct clk_hw *[]){ > + &apcs_alias0_clk_src.clkr.hw }, > + .num_parents = 1, > + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, Please comment why the clk is critical. > + .ops = &clk_branch2_ops, > + }, > + }, > +}; > + > +static const struct regmap_config apss_ipq_regmap_config = { > + .reg_bits = 32, > + .reg_stride = 4, > + .val_bits = 32, > + .max_register = 0x1000, > + .fast_io = true, > +}; > + > +static struct clk_regmap *apss_ipq_clks[] = { > + [APCS_ALIAS0_CLK_SRC] = &apcs_alias0_clk_src.clkr, > + [APCS_ALIAS0_CORE_CLK] = &apcs_alias0_core_clk.clkr, > +}; > + > +static const struct qcom_cc_desc apss_ipq_desc = { > + .config = &apss_ipq_regmap_config, > + .clks = apss_ipq_clks, > + .num_clks = ARRAY_SIZE(apss_ipq_clks), > +}; > + > +static int apss_ipq_probe(struct platform_device *pdev) > +{ > + struct regmap *regmap; > + > + regmap = dev_get_regmap(pdev->dev.parent, NULL); > + if (IS_ERR(regmap)) > + return PTR_ERR(regmap); > + > + return qcom_cc_really_probe(pdev, &apss_ipq_desc, regmap); What is this a child of? > +} > + > +static struct platform_driver apss_ipq_driver = { > + .probe = apss_ipq_probe, > + .driver = { > + .name = "qcom,apss-ipq-clk", > + }, > +};