diff for duplicates of <024f01d3c1cb$304eaa60$90ebff20$@codeaurora.org> diff --git a/a/1.txt b/N1/1.txt index a6e618d..e11b5b4 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -3,19 +3,23 @@ > -----Original Message----- > From: Stephen Boyd <sboyd@kernel.org> > Sent: Monday, March 19, 2018 19:37 -> To: Ilia Lin <ilialin@codeaurora.org>; linux-arm-kernel@lists.infradead.org; +> To: Ilia Lin <ilialin@codeaurora.org>; = +linux-arm-kernel@lists.infradead.org; > linux-arm-msm@vger.kernel.org; linux-clk@vger.kernel.org; > sboyd@codeaurora.org > Cc: mark.rutland@arm.com; devicetree@vger.kernel.org; > rnayak@codeaurora.org; robh@kernel.org; will.deacon@arm.com; -> amit.kucheria@linaro.org; tfinkel@codeaurora.org; ilialin@codeaurora.org; +> amit.kucheria@linaro.org; tfinkel@codeaurora.org; = +ilialin@codeaurora.org; > nicolas.dechesne@linaro.org; celster@codeaurora.org -> Subject: Re: [PATCH v3 03/10] clk: qcom: Add CPU clock driver for msm8996 -> +> Subject: Re: [PATCH v3 03/10] clk: qcom: Add CPU clock driver for = +msm8996 +>=20 > Quoting Ilia Lin (2018-02-14 05:59:45) > > From: Rajendra Nayak <rnayak@codeaurora.org> > > -> > Each of the CPU clusters (Power and Perf) on msm8996 are clocked via 2 +> > Each of the CPU clusters (Power and Perf) on msm8996 are clocked via = +2 > > PLLs, a primary and alternate. There are also > > 2 Mux'es, a primary and secondary all connected together as shown > > below @@ -40,9 +44,10 @@ > > |Alt PLL | | > > | +---------------------------+ > > +---------------+ PLL_EARLY -> -> Thanks for the diagram. Please also put it at the top of the driver file. -> +>=20 +> Thanks for the diagram. Please also put it at the top of the driver = +file. +>=20 > > > > The primary PLL is what drives the CPU clk, except for times when we > > are reprogramming the PLL itself (for rate changes) when we @@ -53,17 +58,21 @@ > > The primary PLL operates on a single VCO range, between 600Mhz and > > 3Ghz. However the CPUs do support OPPs with frequencies between > 300Mhz -> > and 600Mhz. In order to support running the CPUs at those frequencies +> > and 600Mhz. In order to support running the CPUs at those = +frequencies > > we end up having to lock the PLL at twice the rate and drive the CPU > > clk via the PLL/2 output and SMUX. > > -> > So for frequencies above 600Mhz we follow the following path Primary -> > PLL --> PLL_EARLY --> PMUX(1) --> CPU clk and for frequencies between +> > So for frequencies above 600Mhz we follow the following path = +Primary +> > PLL --> PLL_EARLY --> PMUX(1) --> CPU clk and for frequencies = +between > > 300Mhz and 600Mhz we follow -> +>=20 > Capitalize 'h' in units please. -> -> > Primary PLL --> PLL/2 --> SMUX(1) --> PMUX(0) --> CPU clk Support for +>=20 +> > Primary PLL --> PLL/2 --> SMUX(1) --> PMUX(0) --> CPU clk Support = +for > > this is added in a subsequent patch as well. > > > > ACD stands for Adaptive Clock Distribution and is used to detect @@ -80,13 +89,16 @@ > > 3 files changed, 418 insertions(+) > > create mode 100644 drivers/clk/qcom/clk-cpu-8996.c > > -> > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig index +> > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig = +index > > fbf4532..3274877 100644 > > --- a/drivers/clk/qcom/Kconfig > > +++ b/drivers/clk/qcom/Kconfig > > @@ -226,3 +226,11 @@ config SPMI_PMIC_CLKDIV -> > Technologies, Inc. SPMI PMIC. It configures the frequency of -> > clkdiv outputs of the PMIC. These clocks are typically wired +> > Technologies, Inc. SPMI PMIC. It configures the frequency = +of +> > clkdiv outputs of the PMIC. These clocks are typically = +wired > > through alternate functions on GPIO pins. > > + > > +config MSM_APCC_8996 @@ -94,26 +106,27 @@ > > + depends on COMMON_CLK_QCOM > > + help > > + Support for the CPU clock controller on msm8996 devices. -> > + Say Y if you want to support CPU clock scaling using CPUfreq +> > + Say Y if you want to support CPU clock scaling using = +CPUfreq > > + drivers for dyanmic power management. -> +>=20 > Sort? -> +>=20 > > diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile > > index 230332c..57b38ba 100644 > > --- a/drivers/clk/qcom/Makefile > > +++ b/drivers/clk/qcom/Makefile -> > @@ -33,6 +33,7 @@ obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o -> > obj-$(CONFIG_MSM_MMCC_8960) += mmcc-msm8960.o -> > obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o -> > obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o -> > +obj-$(CONFIG_MSM_APCC_8996) += clk-cpu-8996.o -> +> > @@ -33,6 +33,7 @@ obj-$(CONFIG_MSM_LCC_8960) +=3D lcc-msm8960.o +> > obj-$(CONFIG_MSM_MMCC_8960) +=3D mmcc-msm8960.o +> > obj-$(CONFIG_MSM_MMCC_8974) +=3D mmcc-msm8974.o +> > obj-$(CONFIG_MSM_MMCC_8996) +=3D mmcc-msm8996.o +> > +obj-$(CONFIG_MSM_APCC_8996) +=3D clk-cpu-8996.o +>=20 > This doesn't look sorted. -> -> > obj-$(CONFIG_QCOM_A53PLL) += a53-pll.o -> > obj-$(CONFIG_QCOM_CLK_APCS_MSM8916) += apcs-msm8916.o -> > obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o diff --git +>=20 +> > obj-$(CONFIG_QCOM_A53PLL) +=3D a53-pll.o +> > obj-$(CONFIG_QCOM_CLK_APCS_MSM8916) +=3D apcs-msm8916.o +> > obj-$(CONFIG_QCOM_CLK_RPM) +=3D clk-rpm.o diff --git > > a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c > > new file mode 100644 index 0000000..42489f1 > > --- /dev/null @@ -124,7 +137,8 @@ > > + * > > + * This program is free software; you can redistribute it and/or > > +modify -> > + * it under the terms of the GNU General Public License version 2 and +> > + * it under the terms of the GNU General Public License version 2 = +and > > + * only version 2 as published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, @@ -132,141 +146,141 @@ > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ -> +>=20 > Can we get the SPDX tags here please? -> +>=20 > > + > > +#include <linux/clk.h> -> +>=20 > clk-provider.h? -> +>=20 > > +#include <linux/module.h> > > +#include <linux/platform_device.h> > > +#include <linux/regmap.h> > > + > > +#include "clk-alpha-pll.h" -> +>=20 > #include "clk-regmap.h" -> +>=20 > > + > > +#define VCO(a, b, c) { \ -> > + .val = a,\ -> > + .min_freq = b,\ -> > + .max_freq = c,\ +> > + .val =3D a,\ +> > + .min_freq =3D b,\ +> > + .max_freq =3D c,\ > > +} -> +>=20 > Can this define go into whatever PLL header file defines the struct? -> +>=20 > > + > > +#define DIV_2_INDEX 0 > > +#define PLL_INDEX 1 > > +#define ACD_INDEX 2 > > +#define ALT_INDEX 3 > > + -> > +static const u8 prim_pll_regs[PLL_OFF_MAX_REGS] = { -> > + [PLL_OFF_L_VAL] = 0x04, -> > + [PLL_OFF_ALPHA_VAL] = 0x08, -> > + [PLL_OFF_USER_CTL] = 0x10, -> > + [PLL_OFF_CONFIG_CTL] = 0x18, -> > + [PLL_OFF_CONFIG_CTL_U] = 0x1C, -> +> > +static const u8 prim_pll_regs[PLL_OFF_MAX_REGS] =3D { +> > + [PLL_OFF_L_VAL] =3D 0x04, +> > + [PLL_OFF_ALPHA_VAL] =3D 0x08, +> > + [PLL_OFF_USER_CTL] =3D 0x10, +> > + [PLL_OFF_CONFIG_CTL] =3D 0x18, +> > + [PLL_OFF_CONFIG_CTL_U] =3D 0x1C, +>=20 > Please use lowercase hex throughout. Consistency is key! -> -> > + [PLL_OFF_TEST_CTL] = 0x20, -> > + [PLL_OFF_TEST_CTL_U] = 0x24, -> > + [PLL_OFF_STATUS] = 0x28, +>=20 +> > + [PLL_OFF_TEST_CTL] =3D 0x20, +> > + [PLL_OFF_TEST_CTL_U] =3D 0x24, +> > + [PLL_OFF_STATUS] =3D 0x28, > > +}; > > + -> > +static const u8 alt_pll_regs[PLL_OFF_MAX_REGS] = { -> > + [PLL_OFF_L_VAL] = 0x04, -> > + [PLL_OFF_ALPHA_VAL] = 0x08, -> > + [PLL_OFF_ALPHA_VAL_U] = 0x0c, -> > + [PLL_OFF_USER_CTL] = 0x10, -> > + [PLL_OFF_USER_CTL_U] = 0x14, -> > + [PLL_OFF_CONFIG_CTL] = 0x18, -> > + [PLL_OFF_TEST_CTL] = 0x20, -> > + [PLL_OFF_TEST_CTL_U] = 0x24, -> > + [PLL_OFF_STATUS] = 0x28, +> > +static const u8 alt_pll_regs[PLL_OFF_MAX_REGS] =3D { +> > + [PLL_OFF_L_VAL] =3D 0x04, +> > + [PLL_OFF_ALPHA_VAL] =3D 0x08, +> > + [PLL_OFF_ALPHA_VAL_U] =3D 0x0c, +> > + [PLL_OFF_USER_CTL] =3D 0x10, +> > + [PLL_OFF_USER_CTL_U] =3D 0x14, +> > + [PLL_OFF_CONFIG_CTL] =3D 0x18, +> > + [PLL_OFF_TEST_CTL] =3D 0x20, +> > + [PLL_OFF_TEST_CTL_U] =3D 0x24, +> > + [PLL_OFF_STATUS] =3D 0x28, > > +}; > > + > > +/* PLLs */ > > + -> > +static const struct alpha_pll_config hfpll_config = { -> > + .l = 60, -> > + .config_ctl_val = 0x200d4828, -> > + .config_ctl_hi_val = 0x006, -> > + .pre_div_mask = BIT(12), -> > + .post_div_mask = 0x3 << 8, -> > + .main_output_mask = BIT(0), -> > + .early_output_mask = BIT(3), +> > +static const struct alpha_pll_config hfpll_config =3D { +> > + .l =3D 60, +> > + .config_ctl_val =3D 0x200d4828, +> > + .config_ctl_hi_val =3D 0x006, +> > + .pre_div_mask =3D BIT(12), +> > + .post_div_mask =3D 0x3 << 8, +> > + .main_output_mask =3D BIT(0), +> > + .early_output_mask =3D BIT(3), > > +}; > > + -> > +static struct clk_alpha_pll perfcl_pll = { -> > + .offset = 0x80000, -> > + .regs = prim_pll_regs, -> > + .flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE, -> > + .clkr.hw.init = &(struct clk_init_data){ -> > + .name = "perfcl_pll", -> > + .parent_names = (const char *[]){ "xo" }, -> > + .num_parents = 1, -> > + .ops = &clk_alpha_pll_huayra_ops, +> > +static struct clk_alpha_pll perfcl_pll =3D { +> > + .offset =3D 0x80000, +> > + .regs =3D prim_pll_regs, +> > + .flags =3D SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE, +> > + .clkr.hw.init =3D &(struct clk_init_data){ +> > + .name =3D "perfcl_pll", +> > + .parent_names =3D (const char *[]){ "xo" }, +> > + .num_parents =3D 1, +> > + .ops =3D &clk_alpha_pll_huayra_ops, > > + }, > > +}; > > + -> > +static struct clk_alpha_pll pwrcl_pll = { -> > + .offset = 0x0, -> > + .regs = prim_pll_regs, -> > + .flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE, -> > + .clkr.hw.init = &(struct clk_init_data){ -> > + .name = "pwrcl_pll", -> > + .parent_names = (const char *[]){ "xo" }, -> > + .num_parents = 1, -> > + .ops = &clk_alpha_pll_huayra_ops, +> > +static struct clk_alpha_pll pwrcl_pll =3D { +> > + .offset =3D 0x0, +> > + .regs =3D prim_pll_regs, +> > + .flags =3D SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE, +> > + .clkr.hw.init =3D &(struct clk_init_data){ +> > + .name =3D "pwrcl_pll", +> > + .parent_names =3D (const char *[]){ "xo" }, +> > + .num_parents =3D 1, +> > + .ops =3D &clk_alpha_pll_huayra_ops, > > + }, > > +}; > > + -> > +static const struct pll_vco alt_pll_vco_modes[] = { +> > +static const struct pll_vco alt_pll_vco_modes[] =3D { > > + VCO(3, 250000000, 500000000), > > + VCO(2, 500000000, 750000000), > > + VCO(1, 750000000, 1000000000), > > + VCO(0, 1000000000, 2150400000), }; > > + -> > +static const struct alpha_pll_config altpll_config = { -> > + .l = 16, -> > + .vco_val = 0x3 << 20, -> > + .vco_mask = 0x3 << 20, -> > + .config_ctl_val = 0x4001051b, -> > + .post_div_mask = 0x3 << 8, -> > + .post_div_val = 0x1, -> > + .main_output_mask = BIT(0), -> > + .early_output_mask = BIT(3), +> > +static const struct alpha_pll_config altpll_config =3D { +> > + .l =3D 16, +> > + .vco_val =3D 0x3 << 20, +> > + .vco_mask =3D 0x3 << 20, +> > + .config_ctl_val =3D 0x4001051b, +> > + .post_div_mask =3D 0x3 << 8, +> > + .post_div_val =3D 0x1, +> > + .main_output_mask =3D BIT(0), +> > + .early_output_mask =3D BIT(3), > > +}; > > + -> > +static struct clk_alpha_pll perfcl_alt_pll = { -> > + .offset = 0x80100, -> > + .regs = alt_pll_regs, -> > + .vco_table = alt_pll_vco_modes, -> > + .num_vco = ARRAY_SIZE(alt_pll_vco_modes), -> > + .flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE, -> > + .clkr.hw.init = &(struct clk_init_data) { -> > + .name = "perfcl_alt_pll", -> > + .parent_names = (const char *[]){ "xo" }, -> > + .num_parents = 1, -> > + .ops = &clk_alpha_pll_hwfsm_ops, +> > +static struct clk_alpha_pll perfcl_alt_pll =3D { +> > + .offset =3D 0x80100, +> > + .regs =3D alt_pll_regs, +> > + .vco_table =3D alt_pll_vco_modes, +> > + .num_vco =3D ARRAY_SIZE(alt_pll_vco_modes), +> > + .flags =3D SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE, +> > + .clkr.hw.init =3D &(struct clk_init_data) { +> > + .name =3D "perfcl_alt_pll", +> > + .parent_names =3D (const char *[]){ "xo" }, +> > + .num_parents =3D 1, +> > + .ops =3D &clk_alpha_pll_hwfsm_ops, > > + }, > > +}; > > + -> > +static struct clk_alpha_pll pwrcl_alt_pll = { -> > + .offset = 0x100, -> > + .regs = alt_pll_regs, -> > + .vco_table = alt_pll_vco_modes, -> > + .num_vco = ARRAY_SIZE(alt_pll_vco_modes), -> > + .flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE, -> > + .clkr.hw.init = &(struct clk_init_data) { -> > + .name = "pwrcl_alt_pll", -> > + .parent_names = (const char *[]){ "xo" }, -> > + .num_parents = 1, -> > + .ops = &clk_alpha_pll_hwfsm_ops, +> > +static struct clk_alpha_pll pwrcl_alt_pll =3D { +> > + .offset =3D 0x100, +> > + .regs =3D alt_pll_regs, +> > + .vco_table =3D alt_pll_vco_modes, +> > + .num_vco =3D ARRAY_SIZE(alt_pll_vco_modes), +> > + .flags =3D SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE, +> > + .clkr.hw.init =3D &(struct clk_init_data) { +> > + .name =3D "pwrcl_alt_pll", +> > + .parent_names =3D (const char *[]){ "xo" }, +> > + .num_parents =3D 1, +> > + .ops =3D &clk_alpha_pll_hwfsm_ops, > > + }, > > +}; > > + @@ -275,13 +289,13 @@ > > +struct clk_cpu_8996_mux { > > + u32 reg; > > + u32 shift; -> +>=20 > u8 shift? -> +>=20 > > + u32 width; -> +>=20 > u8 width? -> +>=20 > > + struct clk_hw *pll; > > + struct clk_regmap clkr; > > +}; @@ -294,33 +308,37 @@ > > + > > +static u8 clk_cpu_8996_mux_get_parent(struct clk_hw *hw) { > > + unsigned int val; -> > + struct clk_regmap *clkr = to_clk_regmap(hw); -> > + struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw); -> > + unsigned int mask = GENMASK(cpuclk->width - 1, 0); +> > + struct clk_regmap *clkr =3D to_clk_regmap(hw); +> > + struct clk_cpu_8996_mux *cpuclk =3D = +to_clk_cpu_8996_mux_hw(hw); +> > + unsigned int mask =3D GENMASK(cpuclk->width - 1, 0); > > + > > + regmap_read(clkr->regmap, cpuclk->reg, &val); > > + -> > + val >>= cpuclk->shift; -> > + val &= mask; +> > + val >>=3D cpuclk->shift; +> > + val &=3D mask; > > + > > + return val; -> +>=20 > return val & mask; -> +>=20 > > +} > > + -> > +static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index) { +> > +static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index) = +{ > > + unsigned int val; -> +>=20 > u32 val? -> -> > + struct clk_regmap *clkr = to_clk_regmap(hw); -> > + struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw); -> > + unsigned int mask = GENMASK(cpuclk->width + cpuclk->shift - 1, +>=20 +> > + struct clk_regmap *clkr =3D to_clk_regmap(hw); +> > + struct clk_cpu_8996_mux *cpuclk =3D = +to_clk_cpu_8996_mux_hw(hw); +> > + unsigned int mask =3D GENMASK(cpuclk->width + cpuclk->shift = +- 1, > > + cpuclk->shift); > > + -> > + val = index; -> > + val <<= cpuclk->shift; +> > + val =3D index; +> > + val <<=3D cpuclk->shift; > > + > > + return regmap_update_bits(clkr->regmap, cpuclk->reg, mask, > > +val); } @@ -328,95 +346,97 @@ > > +static int > > +clk_cpu_8996_mux_determine_rate(struct clk_hw *hw, struct > > +clk_rate_request *req) { -> > + struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw); -> > + struct clk_hw *parent = cpuclk->pll; +> > + struct clk_cpu_8996_mux *cpuclk =3D = +to_clk_cpu_8996_mux_hw(hw); +> > + struct clk_hw *parent =3D cpuclk->pll; > > + > > + if (!cpuclk->pll) > > + return -EINVAL; -> +>=20 > Does this happen? -> +>=20 > > + -> > + req->best_parent_rate = clk_hw_round_rate(parent, req->rate); -> > + req->best_parent_hw = parent; -> +> > + req->best_parent_rate =3D clk_hw_round_rate(parent, = +req->rate); +> > + req->best_parent_hw =3D parent; +>=20 > Is the parent index always the same? Perhaps just call > clk_hw_get_parent_by_index() then instead of adding a pointer to the > clk_cpu_8996_mux. -> +>=20 > > + > > + return 0; > > +} > > + -> > +const struct clk_ops clk_cpu_8996_mux_ops = { -> > + .set_parent = clk_cpu_8996_mux_set_parent, -> > + .get_parent = clk_cpu_8996_mux_get_parent, -> > + .determine_rate = clk_cpu_8996_mux_determine_rate, }; +> > +const struct clk_ops clk_cpu_8996_mux_ops =3D { +> > + .set_parent =3D clk_cpu_8996_mux_set_parent, +> > + .get_parent =3D clk_cpu_8996_mux_get_parent, +> > + .determine_rate =3D clk_cpu_8996_mux_determine_rate, }; > [...] > > + -> > +static struct clk_cpu_8996_mux pwrcl_pmux = { -> > + .reg = 0x40, -> > + .shift = 0, -> > + .width = 2, -> > + .pll = &pwrcl_pll.clkr.hw, -> > + .clkr.hw.init = &(struct clk_init_data) { -> > + .name = "pwrcl_pmux", -> > + .parent_names = (const char *[]){ +> > +static struct clk_cpu_8996_mux pwrcl_pmux =3D { +> > + .reg =3D 0x40, +> > + .shift =3D 0, +> > + .width =3D 2, +> > + .pll =3D &pwrcl_pll.clkr.hw, +> > + .clkr.hw.init =3D &(struct clk_init_data) { +> > + .name =3D "pwrcl_pmux", +> > + .parent_names =3D (const char *[]){ > > + "pwrcl_smux", > > + "pwrcl_pll", > > + "pwrcl_pll_acd", > > + "pwrcl_alt_pll", > > + }, -> > + .num_parents = 4, -> > + .ops = &clk_cpu_8996_mux_ops, -> > + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, +> > + .num_parents =3D 4, +> > + .ops =3D &clk_cpu_8996_mux_ops, +> > + .flags =3D CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, > > + }, > > +}; > > + -> > +static struct clk_cpu_8996_mux perfcl_pmux = { -> > + .reg = 0x80040, -> > + .shift = 0, -> > + .width = 2, -> > + .pll = &perfcl_pll.clkr.hw, -> > + .clkr.hw.init = &(struct clk_init_data) { -> > + .name = "perfcl_pmux", -> > + .parent_names = (const char *[]){ +> > +static struct clk_cpu_8996_mux perfcl_pmux =3D { +> > + .reg =3D 0x80040, +> > + .shift =3D 0, +> > + .width =3D 2, +> > + .pll =3D &perfcl_pll.clkr.hw, +> > + .clkr.hw.init =3D &(struct clk_init_data) { +> > + .name =3D "perfcl_pmux", +> > + .parent_names =3D (const char *[]){ > > + "perfcl_smux", > > + "perfcl_pll", > > + "perfcl_pll_acd", > > + "perfcl_alt_pll", > > + }, -> > + .num_parents = 4, -> > + .ops = &clk_cpu_8996_mux_ops, -> > + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, -> +> > + .num_parents =3D 4, +> > + .ops =3D &clk_cpu_8996_mux_ops, +> > + .flags =3D CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, +>=20 > Not sure CLK_IS_CRITICAL is the right mode. Perhaps CLK_IGNORE_UNUSED > for now? We don't want to force XO to stay on forever here. -> +>=20 > > + }, > > +}; > > + -> > +static const struct regmap_config cpu_msm8996_regmap_config = { -> > + .reg_bits = 32, -> > + .reg_stride = 4, -> > + .val_bits = 32, -> > + .max_register = 0x80210, -> > + .fast_io = true, -> > + .val_format_endian = REGMAP_ENDIAN_LITTLE, +> > +static const struct regmap_config cpu_msm8996_regmap_config =3D { +> > + .reg_bits =3D 32, +> > + .reg_stride =3D 4, +> > + .val_bits =3D 32, +> > + .max_register =3D 0x80210, +> > + .fast_io =3D true, +> > + .val_format_endian =3D REGMAP_ENDIAN_LITTLE, > > +}; > > + -> > +static const struct of_device_id match_table[] = { -> +> > +static const struct of_device_id match_table[] =3D { +>=20 > Move this next to driver structure and give it a more specific name. -> -> > + { .compatible = "qcom-msm8996-apcc" }, -> +>=20 +> > + { .compatible =3D "qcom-msm8996-apcc" }, +>=20 > Bad compatible? Should be qcom,msm8996-apcc? -> +>=20 > > + {} > > +}; > > + -> > +struct clk_regmap *clks[] = { +> > +struct clk_regmap *clks[] =3D { > > + /* PLLs */ > > + &perfcl_pll.clkr, > > + &pwrcl_pll.clkr, @@ -427,19 +447,20 @@ > > + &pwrcl_smux.clkr, > > + &perfcl_pmux.clkr, > > + &pwrcl_pmux.clkr, -> +>=20 > Please drop useless comments inside this array. -> +>=20 > > +}; > > + > > +struct clk_hw_clks { > > + unsigned int num; > > + struct clk_hw *hws[]; > > +}; -> -> Please just NULL terminate the list of clk_hw pointers, or keep the size +>=20 +> Please just NULL terminate the list of clk_hw pointers, or keep the = +size > around instead. -> +>=20 > > + > > +static int > > +qcom_cpu_clk_msm8996_register_clks(struct device *dev, struct @@ -447,35 +468,42 @@ > > + struct regmap *regmap) { > > + int i, ret; > > + -> > + hws->hws[0] = clk_hw_register_fixed_factor(dev, "perfcl_pll_main", +> > + hws->hws[0] =3D clk_hw_register_fixed_factor(dev, = +"perfcl_pll_main", > > + "perfcl_pll", -> > + CLK_SET_RATE_PARENT, 1, 2); -> > + perfcl_smux.pll = hws->hws[0]; +> > + = +CLK_SET_RATE_PARENT, 1, 2); +> > + perfcl_smux.pll =3D hws->hws[0]; > > + -> > + hws->hws[1] = clk_hw_register_fixed_factor(dev, "pwrcl_pll_main", +> > + hws->hws[1] =3D clk_hw_register_fixed_factor(dev, = +"pwrcl_pll_main", > > + "pwrcl_pll", -> > + CLK_SET_RATE_PARENT, 1, 2); -> > + pwrcl_smux.pll = hws->hws[1]; +> > + = +CLK_SET_RATE_PARENT, 1, 2); +> > + pwrcl_smux.pll =3D hws->hws[1]; > > + -> > + hws->num = 2; -> +> > + hws->num =3D 2; +>=20 > Maybe just open code the fixed factor clk registration? Then the -> devm_clk_hw_register() function can be used on those static structs to make +> devm_clk_hw_register() function can be used on those static structs to = +make > removal simpler. -I will have to change the clk_hw_unregister_fixed_rate() as well then, right? This will be an API change. +I will have to change the clk_hw_unregister_fixed_rate() as well then, = +right? This will be an API change. -> +>=20 > > + -> > + for (i = 0; i < ARRAY_SIZE(clks); i++) { -> > + ret = devm_clk_register_regmap(dev, clks[i]); +> > + for (i =3D 0; i < ARRAY_SIZE(clks); i++) { +> > + ret =3D devm_clk_register_regmap(dev, clks[i]); > > + if (ret) > > + return ret; > > + } > > + > > + clk_alpha_pll_configure(&perfcl_pll, regmap, &hfpll_config); > > + clk_alpha_pll_configure(&pwrcl_pll, regmap, &hfpll_config); -> > + clk_alpha_pll_configure(&perfcl_alt_pll, regmap, &altpll_config); +> > + clk_alpha_pll_configure(&perfcl_alt_pll, regmap, = +&altpll_config); > > + clk_alpha_pll_configure(&pwrcl_alt_pll, regmap, > > + &altpll_config); > > + @@ -488,42 +516,46 @@ I will have to change the clk_hw_unregister_fixed_rate() as well then, right? Th > > + void __iomem *base; > > + struct resource *res; > > + struct regmap *regmap_cpu; -> +>=20 > Just call it regmap please. -> +>=20 > > + struct clk_hw_clks *hws; > > + struct clk_hw_onecell_data *data; -> > + struct device *dev = &pdev->dev; -> > + struct device_node *node = dev->of_node; +> > + struct device *dev =3D &pdev->dev; +> > + struct device_node *node =3D dev->of_node; > > + -> > + data = devm_kzalloc(dev, sizeof(*data) + 2 * sizeof(struct clk_hw *), +> > + data =3D devm_kzalloc(dev, sizeof(*data) + 2 * sizeof(struct = +clk_hw *), > > + GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > + -> > + hws = devm_kzalloc(dev, sizeof(*hws) + 2 * sizeof(struct clk_hw *), +> > + hws =3D devm_kzalloc(dev, sizeof(*hws) + 2 * sizeof(struct = +clk_hw *), > > + GFP_KERNEL); > > + if (!hws) > > + return -ENOMEM; > > + -> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); -> > + base = devm_ioremap_resource(dev, res); +> > + res =3D platform_get_resource(pdev, IORESOURCE_MEM, 0); +> > + base =3D devm_ioremap_resource(dev, res); > > + if (IS_ERR(base)) > > + return PTR_ERR(base); > > + -> > + regmap_cpu = devm_regmap_init_mmio(dev, base, -> > + &cpu_msm8996_regmap_config); +> > + regmap_cpu =3D devm_regmap_init_mmio(dev, base, +> > + = +&cpu_msm8996_regmap_config); > > + if (IS_ERR(regmap_cpu)) > > + return PTR_ERR(regmap_cpu); > > + -> > + ret = qcom_cpu_clk_msm8996_register_clks(dev, hws, regmap_cpu); +> > + ret =3D qcom_cpu_clk_msm8996_register_clks(dev, hws, = +regmap_cpu); > > + if (ret) > > + return ret; > > + -> > + data->hws[0] = &pwrcl_pmux.clkr.hw; -> > + data->hws[1] = &perfcl_pmux.clkr.hw; +> > + data->hws[0] =3D &pwrcl_pmux.clkr.hw; +> > + data->hws[1] =3D &perfcl_pmux.clkr.hw; > > + -> > + data->num = 2; +> > + data->num =3D 2; > > + > > + platform_set_drvdata(pdev, hws); > > + @@ -534,26 +566,26 @@ I will have to change the clk_hw_unregister_fixed_rate() as well then, right? Th > platform_device > > +*pdev) { > > + int i; -> > + struct device *dev = &pdev->dev; -> > + struct clk_hw_clks *hws = platform_get_drvdata(pdev); +> > + struct device *dev =3D &pdev->dev; +> > + struct clk_hw_clks *hws =3D platform_get_drvdata(pdev); > > + -> > + for (i = 0; i < hws->num; i++) +> > + for (i =3D 0; i < hws->num; i++) > > + clk_hw_unregister_fixed_rate(hws->hws[i]); > > + > > + of_clk_del_provider(dev->of_node); -> +>=20 > Use devm_of_clk_add_hw_provider() instead. -> +>=20 > > + > > + return 0; > > +} > > + -> > +static struct platform_driver qcom_cpu_clk_msm8996_driver = { -> > + .probe = qcom_cpu_clk_msm8996_driver_probe, -> > + .remove = qcom_cpu_clk_msm8996_driver_remove, -> > + .driver = { -> > + .name = "qcom-msm8996-apcc", -> > + .of_match_table = match_table, +> > +static struct platform_driver qcom_cpu_clk_msm8996_driver =3D { +> > + .probe =3D qcom_cpu_clk_msm8996_driver_probe, +> > + .remove =3D qcom_cpu_clk_msm8996_driver_remove, +> > + .driver =3D { +> > + .name =3D "qcom-msm8996-apcc", +> > + .of_match_table =3D match_table, > > + }, > > +}; > > + @@ -561,5 +593,5 @@ I will have to change the clk_hw_unregister_fixed_rate() as well then, right? Th > > + > > +MODULE_ALIAS("platform:msm8996-apcc"); > > +MODULE_DESCRIPTION("QCOM MSM8996 CPU clock Driver"); -> +>=20 > Not sure why Driver is capitalized and clock is not. diff --git a/a/content_digest b/N1/content_digest index cb444a9..41d59f8 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -5,19 +5,19 @@ "Subject\0RE: [PATCH v3 03/10] clk: qcom: Add CPU clock driver for msm8996\0" "Date\0Thu, 22 Mar 2018 12:47:30 +0200\0" "To\0'Stephen Boyd' <sboyd@kernel.org>" - linux-arm-kernel@lists.infradead.org - linux-arm-msm@vger.kernel.org - linux-clk@vger.kernel.org - " sboyd@codeaurora.org\0" - "Cc\0mark.rutland@arm.com" - devicetree@vger.kernel.org - rnayak@codeaurora.org - robh@kernel.org - will.deacon@arm.com - amit.kucheria@linaro.org - tfinkel@codeaurora.org - nicolas.dechesne@linaro.org - " celster@codeaurora.org\0" + <linux-arm-kernel@lists.infradead.org> + <linux-arm-msm@vger.kernel.org> + <linux-clk@vger.kernel.org> + " <sboyd@codeaurora.org>\0" + "Cc\0<mark.rutland@arm.com>" + <devicetree@vger.kernel.org> + <rnayak@codeaurora.org> + <robh@kernel.org> + <will.deacon@arm.com> + <amit.kucheria@linaro.org> + <tfinkel@codeaurora.org> + <nicolas.dechesne@linaro.org> + " <celster@codeaurora.org>\0" "\00:1\0" "b\0" "\n" @@ -25,19 +25,23 @@ "> -----Original Message-----\n" "> From: Stephen Boyd <sboyd@kernel.org>\n" "> Sent: Monday, March 19, 2018 19:37\n" - "> To: Ilia Lin <ilialin@codeaurora.org>; linux-arm-kernel@lists.infradead.org;\n" + "> To: Ilia Lin <ilialin@codeaurora.org>; =\n" + "linux-arm-kernel@lists.infradead.org;\n" "> linux-arm-msm@vger.kernel.org; linux-clk@vger.kernel.org;\n" "> sboyd@codeaurora.org\n" "> Cc: mark.rutland@arm.com; devicetree@vger.kernel.org;\n" "> rnayak@codeaurora.org; robh@kernel.org; will.deacon@arm.com;\n" - "> amit.kucheria@linaro.org; tfinkel@codeaurora.org; ilialin@codeaurora.org;\n" + "> amit.kucheria@linaro.org; tfinkel@codeaurora.org; =\n" + "ilialin@codeaurora.org;\n" "> nicolas.dechesne@linaro.org; celster@codeaurora.org\n" - "> Subject: Re: [PATCH v3 03/10] clk: qcom: Add CPU clock driver for msm8996\n" - "> \n" + "> Subject: Re: [PATCH v3 03/10] clk: qcom: Add CPU clock driver for =\n" + "msm8996\n" + ">=20\n" "> Quoting Ilia Lin (2018-02-14 05:59:45)\n" "> > From: Rajendra Nayak <rnayak@codeaurora.org>\n" "> >\n" - "> > Each of the CPU clusters (Power and Perf) on msm8996 are clocked via 2\n" + "> > Each of the CPU clusters (Power and Perf) on msm8996 are clocked via =\n" + "2\n" "> > PLLs, a primary and alternate. There are also\n" "> > 2 Mux'es, a primary and secondary all connected together as shown\n" "> > below\n" @@ -62,9 +66,10 @@ "> > |Alt PLL | |\n" "> > | +---------------------------+\n" "> > +---------------+ PLL_EARLY\n" - "> \n" - "> Thanks for the diagram. Please also put it at the top of the driver file.\n" - "> \n" + ">=20\n" + "> Thanks for the diagram. Please also put it at the top of the driver =\n" + "file.\n" + ">=20\n" "> >\n" "> > The primary PLL is what drives the CPU clk, except for times when we\n" "> > are reprogramming the PLL itself (for rate changes) when we\n" @@ -75,17 +80,21 @@ "> > The primary PLL operates on a single VCO range, between 600Mhz and\n" "> > 3Ghz. However the CPUs do support OPPs with frequencies between\n" "> 300Mhz\n" - "> > and 600Mhz. In order to support running the CPUs at those frequencies\n" + "> > and 600Mhz. In order to support running the CPUs at those =\n" + "frequencies\n" "> > we end up having to lock the PLL at twice the rate and drive the CPU\n" "> > clk via the PLL/2 output and SMUX.\n" "> >\n" - "> > So for frequencies above 600Mhz we follow the following path Primary\n" - "> > PLL --> PLL_EARLY --> PMUX(1) --> CPU clk and for frequencies between\n" + "> > So for frequencies above 600Mhz we follow the following path =\n" + "Primary\n" + "> > PLL --> PLL_EARLY --> PMUX(1) --> CPU clk and for frequencies =\n" + "between\n" "> > 300Mhz and 600Mhz we follow\n" - "> \n" + ">=20\n" "> Capitalize 'h' in units please.\n" - "> \n" - "> > Primary PLL --> PLL/2 --> SMUX(1) --> PMUX(0) --> CPU clk Support for\n" + ">=20\n" + "> > Primary PLL --> PLL/2 --> SMUX(1) --> PMUX(0) --> CPU clk Support =\n" + "for\n" "> > this is added in a subsequent patch as well.\n" "> >\n" "> > ACD stands for Adaptive Clock Distribution and is used to detect\n" @@ -102,13 +111,16 @@ "> > 3 files changed, 418 insertions(+)\n" "> > create mode 100644 drivers/clk/qcom/clk-cpu-8996.c\n" "> >\n" - "> > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig index\n" + "> > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig =\n" + "index\n" "> > fbf4532..3274877 100644\n" "> > --- a/drivers/clk/qcom/Kconfig\n" "> > +++ b/drivers/clk/qcom/Kconfig\n" "> > @@ -226,3 +226,11 @@ config SPMI_PMIC_CLKDIV\n" - "> > Technologies, Inc. SPMI PMIC. It configures the frequency of\n" - "> > clkdiv outputs of the PMIC. These clocks are typically wired\n" + "> > Technologies, Inc. SPMI PMIC. It configures the frequency =\n" + "of\n" + "> > clkdiv outputs of the PMIC. These clocks are typically =\n" + "wired\n" "> > through alternate functions on GPIO pins.\n" "> > +\n" "> > +config MSM_APCC_8996\n" @@ -116,26 +128,27 @@ "> > + depends on COMMON_CLK_QCOM\n" "> > + help\n" "> > + Support for the CPU clock controller on msm8996 devices.\n" - "> > + Say Y if you want to support CPU clock scaling using CPUfreq\n" + "> > + Say Y if you want to support CPU clock scaling using =\n" + "CPUfreq\n" "> > + drivers for dyanmic power management.\n" - "> \n" + ">=20\n" "> Sort?\n" - "> \n" + ">=20\n" "> > diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile\n" "> > index 230332c..57b38ba 100644\n" "> > --- a/drivers/clk/qcom/Makefile\n" "> > +++ b/drivers/clk/qcom/Makefile\n" - "> > @@ -33,6 +33,7 @@ obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o\n" - "> > obj-$(CONFIG_MSM_MMCC_8960) += mmcc-msm8960.o\n" - "> > obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o\n" - "> > obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o\n" - "> > +obj-$(CONFIG_MSM_APCC_8996) += clk-cpu-8996.o\n" - "> \n" + "> > @@ -33,6 +33,7 @@ obj-$(CONFIG_MSM_LCC_8960) +=3D lcc-msm8960.o\n" + "> > obj-$(CONFIG_MSM_MMCC_8960) +=3D mmcc-msm8960.o\n" + "> > obj-$(CONFIG_MSM_MMCC_8974) +=3D mmcc-msm8974.o\n" + "> > obj-$(CONFIG_MSM_MMCC_8996) +=3D mmcc-msm8996.o\n" + "> > +obj-$(CONFIG_MSM_APCC_8996) +=3D clk-cpu-8996.o\n" + ">=20\n" "> This doesn't look sorted.\n" - "> \n" - "> > obj-$(CONFIG_QCOM_A53PLL) += a53-pll.o\n" - "> > obj-$(CONFIG_QCOM_CLK_APCS_MSM8916) += apcs-msm8916.o\n" - "> > obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o diff --git\n" + ">=20\n" + "> > obj-$(CONFIG_QCOM_A53PLL) +=3D a53-pll.o\n" + "> > obj-$(CONFIG_QCOM_CLK_APCS_MSM8916) +=3D apcs-msm8916.o\n" + "> > obj-$(CONFIG_QCOM_CLK_RPM) +=3D clk-rpm.o diff --git\n" "> > a/drivers/clk/qcom/clk-cpu-8996.c b/drivers/clk/qcom/clk-cpu-8996.c\n" "> > new file mode 100644 index 0000000..42489f1\n" "> > --- /dev/null\n" @@ -146,7 +159,8 @@ "> > + *\n" "> > + * This program is free software; you can redistribute it and/or\n" "> > +modify\n" - "> > + * it under the terms of the GNU General Public License version 2 and\n" + "> > + * it under the terms of the GNU General Public License version 2 =\n" + "and\n" "> > + * only version 2 as published by the Free Software Foundation.\n" "> > + *\n" "> > + * This program is distributed in the hope that it will be useful,\n" @@ -154,141 +168,141 @@ "> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n" "> > + * GNU General Public License for more details.\n" "> > + */\n" - "> \n" + ">=20\n" "> Can we get the SPDX tags here please?\n" - "> \n" + ">=20\n" "> > +\n" "> > +#include <linux/clk.h>\n" - "> \n" + ">=20\n" "> clk-provider.h?\n" - "> \n" + ">=20\n" "> > +#include <linux/module.h>\n" "> > +#include <linux/platform_device.h>\n" "> > +#include <linux/regmap.h>\n" "> > +\n" "> > +#include \"clk-alpha-pll.h\"\n" - "> \n" + ">=20\n" "> #include \"clk-regmap.h\"\n" - "> \n" + ">=20\n" "> > +\n" "> > +#define VCO(a, b, c) { \\\n" - "> > + .val = a,\\\n" - "> > + .min_freq = b,\\\n" - "> > + .max_freq = c,\\\n" + "> > + .val =3D a,\\\n" + "> > + .min_freq =3D b,\\\n" + "> > + .max_freq =3D c,\\\n" "> > +}\n" - "> \n" + ">=20\n" "> Can this define go into whatever PLL header file defines the struct?\n" - "> \n" + ">=20\n" "> > +\n" "> > +#define DIV_2_INDEX 0\n" "> > +#define PLL_INDEX 1\n" "> > +#define ACD_INDEX 2\n" "> > +#define ALT_INDEX 3\n" "> > +\n" - "> > +static const u8 prim_pll_regs[PLL_OFF_MAX_REGS] = {\n" - "> > + [PLL_OFF_L_VAL] = 0x04,\n" - "> > + [PLL_OFF_ALPHA_VAL] = 0x08,\n" - "> > + [PLL_OFF_USER_CTL] = 0x10,\n" - "> > + [PLL_OFF_CONFIG_CTL] = 0x18,\n" - "> > + [PLL_OFF_CONFIG_CTL_U] = 0x1C,\n" - "> \n" + "> > +static const u8 prim_pll_regs[PLL_OFF_MAX_REGS] =3D {\n" + "> > + [PLL_OFF_L_VAL] =3D 0x04,\n" + "> > + [PLL_OFF_ALPHA_VAL] =3D 0x08,\n" + "> > + [PLL_OFF_USER_CTL] =3D 0x10,\n" + "> > + [PLL_OFF_CONFIG_CTL] =3D 0x18,\n" + "> > + [PLL_OFF_CONFIG_CTL_U] =3D 0x1C,\n" + ">=20\n" "> Please use lowercase hex throughout. Consistency is key!\n" - "> \n" - "> > + [PLL_OFF_TEST_CTL] = 0x20,\n" - "> > + [PLL_OFF_TEST_CTL_U] = 0x24,\n" - "> > + [PLL_OFF_STATUS] = 0x28,\n" + ">=20\n" + "> > + [PLL_OFF_TEST_CTL] =3D 0x20,\n" + "> > + [PLL_OFF_TEST_CTL_U] =3D 0x24,\n" + "> > + [PLL_OFF_STATUS] =3D 0x28,\n" "> > +};\n" "> > +\n" - "> > +static const u8 alt_pll_regs[PLL_OFF_MAX_REGS] = {\n" - "> > + [PLL_OFF_L_VAL] = 0x04,\n" - "> > + [PLL_OFF_ALPHA_VAL] = 0x08,\n" - "> > + [PLL_OFF_ALPHA_VAL_U] = 0x0c,\n" - "> > + [PLL_OFF_USER_CTL] = 0x10,\n" - "> > + [PLL_OFF_USER_CTL_U] = 0x14,\n" - "> > + [PLL_OFF_CONFIG_CTL] = 0x18,\n" - "> > + [PLL_OFF_TEST_CTL] = 0x20,\n" - "> > + [PLL_OFF_TEST_CTL_U] = 0x24,\n" - "> > + [PLL_OFF_STATUS] = 0x28,\n" + "> > +static const u8 alt_pll_regs[PLL_OFF_MAX_REGS] =3D {\n" + "> > + [PLL_OFF_L_VAL] =3D 0x04,\n" + "> > + [PLL_OFF_ALPHA_VAL] =3D 0x08,\n" + "> > + [PLL_OFF_ALPHA_VAL_U] =3D 0x0c,\n" + "> > + [PLL_OFF_USER_CTL] =3D 0x10,\n" + "> > + [PLL_OFF_USER_CTL_U] =3D 0x14,\n" + "> > + [PLL_OFF_CONFIG_CTL] =3D 0x18,\n" + "> > + [PLL_OFF_TEST_CTL] =3D 0x20,\n" + "> > + [PLL_OFF_TEST_CTL_U] =3D 0x24,\n" + "> > + [PLL_OFF_STATUS] =3D 0x28,\n" "> > +};\n" "> > +\n" "> > +/* PLLs */\n" "> > +\n" - "> > +static const struct alpha_pll_config hfpll_config = {\n" - "> > + .l = 60,\n" - "> > + .config_ctl_val = 0x200d4828,\n" - "> > + .config_ctl_hi_val = 0x006,\n" - "> > + .pre_div_mask = BIT(12),\n" - "> > + .post_div_mask = 0x3 << 8,\n" - "> > + .main_output_mask = BIT(0),\n" - "> > + .early_output_mask = BIT(3),\n" + "> > +static const struct alpha_pll_config hfpll_config =3D {\n" + "> > + .l =3D 60,\n" + "> > + .config_ctl_val =3D 0x200d4828,\n" + "> > + .config_ctl_hi_val =3D 0x006,\n" + "> > + .pre_div_mask =3D BIT(12),\n" + "> > + .post_div_mask =3D 0x3 << 8,\n" + "> > + .main_output_mask =3D BIT(0),\n" + "> > + .early_output_mask =3D BIT(3),\n" "> > +};\n" "> > +\n" - "> > +static struct clk_alpha_pll perfcl_pll = {\n" - "> > + .offset = 0x80000,\n" - "> > + .regs = prim_pll_regs,\n" - "> > + .flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,\n" - "> > + .clkr.hw.init = &(struct clk_init_data){\n" - "> > + .name = \"perfcl_pll\",\n" - "> > + .parent_names = (const char *[]){ \"xo\" },\n" - "> > + .num_parents = 1,\n" - "> > + .ops = &clk_alpha_pll_huayra_ops,\n" + "> > +static struct clk_alpha_pll perfcl_pll =3D {\n" + "> > + .offset =3D 0x80000,\n" + "> > + .regs =3D prim_pll_regs,\n" + "> > + .flags =3D SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,\n" + "> > + .clkr.hw.init =3D &(struct clk_init_data){\n" + "> > + .name =3D \"perfcl_pll\",\n" + "> > + .parent_names =3D (const char *[]){ \"xo\" },\n" + "> > + .num_parents =3D 1,\n" + "> > + .ops =3D &clk_alpha_pll_huayra_ops,\n" "> > + },\n" "> > +};\n" "> > +\n" - "> > +static struct clk_alpha_pll pwrcl_pll = {\n" - "> > + .offset = 0x0,\n" - "> > + .regs = prim_pll_regs,\n" - "> > + .flags = SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,\n" - "> > + .clkr.hw.init = &(struct clk_init_data){\n" - "> > + .name = \"pwrcl_pll\",\n" - "> > + .parent_names = (const char *[]){ \"xo\" },\n" - "> > + .num_parents = 1,\n" - "> > + .ops = &clk_alpha_pll_huayra_ops,\n" + "> > +static struct clk_alpha_pll pwrcl_pll =3D {\n" + "> > + .offset =3D 0x0,\n" + "> > + .regs =3D prim_pll_regs,\n" + "> > + .flags =3D SUPPORTS_DYNAMIC_UPDATE | SUPPORTS_FSM_MODE,\n" + "> > + .clkr.hw.init =3D &(struct clk_init_data){\n" + "> > + .name =3D \"pwrcl_pll\",\n" + "> > + .parent_names =3D (const char *[]){ \"xo\" },\n" + "> > + .num_parents =3D 1,\n" + "> > + .ops =3D &clk_alpha_pll_huayra_ops,\n" "> > + },\n" "> > +};\n" "> > +\n" - "> > +static const struct pll_vco alt_pll_vco_modes[] = {\n" + "> > +static const struct pll_vco alt_pll_vco_modes[] =3D {\n" "> > + VCO(3, 250000000, 500000000),\n" "> > + VCO(2, 500000000, 750000000),\n" "> > + VCO(1, 750000000, 1000000000),\n" "> > + VCO(0, 1000000000, 2150400000), };\n" "> > +\n" - "> > +static const struct alpha_pll_config altpll_config = {\n" - "> > + .l = 16,\n" - "> > + .vco_val = 0x3 << 20,\n" - "> > + .vco_mask = 0x3 << 20,\n" - "> > + .config_ctl_val = 0x4001051b,\n" - "> > + .post_div_mask = 0x3 << 8,\n" - "> > + .post_div_val = 0x1,\n" - "> > + .main_output_mask = BIT(0),\n" - "> > + .early_output_mask = BIT(3),\n" + "> > +static const struct alpha_pll_config altpll_config =3D {\n" + "> > + .l =3D 16,\n" + "> > + .vco_val =3D 0x3 << 20,\n" + "> > + .vco_mask =3D 0x3 << 20,\n" + "> > + .config_ctl_val =3D 0x4001051b,\n" + "> > + .post_div_mask =3D 0x3 << 8,\n" + "> > + .post_div_val =3D 0x1,\n" + "> > + .main_output_mask =3D BIT(0),\n" + "> > + .early_output_mask =3D BIT(3),\n" "> > +};\n" "> > +\n" - "> > +static struct clk_alpha_pll perfcl_alt_pll = {\n" - "> > + .offset = 0x80100,\n" - "> > + .regs = alt_pll_regs,\n" - "> > + .vco_table = alt_pll_vco_modes,\n" - "> > + .num_vco = ARRAY_SIZE(alt_pll_vco_modes),\n" - "> > + .flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,\n" - "> > + .clkr.hw.init = &(struct clk_init_data) {\n" - "> > + .name = \"perfcl_alt_pll\",\n" - "> > + .parent_names = (const char *[]){ \"xo\" },\n" - "> > + .num_parents = 1,\n" - "> > + .ops = &clk_alpha_pll_hwfsm_ops,\n" + "> > +static struct clk_alpha_pll perfcl_alt_pll =3D {\n" + "> > + .offset =3D 0x80100,\n" + "> > + .regs =3D alt_pll_regs,\n" + "> > + .vco_table =3D alt_pll_vco_modes,\n" + "> > + .num_vco =3D ARRAY_SIZE(alt_pll_vco_modes),\n" + "> > + .flags =3D SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,\n" + "> > + .clkr.hw.init =3D &(struct clk_init_data) {\n" + "> > + .name =3D \"perfcl_alt_pll\",\n" + "> > + .parent_names =3D (const char *[]){ \"xo\" },\n" + "> > + .num_parents =3D 1,\n" + "> > + .ops =3D &clk_alpha_pll_hwfsm_ops,\n" "> > + },\n" "> > +};\n" "> > +\n" - "> > +static struct clk_alpha_pll pwrcl_alt_pll = {\n" - "> > + .offset = 0x100,\n" - "> > + .regs = alt_pll_regs,\n" - "> > + .vco_table = alt_pll_vco_modes,\n" - "> > + .num_vco = ARRAY_SIZE(alt_pll_vco_modes),\n" - "> > + .flags = SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,\n" - "> > + .clkr.hw.init = &(struct clk_init_data) {\n" - "> > + .name = \"pwrcl_alt_pll\",\n" - "> > + .parent_names = (const char *[]){ \"xo\" },\n" - "> > + .num_parents = 1,\n" - "> > + .ops = &clk_alpha_pll_hwfsm_ops,\n" + "> > +static struct clk_alpha_pll pwrcl_alt_pll =3D {\n" + "> > + .offset =3D 0x100,\n" + "> > + .regs =3D alt_pll_regs,\n" + "> > + .vco_table =3D alt_pll_vco_modes,\n" + "> > + .num_vco =3D ARRAY_SIZE(alt_pll_vco_modes),\n" + "> > + .flags =3D SUPPORTS_OFFLINE_REQ | SUPPORTS_FSM_MODE,\n" + "> > + .clkr.hw.init =3D &(struct clk_init_data) {\n" + "> > + .name =3D \"pwrcl_alt_pll\",\n" + "> > + .parent_names =3D (const char *[]){ \"xo\" },\n" + "> > + .num_parents =3D 1,\n" + "> > + .ops =3D &clk_alpha_pll_hwfsm_ops,\n" "> > + },\n" "> > +};\n" "> > +\n" @@ -297,13 +311,13 @@ "> > +struct clk_cpu_8996_mux {\n" "> > + u32 reg;\n" "> > + u32 shift;\n" - "> \n" + ">=20\n" "> u8 shift?\n" - "> \n" + ">=20\n" "> > + u32 width;\n" - "> \n" + ">=20\n" "> u8 width?\n" - "> \n" + ">=20\n" "> > + struct clk_hw *pll;\n" "> > + struct clk_regmap clkr;\n" "> > +};\n" @@ -316,33 +330,37 @@ "> > +\n" "> > +static u8 clk_cpu_8996_mux_get_parent(struct clk_hw *hw) {\n" "> > + unsigned int val;\n" - "> > + struct clk_regmap *clkr = to_clk_regmap(hw);\n" - "> > + struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);\n" - "> > + unsigned int mask = GENMASK(cpuclk->width - 1, 0);\n" + "> > + struct clk_regmap *clkr =3D to_clk_regmap(hw);\n" + "> > + struct clk_cpu_8996_mux *cpuclk =3D =\n" + "to_clk_cpu_8996_mux_hw(hw);\n" + "> > + unsigned int mask =3D GENMASK(cpuclk->width - 1, 0);\n" "> > +\n" "> > + regmap_read(clkr->regmap, cpuclk->reg, &val);\n" "> > +\n" - "> > + val >>= cpuclk->shift;\n" - "> > + val &= mask;\n" + "> > + val >>=3D cpuclk->shift;\n" + "> > + val &=3D mask;\n" "> > +\n" "> > + return val;\n" - "> \n" + ">=20\n" "> return val & mask;\n" - "> \n" + ">=20\n" "> > +}\n" "> > +\n" - "> > +static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index) {\n" + "> > +static int clk_cpu_8996_mux_set_parent(struct clk_hw *hw, u8 index) =\n" + "{\n" "> > + unsigned int val;\n" - "> \n" + ">=20\n" "> u32 val?\n" - "> \n" - "> > + struct clk_regmap *clkr = to_clk_regmap(hw);\n" - "> > + struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);\n" - "> > + unsigned int mask = GENMASK(cpuclk->width + cpuclk->shift - 1,\n" + ">=20\n" + "> > + struct clk_regmap *clkr =3D to_clk_regmap(hw);\n" + "> > + struct clk_cpu_8996_mux *cpuclk =3D =\n" + "to_clk_cpu_8996_mux_hw(hw);\n" + "> > + unsigned int mask =3D GENMASK(cpuclk->width + cpuclk->shift =\n" + "- 1,\n" "> > + cpuclk->shift);\n" "> > +\n" - "> > + val = index;\n" - "> > + val <<= cpuclk->shift;\n" + "> > + val =3D index;\n" + "> > + val <<=3D cpuclk->shift;\n" "> > +\n" "> > + return regmap_update_bits(clkr->regmap, cpuclk->reg, mask,\n" "> > +val); }\n" @@ -350,95 +368,97 @@ "> > +static int\n" "> > +clk_cpu_8996_mux_determine_rate(struct clk_hw *hw, struct\n" "> > +clk_rate_request *req) {\n" - "> > + struct clk_cpu_8996_mux *cpuclk = to_clk_cpu_8996_mux_hw(hw);\n" - "> > + struct clk_hw *parent = cpuclk->pll;\n" + "> > + struct clk_cpu_8996_mux *cpuclk =3D =\n" + "to_clk_cpu_8996_mux_hw(hw);\n" + "> > + struct clk_hw *parent =3D cpuclk->pll;\n" "> > +\n" "> > + if (!cpuclk->pll)\n" "> > + return -EINVAL;\n" - "> \n" + ">=20\n" "> Does this happen?\n" - "> \n" + ">=20\n" "> > +\n" - "> > + req->best_parent_rate = clk_hw_round_rate(parent, req->rate);\n" - "> > + req->best_parent_hw = parent;\n" - "> \n" + "> > + req->best_parent_rate =3D clk_hw_round_rate(parent, =\n" + "req->rate);\n" + "> > + req->best_parent_hw =3D parent;\n" + ">=20\n" "> Is the parent index always the same? Perhaps just call\n" "> clk_hw_get_parent_by_index() then instead of adding a pointer to the\n" "> clk_cpu_8996_mux.\n" - "> \n" + ">=20\n" "> > +\n" "> > + return 0;\n" "> > +}\n" "> > +\n" - "> > +const struct clk_ops clk_cpu_8996_mux_ops = {\n" - "> > + .set_parent = clk_cpu_8996_mux_set_parent,\n" - "> > + .get_parent = clk_cpu_8996_mux_get_parent,\n" - "> > + .determine_rate = clk_cpu_8996_mux_determine_rate, };\n" + "> > +const struct clk_ops clk_cpu_8996_mux_ops =3D {\n" + "> > + .set_parent =3D clk_cpu_8996_mux_set_parent,\n" + "> > + .get_parent =3D clk_cpu_8996_mux_get_parent,\n" + "> > + .determine_rate =3D clk_cpu_8996_mux_determine_rate, };\n" "> [...]\n" "> > +\n" - "> > +static struct clk_cpu_8996_mux pwrcl_pmux = {\n" - "> > + .reg = 0x40,\n" - "> > + .shift = 0,\n" - "> > + .width = 2,\n" - "> > + .pll = &pwrcl_pll.clkr.hw,\n" - "> > + .clkr.hw.init = &(struct clk_init_data) {\n" - "> > + .name = \"pwrcl_pmux\",\n" - "> > + .parent_names = (const char *[]){\n" + "> > +static struct clk_cpu_8996_mux pwrcl_pmux =3D {\n" + "> > + .reg =3D 0x40,\n" + "> > + .shift =3D 0,\n" + "> > + .width =3D 2,\n" + "> > + .pll =3D &pwrcl_pll.clkr.hw,\n" + "> > + .clkr.hw.init =3D &(struct clk_init_data) {\n" + "> > + .name =3D \"pwrcl_pmux\",\n" + "> > + .parent_names =3D (const char *[]){\n" "> > + \"pwrcl_smux\",\n" "> > + \"pwrcl_pll\",\n" "> > + \"pwrcl_pll_acd\",\n" "> > + \"pwrcl_alt_pll\",\n" "> > + },\n" - "> > + .num_parents = 4,\n" - "> > + .ops = &clk_cpu_8996_mux_ops,\n" - "> > + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,\n" + "> > + .num_parents =3D 4,\n" + "> > + .ops =3D &clk_cpu_8996_mux_ops,\n" + "> > + .flags =3D CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,\n" "> > + },\n" "> > +};\n" "> > +\n" - "> > +static struct clk_cpu_8996_mux perfcl_pmux = {\n" - "> > + .reg = 0x80040,\n" - "> > + .shift = 0,\n" - "> > + .width = 2,\n" - "> > + .pll = &perfcl_pll.clkr.hw,\n" - "> > + .clkr.hw.init = &(struct clk_init_data) {\n" - "> > + .name = \"perfcl_pmux\",\n" - "> > + .parent_names = (const char *[]){\n" + "> > +static struct clk_cpu_8996_mux perfcl_pmux =3D {\n" + "> > + .reg =3D 0x80040,\n" + "> > + .shift =3D 0,\n" + "> > + .width =3D 2,\n" + "> > + .pll =3D &perfcl_pll.clkr.hw,\n" + "> > + .clkr.hw.init =3D &(struct clk_init_data) {\n" + "> > + .name =3D \"perfcl_pmux\",\n" + "> > + .parent_names =3D (const char *[]){\n" "> > + \"perfcl_smux\",\n" "> > + \"perfcl_pll\",\n" "> > + \"perfcl_pll_acd\",\n" "> > + \"perfcl_alt_pll\",\n" "> > + },\n" - "> > + .num_parents = 4,\n" - "> > + .ops = &clk_cpu_8996_mux_ops,\n" - "> > + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,\n" - "> \n" + "> > + .num_parents =3D 4,\n" + "> > + .ops =3D &clk_cpu_8996_mux_ops,\n" + "> > + .flags =3D CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,\n" + ">=20\n" "> Not sure CLK_IS_CRITICAL is the right mode. Perhaps CLK_IGNORE_UNUSED\n" "> for now? We don't want to force XO to stay on forever here.\n" - "> \n" + ">=20\n" "> > + },\n" "> > +};\n" "> > +\n" - "> > +static const struct regmap_config cpu_msm8996_regmap_config = {\n" - "> > + .reg_bits = 32,\n" - "> > + .reg_stride = 4,\n" - "> > + .val_bits = 32,\n" - "> > + .max_register = 0x80210,\n" - "> > + .fast_io = true,\n" - "> > + .val_format_endian = REGMAP_ENDIAN_LITTLE,\n" + "> > +static const struct regmap_config cpu_msm8996_regmap_config =3D {\n" + "> > + .reg_bits =3D 32,\n" + "> > + .reg_stride =3D 4,\n" + "> > + .val_bits =3D 32,\n" + "> > + .max_register =3D 0x80210,\n" + "> > + .fast_io =3D true,\n" + "> > + .val_format_endian =3D REGMAP_ENDIAN_LITTLE,\n" "> > +};\n" "> > +\n" - "> > +static const struct of_device_id match_table[] = {\n" - "> \n" + "> > +static const struct of_device_id match_table[] =3D {\n" + ">=20\n" "> Move this next to driver structure and give it a more specific name.\n" - "> \n" - "> > + { .compatible = \"qcom-msm8996-apcc\" },\n" - "> \n" + ">=20\n" + "> > + { .compatible =3D \"qcom-msm8996-apcc\" },\n" + ">=20\n" "> Bad compatible? Should be qcom,msm8996-apcc?\n" - "> \n" + ">=20\n" "> > + {}\n" "> > +};\n" "> > +\n" - "> > +struct clk_regmap *clks[] = {\n" + "> > +struct clk_regmap *clks[] =3D {\n" "> > + /* PLLs */\n" "> > + &perfcl_pll.clkr,\n" "> > + &pwrcl_pll.clkr,\n" @@ -449,19 +469,20 @@ "> > + &pwrcl_smux.clkr,\n" "> > + &perfcl_pmux.clkr,\n" "> > + &pwrcl_pmux.clkr,\n" - "> \n" + ">=20\n" "> Please drop useless comments inside this array.\n" - "> \n" + ">=20\n" "> > +};\n" "> > +\n" "> > +struct clk_hw_clks {\n" "> > + unsigned int num;\n" "> > + struct clk_hw *hws[];\n" "> > +};\n" - "> \n" - "> Please just NULL terminate the list of clk_hw pointers, or keep the size\n" + ">=20\n" + "> Please just NULL terminate the list of clk_hw pointers, or keep the =\n" + "size\n" "> around instead.\n" - "> \n" + ">=20\n" "> > +\n" "> > +static int\n" "> > +qcom_cpu_clk_msm8996_register_clks(struct device *dev, struct\n" @@ -469,35 +490,42 @@ "> > + struct regmap *regmap) {\n" "> > + int i, ret;\n" "> > +\n" - "> > + hws->hws[0] = clk_hw_register_fixed_factor(dev, \"perfcl_pll_main\",\n" + "> > + hws->hws[0] =3D clk_hw_register_fixed_factor(dev, =\n" + "\"perfcl_pll_main\",\n" "> > + \"perfcl_pll\",\n" - "> > + CLK_SET_RATE_PARENT, 1, 2);\n" - "> > + perfcl_smux.pll = hws->hws[0];\n" + "> > + =\n" + "CLK_SET_RATE_PARENT, 1, 2);\n" + "> > + perfcl_smux.pll =3D hws->hws[0];\n" "> > +\n" - "> > + hws->hws[1] = clk_hw_register_fixed_factor(dev, \"pwrcl_pll_main\",\n" + "> > + hws->hws[1] =3D clk_hw_register_fixed_factor(dev, =\n" + "\"pwrcl_pll_main\",\n" "> > + \"pwrcl_pll\",\n" - "> > + CLK_SET_RATE_PARENT, 1, 2);\n" - "> > + pwrcl_smux.pll = hws->hws[1];\n" + "> > + =\n" + "CLK_SET_RATE_PARENT, 1, 2);\n" + "> > + pwrcl_smux.pll =3D hws->hws[1];\n" "> > +\n" - "> > + hws->num = 2;\n" - "> \n" + "> > + hws->num =3D 2;\n" + ">=20\n" "> Maybe just open code the fixed factor clk registration? Then the\n" - "> devm_clk_hw_register() function can be used on those static structs to make\n" + "> devm_clk_hw_register() function can be used on those static structs to =\n" + "make\n" "> removal simpler.\n" "\n" - "I will have to change the clk_hw_unregister_fixed_rate() as well then, right? This will be an API change.\n" + "I will have to change the clk_hw_unregister_fixed_rate() as well then, =\n" + "right? This will be an API change.\n" "\n" - "> \n" + ">=20\n" "> > +\n" - "> > + for (i = 0; i < ARRAY_SIZE(clks); i++) {\n" - "> > + ret = devm_clk_register_regmap(dev, clks[i]);\n" + "> > + for (i =3D 0; i < ARRAY_SIZE(clks); i++) {\n" + "> > + ret =3D devm_clk_register_regmap(dev, clks[i]);\n" "> > + if (ret)\n" "> > + return ret;\n" "> > + }\n" "> > +\n" "> > + clk_alpha_pll_configure(&perfcl_pll, regmap, &hfpll_config);\n" "> > + clk_alpha_pll_configure(&pwrcl_pll, regmap, &hfpll_config);\n" - "> > + clk_alpha_pll_configure(&perfcl_alt_pll, regmap, &altpll_config);\n" + "> > + clk_alpha_pll_configure(&perfcl_alt_pll, regmap, =\n" + "&altpll_config);\n" "> > + clk_alpha_pll_configure(&pwrcl_alt_pll, regmap,\n" "> > + &altpll_config);\n" "> > +\n" @@ -510,42 +538,46 @@ "> > + void __iomem *base;\n" "> > + struct resource *res;\n" "> > + struct regmap *regmap_cpu;\n" - "> \n" + ">=20\n" "> Just call it regmap please.\n" - "> \n" + ">=20\n" "> > + struct clk_hw_clks *hws;\n" "> > + struct clk_hw_onecell_data *data;\n" - "> > + struct device *dev = &pdev->dev;\n" - "> > + struct device_node *node = dev->of_node;\n" + "> > + struct device *dev =3D &pdev->dev;\n" + "> > + struct device_node *node =3D dev->of_node;\n" "> > +\n" - "> > + data = devm_kzalloc(dev, sizeof(*data) + 2 * sizeof(struct clk_hw *),\n" + "> > + data =3D devm_kzalloc(dev, sizeof(*data) + 2 * sizeof(struct =\n" + "clk_hw *),\n" "> > + GFP_KERNEL);\n" "> > + if (!data)\n" "> > + return -ENOMEM;\n" "> > +\n" - "> > + hws = devm_kzalloc(dev, sizeof(*hws) + 2 * sizeof(struct clk_hw *),\n" + "> > + hws =3D devm_kzalloc(dev, sizeof(*hws) + 2 * sizeof(struct =\n" + "clk_hw *),\n" "> > + GFP_KERNEL);\n" "> > + if (!hws)\n" "> > + return -ENOMEM;\n" "> > +\n" - "> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);\n" - "> > + base = devm_ioremap_resource(dev, res);\n" + "> > + res =3D platform_get_resource(pdev, IORESOURCE_MEM, 0);\n" + "> > + base =3D devm_ioremap_resource(dev, res);\n" "> > + if (IS_ERR(base))\n" "> > + return PTR_ERR(base);\n" "> > +\n" - "> > + regmap_cpu = devm_regmap_init_mmio(dev, base,\n" - "> > + &cpu_msm8996_regmap_config);\n" + "> > + regmap_cpu =3D devm_regmap_init_mmio(dev, base,\n" + "> > + =\n" + "&cpu_msm8996_regmap_config);\n" "> > + if (IS_ERR(regmap_cpu))\n" "> > + return PTR_ERR(regmap_cpu);\n" "> > +\n" - "> > + ret = qcom_cpu_clk_msm8996_register_clks(dev, hws, regmap_cpu);\n" + "> > + ret =3D qcom_cpu_clk_msm8996_register_clks(dev, hws, =\n" + "regmap_cpu);\n" "> > + if (ret)\n" "> > + return ret;\n" "> > +\n" - "> > + data->hws[0] = &pwrcl_pmux.clkr.hw;\n" - "> > + data->hws[1] = &perfcl_pmux.clkr.hw;\n" + "> > + data->hws[0] =3D &pwrcl_pmux.clkr.hw;\n" + "> > + data->hws[1] =3D &perfcl_pmux.clkr.hw;\n" "> > +\n" - "> > + data->num = 2;\n" + "> > + data->num =3D 2;\n" "> > +\n" "> > + platform_set_drvdata(pdev, hws);\n" "> > +\n" @@ -556,26 +588,26 @@ "> platform_device\n" "> > +*pdev) {\n" "> > + int i;\n" - "> > + struct device *dev = &pdev->dev;\n" - "> > + struct clk_hw_clks *hws = platform_get_drvdata(pdev);\n" + "> > + struct device *dev =3D &pdev->dev;\n" + "> > + struct clk_hw_clks *hws =3D platform_get_drvdata(pdev);\n" "> > +\n" - "> > + for (i = 0; i < hws->num; i++)\n" + "> > + for (i =3D 0; i < hws->num; i++)\n" "> > + clk_hw_unregister_fixed_rate(hws->hws[i]);\n" "> > +\n" "> > + of_clk_del_provider(dev->of_node);\n" - "> \n" + ">=20\n" "> Use devm_of_clk_add_hw_provider() instead.\n" - "> \n" + ">=20\n" "> > +\n" "> > + return 0;\n" "> > +}\n" "> > +\n" - "> > +static struct platform_driver qcom_cpu_clk_msm8996_driver = {\n" - "> > + .probe = qcom_cpu_clk_msm8996_driver_probe,\n" - "> > + .remove = qcom_cpu_clk_msm8996_driver_remove,\n" - "> > + .driver = {\n" - "> > + .name = \"qcom-msm8996-apcc\",\n" - "> > + .of_match_table = match_table,\n" + "> > +static struct platform_driver qcom_cpu_clk_msm8996_driver =3D {\n" + "> > + .probe =3D qcom_cpu_clk_msm8996_driver_probe,\n" + "> > + .remove =3D qcom_cpu_clk_msm8996_driver_remove,\n" + "> > + .driver =3D {\n" + "> > + .name =3D \"qcom-msm8996-apcc\",\n" + "> > + .of_match_table =3D match_table,\n" "> > + },\n" "> > +};\n" "> > +\n" @@ -583,7 +615,7 @@ "> > +\n" "> > +MODULE_ALIAS(\"platform:msm8996-apcc\");\n" "> > +MODULE_DESCRIPTION(\"QCOM MSM8996 CPU clock Driver\");\n" - "> \n" + ">=20\n" > Not sure why Driver is capitalized and clock is not. -fbea92b67120b6b54a0a50178fcd0de931a33e7387b84cc3321e59465efa736f +2be76aa80f9ef6b0209bdbd78bde9ff7a8dce0e327cbc49eceff2f3eec78f6b4
diff --git a/a/1.txt b/N2/1.txt index a6e618d..ecac4b9 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -3,13 +3,13 @@ > -----Original Message----- > From: Stephen Boyd <sboyd@kernel.org> > Sent: Monday, March 19, 2018 19:37 -> To: Ilia Lin <ilialin@codeaurora.org>; linux-arm-kernel@lists.infradead.org; -> linux-arm-msm@vger.kernel.org; linux-clk@vger.kernel.org; -> sboyd@codeaurora.org -> Cc: mark.rutland@arm.com; devicetree@vger.kernel.org; -> rnayak@codeaurora.org; robh@kernel.org; will.deacon@arm.com; -> amit.kucheria@linaro.org; tfinkel@codeaurora.org; ilialin@codeaurora.org; -> nicolas.dechesne@linaro.org; celster@codeaurora.org +> To: Ilia Lin <ilialin@codeaurora.org>; linux-arm-kernel at lists.infradead.org; +> linux-arm-msm at vger.kernel.org; linux-clk at vger.kernel.org; +> sboyd at codeaurora.org +> Cc: mark.rutland at arm.com; devicetree at vger.kernel.org; +> rnayak at codeaurora.org; robh at kernel.org; will.deacon at arm.com; +> amit.kucheria at linaro.org; tfinkel at codeaurora.org; ilialin at codeaurora.org; +> nicolas.dechesne at linaro.org; celster at codeaurora.org > Subject: Re: [PATCH v3 03/10] clk: qcom: Add CPU clock driver for msm8996 > > Quoting Ilia Lin (2018-02-14 05:59:45) diff --git a/a/content_digest b/N2/content_digest index cb444a9..144b1ce 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -1,23 +1,10 @@ "ref\01518616792-29028-1-git-send-email-ilialin@codeaurora.org\0" "ref\01518616792-29028-4-git-send-email-ilialin@codeaurora.org\0" "ref\0152148100496.242365.3846617702949655708@swboyd.mtv.corp.google.com\0" - "From\0<ilialin@codeaurora.org>\0" - "Subject\0RE: [PATCH v3 03/10] clk: qcom: Add CPU clock driver for msm8996\0" + "From\0ilialin@codeaurora.org (ilialin at codeaurora.org)\0" + "Subject\0[PATCH v3 03/10] clk: qcom: Add CPU clock driver for msm8996\0" "Date\0Thu, 22 Mar 2018 12:47:30 +0200\0" - "To\0'Stephen Boyd' <sboyd@kernel.org>" - linux-arm-kernel@lists.infradead.org - linux-arm-msm@vger.kernel.org - linux-clk@vger.kernel.org - " sboyd@codeaurora.org\0" - "Cc\0mark.rutland@arm.com" - devicetree@vger.kernel.org - rnayak@codeaurora.org - robh@kernel.org - will.deacon@arm.com - amit.kucheria@linaro.org - tfinkel@codeaurora.org - nicolas.dechesne@linaro.org - " celster@codeaurora.org\0" + "To\0linux-arm-kernel@lists.infradead.org\0" "\00:1\0" "b\0" "\n" @@ -25,13 +12,13 @@ "> -----Original Message-----\n" "> From: Stephen Boyd <sboyd@kernel.org>\n" "> Sent: Monday, March 19, 2018 19:37\n" - "> To: Ilia Lin <ilialin@codeaurora.org>; linux-arm-kernel@lists.infradead.org;\n" - "> linux-arm-msm@vger.kernel.org; linux-clk@vger.kernel.org;\n" - "> sboyd@codeaurora.org\n" - "> Cc: mark.rutland@arm.com; devicetree@vger.kernel.org;\n" - "> rnayak@codeaurora.org; robh@kernel.org; will.deacon@arm.com;\n" - "> amit.kucheria@linaro.org; tfinkel@codeaurora.org; ilialin@codeaurora.org;\n" - "> nicolas.dechesne@linaro.org; celster@codeaurora.org\n" + "> To: Ilia Lin <ilialin@codeaurora.org>; linux-arm-kernel at lists.infradead.org;\n" + "> linux-arm-msm at vger.kernel.org; linux-clk at vger.kernel.org;\n" + "> sboyd at codeaurora.org\n" + "> Cc: mark.rutland at arm.com; devicetree at vger.kernel.org;\n" + "> rnayak at codeaurora.org; robh at kernel.org; will.deacon at arm.com;\n" + "> amit.kucheria at linaro.org; tfinkel at codeaurora.org; ilialin at codeaurora.org;\n" + "> nicolas.dechesne at linaro.org; celster at codeaurora.org\n" "> Subject: Re: [PATCH v3 03/10] clk: qcom: Add CPU clock driver for msm8996\n" "> \n" "> Quoting Ilia Lin (2018-02-14 05:59:45)\n" @@ -586,4 +573,4 @@ "> \n" > Not sure why Driver is capitalized and clock is not. -fbea92b67120b6b54a0a50178fcd0de931a33e7387b84cc3321e59465efa736f +d3b3a8e859d98765689c565cf768e2f14571b3be093ec840f0666ad917db42ec
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.