RE: [PATCH] clk: imx: introduce imx_clk_hw_critical

[Aisheng Dong]

> From: Peng Fan <peng.fan@xxxxxxx>
> Sent: Thursday, April 23, 2020 6:54 PM
> 
> > Subject: RE: [PATCH] clk: imx: introduce imx_clk_hw_critical
> >
> > > From: Peng Fan <peng.fan@xxxxxxx>
> > > Sent: Thursday, April 23, 2020 2:52 PM
> > >
> > > To i.MX8M SoC, there is an case is when running dual OSes with
> > > hypervisor, the clk of the hardware that passthrough to the 2nd OS
> > > needs to be setup by 1st Linux OS.
> > > So detect clock-critical from ccm node and enable the clocks to let
> > > the 2nd OS could use the hardware without touch CCM module.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > > ---
> > >  drivers/clk/imx/clk.c | 19 +++++++++++++++++++
> > > drivers/clk/imx/clk.h
> > > |  1
> > > +
> > >  2 files changed, 20 insertions(+)
> > >
> > > diff --git a/drivers/clk/imx/clk.c b/drivers/clk/imx/clk.c index
> > > 87ab8db3d282..ec7d422540c1 100644
> > > --- a/drivers/clk/imx/clk.c
> > > +++ b/drivers/clk/imx/clk.c
> > > @@ -177,3 +177,22 @@ static int __init imx_clk_disable_uart(void)
> > >  	return 0;
> > >  }
> > >  late_initcall_sync(imx_clk_disable_uart);
> > > +
> > > +int imx_clk_hw_critical(struct device_node *np, struct clk_hw *
> > > +const
> > > +hws[]) {
> > > +	struct property *prop;
> > > +	const __be32 *cur;
> > > +	u32 idx;
> > > +	int ret;
> > > +
> > > +	if (!np || !hws)
> > > +		return -EINVAL;
> > > +
> > > +	of_property_for_each_u32(np, "clock-critical", prop, cur, idx) {
> >
> > Is there a binding for it already?
> 
> I think clock-critical is a common bindings? See of_clk_detect_critical.
> Please review whether this approach is acceptable, if do need bindings, I could
> add that in v2.
> 

I'm ok if it's a common binding already supported by current clk framework.
But it seems to be more like a common clk feature rather than platform feature.
Also a bit strange that why I did not find the binding doc in latest kernel.
Maybe Stephen could comment more.

BTW, if using this for dual OSes case, will those critical clocks affect the normal users that
not booting the second OS?

Regards
Aisheng

> Thanks,
> Peng.
> 
> >
> > Regards
> > Aisheng
> >
> > > +		ret = clk_prepare_enable(hws[idx]->clk);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h index
> > > d4ea1609bcb7..701d7440f98c 100644
> > > --- a/drivers/clk/imx/clk.h
> > > +++ b/drivers/clk/imx/clk.h
> > > @@ -9,6 +9,7 @@ extern spinlock_t imx_ccm_lock;
> > >
> > >  void imx_check_clocks(struct clk *clks[], unsigned int count);
> > > void imx_check_clk_hws(struct clk_hw *clks[], unsigned int count);
> > > +int imx_clk_hw_critical(struct device_node *np, struct clk_hw *
> > > +const hws[]);
> > >  void imx_register_uart_clocks(struct clk ** const clks[]);  void
> > > imx_mmdc_mask_handshake(void __iomem *ccm_base, unsigned int chn);
> > > void imx_unregister_clocks(struct clk *clks[], unsigned int count);
> > > --
> > > 2.16.4