2021-03-15 17:34:14

by Sameer Pujar

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

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


2021-03-15 17:34:45

by Sameer Pujar

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

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 <[email protected]>
Suggested-by: Mark Brown <[email protected]>
Suggested-by: Michael Walle <[email protected]>
Fixes: 1e30f642cf29 ("ASoC: simple-card-utils: Fix device module clock")
Signed-off-by: Sameer Pujar <[email protected]>
---
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

2021-03-15 17:49:04

by Michael Walle

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

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 <[email protected]>
> Suggested-by: Mark Brown <[email protected]>
> Suggested-by: Michael Walle <[email protected]>
> Fixes: 1e30f642cf29 ("ASoC: simple-card-utils: Fix device module
> clock")
> Signed-off-by: Sameer Pujar <[email protected]>

Thanks!

Tested-by: Michael Walle <[email protected]>

-michael

2021-03-15 18:44:00

by Sameer Pujar

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

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 <[email protected]>
Suggested-by: Michael Walle <[email protected]>
Signed-off-by: Sameer Pujar <[email protected]>
---
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

2021-03-15 23:12:57

by Mark Brown

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

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!


Attachments:
(No filename) (523.00 B)
signature.asc (499.00 B)
Download all attachments

2021-03-16 21:18:56

by Mark Brown

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

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