From: Bjorn Andersson <[email protected]>
Some clock implementations doesn't provide means of implementing
is_enabled(), but still requires to be explicitly disabled when found
unused as part of clk_disable_unused().
One such set of clocks are Qualcomm's display RCGs. These can be enabled
and disabled automatically by the hardware, so it's not possible to
reliably query their configuration. Further more, these clocks need to
be disabled when unused, to allow them to be "parked" onto a safe
parent. Failure to disable the RCG results in the hardware locking up as
clk_disable_unused() traverses up the tree and turns off its source
clocks.
Add a new flag, CLK_ASSUME_ENABLED_BOOT, which clock drivers can use to
signal that these clocks should be disabled even if they don't implement
the is_enabled() ops.
Signed-off-by: Bjorn Andersson <[email protected]>
---
Changes since v1
- Removed Vinods r-b
drivers/clk/clk.c | 2 +-
include/linux/clk-provider.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ed119182aa1b..9789ec137219 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1284,7 +1284,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_WHEN_UNUSED) {
trace_clk_disable(core);
if (core->ops->disable_unused)
core->ops->disable_unused(core->hw);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index c10dc4c659e2..9038022ffebd 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_WHEN_UNUSED BIT(14)
struct clk;
struct clk_hw;
--
2.34.1
On Tue 03 May 08:04 CDT 2022, Robert Foss wrote:
> From: Bjorn Andersson <[email protected]>
>
> Some clock implementations doesn't provide means of implementing
> is_enabled(), but still requires to be explicitly disabled when found
> unused as part of clk_disable_unused().
>
> One such set of clocks are Qualcomm's display RCGs. These can be enabled
> and disabled automatically by the hardware, so it's not possible to
> reliably query their configuration. Further more, these clocks need to
> be disabled when unused, to allow them to be "parked" onto a safe
> parent. Failure to disable the RCG results in the hardware locking up as
> clk_disable_unused() traverses up the tree and turns off its source
> clocks.
>
> Add a new flag, CLK_ASSUME_ENABLED_BOOT, which clock drivers can use to
> signal that these clocks should be disabled even if they don't implement
> the is_enabled() ops.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
>
I discussed this with Stephen a while ago and we agreed that in a
sufficiently complex system with kernel modules booting without
clk_ignore_unused simply isn't supported.
We will have to design something better. So please drop this patch from
the series.
Regards,
Bjorn
> Changes since v1
> - Removed Vinods r-b
>
>
> drivers/clk/clk.c | 2 +-
> include/linux/clk-provider.h | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ed119182aa1b..9789ec137219 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1284,7 +1284,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_WHEN_UNUSED) {
> trace_clk_disable(core);
> if (core->ops->disable_unused)
> core->ops->disable_unused(core->hw);
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index c10dc4c659e2..9038022ffebd 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_WHEN_UNUSED BIT(14)
>
> struct clk;
> struct clk_hw;
> --
> 2.34.1
>
On Tue, 3 May 2022 at 17:02, Bjorn Andersson <[email protected]> wrote:
>
> On Tue 03 May 08:04 CDT 2022, Robert Foss wrote:
>
> > From: Bjorn Andersson <[email protected]>
> >
> > Some clock implementations doesn't provide means of implementing
> > is_enabled(), but still requires to be explicitly disabled when found
> > unused as part of clk_disable_unused().
> >
> > One such set of clocks are Qualcomm's display RCGs. These can be enabled
> > and disabled automatically by the hardware, so it's not possible to
> > reliably query their configuration. Further more, these clocks need to
> > be disabled when unused, to allow them to be "parked" onto a safe
> > parent. Failure to disable the RCG results in the hardware locking up as
> > clk_disable_unused() traverses up the tree and turns off its source
> > clocks.
> >
> > Add a new flag, CLK_ASSUME_ENABLED_BOOT, which clock drivers can use to
> > signal that these clocks should be disabled even if they don't implement
> > the is_enabled() ops.
> >
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > ---
> >
>
> I discussed this with Stephen a while ago and we agreed that in a
> sufficiently complex system with kernel modules booting without
> clk_ignore_unused simply isn't supported.
>
> We will have to design something better. So please drop this patch from
> the series.
Ack