Re: [PATCH v4 13/19] ASoC: tegra: Add fallback implementation for audio mclk

From: Sowjanya Komatineni
Date: Wed Dec 18 2019 - 11:32:14 EST



On 12/18/19 8:29 AM, Dmitry Osipenko wrote:
18.12.2019 18:43, Sowjanya Komatineni ÐÐÑÐÑ:
On 12/17/19 11:31 PM, Dmitry Osipenko wrote:
18.12.2019 10:22, Dmitry Osipenko ÐÐÑÐÑ:
18.12.2019 10:14, Sowjanya Komatineni ÐÐÑÐÑ:
On 12/17/19 11:01 PM, Dmitry Osipenko wrote:
18.12.2019 09:59, Dmitry Osipenko ÐÐÑÐÑ:
17.12.2019 23:04, Sowjanya Komatineni ÐÐÑÐÑ:
mclk is from clk_out_1 which is part of Tegra PMC block and pmc
clocks
are moved to Tegra PMC driver with pmc as clock provider and
using pmc
clock ids.

New device tree uses clk_out_1 from pmc clock provider.

So, this patch adds implementation for mclk fallback to extern1 when
retrieving mclk returns -ENOENT to be backward compatible of new
device
tree with older kernels.

Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx>
---
ÂÂ sound/soc/tegra/tegra_asoc_utils.c | 11 ++++++++++-
ÂÂ 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/sound/soc/tegra/tegra_asoc_utils.c
b/sound/soc/tegra/tegra_asoc_utils.c
index fe9ca8acd0fb..1b88c6043082 100644
--- a/sound/soc/tegra/tegra_asoc_utils.c
+++ b/sound/soc/tegra/tegra_asoc_utils.c
@@ -191,7 +191,16 @@ int tegra_asoc_utils_init(struct
tegra_asoc_utils_data *data,
ÂÂÂÂÂÂ data->clk_cdev1 = devm_clk_get(dev, "mclk");
ÂÂÂÂÂÂ if (IS_ERR(data->clk_cdev1)) {
ÂÂÂÂÂÂÂÂÂÂ dev_err(data->dev, "Can't retrieve clk cdev1\n");
-ÂÂÂÂÂÂÂ return PTR_ERR(data->clk_cdev1);
+ÂÂÂÂÂÂÂ if (PTR_ERR(data->clk_cdev1) != -ENOENT)
+ÂÂÂÂÂÂÂÂÂÂÂ return PTR_ERR(data->clk_cdev1);
+ÂÂÂÂÂÂÂ /* Fall back to extern1 */
+ÂÂÂÂÂÂÂ data->clk_cdev1 = devm_clk_get(dev, "extern1");
+ÂÂÂÂÂÂÂ if (IS_ERR(data->clk_cdev1)) {
+ÂÂÂÂÂÂÂÂÂÂÂ dev_err(data->dev, "Can't retrieve clk extern1\n");
+ÂÂÂÂÂÂÂÂÂÂÂ return PTR_ERR(data->clk_cdev1);
+ÂÂÂÂÂÂÂ }
+
+ÂÂÂÂÂÂÂ dev_err(data->dev, "Falling back to extern1\n");
ÂÂÂÂÂÂ }
ÂÂ ÂÂÂÂÂ /*

[ÂÂÂ 1.769091] ------------[ cut here ]------------
[ÂÂÂ 1.769249] WARNING: CPU: 2 PID: 1 at drivers/clk/clk.c:954
clk_core_disable+0xa5/0x1d4
[ÂÂÂ 1.769330] clk_out_1 already disabled
[ÂÂÂ 1.769459] Modules linked in:
[ÂÂÂ 1.769541] CPU: 2 PID: 1 Comm: swapper/0 Not tainted
5.5.0-rc1-next-20191213-00167-g6b9fbcdac8f3-dirty #266
[ÂÂÂ 1.769676] Hardware name: NVIDIA Tegra SoC (Flattened Device
Tree)
[ÂÂÂ 1.769775] [<c010e4bd>] (unwind_backtrace) from [<c010a0fd>]
(show_stack+0x11/0x14)
[ÂÂÂ 1.769918] [<c010a0fd>] (show_stack) from [<c09a37b1>]
(dump_stack+0x85/0x94)
[ÂÂÂ 1.770061] [<c09a37b1>] (dump_stack) from [<c011f3d1>]
(__warn+0xc1/0xc4)
[ÂÂÂ 1.770144] [<c011f3d1>] (__warn) from [<c011f691>]
(warn_slowpath_fmt+0x61/0x78)
[ÂÂÂ 1.770285] [<c011f691>] (warn_slowpath_fmt) from [<c04a0e7d>]
(clk_core_disable+0xa5/0x1d4)
[ÂÂÂ 1.770427] [<c04a0e7d>] (clk_core_disable) from [<c04a0fc3>]
(clk_core_disable_lock+0x17/0x20)
[ÂÂÂ 1.770516] [<c04a0fc3>] (clk_core_disable_lock) from [<c07792bb>]
(tegra_asoc_utils_set_rate+0x53/0x208)
[ÂÂÂ 1.770662] [<c07792bb>] (tegra_asoc_utils_set_rate) from
[<c077b8c5>] (tegra_rt5640_probe+0xd5/0x128)
[ÂÂÂ 1.770808] [<c077b8c5>] (tegra_rt5640_probe) from [<c0555eb7>]
(platform_drv_probe+0x33/0x68)
[ÂÂÂ 1.770958] [<c0555eb7>] (platform_drv_probe) from [<c055471d>]
(really_probe+0x14d/0x240)
[ÂÂÂ 1.771099] [<c055471d>] (really_probe) from [<c055493f>]
(driver_probe_device+0x43/0x11c)
[ÂÂÂ 1.771187] [<c055493f>] (driver_probe_device) from [<c0554b25>]
(device_driver_attach+0x3d/0x40)
[ÂÂÂ 1.771328] [<c0554b25>] (device_driver_attach) from [<c0554b5f>]
(__driver_attach+0x37/0x78)
[ÂÂÂ 1.771469] [<c0554b5f>] (__driver_attach) from [<c05532fb>]
(bus_for_each_dev+0x43/0x6c)
[ÂÂÂ 1.771609] [<c05532fb>] (bus_for_each_dev) from [<c0553e0f>]
(bus_add_driver+0xe3/0x148)
[ÂÂÂ 1.771692] [<c0553e0f>] (bus_add_driver) from [<c055531d>]
(driver_register+0x39/0xa0)
[ÂÂÂ 1.771833] [<c055531d>] (driver_register) from [<c0102c2f>]
(do_one_initcall+0x43/0x1bc)
[ÂÂÂ 1.771979] [<c0102c2f>] (do_one_initcall) from [<c1000ce5>]
(kernel_init_freeable+0x121/0x194)
[ÂÂÂ 1.772129] [<c1000ce5>] (kernel_init_freeable) from [<c09b40e9>]
(kernel_init+0xd/0xd0)
[ÂÂÂ 1.772215] [<c09b40e9>] (kernel_init) from [<c01010bd>]
(ret_from_fork+0x11/0x34)
[ÂÂÂ 1.772349] Exception stack(0xde907fb0 to 0xde907ff8)

Although, that's probably related to the "ASoC: tegra: Add initial
parent configuration for audio mclk".

Actually I see these warnings of already unprepared and already
disabled
for pll_a, pll_a_out0, and clk_out_1 even without this whole patch
series as well.

I think its from tegra_asoc_utils_set_rate() doing
clk_disable_unprepare
and these clocks are already unprepared and disabled so its just
warning
from clk_core_unprepare and clk_core_disable.
Doesn't happen for me without this series.
I looked at wrong log, right earlier clock driver keeps them enabled so
asoc_utils_set_rate() disables the clock fine but now enabling audio
clock should be done in asoc_utils_init() to let the
clk_disable_unprepare from asoc_utils_set_rate not to show this warning.

But actually we don't need to have clock enabled in asoc_utils_init
prior to invoking asoc_utils_set_rate from utils_init and its just warns
during sound driver probe because clock is already in disabled state. At
same time it doesn't harm to have it kept enabled in utils_init. So will
keep it enabled in asoc_utils_init to prevent this warning to show up.

But sound works with both old/new device-trees.
The rule of thumb: don't enable anything when it shouldn't be enabled.
If clocks are disabled at the time of drivers probe, then drivers
shouldn't disable the clocks.
Will fix asoc_utils_set_rate to disable only when its enabled.