From: Neil Armstrong <neil.armstrong@linaro.org>
To: Caleb Connolly <caleb.connolly@linaro.org>,
Lukasz Majewski <lukma@denx.de>,
Sean Anderson <seanga2@gmail.com>,
Sumit Garg <sumit.garg@kernel.org>, Tom Rini <trini@konsulko.com>
Cc: u-boot-qcom@groups.io, u-boot@lists.denx.de
Subject: Re: [PATCH 1/3] clk/qcom: bubble up qcom_gate_clk_en() errors
Date: Mon, 17 Mar 2025 15:01:20 +0100 [thread overview]
Message-ID: <142fdabb-894b-4760-a89f-e4661a14f49c@linaro.org> (raw)
In-Reply-To: <20250314-sc7280-more-clocks-v1-1-ead54487c38e@linaro.org>
On 14/03/2025 16:31, Caleb Connolly wrote:
> If we try to enable a gate clock that doesn't exist, we used to just
> fail silently. This may make sense for early bringup of some core
> peripherals that we know are already enabled, but it only makes
> debugging missing clocks more difficult.
>
> Bubble up errors now that qcom_gate_clk_en() can return an error code to
> catch any still-missing clocks and make it easier to find missing ones
> as more complicated peripherals are enabled.
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> drivers/clk/qcom/clock-apq8016.c | 3 +--
> drivers/clk/qcom/clock-qcm2290.c | 4 +---
> drivers/clk/qcom/clock-qcom.h | 12 +++++++++---
> drivers/clk/qcom/clock-sa8775p.c | 4 +---
> drivers/clk/qcom/clock-sc7280.c | 4 +---
> drivers/clk/qcom/clock-sdm845.c | 4 +---
> drivers/clk/qcom/clock-sm6115.c | 4 +---
> drivers/clk/qcom/clock-sm8150.c | 4 +---
> drivers/clk/qcom/clock-sm8250.c | 4 +---
> drivers/clk/qcom/clock-sm8550.c | 4 +---
> drivers/clk/qcom/clock-sm8650.c | 4 +---
> drivers/clk/qcom/clock-x1e80100.c | 4 +---
> 12 files changed, 20 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c
> index b5def55dbc2ac671130624dec56592288a0ceb3d..3bafbfea3b6f5bf903f6f5a593e1f062bd40c53a 100644
> --- a/drivers/clk/qcom/clock-apq8016.c
> +++ b/drivers/clk/qcom/clock-apq8016.c
> @@ -144,11 +144,10 @@ static int apq8016_clk_enable(struct clk *clk)
> return 0;
> }
>
> debug("%s: clk %s\n", __func__, apq8016_clks[clk->id].name);
> - qcom_gate_clk_en(priv, clk->id);
>
> - return 0;
> + return qcom_gate_clk_en(priv, clk->id);
> }
>
> static struct msm_clk_data apq8016_clk_data = {
> .set_rate = apq8016_clk_set_rate,
> diff --git a/drivers/clk/qcom/clock-qcm2290.c b/drivers/clk/qcom/clock-qcm2290.c
> index c78705cb8cf19a75508f5a3a0fbc7c04d750d273..1326b770c3ebd723120de4b6657aafac726023d6 100644
> --- a/drivers/clk/qcom/clock-qcm2290.c
> +++ b/drivers/clk/qcom/clock-qcm2290.c
> @@ -133,11 +133,9 @@ static int qcm2290_enable(struct clk *clk)
> qcom_gate_clk_en(priv, GCC_USB3_PRIM_CLKREF_CLK);
> break;
> }
>
> - qcom_gate_clk_en(priv, clk->id);
> -
> - return 0;
> + return qcom_gate_clk_en(priv, clk->id);
> }
>
> static const struct qcom_reset_map qcm2290_gcc_resets[] = {
> [GCC_CAMSS_OPE_BCR] = { 0x55000 },
> diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h
> index ff336dea39cf5cbe35f37f93669285897ba185a4..e9eb659f3ed37522a71e04e9418f48091bf9a0aa 100644
> --- a/drivers/clk/qcom/clock-qcom.h
> +++ b/drivers/clk/qcom/clock-qcom.h
> @@ -6,8 +6,9 @@
> #define _CLOCK_QCOM_H
>
> #include <asm/io.h>
> #include <linux/bitfield.h>
> +#include <errno.h>
>
> #define CFG_CLK_SRC_CXO (0 << 8)
> #define CFG_CLK_SRC_GPLL0 (1 << 8)
> #define CFG_CLK_SRC_GPLL0_AUX2 (2 << 8)
> @@ -104,15 +105,20 @@ void clk_rcg_set_rate_mnd(phys_addr_t base, uint32_t cmd_rcgr,
> void clk_rcg_set_rate(phys_addr_t base, uint32_t cmd_rcgr, int div,
> int source);
> void clk_phy_mux_enable(phys_addr_t base, uint32_t cmd_rcgr, bool enabled);
>
> -static inline void qcom_gate_clk_en(const struct msm_clk_priv *priv, unsigned long id)
> +static inline int qcom_gate_clk_en(const struct msm_clk_priv *priv, unsigned long id)
> {
> u32 val;
> - if (id >= priv->data->num_clks || priv->data->clks[id].reg == 0)
> - return;
> + if (id >= priv->data->num_clks || priv->data->clks[id].reg == 0) {
> + log_err("gcc@%#08llx: unknown clock ID %lu!\n",
> + priv->base, id);
> + return -ENOENT;
> + }
>
> val = readl(priv->base + priv->data->clks[id].reg);
> writel(val | priv->data->clks[id].en_val, priv->base + priv->data->clks[id].reg);
> +
> + return 0;
> }
>
> #endif
> diff --git a/drivers/clk/qcom/clock-sa8775p.c b/drivers/clk/qcom/clock-sa8775p.c
> index e31f24ed4f0ca989f20d1eadcff5f24f173455fa..527cecf5c8282a7c2e16740e6520241a230bca73 100644
> --- a/drivers/clk/qcom/clock-sa8775p.c
> +++ b/drivers/clk/qcom/clock-sa8775p.c
> @@ -72,11 +72,9 @@ static int sa8775p_enable(struct clk *clk)
> qcom_gate_clk_en(priv, GCC_USB3_PRIM_PHY_COM_AUX_CLK);
> break;
> }
>
> - qcom_gate_clk_en(priv, clk->id);
> -
> - return 0;
> + return qcom_gate_clk_en(priv, clk->id);
> }
>
> static const struct qcom_reset_map sa8775p_gcc_resets[] = {
> [GCC_CAMERA_BCR] = { 0x32000 },
> diff --git a/drivers/clk/qcom/clock-sc7280.c b/drivers/clk/qcom/clock-sc7280.c
> index 5d343f120519da02d8ec3e269d348ab46cde70a8..8ffd1f43f23e51140c7822f0f523fdfd8ab1de7a 100644
> --- a/drivers/clk/qcom/clock-sc7280.c
> +++ b/drivers/clk/qcom/clock-sc7280.c
> @@ -72,11 +72,9 @@ static int sc7280_enable(struct clk *clk)
> qcom_gate_clk_en(priv, GCC_USB3_PRIM_PHY_COM_AUX_CLK);
> break;
> }
>
> - qcom_gate_clk_en(priv, clk->id);
> -
> - return 0;
> + return qcom_gate_clk_en(priv, clk->id);
> }
>
> static const struct qcom_reset_map sc7280_gcc_resets[] = {
> [GCC_PCIE_0_BCR] = { 0x6b000 },
> diff --git a/drivers/clk/qcom/clock-sdm845.c b/drivers/clk/qcom/clock-sdm845.c
> index adffb0cb2402a6b27e0e998af2f121c519301820..6a0bf16ba2dce7f922adf971817399c71ac35586 100644
> --- a/drivers/clk/qcom/clock-sdm845.c
> +++ b/drivers/clk/qcom/clock-sdm845.c
> @@ -161,11 +161,9 @@ static int sdm845_clk_enable(struct clk *clk)
> qcom_gate_clk_en(priv, GCC_USB3_SEC_PHY_COM_AUX_CLK);
> break;
> }
>
> - qcom_gate_clk_en(priv, clk->id);
> -
> - return 0;
> + return qcom_gate_clk_en(priv, clk->id);
> }
>
> static const struct qcom_reset_map sdm845_gcc_resets[] = {
> [GCC_QUPV3_WRAPPER_0_BCR] = { 0x17000 },
> diff --git a/drivers/clk/qcom/clock-sm6115.c b/drivers/clk/qcom/clock-sm6115.c
> index 9057dfe0bb1873e9edcfd8c01c0a81b6b8b4ca2c..17c2e5617580aae9ad3f0d802bbc6882c2c81043 100644
> --- a/drivers/clk/qcom/clock-sm6115.c
> +++ b/drivers/clk/qcom/clock-sm6115.c
> @@ -145,11 +145,9 @@ static int sm6115_enable(struct clk *clk)
> qcom_gate_clk_en(priv, GCC_USB3_PRIM_CLKREF_CLK);
> break;
> }
>
> - qcom_gate_clk_en(priv, clk->id);
> -
> - return 0;
> + return qcom_gate_clk_en(priv, clk->id);
> }
>
> static const struct qcom_reset_map sm6115_gcc_resets[] = {
> [GCC_QUSB2PHY_PRIM_BCR] = { 0x1c000 },
> diff --git a/drivers/clk/qcom/clock-sm8150.c b/drivers/clk/qcom/clock-sm8150.c
> index 88f2e678f43cfcc9b80917f26c211049ebca6f9d..7dd0d56eb43031e893541e4031cbda2e92ae402d 100644
> --- a/drivers/clk/qcom/clock-sm8150.c
> +++ b/drivers/clk/qcom/clock-sm8150.c
> @@ -242,11 +242,9 @@ static int sm8150_clk_enable(struct clk *clk)
> qcom_gate_clk_en(priv, GCC_USB3_SEC_PHY_COM_AUX_CLK);
> break;
> };
>
> - qcom_gate_clk_en(priv, clk->id);
> -
> - return 0;
> + return qcom_gate_clk_en(priv, clk->id);
> }
>
> static const struct qcom_reset_map sm8150_gcc_resets[] = {
> [GCC_EMAC_BCR] = { 0x6000 },
> diff --git a/drivers/clk/qcom/clock-sm8250.c b/drivers/clk/qcom/clock-sm8250.c
> index e322a923a5c048769dd5134c6c24e16bb74e7317..26396847d85964d5056cf9af62b8bc89be34dc43 100644
> --- a/drivers/clk/qcom/clock-sm8250.c
> +++ b/drivers/clk/qcom/clock-sm8250.c
> @@ -194,11 +194,9 @@ static int sm8250_enable(struct clk *clk)
> qcom_gate_clk_en(priv, GCC_USB3_SEC_PHY_COM_AUX_CLK);
> break;
> }
>
> - qcom_gate_clk_en(priv, clk->id);
> -
> - return 0;
> + return qcom_gate_clk_en(priv, clk->id);
> }
>
> static const struct qcom_reset_map sm8250_gcc_resets[] = {
> [GCC_GPU_BCR] = { 0x71000 },
> diff --git a/drivers/clk/qcom/clock-sm8550.c b/drivers/clk/qcom/clock-sm8550.c
> index 62b5a409e8e08243ad53a0bc58af1c7754a3d950..7c06489b9c4df9ce94fa9b44a762c10133377423 100644
> --- a/drivers/clk/qcom/clock-sm8550.c
> +++ b/drivers/clk/qcom/clock-sm8550.c
> @@ -219,11 +219,9 @@ static int sm8550_enable(struct clk *clk)
> clk_phy_mux_enable(priv->base, 0x8d078, true);
> break;
> }
>
> - qcom_gate_clk_en(priv, clk->id);
> -
> - return 0;
> + return qcom_gate_clk_en(priv, clk->id);
> }
>
> static const struct qcom_reset_map sm8550_gcc_resets[] = {
> [GCC_CAMERA_BCR] = { 0x26000 },
> diff --git a/drivers/clk/qcom/clock-sm8650.c b/drivers/clk/qcom/clock-sm8650.c
> index 9baaecb571f6c575c0e7c48d66214c1331b6642f..364454644a699c495479b3099f590562c415ce82 100644
> --- a/drivers/clk/qcom/clock-sm8650.c
> +++ b/drivers/clk/qcom/clock-sm8650.c
> @@ -216,11 +216,9 @@ static int sm8650_enable(struct clk *clk)
> clk_phy_mux_enable(priv->base, 0x8d078, true);
> break;
> }
>
> - qcom_gate_clk_en(priv, clk->id);
> -
> - return 0;
> + return qcom_gate_clk_en(priv, clk->id);
> }
>
> static const struct qcom_reset_map sm8650_gcc_resets[] = {
> [GCC_CAMERA_BCR] = { 0x26000 },
> diff --git a/drivers/clk/qcom/clock-x1e80100.c b/drivers/clk/qcom/clock-x1e80100.c
> index bd9c6ed1c8a0b4ca0b5164d849efa5c0755c1e2e..542d6248d658d2921ef18877c70389549c471ab8 100644
> --- a/drivers/clk/qcom/clock-x1e80100.c
> +++ b/drivers/clk/qcom/clock-x1e80100.c
> @@ -173,11 +173,9 @@ static int x1e80100_enable(struct clk *clk)
> clk_phy_mux_enable(priv->base, 0x31088, true);
> break;
> }
>
> - qcom_gate_clk_en(priv, clk->id);
> -
> - return 0;
> + return qcom_gate_clk_en(priv, clk->id);
> }
>
> static const struct qcom_reset_map x1e80100_gcc_resets[] = {
> [GCC_AV1E_BCR] = { 0x4a000 },
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
next prev parent reply other threads:[~2025-03-17 14:01 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-14 15:31 [PATCH 0/3] clk/qcom: fixes and more SC7280 clocks Caleb Connolly
2025-03-14 15:31 ` [PATCH 1/3] clk/qcom: bubble up qcom_gate_clk_en() errors Caleb Connolly
2025-03-17 14:01 ` Neil Armstrong [this message]
2025-03-14 15:31 ` [PATCH 2/3] clk/qcom: sc7280: add some debug data Caleb Connolly
2025-03-17 14:01 ` Neil Armstrong
2025-03-14 15:31 ` [PATCH 3/3] clk/qcom: sc7280: add GENI, PCIe, and more USB clocks Caleb Connolly
2025-03-17 14:03 ` Neil Armstrong
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=142fdabb-894b-4760-a89f-e4661a14f49c@linaro.org \
--to=neil.armstrong@linaro.org \
--cc=caleb.connolly@linaro.org \
--cc=lukma@denx.de \
--cc=seanga2@gmail.com \
--cc=sumit.garg@kernel.org \
--cc=trini@konsulko.com \
--cc=u-boot-qcom@groups.io \
--cc=u-boot@lists.denx.de \
/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