On Thu 09 Dec 00:37 PST 2021, Stephen Boyd wrote:

Quoting Dmitry Baryshkov (2021-12-07 18:22:09)

quoted

Some of RCG2 clocks can become stuck during the boot process, when
device drivers are enabling and disabling the RCG2's parent clocks.
To prevernt such outcome of driver probe sequences, add API to park

s/prevernt/prevent/

quoted

clocks to the safe clock source (typically TCXO).

Signed-off-by: Dmitry Baryshkov <redacted>

I'd prefer this approach vs. adding a new clk flag. The clk framework
doesn't handle handoff properly today so we shouldn't try to bandage
that in the core.

I'm not against putting this responsibility in the drivers, but I don't
think we can blindly park all the RCGs that may or may not be used.

Note that we should do this for all RCGs that downstream are marked as
enable_safe_config (upstream should be using clk_rcg2_shared_ops)
and disabling some of those probe time won't be appreciated by the
hardware.


If you don't like the flag passed to clk_disable_unused (which is like a
very reasonable objection to have), we need to make progress towards a
proper solution that replaces clk_disable_unused().

quoted

diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index e1b1b426fae4..230b04a7427c 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c

@@ -1036,6 +1036,40 @@ static void clk_rcg2_shared_disable(struct clk_hw *hw)
        regmap_write(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, cfg);
 }
 
+int clk_rcg2_park_safely(struct regmap *regmap, u32 offset, unsigned int safe_src)

This seems to just duplicate clk_rcg2_shared_disable()?

Regards,
Bjorn

Please add kernel doc as it's an exported symbol.

quoted

+{
+       unsigned int val, ret, count;
+
+       ret = regmap_read(regmap, offset + CFG_REG, &val);
+       if (ret)
+               return ret;
+
+       /* assume safe source is 0 */

Are we assuming safe source is 0 here? It looks like we pass it in now?

quoted

+       if ((val & CFG_SRC_SEL_MASK) == (safe_src << CFG_SRC_SEL_SHIFT))
+               return 0;
+
+       regmap_write(regmap, offset + CFG_REG, safe_src << CFG_SRC_SEL_SHIFT);
+
+       ret = regmap_update_bits(regmap, offset + CMD_REG,
+                                CMD_UPDATE, CMD_UPDATE);
+       if (ret)
+               return ret;
+
+       /* Wait for update to take effect */
+       for (count = 500; count > 0; count--) {
+               ret = regmap_read(regmap, offset + CMD_REG, &val);
+               if (ret)
+                       return ret;
+               if (!(val & CMD_UPDATE))
+                       return 0;
+               udelay(1);
+       }
+
+       WARN(1, "the rcg didn't update its configuration.");

Add a newline?

quoted

+       return -EBUSY;
+}
+EXPORT_SYMBOL_GPL(clk_rcg2_park_safely);
+