Re: [PATCH v3 4/6] PM / devfreq: Add dynamic scaling for imx ddr controller

[Shawn Guo <shawnguo@kernel.org>]

On Mon, Dec 02, 2019 at 09:12:12AM +0000, Leonard Crestez wrote:
> >> +
> >> +	/* change the ddr freqency */
> >> +	arm_smccc_smc(IMX_SIP_DDR_DVFS, target_freq, online_cpus,
> >> +			0, 0, 0, 0, 0, &res);
> >> +
> >> +	local_irq_enable();
> >> +}
> >> +
> >> +struct clk *clk_get_parent_by_index(struct clk *clk, int index)
> >> +{
> >> +	struct clk_hw *hw;
> >> +
> >> +	hw = clk_hw_get_parent_by_index(__clk_get_hw(clk), index);
> > 
> > Okay, this is why you need clk-provider.h.  But this
> > clk_get_parent_by_index() function looks completely generic, and should
> > be proposed to clock core?
> 
> There are very few driver users of clk_hw_get_parent_by_index:
> 
> $ git grep -wl clk_hw_get_parent_by_index |grep -v drivers/clk
> arch/mips/alchemy/common/clock.c
> drivers/cpufreq/qoriq-cpufreq.c
> drivers/devfreq/imx8m-ddrc.c
> drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
> drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c
> drivers/media/platform/atmel/atmel-isc-base.c
> drivers/rtc/rtc-ac100.c
> include/linux/clk-provider.h
> 
> Even clk_get_parent has few users and it contains this strange comment:
> 
> /* TODO: Create a per-user clk and change callers to call clk_put */
> 
> That proposed change effectively creates a new API? I didn't want to add 
> a new clk core API with unclear semantics.

Since the merged version has 'static' added for clk_get_parent_by_index(),
I'm fine with it being a local function.  It's Stephen's call whether
we should have it at clock core level.

Shawn