[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <enbhxe2ewhguebg7hvjadzqajftfff6whd27smkdwfzz4hbvwm@go72ix5c4d6k>
Date: Thu, 13 Mar 2025 17:39:50 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Jagadeesh Kona <quic_jkona@...cinc.com>
Cc: 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>, 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
Subject: Re: [PATCH v2 2/8] clk: qcom: common: Add support to configure PLL
On Thu, Mar 06, 2025 at 02:25:34PM +0530, Jagadeesh Kona wrote:
> From: Taniya Das <quic_tdas@...cinc.com>
>
> Integrate PLL configuration into clk_alpha_pll structure and add support
> for qcom_cc_clk_alpha_pll_configure() function which can be used to
> configure the clock controller PLLs from common core code.
https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
starts with "Describe your problem."
I don't see a problem description here.
>
> Signed-off-by: Taniya Das <quic_tdas@...cinc.com>
> Signed-off-by: Jagadeesh Kona <quic_jkona@...cinc.com>
> ---
> drivers/clk/qcom/clk-alpha-pll.h | 2 ++
> drivers/clk/qcom/common.c | 55 ++++++++++++++++++++++++++++++++++++++++
> drivers/clk/qcom/common.h | 1 +
> 3 files changed, 58 insertions(+)
>
> diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
> index 79aca8525262211ae5295245427d4540abf1e09a..943320cdcd10a6c07fcd74dccb88be847dc086c2 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)
> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> index 9e3380fd718198c9fe63d7361615a91c3ecb3d60..74d062b5da0647f7f2bd8fd7a004ffdb1116c1ea 100644
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> @@ -13,6 +13,7 @@
> #include <linux/of.h>
>
> #include "common.h"
> +#include "clk-alpha-pll.h"
> #include "clk-rcg.h"
> #include "clk-regmap.h"
> #include "reset.h"
> @@ -284,6 +285,60 @@ static int qcom_cc_icc_register(struct device *dev,
> desc->num_icc_hws, icd);
> }
>
> +static void qcom_cc_clk_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap)
> +{
> + if (!pll->config || !pll->regs)
> + return;
> +
> + 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:
This would be annoying to hit when adding a new PLL type, a BUG(); would
be useful here.
> + break;
> + }
> +}
> +
> int qcom_cc_really_probe(struct device *dev,
> const struct qcom_cc_desc *desc, struct regmap *regmap)
> {
> diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
> index 7ace5d7f5836aa81431153ba92d8f14f2ffe8147..2066c8937936235d7bd03ab3225d4b3f4fb08dd0 100644
> --- a/drivers/clk/qcom/common.h
> +++ b/drivers/clk/qcom/common.h
> @@ -18,6 +18,7 @@ struct clk_hw;
> #define PLL_BIAS_COUNT_MASK 0x3f
> #define PLL_VOTE_FSM_ENA BIT(20)
> #define PLL_VOTE_FSM_RESET BIT(21)
> +#define GET_PLL_TYPE(pll) ((pll->regs - clk_alpha_pll_regs[0]) / PLL_OFF_MAX_REGS)
Why would this go in qcom/common.h, when clk_alpha_pll_regs is defined
in clk-alpha-pll.h?
Regards,
Bjorn
>
> struct qcom_icc_hws_data {
> int master_id;
>
> --
> 2.34.1
>
Powered by blists - more mailing lists