mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] ASoC: rt5631: Support an MCLK clock from the SoC
@ 2026-10-25 14:53 Romain TISSERAND
  2026-10-25 14:53 ` [PATCH 1/2] ASoC: dt-bindings: realtek,rt5631: Add the optional MCLK Romain TISSERAND
  2026-10-25 14:53 ` [PATCH 2/2] ASoC: codecs: rt5631: Enable " Romain TISSERAND
  0 siblings, 2 replies; 4+ messages in thread
From: Romain TISSERAND @ 2026-10-25 14:53 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Oder Chiou, Animesh Agarwal, Jaroslav Kysela, Takashi Iwai,
	linux-sound, devicetree, linux-kernel

The RT5631 driver doesn't take a master clock, so when MCLK comes from a
SoC clock output, that clock is turned off at boot as unused. This
series adds an optional "mclk" clock to the binding and enables it in
the driver while the codec is active, as rt5616 does.

The board that needs it is the GPD XD (RK3288), whose device tree is not
upstream yet. Its MCLK is i2s0_clkout: handed to simple-audio-card, the
mux picked the 12 MHz oscillator for 12.288 MHz (wrong pitch); held by
the codec, it follows the I2S clock, as on the RK3288 boards with ES8388
and MAX98090 codecs.

Testing: dt_binding_check for realtek,rt5631.yaml; W=1 builds for arm
(multi_v7) and arm64 (defconfig) with SND_SOC_RT5631=y and =m, no
warnings. Tested live on the GPD XD (RT5631 on simple-audio-card, mclk-fs
256) on 6.18.54 with this change backported (the bias level read uses
snd_soc_component_get_bias_level() there): i2s0_clkout stays on the I2S
clock at 11.2896 MHz for 44.1 kHz and 12.288 MHz for 48 kHz, and both
play at the right pitch. Boards without an "mclk" clock are not affected
(clk_prepare_enable() on a NULL clock does nothing); not tested on other
boards.

Tool disclosure: this series was prepared with Claude Opus 5.5
(Anthropic) as a coding assistant, in an interactive session where I
asked it to fix the pitch problem without the RK3288 clock change that
was refused in 2014. It traced the clock ordering in simple-audio-card,
proposed taking the clock in the codec driver following rt5616, and
drafted the patches and the messages. I reviewed and understand them,
and tested them on the GPD XD as described above.


Romain TISSERAND (2):
  ASoC: dt-bindings: realtek,rt5631: Add the optional MCLK
  ASoC: codecs: rt5631: Enable the optional MCLK

 .../bindings/sound/realtek,rt5631.yaml          |  8 ++++++++
 sound/soc/codecs/rt5631.c                       | 17 ++++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)


base-commit: cc3fc99f1bce93c3ae3771ef98cb91a566da857e
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] ASoC: dt-bindings: realtek,rt5631: Add the optional MCLK
  2026-10-25 14:53 [PATCH 0/2] ASoC: rt5631: Support an MCLK clock from the SoC Romain TISSERAND
@ 2026-10-25 14:53 ` Romain TISSERAND
  2026-10-25 14:53 ` [PATCH 2/2] ASoC: codecs: rt5631: Enable " Romain TISSERAND
  1 sibling, 0 replies; 4+ messages in thread
From: Romain TISSERAND @ 2026-10-25 14:53 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Oder Chiou, Animesh Agarwal, Jaroslav Kysela, Takashi Iwai,
	linux-sound, devicetree, linux-kernel

The RT5631 runs from a master clock on its MCLK pin. On boards where that
clock comes from a SoC clock output, the codec node needs to name it so
the driver can keep it running while the codec is in use.

Document an optional "mclk" clock, as the realtek,rt5616 binding does.

Signed-off-by: Romain TISSERAND <romain.tisserand@gmail.com>
---
 .../devicetree/bindings/sound/realtek,rt5631.yaml         | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5631.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5631.yaml
index 747a731c44..03ee25e1eb 100644
--- a/Documentation/devicetree/bindings/sound/realtek,rt5631.yaml
+++ b/Documentation/devicetree/bindings/sound/realtek,rt5631.yaml
@@ -45,6 +45,14 @@ properties:
   reg:
     maxItems: 1
 
+  clocks:
+    items:
+      - description: Master clock to the CODEC
+
+  clock-names:
+    items:
+      - const: mclk
+
   port:
     $ref: audio-graph-port.yaml#
     unevaluatedProperties: false
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] ASoC: codecs: rt5631: Enable the optional MCLK
  2026-10-25 14:53 [PATCH 0/2] ASoC: rt5631: Support an MCLK clock from the SoC Romain TISSERAND
  2026-10-25 14:53 ` [PATCH 1/2] ASoC: dt-bindings: realtek,rt5631: Add the optional MCLK Romain TISSERAND
@ 2026-10-25 14:53 ` Romain TISSERAND
       [not found]   ` <sashiko-outbox-184258@kernel.org>
  1 sibling, 1 reply; 4+ messages in thread
From: Romain TISSERAND @ 2026-10-25 14:53 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Oder Chiou, Animesh Agarwal, Jaroslav Kysela, Takashi Iwai,
	linux-sound, devicetree, linux-kernel

When the RT5631 master clock comes from a SoC clock output, nothing in
the codec driver keeps it running, and the clock framework turns it off
at boot as an unused clock.

Get an optional "mclk" clock and enable it while the codec is active,
from the PREPARE bias level, as rt5616 does. The rate is left to the
CPU DAI.

On the GPD XD (RK3288) MCLK is i2s0_clkout, a mux between the I2S clock
and a 12 MHz oscillator. Handed to simple-audio-card instead, it is set
to the stream rate before the CPU DAI sets the I2S clock, so for
12.288 MHz the mux picks the oscillator and audio plays at the wrong
pitch. Held by the codec, it follows the I2S clock.

Signed-off-by: Romain TISSERAND <romain.tisserand@gmail.com>
---
 sound/soc/codecs/rt5631.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt5631.c b/sound/soc/codecs/rt5631.c
index f97fd24e42..82d5e8b9c8 100644
--- a/sound/soc/codecs/rt5631.c
+++ b/sound/soc/codecs/rt5631.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/init.h>
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/pm.h>
 #include <linux/i2c.h>
@@ -27,6 +28,7 @@
 
 struct rt5631_priv {
 	struct regmap *regmap;
+	struct clk *mclk;
 	int codec_version;
 	int master;
 	int sysclk;
@@ -1536,10 +1538,19 @@ static int rt5631_set_bias_level(struct snd_soc_component *component,
 {
 	struct rt5631_priv *rt5631 = snd_soc_component_get_drvdata(component);
 	struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(component);
+	int ret;
 
 	switch (level) {
-	case SND_SOC_BIAS_ON:
 	case SND_SOC_BIAS_PREPARE:
+		if (snd_soc_dapm_get_bias_level(dapm) == SND_SOC_BIAS_ON) {
+			clk_disable_unprepare(rt5631->mclk);
+		} else {
+			ret = clk_prepare_enable(rt5631->mclk);
+			if (ret)
+				return ret;
+		}
+		fallthrough;
+	case SND_SOC_BIAS_ON:
 		snd_soc_component_update_bits(component, RT5631_PWR_MANAG_ADD2,
 			RT5631_PWR_MICBIAS1_VOL | RT5631_PWR_MICBIAS2_VOL,
 			RT5631_PWR_MICBIAS1_VOL | RT5631_PWR_MICBIAS2_VOL);
@@ -1579,6 +1590,10 @@ static int rt5631_probe(struct snd_soc_component *component)
 	struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(component);
 	unsigned int val;
 
+	rt5631->mclk = devm_clk_get_optional(component->dev, "mclk");
+	if (IS_ERR(rt5631->mclk))
+		return PTR_ERR(rt5631->mclk);
+
 	val = rt5631_read_index(component, RT5631_ADDA_MIXER_INTL_REG3);
 	if (val & 0x0002)
 		rt5631->codec_version = 1;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] ASoC: codecs: rt5631: Enable the optional MCLK
       [not found]   ` <sashiko-outbox-184258@kernel.org>
@ 2026-10-25 15:24     ` Romain TISSERAND
  0 siblings, 0 replies; 4+ messages in thread
From: Romain TISSERAND @ 2026-10-25 15:24 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Mark Brown, Liam Girdwood, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Oder Chiou, Animesh Agarwal, Jaroslav Kysela,
	Takashi Iwai, linux-sound, devicetree, linux-kernel

On Sun, 25 Oct 2026 15:12:00 +0000, sashiko-bot@kernel.org wrote:
> Should devm_clk_get_optional() be called in rt5631_i2c_probe() in
> sound/soc/codecs/rt5631.c instead of the ASoC component probe callback
> rt5631_probe() (similar to how rt5616_i2c_probe() in
> sound/soc/codecs/rt5616.c acquires its mclk)?

Right: in the component probe a deferral is lost and a rebind leaks the
clock reference. rt5616 moved it to the I2C probe in commit b5c9df40ca56
("ASoC: codecs: rt5616: Move mclk acquisition to the i2c probe"); v2 will
do the same in rt5631_i2c_probe(). Please don't apply v1.

I'll send v2 in about a week.

Romain

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-25 15:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-25 14:53 [PATCH 0/2] ASoC: rt5631: Support an MCLK clock from the SoC Romain TISSERAND
2026-10-25 14:53 ` [PATCH 1/2] ASoC: dt-bindings: realtek,rt5631: Add the optional MCLK Romain TISSERAND
2026-10-25 14:53 ` [PATCH 2/2] ASoC: codecs: rt5631: Enable " Romain TISSERAND
     [not found]   ` <sashiko-outbox-184258@kernel.org>
2026-10-25 15:24     ` Romain TISSERAND

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®