Message ID | 20220228124112.3974242-2-peng.fan@oss.nxp.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [V2,1/2] clk: imx: add mcore_booted module paratemter | expand |
Quoting Peng Fan (OSS) (2022-02-28 04:41:12) > From: Peng Fan <peng.fan@nxp.com> > > If mcore_booted is true, ignore the clk root gate registration and > this will simplify AMP clock management and avoid system hang unexpectly > especially Linux shutdown clk used by mcore. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > > V2: > None > > drivers/clk/imx/clk-composite-8m.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c > index 2dfd6149e528..b16c2c0ea9f6 100644 > --- a/drivers/clk/imx/clk-composite-8m.c > +++ b/drivers/clk/imx/clk-composite-8m.c > @@ -223,14 +223,19 @@ struct clk_hw *__imx8m_clk_hw_composite(const char *name, > div->lock = &imx_ccm_lock; > div->flags = CLK_DIVIDER_ROUND_CLOSEST; > > - gate = kzalloc(sizeof(*gate), GFP_KERNEL); > - if (!gate) > - goto fail; > - > - gate_hw = &gate->hw; > - gate->reg = reg; > - gate->bit_idx = PCG_CGC_SHIFT; > - gate->lock = &imx_ccm_lock; > + /* skip registering the gate ops if M4 is enabled */ > + if (mcore_booted) { > + gate_hw = NULL; It could even use the protected-clocks property and then parse it to figure out to not register this gate? > + } else { > + gate = kzalloc(sizeof(*gate), GFP_KERNEL); > + if (!gate) > + goto fail; > + > + gate_hw = &gate->hw; > + gate->reg = reg; > + gate->bit_idx = PCG_CGC_SHIFT; > + gate->lock = &imx_ccm_lock; > + }
> Subject: Re: [PATCH V2 2/2] clk: imx8m: check mcore_booted before register > clk > > Quoting Peng Fan (OSS) (2022-02-28 04:41:12) > > From: Peng Fan <peng.fan@nxp.com> > > > > If mcore_booted is true, ignore the clk root gate registration and > > this will simplify AMP clock management and avoid system hang > > unexpectly especially Linux shutdown clk used by mcore. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > > > V2: > > None > > > > drivers/clk/imx/clk-composite-8m.c | 21 +++++++++++++-------- > > 1 file changed, 13 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/clk/imx/clk-composite-8m.c > > b/drivers/clk/imx/clk-composite-8m.c > > index 2dfd6149e528..b16c2c0ea9f6 100644 > > --- a/drivers/clk/imx/clk-composite-8m.c > > +++ b/drivers/clk/imx/clk-composite-8m.c > > @@ -223,14 +223,19 @@ struct clk_hw > *__imx8m_clk_hw_composite(const char *name, > > div->lock = &imx_ccm_lock; > > div->flags = CLK_DIVIDER_ROUND_CLOSEST; > > > > - gate = kzalloc(sizeof(*gate), GFP_KERNEL); > > - if (!gate) > > - goto fail; > > - > > - gate_hw = &gate->hw; > > - gate->reg = reg; > > - gate->bit_idx = PCG_CGC_SHIFT; > > - gate->lock = &imx_ccm_lock; > > + /* skip registering the gate ops if M4 is enabled */ > > + if (mcore_booted) { > > + gate_hw = NULL; > > It could even use the protected-clocks property and then parse it to figure out > to not register this gate? Because of hardware design as I replied in patch 1/2, that means we will add hundreds of clk entry in device tree. I would not do that. Thanks, Peng. > > > + } else { > > + gate = kzalloc(sizeof(*gate), GFP_KERNEL); > > + if (!gate) > > + goto fail; > > + > > + gate_hw = &gate->hw; > > + gate->reg = reg; > > + gate->bit_idx = PCG_CGC_SHIFT; > > + gate->lock = &imx_ccm_lock; > > + }
On 22-02-28 20:41:12, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > If mcore_booted is true, ignore the clk root gate registration and > this will simplify AMP clock management and avoid system hang unexpectly > especially Linux shutdown clk used by mcore. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > > V2: > None > > drivers/clk/imx/clk-composite-8m.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c > index 2dfd6149e528..b16c2c0ea9f6 100644 > --- a/drivers/clk/imx/clk-composite-8m.c > +++ b/drivers/clk/imx/clk-composite-8m.c > @@ -223,14 +223,19 @@ struct clk_hw *__imx8m_clk_hw_composite(const char *name, > div->lock = &imx_ccm_lock; > div->flags = CLK_DIVIDER_ROUND_CLOSEST; > > - gate = kzalloc(sizeof(*gate), GFP_KERNEL); > - if (!gate) > - goto fail; > - > - gate_hw = &gate->hw; > - gate->reg = reg; > - gate->bit_idx = PCG_CGC_SHIFT; > - gate->lock = &imx_ccm_lock; > + /* skip registering the gate ops if M4 is enabled */ > + if (mcore_booted) { > + gate_hw = NULL; Lets have the gate_hw set to NULL when declared and get rid of this if case. > + } else { And then lets do here: if (!mcore_booted) { > + gate = kzalloc(sizeof(*gate), GFP_KERNEL); > + if (!gate) > + goto fail; > + > + gate_hw = &gate->hw; > + gate->reg = reg; > + gate->bit_idx = PCG_CGC_SHIFT; > + gate->lock = &imx_ccm_lock; > + } > Would look a bit cleaner this way. With that done: Reviewed-by: Abel Vesa <abel.vesa@nxp.com> > hw = clk_hw_register_composite(NULL, name, parent_names, num_parents, > mux_hw, mux_ops, div_hw, > -- > 2.25.1 >
On 22-04-12 11:11:17, Abel Vesa wrote: > On 22-02-28 20:41:12, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > If mcore_booted is true, ignore the clk root gate registration and > > this will simplify AMP clock management and avoid system hang unexpectly > > especially Linux shutdown clk used by mcore. > > > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > > > V2: > > None > > > > drivers/clk/imx/clk-composite-8m.c | 21 +++++++++++++-------- > > 1 file changed, 13 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c > > index 2dfd6149e528..b16c2c0ea9f6 100644 > > --- a/drivers/clk/imx/clk-composite-8m.c > > +++ b/drivers/clk/imx/clk-composite-8m.c > > @@ -223,14 +223,19 @@ struct clk_hw *__imx8m_clk_hw_composite(const char *name, > > div->lock = &imx_ccm_lock; > > div->flags = CLK_DIVIDER_ROUND_CLOSEST; > > > > - gate = kzalloc(sizeof(*gate), GFP_KERNEL); > > - if (!gate) > > - goto fail; > > - > > - gate_hw = &gate->hw; > > - gate->reg = reg; > > - gate->bit_idx = PCG_CGC_SHIFT; > > - gate->lock = &imx_ccm_lock; > > + /* skip registering the gate ops if M4 is enabled */ > > + if (mcore_booted) { > > + gate_hw = NULL; > > Lets have the gate_hw set to NULL when declared and get rid of this if > case. > On a second thought, no need to resend, I'll squash this in myself. > > + } else { > > And then lets do here: > > if (!mcore_booted) { > > + gate = kzalloc(sizeof(*gate), GFP_KERNEL); > > + if (!gate) > > + goto fail; > > + > > + gate_hw = &gate->hw; > > + gate->reg = reg; > > + gate->bit_idx = PCG_CGC_SHIFT; > > + gate->lock = &imx_ccm_lock; > > + } > > > > Would look a bit cleaner this way. > > With that done: > > Reviewed-by: Abel Vesa <abel.vesa@nxp.com> > > > hw = clk_hw_register_composite(NULL, name, parent_names, num_parents, > > mux_hw, mux_ops, div_hw, > > -- > > 2.25.1 > >
diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c index 2dfd6149e528..b16c2c0ea9f6 100644 --- a/drivers/clk/imx/clk-composite-8m.c +++ b/drivers/clk/imx/clk-composite-8m.c @@ -223,14 +223,19 @@ struct clk_hw *__imx8m_clk_hw_composite(const char *name, div->lock = &imx_ccm_lock; div->flags = CLK_DIVIDER_ROUND_CLOSEST; - gate = kzalloc(sizeof(*gate), GFP_KERNEL); - if (!gate) - goto fail; - - gate_hw = &gate->hw; - gate->reg = reg; - gate->bit_idx = PCG_CGC_SHIFT; - gate->lock = &imx_ccm_lock; + /* skip registering the gate ops if M4 is enabled */ + if (mcore_booted) { + gate_hw = NULL; + } else { + gate = kzalloc(sizeof(*gate), GFP_KERNEL); + if (!gate) + goto fail; + + gate_hw = &gate->hw; + gate->reg = reg; + gate->bit_idx = PCG_CGC_SHIFT; + gate->lock = &imx_ccm_lock; + } hw = clk_hw_register_composite(NULL, name, parent_names, num_parents, mux_hw, mux_ops, div_hw,