On Fri 19 Jun 17:27 PDT 2020, Stephen Boyd wrote: > Quoting Loic Poulain (2020-06-04 03:35:25) > > 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 > > > > +-------+ > > XO | | > > +------------------>0 | > > | | > > PLL/2 | SMUX +----+ > > +------->1 | | > > | | | | > > | +-------+ | +-------+ > > | +---->0 | > > | | | > > +---------------+ | +----------->1 | CPU clk > > |Primary PLL +----+ PLL_EARLY | | +------> > > | +------+-----------+ +------>2 PMUX | > > +---------------+ | | | | > > | +------+ | +-->3 | > > +--^+ ACD +-----+ | +-------+ > > +---------------+ +------+ | > > |Alt PLL | | > > | +---------------------------+ > > +---------------+ PLL_EARLY > > > > The primary PLL is what drives the CPU clk, except for times > > when we are reprogramming the PLL itself (for rate changes) when > > we temporarily switch to an alternate PLL. A subsequent patch adds > > support to switch between primary and alternate PLL during rate > > changes. > > > > 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 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 300MHz and 600MHz we follow > > Primary PLL --> PLL/2 --> SMUX(1) --> PMUX(0) --> CPU clk > > > > ACD stands for Adaptive Clock Distribution and is used to > > detect voltage droops. > > > > Co-developed-by: Rajendra Nayak <rnayak@xxxxxxxxxxxxxx> > > Co-developed-by: Ilia Lin <ilialin@xxxxxxxxxxxxxx> > > Co-developed-by should be combined with Signed-off-by tags. > > From Documentation/process/submitting-patches.rst > > "Co-developed-by: states that the patch was co-created by multiple developers; > it is a used to give attribution to co-authors (in addition to the author > attributed by the From: tag) when several people work on a single patch. Since > Co-developed-by: denotes authorship, every Co-developed-by: must be immediately > followed by a Signed-off-by: of the associated co-author." > ...which is not what happened here. By using Co-developed-by and a Signed-off-by one would state that Rajendra certifies the origin of any additions done by Ilia and Loic. Instead, Rajendra certified the origin of the original patch (presumably) per 11.a by his s-o-b, then Ilia took the patch in whole or part and certified that he can contribute it per 11.a or 11.c with his s-o-b, then finally Loic should do the same, by adding his s-o-b. As such we should see three Signed-off-by here, preferably with [name: changelog] entries inbetween to document any modifications done to the patch. Regards, Bjorn > > Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxx> > > --- > > drivers/clk/qcom/Kconfig | 8 + > > drivers/clk/qcom/Makefile | 1 + > > drivers/clk/qcom/clk-alpha-pll.h | 6 + > > drivers/clk/qcom/clk-cpu-8996.c | 538 +++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 553 insertions(+) > > create mode 100644 drivers/clk/qcom/clk-cpu-8996.c > > > > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > > index abb121f..87b515d 100644 > > --- a/drivers/clk/qcom/Kconfig > > +++ b/drivers/clk/qcom/Kconfig > > @@ -37,6 +37,14 @@ config QCOM_CLK_APCS_MSM8916 > > Say Y if you want to support CPU frequency scaling on devices > > such as msm8916. > > > > +config QCOM_CLK_APCC_MSM8996 > > + tristate "MSM8996 CPU Clock Controller" > > + select QCOM_KRYO_L2_ACCESSORS > > This needs to depend on ARM || ARM64 because it uses the kryo accessors which > are architecture specific instructions. > > > + help > > + Support for the CPU clock controller on msm8996 devices. > > + Say Y if you want to support CPU clock scaling using CPUfreq > > + drivers for dyanmic power management. > > + > > config QCOM_CLK_RPM > > tristate "RPM based Clock Controller" > > depends on MFD_QCOM_RPM
