| Message ID | 20240510-imx-clk-v2-1-c998f315d29c@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: imx: misc update/fix | expand |
On Fri, May 10, 2024 at 05:18:56PM +0800, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > Bootloader might disable some CCM ROOT Slices. So if mcore_booted set with > display CCM ROOT disabled by Bootloader, kernel display BLK CTRL driver > imx8m_blk_ctrl_driver_init may hang the system because the BUS clk is > disabled. > > Add back gate ops, but with disable doing nothing, then the CCM ROOT > will be enabled when used. > > Fixes: 489bbee0c983 ("clk: imx: composite-8m: Enable gate clk with mcore_booted") I can't find this commitish anywhere, also the subject looks like this patch fixes itself. > Reviewed-by: Ye Li <ye.li@nxp.com> > Reviewed-by: Jacky Bai <ping.bai@nxp.com> > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/clk/imx/clk-composite-8m.c | 53 ++++++++++++++++++++++++++++++-------- > 1 file changed, 42 insertions(+), 11 deletions(-) > > diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c > index 8cc07d056a83..f187582ba491 100644 > --- a/drivers/clk/imx/clk-composite-8m.c > +++ b/drivers/clk/imx/clk-composite-8m.c > @@ -204,6 +204,34 @@ static const struct clk_ops imx8m_clk_composite_mux_ops = { > .determine_rate = imx8m_clk_composite_mux_determine_rate, > }; > > +static int imx8m_clk_composite_gate_enable(struct clk_hw *hw) > +{ > + struct clk_gate *gate = to_clk_gate(hw); > + unsigned long flags; > + u32 val; > + > + spin_lock_irqsave(gate->lock, flags); > + > + val = readl(gate->reg); > + val |= BIT(gate->bit_idx); > + writel(val, gate->reg); > + > + spin_unlock_irqrestore(gate->lock, flags); > + > + return 0; > +} > + > +static void imx8m_clk_composite_gate_disable(struct clk_hw *hw) > +{ > + /* composite clk requires the disable hook */ > +} > + > +static const struct clk_ops imx8m_clk_composite_gate_ops = { > + .enable = imx8m_clk_composite_gate_enable, > + .disable = imx8m_clk_composite_gate_disable, > + .is_enabled = clk_gate_is_enabled, > +}; > + > struct clk_hw *__imx8m_clk_hw_composite(const char *name, > const char * const *parent_names, > int num_parents, void __iomem *reg, > @@ -217,6 +245,7 @@ struct clk_hw *__imx8m_clk_hw_composite(const char *name, > struct clk_mux *mux; > const struct clk_ops *divider_ops; > const struct clk_ops *mux_ops; > + const struct clk_ops *gate_ops; > > mux = kzalloc(sizeof(*mux), GFP_KERNEL); > if (!mux) > @@ -257,20 +286,22 @@ struct clk_hw *__imx8m_clk_hw_composite(const char *name, > div->flags = CLK_DIVIDER_ROUND_CLOSEST; > > /* skip registering the gate ops if M4 is enabled */ This comment doesn't seems to become inaccurate with this patch. > - if (!mcore_booted) { > - gate = kzalloc(sizeof(*gate), GFP_KERNEL); > - if (!gate) > - goto free_div; > - > - gate_hw = &gate->hw; > - gate->reg = reg; > - gate->bit_idx = PCG_CGC_SHIFT; > - gate->lock = &imx_ccm_lock; > - } > + gate = kzalloc(sizeof(*gate), GFP_KERNEL); > + if (!gate) > + goto free_div; > + > + gate_hw = &gate->hw; > + gate->reg = reg; > + gate->bit_idx = PCG_CGC_SHIFT; > + gate->lock = &imx_ccm_lock; > + if (!mcore_booted) > + gate_ops = &clk_gate_ops; > + else > + gate_ops = &imx8m_clk_composite_gate_ops; Please use positive logic. It's easier to read. Sascha
> Subject: Re: [PATCH v2 01/17] clk: imx: composite-8m: Enable gate clk with > mcore_booted > > On Fri, May 10, 2024 at 05:18:56PM +0800, Peng Fan (OSS) wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > > Bootloader might disable some CCM ROOT Slices. So if mcore_booted set > > with display CCM ROOT disabled by Bootloader, kernel display BLK CTRL > > driver imx8m_blk_ctrl_driver_init may hang the system because the BUS > > clk is disabled. > > > > Add back gate ops, but with disable doing nothing, then the CCM ROOT > > will be enabled when used. > > > > Fixes: 489bbee0c983 ("clk: imx: composite-8m: Enable gate clk with > > mcore_booted") > > I can't find this commitish anywhere, also the subject looks like this patch > fixes itself. My bad. Picked the first commit when I use --pretty=fixes. > > > Reviewed-by: Ye Li <ye.li@nxp.com> > > Reviewed-by: Jacky Bai <ping.bai@nxp.com> > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > > --- > > drivers/clk/imx/clk-composite-8m.c | 53 > > ++++++++++++++++++++++++++++++-------- > > 1 file changed, 42 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/clk/imx/clk-composite-8m.c > > b/drivers/clk/imx/clk-composite-8m.c > > index 8cc07d056a83..f187582ba491 100644 > > --- a/drivers/clk/imx/clk-composite-8m.c > > +++ b/drivers/clk/imx/clk-composite-8m.c > > @@ -204,6 +204,34 @@ static const struct clk_ops > imx8m_clk_composite_mux_ops = { > > .determine_rate = imx8m_clk_composite_mux_determine_rate, > > }; > > > > +static int imx8m_clk_composite_gate_enable(struct clk_hw *hw) { > > + struct clk_gate *gate = to_clk_gate(hw); > > + unsigned long flags; > > + u32 val; > > + > > + spin_lock_irqsave(gate->lock, flags); > > + > > + val = readl(gate->reg); > > + val |= BIT(gate->bit_idx); > > + writel(val, gate->reg); > > + > > + spin_unlock_irqrestore(gate->lock, flags); > > + > > + return 0; > > +} > > + > > +static void imx8m_clk_composite_gate_disable(struct clk_hw *hw) { > > + /* composite clk requires the disable hook */ } > > + > > +static const struct clk_ops imx8m_clk_composite_gate_ops = { > > + .enable = imx8m_clk_composite_gate_enable, > > + .disable = imx8m_clk_composite_gate_disable, > > + .is_enabled = clk_gate_is_enabled, > > +}; > > + > > struct clk_hw *__imx8m_clk_hw_composite(const char *name, > > const char * const *parent_names, > > int num_parents, void __iomem > *reg, @@ -217,6 +245,7 @@ struct > > clk_hw *__imx8m_clk_hw_composite(const char *name, > > struct clk_mux *mux; > > const struct clk_ops *divider_ops; > > const struct clk_ops *mux_ops; > > + const struct clk_ops *gate_ops; > > > > mux = kzalloc(sizeof(*mux), GFP_KERNEL); > > if (!mux) > > @@ -257,20 +286,22 @@ struct clk_hw > *__imx8m_clk_hw_composite(const char *name, > > div->flags = CLK_DIVIDER_ROUND_CLOSEST; > > > > /* skip registering the gate ops if M4 is enabled */ > > This comment doesn't seems to become inaccurate with this patch. Right. Drop it in v3. > > > - if (!mcore_booted) { > > - gate = kzalloc(sizeof(*gate), GFP_KERNEL); > > - if (!gate) > > - goto free_div; > > - > > - gate_hw = &gate->hw; > > - gate->reg = reg; > > - gate->bit_idx = PCG_CGC_SHIFT; > > - gate->lock = &imx_ccm_lock; > > - } > > + gate = kzalloc(sizeof(*gate), GFP_KERNEL); > > + if (!gate) > > + goto free_div; > > + > > + gate_hw = &gate->hw; > > + gate->reg = reg; > > + gate->bit_idx = PCG_CGC_SHIFT; > > + gate->lock = &imx_ccm_lock; > > + if (!mcore_booted) > > + gate_ops = &clk_gate_ops; > > + else > > + gate_ops = &imx8m_clk_composite_gate_ops; > > Please use positive logic. It's easier to read. Sure. update in v3. Thanks, Peng. > > Sascha > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | > http://www.p/ > engutronix.de%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C86a2a0908b > 83408401af08dc73178043%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 > %7C0%7C638511791969660985%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi > MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0% > 7C%7C%7C&sdata=eJJ8rdenU2ACBFUBX0CKKFzhkIQA999b7rpOuMkqgoU%3 > D&reserved=0 | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c index 8cc07d056a83..f187582ba491 100644 --- a/drivers/clk/imx/clk-composite-8m.c +++ b/drivers/clk/imx/clk-composite-8m.c @@ -204,6 +204,34 @@ static const struct clk_ops imx8m_clk_composite_mux_ops = { .determine_rate = imx8m_clk_composite_mux_determine_rate, }; +static int imx8m_clk_composite_gate_enable(struct clk_hw *hw) +{ + struct clk_gate *gate = to_clk_gate(hw); + unsigned long flags; + u32 val; + + spin_lock_irqsave(gate->lock, flags); + + val = readl(gate->reg); + val |= BIT(gate->bit_idx); + writel(val, gate->reg); + + spin_unlock_irqrestore(gate->lock, flags); + + return 0; +} + +static void imx8m_clk_composite_gate_disable(struct clk_hw *hw) +{ + /* composite clk requires the disable hook */ +} + +static const struct clk_ops imx8m_clk_composite_gate_ops = { + .enable = imx8m_clk_composite_gate_enable, + .disable = imx8m_clk_composite_gate_disable, + .is_enabled = clk_gate_is_enabled, +}; + struct clk_hw *__imx8m_clk_hw_composite(const char *name, const char * const *parent_names, int num_parents, void __iomem *reg, @@ -217,6 +245,7 @@ struct clk_hw *__imx8m_clk_hw_composite(const char *name, struct clk_mux *mux; const struct clk_ops *divider_ops; const struct clk_ops *mux_ops; + const struct clk_ops *gate_ops; mux = kzalloc(sizeof(*mux), GFP_KERNEL); if (!mux) @@ -257,20 +286,22 @@ struct clk_hw *__imx8m_clk_hw_composite(const char *name, div->flags = CLK_DIVIDER_ROUND_CLOSEST; /* skip registering the gate ops if M4 is enabled */ - if (!mcore_booted) { - gate = kzalloc(sizeof(*gate), GFP_KERNEL); - if (!gate) - goto free_div; - - gate_hw = &gate->hw; - gate->reg = reg; - gate->bit_idx = PCG_CGC_SHIFT; - gate->lock = &imx_ccm_lock; - } + gate = kzalloc(sizeof(*gate), GFP_KERNEL); + if (!gate) + goto free_div; + + gate_hw = &gate->hw; + gate->reg = reg; + gate->bit_idx = PCG_CGC_SHIFT; + gate->lock = &imx_ccm_lock; + if (!mcore_booted) + gate_ops = &clk_gate_ops; + else + gate_ops = &imx8m_clk_composite_gate_ops; hw = clk_hw_register_composite(NULL, name, parent_names, num_parents, mux_hw, mux_ops, div_hw, - divider_ops, gate_hw, &clk_gate_ops, flags); + divider_ops, gate_hw, gate_ops, flags); if (IS_ERR(hw)) goto free_gate;