From: Abel Vesa <abel.vesa@linaro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Andy Gross <agross@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Konrad Dybcio <konrad.dybcio@linaro.org>,
Mike Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-clk@vger.kernel.org
Subject: Re: [PATCH v4 8/9] clk: qcom: rpmh: Add support for SM8550 rpmh clocks
Date: Tue, 6 Dec 2022 12:42:14 +0200 [thread overview]
Message-ID: <Y48chhGMMRFv/6DS@linaro.org> (raw)
In-Reply-To: <CAA8EJppgpe_LK=2=qncHB_fpDz4AmUBzcRQ=8_kb0YrLCQNweg@mail.gmail.com>
On 22-11-30 13:45:19, Dmitry Baryshkov wrote:
> On Wed, 30 Nov 2022 at 13:30, Abel Vesa <abel.vesa@linaro.org> wrote:
> >
> > Adds the RPMH clocks present in SM8550 SoC.
> >
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > ---
> > drivers/clk/qcom/clk-rpmh.c | 110 +++++++++++++++++++++++++++++-------
> > 1 file changed, 90 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c
> > index 0471bab82464..3204df3fda86 100644
> > --- a/drivers/clk/qcom/clk-rpmh.c
> > +++ b/drivers/clk/qcom/clk-rpmh.c
> > @@ -132,6 +132,36 @@ static DEFINE_MUTEX(rpmh_clk_lock);
> > }, \
> > }
> >
> > +#define DEFINE_CLK_RPMH_FIXED(_platform, _name, _name_active, \
> > + _parent_name, _name_active_parent, \
> > + _div) \
> > + static struct clk_fixed_factor _platform##_##_name = { \
> > + .mult = 1, \
> > + .div = _div, \
> > + .hw.init = &(struct clk_init_data){ \
> > + .ops = &clk_fixed_factor_ops, \
> > + .name = #_name, \
> > + .parent_data = &(const struct clk_parent_data){ \
> > + .fw_name = #_parent_name, \
> > + .name = #_parent_name, \
> > + }, \
> > + .num_parents = 1, \
> > + }, \
> > + }; \
> > + static struct clk_fixed_factor _platform##_##_name_active = { \
> > + .mult = 1, \
> > + .div = _div, \
> > + .hw.init = &(struct clk_init_data){ \
> > + .ops = &clk_fixed_factor_ops, \
> > + .name = #_name_active, \
> > + .parent_data = &(const struct clk_parent_data){ \
> > + .fw_name = #_name_active_parent,\
> > + .name = #_name_active_parent, \
> > + }, \
> > + .num_parents = 1, \
> > + }, \
> > + }
> > +
>
> Please use CLK_FIXED_FACTOR_FW_NAME() instead.
>
> > static inline struct clk_rpmh *to_clk_rpmh(struct clk_hw *_hw)
> > {
> > return container_of(_hw, struct clk_rpmh, hw);
> > @@ -579,6 +609,43 @@ static const struct clk_rpmh_desc clk_rpmh_sm8450 = {
> > .num_clks = ARRAY_SIZE(sm8450_rpmh_clocks),
> > };
> >
> > +DEFINE_CLK_RPMH_ARC(sm8550, xo_pad, xo_pad_ao, "xo.lvl", 0x03, 2);
> > +DEFINE_CLK_RPMH_FIXED(sm8550, bi_tcxo, bi_tcxo_ao, xo_pad, xo_pad_ao, 2);
> > +DEFINE_CLK_RPMH_VRM(sm8550, rf_clk1, rf_clk1_ao, "clka1", 1);
> > +DEFINE_CLK_RPMH_VRM(sm8550, rf_clk2, rf_clk2_ao, "clka2", 1);
> > +DEFINE_CLK_RPMH_VRM(sm8550, rf_clk3, rf_clk3_ao, "clka3", 1);
> > +DEFINE_CLK_RPMH_VRM(sm8550, rf_clk4, rf_clk4_ao, "clka4", 1);
> > +DEFINE_CLK_RPMH_VRM(sm8550, ln_bb_clk1, ln_bb_clk1_ao, "clka6", 2);
> > +DEFINE_CLK_RPMH_VRM(sm8550, ln_bb_clk2, ln_bb_clk2_ao, "clka7", 2);
> > +DEFINE_CLK_RPMH_VRM(sm8550, ln_bb_clk3, ln_bb_clk3_ao, "clka8", 2);
> > +
> > +static struct clk_hw *sm8550_rpmh_clocks[] = {
> > + [RPMH_CXO_PAD_CLK] = &sm8550_xo_pad.hw,
> > + [RPMH_CXO_PAD_CLK_A] = &sm8550_xo_pad_ao.hw,
> > + [RPMH_CXO_CLK] = &sm8550_bi_tcxo.hw,
> > + [RPMH_CXO_CLK_A] = &sm8550_bi_tcxo_ao.hw,
> > + [RPMH_LN_BB_CLK1] = &sm8550_ln_bb_clk1.hw,
> > + [RPMH_LN_BB_CLK1_A] = &sm8550_ln_bb_clk1_ao.hw,
> > + [RPMH_LN_BB_CLK2] = &sm8550_ln_bb_clk2.hw,
> > + [RPMH_LN_BB_CLK2_A] = &sm8550_ln_bb_clk2_ao.hw,
> > + [RPMH_LN_BB_CLK3] = &sm8550_ln_bb_clk3.hw,
> > + [RPMH_LN_BB_CLK3_A] = &sm8550_ln_bb_clk3_ao.hw,
> > + [RPMH_RF_CLK1] = &sm8550_rf_clk1.hw,
> > + [RPMH_RF_CLK1_A] = &sm8550_rf_clk1_ao.hw,
> > + [RPMH_RF_CLK2] = &sm8550_rf_clk2.hw,
> > + [RPMH_RF_CLK2_A] = &sm8550_rf_clk2_ao.hw,
> > + [RPMH_RF_CLK3] = &sm8550_rf_clk3.hw,
> > + [RPMH_RF_CLK3_A] = &sm8550_rf_clk3_ao.hw,
> > + [RPMH_RF_CLK4] = &sm8550_rf_clk4.hw,
> > + [RPMH_RF_CLK4_A] = &sm8550_rf_clk4_ao.hw,
> > + [RPMH_IPA_CLK] = &sdm845_ipa.hw,
> > +};
> > +
> > +static const struct clk_rpmh_desc clk_rpmh_sm8550 = {
> > + .clks = sm8550_rpmh_clocks,
> > + .num_clks = ARRAY_SIZE(sm8550_rpmh_clocks),
> > +};
> > +
> > static struct clk_hw *sc7280_rpmh_clocks[] = {
> > [RPMH_CXO_CLK] = &sc7280_bi_tcxo.hw,
> > [RPMH_CXO_CLK_A] = &sc7280_bi_tcxo_ao.hw,
> > @@ -682,29 +749,31 @@ static int clk_rpmh_probe(struct platform_device *pdev)
> >
> > name = hw_clks[i]->init->name;
> >
> > - rpmh_clk = to_clk_rpmh(hw_clks[i]);
> > - res_addr = cmd_db_read_addr(rpmh_clk->res_name);
> > - if (!res_addr) {
> > - dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
> > - rpmh_clk->res_name);
> > - return -ENODEV;
> > - }
> > + if (hw_clks[i]->init->ops != &clk_fixed_factor_ops) {
>
> This looks ugly a bit. I'd rather add something like non_cmd_clks to
Right, but this complicates the xlate function without a really good
reason. Assuming you meant multiple (different) arrays for cmd
based and for non-cmd based (in this case fixed factor only), this would
force the of_clk_rpmh_hw_get to look into different arrays, but then we
only have one set of IDs. Wouldn't that make it even uglier? At least,
right now we are being consistent after probe.
> the clk_rpmh_desc data and register them separately. Or even worse,
> you can keep the rpmh as is, with only cmd-db based clocks and add a
> fixed factor clock to the sm8550 dtsi directly. I have a slight
> preference for the former method, but the latter would also work.
>
> > + rpmh_clk = to_clk_rpmh(hw_clks[i]);
> > + res_addr = cmd_db_read_addr(rpmh_clk->res_name);
> > + if (!res_addr) {
> > + dev_err(&pdev->dev, "missing RPMh resource address for %s\n",
> > + rpmh_clk->res_name);
> > + return -ENODEV;
> > + }
> >
> > - data = cmd_db_read_aux_data(rpmh_clk->res_name, &aux_data_len);
> > - if (IS_ERR(data)) {
> > - ret = PTR_ERR(data);
> > - dev_err(&pdev->dev,
> > - "error reading RPMh aux data for %s (%d)\n",
> > - rpmh_clk->res_name, ret);
> > - return ret;
> > - }
> > + data = cmd_db_read_aux_data(rpmh_clk->res_name, &aux_data_len);
> > + if (IS_ERR(data)) {
> > + ret = PTR_ERR(data);
> > + dev_err(&pdev->dev,
> > + "error reading RPMh aux data for %s (%d)\n",
> > + rpmh_clk->res_name, ret);
> > + return ret;
> > + }
> >
> > - /* Convert unit from Khz to Hz */
> > - if (aux_data_len == sizeof(*data))
> > - rpmh_clk->unit = le32_to_cpu(data->unit) * 1000ULL;
> > + /* Convert unit from Khz to Hz */
> > + if (aux_data_len == sizeof(*data))
> > + rpmh_clk->unit = le32_to_cpu(data->unit) * 1000ULL;
> >
> > - rpmh_clk->res_addr += res_addr;
> > - rpmh_clk->dev = &pdev->dev;
> > + rpmh_clk->res_addr += res_addr;
> > + rpmh_clk->dev = &pdev->dev;
> > + }
> >
> > ret = devm_clk_hw_register(&pdev->dev, hw_clks[i]);
> > if (ret) {
> > @@ -739,6 +808,7 @@ static const struct of_device_id clk_rpmh_match_table[] = {
> > { .compatible = "qcom,sm8250-rpmh-clk", .data = &clk_rpmh_sm8250},
> > { .compatible = "qcom,sm8350-rpmh-clk", .data = &clk_rpmh_sm8350},
> > { .compatible = "qcom,sm8450-rpmh-clk", .data = &clk_rpmh_sm8450},
> > + { .compatible = "qcom,sm8550-rpmh-clk", .data = &clk_rpmh_sm8550},
> > { .compatible = "qcom,sc7280-rpmh-clk", .data = &clk_rpmh_sc7280},
> > { }
> > };
> > --
> > 2.34.1
> >
>
>
> --
> With best wishes
> Dmitry
next prev parent reply other threads:[~2022-12-06 10:43 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-30 11:28 [PATCH v4 0/9] clk: qcom: Add support for SM8550 Abel Vesa
2022-11-30 11:28 ` [PATCH v4 1/9] dt-bindings: clock: Add SM8550 GCC clocks Abel Vesa
2022-11-30 11:28 ` [PATCH v4 2/9] dt-bindings: clock: Add SM8550 TCSR CC clocks Abel Vesa
2022-12-01 22:12 ` Bjorn Andersson
2022-12-02 19:19 ` Abel Vesa
2022-11-30 11:28 ` [PATCH v4 3/9] clk: qcom: gdsc: Increase status poll timeout Abel Vesa
2022-11-30 11:30 ` Konrad Dybcio
2022-11-30 11:28 ` [PATCH v4 4/9] clk: qcom: Add LUCID_OLE PLL type for SM8550 Abel Vesa
2022-11-30 11:28 ` [PATCH v4 5/9] clk: qcom: Add GCC driver " Abel Vesa
2022-11-30 11:28 ` [PATCH v4 6/9] dt-bindings: clock: Add RPMHCC " Abel Vesa
2022-11-30 11:28 ` [PATCH v4 7/9] dt-bindings: clock: qcom,rpmh: Add CXO PAD clock IDs Abel Vesa
2022-11-30 11:28 ` [PATCH v4 8/9] clk: qcom: rpmh: Add support for SM8550 rpmh clocks Abel Vesa
2022-11-30 11:45 ` Dmitry Baryshkov
2022-11-30 11:59 ` Dmitry Baryshkov
2022-12-06 10:52 ` Abel Vesa
2022-12-06 10:42 ` Abel Vesa [this message]
2022-11-30 11:28 ` [PATCH v4 9/9] clk: qcom: Add TCSR clock driver for SM8550 Abel Vesa
2022-12-02 20:58 ` (subset) [PATCH v4 0/9] clk: qcom: Add support " Bjorn Andersson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y48chhGMMRFv/6DS@linaro.org \
--to=abel.vesa@linaro.org \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.baryshkov@linaro.org \
--cc=konrad.dybcio@linaro.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=robh+dt@kernel.org \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).