[PATCH 0/6] ASoC: Intel: Skylake: Add a clk driver to enable ssp clks early

[PATCH 0/6] ASoC: Intel: Skylake: Add a clk driver to enable ssp clks early

[Subhransu S. Prusty]

For certain platforms we require to start the clocks (mclk/sclk/fs) before
the stream start. Example: for few systems, codec needs the mclk/sclk/fs to
be enabled early for a successful clock synchronization and for some
others, clock need to be enabled at boot and should be ON always.

By sending set_dma_control IPC (with the i2s blobs queried from NHLT),
these clocks can be enabled early after the firmware is downloaded.

With this series, a virtual clock driver is created which provides
interface to send the required IPCs from machine driver to enable the
clocks. NHLT is parsed during probe and the clock information is populated.
The pointer to blob is cached and sent along with the set_dma_control IPC
structure during the clk prepare/unprepare callback. Clocks are created for
a ssp if the nhlt table has endpoint configuration for that particular ssp.
Skylake driver creates a platform driver with the clock information and
register the clk ops callback.

kabylake machine driver uses the clock interface to enable the clocks early
as it is required by the rt5663 driver for clock synchronization.

set_dma_control API can be used with different payload configuration.
Modify the arguments to take configurable parameters.

Harsha Priya (1):
  ASoC: Intel: kbl: Enable mclk and ssp sclk early

Jaikrishna Nemallapudi (5):
  ASoC: Intel: Skylake: Modify skl_dsp_set_dma_control API arguments
  ASoC: Intel: Skylake: Parse nhlt to populate clock information
  ASoC: Intel: Skylake: Prepare DMA control IPC to enable/disable clock
  ASoC: Intel: Skylake: Register clock device and ops
  ASoC: Intel: Skylake: Add ssp clock driver

 sound/soc/intel/Kconfig                      |   8 +
 sound/soc/intel/boards/kbl_rt5663_max98927.c |  94 ++++++++-
 sound/soc/intel/skylake/Makefile             |   5 +
 sound/soc/intel/skylake/skl-i2s.h            |  56 ++++++
 sound/soc/intel/skylake/skl-messages.c       | 109 +++++++++-
 sound/soc/intel/skylake/skl-nhlt.c           | 148 ++++++++++++++
 sound/soc/intel/skylake/skl-ssp-clk.c        | 287 +++++++++++++++++++++++++++
 sound/soc/intel/skylake/skl-ssp-clk.h        | 124 ++++++++++++
 sound/soc/intel/skylake/skl-topology.h       |   4 +-
 sound/soc/intel/skylake/skl.c                | 235 ++++++++++++++++++++++
 sound/soc/intel/skylake/skl.h                |  12 ++
 11 files changed, 1070 insertions(+), 12 deletions(-)
 create mode 100644 sound/soc/intel/skylake/skl-i2s.h
 create mode 100644 sound/soc/intel/skylake/skl-ssp-clk.c
 create mode 100644 sound/soc/intel/skylake/skl-ssp-clk.h

-- 
1.9.1

[PATCH 1/6] ASoC: Intel: Skylake: Modify skl_dsp_set_dma_control API arguments

[Subhransu S. Prusty]

From: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>

Set dma control ipc can be used to set the M/N divider, enable the clks. It
takes different payload for different configuration. So modify the
skl_dsp_set_dma_control API to take the size and node_id as argument.

Signed-off-by: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
---
 sound/soc/intel/skylake/skl-messages.c | 23 ++++++++++++++---------
 sound/soc/intel/skylake/skl-topology.h |  4 ++--
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index 89f70133c8e4..f637829833e6 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -613,8 +613,10 @@ static void skl_setup_cpr_gateway_cfg(struct skl_sst *ctx,
 }
 
 #define DMA_CONTROL_ID 5
+#define DMA_I2S_BLOB_SIZE 21
 
-int skl_dsp_set_dma_control(struct skl_sst *ctx, struct skl_module_cfg *mconfig)
+int skl_dsp_set_dma_control(struct skl_sst *ctx, u32 *caps,
+				u32 caps_size, u32 node_id)
 {
 	struct skl_dma_control *dma_ctrl;
 	struct skl_ipc_large_config_msg msg = {0};
@@ -624,24 +626,27 @@ int skl_dsp_set_dma_control(struct skl_sst *ctx, struct skl_module_cfg *mconfig)
 	/*
 	 * if blob size zero, then return
 	 */
-	if (mconfig->formats_config.caps_size == 0)
+	if (caps_size == 0)
 		return 0;
 
 	msg.large_param_id = DMA_CONTROL_ID;
-	msg.param_data_size = sizeof(struct skl_dma_control) +
-				mconfig->formats_config.caps_size;
+	msg.param_data_size = sizeof(struct skl_dma_control) + caps_size;
 
 	dma_ctrl = kzalloc(msg.param_data_size, GFP_KERNEL);
 	if (dma_ctrl == NULL)
 		return -ENOMEM;
 
-	dma_ctrl->node_id = skl_get_node_id(ctx, mconfig);
+	dma_ctrl->node_id = node_id;
 
-	/* size in dwords */
-	dma_ctrl->config_length = mconfig->formats_config.caps_size / 4;
+	/*
+	 * NHLT blob may contain additional configs along with i2s blob.
+	 * firmware expects only the i2s blob size as the config_length.
+	 * So fix to i2s blob size.
+	 * size in dwords.
+	 */
+	dma_ctrl->config_length = DMA_I2S_BLOB_SIZE;
 
-	memcpy(dma_ctrl->config_data, mconfig->formats_config.caps,
-				mconfig->formats_config.caps_size);
+	memcpy(dma_ctrl->config_data, caps, caps_size);
 
 	err = skl_ipc_set_large_config(&ctx->ipc, &msg, (u32 *)dma_ctrl);
 
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h
index 2717db92036b..e11cc1fc0e43 100644
--- a/sound/soc/intel/skylake/skl-topology.h
+++ b/sound/soc/intel/skylake/skl-topology.h
@@ -455,8 +455,8 @@ static inline struct skl *get_skl_ctx(struct device *dev)
 
 int skl_tplg_be_update_params(struct snd_soc_dai *dai,
 	struct skl_pipe_params *params);
-int skl_dsp_set_dma_control(struct skl_sst *ctx,
-		struct skl_module_cfg *mconfig);
+int skl_dsp_set_dma_control(struct skl_sst *ctx, u32 *caps,
+			u32 caps_size, u32 node_id);
 void skl_tplg_set_be_dmic_config(struct snd_soc_dai *dai,
 	struct skl_pipe_params *params, int stream);
 int skl_tplg_init(struct snd_soc_platform *platform,
-- 
1.9.1

Applied "ASoC: Intel: Skylake: Modify skl_dsp_set_dma_control API arguments" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: Skylake: Modify skl_dsp_set_dma_control API arguments

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 5514830dffb2332c034c20db3b264ba1f94de1d8 Mon Sep 17 00:00:00 2001
From: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
Date: Mon, 18 Sep 2017 10:26:44 +0530
Subject: [PATCH] ASoC: Intel: Skylake: Modify skl_dsp_set_dma_control API
 arguments

Set dma control ipc can be used to set the M/N divider, enable the clks. It
takes different payload for different configuration. So modify the
skl_dsp_set_dma_control API to take the size and node_id as argument.

Signed-off-by: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Acked-By: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/skylake/skl-messages.c | 23 ++++++++++++++---------
 sound/soc/intel/skylake/skl-topology.h |  4 ++--
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index 89f70133c8e4..f637829833e6 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -613,8 +613,10 @@ static void skl_setup_cpr_gateway_cfg(struct skl_sst *ctx,
 }
 
 #define DMA_CONTROL_ID 5
+#define DMA_I2S_BLOB_SIZE 21
 
-int skl_dsp_set_dma_control(struct skl_sst *ctx, struct skl_module_cfg *mconfig)
+int skl_dsp_set_dma_control(struct skl_sst *ctx, u32 *caps,
+				u32 caps_size, u32 node_id)
 {
 	struct skl_dma_control *dma_ctrl;
 	struct skl_ipc_large_config_msg msg = {0};
@@ -624,24 +626,27 @@ int skl_dsp_set_dma_control(struct skl_sst *ctx, struct skl_module_cfg *mconfig)
 	/*
 	 * if blob size zero, then return
 	 */
-	if (mconfig->formats_config.caps_size == 0)
+	if (caps_size == 0)
 		return 0;
 
 	msg.large_param_id = DMA_CONTROL_ID;
-	msg.param_data_size = sizeof(struct skl_dma_control) +
-				mconfig->formats_config.caps_size;
+	msg.param_data_size = sizeof(struct skl_dma_control) + caps_size;
 
 	dma_ctrl = kzalloc(msg.param_data_size, GFP_KERNEL);
 	if (dma_ctrl == NULL)
 		return -ENOMEM;
 
-	dma_ctrl->node_id = skl_get_node_id(ctx, mconfig);
+	dma_ctrl->node_id = node_id;
 
-	/* size in dwords */
-	dma_ctrl->config_length = mconfig->formats_config.caps_size / 4;
+	/*
+	 * NHLT blob may contain additional configs along with i2s blob.
+	 * firmware expects only the i2s blob size as the config_length.
+	 * So fix to i2s blob size.
+	 * size in dwords.
+	 */
+	dma_ctrl->config_length = DMA_I2S_BLOB_SIZE;
 
-	memcpy(dma_ctrl->config_data, mconfig->formats_config.caps,
-				mconfig->formats_config.caps_size);
+	memcpy(dma_ctrl->config_data, caps, caps_size);
 
 	err = skl_ipc_set_large_config(&ctx->ipc, &msg, (u32 *)dma_ctrl);
 
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h
index 2717db92036b..e11cc1fc0e43 100644
--- a/sound/soc/intel/skylake/skl-topology.h
+++ b/sound/soc/intel/skylake/skl-topology.h
@@ -455,8 +455,8 @@ static inline struct skl *get_skl_ctx(struct device *dev)
 
 int skl_tplg_be_update_params(struct snd_soc_dai *dai,
 	struct skl_pipe_params *params);
-int skl_dsp_set_dma_control(struct skl_sst *ctx,
-		struct skl_module_cfg *mconfig);
+int skl_dsp_set_dma_control(struct skl_sst *ctx, u32 *caps,
+			u32 caps_size, u32 node_id);
 void skl_tplg_set_be_dmic_config(struct snd_soc_dai *dai,
 	struct skl_pipe_params *params, int stream);
 int skl_tplg_init(struct snd_soc_platform *platform,
-- 
2.14.1

[PATCH 2/6] ASoC: Intel: Skylake: Parse nhlt to populate clock information

[Subhransu S. Prusty]

From: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>

NHLT is parsed to fill the SSP clock information. If an endpoint is present
for a SSP, then the corresponding SSP clocks are created.

MCLK is consistent across endpoints and configuration for an SSP. So NHLT
is queried for the first endpoint for an SSP and MCLK information for that
SSP is populated.

For SCLK/SCLKFS, the best fit is queried from the NHLT configurations which
matches the clock rate requested. Best fit is decided based on below:

1. If rate matches with multiple configurations, then the first
configuration is selected.

2. If for a selected fs and bits_per_sample, there are multiple endpoint
configuration match, then the configuration with max number of channels is
selected. So, the user has to set the rate which fits max number of
channels

Max of 18 clocks created, 3 for each SSP and there are 6 SSPs. There are
also 3 parent clocks XTAL, Cardinal and PLL. This patch populates the all
the clock information.

Cardinal/PLL are fixed for all platforms. For XTAL, APL/GLK uses 19.2MHz,
whereas for SKL/APL it is 24MHz.

Signed-off-by: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
---
 sound/soc/intel/skylake/skl-i2s.h     |  56 +++++++++++++
 sound/soc/intel/skylake/skl-nhlt.c    | 148 ++++++++++++++++++++++++++++++++++
 sound/soc/intel/skylake/skl-ssp-clk.h |  86 ++++++++++++++++++++
 sound/soc/intel/skylake/skl.c         |  71 ++++++++++++++++
 sound/soc/intel/skylake/skl.h         |   3 +
 5 files changed, 364 insertions(+)
 create mode 100644 sound/soc/intel/skylake/skl-i2s.h
 create mode 100644 sound/soc/intel/skylake/skl-ssp-clk.h

diff --git a/sound/soc/intel/skylake/skl-i2s.h b/sound/soc/intel/skylake/skl-i2s.h
new file mode 100644
index 000000000000..378d17adde9d
--- /dev/null
+++ b/sound/soc/intel/skylake/skl-i2s.h
@@ -0,0 +1,56 @@
+/*
+ *  skl-i2s.h - i2s blob mapping
+ *
+ *  Copyright (C) 2017 Intel Corp
+ *  Author: Subhransu S. Prusty < subhransu.s.prusty@intel.com>
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ */
+
+#ifndef __SOUND_SOC_SKL_I2S_H
+#define __SOUND_SOC_SKL_I2S_H
+
+#define SKL_I2S_MAX_TIME_SLOTS		8
+#define SKL_MCLK_DIV_CLK_SRC_MASK	GENMASK(17, 16)
+#define SKL_MCLK_DIV_CLK_SRC_SHIFT	16
+
+#define SKL_MNDSS_DIV_CLK_SRC_MASK	GENMASK(21, 20)
+#define SKL_MNDSS_DIV_CLK_SRC_SHIFT	20
+#define SKL_MCLK_DIV_RATIO_MASK		GENMASK(11, 0)
+
+struct skl_i2s_config {
+	u32 ssc0;
+	u32 ssc1;
+	u32 sscto;
+	u32 sspsp;
+	u32 sstsa;
+	u32 ssrsa;
+	u32 ssc2;
+	u32 sspsp2;
+	u32 ssc3;
+	u32 ssioc;
+} __packed;
+
+struct skl_i2s_config_mclk {
+	u32 mdivctrl;
+	u32 mdivr;
+} __packed;
+
+struct skl_i2s_config_blob_legacy {
+	u32 gtw_attr;
+	u32 tdm_ts_group[SKL_I2S_MAX_TIME_SLOTS];
+	struct skl_i2s_config i2s_cfg;
+	struct skl_i2s_config_mclk mclk;
+};
+
+#endif /* __SOUND_SOC_SKL_I2S_H */
diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c
index e7d766d56c8e..a6bf7cd01ddf 100644
--- a/sound/soc/intel/skylake/skl-nhlt.c
+++ b/sound/soc/intel/skylake/skl-nhlt.c
@@ -19,6 +19,7 @@
  */
 #include <linux/pci.h>
 #include "skl.h"
+#include "skl-i2s.h"
 
 /* Unique identification for getting NHLT blobs */
 static guid_t osc_guid =
@@ -262,3 +263,150 @@ void skl_nhlt_remove_sysfs(struct skl *skl)
 
 	sysfs_remove_file(&dev->kobj, &dev_attr_platform_id.attr);
 }
+
+/*
+ * Queries NHLT for all the fmt configuration for a particular endpoint and
+ * stores all possible rates supported in a rate table for the corresponding
+ * sclk/sclkfs.
+ */
+void skl_get_ssp_clks(struct skl *skl, struct skl_ssp_clk *ssp_clks,
+				struct nhlt_fmt *fmt, u8 id)
+{
+	struct nhlt_fmt_cfg *fmt_cfg;
+	struct skl_i2s_config_blob_legacy *i2s_config;
+	struct wav_fmt_ext *wav_fmt;
+	struct skl_clk_parent_src *parent;
+	struct skl_ssp_clk *sclk, *sclkfs;
+	unsigned long rate = 0;
+	u16 channels, bps;
+	u32 fs;
+	u8 clk_src;
+	int rate_index = 0;
+	int i, j;
+	bool present = false;
+
+	sclk = &ssp_clks[SKL_SCLK_OFS];
+	sclkfs = &ssp_clks[SKL_SCLKFS_OFS];
+
+	if (fmt->fmt_count == 0)
+		return;
+
+	for (i = 0; i < fmt->fmt_count; i++) {
+		fmt_cfg = &fmt->fmt_config[i];
+		wav_fmt = &fmt_cfg->fmt_ext;
+
+		channels = wav_fmt->fmt.channels;
+		bps = wav_fmt->fmt.bits_per_sample;
+		fs = wav_fmt->fmt.samples_per_sec;
+
+		/*
+		 * In case of TDM configuration on the same ssp, there can
+		 * be more than one blob in which channel masks are
+		 * different for each usecase for a specific rate and bps.
+		 * But the sclk rate will be based on the total number of
+		 * channels used for that endpoint.
+		 *
+		 * So there is a blob which represent the superset of all
+		 * the channels used for that endpoint. Select the rate that
+		 * matches with the blob which has the number of channels is
+		 * equall to all channel for that endpoint.
+		 */
+		for (j = i; j < fmt->fmt_count; j++) {
+			fmt_cfg = &fmt->fmt_config[j];
+			wav_fmt = &fmt_cfg->fmt_ext;
+			if ((fs == wav_fmt->fmt.samples_per_sec) &&
+					(bps == wav_fmt->fmt.bits_per_sample) &&
+					(channels < wav_fmt->fmt.channels)) {
+
+				channels = wav_fmt->fmt.channels;
+			}
+		}
+
+		rate = channels * bps * fs;
+
+		/* check if the rate is already present */
+		for (j = 0; (sclk[id].rate_cfg[j].rate != 0) &&
+				(j < SKL_MAX_CLK_RATES); j++) {
+			if (sclk[id].rate_cfg[j].rate == rate) {
+				present = true;
+				break;
+			}
+		}
+
+		/* Fill rate and parent for sclk/sclkfs */
+		if (!present) {
+			sclk[id].rate_cfg[rate_index].rate = rate;
+			sclk[id].rate_cfg[rate_index].config = fmt_cfg;
+			sclkfs[id].rate_cfg[rate_index].rate = rate;
+			sclkfs[id].rate_cfg[rate_index].config = fmt_cfg;
+
+			/* MCLK Divider Source Select */
+			i2s_config = (struct skl_i2s_config_blob_legacy *)
+						fmt->fmt_config[0].config.caps;
+			clk_src = ((i2s_config->mclk.mdivctrl)
+					& SKL_MNDSS_DIV_CLK_SRC_MASK) >>
+					SKL_MNDSS_DIV_CLK_SRC_SHIFT;
+
+			parent = skl_get_parent_clk(clk_src);
+			sclk[id].parent_name = parent->name;
+			sclkfs[id].parent_name = parent->name;
+
+			rate_index++;
+		}
+	}
+}
+
+void skl_get_mclk(struct skl *skl, struct skl_ssp_clk *mclk,
+				struct nhlt_fmt *fmt, u8 id)
+{
+	struct nhlt_specific_cfg *fmt_cfg;
+	struct skl_i2s_config_blob_legacy *i2s_config;
+	struct skl_clk_parent_src *parent;
+	u32 clkdiv, div_ratio;
+	u8 clk_src;
+
+	fmt_cfg = &fmt->fmt_config[0].config;
+	i2s_config = (struct skl_i2s_config_blob_legacy *)fmt_cfg->caps;
+
+	/* MCLK Divider Source Select */
+	clk_src = ((i2s_config->mclk.mdivctrl) & SKL_MCLK_DIV_CLK_SRC_MASK) >>
+						SKL_MCLK_DIV_CLK_SRC_SHIFT;
+
+
+	clkdiv = i2s_config->mclk.mdivr & SKL_MCLK_DIV_RATIO_MASK;
+
+	if (clkdiv == 0xfff) /* bypass divider */
+		div_ratio = 1;
+	else
+		/* Divider is 2 + clkdiv */
+		div_ratio = clkdiv + 2;
+
+	/* Calculate MCLK rate from source using div value */
+	parent = skl_get_parent_clk(clk_src);
+	mclk[id].rate_cfg[0].rate = parent->rate/div_ratio;
+	mclk[id].rate_cfg[0].config = &fmt->fmt_config[0];
+	mclk[id].parent_name = parent->name;
+}
+
+void skl_get_clks(struct skl *skl, struct skl_ssp_clk *ssp_clks)
+{
+	struct nhlt_acpi_table *nhlt = (struct nhlt_acpi_table *)skl->nhlt;
+	struct nhlt_endpoint *epnt;
+	struct nhlt_fmt *fmt;
+	int i;
+	u8 id;
+
+	epnt = (struct nhlt_endpoint *)nhlt->desc;
+	for (i = 0; i < nhlt->endpoint_count; i++) {
+		if (epnt->linktype == NHLT_LINK_SSP) {
+			id = epnt->virtual_bus_id;
+
+			fmt = (struct nhlt_fmt *)(epnt->config.caps
+					+ epnt->config.size);
+
+			skl_get_ssp_clks(skl, ssp_clks, fmt, id);
+			skl_get_mclk(skl, ssp_clks, fmt, id);
+		}
+		epnt = (struct nhlt_endpoint *)((u8 *)epnt + epnt->length);
+	}
+}
diff --git a/sound/soc/intel/skylake/skl-ssp-clk.h b/sound/soc/intel/skylake/skl-ssp-clk.h
new file mode 100644
index 000000000000..9d98f458f08a
--- /dev/null
+++ b/sound/soc/intel/skylake/skl-ssp-clk.h
@@ -0,0 +1,86 @@
+/*
+ *  skl-ssp-clk.h - Skylake ssp clock information and ipc structure
+ *
+ *  Copyright (C) 2017 Intel Corp
+ *  Author: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
+ *  Author: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ */
+
+#ifndef SOUND_SOC_SKL_SSP_CLK_H
+#define SOUND_SOC_SKL_SSP_CLK_H
+
+#define SKL_MAX_SSP		6
+#define SKL_MAX_CLK_SRC		3 /* xtal/cardinal/pll, parent of ssp clocks and mclk */
+#define SKL_MAX_SSP_CLK_TYPES	3 /* mclk, sclk, sclkfs */
+
+#define SKL_MAX_CLK_CNT		(SKL_MAX_SSP * SKL_MAX_SSP_CLK_TYPES)
+
+/* Max number of configurations supported for each clock */
+#define SKL_MAX_CLK_RATES	10
+
+#define SKL_SCLK_OFS		SKL_MAX_SSP
+#define SKL_SCLKFS_OFS		(SKL_SCLK_OFS + SKL_MAX_SSP)
+
+enum skl_clk_type {
+	SKL_MCLK,
+	SKL_SCLK,
+	SKL_SCLK_FS,
+};
+
+enum skl_clk_src_type {
+	SKL_XTAL,
+	SKL_CARDINAL,
+	SKL_PLL,
+};
+
+struct skl_clk_parent_src {
+	u8 clk_id;
+	const char *name;
+	unsigned long rate;
+	const char *parent_name;
+};
+
+struct skl_clk_rate_cfg_table {
+	unsigned long rate;
+	void *config;
+};
+
+/*
+ * rate for mclk will be in rates[0]. For sclk and sclkfs, rates[] store
+ * all possible clocks ssp can generate for that platform.
+ */
+struct skl_ssp_clk {
+	const char *name;
+	const char *parent_name;
+	struct skl_clk_rate_cfg_table rate_cfg[SKL_MAX_CLK_RATES];
+};
+
+struct skl_clk_ops {
+	int (*prepare)(void *pvt_data, u32 id, unsigned long rate);
+	int (*unprepare)(void *pvt_data, u32 id, unsigned long rate);
+	int (*set_rate)(u32 id, unsigned long rate);
+	unsigned long (*recalc_rate)(u32 id, unsigned long parent_rate);
+};
+
+struct skl_clk_pdata {
+	struct skl_clk_parent_src *parent_clks;
+	int num_clks;
+	struct skl_ssp_clk *ssp_clks;
+	struct skl_clk_ops *ops;
+	void *pvt_data;
+};
+
+#endif /* SOUND_SOC_SKL_SSP_CLK_H */
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index f94b484abb99..440caf1ced43 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -435,6 +435,23 @@ static int skl_free(struct hdac_ext_bus *ebus)
 	return 0;
 }
 
+/*
+ * For each ssp there are 3 clocks (mclk/sclk/sclkfs).
+ * e.g. for ssp0, clocks will be named as
+ *      "ssp0_mclk", "ssp0_sclk", "ssp0_sclkfs"
+ * So for skl+, there are 6 ssps, so 18 clocks will be created.
+ */
+static struct skl_ssp_clk skl_ssp_clks[] = {
+	{.name = "ssp0_mclk"}, {.name = "ssp1_mclk"}, {.name = "ssp2_mclk"},
+	{.name = "ssp3_mclk"}, {.name = "ssp4_mclk"}, {.name = "ssp5_mclk"},
+	{.name = "ssp0_sclk"}, {.name = "ssp1_sclk"}, {.name = "ssp2_sclk"},
+	{.name = "ssp3_sclk"}, {.name = "ssp4_sclk"}, {.name = "ssp5_sclk"},
+	{.name = "ssp0_sclkfs"}, {.name = "ssp1_sclkfs"},
+						{.name = "ssp2_sclkfs"},
+	{.name = "ssp3_sclkfs"}, {.name = "ssp4_sclkfs"},
+						{.name = "ssp5_sclkfs"},
+};
+
 static int skl_machine_device_register(struct skl *skl, void *driver_data)
 {
 	struct hdac_bus *bus = ebus_to_hbus(&skl->ebus);
@@ -506,6 +523,60 @@ static void skl_dmic_device_unregister(struct skl *skl)
 		platform_device_unregister(skl->dmic_dev);
 }
 
+static struct skl_clk_parent_src skl_clk_src[] = {
+	{ .clk_id = SKL_XTAL, .name = "xtal" },
+	{ .clk_id = SKL_CARDINAL, .name = "cardinal", .rate = 24576000 },
+	{ .clk_id = SKL_PLL, .name = "pll", .rate = 96000000 },
+};
+
+struct skl_clk_parent_src *skl_get_parent_clk(u8 clk_id)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(skl_clk_src); i++) {
+		if (skl_clk_src[i].clk_id == clk_id)
+			return &skl_clk_src[i];
+	}
+
+	return NULL;
+}
+
+void init_skl_xtal_rate(int pci_id)
+{
+	switch (pci_id) {
+	case 0x9d70:
+	case 0x9d71:
+		skl_clk_src[0].rate = 24000000;
+		return;
+
+	default:
+		skl_clk_src[0].rate = 19200000;
+		return;
+	}
+}
+
+static int skl_clock_device_register(struct skl *skl)
+{
+	struct skl_clk_pdata *clk_pdata;
+
+
+	clk_pdata = devm_kzalloc(&skl->pci->dev, sizeof(*clk_pdata),
+							GFP_KERNEL);
+	if (!clk_pdata)
+		return -ENOMEM;
+
+	init_skl_xtal_rate(skl->pci->device);
+
+	clk_pdata->parent_clks = skl_clk_src;
+	clk_pdata->ssp_clks = skl_ssp_clks;
+	clk_pdata->num_clks = ARRAY_SIZE(skl_ssp_clks);
+
+	/* Query NHLT to fill the rates and parent */
+	skl_get_clks(skl, clk_pdata->ssp_clks);
+
+	return 0;
+}
+
 /*
  * Probe the given codec address
  */
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index 8d9d6899f761..ff75de370c45 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -25,6 +25,7 @@
 #include <sound/hdaudio_ext.h>
 #include <sound/soc.h>
 #include "skl-nhlt.h"
+#include "skl-ssp-clk.h"
 
 #define SKL_SUSPEND_DELAY 2000
 
@@ -123,6 +124,8 @@ struct nhlt_specific_cfg *skl_get_ep_blob(struct skl *skl, u32 instance,
 void skl_update_d0i3c(struct device *dev, bool enable);
 int skl_nhlt_create_sysfs(struct skl *skl);
 void skl_nhlt_remove_sysfs(struct skl *skl);
+void skl_get_clks(struct skl *skl, struct skl_ssp_clk *ssp_clks);
+struct skl_clk_parent_src *skl_get_parent_clk(u8 clk_id);
 
 struct skl_module_cfg;
 
-- 
1.9.1

[PATCH 3/6] ASoC: Intel: Skylake: Prepare DMA control IPC to enable/disable clock

[Subhransu S. Prusty]

From: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>

Add the required structures and create set_dma_control ipc to enable or
disable the clock. To enable sclk without fs, mclk ipc structure is used,
else sclkfs ipc structure is used.

Signed-off-by: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
---
 sound/soc/intel/skylake/skl-messages.c | 86 ++++++++++++++++++++++++++++++++++
 sound/soc/intel/skylake/skl-ssp-clk.h  | 38 +++++++++++++++
 sound/soc/intel/skylake/skl.h          |  8 ++++
 3 files changed, 132 insertions(+)

diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index f637829833e6..089ea3c03846 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -1371,3 +1371,89 @@ int skl_get_module_params(struct skl_sst *ctx, u32 *params, int size,
 
 	return skl_ipc_get_large_config(&ctx->ipc, &msg, params);
 }
+
+void skl_fill_clk_ipc(struct skl_clk_rate_cfg_table *rcfg, u8 clk_type)
+{
+	struct nhlt_fmt_cfg *fmt_cfg;
+	struct wav_fmt *wfmt;
+	union skl_clk_ctrl_ipc *ipc;
+
+	if (!rcfg)
+		return;
+
+	ipc = &rcfg->dma_ctl_ipc;
+	if (clk_type == SKL_SCLK_FS) {
+		fmt_cfg = (struct nhlt_fmt_cfg *)rcfg->config;
+		wfmt = &fmt_cfg->fmt_ext.fmt;
+
+		/* Remove TLV Header size */
+		ipc->sclk_fs.hdr.size = sizeof(struct skl_dmactrl_sclkfs_cfg) -
+						sizeof(struct skl_tlv_hdr);
+		ipc->sclk_fs.sampling_frequency = wfmt->samples_per_sec;
+		ipc->sclk_fs.bit_depth = wfmt->bits_per_sample;
+		ipc->sclk_fs.valid_bit_depth =
+			fmt_cfg->fmt_ext.sample.valid_bits_per_sample;
+		ipc->sclk_fs.number_of_channels = wfmt->channels;
+	} else {
+		ipc->mclk.hdr.type = DMA_CLK_CONTROLS;
+		/* Remove TLV Header size */
+		ipc->mclk.hdr.size = sizeof(struct skl_dmactrl_mclk_cfg) -
+						sizeof(struct skl_tlv_hdr);
+	}
+}
+
+/* Sends dma control IPC to turn the clock ON/OFF */
+int skl_send_clk_dma_control(struct skl *skl, struct skl_clk_rate_cfg_table
+				*rcfg, u32 vbus_id, u8 clk_type, bool enable)
+{
+	struct nhlt_fmt_cfg *fmt_cfg;
+	struct nhlt_specific_cfg *sp_cfg;
+	union skl_clk_ctrl_ipc *ipc;
+	void *i2s_config = NULL;
+	u8 *data, size;
+	u32 i2s_config_size, node_id = 0;
+	int ret;
+
+	if (!rcfg)
+		return -EIO;
+
+	ipc = &rcfg->dma_ctl_ipc;
+	fmt_cfg = (struct nhlt_fmt_cfg *)rcfg->config;
+	sp_cfg = &fmt_cfg->config;
+	if (clk_type == SKL_SCLK_FS) {
+		ipc->sclk_fs.hdr.type =
+			enable ? DMA_TRANSMITION_START : DMA_TRANSMITION_STOP;
+		data = (u8 *)&ipc->sclk_fs;
+		size = sizeof(struct skl_dmactrl_sclkfs_cfg);
+	} else {
+		/* 1 to enable mclk, 0 to enable sclk */
+		if (clk_type == SKL_SCLK)
+			ipc->mclk.mclk = 0;
+		else
+			ipc->mclk.mclk = 1;
+
+		ipc->mclk.keep_running = enable;
+		ipc->mclk.warm_up_over = enable;
+		ipc->mclk.clk_stop_over = !enable;
+		data = (u8 *)&ipc->mclk;
+		size = sizeof(struct skl_dmactrl_mclk_cfg);
+	}
+
+	i2s_config_size = sp_cfg->size + size;
+	i2s_config = kzalloc(i2s_config_size, GFP_KERNEL);
+	if (!i2s_config)
+		return -ENOMEM;
+
+	/* copy blob */
+	memcpy(i2s_config, sp_cfg->caps, sp_cfg->size);
+
+	/* copy additional dma controls information */
+	memcpy(i2s_config + sp_cfg->size, data, size);
+
+	node_id = ((SKL_DMA_I2S_LINK_INPUT_CLASS << 8) | (vbus_id << 4));
+	ret = skl_dsp_set_dma_control(skl->skl_sst, (u32 *)i2s_config,
+					i2s_config_size, node_id);
+	kfree(i2s_config);
+
+	return ret;
+}
diff --git a/sound/soc/intel/skylake/skl-ssp-clk.h b/sound/soc/intel/skylake/skl-ssp-clk.h
index 9d98f458f08a..150519dbcd90 100644
--- a/sound/soc/intel/skylake/skl-ssp-clk.h
+++ b/sound/soc/intel/skylake/skl-ssp-clk.h
@@ -53,8 +53,46 @@ struct skl_clk_parent_src {
 	const char *parent_name;
 };
 
+struct skl_tlv_hdr {
+	u32 type;
+	u32 size;
+};
+
+struct skl_dmactrl_mclk_cfg {
+	struct skl_tlv_hdr hdr;
+	/* DMA Clk TLV params */
+	u32 clk_warm_up:16;
+	u32 mclk:1;
+	u32 warm_up_over:1;
+	u32 rsvd0:14;
+	u32 clk_stop_delay:16;
+	u32 keep_running:1;
+	u32 clk_stop_over:1;
+	u32 rsvd1:14;
+} __packed;
+
+struct skl_dmactrl_sclkfs_cfg {
+	struct skl_tlv_hdr hdr;
+	/* DMA SClk&FS  TLV params */
+	u32 sampling_frequency;
+	u32 bit_depth;
+	u32 channel_map;
+	u32 channel_config;
+	u32 interleaving_style;
+	u32 number_of_channels : 8;
+	u32 valid_bit_depth : 8;
+	u32 sample_type : 8;
+	u32 reserved : 8;
+} __packed;
+
+union skl_clk_ctrl_ipc {
+	struct skl_dmactrl_mclk_cfg mclk;
+	struct skl_dmactrl_sclkfs_cfg sclk_fs;
+};
+
 struct skl_clk_rate_cfg_table {
 	unsigned long rate;
+	union skl_clk_ctrl_ipc dma_ctl_ipc;
 	void *config;
 };
 
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index ff75de370c45..f06e98962a0b 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -36,6 +36,10 @@
 /* D0I3C Register fields */
 #define AZX_REG_VS_D0I3C_CIP      0x1 /* Command in progress */
 #define AZX_REG_VS_D0I3C_I3       0x4 /* D0i3 enable */
+#define SKL_MAX_DMACTRL_CFG	18
+#define DMA_CLK_CONTROLS	1
+#define DMA_TRANSMITION_START	2
+#define DMA_TRANSMITION_STOP	3
 
 struct skl_dsp_resource {
 	u32 max_mcps;
@@ -124,6 +128,10 @@ struct nhlt_specific_cfg *skl_get_ep_blob(struct skl *skl, u32 instance,
 void skl_update_d0i3c(struct device *dev, bool enable);
 int skl_nhlt_create_sysfs(struct skl *skl);
 void skl_nhlt_remove_sysfs(struct skl *skl);
+void skl_fill_clk_ipc(struct skl_clk_rate_cfg_table *rcfg, u8 clk_type);
+int skl_send_clk_dma_control(struct skl *skl,
+		struct skl_clk_rate_cfg_table *rcfg,
+		u32 vbus_id, u8 clk_type, bool enable);
 void skl_get_clks(struct skl *skl, struct skl_ssp_clk *ssp_clks);
 struct skl_clk_parent_src *skl_get_parent_clk(u8 clk_id);
 
-- 
1.9.1

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

[Subhransu S. Prusty]

From: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@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.

Signed-off-by: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
---
 sound/soc/intel/skylake/skl.c | 166 +++++++++++++++++++++++++++++++++++++++++-
 sound/soc/intel/skylake/skl.h |   1 +
 2 files changed, 166 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 440caf1ced43..dc1a35ec1399 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -452,6 +452,133 @@ static int skl_free(struct hdac_ext_bus *ebus)
 						{.name = "ssp5_sclkfs"},
 };
 
+static enum skl_clk_type skl_get_clk_type(u32 index)
+{
+	switch (index) {
+	case 0 ... (SKL_SCLK_OFS - 1):
+		return SKL_MCLK;
+
+	case SKL_SCLK_OFS ... (SKL_SCLKFS_OFS - 1):
+		return SKL_SCLK;
+
+	case SKL_SCLKFS_OFS ... (SKL_MAX_CLK_CNT - 1):
+		return SKL_SCLK_FS;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int skl_get_vbus_id(u32 index, u8 clk_type)
+{
+	switch (clk_type) {
+	case SKL_MCLK:
+		return index;
+
+	case SKL_SCLK:
+		return index - SKL_SCLK_OFS;
+
+	case SKL_SCLK_FS:
+		return index - SKL_SCLKFS_OFS;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static struct skl_clk_rate_cfg_table *skl_get_rate_cfg(
+		struct skl_clk_rate_cfg_table *rcfg,
+				unsigned long rate)
+{
+	int i;
+
+	for (i = 0; (i < SKL_MAX_CLK_RATES) && rcfg[i].rate; i++) {
+		if (rcfg[i].rate == rate)
+			return &rcfg[i];
+	}
+
+	return NULL;
+}
+
+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;
+}
+
+static int skl_clk_unprepare(void *pvt_data, u32 id, unsigned long rate)
+{
+	struct skl *skl = pvt_data;
+	struct skl_clk_rate_cfg_table *rcfg;
+	int vbus_id, ret;
+	u8 clk_type;
+
+	clk_type = skl_get_clk_type(id);
+	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;
+
+	return skl_send_clk_dma_control(skl, rcfg, vbus_id, clk_type, false);
+}
+
+static int skl_clk_set_rate(u32 id, unsigned long rate)
+{
+	struct skl_clk_rate_cfg_table *rcfg;
+	u8 clk_type;
+
+	if (!rate)
+		return -EINVAL;
+
+	clk_type = skl_get_clk_type(id);
+	rcfg = skl_get_rate_cfg(skl_ssp_clks[id].rate_cfg, rate);
+	if (!rcfg)
+		return -EINVAL;
+
+	skl_fill_clk_ipc(rcfg, clk_type);
+
+	return 0;
+}
+
+unsigned long skl_clk_recalc_rate(u32 id, unsigned long parent_rate)
+{
+	struct skl_clk_rate_cfg_table *rcfg;
+	u8 clk_type;
+
+	clk_type = skl_get_clk_type(id);
+	rcfg = skl_get_rate_cfg(skl_ssp_clks[id].rate_cfg, parent_rate);
+	if (!rcfg)
+		return 0;
+
+	skl_fill_clk_ipc(rcfg, clk_type);
+
+	return rcfg->rate;
+}
+
 static int skl_machine_device_register(struct skl *skl, void *driver_data)
 {
 	struct hdac_bus *bus = ebus_to_hbus(&skl->ebus);
@@ -555,10 +682,21 @@ void init_skl_xtal_rate(int pci_id)
 	}
 }
 
+/*
+ * prepare/unprepare are used instead of enable/disable as IPC will be sent
+ * in non-atomic context.
+ */
+static struct skl_clk_ops clk_ops = {
+	.prepare = skl_clk_prepare,
+	.unprepare = skl_clk_unprepare,
+	.set_rate = skl_clk_set_rate,
+	.recalc_rate = skl_clk_recalc_rate,
+};
+
 static int skl_clock_device_register(struct skl *skl)
 {
 	struct skl_clk_pdata *clk_pdata;
-
+	struct platform_device_info pdevinfo = {NULL};
 
 	clk_pdata = devm_kzalloc(&skl->pci->dev, sizeof(*clk_pdata),
 							GFP_KERNEL);
@@ -573,10 +711,28 @@ static int skl_clock_device_register(struct skl *skl)
 
 	/* Query NHLT to fill the rates and parent */
 	skl_get_clks(skl, clk_pdata->ssp_clks);
+	clk_pdata->ops = &clk_ops;
+	clk_pdata->pvt_data = skl;
+
+	/* Register Platform device */
+	pdevinfo.parent = &skl->pci->dev;
+	pdevinfo.id = -1;
+	pdevinfo.name = "skl-ssp-clk";
+	pdevinfo.data = clk_pdata;
+	pdevinfo.size_data = sizeof(*clk_pdata);
+	skl->clk_dev = platform_device_register_full(&pdevinfo);
+	if (IS_ERR(skl->clk_dev))
+		return PTR_ERR(skl->clk_dev);
 
 	return 0;
 }
 
+static void skl_clock_device_unregister(struct skl *skl)
+{
+	if (skl->clk_dev)
+		platform_device_unregister(skl->clk_dev);
+}
+
 /*
  * Probe the given codec address
  */
@@ -859,6 +1015,11 @@ static int skl_probe(struct pci_dev *pci,
 
 	/* check if dsp is there */
 	if (bus->ppcap) {
+		/* create device for dsp clk */
+		err = skl_clock_device_register(skl);
+		if (err < 0)
+			goto out_clk_free;
+
 		err = skl_machine_device_register(skl,
 				  (void *)pci_id->driver_data);
 		if (err < 0)
@@ -890,6 +1051,8 @@ static int skl_probe(struct pci_dev *pci,
 	skl_free_dsp(skl);
 out_mach_free:
 	skl_machine_device_unregister(skl);
+out_clk_free:
+	skl_clock_device_unregister(skl);
 out_nhlt_free:
 	skl_nhlt_free(skl->nhlt);
 out_free:
@@ -940,6 +1103,7 @@ static void skl_remove(struct pci_dev *pci)
 	skl_free_dsp(skl);
 	skl_machine_device_unregister(skl);
 	skl_dmic_device_unregister(skl);
+	skl_clock_device_unregister(skl);
 	skl_nhlt_remove_sysfs(skl);
 	skl_nhlt_free(skl->nhlt);
 	skl_free(ebus);
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index f06e98962a0b..df0fcf1bfe40 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -57,6 +57,7 @@ struct skl {
 	unsigned int init_done:1; /* delayed init status */
 	struct platform_device *dmic_dev;
 	struct platform_device *i2s_dev;
+	struct platform_device *clk_dev;
 	struct snd_soc_platform *platform;
 
 	struct nhlt_acpi_table *nhlt; /* nhlt ptr */
-- 
1.9.1

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

[Pierre-Louis Bossart]

On 09/07/2017 09:29 AM, Subhransu S. Prusty wrote:
> From: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@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?

Note that I am not against this idea, it's fine, but I'd like more 
clarity on the assumptions. Thanks!

> +
> +static int skl_clk_unprepare(void *pvt_data, u32 id, unsigned long rate)
> +{
> +	struct skl *skl = pvt_data;
> +	struct skl_clk_rate_cfg_table *rcfg;
> +	int vbus_id, ret;
> +	u8 clk_type;
> +
> +	clk_type = skl_get_clk_type(id);
> +	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;
> +
> +	return skl_send_clk_dma_control(skl, rcfg, vbus_id, clk_type, false);
> +}
> +
> +static int skl_clk_set_rate(u32 id, unsigned long rate)
> +{
> +	struct skl_clk_rate_cfg_table *rcfg;
> +	u8 clk_type;
> +
> +	if (!rate)
> +		return -EINVAL;
> +
> +	clk_type = skl_get_clk_type(id);
> +	rcfg = skl_get_rate_cfg(skl_ssp_clks[id].rate_cfg, rate);
> +	if (!rcfg)
> +		return -EINVAL;
> +
> +	skl_fill_clk_ipc(rcfg, clk_type);
> +
> +	return 0;
> +}
> +
> +unsigned long skl_clk_recalc_rate(u32 id, unsigned long parent_rate)
> +{
> +	struct skl_clk_rate_cfg_table *rcfg;
> +	u8 clk_type;
> +
> +	clk_type = skl_get_clk_type(id);
> +	rcfg = skl_get_rate_cfg(skl_ssp_clks[id].rate_cfg, parent_rate);
> +	if (!rcfg)
> +		return 0;
> +
> +	skl_fill_clk_ipc(rcfg, clk_type);
> +
> +	return rcfg->rate;
> +}
> +
>   static int skl_machine_device_register(struct skl *skl, void *driver_data)
>   {
>   	struct hdac_bus *bus = ebus_to_hbus(&skl->ebus);
> @@ -555,10 +682,21 @@ void init_skl_xtal_rate(int pci_id)
>   	}
>   }
>   
> +/*
> + * prepare/unprepare are used instead of enable/disable as IPC will be sent
> + * in non-atomic context.
> + */
> +static struct skl_clk_ops clk_ops = {
> +	.prepare = skl_clk_prepare,
> +	.unprepare = skl_clk_unprepare,
> +	.set_rate = skl_clk_set_rate,
> +	.recalc_rate = skl_clk_recalc_rate,
> +};
> +
>   static int skl_clock_device_register(struct skl *skl)
>   {
>   	struct skl_clk_pdata *clk_pdata;
> -
> +	struct platform_device_info pdevinfo = {NULL};
>   
>   	clk_pdata = devm_kzalloc(&skl->pci->dev, sizeof(*clk_pdata),
>   							GFP_KERNEL);
> @@ -573,10 +711,28 @@ static int skl_clock_device_register(struct skl *skl)
>   
>   	/* Query NHLT to fill the rates and parent */
>   	skl_get_clks(skl, clk_pdata->ssp_clks);
> +	clk_pdata->ops = &clk_ops;
> +	clk_pdata->pvt_data = skl;
> +
> +	/* Register Platform device */
> +	pdevinfo.parent = &skl->pci->dev;
> +	pdevinfo.id = -1;
> +	pdevinfo.name = "skl-ssp-clk";
> +	pdevinfo.data = clk_pdata;
> +	pdevinfo.size_data = sizeof(*clk_pdata);
> +	skl->clk_dev = platform_device_register_full(&pdevinfo);
> +	if (IS_ERR(skl->clk_dev))
> +		return PTR_ERR(skl->clk_dev);
>   
>   	return 0;
>   }
>   
> +static void skl_clock_device_unregister(struct skl *skl)
> +{
> +	if (skl->clk_dev)
> +		platform_device_unregister(skl->clk_dev);
> +}
> +
>   /*
>    * Probe the given codec address
>    */
> @@ -859,6 +1015,11 @@ static int skl_probe(struct pci_dev *pci,
>   
>   	/* check if dsp is there */
>   	if (bus->ppcap) {
> +		/* create device for dsp clk */
> +		err = skl_clock_device_register(skl);
> +		if (err < 0)
> +			goto out_clk_free;
> +
>   		err = skl_machine_device_register(skl,
>   				  (void *)pci_id->driver_data);
>   		if (err < 0)
> @@ -890,6 +1051,8 @@ static int skl_probe(struct pci_dev *pci,
>   	skl_free_dsp(skl);
>   out_mach_free:
>   	skl_machine_device_unregister(skl);
> +out_clk_free:
> +	skl_clock_device_unregister(skl);
>   out_nhlt_free:
>   	skl_nhlt_free(skl->nhlt);
>   out_free:
> @@ -940,6 +1103,7 @@ static void skl_remove(struct pci_dev *pci)
>   	skl_free_dsp(skl);
>   	skl_machine_device_unregister(skl);
>   	skl_dmic_device_unregister(skl);
> +	skl_clock_device_unregister(skl);
>   	skl_nhlt_remove_sysfs(skl);
>   	skl_nhlt_free(skl->nhlt);
>   	skl_free(ebus);
> diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
> index f06e98962a0b..df0fcf1bfe40 100644
> --- a/sound/soc/intel/skylake/skl.h
> +++ b/sound/soc/intel/skylake/skl.h
> @@ -57,6 +57,7 @@ struct skl {
>   	unsigned int init_done:1; /* delayed init status */
>   	struct platform_device *dmic_dev;
>   	struct platform_device *i2s_dev;
> +	struct platform_device *clk_dev;
>   	struct snd_soc_platform *platform;
>   
>   	struct nhlt_acpi_table *nhlt; /* nhlt ptr */

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

[Subhransu S. Prusty]

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@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.

> 
> Note that I am not against this idea, it's fine, but I'd like more
> clarity on the assumptions. Thanks!

I agree. I will add some comments explaining the expectation.

Thanks
Subhransu

> 
> >+
> >+static int skl_clk_unprepare(void *pvt_data, u32 id, unsigned long rate)
> >+{
> >+	struct skl *skl = pvt_data;
> >+	struct skl_clk_rate_cfg_table *rcfg;
> >+	int vbus_id, ret;
> >+	u8 clk_type;
> >+
> >+	clk_type = skl_get_clk_type(id);
> >+	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;
> >+
> >+	return skl_send_clk_dma_control(skl, rcfg, vbus_id, clk_type, false);
> >+}
> >+
> >+static int skl_clk_set_rate(u32 id, unsigned long rate)
> >+{
> >+	struct skl_clk_rate_cfg_table *rcfg;
> >+	u8 clk_type;
> >+
> >+	if (!rate)
> >+		return -EINVAL;
> >+
> >+	clk_type = skl_get_clk_type(id);
> >+	rcfg = skl_get_rate_cfg(skl_ssp_clks[id].rate_cfg, rate);
> >+	if (!rcfg)
> >+		return -EINVAL;
> >+
> >+	skl_fill_clk_ipc(rcfg, clk_type);
> >+
> >+	return 0;
> >+}
> >+
> >+unsigned long skl_clk_recalc_rate(u32 id, unsigned long parent_rate)
> >+{
> >+	struct skl_clk_rate_cfg_table *rcfg;
> >+	u8 clk_type;
> >+
> >+	clk_type = skl_get_clk_type(id);
> >+	rcfg = skl_get_rate_cfg(skl_ssp_clks[id].rate_cfg, parent_rate);
> >+	if (!rcfg)
> >+		return 0;
> >+
> >+	skl_fill_clk_ipc(rcfg, clk_type);
> >+
> >+	return rcfg->rate;
> >+}
> >+
> >  static int skl_machine_device_register(struct skl *skl, void *driver_data)
> >  {
> >  	struct hdac_bus *bus = ebus_to_hbus(&skl->ebus);
> >@@ -555,10 +682,21 @@ void init_skl_xtal_rate(int pci_id)
> >  	}
> >  }
> >+/*
> >+ * prepare/unprepare are used instead of enable/disable as IPC will be sent
> >+ * in non-atomic context.
> >+ */
> >+static struct skl_clk_ops clk_ops = {
> >+	.prepare = skl_clk_prepare,
> >+	.unprepare = skl_clk_unprepare,
> >+	.set_rate = skl_clk_set_rate,
> >+	.recalc_rate = skl_clk_recalc_rate,
> >+};
> >+
> >  static int skl_clock_device_register(struct skl *skl)
> >  {
> >  	struct skl_clk_pdata *clk_pdata;
> >-
> >+	struct platform_device_info pdevinfo = {NULL};
> >  	clk_pdata = devm_kzalloc(&skl->pci->dev, sizeof(*clk_pdata),
> >  							GFP_KERNEL);
> >@@ -573,10 +711,28 @@ static int skl_clock_device_register(struct skl *skl)
> >  	/* Query NHLT to fill the rates and parent */
> >  	skl_get_clks(skl, clk_pdata->ssp_clks);
> >+	clk_pdata->ops = &clk_ops;
> >+	clk_pdata->pvt_data = skl;
> >+
> >+	/* Register Platform device */
> >+	pdevinfo.parent = &skl->pci->dev;
> >+	pdevinfo.id = -1;
> >+	pdevinfo.name = "skl-ssp-clk";
> >+	pdevinfo.data = clk_pdata;
> >+	pdevinfo.size_data = sizeof(*clk_pdata);
> >+	skl->clk_dev = platform_device_register_full(&pdevinfo);
> >+	if (IS_ERR(skl->clk_dev))
> >+		return PTR_ERR(skl->clk_dev);
> >  	return 0;
> >  }
> >+static void skl_clock_device_unregister(struct skl *skl)
> >+{
> >+	if (skl->clk_dev)
> >+		platform_device_unregister(skl->clk_dev);
> >+}
> >+
> >  /*
> >   * Probe the given codec address
> >   */
> >@@ -859,6 +1015,11 @@ static int skl_probe(struct pci_dev *pci,
> >  	/* check if dsp is there */
> >  	if (bus->ppcap) {
> >+		/* create device for dsp clk */
> >+		err = skl_clock_device_register(skl);
> >+		if (err < 0)
> >+			goto out_clk_free;
> >+
> >  		err = skl_machine_device_register(skl,
> >  				  (void *)pci_id->driver_data);
> >  		if (err < 0)
> >@@ -890,6 +1051,8 @@ static int skl_probe(struct pci_dev *pci,
> >  	skl_free_dsp(skl);
> >  out_mach_free:
> >  	skl_machine_device_unregister(skl);
> >+out_clk_free:
> >+	skl_clock_device_unregister(skl);
> >  out_nhlt_free:
> >  	skl_nhlt_free(skl->nhlt);
> >  out_free:
> >@@ -940,6 +1103,7 @@ static void skl_remove(struct pci_dev *pci)
> >  	skl_free_dsp(skl);
> >  	skl_machine_device_unregister(skl);
> >  	skl_dmic_device_unregister(skl);
> >+	skl_clock_device_unregister(skl);
> >  	skl_nhlt_remove_sysfs(skl);
> >  	skl_nhlt_free(skl->nhlt);
> >  	skl_free(ebus);
> >diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
> >index f06e98962a0b..df0fcf1bfe40 100644
> >--- a/sound/soc/intel/skylake/skl.h
> >+++ b/sound/soc/intel/skylake/skl.h
> >@@ -57,6 +57,7 @@ struct skl {
> >  	unsigned int init_done:1; /* delayed init status */
> >  	struct platform_device *dmic_dev;
> >  	struct platform_device *i2s_dev;
> >+	struct platform_device *clk_dev;
> >  	struct snd_soc_platform *platform;
> >  	struct nhlt_acpi_table *nhlt; /* nhlt ptr */
> 

-- 

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

[Subhransu S. Prusty]

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@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.

> 
> > 
> > Note that I am not against this idea, it's fine, but I'd like more
> > clarity on the assumptions. Thanks!
> 
> I agree. I will add some comments explaining the expectation.
> 
> Thanks
> Subhransu
> 
> > 
> > >+
> > >+static int skl_clk_unprepare(void *pvt_data, u32 id, unsigned long rate)
> > >+{
> > >+	struct skl *skl = pvt_data;
> > >+	struct skl_clk_rate_cfg_table *rcfg;
> > >+	int vbus_id, ret;
> > >+	u8 clk_type;
> > >+
> > >+	clk_type = skl_get_clk_type(id);
> > >+	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;
> > >+
> > >+	return skl_send_clk_dma_control(skl, rcfg, vbus_id, clk_type, false);
> > >+}
> > >+
> > >+static int skl_clk_set_rate(u32 id, unsigned long rate)
> > >+{
> > >+	struct skl_clk_rate_cfg_table *rcfg;
> > >+	u8 clk_type;
> > >+
> > >+	if (!rate)
> > >+		return -EINVAL;
> > >+
> > >+	clk_type = skl_get_clk_type(id);
> > >+	rcfg = skl_get_rate_cfg(skl_ssp_clks[id].rate_cfg, rate);
> > >+	if (!rcfg)
> > >+		return -EINVAL;
> > >+
> > >+	skl_fill_clk_ipc(rcfg, clk_type);
> > >+
> > >+	return 0;
> > >+}
> > >+
> > >+unsigned long skl_clk_recalc_rate(u32 id, unsigned long parent_rate)
> > >+{
> > >+	struct skl_clk_rate_cfg_table *rcfg;
> > >+	u8 clk_type;
> > >+
> > >+	clk_type = skl_get_clk_type(id);
> > >+	rcfg = skl_get_rate_cfg(skl_ssp_clks[id].rate_cfg, parent_rate);
> > >+	if (!rcfg)
> > >+		return 0;
> > >+
> > >+	skl_fill_clk_ipc(rcfg, clk_type);
> > >+
> > >+	return rcfg->rate;
> > >+}
> > >+
> > >  static int skl_machine_device_register(struct skl *skl, void *driver_data)
> > >  {
> > >  	struct hdac_bus *bus = ebus_to_hbus(&skl->ebus);
> > >@@ -555,10 +682,21 @@ void init_skl_xtal_rate(int pci_id)
> > >  	}
> > >  }
> > >+/*
> > >+ * prepare/unprepare are used instead of enable/disable as IPC will be sent
> > >+ * in non-atomic context.
> > >+ */
> > >+static struct skl_clk_ops clk_ops = {
> > >+	.prepare = skl_clk_prepare,
> > >+	.unprepare = skl_clk_unprepare,
> > >+	.set_rate = skl_clk_set_rate,
> > >+	.recalc_rate = skl_clk_recalc_rate,
> > >+};
> > >+
> > >  static int skl_clock_device_register(struct skl *skl)
> > >  {
> > >  	struct skl_clk_pdata *clk_pdata;
> > >-
> > >+	struct platform_device_info pdevinfo = {NULL};
> > >  	clk_pdata = devm_kzalloc(&skl->pci->dev, sizeof(*clk_pdata),
> > >  							GFP_KERNEL);
> > >@@ -573,10 +711,28 @@ static int skl_clock_device_register(struct skl *skl)
> > >  	/* Query NHLT to fill the rates and parent */
> > >  	skl_get_clks(skl, clk_pdata->ssp_clks);
> > >+	clk_pdata->ops = &clk_ops;
> > >+	clk_pdata->pvt_data = skl;
> > >+
> > >+	/* Register Platform device */
> > >+	pdevinfo.parent = &skl->pci->dev;
> > >+	pdevinfo.id = -1;
> > >+	pdevinfo.name = "skl-ssp-clk";
> > >+	pdevinfo.data = clk_pdata;
> > >+	pdevinfo.size_data = sizeof(*clk_pdata);
> > >+	skl->clk_dev = platform_device_register_full(&pdevinfo);
> > >+	if (IS_ERR(skl->clk_dev))
> > >+		return PTR_ERR(skl->clk_dev);
> > >  	return 0;
> > >  }
> > >+static void skl_clock_device_unregister(struct skl *skl)
> > >+{
> > >+	if (skl->clk_dev)
> > >+		platform_device_unregister(skl->clk_dev);
> > >+}
> > >+
> > >  /*
> > >   * Probe the given codec address
> > >   */
> > >@@ -859,6 +1015,11 @@ static int skl_probe(struct pci_dev *pci,
> > >  	/* check if dsp is there */
> > >  	if (bus->ppcap) {
> > >+		/* create device for dsp clk */
> > >+		err = skl_clock_device_register(skl);
> > >+		if (err < 0)
> > >+			goto out_clk_free;
> > >+
> > >  		err = skl_machine_device_register(skl,
> > >  				  (void *)pci_id->driver_data);
> > >  		if (err < 0)
> > >@@ -890,6 +1051,8 @@ static int skl_probe(struct pci_dev *pci,
> > >  	skl_free_dsp(skl);
> > >  out_mach_free:
> > >  	skl_machine_device_unregister(skl);
> > >+out_clk_free:
> > >+	skl_clock_device_unregister(skl);
> > >  out_nhlt_free:
> > >  	skl_nhlt_free(skl->nhlt);
> > >  out_free:
> > >@@ -940,6 +1103,7 @@ static void skl_remove(struct pci_dev *pci)
> > >  	skl_free_dsp(skl);
> > >  	skl_machine_device_unregister(skl);
> > >  	skl_dmic_device_unregister(skl);
> > >+	skl_clock_device_unregister(skl);
> > >  	skl_nhlt_remove_sysfs(skl);
> > >  	skl_nhlt_free(skl->nhlt);
> > >  	skl_free(ebus);
> > >diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
> > >index f06e98962a0b..df0fcf1bfe40 100644
> > >--- a/sound/soc/intel/skylake/skl.h
> > >+++ b/sound/soc/intel/skylake/skl.h
> > >@@ -57,6 +57,7 @@ struct skl {
> > >  	unsigned int init_done:1; /* delayed init status */
> > >  	struct platform_device *dmic_dev;
> > >  	struct platform_device *i2s_dev;
> > >+	struct platform_device *clk_dev;
> > >  	struct snd_soc_platform *platform;
> > >  	struct nhlt_acpi_table *nhlt; /* nhlt ptr */
> > 
> 
> -- 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

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

[Pierre-Louis Bossart]

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@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.

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

[Subhransu S. Prusty]

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@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.

Regards,
Subhransu

-- 

Re: [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@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
> 
> -- 

-- 

[PATCH 5/6] ASoC: Intel: Skylake: Add ssp clock driver

[Subhransu S. Prusty]

From: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>

For certain platforms it is required to start the clocks (mclk/sclk/fs)
before the stream start. Example: for few chrome systems, codec needs the
mclk/sclk to be enabled early for a successful clock synchronization and
for few IVI platforms, clock need to be enabled at boot and should be ON
always.

This patch creates virtual clock driver, which allows the machine driver to
use the clock interface to send IPCs to DSP to enable/disable the clocks.

Signed-off-by: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
---
 sound/soc/intel/Kconfig               |   7 +
 sound/soc/intel/skylake/Makefile      |   5 +
 sound/soc/intel/skylake/skl-ssp-clk.c | 287 ++++++++++++++++++++++++++++++++++
 3 files changed, 299 insertions(+)
 create mode 100644 sound/soc/intel/skylake/skl-ssp-clk.c

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index b3c7f554ec30..b9d9d692b4e3 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -269,6 +269,13 @@ config SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH
           Say Y if you have such a device.
           If unsure select "N".
 
+config SND_SOC_INTEL_SKYLAKE_SSP_CLK
+	tristate
+	help
+	   Enabling this will make the clk driver to be compiled and allow to
+	   control MCLK/SCLK/FS clocks independent of the stream. It should be
+	   enabled by the respective machine driver.
+
 config SND_SOC_INTEL_SKYLAKE
 	tristate
 	select SND_HDA_EXT_CORE
diff --git a/sound/soc/intel/skylake/Makefile b/sound/soc/intel/skylake/Makefile
index 3380deb81015..9131c35ad4bb 100644
--- a/sound/soc/intel/skylake/Makefile
+++ b/sound/soc/intel/skylake/Makefile
@@ -13,3 +13,8 @@ snd-soc-skl-ipc-objs := skl-sst-ipc.o skl-sst-dsp.o cnl-sst-dsp.o \
 		skl-sst-utils.o
 
 obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE) += snd-soc-skl-ipc.o
+
+#Skylake Clock device support
+snd-soc-skl-ssp-clk-objs := skl-ssp-clk.o
+
+obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE_SSP_CLK) += snd-soc-skl-ssp-clk.o
diff --git a/sound/soc/intel/skylake/skl-ssp-clk.c b/sound/soc/intel/skylake/skl-ssp-clk.c
new file mode 100644
index 000000000000..5b4016826fdc
--- /dev/null
+++ b/sound/soc/intel/skylake/skl-ssp-clk.c
@@ -0,0 +1,287 @@
+/*
+ *  skl-ssp-clk.c - ASoC skylake ssp clock driver
+ *
+ *  Copyright (C) 2017 Intel Corp
+ *  Author: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
+ *  Author: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
+ *
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include "skl-ssp-clk.h"
+
+#define to_skl_clk(_hw)	container_of(_hw, struct skl_clk, hw)
+
+struct skl_clk_parent {
+	struct clk_hw *hw;
+	struct clk_lookup *lookup;
+};
+
+struct skl_clk {
+	struct clk_hw hw;
+	struct clk_lookup *lookup;
+	struct skl_clk_ops *ops;
+	unsigned long rate;
+	void *pvt_data;
+	u32 id;
+};
+
+struct skl_clk_data {
+	struct skl_clk_parent parent[SKL_MAX_CLK_SRC];
+	struct skl_clk *clk[SKL_MAX_CLK_CNT];
+	u8 avail_clk_cnt;
+};
+
+static int skl_clk_prepare(struct clk_hw *hw)
+{
+	struct skl_clk *clkdev = to_skl_clk(hw);
+
+	if (!clkdev->ops || !clkdev->ops->prepare)
+		return -EIO;
+
+	if (!clkdev->rate)
+		return -EINVAL;
+
+	return clkdev->ops->prepare(clkdev->pvt_data, clkdev->id, clkdev->rate);
+}
+
+static void skl_clk_unprepare(struct clk_hw *hw)
+{
+	struct skl_clk *clkdev = to_skl_clk(hw);
+
+	if (!clkdev->ops || !clkdev->ops->unprepare)
+		return;
+
+	if (!clkdev->rate)
+		return;
+
+	clkdev->ops->unprepare(clkdev->pvt_data, clkdev->id, clkdev->rate);
+}
+
+static int skl_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+					unsigned long parent_rate)
+{
+	struct skl_clk *clkdev = to_skl_clk(hw);
+	int ret;
+
+	if (!clkdev->ops || !clkdev->ops->set_rate)
+		return -EIO;
+
+	ret = clkdev->ops->set_rate(clkdev->id, rate);
+	if (!ret)
+		clkdev->rate = rate;
+
+	return ret;
+}
+
+unsigned long skl_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	struct skl_clk *clkdev = to_skl_clk(hw);
+
+	if (clkdev->rate)
+		return clkdev->rate;
+
+	if (!clkdev->ops || !clkdev->ops->recalc_rate)
+		return -EIO;
+
+	clkdev->rate = clkdev->ops->recalc_rate(clkdev->id, parent_rate);
+
+	return clkdev->rate;
+}
+
+/* Not supported by clk driver. Implemented to satisfy clk fw */
+long skl_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long *parent_rate)
+{
+	return rate;
+}
+
+static const struct clk_ops skl_clk_ops = {
+	.prepare = skl_clk_prepare,
+	.unprepare = skl_clk_unprepare,
+	.set_rate = skl_clk_set_rate,
+	.round_rate = skl_clk_round_rate,
+	.recalc_rate = skl_clk_recalc_rate,
+};
+
+static void unregister_parent_src_clk(struct skl_clk_parent *pclk, u8 id)
+{
+	while (id--) {
+		clkdev_drop(pclk[id].lookup);
+		clk_hw_unregister_fixed_rate(pclk[id].hw);
+	}
+}
+
+static void unregister_src_clk(struct skl_clk_data *dclk)
+{
+	u8 cnt = dclk->avail_clk_cnt;
+
+	while (cnt--)
+		clkdev_drop(dclk->clk[cnt]->lookup);
+}
+
+static int skl_register_parent_clks(struct device *dev,
+			struct skl_clk_parent *parent,
+			struct skl_clk_parent_src *pclk)
+{
+	int i, ret;
+
+	for (i = 0; i < SKL_MAX_CLK_SRC; i++) {
+
+		/* Register Parent clock */
+		parent[i].hw = clk_hw_register_fixed_rate(dev, pclk[i].name,
+				pclk[i].parent_name, 0, pclk[i].rate);
+		if (IS_ERR(parent[i].hw)) {
+			ret = PTR_ERR_OR_ZERO(parent[i].hw);
+			goto err;
+		}
+
+		parent[i].lookup = clkdev_hw_create(parent[i].hw, pclk[i].name,
+									NULL);
+		if (!parent[i].lookup) {
+			clk_hw_unregister_fixed_rate(parent[i].hw);
+			ret = PTR_ERR_OR_ZERO(parent[i].lookup);
+			goto err;
+		}
+	}
+
+	return 0;
+err:
+	unregister_parent_src_clk(parent, i);
+	return ret;
+}
+
+/* REMOVE: send only ssp_clks[i], ops */
+/* Assign fmt_config to clk_data */
+static struct skl_clk *register_skl_clk(struct device *dev,
+			struct skl_ssp_clk *clk,
+			struct skl_clk_pdata *clk_pdata, int id)
+{
+	struct skl_clk *clkdev;
+	struct clk_init_data init;
+	int ret;
+
+	clkdev = devm_kzalloc(dev, sizeof(*clkdev), GFP_KERNEL);
+	if (!clkdev)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = clk->name;
+	init.ops = &skl_clk_ops;
+	init.flags = 0;
+	init.parent_names = &clk->parent_name;
+	init.num_parents = 1;
+	clkdev->hw.init = &init;
+	clkdev->ops = clk_pdata->ops;
+	clkdev->pvt_data = clk_pdata->pvt_data;
+
+	clkdev->id = id;
+	ret = devm_clk_hw_register(dev, &clkdev->hw);
+	if (ret) {
+		clkdev = ERR_PTR(ret);
+		return clkdev;
+	}
+
+	clkdev->lookup = clkdev_hw_create(&clkdev->hw, init.name, NULL);
+	if (!clkdev->lookup)
+		clkdev = ERR_PTR(-ENOMEM);
+
+	return clkdev;
+}
+
+static int skl_clk_dev_probe(struct platform_device *pdev)
+{
+	struct skl_clk_pdata *clk_pdata;
+	struct skl_clk_parent_src *parent_clks;
+	struct skl_ssp_clk *clks;
+	struct skl_clk_data *data;
+	struct device *dev = &pdev->dev;
+	int ret, i;
+
+	clk_pdata = dev_get_platdata(&pdev->dev);
+	parent_clks = clk_pdata->parent_clks;
+	clks = clk_pdata->ssp_clks;
+	if (!parent_clks || !clks)
+		return -EIO;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	/* Register Parent clock */
+	ret = skl_register_parent_clks(dev, data->parent, parent_clks);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < clk_pdata->num_clks; i++) {
+		/*
+		 * Only register valid clocks
+		 * i.e. for which nhlt entry is present.
+		 */
+		if (clks[i].rate_cfg[0].rate == 0)
+			continue;
+
+		data->clk[i] = register_skl_clk(dev, &clks[i], clk_pdata, i);
+		if (IS_ERR(data->clk[i])) {
+			ret = PTR_ERR(data->clk[i]);
+			goto err_unreg_skl_clk;
+		}
+
+		data->avail_clk_cnt++;
+	}
+
+	platform_set_drvdata(pdev, data);
+
+	return 0;
+
+err_unreg_skl_clk:
+	unregister_src_clk(data);
+	unregister_parent_src_clk(data->parent, SKL_MAX_CLK_SRC);
+
+	return ret;
+}
+
+static int skl_clk_dev_remove(struct platform_device *pdev)
+{
+	struct skl_clk_data *data;
+
+	data = platform_get_drvdata(pdev);
+	unregister_parent_src_clk(data->parent, SKL_MAX_CLK_SRC);
+	unregister_src_clk(data);
+
+	return 0;
+}
+
+static struct platform_driver skl_clk_driver = {
+	.driver = {
+		.name = "skl-ssp-clk",
+	},
+	.probe = skl_clk_dev_probe,
+	.remove = skl_clk_dev_remove,
+};
+
+module_platform_driver(skl_clk_driver);
+
+MODULE_DESCRIPTION("Skylake clock driver");
+MODULE_AUTHOR("Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>");
+MODULE_AUTHOR("Subhransu S. Prusty <subhransu.s.prusty@intel.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:skl-ssp-clk");
-- 
1.9.1

Re: [PATCH 5/6] ASoC: Intel: Skylake: Add ssp clock driver

[Vinod Koul]

On Thu, Sep 07, 2017 at 07:59:24PM +0530, Subhransu S. Prusty wrote:
> From: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
> 
> For certain platforms it is required to start the clocks (mclk/sclk/fs)
> before the stream start. Example: for few chrome systems, codec needs the
> mclk/sclk to be enabled early for a successful clock synchronization and
> for few IVI platforms, clock need to be enabled at boot and should be ON
> always.
> 
> This patch creates virtual clock driver, which allows the machine driver to
> use the clock interface to send IPCs to DSP to enable/disable the clocks.

Mark, Michael, Stephen,

So this one adds a 'virtual' clock driver for audio clocks (mclk, bclk...)
and we have DSP configuring them.  The SW (driver) sends messages to DSP for
controlling these clocks, that is why we decided to keep this in ASoC, do
let us know if that is okay.

And it goes without saying that this would need ACK from clk guys before
merging.

> 
> Signed-off-by: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> ---
>  sound/soc/intel/Kconfig               |   7 +
>  sound/soc/intel/skylake/Makefile      |   5 +
>  sound/soc/intel/skylake/skl-ssp-clk.c | 287 ++++++++++++++++++++++++++++++++++
>  3 files changed, 299 insertions(+)
>  create mode 100644 sound/soc/intel/skylake/skl-ssp-clk.c
> 
> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
> index b3c7f554ec30..b9d9d692b4e3 100644
> --- a/sound/soc/intel/Kconfig
> +++ b/sound/soc/intel/Kconfig
> @@ -269,6 +269,13 @@ config SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH
>            Say Y if you have such a device.
>            If unsure select "N".
>  
> +config SND_SOC_INTEL_SKYLAKE_SSP_CLK
> +	tristate
> +	help
> +	   Enabling this will make the clk driver to be compiled and allow to
> +	   control MCLK/SCLK/FS clocks independent of the stream. It should be
> +	   enabled by the respective machine driver.
> +
>  config SND_SOC_INTEL_SKYLAKE
>  	tristate
>  	select SND_HDA_EXT_CORE
> diff --git a/sound/soc/intel/skylake/Makefile b/sound/soc/intel/skylake/Makefile
> index 3380deb81015..9131c35ad4bb 100644
> --- a/sound/soc/intel/skylake/Makefile
> +++ b/sound/soc/intel/skylake/Makefile
> @@ -13,3 +13,8 @@ snd-soc-skl-ipc-objs := skl-sst-ipc.o skl-sst-dsp.o cnl-sst-dsp.o \
>  		skl-sst-utils.o
>  
>  obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE) += snd-soc-skl-ipc.o
> +
> +#Skylake Clock device support
> +snd-soc-skl-ssp-clk-objs := skl-ssp-clk.o
> +
> +obj-$(CONFIG_SND_SOC_INTEL_SKYLAKE_SSP_CLK) += snd-soc-skl-ssp-clk.o
> diff --git a/sound/soc/intel/skylake/skl-ssp-clk.c b/sound/soc/intel/skylake/skl-ssp-clk.c
> new file mode 100644
> index 000000000000..5b4016826fdc
> --- /dev/null
> +++ b/sound/soc/intel/skylake/skl-ssp-clk.c
> @@ -0,0 +1,287 @@
> +/*
> + *  skl-ssp-clk.c - ASoC skylake ssp clock driver
> + *
> + *  Copyright (C) 2017 Intel Corp
> + *  Author: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
> + *  Author: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> + *
> + *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; version 2 of the License.
> + *
> + *  This program is distributed in the hope that it will be useful, but
> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include "skl-ssp-clk.h"
> +
> +#define to_skl_clk(_hw)	container_of(_hw, struct skl_clk, hw)
> +
> +struct skl_clk_parent {
> +	struct clk_hw *hw;
> +	struct clk_lookup *lookup;
> +};
> +
> +struct skl_clk {
> +	struct clk_hw hw;
> +	struct clk_lookup *lookup;
> +	struct skl_clk_ops *ops;
> +	unsigned long rate;
> +	void *pvt_data;
> +	u32 id;
> +};
> +
> +struct skl_clk_data {
> +	struct skl_clk_parent parent[SKL_MAX_CLK_SRC];
> +	struct skl_clk *clk[SKL_MAX_CLK_CNT];
> +	u8 avail_clk_cnt;
> +};
> +
> +static int skl_clk_prepare(struct clk_hw *hw)
> +{
> +	struct skl_clk *clkdev = to_skl_clk(hw);
> +
> +	if (!clkdev->ops || !clkdev->ops->prepare)
> +		return -EIO;
> +
> +	if (!clkdev->rate)
> +		return -EINVAL;
> +
> +	return clkdev->ops->prepare(clkdev->pvt_data, clkdev->id, clkdev->rate);
> +}
> +
> +static void skl_clk_unprepare(struct clk_hw *hw)
> +{
> +	struct skl_clk *clkdev = to_skl_clk(hw);
> +
> +	if (!clkdev->ops || !clkdev->ops->unprepare)
> +		return;
> +
> +	if (!clkdev->rate)
> +		return;
> +
> +	clkdev->ops->unprepare(clkdev->pvt_data, clkdev->id, clkdev->rate);
> +}
> +
> +static int skl_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> +					unsigned long parent_rate)
> +{
> +	struct skl_clk *clkdev = to_skl_clk(hw);
> +	int ret;
> +
> +	if (!clkdev->ops || !clkdev->ops->set_rate)
> +		return -EIO;
> +
> +	ret = clkdev->ops->set_rate(clkdev->id, rate);
> +	if (!ret)
> +		clkdev->rate = rate;
> +
> +	return ret;
> +}
> +
> +unsigned long skl_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> +{
> +	struct skl_clk *clkdev = to_skl_clk(hw);
> +
> +	if (clkdev->rate)
> +		return clkdev->rate;
> +
> +	if (!clkdev->ops || !clkdev->ops->recalc_rate)
> +		return -EIO;
> +
> +	clkdev->rate = clkdev->ops->recalc_rate(clkdev->id, parent_rate);
> +
> +	return clkdev->rate;
> +}
> +
> +/* Not supported by clk driver. Implemented to satisfy clk fw */
> +long skl_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> +				unsigned long *parent_rate)
> +{
> +	return rate;
> +}
> +
> +static const struct clk_ops skl_clk_ops = {
> +	.prepare = skl_clk_prepare,
> +	.unprepare = skl_clk_unprepare,
> +	.set_rate = skl_clk_set_rate,
> +	.round_rate = skl_clk_round_rate,
> +	.recalc_rate = skl_clk_recalc_rate,
> +};
> +
> +static void unregister_parent_src_clk(struct skl_clk_parent *pclk, u8 id)
> +{
> +	while (id--) {
> +		clkdev_drop(pclk[id].lookup);
> +		clk_hw_unregister_fixed_rate(pclk[id].hw);
> +	}
> +}
> +
> +static void unregister_src_clk(struct skl_clk_data *dclk)
> +{
> +	u8 cnt = dclk->avail_clk_cnt;
> +
> +	while (cnt--)
> +		clkdev_drop(dclk->clk[cnt]->lookup);
> +}
> +
> +static int skl_register_parent_clks(struct device *dev,
> +			struct skl_clk_parent *parent,
> +			struct skl_clk_parent_src *pclk)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < SKL_MAX_CLK_SRC; i++) {
> +
> +		/* Register Parent clock */
> +		parent[i].hw = clk_hw_register_fixed_rate(dev, pclk[i].name,
> +				pclk[i].parent_name, 0, pclk[i].rate);
> +		if (IS_ERR(parent[i].hw)) {
> +			ret = PTR_ERR_OR_ZERO(parent[i].hw);
> +			goto err;
> +		}
> +
> +		parent[i].lookup = clkdev_hw_create(parent[i].hw, pclk[i].name,
> +									NULL);
> +		if (!parent[i].lookup) {
> +			clk_hw_unregister_fixed_rate(parent[i].hw);
> +			ret = PTR_ERR_OR_ZERO(parent[i].lookup);
> +			goto err;
> +		}
> +	}
> +
> +	return 0;
> +err:
> +	unregister_parent_src_clk(parent, i);
> +	return ret;
> +}
> +
> +/* REMOVE: send only ssp_clks[i], ops */
> +/* Assign fmt_config to clk_data */
> +static struct skl_clk *register_skl_clk(struct device *dev,
> +			struct skl_ssp_clk *clk,
> +			struct skl_clk_pdata *clk_pdata, int id)
> +{
> +	struct skl_clk *clkdev;
> +	struct clk_init_data init;
> +	int ret;
> +
> +	clkdev = devm_kzalloc(dev, sizeof(*clkdev), GFP_KERNEL);
> +	if (!clkdev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	init.name = clk->name;
> +	init.ops = &skl_clk_ops;
> +	init.flags = 0;
> +	init.parent_names = &clk->parent_name;
> +	init.num_parents = 1;
> +	clkdev->hw.init = &init;
> +	clkdev->ops = clk_pdata->ops;
> +	clkdev->pvt_data = clk_pdata->pvt_data;
> +
> +	clkdev->id = id;
> +	ret = devm_clk_hw_register(dev, &clkdev->hw);
> +	if (ret) {
> +		clkdev = ERR_PTR(ret);
> +		return clkdev;
> +	}
> +
> +	clkdev->lookup = clkdev_hw_create(&clkdev->hw, init.name, NULL);
> +	if (!clkdev->lookup)
> +		clkdev = ERR_PTR(-ENOMEM);
> +
> +	return clkdev;
> +}
> +
> +static int skl_clk_dev_probe(struct platform_device *pdev)
> +{
> +	struct skl_clk_pdata *clk_pdata;
> +	struct skl_clk_parent_src *parent_clks;
> +	struct skl_ssp_clk *clks;
> +	struct skl_clk_data *data;
> +	struct device *dev = &pdev->dev;
> +	int ret, i;
> +
> +	clk_pdata = dev_get_platdata(&pdev->dev);
> +	parent_clks = clk_pdata->parent_clks;
> +	clks = clk_pdata->ssp_clks;
> +	if (!parent_clks || !clks)
> +		return -EIO;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	/* Register Parent clock */
> +	ret = skl_register_parent_clks(dev, data->parent, parent_clks);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < clk_pdata->num_clks; i++) {
> +		/*
> +		 * Only register valid clocks
> +		 * i.e. for which nhlt entry is present.
> +		 */
> +		if (clks[i].rate_cfg[0].rate == 0)
> +			continue;
> +
> +		data->clk[i] = register_skl_clk(dev, &clks[i], clk_pdata, i);
> +		if (IS_ERR(data->clk[i])) {
> +			ret = PTR_ERR(data->clk[i]);
> +			goto err_unreg_skl_clk;
> +		}
> +
> +		data->avail_clk_cnt++;
> +	}
> +
> +	platform_set_drvdata(pdev, data);
> +
> +	return 0;
> +
> +err_unreg_skl_clk:
> +	unregister_src_clk(data);
> +	unregister_parent_src_clk(data->parent, SKL_MAX_CLK_SRC);
> +
> +	return ret;
> +}
> +
> +static int skl_clk_dev_remove(struct platform_device *pdev)
> +{
> +	struct skl_clk_data *data;
> +
> +	data = platform_get_drvdata(pdev);
> +	unregister_parent_src_clk(data->parent, SKL_MAX_CLK_SRC);
> +	unregister_src_clk(data);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver skl_clk_driver = {
> +	.driver = {
> +		.name = "skl-ssp-clk",
> +	},
> +	.probe = skl_clk_dev_probe,
> +	.remove = skl_clk_dev_remove,
> +};
> +
> +module_platform_driver(skl_clk_driver);
> +
> +MODULE_DESCRIPTION("Skylake clock driver");
> +MODULE_AUTHOR("Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>");
> +MODULE_AUTHOR("Subhransu S. Prusty <subhransu.s.prusty@intel.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:skl-ssp-clk");
> -- 
> 1.9.1
> 

-- 
~Vinod

Re: [PATCH 5/6] ASoC: Intel: Skylake: Add ssp clock driver

[Stephen Boyd]

On 09/07, Vinod Koul wrote:
> On Thu, Sep 07, 2017 at 07:59:24PM +0530, Subhransu S. Prusty wrote:
> > From: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
> > 
> > For certain platforms it is required to start the clocks (mclk/sclk/fs)
> > before the stream start. Example: for few chrome systems, codec needs the
> > mclk/sclk to be enabled early for a successful clock synchronization and
> > for few IVI platforms, clock need to be enabled at boot and should be ON
> > always.
> > 
> > This patch creates virtual clock driver, which allows the machine driver to
> > use the clock interface to send IPCs to DSP to enable/disable the clocks.
> 
> Mark, Michael, Stephen,
> 
> So this one adds a 'virtual' clock driver for audio clocks (mclk, bclk...)
> and we have DSP configuring them.  The SW (driver) sends messages to DSP for
> controlling these clocks, that is why we decided to keep this in ASoC, do
> let us know if that is okay.
> 

I know there's a v2 and I'm replying here but this is fine from
clk perspective. I tend to think of it as a "remote" driver
instead where the clk control is done by a non-linux processor or
"remote" processor, in this case the DSP. I'll ack v2.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 6/6] ASoC: Intel: kbl: Enable mclk and ssp sclk early

[Subhransu S. Prusty]

From: Harsha Priya <harshapriya.n@intel.com>

rt5663 needs mclk/sclk early to synchronize its internal clocks. Enable
these clocks early.

Signed-off-by: Harsha Priya <harshapriya.n@intel.com>
Signed-off-by: Jaikrishna Nemallapudi <jaikrishnax.nemallapudi@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
---
 sound/soc/intel/Kconfig                      |  1 +
 sound/soc/intel/boards/kbl_rt5663_max98927.c | 94 +++++++++++++++++++++++++++-
 2 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
index b9d9d692b4e3..6f05119e9df2 100644
--- a/sound/soc/intel/Kconfig
+++ b/sound/soc/intel/Kconfig
@@ -247,6 +247,7 @@ config SND_SOC_INTEL_KBL_RT5663_MAX98927_MACH
 	select SND_SOC_MAX98927
 	select SND_SOC_DMIC
 	select SND_SOC_HDAC_HDMI
+	select SND_SOC_INTEL_SKYLAKE_SSP_CLK
 	help
 	  This adds support for ASoC Onboard Codec I2S machine driver. This will
 	  create an alsa sound card for RT5663 + MAX98927.
diff --git a/sound/soc/intel/boards/kbl_rt5663_max98927.c b/sound/soc/intel/boards/kbl_rt5663_max98927.c
index 7f7607420706..b20804c850a7 100644
--- a/sound/soc/intel/boards/kbl_rt5663_max98927.c
+++ b/sound/soc/intel/boards/kbl_rt5663_max98927.c
@@ -27,6 +27,9 @@
 #include "../../codecs/rt5663.h"
 #include "../../codecs/hdac_hdmi.h"
 #include "../skylake/skl.h"
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
 
 #define KBL_REALTEK_CODEC_DAI "rt5663-aif"
 #define KBL_MAXIM_CODEC_DAI "max98927-aif1"
@@ -47,6 +50,8 @@ struct kbl_hdmi_pcm {
 struct kbl_rt5663_private {
 	struct snd_soc_jack kabylake_headset;
 	struct list_head hdmi_pcm_list;
+	struct clk *mclk;
+	struct clk *sclk;
 };
 
 enum {
@@ -68,6 +73,19 @@ enum {
 	SOC_DAPM_PIN_SWITCH("Right Spk"),
 };
 
+static int platform_clock_control(struct snd_soc_dapm_widget *w,
+			struct snd_kcontrol *k, int  event)
+{
+	struct snd_soc_dapm_context *dapm = w->dapm;
+	struct snd_soc_card *card = dapm->card;
+	struct kbl_rt5663_private *priv = snd_soc_card_get_drvdata(card);
+
+	clk_disable_unprepare(priv->mclk);
+	clk_disable_unprepare(priv->sclk);
+
+	return 0;
+}
+
 static const struct snd_soc_dapm_widget kabylake_widgets[] = {
 	SND_SOC_DAPM_HP("Headphone Jack", NULL),
 	SND_SOC_DAPM_MIC("Headset Mic", NULL),
@@ -77,7 +95,8 @@ enum {
 	SND_SOC_DAPM_SPK("HDMI1", NULL),
 	SND_SOC_DAPM_SPK("HDMI2", NULL),
 	SND_SOC_DAPM_SPK("HDMI3", NULL),
-
+	SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
+			platform_clock_control, SND_SOC_DAPM_POST_PMD),
 };
 
 static const struct snd_soc_dapm_route kabylake_map[] = {
@@ -90,6 +109,7 @@ enum {
 	{ "Right Spk", NULL, "Right BE_OUT" },
 
 	/* other jacks */
+	{ "Headset Mic", NULL, "Platform Clock" },
 	{ "IN1P", NULL, "Headset Mic" },
 	{ "IN1N", NULL, "Headset Mic" },
 	{ "DMic", NULL, "SoC DMIC" },
@@ -352,13 +372,56 @@ static int kabylake_ssp_fixup(struct snd_soc_pcm_runtime *rtd,
 	return 0;
 }
 
+static int kabylake_enable_ssp_clks(struct snd_soc_card *card)
+{
+
+	struct kbl_rt5663_private *priv = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	/* Enable MCLK */
+	ret = clk_set_rate(priv->mclk, 24000000);
+	if (ret < 0) {
+		dev_err(card->dev, "Can't set rate for mclk, err: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(priv->mclk);
+	if (ret < 0) {
+		dev_err(card->dev, "Can't enable mclk, err: %d\n", ret);
+		return ret;
+	}
+
+	/* Enable SCLK */
+	ret = clk_set_rate(priv->sclk, 3072000);
+	if (ret < 0) {
+		dev_err(card->dev, "Can't set rate for sclk, err: %d\n", ret);
+		clk_disable_unprepare(priv->mclk);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(priv->sclk);
+	if (ret < 0) {
+		dev_err(card->dev, "Can't enable sclk, err: %d\n", ret);
+		clk_disable_unprepare(priv->mclk);
+	}
+
+	return ret;
+}
+
 static int kabylake_rt5663_hw_params(struct snd_pcm_substream *substream,
 	struct snd_pcm_hw_params *params)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	struct snd_soc_card *card = rtd->card;
 	int ret;
 
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
+		ret = kabylake_enable_ssp_clks(card);
+		if (ret < 0)
+			return ret;
+	}
+
 	/* use ASRC for internal clocks, as PLL rate isn't multiple of BCLK */
 	rt5663_sel_asrc_clk_src(codec_dai->codec,
 			RT5663_DA_STEREO_FILTER | RT5663_AD_STEREO_FILTER,
@@ -839,6 +902,7 @@ static int kabylake_audio_probe(struct platform_device *pdev)
 {
 	struct kbl_rt5663_private *ctx;
 	struct skl_machine_pdata *pdata;
+	int ret;
 
 	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
 	if (!ctx)
@@ -857,6 +921,34 @@ static int kabylake_audio_probe(struct platform_device *pdev)
 		dmic_constraints = pdata->dmic_num == 2 ?
 			&constraints_dmic_2ch : &constraints_dmic_channels;
 
+	ctx->mclk = devm_clk_get(&pdev->dev, "ssp1_mclk");
+	if (IS_ERR(ctx->mclk)) {
+		ret = PTR_ERR(ctx->mclk);
+		if (ret == -ENOENT) {
+			dev_info(&pdev->dev,
+				"Failed to get ssp1_sclk, defer probe\n");
+			return -EPROBE_DEFER;
+		}
+
+		dev_err(&pdev->dev, "Failed to get ssp1_mclk with err:%d\n",
+								ret);
+		return ret;
+	}
+
+	ctx->sclk = devm_clk_get(&pdev->dev, "ssp1_sclk");
+	if (IS_ERR(ctx->sclk)) {
+		ret = PTR_ERR(ctx->sclk);
+		if (ret == -ENOENT) {
+			dev_info(&pdev->dev,
+				"Failed to get ssp1_sclk, defer probe\n");
+			return -EPROBE_DEFER;
+		}
+
+		dev_err(&pdev->dev, "Failed to get ssp1_sclk with err:%d\n",
+								ret);
+		return ret;
+	}
+
 	return devm_snd_soc_register_card(&pdev->dev, kabylake_audio_card);
 }
 
-- 
1.9.1

Re: [alsa-devel] [PATCH 6/6] ASoC: Intel: kbl: Enable mclk and ssp sclk early

[Pierre-Louis Bossart]

> +static int platform_clock_control(struct snd_soc_dapm_widget *w,
> +			struct snd_kcontrol *k, int  event)
> +{
> +	struct snd_soc_dapm_context *dapm = w->dapm;
> +	struct snd_soc_card *card = dapm->card;
> +	struct kbl_rt5663_private *priv = snd_soc_card_get_drvdata(card);
> +
> +	clk_disable_unprepare(priv->mclk);
> +	clk_disable_unprepare(priv->sclk);
> +
> +	return 0;
> +}
> +
>   static const struct snd_soc_dapm_widget kabylake_widgets[] = {
>   	SND_SOC_DAPM_HP("Headphone Jack", NULL),
>   	SND_SOC_DAPM_MIC("Headset Mic", NULL),
> @@ -77,7 +95,8 @@ enum {
>   	SND_SOC_DAPM_SPK("HDMI1", NULL),
>   	SND_SOC_DAPM_SPK("HDMI2", NULL),
>   	SND_SOC_DAPM_SPK("HDMI3", NULL),
> -
> +	SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
> +			platform_clock_control, SND_SOC_DAPM_POST_PMD),
[snip]
> +static int kabylake_enable_ssp_clks(struct snd_soc_card *card)
> +{
> +
> +	struct kbl_rt5663_private *priv = snd_soc_card_get_drvdata(card);
> +	int ret;
> +
> +	/* Enable MCLK */
> +	ret = clk_set_rate(priv->mclk, 24000000);
> +	if (ret < 0) {
> +		dev_err(card->dev, "Can't set rate for mclk, err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(priv->mclk);
> +	if (ret < 0) {
> +		dev_err(card->dev, "Can't enable mclk, err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable SCLK */
> +	ret = clk_set_rate(priv->sclk, 3072000);
> +	if (ret < 0) {
> +		dev_err(card->dev, "Can't set rate for sclk, err: %d\n", ret);
> +		clk_disable_unprepare(priv->mclk);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(priv->sclk);
> +	if (ret < 0) {
> +		dev_err(card->dev, "Can't enable sclk, err: %d\n", ret);
> +		clk_disable_unprepare(priv->mclk);
> +	}
> +
> +	return ret;
> +}
> +
>   static int kabylake_rt5663_hw_params(struct snd_pcm_substream *substream,
>   	struct snd_pcm_hw_params *params)
>   {
>   	struct snd_soc_pcm_runtime *rtd = substream->private_data;
>   	struct snd_soc_dai *codec_dai = rtd->codec_dai;
> +	struct snd_soc_card *card = rtd->card;
>   	int ret;
>   
> +	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
> +		ret = kabylake_enable_ssp_clks(card);

Is there a reason why the clocks need to be enabled in the hw_params() 
instead of platform_clock_control()?
The code is not symmetrical between enable/disable, is this intended? I 
remember seeing this in a different context (dialog codec?).

Re: [alsa-devel] [PATCH 6/6] ASoC: Intel: kbl: Enable mclk and ssp sclk early

[Subhransu S. Prusty]

On Thu, Sep 07, 2017 at 05:19:25PM -0500, Pierre-Louis Bossart wrote:
> 
> >+static int platform_clock_control(struct snd_soc_dapm_widget *w,
> >+			struct snd_kcontrol *k, int  event)
> >+{
> >+	struct snd_soc_dapm_context *dapm = w->dapm;
> >+	struct snd_soc_card *card = dapm->card;
> >+	struct kbl_rt5663_private *priv = snd_soc_card_get_drvdata(card);
> >+
> >+	clk_disable_unprepare(priv->mclk);
> >+	clk_disable_unprepare(priv->sclk);
> >+
> >+	return 0;
> >+}
> >+
> >  static const struct snd_soc_dapm_widget kabylake_widgets[] = {
> >  	SND_SOC_DAPM_HP("Headphone Jack", NULL),
> >  	SND_SOC_DAPM_MIC("Headset Mic", NULL),
> >@@ -77,7 +95,8 @@ enum {
> >  	SND_SOC_DAPM_SPK("HDMI1", NULL),
> >  	SND_SOC_DAPM_SPK("HDMI2", NULL),
> >  	SND_SOC_DAPM_SPK("HDMI3", NULL),
> >-
> >+	SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
> >+			platform_clock_control, SND_SOC_DAPM_POST_PMD),
> [snip]
> >+static int kabylake_enable_ssp_clks(struct snd_soc_card *card)
> >+{
> >+
> >+	struct kbl_rt5663_private *priv = snd_soc_card_get_drvdata(card);
> >+	int ret;
> >+
> >+	/* Enable MCLK */
> >+	ret = clk_set_rate(priv->mclk, 24000000);
> >+	if (ret < 0) {
> >+		dev_err(card->dev, "Can't set rate for mclk, err: %d\n", ret);
> >+		return ret;
> >+	}
> >+
> >+	ret = clk_prepare_enable(priv->mclk);
> >+	if (ret < 0) {
> >+		dev_err(card->dev, "Can't enable mclk, err: %d\n", ret);
> >+		return ret;
> >+	}
> >+
> >+	/* Enable SCLK */
> >+	ret = clk_set_rate(priv->sclk, 3072000);
> >+	if (ret < 0) {
> >+		dev_err(card->dev, "Can't set rate for sclk, err: %d\n", ret);
> >+		clk_disable_unprepare(priv->mclk);
> >+		return ret;
> >+	}
> >+
> >+	ret = clk_prepare_enable(priv->sclk);
> >+	if (ret < 0) {
> >+		dev_err(card->dev, "Can't enable sclk, err: %d\n", ret);
> >+		clk_disable_unprepare(priv->mclk);
> >+	}
> >+
> >+	return ret;
> >+}
> >+
> >  static int kabylake_rt5663_hw_params(struct snd_pcm_substream *substream,
> >  	struct snd_pcm_hw_params *params)
> >  {
> >  	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> >  	struct snd_soc_dai *codec_dai = rtd->codec_dai;
> >+	struct snd_soc_card *card = rtd->card;
> >  	int ret;
> >+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
> >+		ret = kabylake_enable_ssp_clks(card);
> 
> Is there a reason why the clocks need to be enabled in the
> hw_params() instead of platform_clock_control()?

Hi Pierre,

Thanks for the review.

For rt5663 the internal codec clock is configured in set_sysclk callback.
And It is understood from realtek that the mclk/bclk need to turned ON
before the set_sysclk call for a successful clock synchronization by the
codec. So the clocks are enabled in hw_params and disabled in
platform_clk_control.

I will add comments explaining this in the v2 once I receive feedback for
rest of the series.

Regards,
Subhransu

> The code is not symmetrical between enable/disable, is this
> intended? I remember seeing this in a different context (dialog
> codec?).

-- 

Re: [PATCH] ASoC: Intel: eve: Enable mclk and ssp sclk early

[Vinod Koul]

On Sun, Sep 10, 2017 at 08:32:59AM +0530, Naveen M wrote:
> rt5663 needs mclk/sclk early to synchronize its internal clocks.
> Clocks are enabled in hw_params and disabled in platform_clk_control
> as the codec requires mclk/bclk need to turned ON before set_sysclk
> call for a successful clock synchronization.

This would be dependent on the series Subhransu posted right? If so please
do mention that

> +static int kabylake_enable_ssp_clks(struct snd_soc_card *card)
> +{
> +

we don't need the empty line here

> +	struct kbl_codec_private *priv = snd_soc_card_get_drvdata(card);
> +	int ret;
> +
> +	/* Enable MCLK */
> +	ret = clk_set_rate(priv->mclk, 24000000);
> +	if (ret < 0) {
> +		dev_err(card->dev, "Can't set rate for mclk, err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(priv->mclk);
> +	if (ret < 0) {
> +		dev_err(card->dev, "Can't enable mclk, err: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Enable SCLK */
> +	ret = clk_set_rate(priv->sclk, 3072000);

why do we need both mclk and sclk for this codec? One should be enough for
codec to sync

> +	if (ret < 0) {
> +		dev_err(card->dev, "Can't set rate for sclk, err: %d\n", ret);
> +		clk_disable_unprepare(priv->mclk);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(priv->sclk);
> +	if (ret < 0) {
> +		dev_err(card->dev, "Can't enable sclk, err: %d\n", ret);
> +		clk_disable_unprepare(priv->mclk);
> +	}
> +
> +	return 0;

this looks wrong to me, shouldn't this be return ret;

> @@ -647,6 +710,7 @@ static int kabylake_audio_probe(struct platform_device *pdev)
>  {
>  	struct kbl_codec_private *ctx;
>  	struct skl_machine_pdata *pdata;
> +	int ret = 0;
>  
>  	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
>  	if (!ctx)
> @@ -662,6 +726,34 @@ static int kabylake_audio_probe(struct platform_device *pdev)
>  		dmic_constraints = pdata->dmic_num == 2 ?
>  			&constraints_dmic_2ch : &constraints_dmic_channels;
>  
> +	ctx->mclk = devm_clk_get(&pdev->dev, "ssp1_mclk");
> +	if (IS_ERR(ctx->mclk)) {
> +		ret = PTR_ERR(ctx->mclk);
> +		if (ret == -ENOENT) {
> +			dev_info(&pdev->dev,
> +				"Failed to get ssp1_mclk, defer probe\n");

can you check this, I think driver core emits warnings as well, so do we
need it here?

> +			return -EPROBE_DEFER;
> +		}
> +
> +		dev_err(&pdev->dev, "Failed to get ssp1_mclk with err:%d\n",
> +								ret);
> +		return ret;
> +	}
> +
> +	ctx->sclk = devm_clk_get(&pdev->dev, "ssp1_sclk");
> +	if (IS_ERR(ctx->sclk)) {
> +		ret = PTR_ERR(ctx->sclk);
> +		if (ret == -ENOENT) {
> +			dev_info(&pdev->dev,
> +				"Failed to get ssp1_sclk, defer probe\n");

shouldn't the mclk ref be given back?

-- 
~Vinod

Re: [PATCH v2] ASoC: Intel: eve: Enable mclk and ssp sclk early

[Vinod Koul]

On Sun, Sep 10, 2017 at 11:03:20AM +0530, Naveen M wrote:
> rt5663 needs mclk/sclk early to synchronize its internal clocks.
> Clocks are enabled in hw_params and disabled in platform_clk_control
> as the codec requires mclk/bclk need to turned ON before set_sysclk
> call for a successful clock synchronization.

ah there was a v2, sorry missed that.

> 
> Depends-on: patch series ("ASoC: Intel: Skylake: Add a clk driver to
> enable ssp clks early")
> ASoC: Intel: kbl: Enable mclk and ssp sclk early
> ASoC: Intel: Skylake: Modify skl_dsp_set_dma_control API arguments
> ASoC: Intel: Skylake: Parse nhlt to populate clock information
> ASoC: Intel: Skylake: Prepare DMA control IPC to enable/disable clock
> ASoC: Intel: Skylake: Register clock device and ops
> ASoC: Intel: Skylake: Add ssp clock driver

this is good info but is useless for applying
> 
> Signed-off-by: Naveen M <naveen.m@intel.com>
> Signed-off-by: Harsha Priya <harshapriya.n@intel.com>
> 
> V2: Add Depends-on

you have removed the dashes after s-o-b, so when I apply this patch the
depends on, V2 also get applied.

It is go d practice to apply the .patch file yourself after hand editing and
seeing if the output is something you really want.

Can you fix this along with comments on V1.
-- 
~Vinod