[PATCH 0/2] Do not handle MCLK device clock in simple-card-utils

[PATCH 0/2] Do not handle MCLK device clock in simple-card-utils

[Sameer Pujar]

With commit 1e30f642cf29 ("ASoC: simple-card-utils: Fix device module clock")
simple-card-utils can control MCLK clock for rate updates or enable/disable.
But this is breaking some platforms where it is expected that codec drivers
would actually handle the MCLK clock. One such example is following platform.
  - "arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts"

In above case codec, wm8904, is using internal PLL and configures sysclk
based on fixed MCLK input. In such cases it is expected that, required PLL
output or sysclk, is just passed via set_sysclk() callback and card driver
need not actually update MCLK rate. Instead, codec can take ownership of
this clock and do the necessary configuration.

So the original commit is reverted and codec driver for rt5659 is updated
to fix my board which has this codec.

Sameer Pujar (2):
  ASoC: simple-card-utils: Do not handle device clock
  ASoC: rt5659: Update MCLK rate in set_sysclk()

 sound/soc/codecs/rt5659.c             |  5 +++++
 sound/soc/generic/simple-card-utils.c | 13 +++++++------
 2 files changed, 12 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH 1/2] ASoC: simple-card-utils: Do not handle device clock

[Sameer Pujar]

This reverts commit 1e30f642cf29 ("ASoC: simple-card-utils: Fix device
module clock"). The original patch ended up breaking following platform,
which depends on set_sysclk() to configure internal PLL on wm8904 codec
and expects simple-card-utils to not update the MCLK rate.
 - "arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts"

It would be best if codec takes care of setting MCLK clock via DAI
set_sysclk() callback.

Reported-by: Michael Walle <michael@walle.cc>
Suggested-by: Mark Brown <broonie@kernel.org>
Suggested-by: Michael Walle <michael@walle.cc>
Fixes: 1e30f642cf29 ("ASoC: simple-card-utils: Fix device module clock")
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/soc/generic/simple-card-utils.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index ab31045..6cada4c 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -172,15 +172,16 @@ int asoc_simple_parse_clk(struct device *dev,
 	 *  or device's module clock.
 	 */
 	clk = devm_get_clk_from_child(dev, node, NULL);
-	if (IS_ERR(clk))
-		clk = devm_get_clk_from_child(dev, dlc->of_node, NULL);
-
 	if (!IS_ERR(clk)) {
-		simple_dai->clk = clk;
 		simple_dai->sysclk = clk_get_rate(clk);
-	} else if (!of_property_read_u32(node, "system-clock-frequency",
-					 &val)) {
+
+		simple_dai->clk = clk;
+	} else if (!of_property_read_u32(node, "system-clock-frequency", &val)) {
 		simple_dai->sysclk = val;
+	} else {
+		clk = devm_get_clk_from_child(dev, dlc->of_node, NULL);
+		if (!IS_ERR(clk))
+			simple_dai->sysclk = clk_get_rate(clk);
 	}
 
 	if (of_property_read_bool(node, "system-clock-direction-out"))
-- 
2.7.4

[PATCH 2/2] ASoC: rt5659: Update MCLK rate in set_sysclk()

[Sameer Pujar]

Simple-card/audio-graph-card drivers do not handle MCLK clock when it
is specified in the codec device node. The expectation here is that,
the codec should actually own up the MCLK clock and do necessary setup
in the driver.

Suggested-by: Mark Brown <broonie@kernel.org>
Suggested-by: Michael Walle <michael@walle.cc>
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/soc/codecs/rt5659.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/codecs/rt5659.c b/sound/soc/codecs/rt5659.c
index b787515..0af9601 100644
--- a/sound/soc/codecs/rt5659.c
+++ b/sound/soc/codecs/rt5659.c
@@ -3430,12 +3430,17 @@ static int rt5659_set_component_sysclk(struct snd_soc_component *component, int
 {
 	struct rt5659_priv *rt5659 = snd_soc_component_get_drvdata(component);
 	unsigned int reg_val = 0;
+	int ret;
 
 	if (freq == rt5659->sysclk && clk_id == rt5659->sysclk_src)
 		return 0;
 
 	switch (clk_id) {
 	case RT5659_SCLK_S_MCLK:
+		ret = clk_set_rate(rt5659->mclk, freq);
+		if (ret)
+			return ret;
+
 		reg_val |= RT5659_SCLK_SRC_MCLK;
 		break;
 	case RT5659_SCLK_S_PLL1:
-- 
2.7.4

Re: [PATCH 1/2] ASoC: simple-card-utils: Do not handle device clock

[Michael Walle]

Am 2021-03-15 18:31, schrieb Sameer Pujar:
> This reverts commit 1e30f642cf29 ("ASoC: simple-card-utils: Fix device
> module clock"). The original patch ended up breaking following 
> platform,
> which depends on set_sysclk() to configure internal PLL on wm8904 codec
> and expects simple-card-utils to not update the MCLK rate.
>  - 
> "arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts"
> 
> It would be best if codec takes care of setting MCLK clock via DAI
> set_sysclk() callback.
> 
> Reported-by: Michael Walle <michael@walle.cc>
> Suggested-by: Mark Brown <broonie@kernel.org>
> Suggested-by: Michael Walle <michael@walle.cc>
> Fixes: 1e30f642cf29 ("ASoC: simple-card-utils: Fix device module 
> clock")
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>

Thanks!

Tested-by: Michael Walle <michael@walle.cc>

-michael

Re: [PATCH 0/2] Do not handle MCLK device clock in simple-card-utils

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 515 bytes --]

On Mon, Mar 15, 2021 at 11:01:30PM +0530, Sameer Pujar wrote:
> With commit 1e30f642cf29 ("ASoC: simple-card-utils: Fix device module clock")
> simple-card-utils can control MCLK clock for rate updates or enable/disable.
> But this is breaking some platforms where it is expected that codec drivers
> would actually handle the MCLK clock. One such example is following platform.
>   - "arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts"

Thanks both Sameer and Michael for getting this resolved!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/2] Do not handle MCLK device clock in simple-card-utils

[Mark Brown]

On Mon, 15 Mar 2021 23:01:30 +0530, Sameer Pujar wrote:
> With commit 1e30f642cf29 ("ASoC: simple-card-utils: Fix device module clock")
> simple-card-utils can control MCLK clock for rate updates or enable/disable.
> But this is breaking some platforms where it is expected that codec drivers
> would actually handle the MCLK clock. One such example is following platform.
>   - "arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts"
> 
> In above case codec, wm8904, is using internal PLL and configures sysclk
> based on fixed MCLK input. In such cases it is expected that, required PLL
> output or sysclk, is just passed via set_sysclk() callback and card driver
> need not actually update MCLK rate. Instead, codec can take ownership of
> this clock and do the necessary configuration.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/2] ASoC: simple-card-utils: Do not handle device clock
      commit: 8ca88d53351cc58d535b2bfc7386835378fb0db2
[2/2] ASoC: rt5659: Update MCLK rate in set_sysclk()
      commit: dbf54a9534350d6aebbb34f5c1c606b81a4f35dd

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