linux-kernel - Re: [PATCH v4 2/2] clk: qcom: gcc-msm8939: Add MSM8939 Generic Clock Controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 14 May 2020 22:42:40 +0100
From:   Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To:     Stephen Boyd <sboyd@...nel.org>, agross@...nel.org,
        bjorn.andersson@...aro.org, mturquette@...libre.com,
        robh+dt@...nel.org
Cc:     linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        shawn.guo@...aro.org, p.zabel@...gutronix.de,
        vincent.knecht@...loo.org, konradybcio@...il.com
Subject: Re: [PATCH v4 2/2] clk: qcom: gcc-msm8939: Add MSM8939 Generic Clock
 Controller

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@...nel.org>
>> Cc: Bjorn Andersson <bjorn.andersson@...aro.org>
>> Cc: Michael Turquette <mturquette@...libre.com>
>> Cc: Stephen Boyd <sboyd@...nel.org>
>> Cc: Rob Herring <robh+dt@...nel.org>
>> Cc: Philipp Zabel <p.zabel@...gutronix.de>
>> Cc: linux-arm-msm@...r.kernel.org
>> Cc: linux-clk@...r.kernel.org
>> Cc: linux-kernel@...r.kernel.org
>> Cc: devicetree@...r.kernel.org
>> Signed-off-by: Shawn Guo <shawn.guo@...aro.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@...aro.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