| 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
| ||
|
Message-ID: <958877e3-3e14-4259-af7d-697c87b141ee@quicinc.com>
Date: Fri, 11 Apr 2025 12:43:39 +0530
From: Jagadeesh Kona <quic_jkona@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
Bryan O'Donoghue
<bryan.odonoghue@...aro.org>
CC: Bjorn Andersson <andersson@...nel.org>,
Michael Turquette
<mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>, Rob Herring
<robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>,
"Vladimir
Zapolskiy" <vladimir.zapolskiy@...aro.org>,
Dmitry Baryshkov
<lumag@...nel.org>,
Ajit Pandey <quic_ajipan@...cinc.com>,
Imran Shaik
<quic_imrashai@...cinc.com>,
Taniya Das <quic_tdas@...cinc.com>,
"Satya Priya
Kakitapalli" <quic_skakitap@...cinc.com>,
<linux-arm-msm@...r.kernel.org>, <linux-clk@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org>
Subject: Re: [PATCH v3 04/18] clk: qcom: clk-alpha-pll: Add support for common
PLL configuration function
On 3/27/2025 11:50 PM, Dmitry Baryshkov wrote:
> On Thu, Mar 27, 2025 at 03:51:33PM +0000, Bryan O'Donoghue wrote:
>> On 27/03/2025 09:52, Jagadeesh Kona wrote:
>>> From: Taniya Das <quic_tdas@...cinc.com>
>>>
>>> To properly configure the PLLs on recent chipsets, it often requires more
>>> than one power domain to be kept ON. The support to enable multiple power
>>> domains is being added in qcom_cc_really_probe() and PLLs should be
>>> configured post all the required power domains are enabled.
>>>
>>> Hence integrate PLL configuration into clk_alpha_pll structure and add
>>> support for qcom_clk_alpha_pll_configure() function which can be called
>>> from qcom_cc_really_probe() to configure the clock controller PLLs after
>>> all required power domains are enabled.
>>>
>>> Signed-off-by: Taniya Das <quic_tdas@...cinc.com>
>>> Signed-off-by: Jagadeesh Kona <quic_jkona@...cinc.com>
>>> ---
>>> drivers/clk/qcom/clk-alpha-pll.c | 63 ++++++++++++++++++++++++++++++++++++++++
>>> drivers/clk/qcom/clk-alpha-pll.h | 3 ++
>>> 2 files changed, 66 insertions(+)
>>>
>>> diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
>>> index cec0afea8e446010f0d4140d4ef63121706dde47..8ee842254e6690e24469053cdbd99a9953987e40 100644
>>> --- a/drivers/clk/qcom/clk-alpha-pll.c
>>> +++ b/drivers/clk/qcom/clk-alpha-pll.c
>>> @@ -63,6 +63,8 @@
>>> #define PLL_OPMODE(p) ((p)->offset + (p)->regs[PLL_OFF_OPMODE])
>>> #define PLL_FRAC(p) ((p)->offset + (p)->regs[PLL_OFF_FRAC])
>>> +#define GET_PLL_TYPE(pll) (((pll)->regs - clk_alpha_pll_regs[0]) / PLL_OFF_MAX_REGS)
>>> +
>>> const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = {
>>> [CLK_ALPHA_PLL_TYPE_DEFAULT] = {
>>> [PLL_OFF_L_VAL] = 0x04,
>>> @@ -2960,3 +2962,64 @@ const struct clk_ops clk_alpha_pll_regera_ops = {
>>> .set_rate = clk_zonda_pll_set_rate,
>>> };
>>> EXPORT_SYMBOL_GPL(clk_alpha_pll_regera_ops);
>>> +
>>> +void qcom_clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
>>> +{
>>> + const struct clk_init_data *init = pll->clkr.hw.init;
>>> + const char *name = init->name;
>>> +
>>> + if (!pll->config || !pll->regs) {
>>> + pr_err("%s: missing pll config or regs\n", name);
>>> + return;
>>> + }
>>
>> Seems like a strange check - you are calling this function in a loop which
>> looks like
>>
>> for (i = 0; i < desc->num_alpha_plls; i++)
>> qcom_clk_alpha_pll_configure(desc->alpha_plls[i], regmap);
>>
>> Can num_alpha_plls be true but alpha_plls be NULL and why is regmap
>> considered valid ?
>>
>> I think you can drop this check.
>
The regmap check is already performed in qcom_cc_map() before we reach this
point.
It is not possible for num_alpha_plls to be true when alpha_plls is NULL, as
num_alpha_plls is derived using SIZE_OF(alpha_plls).
Including the above check is beneficial to catch any errors in case we missed
adding the necessary config/regs fields to the PLL structure in the CC code.
> I think pll->config should be moved to a calling code.
>
Yes, I can move above checks to calling code in next series.
Thanks,
Jagadeesh
>>
>>> +
>>> + switch (GET_PLL_TYPE(pll)) {
>>> + case CLK_ALPHA_PLL_TYPE_LUCID_OLE:
>>> + clk_lucid_ole_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + case CLK_ALPHA_PLL_TYPE_LUCID_EVO:
>>> + clk_lucid_evo_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + case CLK_ALPHA_PLL_TYPE_TAYCAN_ELU:
>>> + clk_taycan_elu_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + case CLK_ALPHA_PLL_TYPE_RIVIAN_EVO:
>>> + clk_rivian_evo_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + case CLK_ALPHA_PLL_TYPE_TRION:
>>> + clk_trion_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + case CLK_ALPHA_PLL_TYPE_HUAYRA_2290:
>>> + clk_huayra_2290_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + case CLK_ALPHA_PLL_TYPE_FABIA:
>>> + clk_fabia_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + case CLK_ALPHA_PLL_TYPE_AGERA:
>>> + clk_agera_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + case CLK_ALPHA_PLL_TYPE_PONGO_ELU:
>>> + clk_pongo_elu_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + case CLK_ALPHA_PLL_TYPE_ZONDA:
>>> + case CLK_ALPHA_PLL_TYPE_ZONDA_OLE:
>>> + clk_zonda_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + case CLK_ALPHA_PLL_TYPE_STROMER:
>>> + case CLK_ALPHA_PLL_TYPE_STROMER_PLUS:
>>> + clk_stromer_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + case CLK_ALPHA_PLL_TYPE_DEFAULT:
>>> + case CLK_ALPHA_PLL_TYPE_DEFAULT_EVO:
>>> + case CLK_ALPHA_PLL_TYPE_HUAYRA:
>>> + case CLK_ALPHA_PLL_TYPE_HUAYRA_APSS:
>>> + case CLK_ALPHA_PLL_TYPE_BRAMMO:
>>> + case CLK_ALPHA_PLL_TYPE_BRAMMO_EVO:
>>> + clk_alpha_pll_configure(pll, regmap, pll->config);
>>> + break;
>>> + default:
>>> + WARN(1, "%s: invalid pll type\n", name);
>>> + break;
>>> + }
>>> +}
>>> +EXPORT_SYMBOL_GPL(qcom_clk_alpha_pll_configure);
>>> diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
>>> index 79aca8525262211ae5295245427d4540abf1e09a..7f35aaa7a35d88411beb11fd2be5d5dd5bfbe066 100644
>>> --- a/drivers/clk/qcom/clk-alpha-pll.h
>>> +++ b/drivers/clk/qcom/clk-alpha-pll.h
>>> @@ -81,6 +81,7 @@ struct pll_vco {
>>> * struct clk_alpha_pll - phase locked loop (PLL)
>>> * @offset: base address of registers
>>> * @regs: alpha pll register map (see @clk_alpha_pll_regs)
>>> + * @config: array of pll settings
>>> * @vco_table: array of VCO settings
>>> * @num_vco: number of VCO settings in @vco_table
>>> * @flags: bitmask to indicate features supported by the hardware
>>> @@ -90,6 +91,7 @@ struct clk_alpha_pll {
>>> u32 offset;
>>> const u8 *regs;
>>> + const struct alpha_pll_config *config;
>>> const struct pll_vco *vco_table;
>>> size_t num_vco;
>>> #define SUPPORTS_OFFLINE_REQ BIT(0)
>>> @@ -237,5 +239,6 @@ void clk_stromer_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
>>> const struct alpha_pll_config *config);
>>> void clk_regera_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
>>> const struct alpha_pll_config *config);
>>> +void qcom_clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap);
>>> #endif
>>>
>
Powered by blists - more mailing lists