[PATCH v1 0/2] ASoC: nau8822: add MCLK support

[PATCH v1 0/2] ASoC: nau8822: add MCLK support

[Andrejs Cainikovs]

From: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>

This change adds local MCLK handling, which would cover a case when a
reference audio clock is present in a system, but is not allowed to be
changed, see [1].

[1]: https://lore.kernel.org/all/ZfBdxrzX3EnPuGOn@ediswmail9.ad.cirrus.com/

Andrejs Cainikovs (2):
  ASoC: nau8822: move nau8822_set_dai_sysclk()
  ASoC: nau8822: add MCLK support

 sound/soc/codecs/nau8822.c | 69 ++++++++++++++++++++++++++++----------
 sound/soc/codecs/nau8822.h |  1 +
 2 files changed, 53 insertions(+), 17 deletions(-)

-- 
2.34.1

[PATCH v1 1/2] ASoC: nau8822: move nau8822_set_dai_sysclk()

[Andrejs Cainikovs]

From: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>

Next commit in series makes a change which calls nau8822_set_pll() from
nau8822_set_dai_sysclk(). Moving latter after the former would avoid a
forward declaration, and this is exactly what this change does.

Signed-off-by: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>
---
 sound/soc/codecs/nau8822.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/sound/soc/codecs/nau8822.c b/sound/soc/codecs/nau8822.c
index 7199d734c79f..1046801a41ef 100644
--- a/sound/soc/codecs/nau8822.c
+++ b/sound/soc/codecs/nau8822.c
@@ -612,20 +612,6 @@ static const struct snd_soc_dapm_route nau8822_dapm_routes[] = {
 	{"Right DAC", NULL, "Digital Loopback"},
 };
 
-static int nau8822_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
-				 unsigned int freq, int dir)
-{
-	struct snd_soc_component *component = dai->component;
-	struct nau8822 *nau8822 = snd_soc_component_get_drvdata(component);
-
-	nau8822->div_id = clk_id;
-	nau8822->sysclk = freq;
-	dev_dbg(component->dev, "master sysclk %dHz, source %s\n", freq,
-		clk_id == NAU8822_CLK_PLL ? "PLL" : "MCLK");
-
-	return 0;
-}
-
 static int nau8822_calc_pll(unsigned int pll_in, unsigned int fs,
 				struct nau8822_pll *pll_param)
 {
@@ -782,6 +768,20 @@ static int nau8822_set_pll(struct snd_soc_dai *dai, int pll_id, int source,
 	return 0;
 }
 
+static int nau8822_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
+				 unsigned int freq, int dir)
+{
+	struct snd_soc_component *component = dai->component;
+	struct nau8822 *nau8822 = snd_soc_component_get_drvdata(component);
+
+	nau8822->div_id = clk_id;
+	nau8822->sysclk = freq;
+	dev_dbg(component->dev, "master sysclk %dHz, source %s\n", freq,
+		clk_id == NAU8822_CLK_PLL ? "PLL" : "MCLK");
+
+	return 0;
+}
+
 static int nau8822_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 {
 	struct snd_soc_component *component = dai->component;
-- 
2.34.1

[PATCH v1 2/2] ASoC: nau8822: add MCLK support

[Andrejs Cainikovs]

From: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>

This change adds MCLK clock handling directly within driver.
When used in combination with simple-audio-card, and mclk-fs is set,
simple-audio-card will change MCLK frequency before configuring PLL.
In some cases, however, MCLK reference clock should be static (see [1]),
which means it needs to be moved away from simple-audio-card.

[1]: https://lore.kernel.org/all/ZfBdxrzX3EnPuGOn@ediswmail9.ad.cirrus.com/

Signed-off-by: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>
---
 sound/soc/codecs/nau8822.c | 43 ++++++++++++++++++++++++++++++++++----
 sound/soc/codecs/nau8822.h |  1 +
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/nau8822.c b/sound/soc/codecs/nau8822.c
index 1046801a41ef..01db182434a4 100644
--- a/sound/soc/codecs/nau8822.c
+++ b/sound/soc/codecs/nau8822.c
@@ -14,6 +14,7 @@
 #include <linux/moduleparam.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/pm.h>
 #include <linux/i2c.h>
@@ -773,11 +774,26 @@ static int nau8822_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
 {
 	struct snd_soc_component *component = dai->component;
 	struct nau8822 *nau8822 = snd_soc_component_get_drvdata(component);
+	unsigned long mclk_freq;
 
 	nau8822->div_id = clk_id;
 	nau8822->sysclk = freq;
+
+	if (nau8822->mclk) {
+		mclk_freq = clk_get_rate(nau8822->mclk);
+		if (mclk_freq != freq) {
+			int ret = nau8822_set_pll(dai, NAU8822_CLK_MCLK,
+				NAU8822_CLK_MCLK, mclk_freq, freq);
+			if (ret) {
+				dev_err(component->dev, "Failed to set PLL\n");
+				return ret;
+			}
+			nau8822->div_id = NAU8822_CLK_PLL;
+		}
+	}
+
 	dev_dbg(component->dev, "master sysclk %dHz, source %s\n", freq,
-		clk_id == NAU8822_CLK_PLL ? "PLL" : "MCLK");
+		nau8822->div_id == NAU8822_CLK_PLL ? "PLL" : "MCLK");
 
 	return 0;
 }
@@ -848,7 +864,7 @@ static int nau8822_hw_params(struct snd_pcm_substream *substream,
 {
 	struct snd_soc_component *component = dai->component;
 	struct nau8822 *nau8822 = snd_soc_component_get_drvdata(component);
-	int val_len = 0, val_rate = 0;
+	int div = 0, val_len = 0, val_rate = 0;
 	unsigned int ctrl_val, bclk_fs, bclk_div;
 
 	/* make BCLK and LRC divide configuration if the codec as master. */
@@ -915,8 +931,10 @@ static int nau8822_hw_params(struct snd_pcm_substream *substream,
 	/* If the master clock is from MCLK, provide the runtime FS for driver
 	 * to get the master clock prescaler configuration.
 	 */
-	if (nau8822->div_id == NAU8822_CLK_MCLK)
-		nau8822_config_clkdiv(dai, 0, params_rate(params));
+	if (nau8822->div_id != NAU8822_CLK_MCLK)
+		div = nau8822->pll.mclk_scaler;
+
+	nau8822_config_clkdiv(dai, div, params_rate(params));
 
 	return 0;
 }
@@ -940,12 +958,23 @@ static int nau8822_mute(struct snd_soc_dai *dai, int mute, int direction)
 static int nau8822_set_bias_level(struct snd_soc_component *component,
 				 enum snd_soc_bias_level level)
 {
+	struct nau8822 *nau8822 = snd_soc_component_get_drvdata(component);
+
 	switch (level) {
 	case SND_SOC_BIAS_ON:
 	case SND_SOC_BIAS_PREPARE:
 		snd_soc_component_update_bits(component,
 			NAU8822_REG_POWER_MANAGEMENT_1,
 			NAU8822_REFIMP_MASK, NAU8822_REFIMP_80K);
+		if (nau8822->mclk) {
+			int ret = clk_prepare_enable(nau8822->mclk);
+
+			if (ret) {
+				dev_err(component->dev,
+					"Failed to enable MCLK: %d\n", ret);
+				return ret;
+			}
+		}
 		break;
 
 	case SND_SOC_BIAS_STANDBY:
@@ -973,6 +1002,7 @@ static int nau8822_set_bias_level(struct snd_soc_component *component,
 			NAU8822_REG_POWER_MANAGEMENT_2, 0);
 		snd_soc_component_write(component,
 			NAU8822_REG_POWER_MANAGEMENT_3, 0);
+		clk_disable_unprepare(nau8822->mclk);
 		break;
 	}
 
@@ -1125,6 +1155,11 @@ static int nau8822_i2c_probe(struct i2c_client *i2c)
 	}
 	i2c_set_clientdata(i2c, nau8822);
 
+	nau8822->mclk = devm_clk_get_optional(&i2c->dev, "mclk");
+	if (IS_ERR(nau8822->mclk))
+		return dev_err_probe(&i2c->dev, PTR_ERR(nau8822->mclk),
+			"Error getting mclk\n");
+
 	nau8822->regmap = devm_regmap_init_i2c(i2c, &nau8822_regmap_config);
 	if (IS_ERR(nau8822->regmap)) {
 		ret = PTR_ERR(nau8822->regmap);
diff --git a/sound/soc/codecs/nau8822.h b/sound/soc/codecs/nau8822.h
index 6ecd46e45923..13fe0a091e9e 100644
--- a/sound/soc/codecs/nau8822.h
+++ b/sound/soc/codecs/nau8822.h
@@ -215,6 +215,7 @@ struct nau8822_pll {
 struct nau8822 {
 	struct device *dev;
 	struct regmap *regmap;
+	struct clk *mclk;
 	struct nau8822_pll pll;
 	int sysclk;
 	int div_id;
-- 
2.34.1

Re: [PATCH v1 2/2] ASoC: nau8822: add MCLK support

[Francesco Dolcini]

Hello,

On Tue, Apr 09, 2024 at 02:17:19PM +0200, Andrejs Cainikovs wrote:
> From: Andrejs Cainikovs <andrejs.cainikovs@toradex.com>
> 
> This change adds MCLK clock handling directly within driver.
> When used in combination with simple-audio-card, and mclk-fs is set,
> simple-audio-card will change MCLK frequency before configuring PLL.
> In some cases, however, MCLK reference clock should be static (see [1]),
> which means it needs to be moved away from simple-audio-card.
> 
> [1]: https://lore.kernel.org/all/ZfBdxrzX3EnPuGOn@ediswmail9.ad.cirrus.com/


This seems buggy, it introduces the following error:

[ 8490.005519] ------------[ cut here ]------------
[ 8490.005527] audio_refclk:clk:157:10 already disabled
[ 8490.005585] WARNING: CPU: 0 PID: 858 at drivers/clk/clk.c:1038 clk_core_disable+0xb0/0xb8
[ 8490.005623] Modules linked in: 8021q garp stp mrp llc rfcomm af_alg usb_f_ncm u_ether ti_k3_r5_remoteproc virtio_rpmsg_bus btnxpuart rpmsg_ns rtc_ti_k3 ti_k3_m4_remoteproc ti_k3_common mwifiex_sdio mwifiex cfg80211 bluetooth ecdh_generic ecc tidss snd_soc_davinci_mcasp rfkill snd_soc_ti_udma snd_soc_ti_edma sa2ul drm_dma_helper snd_soc_ti_sdma mcrc ina2xx lontium_lt8916
[ 8490.005857] CPU: 0 PID: 858 Comm: rtcwake Tainted: G        W          6.1.80-15754-g2d31b2cc742b-dirty #38
[ 8490.005868] Hardware name: Toradex Verdin AM62 WB on Verdin Development Board (DT)
[ 8490.005876] pstate: 400000c5 (nZcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 8490.005886] pc : clk_core_disable+0xb0/0xb8
[ 8490.005897] lr : clk_core_disable+0xb0/0xb8
[ 8490.005907] sp : ffff80000a5a3960
[ 8490.005911] x29: ffff80000a5a3960 x28: ffff800009246000 x27: ffff800008df4690
[ 8490.005926] x26: ffff8000091dbcc0 x25: ffff800008df4000 x24: ffff8000091dbc18
[ 8490.005941] x23: ffff000002de5080 x22: 0000000000000000 x21: ffff000002f5fc80
[ 8490.005955] x20: ffff00000118df00 x19: ffff00000118df00 x18: ffff800000d15000
[ 8490.005970] x17: 0000000000000000 x16: 0000000000000000 x15: ffff00000605aac0
[ 8490.005984] x14: fffffffffffca828 x13: 0a64656c62617369 x12: 642079646165726c
[ 8490.005997] x11: 656820747563205b x10: 2d2d2d2d2d2d2d2d x9 : 00000000fffffffe
[ 8490.006012] x8 : ffff8000090c1260 x7 : ffff80000a5a3760 x6 : 00000000fffff267
[ 8490.006026] x5 : ffff00001da1ab60 x4 : 0000000000000000 x3 : 0000000000000027
[ 8490.006039] x2 : ffff00001da1ab68 x1 : ae3ede0e49d60900 x0 : 0000000000000000
[ 8490.006055] Call trace:
[ 8490.006060]  clk_core_disable+0xb0/0xb8
[ 8490.006072]  clk_core_disable_lock+0x24/0x40
[ 8490.006082]  clk_disable+0x20/0x30
[ 8490.006093]  nau8822_set_bias_level+0x68/0x1a0 [snd_soc_nau8822]
[ 8490.006123]  snd_soc_component_set_bias_level+0x28/0x78
[ 8490.006136]  snd_soc_dapm_force_bias_level+0x24/0x40
[ 8490.006149]  nau8822_suspend+0x28/0x48 [snd_soc_nau8822]
[ 8490.006167]  snd_soc_component_suspend+0x24/0x40
[ 8490.006176]  snd_soc_suspend+0x1bc/0x218
[ 8490.006185]  platform_pm_suspend+0x2c/0x78
[ 8490.006198]  dpm_run_callback+0x3c/0x88
[ 8490.006209]  __device_suspend+0x110/0x3b0
[ 8490.006219]  dpm_suspend+0x100/0x200
[ 8490.006228]  dpm_suspend_start+0x80/0xa0
[ 8490.006238]  suspend_devices_and_enter+0xc8/0x4e8
[ 8490.006252]  pm_suspend+0x1f8/0x268
[ 8490.006261]  state_store+0x8c/0x118
[ 8490.006270]  kobj_attr_store+0x18/0x30
[ 8490.006287]  sysfs_kf_write+0x44/0x58
[ 8490.006297]  kernfs_fop_write_iter+0x118/0x1a8
[ 8490.006310]  vfs_write+0x33c/0x438
[ 8490.006320]  ksys_write+0x6c/0xf8
[ 8490.006327]  __arm64_sys_write+0x1c/0x28
[ 8490.006335]  invoke_syscall+0x44/0x108
[ 8490.006348]  el0_svc_common.constprop.0+0xcc/0xf0
[ 8490.006358]  do_el0_svc+0x1c/0x28
[ 8490.006367]  el0_svc+0x28/0x98
[ 8490.006377]  el0t_64_sync_handler+0xb8/0xc0
[ 8490.006386]  el0t_64_sync+0x18c/0x190
[ 8490.006397] ---[ end trace 0000000000000000 ]---
[ 8490.006407] ------------[ cut here ]------------
[ 8490.006410] audio_refclk:clk:157:10 already unprepared
[ 8490.006447] WARNING: CPU: 0 PID: 858 at drivers/clk/clk.c:897 clk_core_unprepare+0xf0/0x110
[ 8490.006463] Modules linked in: 8021q garp stp mrp llc rfcomm af_alg usb_f_ncm u_ether ti_k3_r5_remoteproc virtio_rpmsg_bus btnxpuart rpmsg_ns rtc_ti_k3 ti_k3_m4_remoteproc ti_k3_common mwifiex_sdio mwifiex cfg80211 bluetooth ecdh_generic ecc tidss snd_soc_davinci_mcasp rfkill snd_soc_ti_udma snd_soc_ti_edma sa2ul drm_dma_helper snd_soc_ti_sdma mcrc ina2xx lontium_lt8916
[ 8490.006644] CPU: 0 PID: 858 Comm: rtcwake Tainted: G        W          6.1.80-15754-g2d31b2cc742b-dirty #38
[ 8490.006653] Hardware name: Toradex Verdin AM62 WB on Verdin Development Board (DT)
[ 8490.006658] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 8490.006667] pc : clk_core_unprepare+0xf0/0x110
[ 8490.006678] lr : clk_core_unprepare+0xf0/0x110
[ 8490.006688] sp : ffff80000a5a3970
[ 8490.006691] x29: ffff80000a5a3970 x28: ffff800009246000 x27: ffff800008df4690
[ 8490.006706] x26: ffff8000091dbcc0 x25: ffff800008df4000 x24: ffff8000091dbc18
[ 8490.006720] x23: ffff000002de5080 x22: 0000000000000000 x21: ffff000002f5fc80
[ 8490.006735] x20: 0000000000000000 x19: ffff00000118df00 x18: ffff800000d15000
[ 8490.006749] x17: 0000000000000000 x16: 0000000000000000 x15: ffff00000605aac0
[ 8490.006763] x14: fffffffffffcb568 x13: 0a64657261706572 x12: 706e752079646165
[ 8490.006777] x11: 656820747563205b x10: 2d2d2d2d2d2d2d2d x9 : 00000000fffffffe
[ 8490.006791] x8 : ffff8000090c1260 x7 : ffff80000a5a3770 x6 : 00000000fffff29a
[ 8490.006805] x5 : ffff00001da1ab60 x4 : 0000000000000000 x3 : 0000000000000027
[ 8490.006819] x2 : ffff00001da1ab68 x1 : ae3ede0e49d60900 x0 : 0000000000000000
[ 8490.006833] Call trace:
[ 8490.006836]  clk_core_unprepare+0xf0/0x110
[ 8490.006847]  clk_unprepare+0x2c/0x48
[ 8490.006855]  nau8822_set_bias_level+0x70/0x1a0 [snd_soc_nau8822]
[ 8490.006874]  snd_soc_component_set_bias_level+0x28/0x78
[ 8490.006883]  snd_soc_dapm_force_bias_level+0x24/0x40
[ 8490.006893]  nau8822_suspend+0x28/0x48 [snd_soc_nau8822]
[ 8490.006913]  snd_soc_component_suspend+0x24/0x40
[ 8490.006921]  snd_soc_suspend+0x1bc/0x218
[ 8490.006929]  platform_pm_suspend+0x2c/0x78
[ 8490.006939]  dpm_run_callback+0x3c/0x88
[ 8490.006948]  __device_suspend+0x110/0x3b0
[ 8490.006957]  dpm_suspend+0x100/0x200
[ 8490.006967]  dpm_suspend_start+0x80/0xa0
[ 8490.006976]  suspend_devices_and_enter+0xc8/0x4e8
[ 8490.006986]  pm_suspend+0x1f8/0x268
[ 8490.006994]  state_store+0x8c/0x118
[ 8490.007003]  kobj_attr_store+0x18/0x30
[ 8490.007014]  sysfs_kf_write+0x44/0x58
[ 8490.007022]  kernfs_fop_write_iter+0x118/0x1a8
[ 8490.007033]  vfs_write+0x33c/0x438
[ 8490.007041]  ksys_write+0x6c/0xf8
[ 8490.007047]  __arm64_sys_write+0x1c/0x28
[ 8490.007055]  invoke_syscall+0x44/0x108
[ 8490.007064]  el0_svc_common.constprop.0+0xcc/0xf0
[ 8490.007074]  do_el0_svc+0x1c/0x28
[ 8490.007083]  el0_svc+0x28/0x98
[ 8490.007091]  el0t_64_sync_handler+0xb8/0xc0
[ 8490.007100]  el0t_64_sync+0x18c/0x190
[ 8490.007108] ---[ end trace 0000000000000000 ]---


Francesco