[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ddcaa5e5-b5c5-4d78-b44a-4cea75ec6a77@linaro.org>
Date: Thu, 27 Mar 2025 15:51:33 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: 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>
Cc: 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 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.
> +
> + 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