Messages in this thread | | | Subject | Re: [PATCH v4 2/2] clk: qcom: gcc-msm8939: Add MSM8939 Generic Clock Controller | From | Bryan O'Donoghue <> | Date | Thu, 14 May 2020 22:42:40 +0100 |
| |
On 14/05/2020 22:31, Stephen Boyd wrote: > Quoting Bryan O'Donoghue (2020-05-12 04:50:23) >> This patch adds support for the MSM8939 GCC. The MSM8939 is based on the >> MSM8916. MSM8939 is compatible in several ways with MSM8916 but, has >> additional functional blocks added which require additional PLL sources. In >> some cases functional blocks from the MSM8916 have different clock sources >> or different supported frequencies. >> >> Cc: Andy Gross <agross@kernel.org> >> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> >> Cc: Michael Turquette <mturquette@baylibre.com> >> Cc: Stephen Boyd <sboyd@kernel.org> >> Cc: Rob Herring <robh+dt@kernel.org> >> Cc: Philipp Zabel <p.zabel@pengutronix.de> >> Cc: linux-arm-msm@vger.kernel.org >> Cc: linux-clk@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Cc: devicetree@vger.kernel.org >> Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > Is this a co-developed-by tag?
Yep. I'm squashing down about 30-some internal patches to this one patch here including one or two from Shawn in this set.
I wasn't quite sure what the etiquette on Co-developed was i.e. it wasn't something git allowed me to specify with a "git commit -s --co-developed="xyz"" so I just retained the SOB.
Looking through git logs I see an example
I'll apply a Co-developed-by: Shawn Guo <shawn.guo@linaro.org>
for v5.
>> +static int gcc_msm8939_probe(struct platform_device *pdev) >> +{ >> + int ret; >> + struct regmap *regmap; >> + >> + ret = qcom_cc_probe(pdev, &gcc_msm8939_desc); >> + if (ret) >> + return ret; >> + >> + regmap = dev_get_regmap(&pdev->dev, NULL); >> + clk_pll_configure_sr_hpm_lp(&gpll3, regmap, &gpll3_config, true); >> + clk_pll_configure_sr_hpm_lp(&gpll4, regmap, &gpll4_config, true); > > We should probably configure these before registering the clks. Can you > do the usual, map registers, configure stuff, and then > qcom_cc_really_probe()?
I think so. If there was a good reason to configure the plls after the registration, I can't recall what that was, maybe the original flow from downstream ...
>> + >> +MODULE_DESCRIPTION("Qualcomm GCC MSM8939 Driver"); >> +MODULE_LICENSE("GPL v2"); >> +MODULE_ALIAS("platform:gcc-msm8939"); > > The module alias isn't needed right?
Nope g/msm8916/s//msm8939/g - I can zap that.
Thanks for the review.
--- bod
| |