RE: [PATCH V2 2/2] clk: scmi: support state_ctrl_forbidden

[Peng Fan]

> Subject: Re: [PATCH V2 2/2] clk: scmi: support state_ctrl_forbidden
> 
> On Thu, Dec 07, 2023 at 05:33:45PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@xxxxxxx>
> >
> > Some clocks may exported to linux, while those clocks are not allowed
> > to configure by Linux. For example:
> >
> 
> Hi,
> 
> > SYS_CLK1-----
> >              \
> > 	     --MUX--->MMC1_CLK
> >              /
> > SYS_CLK2-----
> >
> > MMC1 needs set parent, so SYS_CLK1 and SYS_CLK2 are exported to Linux,
> > then the clk propagation will touch SYS_CLK1 or SYS_CLK2.
> > So we need bypass the failure for SYS_CLK1 or SYS_CLK2 when enable the
> > clock of MMC1.
> >
> 
> 
> So I was puzzled a bit at first (as said) by the fact that here we silently swallow
> the failure if the SCMI Clock cannot be disabled, BUT then I spotted in
> include/linux/clk.h
> 
> 	/**
> 	 * clk_enable - inform the system when the clock source should be
> running.
> 	 * @clk: clock source
> 	 *
> 	 * If the clock can not be enabled/disabled, this should return success.
> 
> ...so I suppose it is fine for the CLK framework at the end.
> 
> My next remaining question is why are you not doing the same when (ret == -
> EACCES && clk->info->state_ctrl_forbidden) for atomic_ops ?
> 
> I.e. in:
> 
> 	clk-scmi.c::static int scmi_clk_atomic_enable(struct clk_hw *hw)
> 
> Any particular reason (beside not needing it in your particular case...)

No particular reason, we not use atomic_ops in our case. So I am not able
to test it. I could add the same in atomic_ops in V3.

Thanks,
Peng.
> 
> Thanks,
> Cristian