| 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: <zyfr7dd3dwfbcyztiaajr6tac7shn6ecrvy5277bfk2uwh3txp@gt4tbxh6xx54>
Date: Thu, 27 Mar 2025 20:20:49 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Cc: Jagadeesh Kona <quic_jkona@...cinc.com>,
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 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.
I think pll->config should be moved to a calling code.
>
> > +
> > + 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
> >
--
With best wishes
Dmitry
Powered by blists - more mailing lists