On 07/17, Nayak, Rajendra wrote:
>
>
> On 7/14/2017 4:13 PM, Rob Clark wrote:
> >On Fri, Jul 14, 2017 at 12:52 AM, Rajendra Nayak <rnayak@xxxxxxxxxxxxxx> wrote:
> >>Hi Rob,
> >>
> >>On 07/11/2017 11:50 PM, Rob Clark wrote:
> >>
> >>>diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> >>>index d523991c945f..90b698c910d0 100644
> >>>--- a/drivers/clk/qcom/common.c
> >>>+++ b/drivers/clk/qcom/common.c
> >>>@@ -11,6 +11,7 @@
> >>> * GNU General Public License for more details.
> >>> */
> >>>
> >>>+#include <linux/clk.h>
> >>> #include <linux/export.h>
> >>> #include <linux/module.h>
> >>> #include <linux/regmap.h>
> >>>@@ -258,6 +259,33 @@ int qcom_cc_really_probe(struct platform_device *pdev,
> >>> if (ret)
> >>> return ret;
> >>>
> >>>+ /* Check which of clocks that we inherit state from bootloader
> >>>+ * are enabled, and fixup enable/prepare state (as well as that
> >>>+ * of it's parents).
> >>>+ *
> >>>+ * TODO can we assume that parents coming from another clk
> >>>+ * driver are already registered?
> >>>+ */
> >>>+ for (i = 0; i < num_clks; i++) {
> >>>+ struct clk_hw *hw;
> >>>+
> >>>+ if (!rclks[i])
> >>>+ continue;
> >>>+
> >>>+ hw = &rclks[i]->hw;
> >>>+
> >>>+ if (!(hw->init->flags & CLK_INHERIT_BOOTLOADER))
> >>>+ continue;
> >>>+
> >>>+ if (!clk_is_enabled_regmap(hw))
> >>>+ continue;
> >>>+
> >>>+ dev_dbg(dev, "%s is enabled from bootloader!\n",
> >>>+ hw->init->name);
> >>>+
> >>>+ clk_inherit_enabled(hw->clk);
> >>
> >>how about just calling a clk_prepare_enable(hw->clk) instead of adding a new API?
> >>The flag could also be something qcom specific and then we avoid having to add
> >>anything in generic CCF code and its all handled in the qcom clock drivers.
> >
> >Hmm, I could try that.. I *guess* it doesn't hurt to enable an
> >already enabled clk..
>
> yes, ideally it shouldn't hurt.
It hurts when you enable a PLL when it's already enabled. I
recall having to add code specifically for this reason to avoid
enabling a PLL that is already enabled out of the bootloader. But
that's because the enable sequence for a PLL is multiple writes
that first keep the output off and then turn it back on after
things stabilize. For RCGs this isn't the case.
>
> >
> >beyond that, I wonder if this is something that other platforms would
> >want, so maybe I should be doing the check for enabled in CCF core?
> >If not, making this a qcom specific flag makes sense.
>
> I think most previous attempts to solve this were done trying to keep
> it very generic and they didn't go too far.
> So I was thinking maybe we could deal with it within qcom drivers for
> now, and if others come up with something similar, then look to
> consolidate at a generic CCF level.
We know that other SoC vendors need handoff features as well, so
there will definitely be a point where we need to consolidate at
the framework level. Working something up that works for qcom
would be good to try out though to find edge cases, etc.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
