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
