| Message ID | 20210707043859.195870-1-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | [RFC] clk: Allow forcing an unused clock to be disabled | expand |
On Tue 06 Jul 23:38 CDT 2021, Bjorn Andersson wrote: > The process of disabling unused clocks will skip clocks that are not > enabled by Linux, but doesn't implement the is_enabled() callback to > read back the hardware state. In the case that it's determined that the > parent clock is enabled this might be turned off, causing the skipped > clock to lock up. > > One such case is the RCG "mdp_clk_src" in the Qualcomm SDM845, which at > boot is parented by "disp_cc_pll0", which during clk_disable_unused() is > left untouched, while the parent is disabled. > > Later the typical next operation for "mdp_clk_src" is an > assigned-clock-rates will cause the next enable attempt to reparent the > RCG. But the RCG needs both the old and new parent to be ticking to > perform this switch and will therefor not complete. > > Introduce a new flag for clock drivers to mark that clocks that should > be assumed to be enabled even when is_enabled isn't implemented and mark > the "mdp_clk_src" clock for SDM845 as such. > This allows the driver to transition the RCG from the PLL to a clock > source that is known to be enabled when the future reparenting operation > is undertaken. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > > Naturally this patch should be split in a core and driver part, but for the > sake of the RFC I made both changes in the same patch. > Ping? > drivers/clk/clk.c | 2 +- > drivers/clk/qcom/dispcc-sdm845.c | 1 + > include/linux/clk-provider.h | 2 ++ > 3 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 65508eb89ec9..9e4789d106f3 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1265,7 +1265,7 @@ static void __init clk_disable_unused_subtree(struct clk_core *core) > * sequence. call .disable_unused if available, otherwise fall > * back to .disable > */ > - if (clk_core_is_enabled(core)) { > + if (clk_core_is_enabled(core) || core->flags & CLK_ASSUME_ENABLED_BOOT) { > trace_clk_disable(core); > if (core->ops->disable_unused) > core->ops->disable_unused(core->hw); > diff --git a/drivers/clk/qcom/dispcc-sdm845.c b/drivers/clk/qcom/dispcc-sdm845.c > index 735adfefc379..046f7e656f7c 100644 > --- a/drivers/clk/qcom/dispcc-sdm845.c > +++ b/drivers/clk/qcom/dispcc-sdm845.c > @@ -267,6 +267,7 @@ static struct clk_rcg2 disp_cc_mdss_mdp_clk_src = { > .parent_data = disp_cc_parent_data_3, > .num_parents = ARRAY_SIZE(disp_cc_parent_data_3), > .ops = &clk_rcg2_shared_ops, > + .flags = CLK_ASSUME_ENABLED_BOOT, > }, > }; > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index 162a2e5546a3..d00ca925842c 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -32,6 +32,8 @@ > #define CLK_OPS_PARENT_ENABLE BIT(12) > /* duty cycle call may be forwarded to the parent clock */ > #define CLK_DUTY_CYCLE_PARENT BIT(13) > +/* assume clock is enabled if found unused in late init */ > +#define CLK_ASSUME_ENABLED_BOOT BIT(14) > > struct clk; > struct clk_hw; > -- > 2.29.2 >
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 65508eb89ec9..9e4789d106f3 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1265,7 +1265,7 @@ static void __init clk_disable_unused_subtree(struct clk_core *core) * sequence. call .disable_unused if available, otherwise fall * back to .disable */ - if (clk_core_is_enabled(core)) { + if (clk_core_is_enabled(core) || core->flags & CLK_ASSUME_ENABLED_BOOT) { trace_clk_disable(core); if (core->ops->disable_unused) core->ops->disable_unused(core->hw); diff --git a/drivers/clk/qcom/dispcc-sdm845.c b/drivers/clk/qcom/dispcc-sdm845.c index 735adfefc379..046f7e656f7c 100644 --- a/drivers/clk/qcom/dispcc-sdm845.c +++ b/drivers/clk/qcom/dispcc-sdm845.c @@ -267,6 +267,7 @@ static struct clk_rcg2 disp_cc_mdss_mdp_clk_src = { .parent_data = disp_cc_parent_data_3, .num_parents = ARRAY_SIZE(disp_cc_parent_data_3), .ops = &clk_rcg2_shared_ops, + .flags = CLK_ASSUME_ENABLED_BOOT, }, }; diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 162a2e5546a3..d00ca925842c 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -32,6 +32,8 @@ #define CLK_OPS_PARENT_ENABLE BIT(12) /* duty cycle call may be forwarded to the parent clock */ #define CLK_DUTY_CYCLE_PARENT BIT(13) +/* assume clock is enabled if found unused in late init */ +#define CLK_ASSUME_ENABLED_BOOT BIT(14) struct clk; struct clk_hw;
The process of disabling unused clocks will skip clocks that are not enabled by Linux, but doesn't implement the is_enabled() callback to read back the hardware state. In the case that it's determined that the parent clock is enabled this might be turned off, causing the skipped clock to lock up. One such case is the RCG "mdp_clk_src" in the Qualcomm SDM845, which at boot is parented by "disp_cc_pll0", which during clk_disable_unused() is left untouched, while the parent is disabled. Later the typical next operation for "mdp_clk_src" is an assigned-clock-rates will cause the next enable attempt to reparent the RCG. But the RCG needs both the old and new parent to be ticking to perform this switch and will therefor not complete. Introduce a new flag for clock drivers to mark that clocks that should be assumed to be enabled even when is_enabled isn't implemented and mark the "mdp_clk_src" clock for SDM845 as such. This allows the driver to transition the RCG from the PLL to a clock source that is known to be enabled when the future reparenting operation is undertaken. Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- Naturally this patch should be split in a core and driver part, but for the sake of the RFC I made both changes in the same patch. drivers/clk/clk.c | 2 +- drivers/clk/qcom/dispcc-sdm845.c | 1 + include/linux/clk-provider.h | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-)