[alsa-devel] [PATCH 4/6] ASoC: Intel: Skylake: Register clock device and ops

[Subhransu S. Prusty]

On Fri, Sep 15, 2017 at 06:10:20PM +0530, Subhransu S. Prusty wrote:
> On Fri, Sep 08, 2017 at 08:41:54AM -0500, Pierre-Louis Bossart wrote:
> > On 9/8/17 12:01 AM, Subhransu S. Prusty wrote:
> > >On Fri, Sep 08, 2017 at 09:01:36AM +0530, Subhransu S. Prusty wrote:
> > >>On Thu, Sep 07, 2017 at 08:48:38PM -0500, Pierre-Louis Bossart wrote:
> > >>>
> > >>>
> > >>>On 09/07/2017 09:29 AM, Subhransu S. Prusty wrote:
> > >>>>From: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi at intel.com>
> > >>>>
> > >>>>Create a platform device and register the clock ops. Clock
> > >>>>prepare/unprepare are used to enable/disable the clock as the IPC will be
> > >>>>sent in non-atomic context. The clk set_dma_control IPC structures are
> > >>>>populated during the set_rate callback and IPC is sent to enable the clock
> > >>>>during prepare callback.
> > >>>>
> > >>>[snip]
> > >>>>+
> > >>>>+static int skl_clk_prepare(void *pvt_data, u32 id, unsigned long rate)
> > >>>>+{
> > >>>>+	struct skl *skl = pvt_data;
> > >>>>+	struct skl_clk_rate_cfg_table *rcfg;
> > >>>>+	int vbus_id, clk_type, ret;
> > >>>>+
> > >>>>+	clk_type = skl_get_clk_type(id);
> > >>>>+	if (clk_type < 0)
> > >>>>+		return -EINVAL;
> > >>>>+
> > >>>>+	ret = skl_get_vbus_id(id, clk_type);
> > >>>>+	if (ret < 0)
> > >>>>+		return ret;
> > >>>>+
> > >>>>+	vbus_id = ret;
> > >>>>+
> > >>>>+	rcfg = skl_get_rate_cfg(skl_ssp_clks[id].rate_cfg, rate);
> > >>>>+	if (!rcfg)
> > >>>>+		return -EINVAL;
> > >>>>+
> > >>>>+	ret = skl_send_clk_dma_control(skl, rcfg, vbus_id, clk_type, true);
> > >>>>+
> > >>>>+	return ret;
> > >>>>+}
> > >>>In this patchset, the clocks are configured from the machine driver,
> > >>>and the enable/disable conveniently placed in
> > >>>platform_clock_control() or hw_params(), where the DSP is most
> > >>>likely active.
> > >>>If you expose a clock, codec driver implementers may want to use
> > >>>them directly instead of relying on a machine driver. A number of
> > >>>existing codecs do use the clk API, so there could be a case where a
> > >>>codec driver calls devm_clk_get and clk_prepare_enable(), without
> > >>>any ability to know what state the DSP is in.
> > >>>What happens then if the DSP is in suspend? Does this force it back
> > >>>to D0? Does the virtual clock driver return an error? Or are you
> > >>>using the clk API with some restrictions on when the clock can be
> > >>>configured?
> > >>
> > >>No, clk enable will not force the DSP to D0. So if the DSP is not active,
> > >>the IPC will timeout and error will be propagated to the caller.
> > >
> > >Or may be it makes sense to enable the runtime pm for clk driver so that it
> > >can activate the DSP. I will check this.
> > 
> > I was thinking of another case: we should not make the assumption
> > that there is always a platform clock control and a hw_params
> > callback, e.g. when an external component seen as a dummy codec
> > needs the mclk/bitclock at all times to drive a second-level set of
> > audio devices. In those cases the machine driver will get/enable the
> > clock at startup and it needs to remain on no matter what the DSP
> > state is. That's probably another case for disabling runtime-pm for
> > as long as the machine driver wants the clock.
> 
> With the series "[PATCH v9 0/5] Add runtime PM support for clocks (on Exynos
> SoC example)", runtime support is added in the common clock framework. This
> is expected to be merged to clk-next after -rc1 drop. 
> 
> Reference: http://www.spinics.net/lists/linux-clk/msg19755.html
> 
> So marking the parent clock with skylake device will help keep the DSP
> active on call to enable clock.

Will fix this in v2.

> 
> Regards,
> Subhransu
> 
> -- 

--