From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
Prasad Malisetty <quic_pmaliset@quicinc.com>
Cc: Andy Gross <agross@kernel.org>,
Stanimir Varbanov <svarbanov@mm-sol.com>,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Wilczy??ski <kw@linux.com>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <swboyd@chromium.org>,
Bjorn Helgaas <bhelgaas@google.com>,
Vinod Koul <vkoul@kernel.org>,
linux-arm-msm@vger.kernel.org, linux-pci@vger.kernel.org,
linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
Prasad Malisetty <pmaliset@codeaurora.org>
Subject: Re: [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC
Date: Thu, 10 Feb 2022 01:41:44 +0300 [thread overview]
Message-ID: <b7e0ad58-b837-7dcf-4386-ab7ff82ee65c@linaro.org> (raw)
In-Reply-To: <Yf2jRAf5UKYSMYxe@builder.lan>
On 05/02/2022 01:05, Bjorn Andersson wrote:
> On Fri 04 Feb 08:46 CST 2022, Dmitry Baryshkov wrote:
>
>> On newer Qualcomm platforms GCC_PCIE_n_PIPE_CLK_SRC should be controlled
>> together with the PCIE_n_GDSC. The clock should be fed from the TCXO
>> before switching the GDSC off and can be fed from PCIE_n_PIPE_CLK once
>> the GDSC is on.
>>
>> Since commit aa9c0df98c29 ("PCI: qcom: Switch pcie_1_pipe_clk_src after
>> PHY init in SC7280") PCIe controller driver tries to manage this on it's
>> own, resulting in the non-optimal code. Furthermore, if the any of the
>> drivers will have the same requirements, the code would have to be
>> dupliacted there.
>>
>> Move handling of such clocks to the GDSC code, providing special GDSC
>> type.
>>
>
> As discussed on IRC, I'm inclined not to take this, because looks to me
> to be the same situation that we have with all GDSCs in SM8350 and
> onwards - that some clocks must be parked on a safe parent before the
> associated GDSC can be toggled.
>
> Prasad, please advice on what the actual requirements are wrt the
> gcc_pipe_clk_src. When does it need to provide a valid signal and when
> does it need to be parked?
Prasad, any comments?
>
> Regards,
> Bjorn
>
>> Cc: Prasad Malisetty <pmaliset@codeaurora.org>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> drivers/clk/qcom/gdsc.c | 41 +++++++++++++++++++++++++++++++++++++++++
>> drivers/clk/qcom/gdsc.h | 14 ++++++++++++++
>> 2 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
>> index 7e1dd8ccfa38..9913d1b70947 100644
>> --- a/drivers/clk/qcom/gdsc.c
>> +++ b/drivers/clk/qcom/gdsc.c
>> @@ -45,6 +45,7 @@
>> #define TIMEOUT_US 500
>>
>> #define domain_to_gdsc(domain) container_of(domain, struct gdsc, pd)
>> +#define domain_to_pipe_clk_gdsc(domain) container_of(domain, struct pipe_clk_gdsc, base.pd)
>>
>> enum gdsc_status {
>> GDSC_OFF,
>> @@ -549,3 +550,43 @@ int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
>> +
>> +/*
>> + * Special operations for GDSCs with attached pipe clocks.
>> + * The clock should be parked to safe source (tcxo) before turning off the GDSC
>> + * and can be switched on as soon as the GDSC is on.
>> + *
>> + * We remove respective clock sources from clocks map and handle them manually.
>> + */
>> +int gdsc_pipe_enable(struct generic_pm_domain *domain)
>> +{
>> + struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
>> + int i, ret;
>> +
>> + ret = gdsc_enable(domain);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = 0; i< sc->num_clocks; i++)
>> + regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
>> + BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
>> + sc->clocks[i].on_value << sc->clocks[i].shift);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(gdsc_pipe_enable);
>> +
>> +int gdsc_pipe_disable(struct generic_pm_domain *domain)
>> +{
>> + struct pipe_clk_gdsc *sc = domain_to_pipe_clk_gdsc(domain);
>> + int i;
>> +
>> + for (i = sc->num_clocks - 1; i >= 0; i--)
>> + regmap_update_bits(sc->base.regmap, sc->clocks[i].reg,
>> + BIT(sc->clocks[i].shift + sc->clocks[i].width) - BIT(sc->clocks[i].shift),
>> + sc->clocks[i].off_value << sc->clocks[i].shift);
>> +
>> + /* In case of an error do not try turning the clocks again. We can not be sure about the GDSC state. */
>> + return gdsc_disable(domain);
>> +}
>> +EXPORT_SYMBOL_GPL(gdsc_pipe_disable);
>> diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
>> index d7cc4c21a9d4..b1a2f0abe41c 100644
>> --- a/drivers/clk/qcom/gdsc.h
>> +++ b/drivers/clk/qcom/gdsc.h
>> @@ -68,11 +68,25 @@ struct gdsc_desc {
>> size_t num;
>> };
>>
>> +struct pipe_clk_gdsc {
>> + struct gdsc base;
>> + int num_clocks;
>> + struct {
>> + u32 reg;
>> + u32 shift;
>> + u32 width;
>> + u32 off_value;
>> + u32 on_value;
>> + } clocks[];
>> +};
>> +
>> #ifdef CONFIG_QCOM_GDSC
>> int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
>> struct regmap *);
>> void gdsc_unregister(struct gdsc_desc *desc);
>> int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
>> +int gdsc_pipe_enable(struct generic_pm_domain *domain);
>> +int gdsc_pipe_disable(struct generic_pm_domain *domain);
>> #else
>> static inline int gdsc_register(struct gdsc_desc *desc,
>> struct reset_controller_dev *rcdev,
>> --
>> 2.34.1
>>
--
With best wishes
Dmitry
next prev parent reply other threads:[~2022-02-09 22:41 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-04 14:46 [PATCH v2 00/11] PCI: qcom: add support for PCIe on SM8450 platform Dmitry Baryshkov
2022-02-04 14:46 ` [PATCH v2 01/11] dt-bindings: pci: qcom,pcie: drop unused "pipe" clocks Dmitry Baryshkov
2022-02-09 22:22 ` Rob Herring
2022-02-04 14:46 ` [PATCH v2 02/11] dt-bindings: pci: qcom: Document PCIe bindings for SM8450 Dmitry Baryshkov
2022-02-04 14:46 ` [PATCH v2 03/11] clk: qcom: gdsc: add support for clocks tied to the GDSC Dmitry Baryshkov
2022-02-04 22:05 ` Bjorn Andersson
2022-02-09 22:41 ` Dmitry Baryshkov [this message]
2022-02-11 19:52 ` Dmitry Baryshkov
2022-02-15 10:24 ` Prasad Malisetty (Temp) (QUIC)
2022-02-25 1:55 ` Dmitry Baryshkov
2022-03-01 6:42 ` Prasad Malisetty
2022-03-01 6:47 ` Dmitry Baryshkov
2022-03-01 17:43 ` Bjorn Andersson
2022-02-04 14:46 ` [PATCH v2 04/11] clk: qcom: gcc-sc7280: switch PCIe GDSCs to pipe_clk_gdsc Dmitry Baryshkov
2022-02-04 14:46 ` [PATCH v2 05/11] clk: qcom: gcc-sm8450: " Dmitry Baryshkov
2022-02-04 14:46 ` [PATCH v2 06/11] PCI: qcom: Balance pm_runtime_foo() calls Dmitry Baryshkov
2022-02-04 14:46 ` [PATCH v2 07/11] PCI: qcom: Remove unnecessary pipe_clk handling Dmitry Baryshkov
2022-02-04 14:46 ` [PATCH v2 08/11] PCI: qcom: Remove pipe_clk_src reparenting Dmitry Baryshkov
2022-02-04 14:46 ` [PATCH v2 09/11] PCI: qcom: Remove redundancy between qcom_pcie and qcom_pcie_cfg Dmitry Baryshkov
2022-02-04 14:46 ` [PATCH v2 10/11] PCI: qcom: Add ddrss_sf_tbu flag Dmitry Baryshkov
2022-02-04 14:46 ` [PATCH v2 11/11] PCI: qcom: Add SM8450 PCIe support Dmitry Baryshkov
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=b7e0ad58-b837-7dcf-4386-ab7ff82ee65c@linaro.org \
--to=dmitry.baryshkov@linaro.org \
--cc=agross@kernel.org \
--cc=bhelgaas@google.com \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=kw@linux.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=mturquette@baylibre.com \
--cc=pmaliset@codeaurora.org \
--cc=quic_pmaliset@quicinc.com \
--cc=robh+dt@kernel.org \
--cc=svarbanov@mm-sol.com \
--cc=swboyd@chromium.org \
--cc=vkoul@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).